[v4,04/14] drm: WARN when calling drm_format_info() for an unsupported format

Commit Message

Laurent Pinchart Sept. 8, 2016, 2:44 p.m. UTC

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 <[e-mail]>
---
 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(-)

Comments

Tomi Valkeinen Sept. 15, 2016, 9:33 a.m. UTC | #1

On 08/09/16 17:44, 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 <[e-mail]>
> ---
>  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(-)

Reviewed-by: Tomi Valkeinen <[e-mail]>

 Tomi

Patch

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index bf91c5044d84..0355b7ede8e4 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 ce8045228642..da29bdac2009 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -105,7 +105,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 4e79d6159856..7ffb114d2468 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -43,6 +43,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);