[PATCH v5 01/11] drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h

[PATCH v5 01/11] drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h

[sagar.a.kamble]

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

The rotation property stuff should be standardized among all drivers.
Move the bits to drm_crtc.h from omap_drv.h.

Cc: David Airlie <airlied@linux.ie>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Archit Taneja <archit@ti.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h | 7 -------
 include/drm/drm_crtc.h             | 8 ++++++++
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 428b2981..aac8e10 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -119,13 +119,6 @@ struct omap_drm_private {
 	struct omap_drm_irq error_handler;
 };
 
-/* this should probably be in drm-core to standardize amongst drivers */
-#define DRM_ROTATE_0	0
-#define DRM_ROTATE_90	1
-#define DRM_ROTATE_180	2
-#define DRM_ROTATE_270	3
-#define DRM_REFLECT_X	4
-#define DRM_REFLECT_Y	5
 
 #ifdef CONFIG_DEBUG_FS
 int omap_debugfs_init(struct drm_minor *minor);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 71727b6..d5c46c1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -65,6 +65,14 @@ struct drm_object_properties {
 	uint64_t values[DRM_OBJECT_MAX_PROPERTY];
 };
 
+/* rotation property bits */
+#define DRM_ROTATE_0	0
+#define DRM_ROTATE_90	1
+#define DRM_ROTATE_180	2
+#define DRM_ROTATE_270	3
+#define DRM_REFLECT_X	4
+#define DRM_REFLECT_Y	5
+
 /*
  * Note on terminology:  here, for brevity and convenience, we refer to connector
  * control chips as 'CRTCs'.  They can control any type of connector, VGA, LVDS,
-- 
1.8.5

[PATCH v5 02/11] drm: Add support_bits parameter to drm_property_create_bitmask()

[sagar.a.kamble]

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

Make drm_property_create_bitmask() a bit more generic by allowing the
caller to specify which bits are in fact supported. This allows multiple
callers to use the same enum list, but still create different versions
of the same property with different list of supported bits.

v5: Fixed the caller of this function in omapdrm to comply with supported
bitmask definition.

Cc: David Airlie <airlied@linux.ie>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sagar Kamble <sagar.a.kamble@intel.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/drm_crtc.c           | 6 +++++-
 drivers/gpu/drm/omapdrm/omap_plane.c | 8 +++++++-
 include/drm/drm_crtc.h               | 3 ++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3b7d32d..628d3d3 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2906,7 +2906,8 @@ EXPORT_SYMBOL(drm_property_create_enum);
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values)
+					 int num_values,
+					 unsigned int supported_bits)
 {
 	struct drm_property *property;
 	int i, ret;
@@ -2918,6 +2919,9 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 		return NULL;
 
 	for (i = 0; i < num_values; i++) {
+		if (!(supported_bits & (1 << i)))
+			continue;
+
 		ret = drm_property_add_enum(property, i,
 				      props[i].type,
 				      props[i].name);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 046d5e6..b288f38 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -309,7 +309,13 @@ void omap_plane_install_properties(struct drm_plane *plane,
 					{ DRM_REFLECT_Y,  "reflect-y" },
 			};
 			prop = drm_property_create_bitmask(dev, 0, "rotation",
-					props, ARRAY_SIZE(props));
+					props, ARRAY_SIZE(props),
+					BIT(DRM_ROTATE_0) |
+					BIT(DRM_ROTATE_90) |
+					BIT(DRM_ROTATE_180) |
+					BIT(DRM_ROTATE_270) |
+					BIT(DRM_REFLECT_X) |
+					BIT(DRM_REFLECT_Y));
 			if (prop == NULL)
 				return;
 			priv->rotation_prop = prop;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d5c46c1..41b86d2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1070,7 +1070,8 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values);
+					 int num_values,
+					 unsigned int supported_bits);
 struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
 					 const char *name,
 					 uint64_t min, uint64_t max);
-- 
1.8.5

Re: [PATCH v5 02/11] drm: Add support_bits parameter to drm_property_create_bitmask()

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 01:01:09PM +0530, sagar.a.kamble@intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Make drm_property_create_bitmask() a bit more generic by allowing the
> caller to specify which bits are in fact supported. This allows multiple
> callers to use the same enum list, but still create different versions
> of the same property with different list of supported bits.
> 
> v5: Fixed the caller of this function in omapdrm to comply with supported
> bitmask definition.

How can it be v5 w/o any v2,v3,v4?

In any case I think this is still broken. I'll reply with a new patch...

> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sagar Kamble <sagar.a.kamble@intel.com>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> Reviewed-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/gpu/drm/drm_crtc.c           | 6 +++++-
>  drivers/gpu/drm/omapdrm/omap_plane.c | 8 +++++++-
>  include/drm/drm_crtc.h               | 3 ++-
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3b7d32d..628d3d3 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2906,7 +2906,8 @@ EXPORT_SYMBOL(drm_property_create_enum);
>  struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  					 int flags, const char *name,
>  					 const struct drm_prop_enum_list *props,
> -					 int num_values)
> +					 int num_values,
> +					 unsigned int supported_bits)
>  {
>  	struct drm_property *property;
>  	int i, ret;
> @@ -2918,6 +2919,9 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  		return NULL;
>  
>  	for (i = 0; i < num_values; i++) {
> +		if (!(supported_bits & (1 << i)))
> +			continue;
> +
>  		ret = drm_property_add_enum(property, i,
>  				      props[i].type,
>  				      props[i].name);
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 046d5e6..b288f38 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -309,7 +309,13 @@ void omap_plane_install_properties(struct drm_plane *plane,
>  					{ DRM_REFLECT_Y,  "reflect-y" },
>  			};
>  			prop = drm_property_create_bitmask(dev, 0, "rotation",
> -					props, ARRAY_SIZE(props));
> +					props, ARRAY_SIZE(props),
> +					BIT(DRM_ROTATE_0) |
> +					BIT(DRM_ROTATE_90) |
> +					BIT(DRM_ROTATE_180) |
> +					BIT(DRM_ROTATE_270) |
> +					BIT(DRM_REFLECT_X) |
> +					BIT(DRM_REFLECT_Y));
>  			if (prop == NULL)
>  				return;
>  			priv->rotation_prop = prop;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d5c46c1..41b86d2 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1070,7 +1070,8 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
>  struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  					 int flags, const char *name,
>  					 const struct drm_prop_enum_list *props,
> -					 int num_values);
> +					 int num_values,
> +					 unsigned int supported_bits);
>  struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
>  					 const char *name,
>  					 uint64_t min, uint64_t max);
> -- 
> 1.8.5

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 02/11] drm: Add support_bits parameter to drm_property_create_bitmask()

[ville.syrjala]

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

Make drm_property_create_bitmask() a bit more generic by allowing the
caller to specify which bits are in fact supported. This allows multiple
callers to use the same enum list, but still create different versions
of the same property with different list of supported bits.

v2: Populate values[] array as non-sparse
    Make supported_bits 64bit
    Fix up omapdrm call site (Rob)

Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sagar Kamble <sagar.a.kamble@intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
This should also make my original "drm: Add drm_mode_create_rotation_property()"
patch do the right thing. So patch 03/11 of the series needs to be replaced with the
original.

 drivers/gpu/drm/drm_crtc.c           | 18 ++++++++++++++----
 drivers/gpu/drm/omapdrm/omap_plane.c |  5 ++++-
 include/drm/drm_crtc.h               |  3 ++-
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3b7d32d..6e099069 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2906,10 +2906,12 @@ EXPORT_SYMBOL(drm_property_create_enum);
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values)
+					 int num_props,
+					 uint64_t supported_bits)
 {
 	struct drm_property *property;
-	int i, ret;
+	int i, ret, index = 0;
+	int num_values = hweight64(supported_bits);
 
 	flags |= DRM_MODE_PROP_BITMASK;
 
@@ -2917,8 +2919,16 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 	if (!property)
 		return NULL;
 
-	for (i = 0; i < num_values; i++) {
-		ret = drm_property_add_enum(property, i,
+	for (i = 0; i < num_props; i++) {
+		if (!(supported_bits & (1ULL << props[i].type)))
+			continue;
+
+		if (WARN_ON(index >= num_values)) {
+			drm_property_destroy(dev, property);
+			return NULL;
+		}
+
+		ret = drm_property_add_enum(property, index++,
 				      props[i].type,
 				      props[i].name);
 		if (ret) {
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 046d5e6..0d97650 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -309,7 +309,10 @@ void omap_plane_install_properties(struct drm_plane *plane,
 					{ DRM_REFLECT_Y,  "reflect-y" },
 			};
 			prop = drm_property_create_bitmask(dev, 0, "rotation",
-					props, ARRAY_SIZE(props));
+					props, ARRAY_SIZE(props),
+					BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+					BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
+					BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
 			if (prop == NULL)
 				return;
 			priv->rotation_prop = prop;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d5c46c1..4e6d2aa 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1070,7 +1070,8 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values);
+					 int num_props,
+					 uint64_t supported_bits);
 struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
 					 const char *name,
 					 uint64_t min, uint64_t max);
-- 
1.8.3.2

Re: [PATCH v2 02/11] drm: Add support_bits parameter to drm_property_create_bitmask()

[Sagar Arun Kamble]

Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>

On Mon, 2014-02-10 at 16:05 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Make drm_property_create_bitmask() a bit more generic by allowing the
> caller to specify which bits are in fact supported. This allows multiple
> callers to use the same enum list, but still create different versions
> of the same property with different list of supported bits.
> 
> v2: Populate values[] array as non-sparse
>     Make supported_bits 64bit
>     Fix up omapdrm call site (Rob)
> 
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sagar Kamble <sagar.a.kamble@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> This should also make my original "drm: Add drm_mode_create_rotation_property()"
> patch do the right thing. So patch 03/11 of the series needs to be replaced with the
> original.
> 
>  drivers/gpu/drm/drm_crtc.c           | 18 ++++++++++++++----
>  drivers/gpu/drm/omapdrm/omap_plane.c |  5 ++++-
>  include/drm/drm_crtc.h               |  3 ++-
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3b7d32d..6e099069 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2906,10 +2906,12 @@ EXPORT_SYMBOL(drm_property_create_enum);
>  struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  					 int flags, const char *name,
>  					 const struct drm_prop_enum_list *props,
> -					 int num_values)
> +					 int num_props,
> +					 uint64_t supported_bits)
>  {
>  	struct drm_property *property;
> -	int i, ret;
> +	int i, ret, index = 0;
> +	int num_values = hweight64(supported_bits);
>  
>  	flags |= DRM_MODE_PROP_BITMASK;
>  
> @@ -2917,8 +2919,16 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  	if (!property)
>  		return NULL;
>  
> -	for (i = 0; i < num_values; i++) {
> -		ret = drm_property_add_enum(property, i,
> +	for (i = 0; i < num_props; i++) {
> +		if (!(supported_bits & (1ULL << props[i].type)))
> +			continue;
> +
> +		if (WARN_ON(index >= num_values)) {
> +			drm_property_destroy(dev, property);
> +			return NULL;
> +		}
> +
> +		ret = drm_property_add_enum(property, index++,
>  				      props[i].type,
>  				      props[i].name);
>  		if (ret) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 046d5e6..0d97650 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -309,7 +309,10 @@ void omap_plane_install_properties(struct drm_plane *plane,
>  					{ DRM_REFLECT_Y,  "reflect-y" },
>  			};
>  			prop = drm_property_create_bitmask(dev, 0, "rotation",
> -					props, ARRAY_SIZE(props));
> +					props, ARRAY_SIZE(props),
> +					BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> +					BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
> +					BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
>  			if (prop == NULL)
>  				return;
>  			priv->rotation_prop = prop;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d5c46c1..4e6d2aa 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1070,7 +1070,8 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
>  struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
>  					 int flags, const char *name,
>  					 const struct drm_prop_enum_list *props,
> -					 int num_values);
> +					 int num_props,
> +					 uint64_t supported_bits);
>  struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
>  					 const char *name,
>  					 uint64_t min, uint64_t max);

[PATCH v5 03/11] drm: Add drm_mode_create_rotation_property()

[sagar.a.kamble]

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

Add a function to create a standards compliant rotation property.

v4: For creating rotation bitmask property send number of values
as only number of set rotations

Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 33 +++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 628d3d3..e7bbbad 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4118,3 +4118,36 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	idr_destroy(&dev->mode_config.crtc_idr);
 }
 EXPORT_SYMBOL(drm_mode_config_cleanup);
+
+/*
+* Function to get number of bits set in bitmask
+* using Brian Kernighan's Algorithm
+*/
+unsigned int bits_set(unsigned int n)
+{
+	unsigned int count = 0;
+
+	while (n) {
+		n &= (n-1);
+		count ++;
+	}
+	return count;
+}
+
+struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+						       unsigned int supported_rotations)
+{
+	static const struct drm_prop_enum_list props[] = {
+		{ DRM_ROTATE_0,   "rotate-0" },
+		{ DRM_ROTATE_90,  "rotate-90" },
+		{ DRM_ROTATE_180, "rotate-180" },
+		{ DRM_ROTATE_270, "rotate-270" },
+		{ DRM_REFLECT_X,  "reflect-x" },
+		{ DRM_REFLECT_Y,  "reflect-y" },
+	};
+
+	return drm_property_create_bitmask(dev, 0, "rotation",
+					   props, bits_set(supported_rotations),
+					   supported_rotations);
+}
+EXPORT_SYMBOL(drm_mode_create_rotation_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 41b86d2..4b3ac70 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1183,6 +1183,8 @@ extern int drm_format_plane_cpp(uint32_t format, int plane);
 extern int drm_format_horz_chroma_subsampling(uint32_t format);
 extern int drm_format_vert_chroma_subsampling(uint32_t format);
 extern const char *drm_get_format_name(uint32_t format);
+extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+							      unsigned int supported_rotations);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

