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

[Scott Anderson]

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.

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.

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.

Scott

[1]: https://patchwork.freedesktop.org/patch/263061/