Re: [PATCH] drm/fourcc: Add DOC: overview comment

[Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com>]

On Tue, Aug 21, 2018 at 06:51:26PM +0200, Daniel Vetter wrote:
> On Tue, Aug 21, 2018 at 05:16:11PM +0100, Brian Starkey wrote:
> > There's a number of things which haven't previously been documented
> > around the usage of format modifiers. Capture the current
> > understanding in an overview comment and add it to the rst
> > documentation.
> > 
> > Ideally, the generated documentation would also include documentation
> > of all of the #defines, but the kernel-doc system doesn't currently
> > support kernel-doc comments on #define constants.
> > 
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > ---
> >  Documentation/gpu/drm-kms.rst |  6 ++++++
> >  include/uapi/drm/drm_fourcc.h | 36 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 42 insertions(+)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 1dffd1ac4cd4..1dd9f4824d3b 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -322,6 +322,12 @@ Frame Buffer Functions Reference
> >  DRM Format Handling
> >  ===================
> >  
> > +.. kernel-doc:: include/uapi/drm/drm_fourcc.h
> > +   :doc: overview
> > +
> > +Format Functions Reference
> > +--------------------------
> > +
> >  .. kernel-doc:: include/drm/drm_fourcc.h
> >     :internal:
> >  
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 894fa2da32fd..3f6c0b499323 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -30,6 +30,42 @@
> >  extern "C" {
> >  #endif
> >  
> > +/**
> > + * DOC: overview
> > + *
> > + * In the DRM subsystem, framebuffer pixel formats are described using the
> > + * fourcc codes defined in `include/uapi/drm/drm_fourcc.h`. In addition to the
> > + * fourcc code, a Format Modifier may optionally be provided, in order to
> > + * further describe the buffer's format - for example tiling or compression.
> > + *
> > + * Format Modifiers
> > + * ----------------
> > + *
> > + * Format modifiers are used in conjunction with a fourcc code, forming a
> > + * unique fourcc:modifier pair. This format:modifier pair must fully define the
> > + * format and data layout of the buffer, and should be the only way to describe
> > + * that particular buffer.
> > + *
> > + * Having multiple fourcc:modifier pairs which describe the same layout should
> > + * be avoided, as such aliases run the risk of different drivers exposing
> > + * different names for the same data format, forcing userspace to understand
> > + * that they are aliases.
> > + *
> > + * Format modifiers may change any property of the buffer, including the number
> > + * of planes and/or the required allocation size. Format modifiers are
> > + * vendor-namespaced, and as such the relationship between a fourcc code and a
> > + * modifier is specific to the modifer being used. For example, some modifiers
> > + * may preserve meaning - such as number of planes - from the fourcc code,
> > + * whereas others may not.
> > + *
> > + * Vendors are encouraged to document their modifier usage in as much detail as
> 
> I'd go with a slightly stronger "... should document ..." but either way:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'll leave pushing to one of the arm committers for drm-misc.
> -Daniel

Done.

> 
> > + * possible, to ensure maximum compatibility across devices, drivers and
> > + * applications.
> > + *
> > + * The authoritative list of format modifier codes is found in
> > + * `include/uapi/drm/drm_fourcc.h`
> > + */
> > +
> >  #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
> >  				 ((__u32)(c) << 16) | ((__u32)(d) << 24))
> >  
> > -- 
> > 2.16.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Cheers,
Alex G