[wayland-protocols,v3] linux-dmabuf: advertise format modifiers with modifier event

Submitted by Varad Gautam on Nov. 21, 2016, 10:17 a.m.

Details

Message ID 1479723459-8613-1-git-send-email-varadgautam@gmail.com
State Superseded
Series "linux-dmabuf: advertise format modifiers with modifier event"
Headers show

Commit Message

Varad Gautam Nov. 21, 2016, 10:17 a.m.
From: Varad Gautam <varad.gautam@collabora.com>

advertise the supported fourcc format modifiers along with supported
formats to the client.

bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
versions to 3.

v2: specify request name in event description for clarity (Yong Bakos)
v3: grammar fixup (Yong Bakos)

Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
Reviewed-by: Yong Bakos <ybakos@humanoriented.com>
---
 unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 25 +++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 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 a0aa42e..cc5d604 100644
--- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
+++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
@@ -26,7 +26,7 @@ 
     THIS SOFTWARE.
   </copyright>
 
-  <interface name="zwp_linux_dmabuf_v1" version="2">
+  <interface name="zwp_linux_dmabuf_v1" version="3">
     <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
@@ -34,7 +34,8 @@ 
 
       This interface offers ways to create generic dmabuf-based
       wl_buffers. Immediately after a client binds to this interface,
-      the set of supported formats is sent with 'format' events.
+      the set of supported formats and format modifiers is sent with
+      'format' and 'modifier' events.
 
       The following are required from clients:
 
@@ -110,9 +111,27 @@ 
       </description>
       <arg name="format" type="uint" summary="DRM_FORMAT code"/>
     </event>
+
+    <event name="modifier" since="3">
+      <description summary="supported buffer format modifier">
+        This event advertises the formats that the server supports, along with
+        the modifiers supported for each format. All the supported modifiers for
+        all the supported formats are advertised once when the client binds to
+        this interface. A roundtrip after binding guarantees that 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.
+      </description>
+      <arg name="format" type="uint" summary="DRM_FORMAT code"/>
+      <arg name="modifier_hi" type="uint"
+           summary="high 32 bits of layout modifier"/>
+      <arg name="modifier_lo" type="uint"
+           summary="low 32 bits of layout modifier"/>
+    </event>
   </interface>
 
-  <interface name="zwp_linux_buffer_params_v1" version="2">
+  <interface name="zwp_linux_buffer_params_v1" version="3">
     <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

Comments

Daniel Stone Nov. 21, 2016, 7:59 p.m.
Hi Varad,

On 21 November 2016 at 10:17, Varad Gautam <varadgautam@gmail.com> wrote:
> advertise the supported fourcc format modifiers along with supported
> formats to the client.
>
> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> versions to 3.

Reviewed-by: Daniel Stone <daniels@collabora.com>

I'll wait a little while with this series to see if more review pops
up, and then merge it if not.

Cheers,
Daniel
Daniel Vetter Nov. 23, 2016, 1:39 p.m.
On Mon, Nov 21, 2016 at 07:59:51PM +0000, Daniel Stone wrote:
> Hi Varad,
> 
> On 21 November 2016 at 10:17, Varad Gautam <varadgautam@gmail.com> wrote:
> > advertise the supported fourcc format modifiers along with supported
> > formats to the client.
> >
> > bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> > versions to 3.
> 
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> I'll wait a little while with this series to see if more review pops
> up, and then merge it if not.

Well in upstream kernel we just nerved modifier[1-3] and require they
match modifier[0]. Oops. Do we need to take adjustments in wayland now
too? At elast enforcing everywhere that modifiers match is probably a good
thing, and maybe even adjusting proto extensions and internal storage to
only carry one?
-Daniel
Daniel Vetter Nov. 23, 2016, 1:45 p.m.
On Wed, Nov 23, 2016 at 02:39:01PM +0100, Daniel Vetter wrote:
> On Mon, Nov 21, 2016 at 07:59:51PM +0000, Daniel Stone wrote:
> > Hi Varad,
> > 
> > On 21 November 2016 at 10:17, Varad Gautam <varadgautam@gmail.com> wrote:
> > > advertise the supported fourcc format modifiers along with supported
> > > formats to the client.
> > >
> > > bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> > > versions to 3.
> > 
> > Reviewed-by: Daniel Stone <daniels@collabora.com>
> > 
> > I'll wait a little while with this series to see if more review pops
> > up, and then merge it if not.
> 
> Well in upstream kernel we just nerved modifier[1-3] and require they
> match modifier[0]. Oops. Do we need to take adjustments in wayland now
> too? At elast enforcing everywhere that modifiers match is probably a good
> thing, and maybe even adjusting proto extensions and internal storage to
> only carry one?

Reply to the wrong patch. Proto looks good, but the weston implementation
should probably gain some checks to make sure modifier[1-3] == 0 or
modifier[0], and using modifier[0] for the other planes in the egl interop
stuff. Might also want to just throw away the additional modifiers from
internal structs.
-Daniel
Pekka Paalanen Jan. 17, 2017, 2:10 p.m.
On Mon, 21 Nov 2016 15:47:39 +0530
Varad Gautam <varadgautam@gmail.com> wrote:

