Re: [PATCH wayland-protocols v3] linux-dmabuf: add immediate dmabuf import path

[Pekka Paalanen]

On Mon, 6 Feb 2017 13:33:43 +0530
Varad Gautam  wrote:

> Hi Pekka,
> 
> On Fri, Feb 3, 2017 at 8:39 PM, Pekka Paalanen
>  wrote:
> > On Tue, 31 Jan 2017 18:41:52 +0530
> > Varad Gautam  wrote:
> >  
> >> From: Varad Gautam 
> >>
> >> provide a mechanism that allows clients to import the added dmabufs
> >> and immediately use the newly created wl_buffers without waiting on
> >> an event. this is useful to clients that are sure of their import
> >> request succeeding, and wish to avoid the wl_buffer communication
> >> roundtrip.
> >>
> >> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> >> versions.
> >>
> >> v2: specify using incorrectly imported dmabufs as undefined behavior
> >> instead of sending success/failure events. (pq, daniels)
> >> v3: preserve the optional protocol error added in v2 and explicitly
> >> state the outcome of import success or failure (pq)
> >>
> >> Signed-off-by: Varad Gautam 
> >> ---
> >>  unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 58 
> >> +++---
> >>  1 file changed, 51 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml 
> >> b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> >> index ed2c4bb..ddb49cc 100644
> >> --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> >> +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> >> @@ -24,13 +24,13 @@
> >>  DEALINGS IN THE SOFTWARE.
> >>
> >>
> >> -  
> >> +  
> >>  
> >>Following the interfaces from:
> >>
> >> https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
> >>and the Linux DRM sub-system's AddFb2 ioctl.
> >>
> >> -  This interface offers a way to create generic dmabuf-based
> >> +  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.
> >>
> >> @@ -56,10 +56,23 @@
> >>To create a wl_buffer from one or more dmabufs, a client creates a
> >>zwp_linux_dmabuf_params_v1 object with a 
> >> zwp_linux_dmabuf_v1.create_params
> >>request. All planes required by the intended format are added with
> >> -  the 'add' request. Finally, a 'create' request is issued. The server
> >> -  will reply with either a 'created' event which provides the final
> >> -  wl_buffer or a 'failed' event saying that it cannot use the dmabufs
> >> -  provided.
> >> +  the 'add' request. Finally, a 'create' or 'create_immed' request is
> >> +  issued, which has the following outcome depending on the import 
> >> success.
> >> +
> >> +  The 'create' request,
> >> +  - on success, triggers a 'created' event which provides the final
> >> +wl_buffer to the client.
> >> +  - on failure, triggers a 'failed' event to convey that the server
> >> +cannot use the dmabufs received from the client.
> >> +
> >> +  For the 'create_immed' request,
> >> +  - on success, the server immediately imports the added dmabufs to
> >> +create a wl_buffer. No event is sent from the server in this case.
> >> +  - on failure, the server can choose to either:
> >> +- terminate the client by raising a fatal error.
> >> +- create an invalid wl_buffer handle and send a 'failed' event to
> >> +  the client. The result of using this invalid wl_buffer in the
> >> +  client is left implementation-defined by the protocol.  
> >
> > Hi,
> >
> > I would phrase the latter as:
> >
> > - Mark the wl_buffer as failed and send a 'failed' event to the
> >   client. If the client uses a failed wl_buffer as an argument
> >   to any request, the behaviour is compositor
> >   implementation-defined.
> >  
> 
> ack, will fix.
> 
> >>
> >>Warning! The protocol described in this file is experimental and
> >>backward incompatible changes may be made. Backward compatible 
> >> changes
> >> @@ -105,7 +118,7 @@
> >>  
> >>
> >>
> >> -  
> >> +  
> >>  
> >>This temporary object is a collection of dmabufs and other
> >>parameters that together form a single logical buffer. The temporary
> >> @@ -138,6 +151,9 @@
> >>   summary="invalid width or height"/>
> >> >>   summary="offset + stride * height goes out of dmabuf 
> >> bounds"/>
> >> +   >> + summary="invalid wl_buffer resulted from importing dmabufs 
> >> from
> >> +   the given buffer_params"/>  
> >
> > I might have called it "import_failure". Not a big deal.  
> >>  
> >>
> >>  
> >> @@ -269,6 +285,34 @@
> >>  zlinux_buffer_params object.
> >>
> >>  
> >> +
> >> +
> >> +  
> >> +This asks for 

