Initial support for DRM async page flip#1335
Initial support for DRM async page flip#1335PolyMeilex wants to merge 2 commits intoSmithay:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1335 +/- ##
==========================================
- Coverage 21.32% 21.23% -0.09%
==========================================
Files 156 156
Lines 25121 25042 -79
==========================================
- Hits 5356 5317 -39
+ Misses 19765 19725 -40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
1a8c49e to
0d62362
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Yep, it's more or less the only use-case for this. |
74b2cdf to
a7e1c32
Compare
e141600 to
fb65980
Compare
|
Ok, I belive this is ready. Although I still can't test the atomic path, even tho my driver started to report Running with |
| fn presentation_mode(&self) -> Option<PresentationMode> { | ||
| // On Wayland assume VSync as default preference | ||
| Some(PresentationMode::VSync) | ||
| // TODO: Return Async in case wp_tearing is attatched to the surface |
There was a problem hiding this comment.
This is waiting on the tearing control PR?
There was a problem hiding this comment.
Yep, or the other way around, I don't care. Although the protocol PR is probably more merge ready. We could also just merge the PRs into one.
|
Okay, so this is not going to work this way unfortunately. Async page flips have some special limitations we need to account for. For an async page flip to succeed the atomic commit is only allowed to change the framebuffer on the primary plane. Every other change will result in the page flip request to be declined. This unfortunately also includes Not sure what to do about There are also driver specific limitations like for example intel not supporting async page flips if the plane uses some compressed tiling format. So imo the most sane way to handle this is to do best guessing in I prototyped some parts of this and will post a patch when its ready. |
|
Okay, I put something together that should work (at least it seems to work on my system). Curious if this also works on AMD and Nvidia now @PolyMeilex @YaLTeR @Drakulix |
fb65980 to
c5d0073
Compare
|
@cmeissl Atomic backend now works on my AMD systems, looks promising. Rebased on master, and cherrypicked the cmeissl's commit (with some minnor reverts of debug code edits) |
c5d0073 to
e62c854
Compare
|
Confits resolved, everything seems to still work as expected on my AMD machines both with atomic and legacy |
|
Feedback from the display next hackfest:
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as duplicate.
This comment was marked as duplicate.
|
I will try to find some time in the next two weeks to go over the PR @PolyMeilex are you fine with me updating the PR? |
For sure, go ahead |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
And after that, if it doesn't require multiple rounds of such review, then it can potentially get merged. We all want lower latency in our games, me included, and I am avid to implement this and make use of it, but having a solid implementation backing it more important. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
d4e7ff1 to
1072da7
Compare
|
Ok, it took a bit of patience, but I managed to rebase this. It's still possible to test this by editing this line: It's also worth noting that I still don't have a full grasp of what's going on in the Cmeissl's commit that made the atomic backend work, that's why I'm keeping it as a separate WIP commit. Open question: Would dropping the seemingly more problematic atomic backend implementation and merging just the legacy impl make sense? Just to get things rolling, or should I rather just focus on trying to grasp what's needed for less hacky atomic impl? |
Thanks again for taking the time to get this forward. imo it does not make sense without the atomic backend. Also some of the things of my commit should not longer be required as some of the limitations have been lifted in the kernel. But I need some time to look into this again :( |
1072da7 to
7ea3f73
Compare
The one thing I noticed is that your Linear format filters did not survive the rebase, but the impl still seems to work fine, so perhaps that's one of the limitations lifted? |
Possible, I don't remember all details. But one thing that should be lifted is the IN_FENCE_FD iirc. |
7ea3f73 to
a7de7d4
Compare
This allows users to select if frame should be presented with VSync or fully ASync.
blocked by: Smithay/drm-rs#190 (awaiting releasse)blocked by: Smithay/gbm.rs#40related: #1325