On Thu, 18 Dec 2014 10:25:09 +0100 Daniel Vetter <dan...@ffwll.ch> wrote:
> On Fri, Dec 12, 2014 at 04:51:02PM -0500, Louis-Francis > Ratté-Boulianne wrote: > > From: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > > > An experimental (hence the 'z' prefix) linux_dmabuf Wayland protocol > > extension for creating dmabuf-based wl_buffers in a generic manner. > > > > This does not include proper dmabuf metadata negotiation because > > there is no way to communicate all dmabuf constraints from the > > compositor to a client before-hand. The client has to create a > > wl_buffer wrapping one or more dmabuf buffers and then listen at > > the feedback object returned to know if the operation was > > successful. > > > > Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > Signed-off-by: Louis-Francis Ratté-Boulianne <l...@collabora.com> > > So I have no idea about how wayland protos work, so please take that > into account ;-) Hi Daniel, very much appreciated anyway! > Generally I think we should try to follow what the drm addfb2 ioctl > does in drm, since in the end we need to be able to handle pretty > much all the same issues. Well wayland needs to solve a few more > since it also must cope with buffer layouts which can only be used > for rendering but not scanned out. Yes. The current proposal was written based on https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt which has some differences compared to addfb2, like having only 3 planes instead of 4. > Wrt tiling layouts and compressed buffers and similar vendor specific > things: The current proposal is to add a new uint64_t layout > qualifier to addfb with opaque #defines (probably a new header), with > an 8:56 bit split between vendor identifier and opaque vendor > specified content. It will also be per-buffer (like pitches/offsets). > The abi isn't locked down yet, but definitely something to keep in > mind. Yup, I've seen it. > It will definitely complicate the protocol though since the specific > layout modifiers which are acceptable change dynamically at runtime: > Some scanout formats become invalid when we run out of fifo space, > some can only be used on specific planes and ofc this all depends > upon whether gl compositing or hw planes are used to pick the right > one. From the Wayland compositor point of view, things would get awfully hard if an existing wl_buffer (handle to a dmabuf object) could suddenly become completely unusable. That is why we try to make sure during the wl_buffer creation phase (when a client initially shares the dmabuf with the compositor) that the buffer is always usable for at least compositing (i.e. as a GL texture). If the buffer is also usable for direct scanout, that's a nice bonus. It's no problem to fall back to compositing if a buffer suddenly or temporarily turns out to be not scanout-able. We just need to guarantee that the compositing path always works. I don't think we can say to a client "oops, go make a different kind of buffer, this no longer works". Do you think these expectations are realistic? > > Some more random comments below. > > Cheers, Daniel > > > --- > > Makefile.am | 7 +- > > protocol/linux-dmabuf.xml | 224 > > ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 > > insertions(+), 2 deletions(-) create mode 100644 > > protocol/linux-dmabuf.xml > > > > diff --git a/Makefile.am b/Makefile.am > > index 494266d..0462fdd 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -91,7 +91,9 @@ nodist_weston_SOURCES > > = \ > > protocol/presentation_timing-protocol.c \ > > protocol/presentation_timing-server-protocol.h \ > > protocol/scaler-protocol.c \ > > - protocol/scaler-server-protocol.h > > + protocol/scaler-server-protocol.h \ > > + protocol/linux-dmabuf-protocol.c \ > > + protocol/linux-dmabuf-server-protocol.h > > > > BUILT_SOURCES += $(nodist_weston_SOURCES) > > > > @@ -1101,7 +1103,8 @@ EXTRA_DIST > > += \ > > protocol/presentation_timing.xml \ > > protocol/scaler.xml \ > > protocol/ivi-application.xml \ > > - protocol/ivi-hmi-controller.xml > > + protocol/ivi-hmi-controller.xml \ > > + protocol/linux-dmabuf.xml > > > > man_MANS = weston.1 weston.ini.5 > > > > diff --git a/protocol/linux-dmabuf.xml b/protocol/linux-dmabuf.xml > > new file mode 100644 > > index 0000000..c48121c > > --- /dev/null > > +++ b/protocol/linux-dmabuf.xml > > @@ -0,0 +1,224 @@ > > +<?xml version="1.0" encoding="UTF-8"?> > > +<protocol name="linux_dmabuf"> > > + > > + <interface name="zlinux_dmabuf" version="1"> > > + <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 > > + > > + This interface offers a way 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. > > + </description> > > + > > + <enum name="error"> > > + <entry name="invalid_format" value="1"/> > > + <entry name="invalid_dmabuf" value="2"/> > > + <entry name="format_dmabufs_mismatch" value="3"/> > > + <entry name="invalid_dmabuf_params" value="4"/> > > + </enum> > > + > > + <enum name="format"> > > + <!-- The drm format codes match the #defines in drm_fourcc.h. > > + The formats actually supported by the compositor will be > > + reported by the format event. --> > > Is there some way we could autogenerate this from drm_fourcc.h? We > need to do changes to the kernel header to facilitate that we should > do so imo. > > Or should we just mandate that this must be a fourcc code from > drm_fourcc.h and not list them all as enums? Since this by definition must be the same as drm_fourcc.h values, and drm_fourcc.h is always present if there is dmabuf support, I think you're right. We could simply specify, that the values are defined by drm_fourcc.h, and drop this whole list. > > + </enum> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="unbind the factory"> > > + </description> > > + </request> > > + > > + <request name="create_batch"> > > + <description summary="create a collection of dmabufs"> > > + This temporary object is used to collect multiple dmabuf > > handles > > + into a single batch. > > + </description> > > + <arg name="batch_id" type="new_id" interface="dmabuf_batch"/> > > + </request> > > + > > + <request name="create_buffer"> > > + <description summary="create a dmabuf-based wl_buffer"> > > + The result of the operation will be returned via the > > provided > > + zlinux_dmabuf_create_feedback object. > > + > > + The dmabuf_batch object must be destroyed immediately after > > + after this request. > > + > > + Any errors in importing the set of dmabufs can be > > delivered as > > + protocol errors immediately or later. > > + </description> > > + <arg name="batch" type="object" interface="dmabuf_batch"/> > > + <arg name="width" type="int"/> > > + <arg name="height" type="int"/> > > + <arg name="format" type="uint" summary="from format enum"/> > > + <arg name="feedback" type="new_id" > > interface="zlinux_dmabuf_create_feedback"/> > > + </request> > > + > > + <event name="format"> > > + <description summary="supported buffer format"> > > + This event advertises one buffer format that the server > > support. > > + All the supported formats advertised once when the client > > + binds to this interface. A roundtrip after binding > > guarantees, > > + that the client has received all supported formats. > > + </description> > > + <arg name="format" type="uint" summary="from format enum"/> > > + </event> > > + > > + </interface> > > + <interface name="dmabuf_batch" version="1"> > > + <description summary="a collection of dmabufs"> > > + This is a collection of dmabufs, forming a single logical > > buffer. > > + Usually only one dmabuf is needed, but some multi-planar > > formats > > + may require more. > > + > > + The order of dmabufs added for this object is significant, > > and must > > + match the expectations of the format argument to > > + linux_dmabuf.create_buffer. > > + </description> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="delete this object, used or not"> > > + </description> > > + </request> > > + > > + <request name="add"> > > + <description summary="add a dmabuf to the wl_buffer"> > > + Multi-planar formats may require using more than one > > + dmabuf for passing all the data for one logical buffer. > > + This request adds one dmabuf to the set in this > > dmabuf_batch. + > > + When one dmabuf has several planar channels, offset1 & > > stride1 and > > + offset2 & stride2 must be used to denote them without > > sending a > > + new add_dmabuf request which would create a new fd in the > > server > > + while still pointing at the same dmabuf. > > + > > + offset0 and stride0 must always be set. Other unused > > offsets and > > + strides must be zero. > > + </description> > > + > > + <arg name="name" type="fd"/> > > + <arg name="offset0" type="int"/> > > + <arg name="stride0" type="int"/> > > + <arg name="offset1" type="int"/> > > + <arg name="stride1" type="int"/> > > + <arg name="offset2" type="int"/> > > + <arg name="stride2" type="int"/> > > + </request> > > Hm, with addfb we just ask userspace to pass the same fb 3/2 times for > planar data that's all in the same buffer object. Yeah, I chose to handle the fds separately, if that is what you mean. The file descriptors are passed through unix socket SCM_RIGHTS messages, and libwayland deliberately also dup's them. If a client sends the same fd several times, the compositor will get different fds all pointing to the same "file". Can that be a problem for GBM, EGL, or addfb2 et al. dma_buf import? Or rather, an implementation that makes it a problem, it legal or buggy? I assumed it could be a problem, so I designed a way that maintains the numerical identities between the fds. Also, we don't have a mechanism to send "not a valid fd" for an fd field, in case there are less than 3 (4?) planes, so we need a separate request for each valid fd. Thanks, pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel