Wayland generic dmabuf protocol

[Pekka Paalanen]

On Mon, 9 Jun 2014 11:00:04 +0200
Benjamin Gaignard <benjamin.gaignard at linaro.org> wrote:

> Hi,
> 
> We are still interested in this topic too.

Hey,

cool. :-)

> One of the main comment on the latest patches was that wl_dmabuf use
> DRM for buffer allocation.
> This appear to be an issue since wayland doesn't want to rely on one
> specific framework (DRM, or V4L2) for buffer allocation, so we have
> start working on a "central dmabuf allocation" on kernel side. The
> goal is provide some as generic as possible to make it acceptable by
> wayland.

Why would Wayland need a central allocator for dmabuf?

A client can just export from any device/driver it wants (say, V4L),
push the dmabuf fd through the Wayland protocol, and the compositor
attempts to import it on a device it wants (say, via EGL dmabuf import
extension, or maybe DRM directly, or try a fallback to mmap if the
proper path fails).

Note that the proposal does not even carry any "compositor device" file
descriptor or device path or anything.

> For the protocol itself I think the expected behavior is the same than
> what you describe except maybe that buffer allocation will be done by
> using "central dmabuf allocator".

I don't think we need to even define what allocates/exports the
buffers in user space. Isn't that one point of dmabuf?

See https://bugzilla.gnome.org/show_bug.cgi?id=711155#c12 .

