Re: [PATCH 1/9] drm: Add mode_config .get_format_info() hook

[Ben Widawsky <ben@bwidawsk.net>]

On 17-01-04 20:42:24, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Allow drivers to return a custom drm_format_info structure for special
>fb layouts. We'll use this for the compression control surface in i915.
>
>v2: Fix drm_get_format_info() kernel doc (Laurent)
>    Don't pass 'dev' to the new hook (Laurent)
>
>Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>Cc: Ben Widawsky <ben@bwidawsk.net>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/drm_fb_cma_helper.c  |  2 +-
> drivers/gpu/drm/drm_fourcc.c         | 25 +++++++++++++++++++++++++
> drivers/gpu/drm/drm_framebuffer.c    |  9 +++++++--
> drivers/gpu/drm/drm_modeset_helper.c |  2 +-
> include/drm/drm_fourcc.h             |  6 ++++++
> include/drm/drm_mode_config.h        | 14 ++++++++++++++
> 6 files changed, 54 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
>index 20a4011f790d..de3c9fe116fc 100644
>--- a/drivers/gpu/drm/drm_fb_cma_helper.c
>+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>@@ -177,7 +177,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> 	int ret;
> 	int i;
>
>-	info = drm_format_info(mode_cmd->pixel_format);
>+	info = drm_get_format_info(dev, mode_cmd);
> 	if (!info)
> 		return ERR_PTR(-EINVAL);
>
>diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>index 90d2cc8da8eb..f9b6445e846a 100644
>--- a/drivers/gpu/drm/drm_fourcc.c
>+++ b/drivers/gpu/drm/drm_fourcc.c
>@@ -199,6 +199,31 @@ const struct drm_format_info *drm_format_info(u32 format)
> EXPORT_SYMBOL(drm_format_info);
>
> /**
>+ * drm_get_format_info - query information for a given framebuffer configuration
>+ * @dev: DRM device
>+ * @mode_cmd: metadata from the userspace fb creation request
>+ *
>+ * 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_get_format_info(struct drm_device *dev,
>+		    const struct drm_mode_fb_cmd2 *mode_cmd)
>+{
>+	const struct drm_format_info *info = NULL;
>+
>+	if (dev->mode_config.funcs->get_format_info)
>+		info = dev->mode_config.funcs->get_format_info(mode_cmd);
>+
>+	if (!info)
>+		info = drm_format_info(mode_cmd->pixel_format);
>+
>+	return info;
>+}
>+EXPORT_SYMBOL(drm_get_format_info);
>+
>+/**
>  * drm_format_num_planes - get the number of planes for format
>  * @format: pixel format (DRM_FORMAT_*)
>  *
>diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
>index 588ccc3a2218..e276fcdc3a62 100644
>--- a/drivers/gpu/drm/drm_framebuffer.c
>+++ b/drivers/gpu/drm/drm_framebuffer.c
>@@ -126,11 +126,13 @@ int drm_mode_addfb(struct drm_device *dev,
> 	return 0;
> }
>
>-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>+static int framebuffer_check(struct drm_device *dev,
>+			     const struct drm_mode_fb_cmd2 *r)
> {
> 	const struct drm_format_info *info;
> 	int i;
>
>+	/* check if the format is supported at all */
> 	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
> 	if (!info) {
> 		struct drm_format_name_buf format_name;
>@@ -140,6 +142,9 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
> 		return -EINVAL;
> 	}
>
>+	/* now let the driver pick its own format info */
>+	info = drm_get_format_info(dev, r);
>+
> 	if (r->width == 0 || r->width % info->hsub) {
> 		DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
> 		return -EINVAL;
>@@ -263,7 +268,7 @@ drm_internal_framebuffer_create(struct drm_device *dev,
> 		return ERR_PTR(-EINVAL);
> 	}
>
>-	ret = framebuffer_check(r);
>+	ret = framebuffer_check(dev, r);
> 	if (ret)
> 		return ERR_PTR(ret);
>
>diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
>index cc44a9a4b004..2b33825f2f93 100644
>--- a/drivers/gpu/drm/drm_modeset_helper.c
>+++ b/drivers/gpu/drm/drm_modeset_helper.c
>@@ -78,7 +78,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
> 	int i;
>
> 	fb->dev = dev;
>-	fb->format = drm_format_info(mode_cmd->pixel_format);
>+	fb->format = drm_get_format_info(dev, mode_cmd);
> 	fb->width = mode_cmd->width;
> 	fb->height = mode_cmd->height;
> 	for (i = 0; i < 4; i++) {
>diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
>index fcc08da850c8..6942e84b6edd 100644
>--- a/include/drm/drm_fourcc.h
>+++ b/include/drm/drm_fourcc.h
>@@ -25,6 +25,9 @@
> #include <linux/types.h>
> #include <uapi/drm/drm_fourcc.h>
>
>+struct drm_device;
>+struct drm_mode_fb_cmd2;
>+
> /**
>  * struct drm_format_info - information about a DRM format
>  * @format: 4CC format identifier (DRM_FORMAT_*)
>@@ -55,6 +58,9 @@ struct drm_format_name_buf {
>
> const struct drm_format_info *__drm_format_info(u32 format);
> const struct drm_format_info *drm_format_info(u32 format);
>+const struct drm_format_info *
>+drm_get_format_info(struct drm_device *dev,
>+		    const struct drm_mode_fb_cmd2 *mode_cmd);
> uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> int drm_format_num_planes(uint32_t format);
> int drm_format_plane_cpp(uint32_t format, int plane);
>diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>index 17942c0f32a8..b5c2eb825965 100644
>--- a/include/drm/drm_mode_config.h
>+++ b/include/drm/drm_mode_config.h
>@@ -34,6 +34,7 @@ struct drm_file;
> struct drm_device;
> struct drm_atomic_state;
> struct drm_mode_fb_cmd2;
>+struct drm_format_info;
>
> /**
>  * struct drm_mode_config_funcs - basic driver provided mode setting functions
>@@ -70,6 +71,19 @@ struct drm_mode_config_funcs {
> 					     const struct drm_mode_fb_cmd2 *mode_cmd);
>
> 	/**
>+	 * @get_format_info:
>+	 *
>+	 * Allows a driver to return custom format information for special
>+	 * fb layouts (eg. ones with auxiliary compresssion control planes).

compression

>+	 *
>+	 * RETURNS:
>+	 *
>+	 * The format information specific to the given fb metadata, or
>+	 * NULL if none is found.
>+	 */
>+	const struct drm_format_info *(*get_format_info)(const struct drm_mode_fb_cmd2 *mode_cmd);
>+
>+	/**
> 	 * @output_poll_changed:
> 	 *
> 	 * Callback used by helpers to inform the driver of output configuration

Looks like msm and omap could use this too, and then if you allowed mode_cmd
to be NULL, you could potentially deprecate drm_format_info. Just a thought.

LGTM:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

>-- 
>2.10.2
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx