linux-dmabuf: un-deprecate format events

Submitted by Chia-I Wu on April 9, 2019, 10:22 p.m.

Details

Message ID 20190409222213.109922-1-olvaffe@gmail.com
State Superseded
Headers show
Series "linux-dmabuf: un-deprecate format events" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Chia-I Wu April 9, 2019, 10:22 p.m.
Format events are used to return supported formats while modifier
evetns are used to return supported modifiers for each supported
format.

For example, if an implementation is based on EGL, it may send one
format event for each format returned by eglQueryDmaBufFormatsEXT,
and one modifier event for each modifier returned by
eglQueryDmaBufModifiersEXT.

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

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..736cc2b 100644
--- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
@@ -24,10 +24,11 @@ 
     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
+      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
@@ -113,10 +114,8 @@ 
         For the definition of the format codes, see the
         zwp_linux_buffer_params_v1::create request.
 
-        Warning: the 'format' event is likely to be deprecated and replaced
-        with the 'modifier' event introduced in zwp_linux_dmabuf_v1
-        version 3, described below. Please refrain from using the information
-        received from this event.
+        For each buffer format that the server supports, this event must be
+        sent before any 'modifier' event for the same buffer format.
       </description>
       <arg name="format" type="uint" summary="DRM_FORMAT code"/>
     </event>
@@ -130,7 +129,11 @@ 
         the client has received all supported format-modifier pairs.
 
         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.
+
+        Note that DRM_FORMAT_MOD_INVALID (that is, modifier_hi == 0x00ffffff
+        and modifier_lo == 0xffffffff) is not a valid modifier in this event.
       </description>
       <arg name="format" type="uint" summary="DRM_FORMAT code"/>
       <arg name="modifier_hi" type="uint"
@@ -200,6 +203,11 @@ 
         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.
       </description>
       <arg name="fd" type="fd" summary="dmabuf fd"/>
       <arg name="plane_idx" type="uint" summary="plane index"/>

Comments

Hi,

Thanks for your patch.

On Wednesday, April 10, 2019 1:22 AM, Chia-I Wu <olvaffe@gmail.com> wrote:
> Format events are used to return supported formats while modifier
> evetns are used to return supported modifiers for each supported

Typo: events

> format.
>
> For example, if an implementation is based on EGL, it may send one
> format event for each format returned by eglQueryDmaBufFormatsEXT,
> and one modifier event for each modifier returned by
> eglQueryDmaBufModifiersEXT.

Can you explain the rationale behind this change, why the other has
been abandoned, why this one is better?

In wlroots we send modifier events with DRM_FORMAT_MOD_INVALID in case
no modifier can be used with the format.


On Wednesday, April 10, 2019 9:25 PM, Chia-I Wu <olvaffe@gmail.com> wrote:
> Both implementations support formats that have no explicit modifiers.
> But when it comes to buffer creation, modifier_hi/lo is required
> again.  It is unclear how to create a buffer for such formats.
>
> This patch tries to clarify these issues, leaning toward mutter's
> behavior.

Is there a reason to standardize Mutter's behavior instead of Weston's
and wlroots'? (Don't know about others)


On Thursday, April 11, 2019 11:48 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> Hi,
>
> at first hand, I would keep Weston's behaviour, because that is what
> the current protocol specification says exactly. The specification text
> is very clear on the intent: the old format event is not to be used, if
> both the client and server support the version with the modifier event.
> When the protocol will be stabilized (which is always an ABI break),
> the format event without a modifier will be removed.
>
> The difference to EGL API does not seem to be making anything more
> difficult either.
>
> Technically there is little difference between using two events with
> and without a modifier, and one event with a special no-modifier value.
> A technical reason to favour two different events would be bandwidth
> saving, since then one doesn't need to send the dummy modifier. Can we
> make that argument? If yes, that would be a good justification to make
> the change you propose.

Do we have a lot of these? My machines don't have any (i915 driver).

But anyway, I thought that DRM_FORMAT_MOD_INVALID was being phased out?

> At first read of your commit message, I got the feeling you want
> formats to be advertised by both events: format event to declare a
> format at all, plus modifier events to list any modifiers. That is also
> what your proposed spec text reads, so I'd recommend clarifying that
> implementations need to send only one of the two events per format. Or
> both if the same format can be used with and without a modifier?
>
> If we have two events, one without and one with a modifier, should we
> also then have two (well, four actually, for the immed variant)
> requests to create a buffer: one without and one with a modifier? I
> think it would be nice to have these consistent unless there is a
> reason not to.
>
> I can find justification both ways, and whether we need to fix Weston
> or Mutter is not that important to me. I might want to avoid the four
> create requests case though.

I agree that either way, some compositors will need to be fixed, and that we
shouldn't decide based on current implementations.

> I'm not sure the versioning fully works out, if we have:
> - version 1 and 2 have only 'format' event
> - version 3 has 'modifier' event, with 'format' event not to be used
> - version 4 wants to use both 'modifier' and 'format' events

Yeah, that would be a little bit annoying.

> Implementation would need to adhere to these, so Mutter would still
> need fixing for clients that bind version 3. I don't think we should
> retro-actively change the semantics of an existing version.

If the protocol isn't ambiguous, it would be dangerous to change semantics
indeed.

> I notice that currently the modifier is in
> zwp_linux_buffer_params_v1.add request which makes the modifier
> per-plane, but I seem to recall that in practise modifiers are nowadays
> used only per-buffer, not per-plane, so the modifier argument should
> move to the create and create_immed requests.

You're right, modifiers are now per-buffer and not per-plane.

> We also need to consider https://patchwork.freedesktop.org/patch/263061/ .
>
> I would propose the following roadmap:
>
> 1. Do not un-deprecate format events; instead clarify the modifier on
>    create and fix Mutter's behaviour.
>
> 2. Get https://patchwork.freedesktop.org/patch/263061/ merged.
>
> 3. Stop using wl_drm completely when both server and client support the
>    latest zwp_linux_dmabuf.
>
> 4. Do ABI-breaking cleanups, and bump the major or if we are sure
>    enough then declare the extension stable.
>
> The reason why I would throw the 'format' event away as it is is that
> then it will be consistent with the create requests: use
> DRM_FORMAT_MOD_INVALID for "no modifier" in both events and requests. I
> would assume that the bandwidth savings would not be significant,
> considering everything is aiming to have explicit modifiers.
>
> How would that sound?

This makes a lot of sense to me. +1.