> From: Varad Gautam <varad.gautam@collabora.com>
> 
> advertise the supported fourcc format modifiers along with supported
> formats to the client.
> 
> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> versions to 3.
> 
> v2: specify request name in event description for clarity (Yong Bakos)
> v3: grammar fixup (Yong Bakos)
> 
> Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
> Reviewed-by: Yong Bakos <ybakos@humanoriented.com>
> ---
>  unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 25 +++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> index a0aa42e..cc5d604 100644
> --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> @@ -26,7 +26,7 @@
>      THIS SOFTWARE.
>    </copyright>
>  
> -  <interface name="zwp_linux_dmabuf_v1" version="2">
> +  <interface name="zwp_linux_dmabuf_v1" version="3">
>      <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
> @@ -34,7 +34,8 @@
>  
>        This interface offers ways to create generic dmabuf-based
>        wl_buffers. Immediately after a client binds to this interface,
> -      the set of supported formats is sent with 'format' events.
> +      the set of supported formats and format modifiers is sent with
> +      'format' and 'modifier' events.

Hi,

so does this mean that when v3 is bound, the client will receive *both*
'format' and 'modifier' events?

What does it mean if a format comes via 'format' event but not via
'modifiers' event?

Can a format come only via 'modifiers' event?

Otherwise looks fine to me. With those clarifications it's at least:
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

The event size is 5 words which is 20 bytes. One wire buffer can hold
around 200 such events. If we ever need to send more format/modifier
combinations than that, well, I suppose we'll cross that bridge then.


Thanks,
pq

>  
>        The following are required from clients:
>  
> @@ -110,9 +111,27 @@
>        </description>
>        <arg name="format" type="uint" summary="DRM_FORMAT code"/>
>      </event>
> +
> +    <event name="modifier" since="3">
> +      <description summary="supported buffer format modifier">
> +        This event advertises the formats that the server supports, along with
> +        the modifiers supported for each format. All the supported modifiers for
> +        all the supported formats are advertised once when the client binds to
> +        this interface. A roundtrip after binding guarantees that 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.
> +      </description>
> +      <arg name="format" type="uint" summary="DRM_FORMAT code"/>
> +      <arg name="modifier_hi" type="uint"
> +           summary="high 32 bits of layout modifier"/>
> +      <arg name="modifier_lo" type="uint"
> +           summary="low 32 bits of layout modifier"/>
> +    </event>
>    </interface>
>  
> -  <interface name="zwp_linux_buffer_params_v1" version="2">
> +  <interface name="zwp_linux_buffer_params_v1" version="3">
>      <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
Varad Gautam Jan. 17, 2017, 2:28 p.m.
Hi,

On Tue, Jan 17, 2017 at 7:40 PM, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> On Mon, 21 Nov 2016 15:47:39 +0530
> Varad Gautam <varadgautam@gmail.com> wrote:
>
>> From: Varad Gautam <varad.gautam@collabora.com>
>>
>> advertise the supported fourcc format modifiers along with supported
>> formats to the client.
>>
>> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
>> versions to 3.
>>
>> v2: specify request name in event description for clarity (Yong Bakos)
>> v3: grammar fixup (Yong Bakos)
>>
>> Signed-off-by: Varad Gautam <varad.gautam@collabora.com>
>> Reviewed-by: Yong Bakos <ybakos@humanoriented.com>
>> ---
>>  unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 25 +++++++++++++++++++---
>>  1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> index a0aa42e..cc5d604 100644
>> --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> @@ -26,7 +26,7 @@
>>      THIS SOFTWARE.
>>    </copyright>
>>
>> -  <interface name="zwp_linux_dmabuf_v1" version="2">
>> +  <interface name="zwp_linux_dmabuf_v1" version="3">
>>      <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
>> @@ -34,7 +34,8 @@
>>
>>        This interface offers ways to create generic dmabuf-based
>>        wl_buffers. Immediately after a client binds to this interface,
>> -      the set of supported formats is sent with 'format' events.
>> +      the set of supported formats and format modifiers is sent with
>> +      'format' and 'modifier' events.
>
> Hi,
>
> so does this mean that when v3 is bound, the client will receive *both*
> 'format' and 'modifier' events?
>
> What does it mean if a format comes via 'format' event but not via
> 'modifiers' event?
>
> Can a format come only via 'modifiers' event?

Bind triggers both `format` and `modifier` events. - the `format` event carries
only the DRM_FORMAT_* code, (preserved from v1), whereas `modifier`
sends a supported <format, modifier> pair. So, for each supported format,
there would be one `format` event, and as many `modifier` events as the
number of modifiers supported for that format.

Thanks,
Varad

>
> Otherwise looks fine to me. With those clarifications it's at least:
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
>
> The event size is 5 words which is 20 bytes. One wire buffer can hold
> around 200 such events. If we ever need to send more format/modifier
> combinations than that, well, I suppose we'll cross that bridge then.
>
>
> Thanks,
> pq
>
>>
>>        The following are required from clients:
>>
>> @@ -110,9 +111,27 @@
>>        </description>
>>        <arg name="format" type="uint" summary="DRM_FORMAT code"/>
>>      </event>
>> +
>> +    <event name="modifier" since="3">
>> +      <description summary="supported buffer format modifier">
>> +        This event advertises the formats that the server supports, along with
>> +        the modifiers supported for each format. All the supported modifiers for
>> +        all the supported formats are advertised once when the client binds to
>> +        this interface. A roundtrip after binding guarantees that 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.
>> +      </description>
>> +      <arg name="format" type="uint" summary="DRM_FORMAT code"/>
>> +      <arg name="modifier_hi" type="uint"
>> +           summary="high 32 bits of layout modifier"/>
>> +      <arg name="modifier_lo" type="uint"
>> +           summary="low 32 bits of layout modifier"/>
>> +    </event>
>>    </interface>
>>
>> -  <interface name="zwp_linux_buffer_params_v1" version="2">
>> +  <interface name="zwp_linux_buffer_params_v1" version="3">
>>      <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
>