[PATCH] wayland-drm: add a description for every item.
Pekka Paalanen
ppaalanen at gmail.com
Mon Apr 6 00:37:09 PDT 2015
On Wed, 25 Mar 2015 13:10:24 +0100
Emmanuel Gil Peyrot <linkmauve at linkmauve.fr> wrote:
> This makes the generated protocol headers a lot more readable.
> ---
> src/egl/wayland/wayland-drm/wayland-drm.xml | 159 +++++++++++++++++-----------
> 1 file changed, 100 insertions(+), 59 deletions(-)
>
> diff --git a/src/egl/wayland/wayland-drm/wayland-drm.xml b/src/egl/wayland/wayland-drm/wayland-drm.xml
> index 5e64622..7cf06d9 100644
> --- a/src/egl/wayland/wayland-drm/wayland-drm.xml
> +++ b/src/egl/wayland/wayland-drm/wayland-drm.xml
> @@ -27,19 +27,31 @@
> THIS SOFTWARE.
> </copyright>
>
> - <!-- drm support. This object is created by the server and published
> - using the display's global event. -->
> <interface name="wl_drm" version="2">
> + <description summary="drm support">
> + This object is created by the server and published using the
> + display's global event.
Hi,
you can say that a lot shorter: "This is a global object
interface." But you could also say more:
- is this a singleton?
- this is a detail of an EGL implementation
- this is specific to Mesa, an internal implementation detail that
no-one outside Mesa is supposed to access.
> + </description>
> +
> <enum name="error">
> - <entry name="authenticate_fail" value="0"/>
> - <entry name="invalid_format" value="1"/>
> - <entry name="invalid_name" value="2"/>
> + <description summary="error values">
> + These errors can be emitted in response to wl_drm requests.
That's obvious, isn't it?
> + </description>
> + <entry name="authenticate_fail" value="0"
> + summary="authentication failure"/>
> + <entry name="invalid_format" value="1"
> + summary="buffer format is not supported"/>
> + <entry name="invalid_name" value="2"
> + summary="invalid name for buffer creation"/>
Ok.
> </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. -->
> + <description summary="pixel formats">
> + 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.
> + </description>
Ok.
> <entry name="c8" value="0x20203843"/>
> <entry name="rgb332" value="0x38424752"/>
> <entry name="bgr233" value="0x38524742"/>
> @@ -100,84 +112,113 @@
> <entry name="yvu444" value="0x34325659"/>
> </enum>
>
> - <!-- Call this request with the magic received from drmGetMagic().
> - It will be passed on to the drmAuthMagic() or
> - DRIAuthConnection() call. This authentication must be
> - completed before create_buffer could be used. -->
> <request name="authenticate">
> - <arg name="id" type="uint"/>
> + <description summary="authentication request">
> + Call this request with the magic received from drmGetMagic().
> +
> + It will be passed on to the drmAuthMagic() or
> + DRIAuthConnection() call. This authentication must be
> + completed before create_buffer could be used.
> + </description>
> + <arg name="id" type="uint" summary="drm magic identifier"/>
Ok.
> </request>
>
> - <!-- Create a wayland buffer for the named DRM buffer. The DRM
> - surface must have a name using the flink ioctl -->
> <request name="create_buffer">
> - <arg name="id" type="new_id" interface="wl_buffer"/>
> - <arg name="name" type="uint"/>
> - <arg name="width" type="int"/>
> - <arg name="height" type="int"/>
> - <arg name="stride" type="uint"/>
> - <arg name="format" type="uint"/>
> + <description summary="create drm buffer">
> + Create a wayland buffer for the named DRM buffer.
> +
> + The DRM surface must have a name using the flink ioctl.
> + </description>
> + <arg name="id" type="new_id" interface="wl_buffer"
> + summary="wl_buffer assigned to this drm buffer"/>
> + <arg name="name" type="uint" summary="unique buffer name"/>
Would be much more explicit to say that is a flink name, not just
any freely chosen unique number.
> + <arg name="width" type="int" summary="buffer width"/>
> + <arg name="height" type="int" summary="buffer height"/>
Units?
> + <arg name="stride" type="uint" summary="stride of a line"/>
Units?
Without specifying the units, you are not really adding any
information here.
> + <arg name="format" type="uint" summary="see wl_drm_format"/>
> </request>
>
> - <!-- Create a wayland buffer for the named DRM buffer. The DRM
> - surface must have a name using the flink ioctl -->
> <request name="create_planar_buffer">
> - <arg name="id" type="new_id" interface="wl_buffer"/>
> - <arg name="name" type="uint"/>
> - <arg name="width" type="int"/>
> - <arg name="height" type="int"/>
> - <arg name="format" type="uint"/>
> - <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"/>
> + <description summary="create planar drm buffer">
> + Create a wayland buffer for the named planar DRM buffer.
> +
> + The DRM surface must have a name using the flink ioctl.
> + </description>
> + <arg name="id" type="new_id" interface="wl_buffer"
> + summary="wl_buffer assigned to this drm buffer"/>
> + <arg name="name" type="uint" summary="unique buffer name"/>
Flink name.
> + <arg name="width" type="int" summary="buffer width"/>
> + <arg name="height" type="int" summary="buffer height"/>
> + <arg name="format" type="uint" summary="see wl_drm_format"/>
> + <arg name="offset0" type="int" summary="first plane offset"/>
> + <arg name="stride0" type="int" summary="first plane stride"/>
> + <arg name="offset1" type="int" summary="second plane offset"/>
> + <arg name="stride1" type="int" summary="second plane stride"/>
> + <arg name="offset2" type="int" summary="third plane offset"/>
> + <arg name="stride2" type="int" summary="third plane stride"/>
Units?
> </request>
>
> - <!-- Notification of the path of the drm device which is used by
> - the server. The client should use this device for creating
> - local buffers. Only buffers created from this device should
> - be be passed to the server using this drm object's
> - create_buffer request. -->
> <event name="device">
> - <arg name="name" type="string"/>
> + <description summary="drm device of the server">
> + Notification of the path of the drm device which is used by the
> + server.
> +
> + The client should use this device for creating local buffers.
> + Only buffers created from this device should be be passed to
> + the server using this drm object's create_buffer request.
> + </description>
> + <arg name="name" type="string" summary="path of the drm device"/>
Ok.
> </event>
>
> <event name="format">
> - <arg name="format" type="uint"/>
> + <description summary="pixel format description">
> + Informs the client about a valid pixel format that can be used
> + for buffers.
> + </description>
> + <arg name="format" type="uint" summary="pixel format"/>
> </event>
>
> - <!-- Raised if the authenticate request succeeded -->
> - <event name="authenticated"/>
> + <event name="authenticated">
> + <description summary="successful authentication">
> + Raised if the authenticate request succeeded.
> + </description>
> + </event>
Ok.
>
> <enum name="capability" since="2">
> - <description summary="wl_drm capability bitmask">
> - Bitmask of capabilities.
> + <description summary="capability description">
> + Lists the available capabilities the server can expose.
You lost the most important word of the description: "bitmask". It
tells how to add new values to this enum.
> </description>
> <entry name="prime" value="1" summary="wl_drm prime available"/>
> </enum>
>
> <event name="capabilities">
> - <arg name="value" type="uint"/>
> + <description summary="capabilities bitmask">
> + Bitmask of capabilities supported by the server.
> + </description>
> + <arg name="value" type="uint" summary="capabilities"/>
Ok. Might add "see wl_drm_capability".
> </event>
>
> <!-- Version 2 additions -->
>
> - <!-- Create a wayland buffer for the prime fd. Use for regular and planar
> - buffers. Pass 0 for offset and stride for unused planes. -->
> <request name="create_prime_buffer" since="2">
> - <arg name="id" type="new_id" interface="wl_buffer"/>
> - <arg name="name" type="fd"/>
> - <arg name="width" type="int"/>
> - <arg name="height" type="int"/>
> - <arg name="format" type="uint"/>
> - <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"/>
> + <description summary="create prime drm buffer">
> + Create a wayland buffer for the prime fd.
> +
> + Use for regular and planar buffers. Pass 0 for offset and
> + stride for unused planes.
> + </description>
> + <arg name="id" type="new_id" interface="wl_buffer"
> + summary="wl_buffer assigned to this drm buffer"/>
> + <arg name="name" type="fd" summary="prime fd"/>
I'm not really sure what a "prime fd" is, care to explain it in the
description? Is it a dmabuf fd? Any additional semantics or
constraints?
> + <arg name="width" type="int" summary="buffer width"/>
> + <arg name="height" type="int" summary="buffer height"/>
> + <arg name="format" type="uint" summary="see wl_drm_format"/>
> + <arg name="offset0" type="int" summary="first plane offset"/>
> + <arg name="stride0" type="int" summary="first plane stride"/>
> + <arg name="offset1" type="int" summary="second plane offset"/>
> + <arg name="stride1" type="int" summary="second plane stride"/>
> + <arg name="offset2" type="int" summary="third plane offset"/>
> + <arg name="stride2" type="int" summary="third plane stride"/>
> </request>
>
> </interface>
I don't mind if you don't add the extra information I asked for.
So, if you put the "bitmask" back, then this would be:
Revieved-by: Pekka Paalanen <ppaalanen at gmail.com>
Thanks,
pq
PS. the proper mailing lists would be mesa-dev and wayland-devel.
dri-devel is more for the kernel stuff.
More information about the wayland-devel
mailing list