dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo
       [not found] <20171222192231.17981-1-ville.syrjala@linux.intel.com>
@ 2017-12-22 19:22 ` Ville Syrjala
  2018-01-10 13:04   ` [Intel-gfx] " Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Ville Syrjala @ 2017-12-22 19:22 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, dri-devel, Daniel Stone

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>
---
 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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo
  2017-12-22 19:22 ` [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo Ville Syrjala
@ 2018-01-10 13:04   ` Daniel Vetter
  2018-02-26 14:43     ` Ville Syrjälä
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2018-01-10 13:04 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx, Ben Widawsky, dri-devel, Daniel Stone

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo
  2018-01-10 13:04   ` [Intel-gfx] " Daniel Vetter
@ 2018-02-26 14:43     ` Ville Syrjälä
  0 siblings, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2018-02-26 14:43 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Ben Widawsky, dri-devel, Daniel Stone

On Wed, Jan 10, 2018 at 02:04:57PM +0100, Daniel Vetter wrote:
> 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>

Finally pushed this (and the followup i915 patch) to drm-misc-next.
Thanks for the reviews.

> 
> > ---
> >  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

-- 
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] 3+ messages in thread
end of thread, other threads:[~2018-02-26 14:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20171222192231.17981-1-ville.syrjala@linux.intel.com>
2017-12-22 19:22 ` [PATCH 7/8] drm: Check that the plane supports the request format+modifier combo Ville Syrjala
2018-01-10 13:04   ` [Intel-gfx] " Daniel Vetter
2018-02-26 14:43     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox