[PATCH weston v2 1/8] protocol: add linux_dmabuf extension (v2)
Daniel Vetter
daniel at ffwll.ch
Wed Jul 8 01:02:35 PDT 2015
On Wed, Jul 01, 2015 at 05:56:13PM +0300, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at 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.
>
> RFCv1 changes (after a first draft without code):
> - some renames of interfaces and argument, no semantic changes
> - added destructor protocol to dmabuf_batch
> - added feedback interface for buffer creation
>
> v2 changes:
> - use drm_fourcc.h as authoritative source for format codes
> - add support for the 64-bit layout qualifier and y-inverted dmabufs
> - simplify the 'add' request (no need to preserve fd numerical id)
> - add explicit plane index in the 'add' request
> - integrate the 'feedback' object events to the batch interface
> - rename 'create_buffer' to 'create' and move it into the batch interface
> - add requirements needed from the graphics stack and clients
> - improve existing errors and add batch error codes
> - removed error codes from the global interface
> - improve documentation for arguments, enums, etc.
> - rename dmabuf_batch to zlinux_buffer_params
> - The y-inverted property makes more sense as a whole buffer property.
> Y-flipping individual planes of the same buffer object is hardly useful.
> The y-invert is also converted into a flag, so we may add more flags
> later.
> - add flags for interlaced buffer content
>
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> ---
> Makefile.am | 7 +-
> protocol/linux-dmabuf.xml | 279 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 284 insertions(+), 2 deletions(-)
> create mode 100644 protocol/linux-dmabuf.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index f493d16..50385db 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -107,7 +107,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)
>
> @@ -1242,7 +1244,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
>
> #
> # manual test modules in tests subdirectory
> diff --git a/protocol/linux-dmabuf.xml b/protocol/linux-dmabuf.xml
> new file mode 100644
> index 0000000..efa619d
> --- /dev/null
> +++ b/protocol/linux-dmabuf.xml
> @@ -0,0 +1,279 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="linux_dmabuf">
> +
> + <copyright>
> + Copyright © 2014, 2015 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="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
> + and the Linux DRM sub-system's AddFb2 ioctl.
> +
> + 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.
> +
> + Clients must ensure the buffer is coherent with respect to all possible
> + consumers before use. The following steps are needed:
This seems a really strong requirement ...
> +
> + - Create a new buffer, or wait until existing buffer has been released.
> +
> + - Write pixel data to buffer (e.g. V4L2 queue and dequeue)
> +
> + - Make platform-specific call to ensure that caches are flushed, such
> + that access with GPU / display controller / CPU will be coherent, if
> + read-side caches are flushed on those devices (which is explicitly
> + the compositor's responsibility).
and I don't understand this really. Currently the dma-buf design is that
coherency is managed in the kernel, as long as you don't play tricks (like
what mesa might internally do for partial uploads). That will also extend
to a dma-buf mmap extension where we'll add explicit begin/end_cpu_access
ioctls for cpu-side access. If you really want to talk about coherency I'd
go with:
- clients must ensure that either all data in the dma-buf is
coherent for all subsequent read access or that coherency is
correctly handled by the underlying kernel-side dma-buf
implementation.
Or just list this as an open issue. With that addressed this is
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> +
> + - Don't make any more attachments after sending the buffer to the
> + compositor. Making more attachments later increases the risk of
> + the compositor not being able to use (re-import) an existing
> + dmabuf-based wl_buffer.
> +
> + The underlying graphics stack must ensure the following:
> +
> + - The dmabuf file descriptors relayed to the server will stay valid
> + for the whole lifetime of the wl_buffer. This means the server may
> + at any time use those fds to import the dmabuf into any kernel
> + sub-system that might accept it.
> +
> + To create a wl_buffer from one or more dmabufs, a client creates a
> + zlinux_dmabuf_params object with zlinux_dmabuf.create_params
> + request. All planes required by the intended format are added with
> + the 'add' request. Finally, 'create' request is issued. The server
> + will reply with either 'created' event which provides the final
> + wl_buffer or 'failed' event saying that it cannot use the dmabufs
> + provided.
> + </description>
> +
> + <request name="destroy" type="destructor">
> + <description summary="unbind the factory">
> + Objects created through this interface, especially wl_buffers, will
> + remain valid.
> + </description>
> + </request>
> +
> + <request name="create_params">
> + <description summary="create a temporary object for buffer parameters">
> + This temporary object is used to collect multiple dmabuf handles into
> + a single batch to create a wl_buffer. It can only be used once and
> + should be destroyed after an 'created' or 'failed' event has been
> + received.
> + </description>
> + <arg name="params_id" type="new_id" interface="zlinux_buffer_params"
> + summary="the new temporary"/>
> + </request>
> +
> + <event name="format">
> + <description summary="supported buffer format">
> + This event advertises one buffer format that the server supports.
> + 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 formats.
> +
> + For the definition of the format codes, see create request.
> +
> + XXX: Can a compositor ever enumerate them?
> + </description>
> + <arg name="format" type="uint" summary="DRM_FORMAT code"/>
> + </event>
> +
> + </interface>
> +
> + <interface name="zlinux_buffer_params" version="1">
> + <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
> + object may eventually create one wl_buffer unless cancelled by
> + destroying it before requesting 'create'.
> +
> + Single-planar formats only require one dmabuf, however
> + multi-planar formats may require more than one dmabuf. For all
> + formats, 'add' request must be called once per plane (even if the
> + underlying dmabuf fd is identical).
> +
> + You must use consecutive plane indices ('plane_idx' argument for 'add')
> + from zero to the number of planes used by the drm_fourcc format code.
> + All planes required by the format must be given exactly once, but can
> + be given in any order. Each plane index can be set only once.
> + </description>
> +
> + <enum name="error">
> + <entry name="already_used" value="0"
> + summary="the dmabuf_batch object has already been used to create a wl_buffer"/>
> +
> + <entry name="plane_idx" value="1"
> + summary="plane index out of bounds"/>
> +
> + <entry name="plane_set" value="2"
> + summary="the plane index was already set"/>
> +
> + <entry name="incomplete" value="3"
> + summary="missing or too many planes to create a buffer"/>
> +
> + <entry name="invalid_format" value="4"
> + summary="format not supported"/>
> +
> + <entry name="invalid_dimensions" value="5"
> + summary="invalid width or height"/>
> +
> + <entry name="out_of_bounds" value="6"
> + summary="offset + stride * height goes out of dmabuf bounds"/>
> + </enum>
> +
> + <request name="destroy" type="destructor">
> + <description summary="delete this object, used or not">
> + Cleans up the temporary data sent to the server for dmabuf-based
> + wl_buffer creation.
> + </description>
> + </request>
> +
> + <request name="add">
> + <description summary="add a dmabuf to the temporary set">
> + This request adds one dmabuf to the set in this zlinux_buffer_params.
> +
> + The 64-bit unsigned value combined from modifier_hi and modifier_lo
> + is the dmabuf layout modifier. DRM AddFB2 ioctl calls this the
> + fb modifier, which is defined in drm_mode.h of Linux UAPI.
> + This is an opaque token. Drivers use this token to express tiling,
> + compression, etc. driver-specific modifications to the base format
> + defined by the DRM fourcc code.
> +
> + This request raises the PLANE_IDX error if plane_idx is too large.
> + The error PLANE_SET is raised if attempting to set a plane that
> + was already set.
> + </description>
> +
> + <arg name="fd" type="fd" summary="dmabuf fd"/>
> + <arg name="plane_idx" type="uint" summary="plane index"/>
> + <arg name="offset" type="uint" summary="offset in bytes"/>
> + <arg name="stride" type="uint" summary="stride in bytes"/>
> + <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"/>
> + </request>
> +
> + <enum name="flags">
> + <entry name="y_invert" value="1" summary="contents are y-inverted"/>
> + <entry name="interlaced" value="2" summary="content is interlaced"/>
> + <entry name="bottom_first" value="4" summary="bottom field first"/>
> + </enum>
> +
> + <request name="create">
> + <description summary="create a wl_buffer from the given dmabufs">
> + This asks for creation of a wl_buffer from the added dmabuf
> + buffers. The wl_buffer is not created immediately but returned via
> + the 'created' event if the dmabuf sharing succeeds. The sharing
> + may fail at runtime for reasons a client cannot predict, in
> + which case the 'failed' event is triggered.
> +
> + The 'format' argument is a DRM_FORMAT code, as defined by the
> + libdrm's drm_fourcc.h. The Linux kernel's DRM sub-system is the
> + authoritative source on how the format codes should work.
> +
> + The 'flags' is a bitfield of the flags defined in enum "flags".
> + 'y_invert' means the that the image needs to be y-flipped.
> +
> + Flag 'interlaced' means that the frame in the buffer is not
> + progressive as usual, but interlaced. An interlaced buffer as
> + supported here must always contain both top and bottom fields.
> + The top field always begins on the first pixel row. The temporal
> + ordering between the two fields is top field first, unless
> + 'bottom_first' is specified. It is undefined whether 'bottom_first'
> + is ignored if 'interlaced' is not set.
> +
> + This protocol does not convey any information about field rate,
> + duration, or timing, other than the relative ordering between the
> + two fields in one buffer. A compositor may have to estimate the
> + intended field rate from the incoming buffer rate. It is undefined
> + whether the time of receiving wl_surface.commit with a new buffer
> + attached, applying the wl_surface state, wl_surface.frame callback
> + trigger, presentation, or any other point in the compositor cycle
> + is used to measure the frame or field times. There is no support
> + for detecting missed or late frames/fields/buffers either, and
> + there is no support whatsoever for cooperating with interlaced
> + compositor output.
> +
> + The composited image quality resulting from the use of interlaced
> + buffers is explicitly undefined. A compositor may use elaborate
> + hardware features or software to deinterlace and create progressive
> + output frames from a sequence of interlaced input buffers, or it
> + may produce substandard image quality. However, compositors that
> + cannot guarantee reasonable image quality in all cases are recommended
> + to just reject all interlaced buffers.
> +
> + Any argument errors, including non-positive width or height,
> + mismatch between the number of planes and the format, bad
> + format, bad offset or stride, may be indicated by fatal protocol
> + errors: INCOMPLETE, INVALID_FORMAT, INVALID_DIMENSIONS,
> + OUT_OF_BOUNDS.
> +
> + Dmabuf import errors in the server that are not obvious client
> + bugs are returned via the 'failed' event as non-fatal. This
> + allows attempting dmabuf sharing and falling back in the client
> + if it fails.
> +
> + This request can be sent only once in the object's lifetime, after
> + which the only legal request is destroy. This object should be
> + destroyed after issuing 'create' request. Attempting to use this
> + object after issuing 'create' raises ALREADY_USED protocol error.
> +
> + It is not mandatory to issue 'create'. If a client wants to
> + cancel the buffer creation, it can just destroy this object.
> + </description>
> + <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
> + successful. It provides the new wl_buffer referencing the dmabuf(s).
> +
> + Upon receiving this event, the client should destroy the
> + zlinux_dmabuf_params object.
> + </description>
> + <arg name="buffer" type="new_id" interface="wl_buffer"
> + summary="the newly created wl_buffer"/>
> + </event>
> +
> + <event name="failed">
> + <description summary="buffer creation failed">
> + This event indicates that the attempted 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>
> --
> 2.3.6
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the wayland-devel
mailing list