Re: [PATCH wayland-protocols v3] linux-dmabuf: add immediate dmabuf import path

[Varad Gautam]

Hi Pekka,

On Fri, Feb 3, 2017 at 8:39 PM, Pekka Paalanen
 wrote:
> On Tue, 31 Jan 2017 18:41:52 +0530
> Varad Gautam  wrote:
>
>> From: Varad Gautam 
>>
>> provide a mechanism that allows clients to import the added dmabufs
>> and immediately use the newly created wl_buffers without waiting on
>> an event. this is useful to clients that are sure of their import
>> request succeeding, and wish to avoid the wl_buffer communication
>> roundtrip.
>>
>> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
>> versions.
>>
>> v2: specify using incorrectly imported dmabufs as undefined behavior
>> instead of sending success/failure events. (pq, daniels)
>> v3: preserve the optional protocol error added in v2 and explicitly
>> state the outcome of import success or failure (pq)
>>
>> Signed-off-by: Varad Gautam 
>> ---
>>  unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 58 
>> +++---
>>  1 file changed, 51 insertions(+), 7 deletions(-)
>>
>> diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml 
>> b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> index ed2c4bb..ddb49cc 100644
>> --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
>> @@ -24,13 +24,13 @@
>>  DEALINGS IN THE SOFTWARE.
>>
>>
>> -  
>> +  
>>  
>>Following the interfaces from:
>>
>> https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
>>and the Linux DRM sub-system's AddFb2 ioctl.
>>
>> -  This interface offers a way to create generic dmabuf-based
>> +  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.
>>
>> @@ -56,10 +56,23 @@
>>To create a wl_buffer from one or more dmabufs, a client creates a
>>zwp_linux_dmabuf_params_v1 object with a 
>> zwp_linux_dmabuf_v1.create_params
>>request. All planes required by the intended format are added with
>> -  the 'add' request. Finally, a 'create' request is issued. The server
>> -  will reply with either a 'created' event which provides the final
>> -  wl_buffer or a 'failed' event saying that it cannot use the dmabufs
>> -  provided.
>> +  the 'add' request. Finally, a 'create' or 'create_immed' request is
>> +  issued, which has the following outcome depending on the import 
>> success.
>> +
>> +  The 'create' request,
>> +  - on success, triggers a 'created' event which provides the final
>> +wl_buffer to the client.
>> +  - on failure, triggers a 'failed' event to convey that the server
>> +cannot use the dmabufs received from the client.
>> +
>> +  For the 'create_immed' request,
>> +  - on success, the server immediately imports the added dmabufs to
>> +create a wl_buffer. No event is sent from the server in this case.
>> +  - on failure, the server can choose to either:
>> +- terminate the client by raising a fatal error.
>> +- create an invalid wl_buffer handle and send a 'failed' event to
>> +  the client. The result of using this invalid wl_buffer in the
>> +  client is left implementation-defined by the protocol.
>
> Hi,
>
> I would phrase the latter as:
>
> - Mark the wl_buffer as failed and send a 'failed' event to the
>   client. If the client uses a failed wl_buffer as an argument
>   to any request, the behaviour is compositor
>   implementation-defined.
>

ack, will fix.

>>
>>Warning! The protocol described in this file is experimental and
>>backward incompatible changes may be made. Backward compatible changes
>> @@ -105,7 +118,7 @@
>>  
>>
>>
>> -  
>> +  
>>  
>>This temporary object is a collection of dmabufs and other
>>parameters that together form a single logical buffer. The temporary
>> @@ -138,6 +151,9 @@
>>   summary="invalid width or height"/>
>>>   summary="offset + stride * height goes out of dmabuf bounds"/>
>> +  > + summary="invalid wl_buffer resulted from importing dmabufs from
>> +   the given buffer_params"/>
>
> I might have called it "import_failure". Not a big deal.
>>  
>>
>>  
>> @@ -269,6 +285,34 @@
>>  zlinux_buffer_params object.
>>
>>  
>> +
>> +
>> +  
>> +This asks for immediate creation of a wl_buffer by importing the
>> +added dmabufs.
>> +
>> +In case of import success, no event is sent from the server, and the
>> +wl_buffer is ready to be used by the client.
>> +
>> +Upon import failure, either of the following may happen, as seen fit
>> +by the implementation:
>> +- 