[PATCH v5 04/11] drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()

[sagar.a.kamble]

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

Use the new drm_mode_create_rotation_property() in omapdrm.

v5: Fixed conflict due to change in the prior patch in call to
drm_property_create_bitmask()

Cc: David Airlie <airlied@linux.ie>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sagar Kamble <sagar.a.kamble@intel.com>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index b288f38..fee8f35 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -300,22 +300,13 @@ void omap_plane_install_properties(struct drm_plane *plane,
 	if (priv->has_dmm) {
 		prop = priv->rotation_prop;
 		if (!prop) {
-			const struct drm_prop_enum_list props[] = {
-					{ DRM_ROTATE_0,   "rotate-0" },
-					{ DRM_ROTATE_90,  "rotate-90" },
-					{ DRM_ROTATE_180, "rotate-180" },
-					{ DRM_ROTATE_270, "rotate-270" },
-					{ DRM_REFLECT_X,  "reflect-x" },
-					{ DRM_REFLECT_Y,  "reflect-y" },
-			};
-			prop = drm_property_create_bitmask(dev, 0, "rotation",
-					props, ARRAY_SIZE(props),
-					BIT(DRM_ROTATE_0) |
-					BIT(DRM_ROTATE_90) |
-					BIT(DRM_ROTATE_180) |
-					BIT(DRM_ROTATE_270) |
-					BIT(DRM_REFLECT_X) |
-					BIT(DRM_REFLECT_Y));
+			prop = drm_mode_create_rotation_property(dev,
+								 BIT(DRM_ROTATE_0) |
+								 BIT(DRM_ROTATE_90) |
+								 BIT(DRM_ROTATE_180) |
+								 BIT(DRM_ROTATE_270) |
+								 BIT(DRM_REFLECT_X) |
+								 BIT(DRM_REFLECT_Y));
 			if (prop == NULL)
 				return;
 			priv->rotation_prop = prop;
-- 
1.8.5

[PATCH v5 05/11] drm: Add drm_rect rotation functions

[sagar.a.kamble]

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

Add some helper functions to move drm_rects between different rotated
coordinate spaces. One function does the forward transform and
another does the inverse.

Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/drm_rect.c | 140 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_rect.h     |   6 ++
 2 files changed, 146 insertions(+)

diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
index 7047ca0..631f5af 100644
--- a/drivers/gpu/drm/drm_rect.c
+++ b/drivers/gpu/drm/drm_rect.c
@@ -293,3 +293,143 @@ void drm_rect_debug_print(const struct drm_rect *r, bool fixed_point)
 		DRM_DEBUG_KMS("%dx%d%+d%+d\n", w, h, r->x1, r->y1);
 }
 EXPORT_SYMBOL(drm_rect_debug_print);
+
+/**
+ * drm_rect_rotate - Rotate the rectangle
+ * @r: rectangle to be rotated
+ * @width: Width of the coordinate space
+ * @height: Height of the coordinate space
+ * @rotation: Transformation to be applied
+ *
+ * Apply @rotation to the coordinates of rectangle @r.
+ *
+ * @width and @height combined with @rotation define
+ * the location of the new origin.
+ *
+ * @width correcsponds to the horizontal and @height
+ * to the vertical axis of the untransformed coordinate
+ * space.
+ */
+void drm_rect_rotate(struct drm_rect *r,
+		     int width, int height,
+		     unsigned int rotation)
+{
+	struct drm_rect tmp;
+
+	if (rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y))) {
+		tmp = *r;
+
+		if (rotation & BIT(DRM_REFLECT_X)) {
+			r->x1 = width - tmp.x2;
+			r->x2 = width - tmp.x1;
+		}
+
+		if (rotation & BIT(DRM_REFLECT_Y)) {
+			r->y1 = height - tmp.y2;
+			r->y2 = height - tmp.y1;
+		}
+	}
+
+	switch (rotation & 0xf) {
+	case BIT(DRM_ROTATE_0):
+		break;
+	case BIT(DRM_ROTATE_90):
+		tmp = *r;
+		r->x1 = tmp.y1;
+		r->x2 = tmp.y2;
+		r->y1 = width - tmp.x2;
+		r->y2 = width - tmp.x1;
+		break;
+	case BIT(DRM_ROTATE_180):
+		tmp = *r;
+		r->x1 = width - tmp.x2;
+		r->x2 = width - tmp.x1;
+		r->y1 = height - tmp.y2;
+		r->y2 = height - tmp.y1;
+		break;
+	case BIT(DRM_ROTATE_270):
+		tmp = *r;
+		r->x1 = height - tmp.y2;
+		r->x2 = height - tmp.y1;
+		r->y1 = tmp.x1;
+		r->y2 = tmp.x2;
+		break;
+	default:
+		break;
+	}
+}
+EXPORT_SYMBOL(drm_rect_rotate);
+
+/**
+ * drm_rect_rotate_inv - Inverse rotate the rectangle
+ * @r: rectangle to be rotated
+ * @width: Width of the coordinate space
+ * @height: Height of the coordinate space
+ * @rotation: Transformation whose inverse is to be applied
+ *
+ * Apply the inverse of @rotation to the coordinates
+ * of rectangle @r.
+ *
+ * @width and @height combined with @rotation define
+ * the location of the new origin.
+ *
+ * @width correcsponds to the horizontal and @height
+ * to the vertical axis of the original untransformed
+ * coordinate space, so that you never have to flip
+ * them when doing a rotatation and its inverse.
+ * That is, if you do:
+ *
+ * drm_rotate(&r, width, height, rotation);
+ * drm_rotate_inv(&r, width, height, rotation);
+ *
+ * you will always get back the original rectangle.
+ */
+void drm_rect_rotate_inv(struct drm_rect *r,
+			 int width, int height,
+			 unsigned int rotation)
+{
+	struct drm_rect tmp;
+
+	switch (rotation & 0xf) {
+	case BIT(DRM_ROTATE_0):
+		break;
+	case BIT(DRM_ROTATE_90):
+		tmp = *r;
+		r->x1 = width - tmp.y2;
+		r->x2 = width - tmp.y1;
+		r->y1 = tmp.x1;
+		r->y2 = tmp.x2;
+		break;
+	case BIT(DRM_ROTATE_180):
+		tmp = *r;
+		r->x1 = width - tmp.x2;
+		r->x2 = width - tmp.x1;
+		r->y1 = height - tmp.y2;
+		r->y2 = height - tmp.y1;
+		break;
+	case BIT(DRM_ROTATE_270):
+		tmp = *r;
+		r->x1 = tmp.y1;
+		r->x2 = tmp.y2;
+		r->y1 = height - tmp.x2;
+		r->y2 = height - tmp.x1;
+		break;
+	default:
+		break;
+	}
+
+	if (rotation & (BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y))) {
+		tmp = *r;
+
+		if (rotation & BIT(DRM_REFLECT_X)) {
+			r->x1 = width - tmp.x2;
+			r->x2 = width - tmp.x1;
+		}
+
+		if (rotation & BIT(DRM_REFLECT_Y)) {
+			r->y1 = height - tmp.y2;
+			r->y2 = height - tmp.y1;
+		}
+	}
+}
+EXPORT_SYMBOL(drm_rect_rotate_inv);
diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
index d128629..26bb55e 100644
--- a/include/drm/drm_rect.h
+++ b/include/drm/drm_rect.h
@@ -163,5 +163,11 @@ int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
 				 struct drm_rect *dst,
 				 int min_vscale, int max_vscale);
 void drm_rect_debug_print(const struct drm_rect *r, bool fixed_point);
