On Fri, 11 Nov 2016 17:10:44 +0530 Varad Gautam <varadgau...@gmail.com> wrote:
> From: Varad Gautam <varad.gau...@collabora.com> > > provide a mechanism that allows clients to import the added dmabufs > and immediately receive the newly created wl_buffer handle. 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. > > Signed-off-by: Varad Gautam <varad.gau...@collabora.com> > --- > unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml | 58 > +++++++++++++++++++--- > 1 file changed, 52 insertions(+), 6 deletions(-) > > diff --git a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml > b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml > index 3b4861f..a0aa42e 100644 > --- a/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml > +++ b/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml > @@ -26,13 +26,13 @@ > THIS SOFTWARE. > </copyright> > > - <interface name="zwp_linux_dmabuf_v1" version="1"> > + <interface name="zwp_linux_dmabuf_v1" version="2"> > <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 > 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. > > @@ -58,10 +58,16 @@ > 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 > + the 'add' request. Finally, a 'create' or 'create_immed' request is > + issued. Depending on the request, 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. > + provided, in case of a 'create' request. > + > + - reply with either a 'created_immed' event or a 'failed_immed' event > to > + notify successful wl_buffer creation, in case of a 'create_immed' > + request. The created wl_buffer is returned to the client by the > request. > > Warning! The protocol described in this file is experimental and > backward incompatible changes may be made. Backward compatible changes > @@ -106,7 +112,7 @@ > </event> > </interface> > > - <interface name="zwp_linux_buffer_params_v1" version="1"> > + <interface name="zwp_linux_buffer_params_v1" version="2"> > <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 > @@ -255,6 +261,25 @@ > <arg name="flags" type="uint" summary="see enum flags"/> > </request> > > + <request name="create_immed" since="2"> > + <description summary="create and return a wl_buffer from the given > dmabufs"> > + This asks for immediate creation of a wl_buffer from added dmabufs > and > + returns the newly created wl_buffer. On successful creation, a > + 'created_immed' event is triggered that can be used to verify that > the > + wl_buffer received from this request is a valid handle. A > 'failed_immed' > + event notifies unsuccessful dmabuf import. > + > + This takes the same arguments as a 'create' request, and obeys the > + same restrictions. > + </description> > + <arg name="buffer_id" type="new_id" interface="wl_buffer" > + summary="id for the newly created wl_buffer"/> > + <arg name="width" type="int" summary="base plane width in pixels"/> > + <arg name="height" type="int" summary="base plane height in pixels"/> > + <arg name="format" type="uint" summary="DRM_FORMAT code"/> > + <arg name="flags" type="uint" summary="see enum flags"/> > + </request> > + > <event name="created"> > <description summary="buffer creation succeeded"> > This event indicates that the attempted buffer creation was > @@ -277,6 +302,27 @@ > zlinux_buffer_params object. > </description> > </event> > + > + <event name="created_immed" since="2"> > + <description summary="immediate buffer creation succeeded"> > + This event notifies the success of 'create_immed' request. > + > + Upon receiving this event, the client should destroy the > + zlinux_dmabuf_params object. > + </description> > + </event> > + > + <event name="failed_immed" since="2"> > + <description summary="immediate buffer creation failed"> > + This event indicates that the attempted immediate buffer creation has > + failed. It usually means that one of the dmabuf constraints has not > + been fulfilled. > + > + Upon receiving this event, the client should destroy the > + zlinux_buffer_params object. > + </description> > + </event> > + > </interface> > > </protocol> Hi, Daniel asked me to take quick look at this, and I have a dilemma. The original intention is that one can create a wl_buffer without the roundtrip and use it. In that respect, there is no need for the "created_immed" and "failed_immed" events. No-one would ever inspect them anyway, they just assume things work. If things then don't work, the compositor should just fire a fatal protocol error. That will kill the client rather than letting it run oblivious of not actually showing anything. However, would there ever be need to have a non-fatal failure mode? Maybe if some vendor EGL used this internally and needs to expose errors via its API, rather than just cause a Wayland disconnection? If yes, then the events are both necessary. But it also raises the question of what will happen when a client commits a wl_buffer the compositor has failed? That case needs to be specified. Quite likely a client that cannot wait for a roundtrip will just render into the buffer and commit it ASAP, so the wl_buffer will be used before the client could receive any feedback. Feedback comes only after the fact, which is contrary to the fundamental design guidelines of Wayland, but so is the whole core idea of "create_immed" anyway. So there is justification for both ways: 1) things cannot fail, but if they do you die; 2) things should not fail, but if they do you get told only after the fact. I suspect we might want to go for the latter, but it really must be specified what it means when a client uses a failed wl_buffer, in this spec. Daniel suggested undefined behaviour, and I can agree. Weston can implement it by sending a fatal error, while someone who actually needs graceful failures can patch their compositor to, well, do whatever is best for them. Presumably the use cases for the graceful failure, like for "create_immed" to begin with, are embedded use cases where the vendor controls all the relevant software. Undefined behaviour should also discourage desktop software stacks from using "create_immed" without a roundtrip. By definition, using "create_immed" without a roundtrip risks glitching, and we don't want such software outside strictly embedded use cases. Thanks, pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel