Re: [PATCH v5 04/13] drm: WARN when calling drm_format_info() for an unsupported format

[Eric Engestrom <eric.engestrom@imgtec.com>]

On Tuesday, 2016-10-18 01:41:12 +0300, Laurent Pinchart wrote:
> The format helpers have historically treated unsupported formats as part
> of the default case, returning values that are likely wrong. We can't
> change this behaviour now without risking breaking drivers in difficult
> to detect ways, but we can WARN on unsupported formats to catch faulty
> callers.
> 
> The only exception is the framebuffer_check() function that calls
> drm_format_info() to validate the format passed from userspace. This is
> a valid use case that shouldn't generate a warning.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> ---
>  drivers/gpu/drm/drm_fourcc.c      | 32 ++++++++++++++++++++++++--------
>  drivers/gpu/drm/drm_framebuffer.c |  2 +-
>  include/drm/drm_fourcc.h          |  1 +
>  3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 23d4b82ec17c..523ed916a1c0 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -102,15 +102,11 @@ char *drm_get_format_name(uint32_t format)
>  }
>  EXPORT_SYMBOL(drm_get_format_name);
>  
> -/**
> - * drm_format_info - query information for a given format
> - * @format: pixel format (DRM_FORMAT_*)
> - *
> - * Returns:
> - * The instance of struct drm_format_info that describes the pixel format, or
> - * NULL if the format is unsupported.
> +/*
> + * Internal function to query information for a given format. See
> + * drm_format_info() for the public API.
>   */
> -const struct drm_format_info *drm_format_info(u32 format)
> +const struct drm_format_info *__drm_format_info(u32 format)
>  {
>  	static const struct drm_format_info formats[] = {
>  		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
> @@ -184,6 +180,26 @@ const struct drm_format_info *drm_format_info(u32 format)
>  
>  	return NULL;
>  }
> +
> +/**
> + * drm_format_info - query information for a given format
> + * @format: pixel format (DRM_FORMAT_*)
> + *
> + * The caller should only pass a supported pixel format to this function.
> + * Unsupported pixel formats will generate a warning in the kernel log.
> + *
> + * Returns:
> + * The instance of struct drm_format_info that describes the pixel format, or
> + * NULL if the format is unsupported.
> + */
> +const struct drm_format_info *drm_format_info(u32 format)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = __drm_format_info(format);
> +	WARN_ON(!info);
> +	return info;
> +}
>  EXPORT_SYMBOL(drm_format_info);
>  
>  /**
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 386977df72ce..49fd7db758e0 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -131,7 +131,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>  	const struct drm_format_info *info;
>  	int i;
>  
> -	info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
> +	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>  	if (!info) {
>  		char *format_name = drm_get_format_name(r->pixel_format);
>  		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 135fef050ee6..f73f97afd1e2 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,7 @@ struct drm_format_info {
>  	u8 vsub;
>  };
>  
> +const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
>  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel