This repository has been archived by the owner on Nov 1, 2021. It is now read-only.
backend/drm: get rid of BO handle table #3164
Labels
Comments
|
@emersion, I was going to create a new issue as you directed on #2916 about fullscreen corruption after the introduction of wlr_drm_bo_handle_table, but I guess this supersedes that. FWIW nothing particularly unusual stood out in the debug output of sway (but I can attach it) and dmesg showed nothing.
Reverting the commit introducing the BO handle table "fixes" it. Also: |
emersion
added a commit
to emersion/wlroots
that referenced
this issue
Oct 4, 2021
The BO handle table exists to avoid double-closing a BO handle, which aren't reference-counted by the kernel. But if we can guarantee that there is only ever a single ref for each BO handle, then we don't need the BO handle table anymore. This is possible if we create the handle right before the ADDFB2 IOCTL, and close the handle right after. The handles are very short-lived and we don't need to track their lifetime. Because of multi-planar FBs, we need to be a bit careful: some FB planes might share the same handle. But with a small check, it's easy to avoid double-closing the same handle (which wouldn't be a big deal anyways). There's one gotcha though: drmModeSetCursor2 takes a BO handle as input. Saving the handles until drmModeSetCursor2 time would require us to track BO handle lifetimes, so we wouldn't be able to get rid of the BO handle table. As a workaround, use drmModeGetFB to turn the FB ID back to a BO handle, call drmModeSetCursor2 and then immediately close the BO handle. The overhead should be minimal since these IOCTLs are pretty cheap. Closes: swaywm#3164
emersion
added a commit
to emersion/wlroots
that referenced
this issue
Oct 4, 2021
The BO handle table exists to avoid double-closing a BO handle, which aren't reference-counted by the kernel. But if we can guarantee that there is only ever a single ref for each BO handle, then we don't need the BO handle table anymore. This is possible if we create the handle right before the ADDFB2 IOCTL, and close the handle right after. The handles are very short-lived and we don't need to track their lifetime. Because of multi-planar FBs, we need to be a bit careful: some FB planes might share the same handle. But with a small check, it's easy to avoid double-closing the same handle (which wouldn't be a big deal anyways). There's one gotcha though: drmModeSetCursor2 takes a BO handle as input. Saving the handles until drmModeSetCursor2 time would require us to track BO handle lifetimes, so we wouldn't be able to get rid of the BO handle table. As a workaround, use drmModeGetFB to turn the FB ID back to a BO handle, call drmModeSetCursor2 and then immediately close the BO handle. The overhead should be minimal since these IOCTLs are pretty cheap. Closes: swaywm#3164
emersion
added a commit
that referenced
this issue
Oct 29, 2021
The BO handle table exists to avoid double-closing a BO handle, which aren't reference-counted by the kernel. But if we can guarantee that there is only ever a single ref for each BO handle, then we don't need the BO handle table anymore. This is possible if we create the handle right before the ADDFB2 IOCTL, and close the handle right after. The handles are very short-lived and we don't need to track their lifetime. Because of multi-planar FBs, we need to be a bit careful: some FB planes might share the same handle. But with a small check, it's easy to avoid double-closing the same handle (which wouldn't be a big deal anyways). There's one gotcha though: drmModeSetCursor2 takes a BO handle as input. Saving the handles until drmModeSetCursor2 time would require us to track BO handle lifetimes, so we wouldn't be able to get rid of the BO handle table. As a workaround, use drmModeGetFB to turn the FB ID back to a BO handle, call drmModeSetCursor2 and then immediately close the BO handle. The overhead should be minimal since these IOCTLs are pretty cheap. Closes: #3164
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
#3131 has introduced a table to ref'count buffer handles.
However we don't really need a buffer handle table if all we're going to do is to turn these into KMS FB IDs. Instead we can create the handles, then create the KMS FB, then close the handles:
https://github.com/Plagman/gamescope/blob/1989c291ee87b01ec726e1bc6dfecfa2970e695e/src/drm.cpp#L797
One unfortunate issue is
drmModeSetCursor2, which takes a handle instead of a FB ID. So we can't do that before we drop support for the legacy KMS API.The text was updated successfully, but these errors were encountered: