Re: [Intel-gfx] [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo

Daniel Vetter
2018-01-10T13:04:57Z

On Fri, Dec 22, 2017 at 09:22:30PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently we only check that the plane supports the pixel format of the
> fb we're about to feed to it. Extend it to check also the modifier, and
> more specifically that the combination of the format and modifier is
> supported.
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
> drivers/gpu/drm/drm_atomic.c | 10 ++++++----
> drivers/gpu/drm/drm_crtc.c | 10 ++++++----
> drivers/gpu/drm/drm_crtc_internal.h | 4 ++--
> drivers/gpu/drm/drm_plane.c | 33 ++++++++++++++++++++++++++-------
> 4 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b76d49218cf1..6e5c3ea1e43b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -882,12 +882,14 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
> }
>
> /* Check whether this plane supports the fb pixel format. */
> - ret = drm_plane_check_pixel_format(plane, state->fb->format->format);
> + ret = drm_plane_check_pixel_format(plane, state->fb->format->format,
> + state->fb->modifier);
> if (ret) {
> struct drm_format_name_buf format_name;
> - DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> - drm_get_format_name(state->fb->format->format,
> - &format_name));
> + DRM_DEBUG_ATOMIC("Invalid pixel format %s, modifier 0x%llx\n",
> + drm_get_format_name(state->fb->format->format,
> + &format_name),
> + state->fb->modifier);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e654116..816c30a0f030 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -625,12 +625,14 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> */
> if (!crtc->primary->format_default) {
> ret = drm_plane_check_pixel_format(crtc->primary,
> - fb->format->format);
> + fb->format->format,
> + fb->modifier);
> if (ret) {
> struct drm_format_name_buf format_name;
> - DRM_DEBUG_KMS("Invalid pixel format %s\n",
> - drm_get_format_name(fb->format->format,
> - &format_name));
> + DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
> + drm_get_format_name(fb->format->format,
> + &format_name),
> + fb->modifier);
> goto out;
> }
> }
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index af00f42ba269..860968a64ae7 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -196,8 +196,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> /* drm_plane.c */
> int drm_plane_register_all(struct drm_device *dev);
> void drm_plane_unregister_all(struct drm_device *dev);
> -int drm_plane_check_pixel_format(const struct drm_plane *plane,
> - u32 format);
> +int drm_plane_check_pixel_format(struct drm_plane *plane,
> + u32 format, u64 modifier);
>
> /* drm_bridge.c */
> void drm_bridge_detach(struct drm_bridge *bridge);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 2c90519576a3..8ac19379fe99 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -545,16 +545,33 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
> return 0;
> }
>
> -int drm_plane_check_pixel_format(const struct drm_plane *plane, u32 format)
> +int drm_plane_check_pixel_format(struct drm_plane *plane,
> + u32 format, u64 modifier)
> {
> unsigned int i;
>
> for (i = 0; i < plane->format_count; i++) {
> if (format == plane->format_types[i])
> - return 0;
> + break;
> + }
> + if (i == plane->format_count)
> + return -EINVAL;
> +
> + if (!plane->modifier_count)
> + return 0;
> +
> + for (i = 0; i < plane->modifier_count; i++) {
> + if (modifier == plane->modifiers[i])
> + break;
> }
> + if (i == plane->modifier_count)
> + return -EINVAL;
>
> - return -EINVAL;
> + if (plane->funcs->format_mod_supported &&
> + !plane->funcs->format_mod_supported(plane, format, modifier))
> + return -EINVAL;
> +
> + return 0;
> }
>
> /*
> @@ -598,12 +615,14 @@ static int __setplane_internal(struct drm_plane *plane,
> }
>
> /* Check whether this plane supports the fb pixel format. */
> - ret = drm_plane_check_pixel_format(plane, fb->format->format);
> + ret = drm_plane_check_pixel_format(plane, fb->format->format,
> + fb->modifier);
> if (ret) {
> struct drm_format_name_buf format_name;
> - DRM_DEBUG_KMS("Invalid pixel format %s\n",
> - drm_get_format_name(fb->format->format,
> - &format_name));
> + DRM_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
> + drm_get_format_name(fb->format->format,
> + &format_name),
> + fb->modifier);
> goto out;
> }
>
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org