New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
config/output: Search for output config fallbacks #8095
Conversation
aa76b6f
to
7378259
Compare
I have tested with:
On my poor intel, going through all those combinations takes roughly 230 ms. What I have not tested:
|
731312e
to
fb7844a
Compare
fb7844a
to
bb75acd
Compare
Re-arranged the search to do the VRR test last, which significantly speeds up matching when attempting to enable VRR when it doesn't work, without impacting tests where VRR is not an issue. |
461b899
to
c276b36
Compare
Hi, I've tried to track the issue and so far found that the slot for the affected display, after being acquired in
in the logs, line 3011: Reverting back to commit 125c743 brings back the display, compare starting with line 2928: Output of Also note that the DisplayLink connected display chooses a non-default mode: Not sure, (not tested/not confirmed) but this doesn't have to be DisplayLink specific behavior and may affect any display working with non-default mode. Regards |
Since you are commenting on this unmerged PR which implements a more aggressive search for possible configurations, did you try it to see if it improved the situation?
During output enable testing, many buffers will be acquired and released quickly to find a working configuration before anything makes it to screen. We cannot test configurations without buffers with the current kernel uAPI. The swapchain manager tries to be smart, reusing the previously used swapchain from enabled outputs if they are compatible with the configuration being tested, and reusing the previously tested swapchain if implicit/explicit modifiers made no difference. General note: DisplayLink should be avoided like the plague, moreso than nvidia which has actually sobered up a bit in recent times. We can't really help debug much - especially when running random patches on top of wlroots as well - but we don't mind supporting it if there is a simple, non-hack fix. |
Tested PR#8095 on top of: Display remains disabled, log:
Really, you don't need to tell me that )) Regards |
The "Failed to acquire FB for plane XYZ" is printed by Check if |
I'll try to check following your hints (it takes some time, since the codebase is new for me, same as the whole graphics topic as well this is the production environment for my daily work). The "old", pre e2f3eba implementation didn't handle the display during the first buffer allocation attempt too, yet no "Failed to acquire FB for plane XYZ" messages were observed (because it wasn't tried to be acquired during that phase?). If I'm not mistaken it performed a fallback between both the pixel format and the modifier:
Based on dmesg output for the "perfect" evdi driver, the correct mode for this display is: 1920x1080@60; bpp 32; pixel format AR24 little-endian (0x34325241)
In this PR you start the fallback with changing the resolution/refresh rate first, what if performing the pixel format and/or modifier would be performed prior to that? |
Each try of the swapchain manager tries both explicit and implicit modifiers, the latter being the fallback. We don't try every modifier due to buffer allocation still being a little costly, and that would mean way more buffers allocated, but that's nothing new to sway. The idea with this PR is that every possible combination is tested using cheaper test commits, stopping at the first found that works. The old logic only tested these things for each output in isolation, which could lead to an Nth output falling because previous outputs were configured in a resource-intensive manner when even though though this could be fixed. The new logic instead always looks at all outputs, searching through all possible combinations. The only thing off the top of my head that differs - other than more buffers being allocated - is that the new logic does not mix implicit/explicit modifiers. wlr_output_swapchain_manager tries all outputs as either/or, while old sway lighting up one output at a time would try explicit/implicit modifiers for a single output when it is added... |
Yup, that might be the case, when there is a need for Y_TILED_GEN12_RC_CCS_CC for one output and LINEAR for other output. I did a more verbose run with printing the slot addresses, also to handle eDP-1 and DVI-I-1 resolution ambiguity that I've seen in the logs (couldn't distinguish between eDP-1 and DVI-I-1 swapchains/buffers). Here's the log (cut after it enters a loop): sway-debug.13_.log
Therefore the FB acquire attempt fails after the very first run of |
This isn't related to this PR, so please open a separate issue for it. Post the output of |
2174f50
to
4b840dc
Compare
1868e48
to
3e2047e
Compare
|
The original sway output config implementation enabled one output at a time, testing modes, render formats and VRR support as it went along. While this sort of fallback is easy to do, it has the downside of not considering the effect of neighbor outputs on the configuration viability. With backend-wide commits, we can now better consider the effect of neighbor outputs, but to handle the fact that we commit all outputs at once we need to perform a more elaborate search of viable configurations. Implement a recursive configuration search for when the primary configuration failed to apply.
Instead of having each search function print its various test decisions, print the full state at the end of every search. This makes it much clearer what state a particular test includes.
3e2047e
to
b341cd4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The original sway output config implementation enabled one output at a time, testing modes, render formats and VRR support as it went along. While this sort of fallback is easy to do, it has the downside of not considering the effect of neighbor outputs on the configuration viability.
With backend-wide commits, we can now better consider the effect of neighbor outputs, but to handle the fact that we commit all outputs at once we need to perform a more elaborate search of viable configurations.
Implement a recursive configuration search for when the primary configuration failed to apply.