[PATCH v2 2/5] drm: Centralize format information

[Eric Engestrom]

On Wed, Jun 08, 2016 at 02:35:35AM +0300, Laurent Pinchart wrote:
> Various pieces of information about DRM formats (number of planes, color
> depth, chroma subsampling, ...) are scattered across different helper
> functions in the DRM core. Callers of those functions often need to
> access more than a single parameter of the format, leading to
> inefficiencies due to multiple lookups.
> 
> Centralize all format information in a data structure and create a
> function to look up information based on the format 4CC.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_fourcc.h     | 21 +++++++++++
>  2 files changed, 105 insertions(+)
> 

[snip]

> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 7f90a396cf2b..d33411bd19b1 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -25,6 +25,27 @@
>  #include <linux/types.h>
>  #include <uapi/drm/drm_fourcc.h>
>  
> +/**
> + * struct drm_format_info - information about a DRM format
> + * @format: 4CC format identifier (DRM_FORMAT_*)
> + * @depth: color depth (number of bits per pixel excluding padding bits)
> + * @bpp: number of bits per pixel including padding
> + * @num_planes: number of color planes (1 to 3)
> + * @cpp: number of bytes per pixel (per plane)
> + * @hsub: horizontal chroma subsampling factor
> + * @vsub: vertical chroma subsampling factor
> + */
> +struct drm_format_info {
> +	u32 format;
> +	u8 depth;
> +	u8 bpp;

I think `bpp` is redundant here: it's always `cpp[0] << 3`, except when
ignored (yuv formats), which `depth` already indicates by being 0.

Unless I missed something, the only change required to drop this field
would be in patch #3, in drm_fb_get_bpp_depth():

	*bpp = info->bpp;

becomes:

	*bpp = info->depth ? info->cpp[0] << 3 : 0;

I'm not really opposed to it being here if someone else finds it useful
though, so either way, the series is:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> +	u8 num_planes;
> +	u8 cpp[3];
> +	u8 hsub;
> +	u8 vsub;
> +};
> +
> +const struct drm_format_info *drm_format_info(u32 format);
>  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
>  int drm_format_num_planes(uint32_t format);
>  int drm_format_plane_cpp(uint32_t format, int plane);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel