Repository navigation
modesetting: don't advertise TearFree for a pixmap we never checked - #3683
kiyoshispreclerg wants to merge 1 commit into
Conversation
| if (ms_tearfree_is_active_on_crtc(xf86_crtc)) { | ||
| if (trf->flip_seq) | ||
| /* The driver has a TearFree flip pending */ | ||
| *reason = PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING; |
There was a problem hiding this comment.
We probably still want to report when tearfree is mid-flip regardless of what is known about this pixmap.Maybe just change the else below to else if (!pixmap_unchecked)
There was a problem hiding this comment.
Okay I totally agree, will update.
There was a problem hiding this comment.
Sorry, I did as you suggested, but then it doesn't fix the issue (#3680)
There was a problem hiding this comment.
Sorry, I did as you suggested, but then it doesn't fix the issue (#3680)
In that case, we should probably look for another solution, since as-is, this would most likely reintroduce #1812
In the meantime, if we can't report that we are mid-flip, this patch logically replaces the goto with return FALSE;, but implemented with far more code. Please remove the extra code and just replace the goto with return FALSE;, while keeping the added comment.
@cepelinas9000 @metux Maybe we should change the behavior of PRESENT_FLIP_REASON_DRIVER_TEARFREE_FLIPPING?
b534987 to
ccc8a73
Compare
|
I updated as suggested by Stefan11111, and also changed the commit message a bit, it was too verbose in my opinion (llms like a lot of text :) |
ccc8a73 to
d91fd11
Compare
ms_present_check_flip() bails out early when the window and pixmap don't match exactly, landing in no_flip before ms_present_check_unflip() ever runs. At that point the pixmap is unverified, so present_execute() shouldn't be told a TearFree flip is about to start for it: if the fake flip fails to register, it falls back to guessing completion_msc, and two presentations guessing the same MSC collide and stall. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Kiyoshi Spreclerg <kiyoshi_pip@protonmail.com>
d91fd11 to
3b3d26f
Compare
eae2c21 to
483a172
Compare
This is a way we found to fix #3680. I assume I have bare idea how this fixes the issue, but not 100% understanding the code path.
Also I don't know if this could affect #1812 or other things fixed by 2e31b2f.
Please see if this is a good solution or at least it points to the direction for a good one. And if approved, would go to 25.2 branch too. Full llm description:
ms_present_check_flip() bails out early when the window and the pixmap do not match exactly, which is what keeps a misaligned pixmap out of ms_present_check_unflip() and its gbm_bo_from_pixmap() call. But that early exit lands in the no_flip block, which then reports one of the TearFree flip reasons.
At that point nothing is known about the pixmap: not its format, not its modifier, not whether the scanout could ever take it. Reporting TearFree tells Present to adjust exec_msc and to expect a completion from a fake flip that the driver may never be able to register, in which case present_execute() falls back to guessing completion_msc = crtc_msc + 1. Two presentations that guess the same MSC then complete at the same MSC, which destroys a frame and wrecks the cadence of a client pacing on those completions.
Say nothing about TearFree on that path instead. The crash guard stays, and a presentation that did get its pixmap checked still gets the TearFree reasons as before.