+void drm_rect_rotate(struct drm_rect *r,
+		     int width, int height,
+		     unsigned int rotation);
+void drm_rect_rotate_inv(struct drm_rect *r,
+			 int width, int height,
+			 unsigned int rotation);
 
 #endif
-- 
1.8.5

[PATCH v5 06/11] drm: Add drm_rotation_simplify()

[sagar.a.kamble]

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

drm_rotation_simplify() can be used to eliminate unsupported rotation
flags. It will check if any unsupported flags are present, and if so
it will modify the rotation to an alternate form by adding 180 degrees
to rotation angle, and flipping the reflect x and y bits. The hope is
that this identity transform will eliminate the unsupported flags.

Of course that might not result in any more supported rotation, so
the caller is still responsible for checking the result afterwards.

Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e7bbbad..30e0008 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4006,6 +4006,36 @@ int drm_format_vert_chroma_subsampling(uint32_t format)
 EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
 
 /**
+ * drm_rotation_simplify() - Try to simplify the rotation
+ * @rotation: Rotation to be simplified
+ * @supported_rotations: Supported rotations
+ *
+ * Attempt to simplify the rotation to a form that is supported.
+ * Eg. if the hardware supports everything except DRM_REFLECT_X
+ * one could call this function like this:
+ *
+ * drm_rotation_simplify(rotation, BIT(DRM_ROTATE_0) |
+ *                       BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_180) |
+ *                       BIT(DRM_ROTATE_270) | BIT(DRM_REFLECT_Y));
+ *
+ * to eliminate the DRM_ROTATE_X flag. Depending on what kind of
+ * transforms the hardware supports, this function may not
+ * be able to produce a supported transform, so the caller should
+ * check the result afterwards.
+ */
+unsigned int drm_rotation_simplify(unsigned int rotation,
+				   unsigned int supported_rotations)
+{
+	if (rotation & ~supported_rotations) {
+		rotation ^= BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y);
+		rotation = (rotation & ~0xf) | BIT((ffs(rotation & 0xf) + 1) % 4);
+	}
+
+	return rotation;
+}
+EXPORT_SYMBOL(drm_rotation_simplify);
+
+/**
  * drm_mode_config_init - initialize DRM mode_configuration structure
  * @dev: DRM device
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4b3ac70..18f2eed 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1185,6 +1185,8 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
 extern const char *drm_get_format_name(uint32_t format);
 extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 							      unsigned int supported_rotations);
+extern unsigned int drm_rotation_simplify(unsigned int rotation,
+					  unsigned int supported_rotations);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

[PATCH v5 07/11] drm/i915: Add 180 degree sprite rotation support

[sagar.a.kamble]

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

The sprite planes (in fact all display planes starting from gen4)
support 180 degree rotation. Add the relevant low level bits to the
sprite code to make use of that feature.

The upper layers are not yet plugged in.

v2: HSW handles the rotated buffer offset automagically

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  3 +++
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index abd18cd..57906c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3637,6 +3637,7 @@
 #define   DVS_YUV_ORDER_UYVY	(1<<16)
 #define   DVS_YUV_ORDER_YVYU	(2<<16)
 #define   DVS_YUV_ORDER_VYUY	(3<<16)
+#define   DVS_ROTATE_180	(1<<15)
 #define   DVS_DEST_KEY		(1<<2)
 #define   DVS_TRICKLE_FEED_DISABLE (1<<14)
 #define   DVS_TILED		(1<<10)
@@ -3707,6 +3708,7 @@
 #define   SPRITE_YUV_ORDER_UYVY		(1<<16)
 #define   SPRITE_YUV_ORDER_YVYU		(2<<16)
 #define   SPRITE_YUV_ORDER_VYUY		(3<<16)
+#define   SPRITE_ROTATE_180		(1<<15)
 #define   SPRITE_TRICKLE_FEED_DISABLE	(1<<14)
 #define   SPRITE_INT_GAMMA_ENABLE	(1<<13)
 #define   SPRITE_TILED			(1<<10)
@@ -3780,6 +3782,7 @@
 #define   SP_YUV_ORDER_UYVY		(1<<16)
 #define   SP_YUV_ORDER_YVYU		(2<<16)
 #define   SP_YUV_ORDER_VYUY		(3<<16)
+#define   SP_ROTATE_180			(1<<15)
 #define   SP_TILED			(1<<10)
 #define _SPALINOFF		(VLV_DISPLAY_BASE + 0x72184)
 #define _SPASTRIDE		(VLV_DISPLAY_BASE + 0x72188)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 44067bc..85864fc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -397,6 +397,7 @@ struct intel_plane {
 	unsigned int crtc_w, crtc_h;
 	uint32_t src_x, src_y;
 	uint32_t src_w, src_h;
+	unsigned int rotation;
 
 	/* Since we need to change the watermarks before/after
 	 * enabling/disabling the planes, we need to store the parameters here
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..477d4d7 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -60,6 +60,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	sprctl &= ~SP_PIXFORMAT_MASK;
 	sprctl &= ~SP_YUV_BYTE_ORDER_MASK;
 	sprctl &= ~SP_TILED;
+	sprctl &= ~SP_ROTATE_180;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_YUYV:
@@ -131,6 +132,14 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 							fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
+	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+		sprctl |= SP_ROTATE_180;
+
+		x += src_w;
+		y += src_h;
+		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
+	}
+
 	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
 
@@ -238,6 +247,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	sprctl &= ~SPRITE_RGB_ORDER_RGBX;
 	sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
 	sprctl &= ~SPRITE_TILED;
+	sprctl &= ~SPRITE_ROTATE_180;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_XBGR8888:
@@ -299,6 +309,17 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
+	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+		sprctl |= SPRITE_ROTATE_180;
+
+		/* HSW does this automagically in hardware */
+		if (!IS_HASWELL(dev)) {
+			x += src_w;
+			y += src_h;
+			linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
+		}
+	}
+
 	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -422,6 +443,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	dvscntr &= ~DVS_RGB_ORDER_XBGR;
 	dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
 	dvscntr &= ~DVS_TILED;
