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

[Daniel Vetter <daniel@ffwll.ch>]

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
https://lists.freedesktop.org/mailman/listinfo/dri-devel