On Wed, 25 Mar 2015 13:10:24 +0100 Emmanuel Gil Peyrot <linkmauve@xxxxxxxxxxxx> 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@xxxxxxxxx> Thanks, pq PS. the proper mailing lists would be mesa-dev and wayland-devel. dri-devel is more for the kernel stuff. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel