On Fri, 03 Sep 2021 13:00:32 +0000 Simon Ser <contact@xxxxxxxxxxx> wrote: > This can be used to create a separate DRM file description, thus > creating a new GEM handle namespace. > > My use-case is wlroots. The library splits responsibilities between > separate components: the GBM allocator creates buffers, the GLES2 > renderer uses EGL to import them and render to them, the DRM > backend imports the buffers and displays them. wlroots has a > modular architecture, and any of these components can be swapped > and replaced with something else. For instance, the pipeline can > be set up so that the DRM dumb buffer allocator is used instead of > GBM and the Pixman renderer is used instead of GLES2. Library users > can also replace any of these components with their own custom one. > > DMA-BUFs are used to pass buffer references across components. We > could use GEM handles instead, but this would result in pain if > multiple GPUs are in use: wlroots copies buffers across GPUs as > needed. Importing a GEM handle created on one GPU into a completely > different GPU will blow up (fail at best, mix unrelated buffers > otherwise). > > Everything is fine if all components use Mesa. However, this isn't > always desirable. For instance when running with DRM dumb buffers > and the Pixman software renderer it's unfortunate to depend on GBM > in the DRM backend just to turn DMA-BUFs into FB IDs. GBM loads > Mesa drivers to perform an action which has nothing driver-specific. > Additionally, drivers will fail the import if the 3D engine can't > use the imported buffer, for instance amdgpu will refuse to import > DRM dumb buffers [1]. We might also want to be running with a Vulkan > renderer and a Vulkan allocator in the future, and GBM wouldn't be > welcome in this setup. > > To address this, GBM can be side-stepped in the DRM backend, and > can be replaced with drmPrimeFDToHandle calls. However because of > GEM handle reference counting issues, care must be taken to avoid > double-closing the same GEM handle. In particular, it's not > possible to share a DRM FD with GBM or EGL and perform some > drmPrimeFDToHandle calls manually. > > So wlroots needs to re-open the DRM FD to create a new GEM handle > namespace. However there's no guarantee that the file-system > permissions will be set up so that the primary FD can be opened > by the compsoitor. On modern systems seatd or logind is a privileged > process responsible for doing this, and other processes aren't > expected to do it. For historical reasons systemd still allows > physically logged in users to open primary DRM nodes, but this > doesn't work on non-systemd setups and it's desirable to lock > them down at some point. > > Some might suggest to open the render node instead of re-opening > the primary node. However some systems don't have a render node > at all (e.g. no GPU, or a split render/display SoC). > > Solutions to this issue have been discussed in [2]. One solution > would be to open the magic /proc/self/fd/<fd> file, but it's a > Linux-specific hack (wlroots supports BSDs too). Another solution > is to add support for re-opening a DRM primary node to seatd/logind, > but they don't support it now and really haven't been designed for > this (logind would need to grow a completely new API, because it > assumes unique dev_t IDs). Also this seems like pushing down a > kernel limitation to user-space a bit too hard. > > Another solution is to allow creating empty DRM leases. The lessee > FD would have its own GEM handle namespace, so wouldn't conflict > wth GBM/EGL. It would have the master bit set, but would be able > to manage zero resources. wlroots doesn't intend to share this FD > with any other process. > > All in all IMHO that seems like a pretty reasonable solution to the > issue at hand. > > Note, I've discussed with Jonas Ådahl and Mutter plans to adopt a > similar design in the future. > > Example usage in wlroots is available at [3]. IGT test available > at [4]. > > [1]: https://github.com/swaywm/wlroots/issues/2916 > [2]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 > [3]: https://github.com/swaywm/wlroots/pull/3158 > [4]: https://patchwork.freedesktop.org/series/94323/ > > Signed-off-by: Simon Ser <contact@xxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: Daniel Stone <daniels@xxxxxxxxxxxxx> > Cc: Pekka Paalanen <pekka.paalanen@xxxxxxxxxxxxxxx> > Cc: Michel Dänzer <michel@xxxxxxxxxxx> > Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx> > Cc: Keith Packard <keithp@xxxxxxxxxx> > Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> > Cc: Dave Airlie <airlied@xxxxxxxxxx> > --- > drivers/gpu/drm/drm_lease.c | 39 +++++++++++++++++-------------------- > include/uapi/drm/drm_mode.h | 3 ++- > 2 files changed, 20 insertions(+), 22 deletions(-) Hi Simon, that is one awesome commit message! It explains everything I might have wanted to ask. I also agree with your analysis, so this is an easy: Acked-by: Pekka Paalanen <pekka.paalanen@xxxxxxxxxxxxx> Of course, I can't say anything about the actual code. Thanks, pq
Attachment:
pgpD0iJ72fTtp.pgp
Description: OpenPGP digital signature