Re: [PATCH wayland-protocols v3] linux-dmabuf: add immediate dmabuf import path

[Pekka Paalanen]

On Tue, 31 Jan 2017 18:41:52 +0530
Varad Gautam  wrote:

> From: Varad Gautam 
> 
> provide a mechanism that allows clients to import the added dmabufs
> and immediately use the newly created wl_buffers without waiting on
> an event. this is useful to clients that are sure of their import
> request succeeding, and wish to avoid the wl_buffer communication
> roundtrip.
> 
> bump zwp_linux_dmabuf_v1, zwp_linux_buffer_params_v1 interface
> versions.
> 
> v2: specify using incorrectly imported dmabufs as undefined behavior
> instead of sending success/failure events. (pq, daniels)
> v3: preserve the optional protocol error added in v2 and explicitly
> state the outcome of import success or failure (pq)
> 
> Signed-off-by: Varad Gautam 
> ---
>  unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 58 
> +++---
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml 
> b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> index ed2c4bb..ddb49cc 100644
> --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml
> @@ -24,13 +24,13 @@
>  DEALINGS IN THE SOFTWARE.
>
>  
> -  
> +  
>  
>Following the interfaces from:
>
> https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
>and the Linux DRM sub-system's AddFb2 ioctl.
>  
> -  This interface offers a way to create generic dmabuf-based
> +  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.
>  
> @@ -56,10 +56,23 @@
>To create a wl_buffer from one or more dmabufs, a client creates a
>zwp_linux_dmabuf_params_v1 object with a 
> zwp_linux_dmabuf_v1.create_params
>request. All planes required by the intended format are added with
> -  the 'add' request. Finally, a 'create' request is issued. The server
> -  will reply with either a 'created' event which provides the final
> -  wl_buffer or a 'failed' event saying that it cannot use the dmabufs
> -  provided.
> +  the 'add' request. Finally, a 'create' or 'create_immed' request is
> +  issued, which has the following outcome depending on the import 
> success.
> +
> +  The 'create' request,
> +  - on success, triggers a 'created' event which provides the final
> +wl_buffer to the client.
> +  - on failure, triggers a 'failed' event to convey that the server
> +cannot use the dmabufs received from the client.
> +
> +  For the 'create_immed' request,
> +  - on success, the server immediately imports the added dmabufs to
> +create a wl_buffer. No event is sent from the server in this case.
> +  - on failure, the server can choose to either:
> +- terminate the client by raising a fatal error.
> +- create an invalid wl_buffer handle and send a 'failed' event to
> +  the client. The result of using this invalid wl_buffer in the
> +  client is left implementation-defined by the protocol.

Hi,

I would phrase the latter as:

- Mark the wl_buffer as failed and send a 'failed' event to the
  client. If the client uses a failed wl_buffer as an argument
  to any request, the behaviour is compositor
  implementation-defined.

>  
>Warning! The protocol described in this file is experimental and
>backward incompatible changes may be made. Backward compatible changes
> @@ -105,7 +118,7 @@
>  
>
>  
> -  
> +  
>  
>This temporary object is a collection of dmabufs and other
>parameters that together form a single logical buffer. The temporary
> @@ -138,6 +151,9 @@
>   summary="invalid width or height"/>
>   summary="offset + stride * height goes out of dmabuf bounds"/>
> +   + summary="invalid wl_buffer resulted from importing dmabufs from
> +   the given buffer_params"/>

I might have called it "import_failure". Not a big deal.

>  
>  
>  
> @@ -269,6 +285,34 @@
>  zlinux_buffer_params object.
>
>  
> +
> +
> +  
> +This asks for immediate creation of a wl_buffer by importing the
> +added dmabufs.
> +
> +In case of import success, no event is sent from the server, and the
> +wl_buffer is ready to be used by the client.
> +
> +Upon import failure, either of the following may happen, as seen fit
> +by the implementation:
> +- the client is terminated with a fatal protocol error.

Could mention the error code here, since there is one specifically for
this case.

> +- the server creates an invalid wl_buffer and sends a 'failed' event
> +