linux-dmabuf: clarify DRM_FORMAT_MOD_INVALID

Submitted by Chia-I Wu on April 1, 2019, 5:41 p.m.

Details

Message ID 20190401174117.22784-1-olvaffe@gmail.com
State Superseded
Headers show
Series "linux-dmabuf: clarify DRM_FORMAT_MOD_INVALID" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Chia-I Wu April 1, 2019, 5:41 p.m.
DRM_FORMAT_MOD_INVALID means to derive the modifier from the dmabuf.

Signed-off-by: Chia-I Wu <olvaffe@gmail.com>
---
 .../linux-dmabuf/linux-dmabuf-unstable-v1.xml     | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
index 154afe2..7c76441 100644
--- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
@@ -28,6 +28,7 @@ 
     <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
+      https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
       and the Linux DRM sub-system's AddFb2 ioctl.
 
       This interface offers ways to create generic dmabuf-based
@@ -129,8 +130,13 @@ 
         binds to this interface. A roundtrip after binding guarantees that
         the client has received all supported format-modifier pairs.
 
+        For each supported format, DRM_FORMAT_MOD_INVALID (that is,
+        modifier_hi == 0x00ffffff and modifier_lo == 0xffffffff) is implied
+        and may not be sent.
+
         For the definition of the format and modifier codes, see the
-        zwp_linux_buffer_params_v1::create request.
+        zwp_linux_buffer_params_v1::create and zwp_linux_buffer_params_v1::add
+        requests.
       </description>
       <arg name="format" type="uint" summary="DRM_FORMAT code"/>
       <arg name="modifier_hi" type="uint"
@@ -200,6 +206,13 @@ 
         This request raises the PLANE_IDX error if plane_idx is too large.
         The error PLANE_SET is raised if attempting to set a plane that
         was already set.
+
+        Note that DRM_FORMAT_MOD_INVALID (that is, modifier_hi == 0x00ffffff
+        and modifier_lo == 0xffffffff) is always allowed and has a special
+        meaning. It indicates that the modifier is derived from the dmabuf fd
+        rather than explicitly specified. This use is discouraged and may not
+        work if the dmabuf is imported to a different device than the device
+        that allocated it.
       </description>
       <arg name="fd" type="fd" summary="dmabuf fd"/>
       <arg name="plane_idx" type="uint" summary="plane index"/>

Comments




On Tue, Apr 16, 2019 at 2:52 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Fri, 12 Apr 2019 10:50:49 -0700
> Chia-I Wu <olvaffe@gmail.com> wrote:
>
> > Moving the discussion to this patch...
> >
> > This patch clarifies how implicit modifier can be supported, modelling
> > after Weston's behavior.  I can see three options
> >
> >  1. DRM_FORMAT_MOD_INVALID means implicit modifier, and is always allowed
> > in buffer creation
> >  2. DRM_FORMAT_MOD_INVALID means implicit modifier, and a modifier event of
> > the value must be sent to indicate buffer creation with implicit modifier
> > is allowed
> >  3. DRM_FORMAT_MOD_INVALID is invalid and there is no implicit modifier
> > support
> >
> > This patch picks option 1.
> >
> > Option 3 makes legacy support impossible, and in turn makes wl_drm
> > deprecation take longer.
> >
> > I've been thinking about moving away from implicit modifier as well so
> > option 2 might be a good compromise.  The protocol is also more consistent
> > in that one can create a buffer with a format/modifier pair only when the
> > pair is advertised via the modifier event.
>
> Hi,
>
> yes, allowing to use only combinations of format and modifier that were
> advertised would be nice. We should probably do that when we break the
> protocol ABI the next time if that was not already the case. Until
> then, if compositors have possibly accepted DRM_FORMAT_MOD_INVALID
> without explicitly advertising it, we probably cannot make that illegal
> at the protocol level. This would need a TODO note in the protocol spec
> so we remember to do it during stabilization the latest.
Mutter does not advertise DRM_FORMAT_MOD_INVALID.  When a client
creates a buffer with DRM_FORMAT_MOD_INVALID, Mutter passes the
modifier to EGL and let EGL fail the buffer creation.

Weston always accepts DRM_FORMAT_MOD_INVALID however.  It strips the
modifier out so that the drivers do not see the modifier.

Neither Mesa nor XWayland creates buffers with DRM_FORMAT_MOD_INVALID.
They both fall back to wl_drm when the modifier is
DRM_FORMAT_MOD_INVALID.  I guess not a lot of clients, if any, rely on
weston's behavior.



>
> A compositor can also refuse a dmabuf creation for any reason by
> sending the "failed" event instead of "created". If we did allow
> DRM_FORMAT_MOD_INVALID in the protocol temporarily, but drivers/GBM/EGL
> don't allow the implicit modifier, no problem from protocol
> perspective. The "create_immed" request is specifically outside of
> validity considerations, we do not have to care about breaking that.
>
> Option 1 we can certainly do right now. Option 2 I'd have to do some
> research to see if it makes current practices illegal. Option 3 is
> possible with a protocol ABI break, if all drivers and Mesa agree it
> can be done. All of these options are fine with me with these
> conditions.
I will send out v2 to do option 2.
>
>
> Thanks,
> pq