* [PATCH v3 1/4] drm: Add drm_any_plane_has_format()
@ 2018-03-09 15:14 Ville Syrjala
2018-03-09 15:14 ` [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code Ville Syrjala
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Ville Syrjala @ 2018-03-09 15:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Add a function to check whether there is at least one plane that
supports a specific format and modifier combination. Drivers can
use this to reject unsupported formats/modifiers in .fb_create().
v2: Accept anyformat if the driver doesn't do planes (Eric)
s/planes_have_format/any_plane_has_format/ (Eric)
Check the modifier as well since we already have a function
that does both
v3: Don't do the check in the core since we may not know the
modifier yet, instead export the function and let drivers
call it themselves
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_plane.c | 23 +++++++++++++++++++++++
include/drm/drm_mode_config.h | 6 ++++++
include/drm/drm_plane.h | 2 ++
3 files changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index a5d1fc7e8a37..3b2d6f8d889d 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -578,6 +578,29 @@ int drm_plane_check_pixel_format(struct drm_plane *plane,
return 0;
}
+/**
+ * drm_any_plane_has_format - Check whether any plane supports this format and modifier combination
+ * @dev: DRM device
+ * @format: pixel format (DRM_FORMAT_*)
+ * @modifier: data layout modifier
+ *
+ * Returns:
+ * Whether at least one plane supports the specified format and modifier combination.
+ */
+bool drm_any_plane_has_format(struct drm_device *dev,
+ u32 format, u64 modifier)
+{
+ struct drm_plane *plane;
+
+ drm_for_each_plane(plane, dev) {
+ if (drm_plane_check_pixel_format(plane, format, modifier) == 0)
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(drm_any_plane_has_format);
+
/*
* __setplane_internal - setplane handler for internal callers
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 7569f22ffef6..9b894de9a75d 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -52,6 +52,12 @@ struct drm_mode_config_funcs {
* requested metadata, but most of that is left to the driver. See
* &struct drm_mode_fb_cmd2 for details.
*
+ * To validate the pixel format and modifier drivers can use
+ * drm_any_plane_has_format() to make sure at least one plane supports
+ * the requested values. Note that the driver must first determine the
+ * actual modifier used if the request doesn't have it specified,
+ * ie. when (@mode_cmd->flags & DRM_MODE_FB_MODIFIERS) == 0.
+ *
* If the parameters are deemed valid and the backing storage objects in
* the underlying memory manager all exist, then the driver allocates
* a new &drm_framebuffer structure, subclassed to contain
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index f7bf4a48b1c3..930e8fdd90f8 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -683,5 +683,7 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
#define drm_for_each_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
+bool drm_any_plane_has_format(struct drm_device *dev,
+ u32 format, u64 modifier);
#endif
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code
2018-03-09 15:14 [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Ville Syrjala
@ 2018-03-09 15:14 ` Ville Syrjala
2018-10-26 20:08 ` [Intel-gfx] " Dhinakaran Pandiyan
2018-03-09 15:14 ` [PATCH 3/4] drm: Fix some coding style issues Ville Syrjala
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2018-03-09 15:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Replace the messy framebuffer format/modifier validation code
with a single call to drm_any_plane_has_format(). The code was
extremely annoying to maintain as you had to have a lot of platform
checks for different formats. The new code requires zero maintenance.
v2: Nuke the modifier checks as well since the core does that too now
v3: Call drm_any_plane_has_format() from the driver code
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 90 ++++--------------------------------
1 file changed, 8 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2933ad38094f..7f06fa83d894 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13989,7 +13989,6 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
{
struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
struct drm_framebuffer *fb = &intel_fb->base;
- struct drm_format_name_buf format_name;
u32 pitch_limit;
unsigned int tiling, stride;
int ret = -EINVAL;
@@ -14020,33 +14019,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
}
}
- /* Passed in modifier sanity checking. */
- switch (mode_cmd->modifier[0]) {
- case I915_FORMAT_MOD_Y_TILED_CCS:
- case I915_FORMAT_MOD_Yf_TILED_CCS:
- switch (mode_cmd->pixel_format) {
- case DRM_FORMAT_XBGR8888:
- case DRM_FORMAT_ABGR8888:
- case DRM_FORMAT_XRGB8888:
- case DRM_FORMAT_ARGB8888:
- break;
- default:
- DRM_DEBUG_KMS("RC supported only with RGB8888 formats\n");
- goto err;
- }
- /* fall through */
- case I915_FORMAT_MOD_Y_TILED:
- case I915_FORMAT_MOD_Yf_TILED:
- if (INTEL_GEN(dev_priv) < 9) {
- DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n",
- mode_cmd->modifier[0]);
- goto err;
- }
- case DRM_FORMAT_MOD_LINEAR:
- case I915_FORMAT_MOD_X_TILED:
- break;
- default:
- DRM_DEBUG_KMS("Unsupported fb modifier 0x%llx!\n",
+ if (!drm_any_plane_has_format(&dev_priv->drm,
+ mode_cmd->pixel_format,
+ mode_cmd->modifier[0])) {
+ struct drm_format_name_buf format_name;
+
+ DRM_DEBUG_KMS("unsupported pixel format %s / modifier 0x%llx\n",
+ drm_get_format_name(mode_cmd->pixel_format,
+ &format_name),
mode_cmd->modifier[0]);
goto err;
}
@@ -14081,60 +14061,6 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
goto err;
}
- /* Reject formats not supported by any plane early. */
- switch (mode_cmd->pixel_format) {
- case DRM_FORMAT_C8:
- case DRM_FORMAT_RGB565:
- case DRM_FORMAT_XRGB8888:
- case DRM_FORMAT_ARGB8888:
- break;
- case DRM_FORMAT_XRGB1555:
- if (INTEL_GEN(dev_priv) > 3) {
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
- break;
- case DRM_FORMAT_ABGR8888:
- if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
- INTEL_GEN(dev_priv) < 9) {
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
- break;
- case DRM_FORMAT_XBGR8888:
- case DRM_FORMAT_XRGB2101010:
- case DRM_FORMAT_XBGR2101010:
- if (INTEL_GEN(dev_priv) < 4) {
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
- break;
- case DRM_FORMAT_ABGR2101010:
- if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
- break;
- case DRM_FORMAT_YUYV:
- case DRM_FORMAT_UYVY:
- case DRM_FORMAT_YVYU:
- case DRM_FORMAT_VYUY:
- if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
- break;
- default:
- DRM_DEBUG_KMS("unsupported pixel format: %s\n",
- drm_get_format_name(mode_cmd->pixel_format, &format_name));
- goto err;
- }
-
/* FIXME need to adjust LINOFF/TILEOFF accordingly. */
if (mode_cmd->offsets[0] != 0)
goto err;
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] drm: Fix some coding style issues
2018-03-09 15:14 [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Ville Syrjala
2018-03-09 15:14 ` [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code Ville Syrjala
@ 2018-03-09 15:14 ` Ville Syrjala
2018-03-09 15:14 ` [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier Ville Syrjala
2018-10-26 20:04 ` [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Dhinakaran Pandiyan
3 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjala @ 2018-03-09 15:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Put an empty line between the variable declarations and the code, and
use tabs for alignment.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_framebuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index c0530a1af5e3..7df025669067 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -162,9 +162,10 @@ static int framebuffer_check(struct drm_device *dev,
info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
if (!info) {
struct drm_format_name_buf format_name;
+
DRM_DEBUG_KMS("bad framebuffer format %s\n",
- drm_get_format_name(r->pixel_format,
- &format_name));
+ drm_get_format_name(r->pixel_format,
+ &format_name));
return -EINVAL;
}
--
2.16.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
2018-03-09 15:14 [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Ville Syrjala
2018-03-09 15:14 ` [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code Ville Syrjala
2018-03-09 15:14 ` [PATCH 3/4] drm: Fix some coding style issues Ville Syrjala
@ 2018-03-09 15:14 ` Ville Syrjala
2018-03-09 20:54 ` Eric Anholt
2018-10-26 20:04 ` [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Dhinakaran Pandiyan
3 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2018-03-09 15:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Only create framebuffers with supported format/modifier combinations by
checking that at least one plane supports the requested combination.
Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since
the planes have (mostly) uniform capabilities. But I was lazy and
didn't feel like exporting drm_plane_format_check() and hand rolling
anything better. Also I really just wanted to come up with another
user for drm_any_plane_has_format() ;)
Compile tested only.
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/vc4/vc4_kms.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index ba60153dddb5..b6f15102dda0 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -184,6 +184,17 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev,
mode_cmd = &mode_cmd_local;
}
+ if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format,
+ mode_cmd->modifier[0])) {
+ struct drm_format_name_buf format_name;
+
+ DRM_DEBUG_KMS("unsupported pixel format %s / modifier 0x%llx\n",
+ drm_get_format_name(mode_cmd->pixel_format,
+ &format_name),
+ mode_cmd->modifier[0]);
+ return ERR_PTR(-EINVAL);
+ }
+
return drm_gem_fb_create(dev, file_priv, mode_cmd);
}
--
2.16.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
2018-03-09 15:14 ` [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier Ville Syrjala
@ 2018-03-09 20:54 ` Eric Anholt
2018-03-12 15:04 ` Ville Syrjälä
2018-03-12 16:57 ` Daniel Vetter
0 siblings, 2 replies; 9+ messages in thread
From: Eric Anholt @ 2018-03-09 20:54 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 907 bytes --]
Ville Syrjala <ville.syrjala@linux.intel.com> writes:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Only create framebuffers with supported format/modifier combinations by
> checking that at least one plane supports the requested combination.
>
> Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since
> the planes have (mostly) uniform capabilities. But I was lazy and
> didn't feel like exporting drm_plane_format_check() and hand rolling
> anything better. Also I really just wanted to come up with another
> user for drm_any_plane_has_format() ;)
I'm not excited about vc4 having error-return behavior that other
drivers don't have. Actually, I don't really see why we should be doing
this check in fb create at all, given that you have to do so again at
atomic_check time with the specific plane.
Could you just delete the i915 fb format check code?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
2018-03-09 20:54 ` Eric Anholt
@ 2018-03-12 15:04 ` Ville Syrjälä
2018-03-12 16:57 ` Daniel Vetter
1 sibling, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2018-03-12 15:04 UTC (permalink / raw)
To: Eric Anholt; +Cc: intel-gfx, dri-devel
On Fri, Mar 09, 2018 at 12:54:43PM -0800, Eric Anholt wrote:
> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
>
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Only create framebuffers with supported format/modifier combinations by
> > checking that at least one plane supports the requested combination.
> >
> > Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since
> > the planes have (mostly) uniform capabilities. But I was lazy and
> > didn't feel like exporting drm_plane_format_check() and hand rolling
> > anything better. Also I really just wanted to come up with another
> > user for drm_any_plane_has_format() ;)
>
> I'm not excited about vc4 having error-return behavior that other
> drivers don't have. Actually, I don't really see why we should be doing
> this check in fb create at all, given that you have to do so again at
> atomic_check time with the specific plane.
>
> Could you just delete the i915 fb format check code?
I don't want unsupported formats to get anywhere near the driver
code. Makes life much less stressful when you know what can and
can't get in.
Also I see no good reason to allow userspace to create fbs it
can't actually use later on. Much better to reject early and tell
userspace to pick another format. I imagine pre-blobifier userspace
could even use this as a way to probe what's supported by the driver.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
2018-03-09 20:54 ` Eric Anholt
2018-03-12 15:04 ` Ville Syrjälä
@ 2018-03-12 16:57 ` Daniel Vetter
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2018-03-12 16:57 UTC (permalink / raw)
To: Eric Anholt; +Cc: intel-gfx, dri-devel
On Fri, Mar 09, 2018 at 12:54:43PM -0800, Eric Anholt wrote:
> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
>
> > From: Ville Syrj??l?? <ville.syrjala@linux.intel.com>
> >
> > Only create framebuffers with supported format/modifier combinations by
> > checking that at least one plane supports the requested combination.
> >
> > Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since
> > the planes have (mostly) uniform capabilities. But I was lazy and
> > didn't feel like exporting drm_plane_format_check() and hand rolling
> > anything better. Also I really just wanted to come up with another
> > user for drm_any_plane_has_format() ;)
>
> I'm not excited about vc4 having error-return behavior that other
> drivers don't have. Actually, I don't really see why we should be doing
> this check in fb create at all, given that you have to do so again at
> atomic_check time with the specific plane.
>
> Could you just delete the i915 fb format check code?
Yeah I thought we agreed that any driver not using the compat primary
plane helper will get this checking by default? That seems simplest, and
safest too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/4] drm: Add drm_any_plane_has_format()
2018-03-09 15:14 [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Ville Syrjala
` (2 preceding siblings ...)
2018-03-09 15:14 ` [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier Ville Syrjala
@ 2018-10-26 20:04 ` Dhinakaran Pandiyan
3 siblings, 0 replies; 9+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-26 20:04 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 2018-03-09 at 17:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a function to check whether there is at least one plane that
> supports a specific format and modifier combination. Drivers can
> use this to reject unsupported formats/modifiers in .fb_create().
>
> v2: Accept anyformat if the driver doesn't do planes (Eric)
> s/planes_have_format/any_plane_has_format/ (Eric)
> Check the modifier as well since we already have a function
> that does both
> v3: Don't do the check in the core since we may not know the
> modifier yet, instead export the function and let drivers
> call it themselves
>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I ended up writing a similar patch for i915. Having this in the core
seems better and patch still applies cleanly.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
> drivers/gpu/drm/drm_plane.c | 23 +++++++++++++++++++++++
> include/drm/drm_mode_config.h | 6 ++++++
> include/drm/drm_plane.h | 2 ++
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_plane.c
> b/drivers/gpu/drm/drm_plane.c
> index a5d1fc7e8a37..3b2d6f8d889d 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -578,6 +578,29 @@ int drm_plane_check_pixel_format(struct
> drm_plane *plane,
> return 0;
> }
>
> +/**
> + * drm_any_plane_has_format - Check whether any plane supports this
> format and modifier combination
> + * @dev: DRM device
> + * @format: pixel format (DRM_FORMAT_*)
> + * @modifier: data layout modifier
> + *
> + * Returns:
> + * Whether at least one plane supports the specified format and
> modifier combination.
> + */
> +bool drm_any_plane_has_format(struct drm_device *dev,
> + u32 format, u64 modifier)
> +{
> + struct drm_plane *plane;
> +
> + drm_for_each_plane(plane, dev) {
> + if (drm_plane_check_pixel_format(plane, format,
> modifier) == 0)
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL(drm_any_plane_has_format);
> +
> /*
> * __setplane_internal - setplane handler for internal callers
> *
> diff --git a/include/drm/drm_mode_config.h
> b/include/drm/drm_mode_config.h
> index 7569f22ffef6..9b894de9a75d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -52,6 +52,12 @@ struct drm_mode_config_funcs {
> * requested metadata, but most of that is left to the driver.
> See
> * &struct drm_mode_fb_cmd2 for details.
> *
> + * To validate the pixel format and modifier drivers can use
> + * drm_any_plane_has_format() to make sure at least one plane
> supports
> + * the requested values. Note that the driver must first
> determine the
> + * actual modifier used if the request doesn't have it
> specified,
> + * ie. when (@mode_cmd->flags & DRM_MODE_FB_MODIFIERS) == 0.
> + *
> * If the parameters are deemed valid and the backing storage
> objects in
> * the underlying memory manager all exist, then the driver
> allocates
> * a new &drm_framebuffer structure, subclassed to contain
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index f7bf4a48b1c3..930e8fdd90f8 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -683,5 +683,7 @@ static inline struct drm_plane
> *drm_plane_find(struct drm_device *dev,
> #define drm_for_each_plane(plane, dev) \
> list_for_each_entry(plane, &(dev)->mode_config.plane_list,
> head)
>
> +bool drm_any_plane_has_format(struct drm_device *dev,
> + u32 format, u64 modifier);
>
> #endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code
2018-03-09 15:14 ` [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code Ville Syrjala
@ 2018-10-26 20:08 ` Dhinakaran Pandiyan
0 siblings, 0 replies; 9+ messages in thread
From: Dhinakaran Pandiyan @ 2018-10-26 20:08 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 2018-03-09 at 17:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace the messy framebuffer format/modifier validation code
> with a single call to drm_any_plane_has_format(). The code was
> extremely annoying to maintain as you had to have a lot of platform
> checks for different formats. The new code requires zero maintenance.
>
> v2: Nuke the modifier checks as well since the core does that too now
> v3: Call drm_any_plane_has_format() from the driver code
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Patch looks good to me, but does not apply cleanly now.
> ---
> drivers/gpu/drm/i915/intel_display.c | 90 ++++--------------------
> ------------
> 1 file changed, 8 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..7f06fa83d894 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13989,7 +13989,6 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> {
> struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> struct drm_framebuffer *fb = &intel_fb->base;
> - struct drm_format_name_buf format_name;
> u32 pitch_limit;
> unsigned int tiling, stride;
> int ret = -EINVAL;
> @@ -14020,33 +14019,14 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> }
> }
>
> - /* Passed in modifier sanity checking. */
> - switch (mode_cmd->modifier[0]) {
> - case I915_FORMAT_MOD_Y_TILED_CCS:
> - case I915_FORMAT_MOD_Yf_TILED_CCS:
> - switch (mode_cmd->pixel_format) {
> - case DRM_FORMAT_XBGR8888:
> - case DRM_FORMAT_ABGR8888:
> - case DRM_FORMAT_XRGB8888:
> - case DRM_FORMAT_ARGB8888:
> - break;
> - default:
> - DRM_DEBUG_KMS("RC supported only with RGB8888
> formats\n");
> - goto err;
> - }
> - /* fall through */
> - case I915_FORMAT_MOD_Y_TILED:
> - case I915_FORMAT_MOD_Yf_TILED:
> - if (INTEL_GEN(dev_priv) < 9) {
> - DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n",
> - mode_cmd->modifier[0]);
> - goto err;
> - }
> - case DRM_FORMAT_MOD_LINEAR:
> - case I915_FORMAT_MOD_X_TILED:
> - break;
> - default:
> - DRM_DEBUG_KMS("Unsupported fb modifier 0x%llx!\n",
> + if (!drm_any_plane_has_format(&dev_priv->drm,
> + mode_cmd->pixel_format,
> + mode_cmd->modifier[0])) {
> + struct drm_format_name_buf format_name;
> +
> + DRM_DEBUG_KMS("unsupported pixel format %s / modifier
> 0x%llx\n",
> + drm_get_format_name(mode_cmd-
> >pixel_format,
> + &format_name),
> mode_cmd->modifier[0]);
> goto err;
> }
> @@ -14081,60 +14061,6 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> goto err;
> }
>
> - /* Reject formats not supported by any plane early. */
> - switch (mode_cmd->pixel_format) {
> - case DRM_FORMAT_C8:
> - case DRM_FORMAT_RGB565:
> - case DRM_FORMAT_XRGB8888:
> - case DRM_FORMAT_ARGB8888:
> - break;
> - case DRM_FORMAT_XRGB1555:
> - if (INTEL_GEN(dev_priv) > 3) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_ABGR8888:
> - if (!IS_VALLEYVIEW(dev_priv) &&
> !IS_CHERRYVIEW(dev_priv) &&
> - INTEL_GEN(dev_priv) < 9) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_XBGR8888:
> - case DRM_FORMAT_XRGB2101010:
> - case DRM_FORMAT_XBGR2101010:
> - if (INTEL_GEN(dev_priv) < 4) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_ABGR2101010:
> - if (!IS_VALLEYVIEW(dev_priv) &&
> !IS_CHERRYVIEW(dev_priv)) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> - break;
> - case DRM_FORMAT_YUYV:
> - case DRM_FORMAT_UYVY:
> - case DRM_FORMAT_YVYU:
> - case DRM_FORMAT_VYUY:
> - if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> - break;
> - default:
> - DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd-
> >pixel_format, &format_name));
> - goto err;
> - }
> -
> /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> if (mode_cmd->offsets[0] != 0)
> goto err;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-26 20:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 15:14 [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Ville Syrjala
2018-03-09 15:14 ` [PATCH v3 2/4] drm/i915: Eliminate the horrendous format check code Ville Syrjala
2018-10-26 20:08 ` [Intel-gfx] " Dhinakaran Pandiyan
2018-03-09 15:14 ` [PATCH 3/4] drm: Fix some coding style issues Ville Syrjala
2018-03-09 15:14 ` [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier Ville Syrjala
2018-03-09 20:54 ` Eric Anholt
2018-03-12 15:04 ` Ville Syrjälä
2018-03-12 16:57 ` Daniel Vetter
2018-10-26 20:04 ` [PATCH v3 1/4] drm: Add drm_any_plane_has_format() Dhinakaran Pandiyan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).