+	dvscntr &= ~DVS_ROTATE_180;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_XBGR8888:
@@ -478,6 +500,14 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= dvssurf_offset;
 
+	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
+		dvscntr |= DVS_ROTATE_180;
+
+		x += src_w;
+		y += src_h;
+		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
+	}
+
 	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -739,6 +769,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	max_scale = intel_plane->max_downscale << 16;
 	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
+	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
+			intel_plane->rotation);
+
 	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
 	BUG_ON(hscale < 0);
 
@@ -777,6 +810,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 				     drm_rect_width(&dst) * hscale - drm_rect_width(&src),
 				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
 
+		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
+				    intel_plane->rotation);
+
 		/* sanity check to make sure the src viewport wasn't enlarged */
 		WARN_ON(src.x1 < (int) src_x ||
 			src.y1 < (int) src_y ||
@@ -1141,6 +1177,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 
 	intel_plane->pipe = pipe;
 	intel_plane->plane = plane;
+	intel_plane->rotation = BIT(DRM_ROTATE_0);
 	possible_crtcs = (1 << pipe);
 	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
 			     &intel_plane_funcs,
-- 
1.8.5

Re: [PATCH v5 07/11] drm/i915: Add 180 degree sprite rotation support

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 01:01:14PM +0530, sagar.a.kamble@intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The sprite planes (in fact all display planes starting from gen4)
> support 180 degree rotation. Add the relevant low level bits to the
> sprite code to make use of that feature.
> 
> The upper layers are not yet plugged in.
> 
> v2: HSW handles the rotated buffer offset automagically
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  3 +++
>  drivers/gpu/drm/i915/intel_drv.h    |  1 +
>  drivers/gpu/drm/i915/intel_sprite.c | 37 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index abd18cd..57906c5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3637,6 +3637,7 @@
>  #define   DVS_YUV_ORDER_UYVY	(1<<16)
>  #define   DVS_YUV_ORDER_YVYU	(2<<16)
>  #define   DVS_YUV_ORDER_VYUY	(3<<16)
> +#define   DVS_ROTATE_180	(1<<15)
>  #define   DVS_DEST_KEY		(1<<2)
>  #define   DVS_TRICKLE_FEED_DISABLE (1<<14)
>  #define   DVS_TILED		(1<<10)
> @@ -3707,6 +3708,7 @@
>  #define   SPRITE_YUV_ORDER_UYVY		(1<<16)
>  #define   SPRITE_YUV_ORDER_YVYU		(2<<16)
>  #define   SPRITE_YUV_ORDER_VYUY		(3<<16)
> +#define   SPRITE_ROTATE_180		(1<<15)
>  #define   SPRITE_TRICKLE_FEED_DISABLE	(1<<14)
>  #define   SPRITE_INT_GAMMA_ENABLE	(1<<13)
>  #define   SPRITE_TILED			(1<<10)
> @@ -3780,6 +3782,7 @@
>  #define   SP_YUV_ORDER_UYVY		(1<<16)
>  #define   SP_YUV_ORDER_YVYU		(2<<16)
>  #define   SP_YUV_ORDER_VYUY		(3<<16)
> +#define   SP_ROTATE_180			(1<<15)
>  #define   SP_TILED			(1<<10)
>  #define _SPALINOFF		(VLV_DISPLAY_BASE + 0x72184)
>  #define _SPASTRIDE		(VLV_DISPLAY_BASE + 0x72188)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 44067bc..85864fc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -397,6 +397,7 @@ struct intel_plane {
>  	unsigned int crtc_w, crtc_h;
>  	uint32_t src_x, src_y;
>  	uint32_t src_w, src_h;
> +	unsigned int rotation;
>  
>  	/* Since we need to change the watermarks before/after
>  	 * enabling/disabling the planes, we need to store the parameters here
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 336ae6c..477d4d7 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -60,6 +60,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  	sprctl &= ~SP_PIXFORMAT_MASK;
>  	sprctl &= ~SP_YUV_BYTE_ORDER_MASK;
>  	sprctl &= ~SP_TILED;
> +	sprctl &= ~SP_ROTATE_180;
>  
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_YUYV:
> @@ -131,6 +132,14 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  							fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
>  
> +	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> +		sprctl |= SP_ROTATE_180;
> +
> +		x += src_w;
> +		y += src_h;
> +		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
> +	}
> +
>  	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>  	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
>  
> @@ -238,6 +247,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	sprctl &= ~SPRITE_RGB_ORDER_RGBX;
>  	sprctl &= ~SPRITE_YUV_BYTE_ORDER_MASK;
>  	sprctl &= ~SPRITE_TILED;
> +	sprctl &= ~SPRITE_ROTATE_180;
>  
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_XBGR8888:
> @@ -299,6 +309,17 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
>  
> +	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> +		sprctl |= SPRITE_ROTATE_180;
> +
> +		/* HSW does this automagically in hardware */
> +		if (!IS_HASWELL(dev)) {

Still missing the BDW check here.

> +			x += src_w;
> +			y += src_h;
> +			linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
> +		}
> +	}
> +
>  	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
>  
> @@ -422,6 +443,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	dvscntr &= ~DVS_RGB_ORDER_XBGR;
>  	dvscntr &= ~DVS_YUV_BYTE_ORDER_MASK;
>  	dvscntr &= ~DVS_TILED;
> +	dvscntr &= ~DVS_ROTATE_180;
>  
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_XBGR8888:
> @@ -478,6 +500,14 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= dvssurf_offset;
>  
> +	if (intel_plane->rotation == BIT(DRM_ROTATE_180)) {
> +		dvscntr |= DVS_ROTATE_180;
> +
> +		x += src_w;
> +		y += src_h;
> +		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
> +	}
> +
>  	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
>  
> @@ -739,6 +769,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	max_scale = intel_plane->max_downscale << 16;
>  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>  
> +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> +			intel_plane->rotation);
> +
>  	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
>  	BUG_ON(hscale < 0);
>  
> @@ -777,6 +810,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  				     drm_rect_width(&dst) * hscale - drm_rect_width(&src),
>  				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
>  
> +		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> +				    intel_plane->rotation);
> +
>  		/* sanity check to make sure the src viewport wasn't enlarged */
>  		WARN_ON(src.x1 < (int) src_x ||
>  			src.y1 < (int) src_y ||
> @@ -1141,6 +1177,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  
>  	intel_plane->pipe = pipe;
>  	intel_plane->plane = plane;
> +	intel_plane->rotation = BIT(DRM_ROTATE_0);
>  	possible_crtcs = (1 << pipe);
>  	ret = drm_plane_init(dev, &intel_plane->base, possible_crtcs,
>  			     &intel_plane_funcs,
> -- 
> 1.8.5

-- 
Ville Syrjälä
Intel OTC

[PATCH v5 08/11] drm/i915: Make intel_plane_restore() return an error

[sagar.a.kamble]

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

Propagate the error from intel_update_plane() up through
intel_plane_restore() to the caller. This will be used for
rollback purposes when setting properties fails.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85864fc..7a79b8e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -897,7 +897,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
 int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
 void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
 			       enum plane plane);
-void intel_plane_restore(struct drm_plane *plane);
+int intel_plane_restore(struct drm_plane *plane);
 void intel_plane_disable(struct drm_plane *plane);
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 477d4d7..511934c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1047,18 +1047,18 @@ out_unlock:
 	return ret;
 }
 