> On my hardware the patches you have (+ this one on gstwaylandsink
> https://bugzilla.gnome.org/show_bug.cgi?id=711155) allow me to do zero
> copy between the hardware video decoder and the display engine. I
> don't have implemented GPU yet because my hardware is able to do
> compose few video overlays planes and it was enough for my tests.

Right.

What I have been thinking is, that the compositor must be able to use
the new wl_buffer and we need to guarantee that before-hand. If the
compositor fails to use a wl_buffer when the client has already
attached it to a wl_surface and it is time to repaint, it is too late
and the user will see a glitch. Recovering from that requires asking
the client to provide a new wl_buffer of a different kind, which might
take time. Or a very rude compositor would just send a protocol error,
and then we'd get bug reports like "the video player just disappears
when I try to play (and ps. I have an old kernel that doesn't support
importing whatever)".

I believe we must allow the compositor to test the wl_buffer before it
is usable for the client. That is the reason for the roundtrippy design
of the below proposal.

Because we do not even try to communicate all the possible restrictions
to the client for it to match, we can leave the validation strictly as
a kernel internal issue. Buffer migration inside the kernel might even
magically solve some of the mismatches. It does leave the problem of
what can the client do, if it doesn't fill all the requirements for the
compositor to be able to import the dmabufs. But what restrictions
other than color format we can or should communicate, and where does
user space get them in the first place... *hand-waving*

But, this also leaves it up to the compositor to choose how/where it
wants to import the dmabufs. If a compositor is usually compositing
with GL, it will try to import with EGL on whatever GPU it is using. If
the compositor uses a software renderer, it can try to mmap the dmabufs
(or try this as a fallback, if the EGL import fails). If the compositor
is absolutely sure it can rely on the hardware display engine to
composite these buffers (note, buffers! You don't know to which
surfaces these buffers will be attached to), it can import directly
with DRM as FB objects, or V4L, or whatever. A compositor with the
fullscreen shell extension but without the sub-surface extension comes
to mind.

In summary, the compositor must be able to use the wl_buffer in its
default/fallback compositing path. If the wl_buffer is also suitable
for direct scanout, e.g. on an overlay, that is "just" a bonus.

With the round-trippy design, I am assuming that you can
export-pass-import a set of dmabufs once, and then reuse them as long
as you don't need to e.g. resize them. Is this a reasonable assumption?
Are there any, for instance, hardware video decoders that just insist on
exporting a new buffer for every frame?

I am tracking the proposal in
http://cgit.collabora.com/git/user/pq/weston.git/log/?h=linux_dmabuf

So far I added back the event to advertise the supported
drm_fourcc formats, since that is probably quite crucial.


Thanks,
pq

> 2014-06-06 17:30 GMT+02:00 Pekka Paalanen <pekka.paalanen at collabora.co.uk>:
> > Hi,
> >
> > the previous attempt at introducing a generic wl_dmabuf protocol to
> > Wayland didn't end too well:
> > http://lists.freedesktop.org/archives/wayland-devel/2013-December/012390.html
> > http://lists.freedesktop.org/archives/wayland-devel/2013-December/012455.html
> > http://lists.freedesktop.org/archives/wayland-devel/2013-December/012566.html
> > http://lists.freedesktop.org/archives/wayland-devel/2014-January/012727.html
> >
> > We are again interested in this, and I did a quick Friday evening draft
> > to open the discussion again. The base of the draft was a quick look at
> > https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
> >
> > The basic idea is, that a client has one or more dmabufs it wants to
> > share with the compositor, making up a single logical buffer (a single
> > image). The client chooses where and how to export those dmabufs. The
> > dmabuf fds and metadata are sent to the compositor, the compositor
> > assembles and tries to import them. If the import succeeds, a wl_buffer
> > object is created. If the import fails, the client is notified that the
> > compositor can't use these, it would be better to try something else.
> >
> > I assume that if the "import" succeeds, the compositor is able to use
> > the buffers, e.g. at least turn them into a GL-texture or mmap them, if
> > not also able to scan out or put on a hw overlay. This could be any kind
> > of checking to verify that the buffers are usable. Finding out that it
> > won't work after the client is already using the wl_buffer must not
> > happen, as we have no way to recover from it: the client will get
> > disconnected. So the point is knowing in advance, that the buffers are
> > usable on both sides, preferrably before the client has filled them with
> > data, but I suppose in the usual case the buffer is already filled.
> >
> > As creating a dmabuf-based wl_buffer requires a roundtrip in this
> > scheme, I assume it only needs to be done rarely, and the same buffer
> > can be re-used many times with proper synchronization.
> >
> > The crude draft is below. Some questions:
> > - Does this sound sane to you?
> > - What other metadata would we need? Thierry had some issues with
> >   tiling formats I think.
> > - This "check if the dmabuf is really usable" is needed, right? We
> >   can't just assume that any dmabuf will work?
> > - Do we need anything for fences here, or is the dmabuf fd enough?
> > - Does someone already have something similar running?
> >
> >
> > Thanks,
> > pq
> >
> >
> >
> > From b868dd816c38ec6e3741970cf3a29006fd10a263 Mon Sep 17 00:00:00 2001
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > Date: Fri, 6 Jun 2014 17:56:41 +0300
> > Subject: [PATCH weston] protocol: linux_dmabuf RFC
> >
> > ---
> >  protocol/linux-dmabuf.xml | 188 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 188 insertions(+)
> >  create mode 100644 protocol/linux-dmabuf.xml
> >
> > diff --git a/protocol/linux-dmabuf.xml b/protocol/linux-dmabuf.xml
> > new file mode 100644
> > index 0000000..643e5ca
> > --- /dev/null
> > +++ b/protocol/linux-dmabuf.xml
> > @@ -0,0 +1,188 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<protocol name="linux-dmabuf">
> > +
> > +  <copyright>
> > +    Copyright © 2008-2011 Kristian Høgsberg
> > +    Copyright © 2010-2011 Intel Corporation
> > +    Copyright © 2014 Collabora, Ltd.
> > +
> > +    Permission to use, copy, modify, distribute, and sell this
> > +    software and its documentation for any purpose is hereby granted
> > +    without fee, provided that the above copyright notice appear in
> > +    all copies and that both that copyright notice and this permission
> > +    notice appear in supporting documentation, and that the name of
> > +    the copyright holders not be used in advertising or publicity
> > +    pertaining to distribution of the software without specific,
> > +    written prior permission.  The copyright holders make no
> > +    representations about the suitability of this software for any
> > +    purpose.  It is provided "as is" without express or implied
> > +    warranty.
> > +
> > +    THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > +    SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > +    FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > +    SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > +    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> > +    AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> > +    ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> > +    THIS SOFTWARE.
> > +  </copyright>
> > +
> > +  <interface name="_linux_dmabuf_factory" 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.
> > +    </description>
> > +
> > +    <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. -->
> > +      <entry name="c8" value="0x20203843"/>
> > +      <entry name="rgb332" value="0x38424752"/>
> > +      <entry name="bgr233" value="0x38524742"/>
> > +      <entry name="xrgb4444" value="0x32315258"/>
> > +      <entry name="xbgr4444" value="0x32314258"/>
> > +      <entry name="rgbx4444" value="0x32315852"/>
> > +      <entry name="bgrx4444" value="0x32315842"/>
> > +      <entry name="argb4444" value="0x32315241"/>
> > +      <entry name="abgr4444" value="0x32314241"/>
> > +      <entry name="rgba4444" value="0x32314152"/>
> > +      <entry name="bgra4444" value="0x32314142"/>
> > +      <entry name="xrgb1555" value="0x35315258"/>
> > +      <entry name="xbgr1555" value="0x35314258"/>
> > +      <entry name="rgbx5551" value="0x35315852"/>
> > +      <entry name="bgrx5551" value="0x35315842"/>
> > +      <entry name="argb1555" value="0x35315241"/>
> > +      <entry name="abgr1555" value="0x35314241"/>
> > +      <entry name="rgba5551" value="0x35314152"/>
> > +      <entry name="bgra5551" value="0x35314142"/>
> > +      <entry name="rgb565" value="0x36314752"/>
> > +      <entry name="bgr565" value="0x36314742"/>
> > +      <entry name="rgb888" value="0x34324752"/>
> > +      <entry name="bgr888" value="0x34324742"/>
> > +      <entry name="xrgb8888" value="0x34325258"/>
> > +      <entry name="xbgr8888" value="0x34324258"/>
> > +      <entry name="rgbx8888" value="0x34325852"/>
> > +      <entry name="bgrx8888" value="0x34325842"/>
> > +      <entry name="argb8888" value="0x34325241"/>
> > +      <entry name="abgr8888" value="0x34324241"/>
> > +      <entry name="rgba8888" value="0x34324152"/>
> > +      <entry name="bgra8888" value="0x34324142"/>
> > +      <entry name="xrgb2101010" value="0x30335258"/>
> > +      <entry name="xbgr2101010" value="0x30334258"/>
> > +      <entry name="rgbx1010102" value="0x30335852"/>
> > +      <entry name="bgrx1010102" value="0x30335842"/>
> > +      <entry name="argb2101010" value="0x30335241"/>
> > +      <entry name="abgr2101010" value="0x30334241"/>
> > +      <entry name="rgba1010102" value="0x30334152"/>
> > +      <entry name="bgra1010102" value="0x30334142"/>
> > +      <entry name="yuyv" value="0x56595559"/>
> > +      <entry name="yvyu" value="0x55595659"/>
> > +      <entry name="uyvy" value="0x59565955"/>
> > +      <entry name="vyuy" value="0x59555956"/>
> > +      <entry name="ayuv" value="0x56555941"/>
> > +      <entry name="nv12" value="0x3231564e"/>
> > +      <entry name="nv21" value="0x3132564e"/>
> > +      <entry name="nv16" value="0x3631564e"/>
> > +      <entry name="nv61" value="0x3136564e"/>
> > +      <entry name="yuv410" value="0x39565559"/>
> > +      <entry name="yvu410" value="0x39555659"/>
> > +      <entry name="yuv411" value="0x31315559"/>
> > +      <entry name="yvu411" value="0x31315659"/>
> > +      <entry name="yuv420" value="0x32315559"/>
> > +      <entry name="yvu420" value="0x32315659"/>
> > +      <entry name="yuv422" value="0x36315559"/>
> > +      <entry name="yvu422" value="0x36315659"/>
> > +      <entry name="yuv444" value="0x34325559"/>
> > +      <entry name="yvu444" value="0x34325659"/>
> > +    </enum>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="unbind the factory">
> > +      </description>
> > +    </request>
> > +
> > +    <request name="create_buffer">
> > +      <description summary="start creating a wl_buffer">
> > +        Any errors are returned in the dmabuf_proxy object as non-fatal.
> > +      </description>
> > +      <arg name="stub" type="new_id" interface="dmabuf_proxy"/>
> > +      <arg name="width" type="int"/>
> > +      <arg name="height" type="int"/>
> > +      <arg name="format" type="uint" summary="from format enum"/>
> > +    </request>
> > +  </interface>
> > +
> > +  <interface name="dmabuf_proxy" version="1">
> > +    <description summary="a placeholder logical buffer object">
> > +      As a logical buffer cannot always be create in a single
> > +      request, this object acts as a temporary logical buffer object,
> > +      that in the end will be turned into a wl_buffer object.
> > +
> > +      If the server cannot handle the set of dmabufs and the other
> > +      arguments, a non-fatal error is returned instead of the
> > +      new wl_buffer object. After either reply, this temporary object
> > +      must be destroyed.
> > +    </description>
> > +
> > +    <enum name="error">
> > +      <entry name="invalid_format" value="1"/>
> > +      <entry name="invalid_name" value="2"/>
> > +    </enum>
> > +
> > +    <request name="add_dmabuf">
> > +      <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_proxy.
> > +
> > +        When one dmabuf has several planar channels, offset1 & stride1 and
> > +        offset2 and 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>
> > +
> > +    <request name="finish">
> > +      <description summary="finish creating the wl_buffer">
> > +        This tells the server that all dmabufs have been added,
> > +        and the server can proceed creating the wl_buffer.
> > +        The server will reply with an event, either "created" or "failed".
> > +      </description>
> > +    </request>
> > +
> > +    <event name="created">
> > +      <description summary="the wl_buffer was created">
> > +        The server has registered and tested the set of dmabufs, the format,
> > +        etc. and found that it can use the resulting buffer.
> > +
> > +        The dmabuf_proxy object must be destroyed, and not used again.
> > +      </description>
> > +      <arg name="id" type="new_id" interface="wl_buffer"
> > +           summary="the new wl_buffer"/>
> > +    </event>
> > +
> > +    <event name="failed">
> > +      <description summary="finish creating the wl_buffer">
> > +        The dmabuf_proxy object must be destroyed, and not used again.
> > +      </description>
> > +      <!-- TODO: add some failure indicators: bad dmabuf, format, incomplete...
> > +       -->
> > +    </event>
> > +
> > +  </interface>
> > +</protocol>
> > --