[PATCH] unstable/linux-dmabuf: Add 'direct_display' flag

Thu Nov 14 15:13:09 UTC 2019

On Thu, 14 Nov 2019 17:05:41 +1300
Scott Anderson <scott at anderso.nz> wrote:

> On 14/11/19 3:08 am, Marius Vlad wrote:
> > Flag used to tell the compositor that it should avoid touching the
> > dmabuf buffer and forward it directly to the display controller.
> > 
> > Most likely this flag can be used together with the content-protection
> > protocol. It assures the client that the compositor will never handle
> > the buffer over to the GPU but instead it will forward it straight to
> > the display controller.
> > 
> > While content-protection protocol should be sufficient to restrict the
> > content on certain displays, on certain hardware platforms the GPU is
> > not a secure-path link in the secure-content-path chain, and as such,
> > this flag would be necessary to avoid passing the dmabuf buffer
> > over to the GPU altogether. GPUs reading the dmabuf residing in a
> > specially designed memory zone would be seen as an illegal memory access.
> > 
> > Other example include video players which might need this flag passed
> > down to avoid doing any composition, thus having an improvement of
> > bandwidth usage and performance. Set-up boxes with dedicated video
> > players could use this facility alongside the Pixman renderer, is
> > another potential example of why this flag would be useful.
> > 
> > Bumps zwp_linux_dmabuf_v1 and zwp_linux_buffer_params_v1 to version 4.
> > 
> > Signed-off-by: Marius Vlad <marius.vlad at collabora.com>
> > 
> > ---
> > This patch initially started as fork at
> > https://gitlab.freedesktop.org/marius.vlad0/wayland-protocols/merge_requests/3
> > 
> > This flag was used to be called 'no_gpu_import' but 'direct_display'
> > should better reflect its name and purpose.
> > 
> > Lastly, a WIP for the implementation of this flag for weston is at
> > https://gitlab.freedesktop.org/wayland/weston/merge_requests/298, which
> > should follow up closely.
> > ---
> >   .../linux-dmabuf/linux-dmabuf-unstable-v1.xml     | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> > index b43e81c..5b4b1e6 100644
> > --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> > +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> > @@ -2,7 +2,7 @@
> >   <protocol name="linux_dmabuf_unstable_v1">
> >   
> >     <copyright>
> > -    Copyright © 2014, 2015 Collabora, Ltd.
> > +    Copyright © 2014, 2015, 2019 Collabora, Ltd.
> >   
> >       Permission is hereby granted, free of charge, to any person obtaining a
> >       copy of this software and associated documentation files (the "Software"),
> > @@ -24,7 +24,7 @@
> >       DEALINGS IN THE SOFTWARE.
> >     </copyright>
> >   
> > -  <interface name="zwp_linux_dmabuf_v1" version="3">
> > +  <interface name="zwp_linux_dmabuf_v1" version="4">
> >       <description summary="factory for creating dmabuf-based wl_buffers">
> >         Following the interfaces from:
> >         https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
> > @@ -149,7 +149,7 @@
> >       </event>
> >     </interface>
> >   
> > -  <interface name="zwp_linux_buffer_params_v1" version="3">
> > +  <interface name="zwp_linux_buffer_params_v1" version="4">
> >       <description summary="parameters for creating a dmabuf-based wl_buffer">
> >         This temporary object is a collection of dmabufs and other
> >         parameters that together form a single logical buffer. The temporary
> > @@ -229,6 +229,7 @@
> >         <entry name="y_invert" value="1" summary="contents are y-inverted"/>
> >         <entry name="interlaced" value="2" summary="content is interlaced"/>
> >         <entry name="bottom_first" value="4" summary="bottom field first"/>
> > +      <entry name="direct_display" value="8" summary="forward buffer to display controller" since="4"/>
> >       </enum>
> >   
> >       <request name="create">
> > @@ -254,6 +255,14 @@
> >           'bottom_first' is specified. It is undefined whether 'bottom_first'
> >           is ignored if 'interlaced' is not set.
> >   
> > +        Flag 'direct_display' tells the compositor not to import it to the GPU
> > +        in order to bypass it entirely, such that the buffer will be directly
> > +        scanned-out by the display controller. If HW is not capable/or there
> > +        aren't any available resources to directly scan-out the buffer, a
> > +        placeholder should be installed in-place by the compositor. The
> > +        compositor may perform checks on the dmabuf and refuse to create a
> > +        wl_buffer if the dmabuf seems unusable for being used directly.
> > +
> >           This protocol does not convey any information about field rate,
> >           duration, or timing, other than the relative ordering between the
> >           two fields in one buffer. A compositor may have to estimate the  
> 
> Has any thought been into how this would need to interact with 
> dmabuf-hints[1]? Without that, it seems like it would be a total 
> crapshoot for clients to try and use this flag, since they have no idea 
> what formats+modifers the display controller supports, and instead only 
> has the list that the GPU supports.
> dmabuf-hints would also need to explicitly state that a tranche of 
> formats+modifiers are supported for this flag.

Hi,

yes. The main use for 'direct-display' flag is dmabufs that are
"poisonous". Touching their content in any way might kill the
compositor, or so I hear. Obviously no normal desktop system would ever
have those, since they would be a trivial DoS attack vector.

The hints extension will be useful for the users of 'direct-display',
but indeed it is missing the bit to say which format+modifier would be
scanout-able, so it's not sufficient as is.

Possibly the closest use case on normal desktops would be XR without
DRM leasing. You want the XR buffers to always go straight to display
without compositing, but if they cannot, it is better to show e.g. black
than a nauseating animation. Of course, such a use case would need
other extensions as well, so you can rightly say that the other
extensions should take care of this instead.

> I'm also worried about compositors that are capable of taking 
> screenshots. This flag implies that it's "dangerous" to ever touch the 
> buffer for any purpose. But then it sounds like you're encouraging video 
> players to use this, which is only going to break basic functionality. I 
> suspect than the only sensible thing for a screenshot-capable compositor 
> to do is to simply reject all uses of this flag, making it useless 
> outside of highly-integrated environments.

Yes, screenshooting would only show the placeholder graphics, so you
can argue that screenshooting would be broken.

Quite the contrary, I'd recommend no-one to use this flag unless they
know they really have to and have no other choice.

A compositor implementation that outright always rejects this flag
would be completely conformant. I'd probably recommend that to desktop
compositors even, just like they already reject all flags they do not
recognise. Hence a minimal implementation of this revision of
zwp_linux_dmabuf would be trivial.

(Did anyone actually do anything meaningful with the interlaced flag?)

> dmabuf-hints seems like a much better way to achieve the same effect of 
> efficiency, instead of the client trying to dictate what the compositor 
> should do. If this flag is added, I would want a stern warning to not 
> carelessly use it unless the compositor literally can't access the 
> buffer from the GPU.

It's a different problem.

I can well understand if this flag gets rejected from zwp_linux_dmabuf.
Maybe we should make this a separate extension instead, it would be
quite simple too.

Thanks,
pq

> 
> Scott
> 
> [1]: https://patchwork.freedesktop.org/patch/263061/
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20191114/f518a7de/attachment.sig>