Re: [PATCH] wayland-drm: add a description for every item.

On Wed, 25 Mar 2015 13:10:24 +0100 Emmanuel Gil Peyrot linkmauve@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@gmail.com

Thanks, pq

PS. the proper mailing lists would be mesa-dev and wayland-devel. dri-devel is more for the kernel stuff.