[Intel-gfx] [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo

[Daniel Vetter]

On Thu, Aug 24, 2017 at 10:10:59PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at lists.freedesktop.org
> Cc: Ben Widawsky <ben at bwidawsk.net>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Cc: Daniel Stone <daniels at collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

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 at ffwll.ch>


> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch