dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 10/12] drm: Fix modifiers_property kernel doc
       [not found] <20170824191100.10949-1-ville.syrjala@linux.intel.com>
@ 2017-08-24 19:10 ` ville.syrjala
  2017-08-25 13:44   ` Daniel Vetter
  2017-08-24 19:10 ` [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo ville.syrjala
  1 sibling, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2017-08-24 19:10 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky, Jason Ekstrand, dri-devel, Daniel Stone

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The member is called 'modifiers_property' instead of 'modifiers'. Adjust
the kernel docs to match.

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>
---
 include/drm/drm_mode_config.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..6040c4b73e6d 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -758,7 +758,7 @@ struct drm_mode_config {
 	bool allow_fb_modifiers;
 
 	/**
-	 * @modifiers: Plane property to list support modifier/format
+	 * @modifiers_property: Plane property to list support modifier/format
 	 * combination.
 	 */
 	struct drm_property *modifiers_property;
-- 
2.13.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo
       [not found] <20170824191100.10949-1-ville.syrjala@linux.intel.com>
  2017-08-24 19:10 ` [PATCH 10/12] drm: Fix modifiers_property kernel doc ville.syrjala
@ 2017-08-24 19:10 ` ville.syrjala
  2017-08-25 13:47   ` [Intel-gfx] " Daniel Vetter
  1 sibling, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2017-08-24 19:10 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        |  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 10/12] drm: Fix modifiers_property kernel doc
  2017-08-24 19:10 ` [PATCH 10/12] drm: Fix modifiers_property kernel doc ville.syrjala
@ 2017-08-25 13:44   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2017-08-25 13:44 UTC (permalink / raw)
  To: ville.syrjala
  Cc: dri-devel, intel-gfx, Ben Widawsky, Jason Ekstrand, Daniel Stone

On Thu, Aug 24, 2017 at 10:10:58PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The member is called 'modifiers_property' instead of 'modifiers'. Adjust
> the kernel docs to match.
> 
> 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>

Pls push to drm-misc-next (imo no need to for this to land in 4.14).
-Daniel

> ---
>  include/drm/drm_mode_config.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1b37368416c8..6040c4b73e6d 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -758,7 +758,7 @@ struct drm_mode_config {
>  	bool allow_fb_modifiers;
>  
>  	/**
> -	 * @modifiers: Plane property to list support modifier/format
> +	 * @modifiers_property: Plane property to list support modifier/format
>  	 * combination.
>  	 */
>  	struct drm_property *modifiers_property;
> -- 
> 2.13.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 4+ messages in thread

* Re: [Intel-gfx] [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo
  2017-08-24 19:10 ` [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo ville.syrjala
@ 2017-08-25 13:47   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2017-08-25 13:47 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, Ben Widawsky, dri-devel, Daniel Stone

On Thu, Aug 24, 2017 at 10:10:59PM +0300, ville.syrjala@linux.intel.com 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>

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@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@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] 4+ messages in thread

end of thread, other threads:[~2017-08-25 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20170824191100.10949-1-ville.syrjala@linux.intel.com>
2017-08-24 19:10 ` [PATCH 10/12] drm: Fix modifiers_property kernel doc ville.syrjala
2017-08-25 13:44   ` Daniel Vetter
2017-08-24 19:10 ` [PATCH 11/12] drm: Check that the plane supports the request format+modifier combo ville.syrjala
2017-08-25 13:47   ` [Intel-gfx] " Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).