-void intel_plane_restore(struct drm_plane *plane)
+int intel_plane_restore(struct drm_plane *plane)
 {
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 
 	if (!plane->crtc || !plane->fb)
-		return;
+		return 0;
 
-	intel_update_plane(plane, plane->crtc, plane->fb,
-			   intel_plane->crtc_x, intel_plane->crtc_y,
-			   intel_plane->crtc_w, intel_plane->crtc_h,
-			   intel_plane->src_x, intel_plane->src_y,
-			   intel_plane->src_w, intel_plane->src_h);
+	return intel_update_plane(plane, plane->crtc, plane->fb,
+				  intel_plane->crtc_x, intel_plane->crtc_y,
+				  intel_plane->crtc_w, intel_plane->crtc_h,
+				  intel_plane->src_x, intel_plane->src_y,
+				  intel_plane->src_w, intel_plane->src_h);
 }
 
 void intel_plane_disable(struct drm_plane *plane)
-- 
1.8.5

[PATCH v5 09/11] drm/i915: Add rotation property for sprites

[sagar.a.kamble]

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

Sprite planes support 180 degree rotation. The lower layers are now in
place, so hook in the standard rotation property to expose the feature
to the users.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 42 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fa37dfd..ea2efc3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1548,6 +1548,7 @@ typedef struct drm_i915_private {
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
+	struct drm_property *rotation_property;
 
 	uint32_t hw_context_size;
 	struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 511934c..62b9f84 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1047,6 +1047,30 @@ out_unlock:
 	return ret;
 }
 
+static int intel_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_i915_private *dev_priv = plane->dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	uint64_t old_val;
+	int ret = -ENOENT;
+
+	if (prop == dev_priv->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		old_val = intel_plane->rotation;
+		intel_plane->rotation = val;
+		ret = intel_plane_restore(plane);
+		if (ret)
+			intel_plane->rotation = old_val;
+	}
+
+	return ret;
+}
+
 int intel_plane_restore(struct drm_plane *plane)
 {
 	struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1073,6 +1097,7 @@ static const struct drm_plane_funcs intel_plane_funcs = {
 	.update_plane = intel_update_plane,
 	.disable_plane = intel_disable_plane,
 	.destroy = intel_destroy_plane,
+	.set_property = intel_plane_set_property,
 };
 
 static uint32_t ilk_plane_formats[] = {
@@ -1109,6 +1134,7 @@ static uint32_t vlv_plane_formats[] = {
 int
 intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane;
 	unsigned long possible_crtcs;
 	const uint32_t *plane_formats;
@@ -1183,8 +1209,22 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 			     &intel_plane_funcs,
 			     plane_formats, num_plane_formats,
 			     false);
-	if (ret)
+	if (ret) {
 		kfree(intel_plane);
+		goto out;
+	}
+
+	if (!dev_priv->rotation_property)
+		dev_priv->rotation_property =
+			drm_mode_create_rotation_property(dev,
+							  BIT(DRM_ROTATE_0) |
+							  BIT(DRM_ROTATE_180));
+
+	if (dev_priv->rotation_property)
+		drm_object_attach_property(&intel_plane->base.base,
+					   dev_priv->rotation_property,
+					   intel_plane->rotation);
 
+ out:
 	return ret;
 }
-- 
1.8.5

[PATCH v5 10/11] drm/i915: Add 180 degree primary plane rotation support

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.

v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.

v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.

v4: Updated rotation checks for pending flips, fbc disable. Creating
rotation property only for Gen4 onwards. Property resetting as part
of lastclose.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: vijay.a.purushothaman@intel.com
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      | 12 +++++
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 86 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 drivers/gpu/drm/i915/intel_pm.c      |  9 ++++
 5 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 258b1be..fcd9e34 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1836,6 +1836,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 void i915_driver_lastclose(struct drm_device * dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
 
 	/* On gen6+ we refuse to init without kms enabled, but then the drm core
 	 * goes right around and calls lastclose. Check for this and don't clean
@@ -1843,6 +1845,16 @@ void i915_driver_lastclose(struct drm_device * dev)
 	if (!dev_priv)
 		return;
 
+	if (dev_priv->rotation_property) {
+		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+			drm_object_property_set_value(&crtc->base,
+					dev_priv->rotation_property, 0);
+
+		list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+			drm_object_property_set_value(&plane->base,
+					dev_priv->rotation_property, 0);
+	}
+
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		intel_fbdev_restore_mode(dev);
 		vga_switcheroo_process_delayed_switch();
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 57906c5..d3000c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3553,6 +3553,7 @@
 #define   DISPPLANE_NO_LINE_DOUBLE		0
 #define   DISPPLANE_STEREO_POLARITY_FIRST	0
 #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
+#define   DISPPLANE_ROTATE_180			(1<<15)
 #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
 #define   DISPPLANE_TILED			(1<<10)
 #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..4a4f650 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
 	switch (plane) {
 	case 0:
@@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		return -EINVAL;
 	}
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
 	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+	dspcntr &= ~DISPPLANE_ROTATE_180;
+
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
@@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	if (IS_G4X(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
-	I915_WRITE(reg, dspcntr);
-
 	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
 
 	if (INTEL_INFO(dev)->gen >= 4) {
@@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
+		dspcntr |= DISPPLANE_ROTATE_180;
+
+		x += (intel_crtc->config.pipe_src_w - 1);
+		y += (intel_crtc->config.pipe_src_h - 1);
+		linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
+                                   intel_crtc->config.pipe_src_w * pixel_size;
+	}
+
+	I915_WRITE(reg, dspcntr);
+
 	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
 		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
 		      fb->pitches[0]);
@@ -2137,6 +2150,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
 	switch (plane) {
 	case 0:
@@ -2148,6 +2162,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2155,6 +2170,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
 	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+	dspcntr &= ~DISPPLANE_ROTATE_180;
+
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
@@ -2192,8 +2209,6 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	else
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
-	I915_WRITE(reg, dspcntr);
-
 	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
 	intel_crtc->dspaddr_offset =
 		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
@@ -2201,6 +2216,19 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
+	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
+		dspcntr |= DISPPLANE_ROTATE_180;
+
+		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
+			x += (intel_crtc->config.pipe_src_w - 1);
+			y += (intel_crtc->config.pipe_src_h - 1);
+			linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
+					intel_crtc->config.pipe_src_w * pixel_size;
+		}
+	}
+
+	I915_WRITE(reg, dspcntr);
+
 	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
 		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
 		      fb->pitches[0]);
@@ -8748,6 +8776,42 @@ free_work:
 	return ret;
 }
 
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	uint64_t old_val;
+	int ret = -ENOENT;
+
+	if (prop == dev_priv->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		old_val = intel_crtc->rotation;
+		intel_crtc->rotation = val;
+
+		/* Updating rotation on plane should be done after pending flips.
+		FBC does not work on some platforms for rotated planes */
+		intel_crtc_wait_for_pending_flips(crtc);
+		if (dev_priv->fbc.plane == intel_crtc->plane &&
+			INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			(intel_crtc->rotation != BIT(DRM_ROTATE_0)))
+			intel_disable_fbc(dev);
+
+		if (intel_crtc->active) {
+			ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
+			if (ret)
+				intel_crtc->rotation = old_val;
+		}
+	}
+
+	return ret;
+}
+
 static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
 	.load_lut = intel_crtc_load_lut,
@@ -10160,6 +10224,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property
 };
 
 static void intel_cpu_pll_init(struct drm_device *dev)
@@ -10288,6 +10353,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	 */
 	intel_crtc->pipe = pipe;
 	intel_crtc->plane = pipe;
+	intel_crtc->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
 		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
 		intel_crtc->plane = !pipe;
@@ -10298,6 +10364,18 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
+	if (INTEL_INFO(dev)->gen >= 4) {
+		if (!dev_priv->rotation_property)
+			dev_priv->rotation_property =
+				drm_mode_create_rotation_property(dev,
+								BIT(DRM_ROTATE_0) |
+								BIT(DRM_ROTATE_180));
+		if (dev_priv->rotation_property)
+			drm_object_attach_property(&intel_crtc->base.base,
+						dev_priv->rotation_property,
+						intel_crtc->rotation);
+	}
+
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7a79b8e..02dfdb6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -331,6 +331,8 @@ struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
 	enum plane plane;
+	unsigned int rotation;
+
 	u8 lut_r[256], lut_g[256], lut_b[256];
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3c79b63..d5b87c8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -508,6 +508,15 @@ void intel_update_fbc(struct drm_device *dev)
 	obj = intel_fb->obj;
 	adjusted_mode = &intel_crtc->config.adjusted_mode;
 
