On Thu, Aug 24, 2017 at 10:10:59PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> Cc: Ben Widawsky <ben@xxxxxxxxxxxx>
> Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> Cc: Daniel Stone <daniels@xxxxxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I think Daniel Stone is on the hook to augment kms_ccs to properly test
this all in igt. Would be nice to add the corresponding Testcase: lines
when that's done.
With the test coverage gap addressed:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 8 +++++---
> drivers/gpu/drm/drm_crtc.c | 8 +++++---
> drivers/gpu/drm/drm_crtc_internal.h | 4 ++--
> drivers/gpu/drm/drm_plane.c | 31 +++++++++++++++++++++++++------
> 4 files changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 2fd383d7253a..51cd05a7360b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -884,12 +884,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_DEBUG_ATOMIC("Invalid pixel format %s, modifier 0x%llx\n",
> drm_get_format_name(state->fb->format->format,
> - &format_name));
> + &format_name),
> + state->fb->modifier);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5af25ce5bf7c..dd54deb75c0d 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_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
> drm_get_format_name(fb->format->format,
> - &format_name));
> + &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 a43582076b20..81865841b656 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -194,8 +194,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 7a00351d5b5d..c63a81e32e23 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -555,16 +555,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;
> }
>
> /*
> @@ -609,12 +626,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_DEBUG_KMS("Invalid pixel format %s, modifier 0x%llx\n",
> drm_get_format_name(fb->format->format,
> - &format_name));
> + &format_name),
> + fb->modifier);
> goto out;
> }
>
> --
> 2.13.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
