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

[Daniel Vetter <daniel@ffwll.ch>]

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

> + * 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