+	if (dev_priv->fbc.plane == intel_crtc->plane &&
+		INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+		(intel_crtc->rotation != BIT(DRM_ROTATE_0))) {
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+			DRM_DEBUG_KMS("mode incompatible with compression, "
+				      "disabling\n");
+		goto out_disable;
+	}
+
 	if (i915.enable_fbc < 0 &&
 	    INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) {
 		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
-- 
1.8.5

Re: [Intel-gfx] [PATCH v5 10/11] drm/i915: Add 180 degree primary plane rotation support

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 01:01:17PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Primary planes support 180 degree rotation. Expose the feature
> through rotation drm property.
> 
> v2: Calculating linear/tiled offsets based on pipe source width and
> height. Added 180 degree rotation support in ironlake_update_plane.
> 
> v3: Checking if CRTC is active before issueing update_plane. Added
> wait for vblank to make sure we dont overtake page flips. Disabling
> FBC since it does not work with rotated planes.
> 
> v4: Updated rotation checks for pending flips, fbc disable. Creating
> rotation property only for Gen4 onwards. Property resetting as part
> of lastclose.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: vijay.a.purushothaman@intel.com
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      | 12 +++++
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 86 ++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  drivers/gpu/drm/i915/intel_pm.c      |  9 ++++
>  5 files changed, 106 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 258b1be..fcd9e34 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1836,6 +1836,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>  void i915_driver_lastclose(struct drm_device * dev)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
>  
>  	/* On gen6+ we refuse to init without kms enabled, but then the drm core
>  	 * goes right around and calls lastclose. Check for this and don't clean
> @@ -1843,6 +1845,16 @@ void i915_driver_lastclose(struct drm_device * dev)
>  	if (!dev_priv)
>  		return;
>  
> +	if (dev_priv->rotation_property) {
> +		list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> +			drm_object_property_set_value(&crtc->base,
> +					dev_priv->rotation_property, 0);
> +
> +		list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +			drm_object_property_set_value(&plane->base,
> +					dev_priv->rotation_property, 0);
> +	}

That would need to be something like:

	struct intel_crtc *crtc
	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
		crtc->rotation = BIT(DRM_ROTATE_0);
		drm_object_property_set_value(&crtc->base.base,
					      dev_priv->rotation_property,
					      crtc->rotation);
	}

and similarly for planes.

> +
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>  		intel_fbdev_restore_mode(dev);
>  		vga_switcheroo_process_delayed_switch();
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57906c5..d3000c4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3553,6 +3553,7 @@
>  #define   DISPPLANE_NO_LINE_DOUBLE		0
>  #define   DISPPLANE_STEREO_POLARITY_FIRST	0
>  #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
> +#define   DISPPLANE_ROTATE_180			(1<<15)
>  #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
>  #define   DISPPLANE_TILED			(1<<10)
>  #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d4a0d9..4a4f650 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
>  	switch (plane) {
>  	case 0:
> @@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		return -EINVAL;
>  	}
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	dspcntr = I915_READ(reg);
>  	/* Mask out pixel format bits in case we change it */
>  	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> +	dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> -	I915_WRITE(reg, dspcntr);
> -
>  	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> @@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> +	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
> +		x += (intel_crtc->config.pipe_src_w - 1);
> +		y += (intel_crtc->config.pipe_src_h - 1);
> +		linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
> +                                   intel_crtc->config.pipe_src_w * pixel_size;

Off by one:
(pipe_src_w - 1) * pixel_size

> +	}
> +
> +	I915_WRITE(reg, dspcntr);
> +
>  	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
>  		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
>  		      fb->pitches[0]);
> @@ -2137,6 +2150,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
>  	switch (plane) {
>  	case 0:
> @@ -2148,6 +2162,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2155,6 +2170,8 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  	dspcntr = I915_READ(reg);
>  	/* Mask out pixel format bits in case we change it */
>  	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> +	dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2192,8 +2209,6 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  	else
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> -	I915_WRITE(reg, dspcntr);
> -
>  	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>  	intel_crtc->dspaddr_offset =
>  		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> @@ -2201,6 +2216,19 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
>  					       fb->pitches[0]);
>  	linear_offset -= intel_crtc->dspaddr_offset;
>  
> +	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
> +		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> +			x += (intel_crtc->config.pipe_src_w - 1);
> +			y += (intel_crtc->config.pipe_src_h - 1);
> +			linear_offset += (intel_crtc->config.pipe_src_h - 1) * fb->pitches[0] +
> +					intel_crtc->config.pipe_src_w * pixel_size;

Again:
(pipe_src_w - 1) * pixel_size

> +		}
> +	}
> +
> +	I915_WRITE(reg, dspcntr);
> +
>  	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
>  		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
>  		      fb->pitches[0]);
> @@ -8748,6 +8776,42 @@ free_work:
>  	return ret;
>  }
>  
> +static int intel_crtc_set_property(struct drm_crtc *crtc,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	uint64_t old_val;
> +	int ret = -ENOENT;
> +
> +	if (prop == dev_priv->rotation_property) {
> +		/* exactly one rotation angle please */
> +		if (hweight32(val & 0xf) != 1)
> +			return -EINVAL;
> +
> +		old_val = intel_crtc->rotation;
> +		intel_crtc->rotation = val;
> +
> +		/* Updating rotation on plane should be done after pending flips.
> +		FBC does not work on some platforms for rotated planes */
> +		intel_crtc_wait_for_pending_flips(crtc);
> +		if (dev_priv->fbc.plane == intel_crtc->plane &&
> +			INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +			(intel_crtc->rotation != BIT(DRM_ROTATE_0)))

There's some weird indentation in the if-statement. When splitting such
things over multiple lines, the following lines should normally line up
after the opening parenthesis. Decent editors will do this for you
automagically.

Also the extra parens around the
'intel_crtc->rotation != BIT(DRM_ROTATE_0)' comparison aren't needed.

> +			intel_disable_fbc(dev);

Inactive crtc can't have any pending flips, nor can FBC be enabled in
that case. So all of this can be moved into the 'if (intel_crtc->active)'
block.

> +
> +		if (intel_crtc->active) {
> +			ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
> +			if (ret)
> +				intel_crtc->rotation = old_val;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
>  	.load_lut = intel_crtc_load_lut,
> @@ -10160,6 +10224,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = intel_crtc_set_config,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
> +	.set_property = intel_crtc_set_property
>  };
>  
>  static void intel_cpu_pll_init(struct drm_device *dev)
> @@ -10288,6 +10353,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	 */
>  	intel_crtc->pipe = pipe;
>  	intel_crtc->plane = pipe;
> +	intel_crtc->rotation = BIT(DRM_ROTATE_0);
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
>  		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
>  		intel_crtc->plane = !pipe;
> @@ -10298,6 +10364,18 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
>  
> +	if (INTEL_INFO(dev)->gen >= 4) {
> +		if (!dev_priv->rotation_property)
> +			dev_priv->rotation_property =
> +				drm_mode_create_rotation_property(dev,
> +								BIT(DRM_ROTATE_0) |
> +								BIT(DRM_ROTATE_180));
> +		if (dev_priv->rotation_property)
> +			drm_object_attach_property(&intel_crtc->base.base,
> +						dev_priv->rotation_property,
> +						intel_crtc->rotation);
> +	}
> +
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7a79b8e..02dfdb6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -331,6 +331,8 @@ struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
>  	enum plane plane;
> +	unsigned int rotation;
> +
>  	u8 lut_r[256], lut_g[256], lut_b[256];
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3c79b63..d5b87c8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -508,6 +508,15 @@ void intel_update_fbc(struct drm_device *dev)
>  	obj = intel_fb->obj;
>  	adjusted_mode = &intel_crtc->config.adjusted_mode;
>  
> +	if (dev_priv->fbc.plane == intel_crtc->plane &&
> +		INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +		(intel_crtc->rotation != BIT(DRM_ROTATE_0))) {

More weird indentation and needless parens.

> +		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> +			DRM_DEBUG_KMS("mode incompatible with compression, "
> +				      "disabling\n");
> +		goto out_disable;
> +	}
> +
>  	if (i915.enable_fbc < 0 &&
>  	    INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) {
>  		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@intel.com>

With clipped sprites these transformations are not working. these
functions transform complete sprite irrespective of clipping present.
This leads to invisible portion of sprite show up when rotate 180 if
it was out of visible area before.

v4: Moved rotate transform for source rectangle after clipping.
Added rotate and inverse rotate transform for destination rect.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: vijay.a.purushothaman@intel.com
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 62b9f84..799f6a9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	max_scale = intel_plane->max_downscale << 16;
 	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
-	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
-			intel_plane->rotation);
-
 	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
 	BUG_ON(hscale < 0);
 
@@ -785,6 +782,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	crtc_w = drm_rect_width(&dst);
 	crtc_h = drm_rect_height(&dst);
 
+	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
+				intel_plane->rotation);
+
+	drm_rect_rotate(&dst, intel_crtc->config.pipe_src_w,
+				intel_crtc->config.pipe_src_h,
+				intel_plane->rotation);
+
 	if (visible) {
 		/* check again in case clipping clamped the results */
 		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
@@ -811,7 +815,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
 
 		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
-				    intel_plane->rotation);
+					intel_plane->rotation);
+
+		drm_rect_rotate_inv(&dst, intel_crtc->config.pipe_src_w,
+					intel_crtc->config.pipe_src_h,
+					intel_plane->rotation);
 
 		/* sanity check to make sure the src viewport wasn't enlarged */
 		WARN_ON(src.x1 < (int) src_x ||
-- 
1.8.5

Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[Ville Syrjälä]

On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With clipped sprites these transformations are not working. these
> functions transform complete sprite irrespective of clipping present.
> This leads to invisible portion of sprite show up when rotate 180 if
> it was out of visible area before.
> 
> v4: Moved rotate transform for source rectangle after clipping.
> Added rotate and inverse rotate transform for destination rect.

Still NAK.

I just pushed rotation support to my glplane test app [1], and with
with that my rotated clipping code works exactly as intended.

[1] git://gitorious.org/vsyrjala/glplane.git

> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: vijay.a.purushothaman@intel.com
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 62b9f84..799f6a9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	max_scale = intel_plane->max_downscale << 16;
>  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>  
> -	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> -			intel_plane->rotation);
> -
>  	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
>  	BUG_ON(hscale < 0);
>  
> @@ -785,6 +782,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	crtc_w = drm_rect_width(&dst);
>  	crtc_h = drm_rect_height(&dst);
>  
> +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> +				intel_plane->rotation);
> +
> +	drm_rect_rotate(&dst, intel_crtc->config.pipe_src_w,
> +				intel_crtc->config.pipe_src_h,
> +				intel_plane->rotation);
> +
>  	if (visible) {
>  		/* check again in case clipping clamped the results */
>  		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> @@ -811,7 +815,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
>  
>  		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> -				    intel_plane->rotation);
> +					intel_plane->rotation);
> +
> +		drm_rect_rotate_inv(&dst, intel_crtc->config.pipe_src_w,
> +					intel_crtc->config.pipe_src_h,
> +					intel_plane->rotation);
>  
>  		/* sanity check to make sure the src viewport wasn't enlarged */
>  		WARN_ON(src.x1 < (int) src_x ||
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[Daniel Vetter]

On Tue, Feb 11, 2014 at 05:02:31PM +0530, Sagar Arun Kamble wrote:
> On Mon, 2014-02-10 at 15:32 +0200, Ville Syrjälä wrote:
> > On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@intel.com wrote:
> > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > 
> > > With clipped sprites these transformations are not working. these
> > > functions transform complete sprite irrespective of clipping present.
> > > This leads to invisible portion of sprite show up when rotate 180 if
> > > it was out of visible area before.
> > > 
> > > v4: Moved rotate transform for source rectangle after clipping.
> > > Added rotate and inverse rotate transform for destination rect.
> > 
> > Still NAK.
> > 
> > I just pushed rotation support to my glplane test app [1], and with
> > with that my rotated clipping code works exactly as intended.
> > 
> > [1] git://gitorious.org/vsyrjala/glplane.git
> I tried this app. I think I am considering 180 degree rotation of
> clipped sprite plane differently. I have captured output with these
> rotate transforms moved before(clip-rotated) and after(rotate-clipped)
> clipping code.
> 
> Which is valid? Rotating entire sprite and then clipping or Rotating
> clipped portion?
> 
> Reference and Rotated output is attached FYI.
> 
> If rotating entire sprite is correct then this patch 11/11 is not needed
> and can be abandoned.

I think doing the clipping first (as a viewport) and then rotating the
viewport makes more sense to me. Dunno what other people think, but I
guess we should document that somewhere.
-Daniel

> 
> thanks,
> Sagar
> > > 
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: vijay.a.purushothaman@intel.com
> > > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 62b9f84..799f6a9 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > >  	max_scale = intel_plane->max_downscale << 16;
> > >  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> > >  
> > > -	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > -			intel_plane->rotation);
> > > -
> > >  	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> > >  	BUG_ON(hscale < 0);
> > >  
> > > @@ -785,6 +782,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > >  	crtc_w = drm_rect_width(&dst);
> > >  	crtc_h = drm_rect_height(&dst);
> > >  
> > > +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > +				intel_plane->rotation);
> > > +
> > > +	drm_rect_rotate(&dst, intel_crtc->config.pipe_src_w,
> > > +				intel_crtc->config.pipe_src_h,
> > > +				intel_plane->rotation);
> > > +
> > >  	if (visible) {
> > >  		/* check again in case clipping clamped the results */
> > >  		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> > > @@ -811,7 +815,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > >  				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
> > >  
> > >  		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> > > -				    intel_plane->rotation);
> > > +					intel_plane->rotation);
> > > +
> > > +		drm_rect_rotate_inv(&dst, intel_crtc->config.pipe_src_w,
> > > +					intel_crtc->config.pipe_src_h,
> > > +					intel_plane->rotation);
> > >  
> > >  		/* sanity check to make sure the src viewport wasn't enlarged */
> > >  		WARN_ON(src.x1 < (int) src_x ||
> > > -- 
> > > 1.8.5
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 






-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[Sagar Arun Kamble]

On Tue, 2014-02-11 at 12:59 +0100, Daniel Vetter wrote:
> On Tue, Feb 11, 2014 at 05:02:31PM +0530, Sagar Arun Kamble wrote:
> > On Mon, 2014-02-10 at 15:32 +0200, Ville Syrjälä wrote:
> > > On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@intel.com wrote:
> > > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > > 
> > > > With clipped sprites these transformations are not working. these
> > > > functions transform complete sprite irrespective of clipping present.
> > > > This leads to invisible portion of sprite show up when rotate 180 if
> > > > it was out of visible area before.
> > > > 
> > > > v4: Moved rotate transform for source rectangle after clipping.
> > > > Added rotate and inverse rotate transform for destination rect.
> > > 
> > > Still NAK.
> > > 
> > > I just pushed rotation support to my glplane test app [1], and with
> > > with that my rotated clipping code works exactly as intended.
> > > 
> > > [1] git://gitorious.org/vsyrjala/glplane.git
> > I tried this app. I think I am considering 180 degree rotation of
> > clipped sprite plane differently. I have captured output with these
> > rotate transforms moved before(clip-rotated) and after(rotate-clipped)
> > clipping code.
> > 
> > Which is valid? Rotating entire sprite and then clipping or Rotating
> > clipped portion?
> > 
> > Reference and Rotated output is attached FYI.
> > 
> > If rotating entire sprite is correct then this patch 11/11 is not needed
> > and can be abandoned.
> 
> I think doing the clipping first (as a viewport) and then rotating the
> viewport makes more sense to me. Dunno what other people think, but I
> guess we should document that somewhere.
> -Daniel

Agree. Thought of use-case where if media player(that will use sprite
for video) is moved such that it is off the screen partially, after
rotation as well we should see same portion of clip inside(rotated
though).

> 
> > 
> > thanks,
> > Sagar
> > > > 
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: David Airlie <airlied@linux.ie>
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: vijay.a.purushothaman@intel.com
> > > > Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++----
> > > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > > index 62b9f84..799f6a9 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > > @@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > >  	max_scale = intel_plane->max_downscale << 16;
> > > >  	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> > > >  
> > > > -	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > > -			intel_plane->rotation);
> > > > -
> > > >  	hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> > > >  	BUG_ON(hscale < 0);
> > > >  
> > > > @@ -785,6 +782,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > >  	crtc_w = drm_rect_width(&dst);
> > > >  	crtc_h = drm_rect_height(&dst);
> > > >  
> > > > +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > > +				intel_plane->rotation);
> > > > +
> > > > +	drm_rect_rotate(&dst, intel_crtc->config.pipe_src_w,
> > > > +				intel_crtc->config.pipe_src_h,
> > > > +				intel_plane->rotation);
> > > > +
> > > >  	if (visible) {
> > > >  		/* check again in case clipping clamped the results */
> > > >  		hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> > > > @@ -811,7 +815,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > >  				     drm_rect_height(&dst) * vscale - drm_rect_height(&src));
> > > >  
> > > >  		drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> > > > -				    intel_plane->rotation);
> > > > +					intel_plane->rotation);
> > > > +
> > > > +		drm_rect_rotate_inv(&dst, intel_crtc->config.pipe_src_w,
> > > > +					intel_crtc->config.pipe_src_h,
> > > > +					intel_plane->rotation);
> > > >  
> > > >  		/* sanity check to make sure the src viewport wasn't enlarged */
> > > >  		WARN_ON(src.x1 < (int) src_x ||
> > > > -- 
> > > > 1.8.5
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > 
> 
> 
> 
> 
> 
> 

Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[Ville Syrjälä]

On Tue, Feb 11, 2014 at 05:02:31PM +0530, Sagar Arun Kamble wrote:
> On Mon, 2014-02-10 at 15:32 +0200, Ville Syrjälä wrote:
> > On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@intel.com wrote:
> > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > 
> > > With clipped sprites these transformations are not working. these
> > > functions transform complete sprite irrespective of clipping present.
> > > This leads to invisible portion of sprite show up when rotate 180 if
> > > it was out of visible area before.
> > > 
> > > v4: Moved rotate transform for source rectangle after clipping.
> > > Added rotate and inverse rotate transform for destination rect.
> > 
> > Still NAK.
> > 
> > I just pushed rotation support to my glplane test app [1], and with
> > with that my rotated clipping code works exactly as intended.
> > 
> > [1] git://gitorious.org/vsyrjala/glplane.git
> I tried this app. I think I am considering 180 degree rotation of
> clipped sprite plane differently. I have captured output with these
> rotate transforms moved before(clip-rotated) and after(rotate-clipped)
> clipping code.
> 
> Which is valid? Rotating entire sprite and then clipping or Rotating
> clipped portion?
> 
> Reference and Rotated output is attached FYI.
> 
> If rotating entire sprite is correct then this patch 11/11 is not needed
> and can be abandoned.

The way I think of these things is roughly this:

You have the user specified source rectangle, where the coordinates specify
the viewport into the framebuffer. This coordinate space is oriented the
same was as the framebuffer itself, ie. the first pixel of the
framebuffer is at coordinates 0,0. So plane rotation doesn't affect this
at all.

Then you have the user specified destination/crtc rectangle, where the
coordinates specify the position of the plane within the crtc coordinate
space. So the first visible pixel the pipe will push out is at
coordinates 0,0. So again plane rotation doesn't affect this.

Then you have the rotation which simply specifies the transformation to
be applied to the pixels when they "move" from the source rectangle to
the destination rectangle. So w/ 0 degree rotation the pixel at
src_x,src_y in the framebuffer will appear at position crtc_x,crtc_y
on the crtc output. With 180 degree rotation the pixel at src_x,src_y
will appear at crtc_x+crtc_w-1,crtc_y+crtc_h-1.

As clipping happens in the crtc coordinate space, we need to orient
the source coordindates the same way to get the correct clipping result.
So for example with 0 degrees rotation clipping the left side of the
destination rectangle must result in clipping the left side of the source
rectangle as well. And with 180 degree rotation clipping the destination
rectangle on the left side must result in clipping the source rectangle
on the right side. Left and right in each case referring to the original
unrotate coordinates.

So let's say we have the following situation w/ 180 degree rotation.
The letters inside the rects represented specific named pixels,
the FB rectangle represents the FB as specified by addfb2 ioctl,
the CRTC rectangle represents the pipe output (0,0 -> PIPESRC.w,h):

FB:                 CRTC:
0,0 ___________     0.0 __________
    |  abcd   |         |         |
    |  efgh   |         |         |
    |_________|         |hgfe     |
                        |dcba_____|
unclipped coordinates specified by user:
src_x=2,src_y=0     crtc_x=0,crtc_y=2
src_w=4.src_h=2     crtc_w=4,crtc_h=2

clipped coordinates:
src_x=2,src_y=0     crtc_x=0,crtc_y=2
src_w=4.src_h=2     crtc_w=4,crtc_h=2


Then the user moves the sprite one pixel to the left resulting on some
clipping (the X pixels). Note that the unclipped source coordinates do
not change here at all, in fact crtc_x is the only thing changed by the
user:

FB:                 CRTC:
0,0 ___________     0.0 __________
    |  abcX   |         |         |
    |  efgX   |         |         |
    |_________|         Xgfe      |
                        Xcba______|
unclipped coordinates specified by user:
src_x=2,src_y=0     crtc_x=-1,crtc_y=2
src_w=4.src_h=2     crtc_w= 4,crtc_h=2

clipped coordinates:
src_x=2,src_y=0     crtc_x=0,crtc_y=2
src_w=3.src_h=2     crtc_w=3,crtc_h=2

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and inverse rotate transformations after clipping

[Sagar Arun Kamble]

On Tue, 2014-02-11 at 16:56 +0200, Ville Syrjälä wrote:
> On Tue, Feb 11, 2014 at 05:02:31PM +0530, Sagar Arun Kamble wrote:
> > On Mon, 2014-02-10 at 15:32 +0200, Ville Syrjälä wrote:
> > > On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@intel.com wrote:
> > > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > > 
> > > > With clipped sprites these transformations are not working. these
> > > > functions transform complete sprite irrespective of clipping present.
> > > > This leads to invisible portion of sprite show up when rotate 180 if
> > > > it was out of visible area before.
> > > > 
> > > > v4: Moved rotate transform for source rectangle after clipping.
> > > > Added rotate and inverse rotate transform for destination rect.
> > > 
> > > Still NAK.
> > > 
> > > I just pushed rotation support to my glplane test app [1], and with
> > > with that my rotated clipping code works exactly as intended.
> > > 
> > > [1] git://gitorious.org/vsyrjala/glplane.git
> > I tried this app. I think I am considering 180 degree rotation of
> > clipped sprite plane differently. I have captured output with these
> > rotate transforms moved before(clip-rotated) and after(rotate-clipped)
> > clipping code.
> > 
> > Which is valid? Rotating entire sprite and then clipping or Rotating
> > clipped portion?
> > 
> > Reference and Rotated output is attached FYI.
> > 
> > If rotating entire sprite is correct then this patch 11/11 is not needed
> > and can be abandoned.
> 
> The way I think of these things is roughly this:
> 
> You have the user specified source rectangle, where the coordinates specify
> the viewport into the framebuffer. This coordinate space is oriented the
> same was as the framebuffer itself, ie. the first pixel of the
> framebuffer is at coordinates 0,0. So plane rotation doesn't affect this
> at all.
> 
> Then you have the user specified destination/crtc rectangle, where the
> coordinates specify the position of the plane within the crtc coordinate
> space. So the first visible pixel the pipe will push out is at
> coordinates 0,0. So again plane rotation doesn't affect this.
> 
> Then you have the rotation which simply specifies the transformation to
> be applied to the pixels when they "move" from the source rectangle to
> the destination rectangle. So w/ 0 degree rotation the pixel at
> src_x,src_y in the framebuffer will appear at position crtc_x,crtc_y
> on the crtc output. With 180 degree rotation the pixel at src_x,src_y
> will appear at crtc_x+crtc_w-1,crtc_y+crtc_h-1.
> 
> As clipping happens in the crtc coordinate space, we need to orient
> the source coordindates the same way to get the correct clipping result.
> So for example with 0 degrees rotation clipping the left side of the
> destination rectangle must result in clipping the left side of the source
> rectangle as well. And with 180 degree rotation clipping the destination
> rectangle on the left side must result in clipping the source rectangle
> on the right side. Left and right in each case referring to the original
> unrotate coordinates.
> 
> So let's say we have the following situation w/ 180 degree rotation.
> The letters inside the rects represented specific named pixels,
> the FB rectangle represents the FB as specified by addfb2 ioctl,
> the CRTC rectangle represents the pipe output (0,0 -> PIPESRC.w,h):
> 
> FB:                 CRTC:
> 0,0 ___________     0.0 __________
>     |  abcd   |         |         |
>     |  efgh   |         |         |
>     |_________|         |hgfe     |
>                         |dcba_____|
> unclipped coordinates specified by user:
> src_x=2,src_y=0     crtc_x=0,crtc_y=2
> src_w=4.src_h=2     crtc_w=4,crtc_h=2
> 
> clipped coordinates:
> src_x=2,src_y=0     crtc_x=0,crtc_y=2
> src_w=4.src_h=2     crtc_w=4,crtc_h=2
> 
> 
> Then the user moves the sprite one pixel to the left resulting on some
> clipping (the X pixels). Note that the unclipped source coordinates do
> not change here at all, in fact crtc_x is the only thing changed by the
> user:
> 
> FB:                 CRTC:
> 0,0 ___________     0.0 __________
>     |  abcX   |         |         |
>     |  efgX   |         |         |
>     |_________|         Xgfe      |
>                         Xcba______|
> unclipped coordinates specified by user:
> src_x=2,src_y=0     crtc_x=-1,crtc_y=2
> src_w=4.src_h=2     crtc_w= 4,crtc_h=2
> 
> clipped coordinates:
> src_x=2,src_y=0     crtc_x=0,crtc_y=2
> src_w=3.src_h=2     crtc_w=3,crtc_h=2
> 


Understood this now. Thank you Ville for providing this elaborate
graphical view of the rotation cases.

-Sagar