[PATCH 00/11] Support for 180 degree HW rotation

[PATCH 00/11] Support for 180 degree HW rotation

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

Enables 180 degree rotation for sprite and primary planes.
Updated the primary plane rotation support as per the new universal plane
design.

Most of these patches were already reviewed in intel-gfx in February 2014 thats
why there is version history in few of them.

Testcase: kms_rotation_crc
This igt can be extended for clipped rotation cases. Right it only tests 180
degree rotation for sprite and primary plane with crc check.

Sonika Jindal (2):
  tests/kms_rotation_crc: IGT for 180 degree HW rotation
  drm/i915: Add 180 degree primary plane rotation support

Ville Syrjälä (9):
  drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
  drm: Add support_bits parameter to drm_property_create_bitmask()
  drm: Add drm_mode_create_rotation_property()
  drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
  drm: Add drm_rect rotation functions
  drm: Add drm_rotation_simplify()
  drm/i915: Add 180 degree sprite rotation support
  drm/i915: Make intel_plane_restore() return an error
  drm/i915: Add rotation property for sprites

 drivers/gpu/drm/drm_crtc.c           |   66 +++++++++++++++-
 drivers/gpu/drm/drm_rect.c           |  140 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_dma.c      |   17 +++++
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |    4 +
 drivers/gpu/drm/i915/intel_display.c |   91 +++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |    3 +-
 drivers/gpu/drm/i915/intel_pm.c      |    8 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   93 ++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_drv.h   |    7 --
 drivers/gpu/drm/omapdrm/omap_plane.c |   17 ++---
 include/drm/drm_crtc.h               |   15 +++-
 include/drm/drm_rect.h               |    6 ++
 13 files changed, 433 insertions(+), 35 deletions(-)

-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

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.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 284b80f..b08a450 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 4ee7e26..bfc7235 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -75,6 +75,14 @@ static inline uint64_t I642U64(int64_t val)
 	return (uint64_t)*((uint64_t *)&val);
 }
 
+/* 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
+
 enum drm_connector_force {
 	DRM_FORCE_UNSPECIFIED,
 	DRM_FORCE_OFF,
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

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 at ti.com>
Cc: Rob Clark <robdclark at gmail.com>
Cc: Sagar Kamble <sagar.a.kamble at intel.com>
Cc: dri-devel at lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c           |   17 +++++++++++++----
 drivers/gpu/drm/omapdrm/omap_plane.c |    5 ++++-
 include/drm/drm_crtc.h               |    3 ++-
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 41c7212..2fbee61 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3395,19 +3395,28 @@ 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;
 
 	property = drm_property_create(dev, flags, name, num_values);
 	if (!property)
 		return NULL;
+	for (i = 0; i < num_props; i++) {
+		if (!(supported_bits & (1ULL << props[i].type)))
+			continue;
 
-	for (i = 0; i < num_values; i++) {
-		ret = drm_property_add_enum(property, i,
+		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 3cf31ee..aff06e7 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -317,7 +317,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 bfc7235..cb4850a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1006,7 +1006,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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

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

Add a function to create a standards compliant rotation property.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c |   18 ++++++++++++++++++
 include/drm/drm_crtc.h     |    2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2fbee61..f224d4d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4959,3 +4959,21 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
 }
 EXPORT_SYMBOL(drm_mode_config_cleanup);
+
+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, ARRAY_SIZE(props),
+					   supported_rotations);
+}
+EXPORT_SYMBOL(drm_mode_create_rotation_property);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index cb4850a..f7b383b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1113,6 +1113,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 */
 
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

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

Use the new drm_mode_create_rotation_property() in omapdrm.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index aff06e7..da9d15d 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -308,19 +308,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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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 at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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 at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala at 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 f224d4d..89bab66 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4842,6 +4842,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 f7b383b..08ed55e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1115,6 +1115,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 */
 
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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

v3: BDW also handles the rotated buffer offset automagically

Testcase: igt/kms_rotation_crc

Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: David Airlie <airlied at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble at 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 3488567..c70c804 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4171,6 +4171,7 @@ enum punit_power_well {
 #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)
@@ -4241,6 +4242,7 @@ enum punit_power_well {
 #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)
@@ -4314,6 +4316,7 @@ enum punit_power_well {
 #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 ab5962b..67b1c59 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -437,6 +437,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 404335d..54d4224 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -163,6 +163,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:
@@ -234,6 +235,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;
+	}
+
 	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
 
 	intel_update_primary_plane(intel_crtc);
@@ -363,6 +372,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:
@@ -424,6 +434,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 and BDW does this automagically in hardware */
+		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
+			x += src_w;
+			y += src_h;
+			linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
+		}
+	}
+
 	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
 
 	intel_update_primary_plane(intel_crtc);
@@ -569,6 +590,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:
@@ -625,6 +647,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;
+	}
+
 	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
 
 	intel_update_primary_plane(intel_crtc);
@@ -883,6 +913,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);
 
@@ -921,6 +954,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 ||
@@ -1296,6 +1332,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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: David Airlie <airlied at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 67b1c59..da5a3ca 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -993,7 +993,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 54d4224..cbad738 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1202,18 +1202,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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
Cc: Jani Nikula <jani.nikula at linux.intel.com>
Cc: David Airlie <airlied at linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 0640071..b56a1a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1514,6 +1514,7 @@ 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 cbad738..b9af256 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1202,6 +1202,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);
@@ -1228,6 +1252,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[] = {
@@ -1264,6 +1289,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;
@@ -1338,8 +1364,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.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Damien Lespiau]

On Wed, Jun 18, 2014 at 02:27:25PM +0530, sonika.jindal@intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: David Airlie <airlied at linux.ie>
> Cc: dri-devel at lists.freedesktop.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 0640071..b56a1a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1514,6 +1514,7 @@ 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 cbad738..b9af256 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1202,6 +1202,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) {

Shouldn't we add a:
	
		if (val & (BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)))
			return -EINVAL;

To ensure userspace doesn't send garbage in the upper bits so we can
reuse them down the road?

-- 
Damien

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

[Jindal, Sonika]

On 6/18/2014 4:42 PM, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:25PM +0530, sonika.jindal@intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>> Cc: David Airlie <airlied at linux.ie>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: linux-kernel at vger.kernel.org
>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 0640071..b56a1a5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1514,6 +1514,7 @@ 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 cbad738..b9af256 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -1202,6 +1202,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) {
>
> Shouldn't we add a:
> 	
> 		if (val & (BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)))
> 			return -EINVAL;
>
> To ensure userspace doesn't send garbage in the upper bits so we can
> reuse them down the road?
>
But we are checking if more than one bit is set, we return EINVAL.
So we only care for one rotation angle being sent from user.
Shouldn't that suffice?

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

[Damien Lespiau]

On Wed, Jun 18, 2014 at 05:24:56PM +0530, Jindal, Sonika wrote:
> >Shouldn't we add a:
> >	
> >		if (val & (BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)))
> >			return -EINVAL;
> >
> >To ensure userspace doesn't send garbage in the upper bits so we can
> >reuse them down the road?
> >
> But we are checking if more than one bit is set, we return EINVAL.
> So we only care for one rotation angle being sent from user.
> Shouldn't that suffice?

Nop. If given (1 << 50) we'd still pass the test but with an invalid
(reserved) value.

I didn't spot the generic drm_property_change_is_valid() (Ville's
answer), so it should be handled for us by the DRM core already.

All is fine.

-- 
Damien

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

[Ville Syrjälä]

On Wed, Jun 18, 2014 at 12:12:52PM +0100, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:25PM +0530, sonika.jindal@intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at 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 at ffwll.ch>
> > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > Cc: David Airlie <airlied at linux.ie>
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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 0640071..b56a1a5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1514,6 +1514,7 @@ 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 cbad738..b9af256 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1202,6 +1202,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) {
> 
> Shouldn't we add a:
> 	
> 		if (val & (BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180)))
> 			return -EINVAL;
> 
> To ensure userspace doesn't send garbage in the upper bits so we can
> reuse them down the road?

drm_property_change_is_valid() should already check that no unsupported
bits can escape into the driver.

-- 
Ville Syrjälä
Intel OTC

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

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@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.

v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.

v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.

v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.

v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb  and return value of update_primary_plane is ignored.

v9: added rotation property to primary plane instead of crtc.

Testcase: igt/kms_rotation_crc

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

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5e583a1..4c91fbc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 void i915_driver_lastclose(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc;
+	struct intel_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
@@ -1946,6 +1948,21 @@ 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, base.head) {
+			to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
+			drm_object_property_set_value(&crtc->base.base,
+						dev_priv->rotation_property,
+						to_intel_plane(crtc->base.primary)->rotation);
+		}
+		list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
+			plane->rotation = BIT(DRM_ROTATE_0);
+			drm_object_property_set_value(&plane->base.base,
+						dev_priv->rotation_property,
+						plane->rotation);
+		}
+	}
+
 	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 c70c804..c600d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4087,6 +4087,7 @@ enum punit_power_well {
 #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				0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..1dc8b68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
@@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	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) {
@@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -2504,7 +2517,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2512,6 +2527,8 @@ static void ironlake_update_primary_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;
@@ -2549,8 +2566,6 @@ static void ironlake_update_primary_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,
@@ -2558,6 +2573,19 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -11324,10 +11352,51 @@ static void intel_plane_destroy(struct drm_plane *plane)
 	kfree(intel_plane);
 }
 
+static int intel_primary_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
+	struct drm_crtc *crtc = &intel_crtc->base;
+	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;
+
+		if (intel_crtc->active && intel_crtc->primary_enabled) {
+			intel_crtc_wait_for_pending_flips(crtc);
+
+			/* FBC does not work on some platforms for rotated planes */
+			if (dev_priv->fbc.plane == intel_crtc->plane &&
+			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			    intel_plane->rotation != BIT(DRM_ROTATE_0))
+				intel_disable_fbc(dev);
+
+			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, 0, 0);
+		} else {
+			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
+					crtc->base.id);
+			ret = 0;
+		}
+	}
+
+	return ret;
+}
+
 static const struct drm_plane_funcs intel_primary_plane_funcs = {
 	.update_plane = intel_primary_plane_setplane,
 	.disable_plane = intel_primary_plane_disable,
 	.destroy = intel_plane_destroy,
+	.set_property = intel_primary_plane_set_property
 };
 
 static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -11335,6 +11404,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 {
 	struct intel_plane *primary;
 	const uint32_t *intel_primary_formats;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int num_formats;
 
 	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
@@ -11345,6 +11415,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->max_downscale = 1;
 	primary->pipe = pipe;
 	primary->plane = pipe;
+	primary->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
@@ -11360,6 +11431,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 				 &intel_primary_plane_funcs,
 				 intel_primary_formats, num_formats,
 				 DRM_PLANE_TYPE_PRIMARY);
+	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(&primary->base.base,
+						dev_priv->rotation_property,
+						primary->rotation);
+	}
+
 	return &primary->base;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..cabccfb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -562,6 +562,14 @@ void intel_update_fbc(struct drm_device *dev)
 		goto out_disable;
 	}
 
+	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+	    to_intel_plane(crtc->primary)->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 the kernel debugger is active, always disable compression */
 	if (in_dbg_master())
 		goto out_disable;
-- 
1.7.10.4

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

[Damien Lespiau]

On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@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.
> 
> v5: Resetting property in i915_driver_lastclose properly for planes
> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
> width in i9xx_update_plane and ironlake_update_plane. Removed tab
> based indentation and unnecessary braces in intel_crtc_set_property
> and intel_update_fbc. FBC and flip related checks should be done only
> for valid crtcs.
> 
> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
> and positioning the disable code in intel_update_fbc.
> 
> v7: In case rotation property on inactive crtc is updated, we return
> successfully printing debug log as crtc is inactive and only property change
> is preserved.
> 
> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
> crtc->primary->fb  and return value of update_primary_plane is ignored.
> 
> v9: added rotation property to primary plane instead of crtc.
> 
> Testcase: igt/kms_rotation_crc
> 
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: David Airlie <airlied at linux.ie>
> Cc: dri-devel at lists.freedesktop.org
> Cc: linux-kernel at vger.kernel.org
> Cc: vijay.a.purushothaman at intel.com
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>

Some checkpatch.pl warnings:

ERROR: code indent should use tabs where possible
#145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
+                                   (intel_crtc->config.pipe_src_w - 1) * pixel_size;$

WARNING: please, no spaces at the start of a line
#145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
+                                   (intel_crtc->config.pipe_src_w - 1) * pixel_size;$


> ---
>  drivers/gpu/drm/i915/i915_dma.c      |   17 +++++++
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   91 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>  4 files changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5e583a1..4c91fbc 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>  void i915_driver_lastclose(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *crtc;
> +	struct intel_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
> @@ -1946,6 +1948,21 @@ 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, base.head) {
> +			to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
> +			drm_object_property_set_value(&crtc->base.base,
> +						dev_priv->rotation_property,
> +						to_intel_plane(crtc->base.primary)->rotation);
> +		}
> +		list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
> +			plane->rotation = BIT(DRM_ROTATE_0);
> +			drm_object_property_set_value(&plane->base.base,
> +						dev_priv->rotation_property,
> +						plane->rotation);
> +		}
> +	}
> +

The purpose of this seems to be restoring rotation to 0 and rely on the next
modeset to re-program the register. This code, is orthogonal to the pure
primary plane rotation enabling, spans through both sprite and primary planes
and may actually be a bit tricky to get right.

-> This shouldn't be part of the same commit as the primary plane rotation.
Please span a new commit for it.

Now, we also need something like this when switching VT, and probably for
kernel panic and debug handling as well, so lastclose() is not enough (and we
can probably do better).

One idea would be for the core DRM code to know about this property, and make
sure we put the rotation back to 0 in restore_fbdev_mode(), we do something
similar for the for the sprite planes in there already. Another idea would be
to add a vfunc to execute driver specific code there in restore_fbdev_mode().

There is probably a better way to do it, I have to say I'm not super familiar
with this part of the driver.


>  	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 c70c804..c600d3b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4087,6 +4087,7 @@ enum punit_power_well {
>  #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				0x70184
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e8e711..1dc8b68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
> @@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	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) {
> @@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> +	if (to_intel_plane(crtc->primary)->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 - 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]);
> @@ -2504,7 +2517,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2512,6 +2527,8 @@ static void ironlake_update_primary_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;
> @@ -2549,8 +2566,6 @@ static void ironlake_update_primary_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,
> @@ -2558,6 +2573,19 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  					       fb->pitches[0]);
>  	linear_offset -= intel_crtc->dspaddr_offset;
>  
> +	if (to_intel_plane(crtc->primary)->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 - 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]);
> @@ -11324,10 +11352,51 @@ static void intel_plane_destroy(struct drm_plane *plane)
>  	kfree(intel_plane);
>  }
>  
> +static int intel_primary_plane_set_property(struct drm_plane *plane,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
> +	struct drm_crtc *crtc = &intel_crtc->base;
> +	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;

This value is set but never used again?

> +		intel_plane->rotation = val;
> +
> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
> +			intel_crtc_wait_for_pending_flips(crtc);
> +
> +			/* FBC does not work on some platforms for rotated planes */
> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
> +				intel_disable_fbc(dev);
> +
> +			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, 0, 0);
> +		} else {
> +			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
> +					crtc->base.id);
> +			ret = 0;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct drm_plane_funcs intel_primary_plane_funcs = {
>  	.update_plane = intel_primary_plane_setplane,
>  	.disable_plane = intel_primary_plane_disable,
>  	.destroy = intel_plane_destroy,
> +	.set_property = intel_primary_plane_set_property
>  };
>  
>  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> @@ -11335,6 +11404,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  {
>  	struct intel_plane *primary;
>  	const uint32_t *intel_primary_formats;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int num_formats;
>  
>  	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> @@ -11345,6 +11415,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	primary->max_downscale = 1;
>  	primary->pipe = pipe;
>  	primary->plane = pipe;
> +	primary->rotation = BIT(DRM_ROTATE_0);
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>  		primary->plane = !pipe;
>  
> @@ -11360,6 +11431,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  				 &intel_primary_plane_funcs,
>  				 intel_primary_formats, num_formats,
>  				 DRM_PLANE_TYPE_PRIMARY);
> +	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(&primary->base.base,
> +						dev_priv->rotation_property,
> +						primary->rotation);
> +	}
> +
>  	return &primary->base;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2043c4b..cabccfb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -562,6 +562,14 @@ void intel_update_fbc(struct drm_device *dev)
>  		goto out_disable;
>  	}
>  
> +	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +	    to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
> +		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> +			DRM_DEBUG_KMS("mode incompatible with compression, "
> +				      "disabling\n");

This debug message isn't helpful. You need to add that's because of the
rotation.

> +		goto out_disable;
> +	}
> +
>  	/* If the kernel debugger is active, always disable compression */
>  	if (in_dbg_master())
>  		goto out_disable;
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jindal, Sonika]

On 6/18/2014 10:32 PM, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@intel.com wrote:
>> From: Sonika Jindal <sonika.jindal@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.
>>
>> v5: Resetting property in i915_driver_lastclose properly for planes
>> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
>> width in i9xx_update_plane and ironlake_update_plane. Removed tab
>> based indentation and unnecessary braces in intel_crtc_set_property
>> and intel_update_fbc. FBC and flip related checks should be done only
>> for valid crtcs.
>>
>> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
>> and positioning the disable code in intel_update_fbc.
>>
>> v7: In case rotation property on inactive crtc is updated, we return
>> successfully printing debug log as crtc is inactive and only property change
>> is preserved.
>>
>> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
>> crtc->primary->fb  and return value of update_primary_plane is ignored.
>>
>> v9: added rotation property to primary plane instead of crtc.
>>
>> Testcase: igt/kms_rotation_crc
>>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>> Cc: David Airlie <airlied at linux.ie>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: linux-kernel at vger.kernel.org
>> Cc: vijay.a.purushothaman at intel.com
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>> Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>
>
> Some checkpatch.pl warnings:
>
> ERROR: code indent should use tabs where possible
> #145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
> +                                   (intel_crtc->config.pipe_src_w - 1) * pixel_size;$
>
> WARNING: please, no spaces at the start of a line
> #145: FILE: drivers/gpu/drm/i915/intel_display.c:2488:
> +                                   (intel_crtc->config.pipe_src_w - 1) * pixel_size;$
>
>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c      |   17 +++++++
>>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>   drivers/gpu/drm/i915/intel_display.c |   91 ++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>>   4 files changed, 113 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 5e583a1..4c91fbc 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>>   void i915_driver_lastclose(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_crtc *crtc;
>> +	struct intel_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
>> @@ -1946,6 +1948,21 @@ 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, base.head) {
>> +			to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
>> +			drm_object_property_set_value(&crtc->base.base,
>> +						dev_priv->rotation_property,
>> +						to_intel_plane(crtc->base.primary)->rotation);
>> +		}
>> +		list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
>> +			plane->rotation = BIT(DRM_ROTATE_0);
>> +			drm_object_property_set_value(&plane->base.base,
>> +						dev_priv->rotation_property,
>> +						plane->rotation);
>> +		}
>> +	}
>> +
>
> The purpose of this seems to be restoring rotation to 0 and rely on the next
> modeset to re-program the register. This code, is orthogonal to the pure
> primary plane rotation enabling, spans through both sprite and primary planes
> and may actually be a bit tricky to get right.
>
> -> This shouldn't be part of the same commit as the primary plane rotation.
> Please span a new commit for it.
Sure I will add another patch.
>
> Now, we also need something like this when switching VT, and probably for
> kernel panic and debug handling as well, so lastclose() is not enough (and we
> can probably do better).
>
> One idea would be for the core DRM code to know about this property, and make
> sure we put the rotation back to 0 in restore_fbdev_mode(), we do something
> similar for the for the sprite planes in there already. Another idea would be
> to add a vfunc to execute driver specific code there in restore_fbdev_mode().
>
> There is probably a better way to do it, I have to say I'm not super familiar
> with this part of the driver.
>
I see that in omap driver too it is done in lastclose of the driver. 
Also, from driver fbdev_restore is only called during lastclose.
Again I don't have more knowledge on this.
Can we keep it here in this lastclose function to comply with omap driver?
>
>>   	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 c70c804..c600d3b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4087,6 +4087,7 @@ enum punit_power_well {
>>   #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				0x70184
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 5e8e711..1dc8b68 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   	unsigned long linear_offset;
>>   	u32 dspcntr;
>>   	u32 reg;
>> +	int pixel_size;
>>
>> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>>   	intel_fb = to_intel_framebuffer(fb);
>>   	obj = intel_fb->obj;
>>
>> @@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
>> @@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   	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) {
>> @@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   		intel_crtc->dspaddr_offset = linear_offset;
>>   	}
>>
>> +	if (to_intel_plane(crtc->primary)->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 - 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]);
>> @@ -2504,7 +2517,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>>   	unsigned long linear_offset;
>>   	u32 dspcntr;
>>   	u32 reg;
>> +	int pixel_size;
>>
>> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>>   	intel_fb = to_intel_framebuffer(fb);
>>   	obj = intel_fb->obj;
>>
>> @@ -2512,6 +2527,8 @@ static void ironlake_update_primary_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;
>> @@ -2549,8 +2566,6 @@ static void ironlake_update_primary_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,
>> @@ -2558,6 +2573,19 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>>   					       fb->pitches[0]);
>>   	linear_offset -= intel_crtc->dspaddr_offset;
>>
>> +	if (to_intel_plane(crtc->primary)->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 - 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]);
>> @@ -11324,10 +11352,51 @@ static void intel_plane_destroy(struct drm_plane *plane)
>>   	kfree(intel_plane);
>>   }
>>
>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>> +				    struct drm_property *prop,
>> +				    uint64_t val)
>> +{
>> +	struct drm_device *dev = plane->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>> +	struct drm_crtc *crtc = &intel_crtc->base;
>> +	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;
>
> This value is set but never used again?
This was a miss from the last version of the patch. Will remove it.
>
>> +		intel_plane->rotation = val;
>> +
>> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
>> +			intel_crtc_wait_for_pending_flips(crtc);
>> +
>> +			/* FBC does not work on some platforms for rotated planes */
>> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
>> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
>> +				intel_disable_fbc(dev);
>> +
>> +			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, 0, 0);
>> +		} else {
>> +			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
>> +					crtc->base.id);
>> +			ret = 0;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   static const struct drm_plane_funcs intel_primary_plane_funcs = {
>>   	.update_plane = intel_primary_plane_setplane,
>>   	.disable_plane = intel_primary_plane_disable,
>>   	.destroy = intel_plane_destroy,
>> +	.set_property = intel_primary_plane_set_property
>>   };
>>
>>   static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>> @@ -11335,6 +11404,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>>   {
>>   	struct intel_plane *primary;
>>   	const uint32_t *intel_primary_formats;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	int num_formats;
>>
>>   	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
>> @@ -11345,6 +11415,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>>   	primary->max_downscale = 1;
>>   	primary->pipe = pipe;
>>   	primary->plane = pipe;
>> +	primary->rotation = BIT(DRM_ROTATE_0);
>>   	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>>   		primary->plane = !pipe;
>>
>> @@ -11360,6 +11431,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>>   				 &intel_primary_plane_funcs,
>>   				 intel_primary_formats, num_formats,
>>   				 DRM_PLANE_TYPE_PRIMARY);
>> +	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(&primary->base.base,
>> +						dev_priv->rotation_property,
>> +						primary->rotation);
>> +	}
>> +
>>   	return &primary->base;
>>   }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 2043c4b..cabccfb 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -562,6 +562,14 @@ void intel_update_fbc(struct drm_device *dev)
>>   		goto out_disable;
>>   	}
>>
>> +	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>> +	    to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
>> +		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
>> +			DRM_DEBUG_KMS("mode incompatible with compression, "
>> +				      "disabling\n");
>
> This debug message isn't helpful. You need to add that's because of the
> rotation.
Sure, will add.
>
>> +		goto out_disable;
>> +	}
>> +
>>   	/* If the kernel debugger is active, always disable compression */
>>   	if (in_dbg_master())
>>   		goto out_disable;
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

On Thu, Jun 19, 2014 at 12:13:54PM +0530, Jindal, Sonika wrote:
> On 6/18/2014 10:32 PM, Damien Lespiau wrote:
> >On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@intel.com wrote:
> >>From: Sonika Jindal <sonika.jindal@intel.com>


> >>diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >>index 5e583a1..4c91fbc 100644
> >>--- a/drivers/gpu/drm/i915/i915_dma.c
> >>+++ b/drivers/gpu/drm/i915/i915_dma.c
> >>@@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
> >>  void i915_driver_lastclose(struct drm_device *dev)
> >>  {
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>+	struct intel_crtc *crtc;
> >>+	struct intel_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
> >>@@ -1946,6 +1948,21 @@ 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, base.head) {
> >>+			to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
> >>+			drm_object_property_set_value(&crtc->base.base,
> >>+						dev_priv->rotation_property,
> >>+						to_intel_plane(crtc->base.primary)->rotation);
> >>+		}
> >>+		list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
> >>+			plane->rotation = BIT(DRM_ROTATE_0);
> >>+			drm_object_property_set_value(&plane->base.base,
> >>+						dev_priv->rotation_property,
> >>+						plane->rotation);
> >>+		}
> >>+	}
> >>+
> >
> >The purpose of this seems to be restoring rotation to 0 and rely on the next
> >modeset to re-program the register. This code, is orthogonal to the pure
> >primary plane rotation enabling, spans through both sprite and primary planes
> >and may actually be a bit tricky to get right.
> >
> >-> This shouldn't be part of the same commit as the primary plane rotation.
> >Please span a new commit for it.
> Sure I will add another patch.
> >
> >Now, we also need something like this when switching VT, and probably for
> >kernel panic and debug handling as well, so lastclose() is not enough (and we
> >can probably do better).
> >
> >One idea would be for the core DRM code to know about this property, and make
> >sure we put the rotation back to 0 in restore_fbdev_mode(), we do something
> >similar for the for the sprite planes in there already. Another idea would be
> >to add a vfunc to execute driver specific code there in restore_fbdev_mode().
> >
> >There is probably a better way to do it, I have to say I'm not super familiar
> >with this part of the driver.
> >
> I see that in omap driver too it is done in lastclose of the driver. Also,
> from driver fbdev_restore is only called during lastclose.
> Again I don't have more knowledge on this.
> Can we keep it here in this lastclose function to comply with omap driver?

No, this really should be done in
drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the
restore_fbdev_mode function in there. Once that's done and once omap is
also using the generic rotation properties (I think it is already) we can
remove the rotation handling code from omap's last_close. Please also
throw a (compile-tested-only) patch on top for that so that Rob Clark can
pick it up.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Jindal, Sonika]

On 6/19/2014 12:37 PM, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 12:13:54PM +0530, Jindal, Sonika wrote:
>> On 6/18/2014 10:32 PM, Damien Lespiau wrote:
>>> On Wed, Jun 18, 2014 at 02:27:26PM +0530, sonika.jindal@intel.com wrote:
>>>> From: Sonika Jindal <sonika.jindal@intel.com>
>
>
>>>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>>>> index 5e583a1..4c91fbc 100644
>>>> --- a/drivers/gpu/drm/i915/i915_dma.c
>>>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>>>> @@ -1939,6 +1939,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>>>>   void i915_driver_lastclose(struct drm_device *dev)
>>>>   {
>>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>>> +	struct intel_crtc *crtc;
>>>> +	struct intel_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
>>>> @@ -1946,6 +1948,21 @@ 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, base.head) {
>>>> +			to_intel_plane(crtc->base.primary)->rotation = BIT(DRM_ROTATE_0);
>>>> +			drm_object_property_set_value(&crtc->base.base,
>>>> +						dev_priv->rotation_property,
>>>> +						to_intel_plane(crtc->base.primary)->rotation);
>>>> +		}
>>>> +		list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
>>>> +			plane->rotation = BIT(DRM_ROTATE_0);
>>>> +			drm_object_property_set_value(&plane->base.base,
>>>> +						dev_priv->rotation_property,
>>>> +						plane->rotation);
>>>> +		}
>>>> +	}
>>>> +
>>>
>>> The purpose of this seems to be restoring rotation to 0 and rely on the next
>>> modeset to re-program the register. This code, is orthogonal to the pure
>>> primary plane rotation enabling, spans through both sprite and primary planes
>>> and may actually be a bit tricky to get right.
>>>
>>> -> This shouldn't be part of the same commit as the primary plane rotation.
>>> Please span a new commit for it.
>> Sure I will add another patch.
>>>
>>> Now, we also need something like this when switching VT, and probably for
>>> kernel panic and debug handling as well, so lastclose() is not enough (and we
>>> can probably do better).
>>>
>>> One idea would be for the core DRM code to know about this property, and make
>>> sure we put the rotation back to 0 in restore_fbdev_mode(), we do something
>>> similar for the for the sprite planes in there already. Another idea would be
>>> to add a vfunc to execute driver specific code there in restore_fbdev_mode().
>>>
>>> There is probably a better way to do it, I have to say I'm not super familiar
>>> with this part of the driver.
>>>
>> I see that in omap driver too it is done in lastclose of the driver. Also,
>> from driver fbdev_restore is only called during lastclose.
>> Again I don't have more knowledge on this.
>> Can we keep it here in this lastclose function to comply with omap driver?
>
> No, this really should be done in
> drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the
> restore_fbdev_mode function in there. Once that's done and once omap is
> also using the generic rotation properties (I think it is already) we can
> remove the rotation handling code from omap's last_close. Please also
> throw a (compile-tested-only) patch on top for that so that Rob Clark can
> pick it up.
> -Daniel
>
Ok, I will add it. So should I add a function pointer say 
reset_properties in crtc, which will be called from restore_fbdev_mode?

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

[Daniel Vetter]

On Thu, Jun 19, 2014 at 9:52 AM, Jindal, Sonika <sonika.jindal@intel.com> wrote:
>> No, this really should be done in
>> drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the
>> restore_fbdev_mode function in there. Once that's done and once omap is
>> also using the generic rotation properties (I think it is already) we can
>> remove the rotation handling code from omap's last_close. Please also
>> throw a (compile-tested-only) patch on top for that so that Rob Clark can
>> pick it up.
>> -Daniel
>>
> Ok, I will add it. So should I add a function pointer say reset_properties
> in crtc, which will be called from restore_fbdev_mode?

No, I think it should directly reset the relevant properties. This
might mean that we have to move the rotation property pointer to
struct drm_plane so that restore_fbdev_mode can get at it. Or we wrap
up a helper for internal property setting purposes which bails out if
the property isn't attached to the relevant object.

This way it will automatically work for all drivers that support
rotation. With a callback we still have the same problem that each
driver needs to do their own magic, which means they'll get it wrong.
Letting helpers take care of such details gives us a much stronger
platfrom with drm drivers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Jindal, Sonika]

On 6/19/2014 1:25 PM, Daniel Vetter wrote:
> On Thu, Jun 19, 2014 at 9:52 AM, Jindal, Sonika <sonika.jindal@intel.com> wrote:
>>> No, this really should be done in
>>> drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the
>>> restore_fbdev_mode function in there. Once that's done and once omap is
>>> also using the generic rotation properties (I think it is already) we can
>>> remove the rotation handling code from omap's last_close. Please also
>>> throw a (compile-tested-only) patch on top for that so that Rob Clark can
>>> pick it up.
>>> -Daniel
>>>
>> Ok, I will add it. So should I add a function pointer say reset_properties
>> in crtc, which will be called from restore_fbdev_mode?
>
> No, I think it should directly reset the relevant properties. This
> might mean that we have to move the rotation property pointer to
> struct drm_plane so that restore_fbdev_mode can get at it. Or we wrap
> up a helper for internal property setting purposes which bails out if
> the property isn't attached to the relevant object.
So, I will move rotation_property to drm_plane and for each plane where 
this property is attached, will call drm_object_property_set_value.
Please correct me if I am wrong.
>
> This way it will automatically work for all drivers that support
> rotation. With a callback we still have the same problem that each
> driver needs to do their own magic, which means they'll get it wrong.
> Letting helpers take care of such details gives us a much stronger
> platfrom with drm drivers.
> -Daniel
>

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

[Daniel Vetter]

On Thu, Jun 19, 2014 at 01:39:17PM +0530, Jindal, Sonika wrote:
> 
> 
> On 6/19/2014 1:25 PM, Daniel Vetter wrote:
> >On Thu, Jun 19, 2014 at 9:52 AM, Jindal, Sonika <sonika.jindal@intel.com> wrote:
> >>>No, this really should be done in
> >>>drm_fb_helper_restore_fbdev_mode_unlocked in drm_fb_helper.c. Well, in the
> >>>restore_fbdev_mode function in there. Once that's done and once omap is
> >>>also using the generic rotation properties (I think it is already) we can
> >>>remove the rotation handling code from omap's last_close. Please also
> >>>throw a (compile-tested-only) patch on top for that so that Rob Clark can
> >>>pick it up.
> >>>-Daniel
> >>>
> >>Ok, I will add it. So should I add a function pointer say reset_properties
> >>in crtc, which will be called from restore_fbdev_mode?
> >
> >No, I think it should directly reset the relevant properties. This
> >might mean that we have to move the rotation property pointer to
> >struct drm_plane so that restore_fbdev_mode can get at it. Or we wrap
> >up a helper for internal property setting purposes which bails out if
> >the property isn't attached to the relevant object.
> So, I will move rotation_property to drm_plane and for each plane where this
> property is attached, will call drm_object_property_set_value.
> Please correct me if I am wrong.

Yeah, that sounds like a plan.
-Daniel

> >
> >This way it will automatically work for all drivers that support
> >rotation. With a callback we still have the same problem that each
> >driver needs to do their own magic, which means they'll get it wrong.
> >Letting helpers take care of such details gives us a much stronger
> >platfrom with drm drivers.
> >-Daniel
> >

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

[PATCH] drm: Resetting rotation property

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

Reset rotation to 0 wherever applicable

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d5d8cea..6d96ea1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -282,9 +282,15 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+
+		if (plane->rotation_property)
+			drm_object_property_set_value(&plane->base,
+				plane->rotation_property, BIT(DRM_ROTATE_0));
+
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
+	}
 
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
-- 
1.7.10.4

[PATCH 0/3] Moving rotation_property to drm_plane

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

As suggested by Daniel and Damien, moved rotation_property to drm_plane.
Also moved resetting of rotation_property to restore_fbdev_mode which will be
used in switching VT use case along with the driver lastclose path.

Sonika Jindal (2):
  drm/i915: Add 180 degree primary plane rotation support
  drm: Resetting rotation property

Ville Syrjälä (1):
  drm/i915: Add rotation property for sprites

 drivers/gpu/drm/drm_fb_helper.c      |   14 ++++-
 drivers/gpu/drm/i915/i915_dma.c      |    5 --
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   93 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      |    8 +++
 drivers/gpu/drm/i915/intel_sprite.c  |   40 ++++++++++++++-
 include/drm/drm_crtc.h               |    1 +
 7 files changed, 150 insertions(+), 12 deletions(-)

-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Add rotation property for sprites

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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.

v2: Moving rotation_property to drm_plane

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |   42 ++++++++++++++++++++++++++++++++++-
 include/drm/drm_crtc.h              |    1 +
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cbad738..aa94f67 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1202,6 +1202,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 == plane->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);
@@ -1228,6 +1252,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[] = {
@@ -1264,6 +1289,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;
@@ -1338,8 +1364,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 (!intel_plane->base.rotation_property)
+		intel_plane->base.rotation_property =
+			drm_mode_create_rotation_property(dev,
+							  BIT(DRM_ROTATE_0) |
+							  BIT(DRM_ROTATE_180));
+
+	if (intel_plane->base.rotation_property)
+		drm_object_attach_property(&intel_plane->base.base,
+					   intel_plane->base.rotation_property,
+					   intel_plane->rotation);
 
+ out:
 	return ret;
 }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 08ed55e..6006c70 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -615,6 +615,7 @@ struct drm_plane {
 
 	const struct drm_plane_funcs *funcs;
 
+	struct drm_property *rotation_property;
 	struct drm_object_properties properties;
 
 	enum drm_plane_type type;
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@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.

v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.

v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.

v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.

v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb  and return value of update_primary_plane is ignored.

v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to drm_plane,so
drm layer will take care of resetting.

Testcase: igt/kms_rotation_crc

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |    5 --
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      |    8 +++
 4 files changed, 98 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5e583a1..4b6b911 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1938,14 +1938,9 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
  */
 void i915_driver_lastclose(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	/* 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
 	 * up anything. */
-	if (!dev_priv)
-		return;
-
 	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 c70c804..c600d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4087,6 +4087,7 @@ enum punit_power_well {
 #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				0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..85bd3b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
@@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	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) {
@@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -2487,7 +2500,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else
-		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + linear_offset);
+		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) +
+				linear_offset);
 	POSTING_READ(reg);
 }
 
@@ -2504,7 +2518,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2512,6 +2528,8 @@ static void ironlake_update_primary_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;
@@ -2549,8 +2567,6 @@ static void ironlake_update_primary_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,
@@ -2558,6 +2574,20 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -11324,10 +11354,50 @@ static void intel_plane_destroy(struct drm_plane *plane)
 	kfree(intel_plane);
 }
 
+static int intel_primary_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
+	struct drm_crtc *crtc = &intel_crtc->base;
+	uint64_t old_val;
+
+	if (prop == plane->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		old_val = intel_plane->rotation;
+		intel_plane->rotation = val;
+
+		if (intel_crtc->active && intel_crtc->primary_enabled) {
+			intel_crtc_wait_for_pending_flips(crtc);
+
+			/* FBC does not work on some platforms for rotated planes */
+			if (dev_priv->fbc.plane == intel_crtc->plane &&
+			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			    intel_plane->rotation != BIT(DRM_ROTATE_0))
+				intel_disable_fbc(dev);
+			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
+						0, 0);
+
+		} else {
+			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
+					crtc->base.id);
+		}
+	}
+
+	return 0;
+}
+
 static const struct drm_plane_funcs intel_primary_plane_funcs = {
 	.update_plane = intel_primary_plane_setplane,
 	.disable_plane = intel_primary_plane_disable,
 	.destroy = intel_plane_destroy,
+	.set_property = intel_primary_plane_set_property
 };
 
 static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -11335,6 +11405,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 {
 	struct intel_plane *primary;
 	const uint32_t *intel_primary_formats;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	int num_formats;
 
 	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
@@ -11345,6 +11416,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->max_downscale = 1;
 	primary->pipe = pipe;
 	primary->plane = pipe;
+	primary->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
@@ -11360,6 +11432,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 				 &intel_primary_plane_funcs,
 				 intel_primary_formats, num_formats,
 				 DRM_PLANE_TYPE_PRIMARY);
+	if (INTEL_INFO(dev)->gen >= 4) {
+		if (!primary->base.rotation_property)
+			primary->base.rotation_property =
+				drm_mode_create_rotation_property(dev,
+								BIT(DRM_ROTATE_0) |
+								BIT(DRM_ROTATE_180));
+		if (primary->base.rotation_property)
+			drm_object_attach_property(&primary->base.base,
+						primary->base.rotation_property,
+						primary->rotation);
+	}
+
 	return &primary->base;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..cabccfb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -562,6 +562,14 @@ void intel_update_fbc(struct drm_device *dev)
 		goto out_disable;
 	}
 
+	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+	    to_intel_plane(crtc->primary)->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 the kernel debugger is active, always disable compression */
 	if (in_dbg_master())
 		goto out_disable;
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[Damien Lespiau]

On Mon, Jun 23, 2014 at 11:06:00AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@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.
> 
> v5: Resetting property in i915_driver_lastclose properly for planes
> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
> width in i9xx_update_plane and ironlake_update_plane. Removed tab
> based indentation and unnecessary braces in intel_crtc_set_property
> and intel_update_fbc. FBC and flip related checks should be done only
> for valid crtcs.
> 
> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
> and positioning the disable code in intel_update_fbc.
> 
> v7: In case rotation property on inactive crtc is updated, we return
> successfully printing debug log as crtc is inactive and only property change
> is preserved.
> 
> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
> crtc->primary->fb  and return value of update_primary_plane is ignored.
> 
> v9: added rotation property to primary plane instead of crtc. Removing reset
> of rotation property from lastclose. rotation_property is moved to drm_plane,so
> drm layer will take care of resetting.
> 
> Testcase: igt/kms_rotation_crc
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.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>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |    5 --
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>  4 files changed, 98 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5e583a1..4b6b911 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1938,14 +1938,9 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>   */
>  void i915_driver_lastclose(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
>  	/* 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
>  	 * up anything. */
> -	if (!dev_priv)
> -		return;
> -

Hum, I don't think this was supposed to be removed?

-- 
Damien

Re: [PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[Jindal, Sonika]

On 6/24/2014 3:44 PM, Damien Lespiau wrote:
> On Mon, Jun 23, 2014 at 11:06:00AM +0530, sonika.jindal@intel.com wrote:
>> From: Sonika Jindal <sonika.jindal@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.
>>
>> v5: Resetting property in i915_driver_lastclose properly for planes
>> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
>> width in i9xx_update_plane and ironlake_update_plane. Removed tab
>> based indentation and unnecessary braces in intel_crtc_set_property
>> and intel_update_fbc. FBC and flip related checks should be done only
>> for valid crtcs.
>>
>> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
>> and positioning the disable code in intel_update_fbc.
>>
>> v7: In case rotation property on inactive crtc is updated, we return
>> successfully printing debug log as crtc is inactive and only property change
>> is preserved.
>>
>> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
>> crtc->primary->fb  and return value of update_primary_plane is ignored.
>>
>> v9: added rotation property to primary plane instead of crtc. Removing reset
>> of rotation property from lastclose. rotation_property is moved to drm_plane,so
>> drm layer will take care of resetting.
>>
>> Testcase: igt/kms_rotation_crc
>>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: dri-devel@lists.freedesktop.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>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c      |    5 --
>>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>   drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>>   4 files changed, 98 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 5e583a1..4b6b911 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1938,14 +1938,9 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>>    */
>>   void i915_driver_lastclose(struct drm_device *dev)
>>   {
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -
>>   	/* 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
>>   	 * up anything. */
>> -	if (!dev_priv)
>> -		return;
>> -
>
> Hum, I don't think this was supposed to be removed?
>
Yes, my mistake.. I will revert it and resend the patch.

Re: [PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[Damien Lespiau]

On Mon, Jun 23, 2014 at 11:06:00AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@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.
> 
> v5: Resetting property in i915_driver_lastclose properly for planes
> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
> width in i9xx_update_plane and ironlake_update_plane. Removed tab
> based indentation and unnecessary braces in intel_crtc_set_property
> and intel_update_fbc. FBC and flip related checks should be done only
> for valid crtcs.
> 
> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
> and positioning the disable code in intel_update_fbc.
> 
> v7: In case rotation property on inactive crtc is updated, we return
> successfully printing debug log as crtc is inactive and only property change
> is preserved.
> 
> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
> crtc->primary->fb  and return value of update_primary_plane is ignored.
> 
> v9: added rotation property to primary plane instead of crtc. Removing reset
> of rotation property from lastclose. rotation_property is moved to drm_plane,so
> drm layer will take care of resetting.
> 
> Testcase: igt/kms_rotation_crc
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.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>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |    5 --
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>  4 files changed, 98 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5e583a1..4b6b911 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1938,14 +1938,9 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>   */
>  void i915_driver_lastclose(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
>  	/* 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
>  	 * up anything. */
> -	if (!dev_priv)
> -		return;
> -
>  	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 c70c804..c600d3b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4087,6 +4087,7 @@ enum punit_power_well {
>  #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				0x70184
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e8e711..85bd3b8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
> @@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	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) {
> @@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> +	if (to_intel_plane(crtc->primary)->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 - 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]);
> @@ -2487,7 +2500,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
>  		I915_WRITE(DSPLINOFF(plane), linear_offset);
>  	} else
> -		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + linear_offset);
> +		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) +
> +				linear_offset);
>  	POSTING_READ(reg);
>  }
>  
> @@ -2504,7 +2518,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2512,6 +2528,8 @@ static void ironlake_update_primary_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;
> @@ -2549,8 +2567,6 @@ static void ironlake_update_primary_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,
> @@ -2558,6 +2574,20 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  					       fb->pitches[0]);
>  	linear_offset -= intel_crtc->dspaddr_offset;
>  
> +	if (to_intel_plane(crtc->primary)->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 - 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]);
> @@ -11324,10 +11354,50 @@ static void intel_plane_destroy(struct drm_plane *plane)
>  	kfree(intel_plane);
>  }
>  
> +static int intel_primary_plane_set_property(struct drm_plane *plane,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
> +	struct drm_crtc *crtc = &intel_crtc->base;
> +	uint64_t old_val;
> +
> +	if (prop == plane->rotation_property) {
> +		/* exactly one rotation angle please */
> +		if (hweight32(val & 0xf) != 1)
> +			return -EINVAL;
> +
> +		old_val = intel_plane->rotation;
> +		intel_plane->rotation = val;
> +
> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
> +			intel_crtc_wait_for_pending_flips(crtc);
> +
> +			/* FBC does not work on some platforms for rotated planes */
> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
> +				intel_disable_fbc(dev);
> +			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
> +						0, 0);
> +
> +		} else {
> +			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
> +					crtc->base.id);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct drm_plane_funcs intel_primary_plane_funcs = {
>  	.update_plane = intel_primary_plane_setplane,
>  	.disable_plane = intel_primary_plane_disable,
>  	.destroy = intel_plane_destroy,
> +	.set_property = intel_primary_plane_set_property
>  };
>  
>  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> @@ -11335,6 +11405,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  {
>  	struct intel_plane *primary;
>  	const uint32_t *intel_primary_formats;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int num_formats;

Here and elsewhere, you're adding dev_priv to remove it in the next
patch. A left over from putting the rotation property in drm_plane.

-- 
Damien

Re: [PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[Jindal, Sonika]

On 6/24/2014 3:59 PM, Damien Lespiau wrote:
> On Mon, Jun 23, 2014 at 11:06:00AM +0530, sonika.jindal@intel.com wrote:
>> From: Sonika Jindal <sonika.jindal@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.
>>
>> v5: Resetting property in i915_driver_lastclose properly for planes
>> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
>> width in i9xx_update_plane and ironlake_update_plane. Removed tab
>> based indentation and unnecessary braces in intel_crtc_set_property
>> and intel_update_fbc. FBC and flip related checks should be done only
>> for valid crtcs.
>>
>> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
>> and positioning the disable code in intel_update_fbc.
>>
>> v7: In case rotation property on inactive crtc is updated, we return
>> successfully printing debug log as crtc is inactive and only property change
>> is preserved.
>>
>> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
>> crtc->primary->fb  and return value of update_primary_plane is ignored.
>>
>> v9: added rotation property to primary plane instead of crtc. Removing reset
>> of rotation property from lastclose. rotation_property is moved to drm_plane,so
>> drm layer will take care of resetting.
>>
>> Testcase: igt/kms_rotation_crc
>>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: dri-devel@lists.freedesktop.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>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c      |    5 --
>>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>   drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>>   4 files changed, 98 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 5e583a1..4b6b911 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1938,14 +1938,9 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
>>    */
>>   void i915_driver_lastclose(struct drm_device *dev)
>>   {
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -
>>   	/* 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
>>   	 * up anything. */
>> -	if (!dev_priv)
>> -		return;
>> -
>>   	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 c70c804..c600d3b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4087,6 +4087,7 @@ enum punit_power_well {
>>   #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				0x70184
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 5e8e711..85bd3b8 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   	unsigned long linear_offset;
>>   	u32 dspcntr;
>>   	u32 reg;
>> +	int pixel_size;
>>
>> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>>   	intel_fb = to_intel_framebuffer(fb);
>>   	obj = intel_fb->obj;
>>
>> @@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
>> @@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   	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) {
>> @@ -2477,6 +2479,17 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   		intel_crtc->dspaddr_offset = linear_offset;
>>   	}
>>
>> +	if (to_intel_plane(crtc->primary)->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 - 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]);
>> @@ -2487,7 +2500,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>>   		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
>>   		I915_WRITE(DSPLINOFF(plane), linear_offset);
>>   	} else
>> -		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + linear_offset);
>> +		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) +
>> +				linear_offset);
>>   	POSTING_READ(reg);
>>   }
>>
>> @@ -2504,7 +2518,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>>   	unsigned long linear_offset;
>>   	u32 dspcntr;
>>   	u32 reg;
>> +	int pixel_size;
>>
>> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>>   	intel_fb = to_intel_framebuffer(fb);
>>   	obj = intel_fb->obj;
>>
>> @@ -2512,6 +2528,8 @@ static void ironlake_update_primary_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;
>> @@ -2549,8 +2567,6 @@ static void ironlake_update_primary_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,
>> @@ -2558,6 +2574,20 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>>   					       fb->pitches[0]);
>>   	linear_offset -= intel_crtc->dspaddr_offset;
>>
>> +	if (to_intel_plane(crtc->primary)->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 - 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]);
>> @@ -11324,10 +11354,50 @@ static void intel_plane_destroy(struct drm_plane *plane)
>>   	kfree(intel_plane);
>>   }
>>
>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>> +				    struct drm_property *prop,
>> +				    uint64_t val)
>> +{
>> +	struct drm_device *dev = plane->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>> +	struct drm_crtc *crtc = &intel_crtc->base;
>> +	uint64_t old_val;
>> +
>> +	if (prop == plane->rotation_property) {
>> +		/* exactly one rotation angle please */
>> +		if (hweight32(val & 0xf) != 1)
>> +			return -EINVAL;
>> +
>> +		old_val = intel_plane->rotation;
>> +		intel_plane->rotation = val;
>> +
>> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
>> +			intel_crtc_wait_for_pending_flips(crtc);
>> +
>> +			/* FBC does not work on some platforms for rotated planes */
>> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
>> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
>> +				intel_disable_fbc(dev);
>> +			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
>> +						0, 0);
>> +
>> +		} else {
>> +			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
>> +					crtc->base.id);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static const struct drm_plane_funcs intel_primary_plane_funcs = {
>>   	.update_plane = intel_primary_plane_setplane,
>>   	.disable_plane = intel_primary_plane_disable,
>>   	.destroy = intel_plane_destroy,
>> +	.set_property = intel_primary_plane_set_property
>>   };
>>
>>   static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>> @@ -11335,6 +11405,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>>   {
>>   	struct intel_plane *primary;
>>   	const uint32_t *intel_primary_formats;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	int num_formats;
>
> Here and elsewhere, you're adding dev_priv to remove it in the next
> patch. A left over from putting the rotation property in drm_plane.
>
Yes..I will remove it from here and resend the patches.

[PATCH 3/3] drm: Resetting rotation property

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

Reset rotation property to 0 wherever applicable

v2: Also calling set_property of the plane to set the rotation in the plane
structure. Removed few unused variables.

Cc: damien.lespiau@intel.com
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c      |   14 +++++++++++++-
 drivers/gpu/drm/i915/intel_display.c |    1 -
 drivers/gpu/drm/i915/intel_sprite.c  |    2 --
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d5d8cea..91e611a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -282,9 +282,21 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+
+		if (plane->rotation_property) {
+			int ret = 0;
+			if (plane->funcs->set_property)
+				ret = plane->funcs->set_property(plane,
+					plane->rotation_property, BIT(DRM_ROTATE_0));
+			if (!ret)
+				drm_object_property_set_value(&plane->base,
+					plane->rotation_property, BIT(DRM_ROTATE_0));
+		}
+
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
+	}
 
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 85bd3b8..2006e91 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11405,7 +11405,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 {
 	struct intel_plane *primary;
 	const uint32_t *intel_primary_formats;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int num_formats;
 
 	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index aa94f67..aa63027 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1206,7 +1206,6 @@ 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;
@@ -1289,7 +1288,6 @@ 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;
-- 
1.7.10.4

Re: [PATCH 3/3] drm: Resetting rotation property

[Damien Lespiau]

On Mon, Jun 23, 2014 at 11:06:01AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> Reset rotation property to 0 wherever applicable
> 
> v2: Also calling set_property of the plane to set the rotation in the plane
> structure. Removed few unused variables.
> 
> Cc: damien.lespiau@intel.com
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c      |   14 +++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c |    1 -
>  drivers/gpu/drm/i915/intel_sprite.c  |    2 --
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d5d8cea..91e611a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -282,9 +282,21 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  
>  	drm_warn_on_modeset_not_all_locked(dev);
>  
> -	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
> +	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> +
> +		if (plane->rotation_property) {
> +			int ret = 0;
> +			if (plane->funcs->set_property)
> +				ret = plane->funcs->set_property(plane,
> +					plane->rotation_property, BIT(DRM_ROTATE_0));
> +			if (!ret)
> +				drm_object_property_set_value(&plane->base,
> +					plane->rotation_property, BIT(DRM_ROTATE_0));
> +		}
> +
>  		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>  			drm_plane_force_disable(plane);
> +	}
>  
>  	for (i = 0; i < fb_helper->crtc_count; i++) {
>  		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 85bd3b8..2006e91 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11405,7 +11405,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  {
>  	struct intel_plane *primary;
>  	const uint32_t *intel_primary_formats;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int num_formats;
>  
>  	primary = kzalloc(sizeof(*primary), GFP_KERNEL);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index aa94f67..aa63027 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1206,7 +1206,6 @@ 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;
> @@ -1289,7 +1288,6 @@ 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;

The removal of dev_priv doesn't belong to this patch.

-- 
Damien

[PATCH 0/3] Moving rotation_property to drm_plane

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

As suggested by Daniel and Damien, moved rotation_property to drm_plane.
Also moved resetting of rotation_property to restore_fbdev_mode which will be
used in switching VT use case along with the driver lastclose path.

v2: Removing unused dev_priv from second patch instead of third (some mixup
due to earlier reformatting).

Sonika Jindal (2):
  drm/i915: Add 180 degree primary plane rotation support
  drm: Resetting rotation property

Ville Syrjälä (1):
  drm/i915: Add rotation property for sprites

 drivers/gpu/drm/drm_fb_helper.c      |   14 ++++-
 drivers/gpu/drm/i915/i915_dma.c      |    5 --
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   93 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      |    8 +++
 drivers/gpu/drm/i915/intel_sprite.c  |   40 ++++++++++++++-
 include/drm/drm_crtc.h               |    1 +
 7 files changed, 150 insertions(+), 12 deletions(-)

-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Add rotation property for sprites

[sonika.jindal]

From: Ville Syrjälä <ville.syrjala at 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.

v2: Moving rotation_property to drm_plane

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c |   40 ++++++++++++++++++++++++++++++++++-
 include/drm/drm_crtc.h              |    1 +
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cbad738..aa63027 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1202,6 +1202,29 @@ out_unlock:
 	return ret;
 }
 
+static int intel_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	uint64_t old_val;
+	int ret = -ENOENT;
+
+	if (prop == plane->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);
@@ -1228,6 +1251,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[] = {
@@ -1338,8 +1362,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 (!intel_plane->base.rotation_property)
+		intel_plane->base.rotation_property =
+			drm_mode_create_rotation_property(dev,
+							  BIT(DRM_ROTATE_0) |
+							  BIT(DRM_ROTATE_180));
+
+	if (intel_plane->base.rotation_property)
+		drm_object_attach_property(&intel_plane->base.base,
+					   intel_plane->base.rotation_property,
+					   intel_plane->rotation);
 
+ out:
 	return ret;
 }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 08ed55e..6006c70 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -615,6 +615,7 @@ struct drm_plane {
 
 	const struct drm_plane_funcs *funcs;
 
+	struct drm_property *rotation_property;
 	struct drm_object_properties properties;
 
 	enum drm_plane_type type;
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Add 180 degree primary plane rotation support

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@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.

v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.

v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.

v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.

v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb  and return value of update_primary_plane is ignored.

v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to drm_plane,so
drm layer will take care of resetting.

Testcase: igt/kms_rotation_crc

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   94 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      |    7 +++
 3 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c70c804..c600d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4087,6 +4087,7 @@ enum punit_power_well {
 #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				0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..6d87e3b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
@@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	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) {
@@ -2477,6 +2479,18 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -2487,7 +2501,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else
-		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + linear_offset);
+		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) +
+				linear_offset);
 	POSTING_READ(reg);
 }
 
@@ -2504,7 +2519,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2512,6 +2529,8 @@ static void ironlake_update_primary_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;
@@ -2549,8 +2568,6 @@ static void ironlake_update_primary_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,
@@ -2558,6 +2575,21 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -11324,10 +11356,49 @@ static void intel_plane_destroy(struct drm_plane *plane)
 	kfree(intel_plane);
 }
 
+static int intel_primary_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
+	struct drm_crtc *crtc = &intel_crtc->base;
+
+	if (prop == plane->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		intel_plane->rotation = val;
+
+		if (intel_crtc->active && intel_crtc->primary_enabled) {
+			intel_crtc_wait_for_pending_flips(crtc);
+
+		/* FBC does not work on some platforms for rotated planes */
+			if (dev_priv->fbc.plane == intel_crtc->plane &&
+			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			    intel_plane->rotation != BIT(DRM_ROTATE_0))
+				intel_disable_fbc(dev);
+
+			dev_priv->display.update_primary_plane(crtc,
+				crtc->primary->fb, 0, 0);
+
+		} else {
+			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation"
+				"property is updated\n", crtc->base.id);
+		}
+	}
+
+	return 0;
+}
+
 static const struct drm_plane_funcs intel_primary_plane_funcs = {
 	.update_plane = intel_primary_plane_setplane,
 	.disable_plane = intel_primary_plane_disable,
 	.destroy = intel_plane_destroy,
+	.set_property = intel_primary_plane_set_property
 };
 
 static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -11345,6 +11416,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->max_downscale = 1;
 	primary->pipe = pipe;
 	primary->plane = pipe;
+	primary->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
@@ -11360,6 +11432,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 				 &intel_primary_plane_funcs,
 				 intel_primary_formats, num_formats,
 				 DRM_PLANE_TYPE_PRIMARY);
+	if (INTEL_INFO(dev)->gen >= 4) {
+		if (!primary->base.rotation_property)
+			primary->base.rotation_property =
+				drm_mode_create_rotation_property(dev,
+							BIT(DRM_ROTATE_0) |
+							BIT(DRM_ROTATE_180));
+		if (primary->base.rotation_property)
+			drm_object_attach_property(&primary->base.base,
+						primary->base.rotation_property,
+						primary->rotation);
+	}
+
 	return &primary->base;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..bd6af91 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -562,6 +562,13 @@ void intel_update_fbc(struct drm_device *dev)
 		goto out_disable;
 	}
 
+	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+	    to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
+		goto out_disable;
+	}
+
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master())
 		goto out_disable;
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@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.

v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.

v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.

v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.

v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb  and return value of update_primary_plane is ignored.

v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to drm_plane,so
drm layer will take care of resetting.

v10: adding updation of fbc when rotation is set to 0.

Testcase: igt/kms_rotation_crc

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: dri-devel@lists.freedesktop.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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |  102 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      |    7 +++
 3 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c70c804..c600d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4087,6 +4087,7 @@ enum punit_power_well {
 #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				0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..7f16d6f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2414,7 +2414,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2422,6 +2424,8 @@ static void i9xx_update_primary_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;
@@ -2463,8 +2467,6 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	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) {
@@ -2477,6 +2479,18 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -2487,7 +2501,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else
-		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) + linear_offset);
+		I915_WRITE(DSPADDR(plane), i915_gem_obj_ggtt_offset(obj) +
+				linear_offset);
 	POSTING_READ(reg);
 }
 
@@ -2504,7 +2519,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2512,6 +2529,8 @@ static void ironlake_update_primary_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;
@@ -2549,8 +2568,6 @@ static void ironlake_update_primary_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,
@@ -2558,6 +2575,21 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
+	if (to_intel_plane(crtc->primary)->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 - 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]);
@@ -11324,10 +11356,57 @@ static void intel_plane_destroy(struct drm_plane *plane)
 	kfree(intel_plane);
 }
 
+static int intel_primary_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
+	struct drm_crtc *crtc = &intel_crtc->base;
+	uint64_t old_val;
+
+	if (prop == plane->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		old_val = intel_plane->rotation;
+		intel_plane->rotation = val;
+
+		if (intel_crtc->active && intel_crtc->primary_enabled) {
+			intel_crtc_wait_for_pending_flips(crtc);
+
+		/* FBC does not work on some platforms for rotated planes */
+			if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev)) {
+				if (dev_priv->fbc.plane == intel_crtc->plane &&
+					intel_plane->rotation != BIT(DRM_ROTATE_0))
+					intel_disable_fbc(dev);
+				/* If rotation was set earlier and new rotation is 0, we might
+				 * have disabled fbc earlier. So update it now */
+				else if (intel_plane->rotation == BIT(DRM_ROTATE_0) &&
+					old_val != BIT(DRM_ROTATE_0))
+					intel_update_fbc(dev);
+			}
+
+			dev_priv->display.update_primary_plane(crtc,
+				crtc->primary->fb, 0, 0);
+
+		} else {
+			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation"
+				"property is updated\n", crtc->base.id);
+		}
+	}
+
+	return 0;
+}
+
 static const struct drm_plane_funcs intel_primary_plane_funcs = {
 	.update_plane = intel_primary_plane_setplane,
 	.disable_plane = intel_primary_plane_disable,
 	.destroy = intel_plane_destroy,
+	.set_property = intel_primary_plane_set_property
 };
 
 static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
@@ -11345,6 +11424,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->max_downscale = 1;
 	primary->pipe = pipe;
 	primary->plane = pipe;
+	primary->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
@@ -11360,6 +11440,18 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 				 &intel_primary_plane_funcs,
 				 intel_primary_formats, num_formats,
 				 DRM_PLANE_TYPE_PRIMARY);
+	if (INTEL_INFO(dev)->gen >= 4) {
+		if (!primary->base.rotation_property)
+			primary->base.rotation_property =
+				drm_mode_create_rotation_property(dev,
+							BIT(DRM_ROTATE_0) |
+							BIT(DRM_ROTATE_180));
+		if (primary->base.rotation_property)
+			drm_object_attach_property(&primary->base.base,
+						primary->base.rotation_property,
+						primary->rotation);
+	}
+
 	return &primary->base;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..bd6af91 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -562,6 +562,13 @@ void intel_update_fbc(struct drm_device *dev)
 		goto out_disable;
 	}
 
+	if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+	    to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+			DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
+		goto out_disable;
+	}
+
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master())
 		goto out_disable;
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm: Resetting rotation property

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

Reset rotation property to 0 wherever applicable

v2: Also calling set_property of the plane to set the rotation in the plane
structure.

Cc: damien.lespiau@intel.com
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d5d8cea..30806b4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -282,9 +282,23 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
+
+		if (plane->rotation_property) {
+			int ret = 0;
+			if (plane->funcs->set_property)
+				ret = plane->funcs->set_property(plane,
+					plane->rotation_property,
+					BIT(DRM_ROTATE_0));
+			if (!ret)
+				drm_object_property_set_value(&plane->base,
+					plane->rotation_property,
+					BIT(DRM_ROTATE_0));
+		}
+
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
+	}
 
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
-- 
1.7.10.4

Re: [PATCH 0/3] Moving rotation_property to drm_plane

[Jindal, Sonika]

Hi,

Did anybody get a chance to review these patches?

Thanks,
Sonika

On 6/24/2014 5:38 PM, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
>
> As suggested by Daniel and Damien, moved rotation_property to drm_plane.
> Also moved resetting of rotation_property to restore_fbdev_mode which will be
> used in switching VT use case along with the driver lastclose path.
>
> v2: Removing unused dev_priv from second patch instead of third (some mixup
> due to earlier reformatting).
>
> Sonika Jindal (2):
>    drm/i915: Add 180 degree primary plane rotation support
>    drm: Resetting rotation property
>
> Ville Syrjälä (1):
>    drm/i915: Add rotation property for sprites
>
>   drivers/gpu/drm/drm_fb_helper.c      |   14 ++++-
>   drivers/gpu/drm/i915/i915_dma.c      |    5 --
>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>   drivers/gpu/drm/i915/intel_display.c |   93 ++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/i915/intel_pm.c      |    8 +++
>   drivers/gpu/drm/i915/intel_sprite.c  |   40 ++++++++++++++-
>   include/drm/drm_crtc.h               |    1 +
>   7 files changed, 150 insertions(+), 12 deletions(-)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] Moving rotation_property to drm_plane

[Damien Lespiau]

On Wed, Jul 02, 2014 at 02:21:52PM +0530, Jindal, Sonika wrote:
> Hi,
> 
> Did anybody get a chance to review these patches?

Ok, now those patches are all over the place. It's pretty much
impossible to have the big picture again with the latest changes (the
rotation on drm_plane) nor the limit of the the series, esp. as the
documentation patches have been split off.

I'm not sure why, but it seems like the Cc: tags in your emails have
never actually ended up in the Cc: header of your mails. Which means
they never hit dri-devel. Can you make sure you fix that for the next
resend. This means we haven't gathered their feedback during all this
time...

What you can do: Use --dry-run to make sure you're sending them the
dri-devel. Maybe use the --cc command line option instead?

Also, as already mentioned, don't send the patches to the LKML, noone
will review them there.

Could you resend the full series with the reviewed-by tags gathered so
far? with dri-devel in copy? in a separarte mail thread? We usually
version the big resend of series in the cover letter, explaning the
changes.

Thanks,

-- 
Damien

Re: [PATCH 0/3] Moving rotation_property to drm_plane

[Daniel Vetter]

On Wed, Jul 02, 2014 at 02:17:00PM +0100, Damien Lespiau wrote:
> On Wed, Jul 02, 2014 at 02:21:52PM +0530, Jindal, Sonika wrote:
> > Hi,
> > 
> > Did anybody get a chance to review these patches?
> 
> Ok, now those patches are all over the place. It's pretty much
> impossible to have the big picture again with the latest changes (the
> rotation on drm_plane) nor the limit of the the series, esp. as the
> documentation patches have been split off.
> 
> I'm not sure why, but it seems like the Cc: tags in your emails have
> never actually ended up in the Cc: header of your mails. Which means
> they never hit dri-devel. Can you make sure you fix that for the next
> resend. This means we haven't gathered their feedback during all this
> time...
> 
> What you can do: Use --dry-run to make sure you're sending them the
> dri-devel. Maybe use the --cc command line option instead?
> 
> Also, as already mentioned, don't send the patches to the LKML, noone
> will review them there.
> 
> Could you resend the full series with the reviewed-by tags gathered so
> far? with dri-devel in copy? in a separarte mail thread? We usually
> version the big resend of series in the cover letter, explaning the
> changes.

BKM for resending patches: When you resend the full series or large parts
of it, start a new thread with _all_ the patches. But don't do that before
a few days have passed to make sure that the review won't get get split
between two threads.

If you have some small fixups and want to get quick feedback just resubmit
_individual_ patches in-reply-to the previous version. Yes that's quite a
bit of work since you need to send out each patch individually with the
right --in-reply-to parameters.

Following these bkms will make sure that the discussion stays together as
much as possible and that everyone can find the latest patch versions even
in a really busy review thread. Resending only part of patch series and
then in-reply-to somewehere deep in an existing thread as on group will
make a mess.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Damien Lespiau]

On Thu, Jun 19, 2014 at 01:22:25PM +0530, Jindal, Sonika wrote:
> >>I see that in omap driver too it is done in lastclose of the driver. Also,
> >>from driver fbdev_restore is only called during lastclose.
> >>Again I don't have more knowledge on this.
> >>Can we keep it here in this lastclose function to comply with omap driver?

Just to be thorough, there's an important path that is not from
lastclose():

  + drm_fb_helper_set_par()
    + drm_fb_helper_restore_fbdev_mode_unlocked()
      + restore_fbdev_mode()

-- 
Damien

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

[Daniel Vetter]

On Thu, Jun 19, 2014 at 12:07 PM, Damien Lespiau
<damien.lespiau@intel.com> wrote:
> On Thu, Jun 19, 2014 at 01:22:25PM +0530, Jindal, Sonika wrote:
>> >>I see that in omap driver too it is done in lastclose of the driver. Also,
>> >>from driver fbdev_restore is only called during lastclose.
>> >>Again I don't have more knowledge on this.
>> >>Can we keep it here in this lastclose function to comply with omap driver?
>
> Just to be thorough, there's an important path that is not from
> lastclose():
>
>   + drm_fb_helper_set_par()
>     + drm_fb_helper_restore_fbdev_mode_unlocked()
>       + restore_fbdev_mode()

Yeah, that's the vt restore thing when leaving X/wayland. So we want
want this in restore_fbdev_mode even just for i915 - fixing up other
drivers is just a bit a bonus.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

[Tvrtko Ursulin]

Hi,

On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
[snip]
> +static int intel_primary_plane_set_property(struct drm_plane *plane,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
> +	struct drm_crtc *crtc = &intel_crtc->base;
> +	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;
> +
> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
> +			intel_crtc_wait_for_pending_flips(crtc);
> +
> +			/* FBC does not work on some platforms for rotated planes */
> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
> +				intel_disable_fbc(dev);
> +
> +			dev_priv->display.update_primary_plane(crtc, crtc->primary->fb, 0, 0);
> +		} else {
> +			DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation property is updated\n",
> +					crtc->base.id);
> +			ret = 0;
> +		}
> +	}
> +
> +	return ret;
> +}

It looks like this will incorrectly propagate -ENOENT if property on an 
active plane is modified.

Regards,

Tvrtko

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

[Jindal, Sonika]

On 6/27/2014 4:04 PM, Tvrtko Ursulin wrote:
> Hi,
>
> On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
> [snip]
>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>> +                    struct drm_property *prop,
>> +                    uint64_t val)
>> +{
>> +    struct drm_device *dev = plane->dev;
>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>> +    struct intel_plane *intel_plane = to_intel_plane(plane);
>> +    struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>> +    struct drm_crtc *crtc = &intel_crtc->base;
>> +    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;
>> +
>> +        if (intel_crtc->active && intel_crtc->primary_enabled) {
>> +            intel_crtc_wait_for_pending_flips(crtc);
>> +
>> +            /* FBC does not work on some platforms for rotated planes */
>> +            if (dev_priv->fbc.plane == intel_crtc->plane &&
>> +                INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>> +                intel_plane->rotation != BIT(DRM_ROTATE_0))
>> +                intel_disable_fbc(dev);
>> +
>> +            dev_priv->display.update_primary_plane(crtc,
>> crtc->primary->fb, 0, 0);
>> +        } else {
>> +            DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation
>> property is updated\n",
>> +                    crtc->base.id);
>> +            ret = 0;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>
> It looks like this will incorrectly propagate -ENOENT if property on an
> active plane is modified.
>
> Regards,
>
> Tvrtko
>
>
Yes, this was corrected in the next patch set. Can you please refer to 
the latest patches where we moved the property to drm_plane instead of 
intel_plane: 
http://lists.freedesktop.org/archives/intel-gfx/2014-June/047910.html
-Sonika

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

[Tvrtko Ursulin]

On 06/27/2014 11:49 AM, Jindal, Sonika wrote:
>
>
> On 6/27/2014 4:04 PM, Tvrtko Ursulin wrote:
>> Hi,
>>
>> On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
>> [snip]
>>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>>> +                    struct drm_property *prop,
>>> +                    uint64_t val)
>>> +{
>>> +    struct drm_device *dev = plane->dev;
>>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>>> +    struct intel_plane *intel_plane = to_intel_plane(plane);
>>> +    struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>>> +    struct drm_crtc *crtc = &intel_crtc->base;
>>> +    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;
>>> +
>>> +        if (intel_crtc->active && intel_crtc->primary_enabled) {
>>> +            intel_crtc_wait_for_pending_flips(crtc);
>>> +
>>> +            /* FBC does not work on some platforms for rotated
>>> planes */
>>> +            if (dev_priv->fbc.plane == intel_crtc->plane &&
>>> +                INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>>> +                intel_plane->rotation != BIT(DRM_ROTATE_0))
>>> +                intel_disable_fbc(dev);
>>> +
>>> +            dev_priv->display.update_primary_plane(crtc,
>>> crtc->primary->fb, 0, 0);
>>> +        } else {
>>> +            DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation
>>> property is updated\n",
>>> +                    crtc->base.id);
>>> +            ret = 0;
>>> +        }
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>
>> It looks like this will incorrectly propagate -ENOENT if property on an
>> active plane is modified.
>>
>> Regards,
>>
>> Tvrtko
>>
>>
> Yes, this was corrected in the next patch set. Can you please refer to
> the latest patches where we moved the property to drm_plane instead of
> intel_plane:
> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047910.html

Alright, I missed that series since it is bit indented in the thread. 
Does it replace only patch 10 from the original series? Or in other 
words first nine should be applied first, then these three?

Tvrtko

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

[Jindal, Sonika]

On 6/27/2014 4:42 PM, Tvrtko Ursulin wrote:
>
> On 06/27/2014 11:49 AM, Jindal, Sonika wrote:
>>
>>
>> On 6/27/2014 4:04 PM, Tvrtko Ursulin wrote:
>>> Hi,
>>>
>>> On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
>>> [snip]
>>>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>>>> +                    struct drm_property *prop,
>>>> +                    uint64_t val)
>>>> +{
>>>> +    struct drm_device *dev = plane->dev;
>>>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>>>> +    struct intel_plane *intel_plane = to_intel_plane(plane);
>>>> +    struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>>>> +    struct drm_crtc *crtc = &intel_crtc->base;
>>>> +    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;
>>>> +
>>>> +        if (intel_crtc->active && intel_crtc->primary_enabled) {
>>>> +            intel_crtc_wait_for_pending_flips(crtc);
>>>> +
>>>> +            /* FBC does not work on some platforms for rotated
>>>> planes */
>>>> +            if (dev_priv->fbc.plane == intel_crtc->plane &&
>>>> +                INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>>>> +                intel_plane->rotation != BIT(DRM_ROTATE_0))
>>>> +                intel_disable_fbc(dev);
>>>> +
>>>> +            dev_priv->display.update_primary_plane(crtc,
>>>> crtc->primary->fb, 0, 0);
>>>> +        } else {
>>>> +            DRM_DEBUG_KMS("[CRTC:%d] is not active. Only rotation
>>>> property is updated\n",
>>>> +                    crtc->base.id);
>>>> +            ret = 0;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>
>>> It looks like this will incorrectly propagate -ENOENT if property on an
>>> active plane is modified.
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>
>> Yes, this was corrected in the next patch set. Can you please refer to
>> the latest patches where we moved the property to drm_plane instead of
>> intel_plane:
>> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047910.html
>
> Alright, I missed that series since it is bit indented in the thread.
> Does it replace only patch 10 from the original series? Or in other
> words first nine should be applied first, then these three?
>
> Tvrtko
>
>
>
It replaces last two patches in the series, rotation property for 
sprites as well as primary planes.
-Sonika

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

[Tvrtko Ursulin]

Hi,

On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
[snip]
> +static int intel_primary_plane_set_property(struct drm_plane *plane,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
> +	struct drm_crtc *crtc = &intel_crtc->base;
> +	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;
> +
> +		if (intel_crtc->active && intel_crtc->primary_enabled) {
> +			intel_crtc_wait_for_pending_flips(crtc);
> +
> +			/* FBC does not work on some platforms for rotated planes */
> +			if (dev_priv->fbc.plane == intel_crtc->plane &&
> +			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> +			    intel_plane->rotation != BIT(DRM_ROTATE_0))
> +				intel_disable_fbc(dev);

Also, do we need a path for turning FBC back on once plane orientation 
goes back to a supported configuration?

Regards,

Tvrtko

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

[Jindal, Sonika]

On 6/27/2014 4:08 PM, Tvrtko Ursulin wrote:
> Hi,
>
> On 06/18/2014 09:57 AM, sonika.jindal@intel.com wrote:
> [snip]
>> +static int intel_primary_plane_set_property(struct drm_plane *plane,
>> +                    struct drm_property *prop,
>> +                    uint64_t val)
>> +{
>> +    struct drm_device *dev = plane->dev;
>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>> +    struct intel_plane *intel_plane = to_intel_plane(plane);
>> +    struct intel_crtc *intel_crtc = to_intel_crtc(plane->crtc);
>> +    struct drm_crtc *crtc = &intel_crtc->base;
>> +    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;
>> +
>> +        if (intel_crtc->active && intel_crtc->primary_enabled) {
>> +            intel_crtc_wait_for_pending_flips(crtc);
>> +
>> +            /* FBC does not work on some platforms for rotated planes */
>> +            if (dev_priv->fbc.plane == intel_crtc->plane &&
>> +                INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>> +                intel_plane->rotation != BIT(DRM_ROTATE_0))
>> +                intel_disable_fbc(dev);
>
> Also, do we need a path for turning FBC back on once plane orientation
> goes back to a supported configuration?
>
> Regards,
>
> Tvrtko
>
True, looks like it should be added. I'l add and post.
-Sonika

[PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[sonika.jindal]

From: Sonika Jindal <sonika.jindal@intel.com>

Testcase for 180 degree HW rotation

Cc: sagar.a.kamble@intel.com

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 tests/Makefile.sources   |    1 +
 tests/kms_rotation_crc.c |  427 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 428 insertions(+)
 create mode 100644 tests/kms_rotation_crc.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index e4c23b3..b59c146 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -69,6 +69,7 @@ TESTS_progs_M = \
 	kms_plane \
 	kms_render \
 	kms_setmode \
+	kms_rotation_crc \
 	pm_lpsp \
 	pm_rpm \
 	pm_rps \
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
new file mode 100644
index 0000000..74a52cd
--- /dev/null
+++ b/tests/kms_rotation_crc.c
@@ -0,0 +1,427 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <fcntl.h>
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "igt_core.h"
+
+#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
+#define DRM_ROTATE_NUM	6
+
+#define BIT(x)	(1 << x)
+
+// This will be part of libdrm later. Adding here temporarily
+#define DRM_PLANE_TYPE_OVERLAY 0
+#define DRM_PLANE_TYPE_PRIMARY 1
+#define DRM_PLANE_TYPE_CURSOR  2
+#define DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES 2
+
+typedef struct {
+	int gfx_fd;
+	igt_display_t display;
+	igt_output_t *output;
+	int type;
+	int pipe;
+	struct igt_fb fb;
+	igt_crc_t ref_crc[2];
+	igt_pipe_crc_t *pipe_crc;
+	int rotate;
+} data_t;
+
+bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type);
+int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
+		val, igt_crc_t *crc_output);
+void test_sprite_rotation(data_t *data);
+void test_primary_rotation(data_t *data);
+bool prepare_crtc(data_t *data);
+
+bool prepare_crtc(data_t *data)
+{
+	drmModeModeInfo *mode;
+	igt_display_t *display = &data->display;
+	igt_output_t *output = data->output;
+	cairo_t *cr;
+	igt_plane_t *primary, *sprite;
+	int fb_id;
+	int w, h;
+
+	igt_output_set_pipe(output, data->pipe);
+	igt_display_commit(display);
+
+	/* create the pipe_crc object for this pipe */
+	if (data->pipe_crc)
+		igt_pipe_crc_free(data->pipe_crc);
+
+	data->pipe_crc = igt_pipe_crc_new(data->pipe,
+			INTEL_PIPE_CRC_SOURCE_AUTO);
+	if (!data->pipe_crc) {
+		igt_info("auto crc not supported on this connector with pipe %i\n",
+				data->pipe);
+		return false;
+	}
+
+
+	if (!data->output->valid) {
+		igt_output_set_pipe(output, PIPE_ANY);
+		igt_display_commit(display);
+		return false;
+	}
+
+	switch (data->type) {
+
+		case DRM_PLANE_TYPE_OVERLAY: /* Sprite */
+			igt_info("Sprite plane\n");
+			mode = igt_output_get_mode(output);
+			w = mode->hdisplay;
+			h = mode->vdisplay;
+
+			fb_id = igt_create_fb(data->gfx_fd,
+					mode->hdisplay, mode->vdisplay,
+					DRM_FORMAT_XRGB8888,
+					false, /* tiled */
+					&data->fb);
+			igt_assert(fb_id);
+			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+
+			/* Paint rotated image of 4 colors */
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.0, 0.0);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 1.0, 0.0);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 0.0, 1.0);
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 1.0, 1.0);
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			igt_plane_set_fb(sprite, &data->fb);
+			igt_display_commit(display);
+
+			/* Collect reference crc */
+			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[1]);
+
+			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.0, 0.0);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.0, 1.0, 0.0);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.0, 0.0, 1.0);
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 1.0, 1.0);
+			cairo_destroy(cr);
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			igt_plane_set_fb(sprite, &data->fb);
+			igt_display_commit(display);
+
+			break;
+		case DRM_PLANE_TYPE_PRIMARY: /* Primary */
+			igt_info("Primary plane\n");
+			mode = igt_output_get_mode(output);
+			w = mode->hdisplay;
+			h = mode->vdisplay;
+
+			fb_id = igt_create_fb(data->gfx_fd,
+					mode->hdisplay, mode->vdisplay,
+					DRM_FORMAT_XRGB8888,
+					false, /* tiled */
+					&data->fb);
+			igt_assert(fb_id);
+			cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+
+			/* Paint rotated image of 4 colors */
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 1.0, 0.2, 0.2);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 1.0, 0.2);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 0.2, 1.0);
+			igt_paint_color(cr, 0, 0, w/2, h/2, 0.8, 0.8, 0.8);
+			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(primary, &data->fb);
+			igt_display_commit(display);
+
+			/* Collect reference crc */
+			igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc[0]);
+
+			/* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+			igt_paint_color(cr, 0, 0, w/2, h/2, 1.0, 0.2, 0.2);
+			igt_paint_color(cr, (w/2)-1, 0, w/2, h/2, 0.2, 1.0, 0.2);
+			igt_paint_color(cr, 0, (h/2)-1, w/2, h/2, 0.2, 0.2, 1.0);
+			igt_paint_color(cr, (w/2)-1, (h/2)-1, w/2, h/2, 0.8, 0.8, 0.8);
+			cairo_destroy(cr);
+			primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+			igt_plane_set_fb(primary, &data->fb);
+			igt_display_commit(display);
+
+			break;
+
+
+	}
+
+	return true;
+}
+
+static int connector_find_plane(int gfx_fd, uint32_t pipe, uint32_t type)
+{
+	drmModePlaneRes *plane_resources;
+	drmModePlane *ovr;
+	uint32_t id = 0;
+	int i, ret = 0;
+
+	ret = drmSetClientCap(gfx_fd, DRM_CLIENT_CAP_EXPOSE_PRIMARY_PLANES, 1);
+	if (ret < 0) {
+		igt_info("Failed to set client cap:%d\n", ret);
+		return 0;
+	}
+
+	plane_resources = drmModeGetPlaneResources(gfx_fd);
+	if (!plane_resources) {
+		igt_info("drmModeGetPlaneResources failed: %s\n",
+				strerror(errno));
+		return 0;
+	}
+
+	for (i = 0; i < plane_resources->count_planes; i++) {
+		ovr = drmModeGetPlane(gfx_fd, plane_resources->planes[i]);
+		if (!ovr) {
+			igt_info("drmModeGetPlane failed: %s\n",
+					strerror(errno));
+			continue;
+		}
+
+		if (ovr->possible_crtcs & (1 << pipe)) {
+			id = ovr->plane_id;
+			if (check_plane_type(gfx_fd, id, type)) {
+				drmModeFreePlane(ovr);
+				return id;
+			}
+		}
+		drmModeFreePlane(ovr);
+	}
+
+	return 0;
+}
+
+bool check_plane_type(int drm_fd, uint32_t plane_id, uint32_t type)
+{
+	int i = 0;
+	drmModeObjectPropertiesPtr props = NULL;
+
+	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
+
+	for (i = 0; i < props->count_props; i++)
+	{
+		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
+
+		if (strcmp(prop->name, "type") == 0)
+		{
+			if (props->prop_values[i] == type) {
+				return true;
+			}
+			igt_info("Didn't find the requested type:%u\n", (unsigned int)props->prop_values[i]);
+		}
+	}
+	return false;
+}
+
+int set_plane_property(data_t *data, int plane_id, const char *prop_name, int
+		val, igt_crc_t *crc_output)
+{
+	int i = 0, ret = 0;
+	int drm_fd = data->gfx_fd;
+	uint64_t value;
+	drmModeObjectPropertiesPtr props = NULL;
+
+	value = (uint64_t)val;
+	props = drmModeObjectGetProperties(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE);
+
+	for (i = 0; i < props->count_props; i++)
+	{
+		drmModePropertyPtr prop = drmModeGetProperty(drm_fd, props->props[i]);
+		igt_info("\nProp->name=%s: plane_id:%d\n ", prop->name,	plane_id);
+
+		if (strcmp(prop->name, prop_name) == 0)
+		{
+			ret = drmModeObjectSetProperty(drm_fd, plane_id, DRM_MODE_OBJECT_PLANE,
+					(uint32_t)prop->prop_id, value);
+			if (ret)
+			{
+				igt_info("set_property \"%s\" to %d for plane %d is failed,	err:%d\n", prop_name, val, plane_id, ret);
+				drmModeFreeProperty(prop);
+				drmModeFreeObjectProperties(props);
+				return ret;
+			}
+			else {
+				/* Collect crc after rotation */
+				igt_pipe_crc_collect_crc(data->pipe_crc, crc_output);
+				drmModeFreeProperty(prop);
+				break;
+			}
+		}
+		drmModeFreeProperty(prop);
+	}
+	drmModeFreeObjectProperties(props);
+	return 0;
+}
+
+static void cleanup_crtc(data_t *data, igt_output_t *output)
+{
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane = NULL;
+
+	igt_pipe_crc_free(data->pipe_crc);
+	data->pipe_crc = NULL;
+
+	igt_remove_fb(data->gfx_fd, &data->fb);
+
+	if (data->type == DRM_PLANE_TYPE_PRIMARY) {
+		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+	}
+	else if (data->type == DRM_PLANE_TYPE_OVERLAY) {
+		plane = igt_output_get_plane(output, IGT_PLANE_2);
+	}
+
+	if (plane != NULL)
+		igt_plane_set_fb(plane, NULL);
+
+	igt_output_set_pipe(output, PIPE_ANY);
+	igt_display_commit(display);
+}
+
+void test_sprite_rotation(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	igt_plane_t *sprite;
+	igt_crc_t crc_output;
+	int p;
+	int plane_id;
+	int ret;
+	int valid_tests = 0;
+
+	for_each_connected_output(display, output) {
+		data->output = output;
+		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
+			data->pipe = p;
+			data->type = 0;
+			data->rotate = DRM_ROTATE_180;
+
+			if (!prepare_crtc(data))
+				continue;
+			sleep(2);
+
+			sprite = igt_output_get_plane(output, IGT_PLANE_2);
+			plane_id = sprite->drm_plane->plane_id;
+			if (plane_id != 0) {
+				igt_info("Setting rotation property for plane:%d\n", plane_id);
+				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
+				if (ret < 0) {
+					igt_info("Setting rotation failed!");
+					return;
+				}
+			}
+			igt_assert(igt_crc_equal(&data->ref_crc[1], &crc_output));
+			sleep(2);
+			valid_tests++;
+			cleanup_crtc(data, output);
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+
+void test_primary_rotation(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	int p;
+	int plane_id;
+	int ret;
+	int valid_tests = 0;
+	igt_crc_t crc_output;
+
+	for_each_connected_output(display, output) {
+		data->output = output;
+		for (p = 0; p < igt_display_get_n_pipes(display); p++) {
+			data->pipe = p;
+			data->type = 1;
+			data->rotate = DRM_ROTATE_180;
+
+			if (!prepare_crtc(data))
+				continue;
+			sleep(2);
+
+			/* Find primary plane. Currently igt_plane_t returned from
+			 * igt_output_get_plane has NULL drm_plane which is needed to get
+			 * the plane_id. So finding the primary plane by checking the "type"
+			 * property of the plane */
+			plane_id = connector_find_plane(data->gfx_fd, data->pipe, data->type);
+			if (plane_id != 0) {
+				igt_info("Setting rotation property for plane:%d\n", plane_id);
+				ret = set_plane_property(data, plane_id, "rotation", BIT(data->rotate), &crc_output);
+				if (ret < 0) {
+					igt_info("Setting rotation failed!");
+					return;
+				}
+			}
+			igt_assert(igt_crc_equal(&data->ref_crc[0], &crc_output));
+			sleep(2);
+			valid_tests++;
+			cleanup_crtc(data, output);
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+igt_main
+{
+	data_t data = {};
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		data.gfx_fd = drm_open_any();
+
+		igt_set_vt_graphics_mode();
+
+		igt_require_pipe_crc();
+
+		igt_display_init(&data.display, data.gfx_fd);
+	}
+
+	igt_subtest_f("primary-rotation")
+		test_primary_rotation(&data);
+
+	igt_subtest_f("sprite-rotation")
+		test_sprite_rotation(&data);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+	}
+}
-- 
1.7.10.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Damien Lespiau]

On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> Testcase for 180 degree HW rotation
> 
> Cc: sagar.a.kamble@intel.com
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

The test looks good to me (I haven't checked in details, the bar for igt
is quite a bit lower). It shows two gaps in the igt kms API:

  - Retrieving the primary plane (there's a series from Matt fixing this
    and exposing the primary plane through igt_output_get_plane())
  - Adding a set_property() convenience function
    (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
    (no-one is working on that just yet, can de done later)

A small question before pushing this, have you checked that the test
correctly skips when running with a kernel without rotation support?

Thanks,

-- 
Damien

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Chris Wilson]

On Wed, Jun 18, 2014 at 12:32:00PM +0100, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
> > From: Sonika Jindal <sonika.jindal@intel.com>
> > 
> > Testcase for 180 degree HW rotation
> > 
> > Cc: sagar.a.kamble@intel.com
> > 
> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> The test looks good to me (I haven't checked in details, the bar for igt
> is quite a bit lower). It shows two gaps in the igt kms API:
> 
>   - Retrieving the primary plane (there's a series from Matt fixing this
>     and exposing the primary plane through igt_output_get_plane())
>   - Adding a set_property() convenience function
>     (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
>     (no-one is working on that just yet, can de done later)
> 
> A small question before pushing this, have you checked that the test
> correctly skips when running with a kernel without rotation support?

Note: don't push userspace using new ABI until that ABI has been
agreed upon and committed to the kernel.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Jindal, Sonika]

On 6/18/2014 5:09 PM, Chris Wilson wrote:
> On Wed, Jun 18, 2014 at 12:32:00PM +0100, Damien Lespiau wrote:
>> On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
>>> From: Sonika Jindal <sonika.jindal@intel.com>
>>>
>>> Testcase for 180 degree HW rotation
>>>
>>> Cc: sagar.a.kamble@intel.com
>>>
>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>
>> The test looks good to me (I haven't checked in details, the bar for igt
>> is quite a bit lower). It shows two gaps in the igt kms API:
>>
>>    - Retrieving the primary plane (there's a series from Matt fixing this
>>      and exposing the primary plane through igt_output_get_plane())
>>    - Adding a set_property() convenience function
>>      (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
>>      (no-one is working on that just yet, can de done later)
>>
>> A small question before pushing this, have you checked that the test
>> correctly skips when running with a kernel without rotation support?
>
> Note: don't push userspace using new ABI until that ABI has been
> agreed upon and committed to the kernel.
> -Chris
>
Hi Chris,
Are you referring to igt kms APIs? In this igt we are not using any API 
which is not merged.
-Sonika

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Chris Wilson]

On Wed, Jun 25, 2014 at 11:24:27AM +0530, Jindal, Sonika wrote:
> 
> 
> On 6/18/2014 5:09 PM, Chris Wilson wrote:
> >On Wed, Jun 18, 2014 at 12:32:00PM +0100, Damien Lespiau wrote:
> >>On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
> >>>From: Sonika Jindal <sonika.jindal@intel.com>
> >>>
> >>>Testcase for 180 degree HW rotation
> >>>
> >>>Cc: sagar.a.kamble@intel.com
> >>>
> >>>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>
> >>The test looks good to me (I haven't checked in details, the bar for igt
> >>is quite a bit lower). It shows two gaps in the igt kms API:
> >>
> >>   - Retrieving the primary plane (there's a series from Matt fixing this
> >>     and exposing the primary plane through igt_output_get_plane())
> >>   - Adding a set_property() convenience function
> >>     (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
> >>     (no-one is working on that just yet, can de done later)
> >>
> >>A small question before pushing this, have you checked that the test
> >>correctly skips when running with a kernel without rotation support?
> >
> >Note: don't push userspace using new ABI until that ABI has been
> >agreed upon and committed to the kernel.
> >-Chris
> >
> Hi Chris,
> Are you referring to igt kms APIs? In this igt we are not using any
> API which is not merged.

API also includes property names. If you are happy that everything is
upstream, then it cannot change and is ready to be used. Otherwise we
just end up with broken tests.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Jindal, Sonika]

On 6/25/2014 11:27 AM, Chris Wilson wrote:
> On Wed, Jun 25, 2014 at 11:24:27AM +0530, Jindal, Sonika wrote:
>>
>>
>> On 6/18/2014 5:09 PM, Chris Wilson wrote:
>>> On Wed, Jun 18, 2014 at 12:32:00PM +0100, Damien Lespiau wrote:
>>>> On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
>>>>> From: Sonika Jindal <sonika.jindal@intel.com>
>>>>>
>>>>> Testcase for 180 degree HW rotation
>>>>>
>>>>> Cc: sagar.a.kamble@intel.com
>>>>>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>
>>>> The test looks good to me (I haven't checked in details, the bar for igt
>>>> is quite a bit lower). It shows two gaps in the igt kms API:
>>>>
>>>>    - Retrieving the primary plane (there's a series from Matt fixing this
>>>>      and exposing the primary plane through igt_output_get_plane())
>>>>    - Adding a set_property() convenience function
>>>>      (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
>>>>      (no-one is working on that just yet, can de done later)
>>>>
>>>> A small question before pushing this, have you checked that the test
>>>> correctly skips when running with a kernel without rotation support?
>>>
>>> Note: don't push userspace using new ABI until that ABI has been
>>> agreed upon and committed to the kernel.
>>> -Chris
>>>
>> Hi Chris,
>> Are you referring to igt kms APIs? In this igt we are not using any
>> API which is not merged.
>
> API also includes property names. If you are happy that everything is
> upstream, then it cannot change and is ready to be used. Otherwise we
> just end up with broken tests.
> -Chris
>
Ok, got it. This test will not be merged unless the rotation patches and 
the documentation is merged.
-Sonika

Re: [PATCH 11/11] tests/kms_rotation_crc: IGT for 180 degree HW rotation

[Jindal, Sonika]

On 6/18/2014 5:02 PM, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:27PM +0530, sonika.jindal@intel.com wrote:
>> From: Sonika Jindal <sonika.jindal@intel.com>
>>
>> Testcase for 180 degree HW rotation
>>
>> Cc: sagar.a.kamble@intel.com
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>
> The test looks good to me (I haven't checked in details, the bar for igt
> is quite a bit lower). It shows two gaps in the igt kms API:
>
>    - Retrieving the primary plane (there's a series from Matt fixing this
>      and exposing the primary plane through igt_output_get_plane())
True, thats why I am checking for type property in this test. Can be 
removed once Matt's patches are merged
>    - Adding a set_property() convenience function
>      (ala igt_plane_set_property("rotation", BIT(DRM_ROTATE_180)))
>      (no-one is working on that just yet, can de done later)
Sure, will be good to have.
>
> A small question before pushing this, have you checked that the test
> correctly skips when running with a kernel without rotation support?
NO, I haven't tested that. Will try in sometime and let you know if I 
see any issue there.
>
> Thanks,
>

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Damien Lespiau]

On Wed, Jun 18, 2014 at 02:27:16PM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> Enables 180 degree rotation for sprite and primary planes.
> Updated the primary plane rotation support as per the new universal plane
> design.
> 
> Most of these patches were already reviewed in intel-gfx in February 2014 thats
> why there is version history in few of them.
> 
> Testcase: kms_rotation_crc
> This igt can be extended for clipped rotation cases. Right it only tests 180
> degree rotation for sprite and primary plane with crc check.
> 
> Sonika Jindal (2):
>   tests/kms_rotation_crc: IGT for 180 degree HW rotation
>   drm/i915: Add 180 degree primary plane rotation support
> 
> Ville Syrjälä (9):
>   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
>   drm: Add support_bits parameter to drm_property_create_bitmask()
>   drm: Add drm_mode_create_rotation_property()
>   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
>   drm: Add drm_rect rotation functions
>   drm: Add drm_rotation_simplify()
>   drm/i915: Add 180 degree sprite rotation support
>   drm/i915: Make intel_plane_restore() return an error
>   drm/i915: Add rotation property for sprites

Now that we have universal planes, I guess the last blocker for this
series is gone. Thanks for keeping at it.

Some of these patches already had review tags. Would you mind:

  - Re-add the r-b tags already gathered,
  - Remove Greg KH from the Cc. list, let's not bother him with that
  - Remove Dave Airlie from the Cc. list, dri-devel is enough (+ us
    pinging him about the series if necessary)
  - Don't resend the igt patch with the kernel ones,
  - Add a patch on top to update the big property table
    Documentation/DocBook/drm.tmpl - let's make sure we don't upstream
    new properties without updating that list

-- 
Damien

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Chris Wilson]

On Wed, Jun 18, 2014 at 12:00:36PM +0100, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:16PM +0530, sonika.jindal@intel.com wrote:
> > From: Sonika Jindal <sonika.jindal@intel.com>
> > 
> > Enables 180 degree rotation for sprite and primary planes.
> > Updated the primary plane rotation support as per the new universal plane
> > design.
> > 
> > Most of these patches were already reviewed in intel-gfx in February 2014 thats
> > why there is version history in few of them.
> > 
> > Testcase: kms_rotation_crc
> > This igt can be extended for clipped rotation cases. Right it only tests 180
> > degree rotation for sprite and primary plane with crc check.
> > 
> > Sonika Jindal (2):
> >   tests/kms_rotation_crc: IGT for 180 degree HW rotation
> >   drm/i915: Add 180 degree primary plane rotation support
> > 
> > Ville Syrjälä (9):
> >   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
> >   drm: Add support_bits parameter to drm_property_create_bitmask()
> >   drm: Add drm_mode_create_rotation_property()
> >   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
> >   drm: Add drm_rect rotation functions
> >   drm: Add drm_rotation_simplify()
> >   drm/i915: Add 180 degree sprite rotation support
> >   drm/i915: Make intel_plane_restore() return an error
> >   drm/i915: Add rotation property for sprites
> 
> Now that we have universal planes, I guess the last blocker for this
> series is gone. Thanks for keeping at it.

Hah. We were last arguing over the property names.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Damien Lespiau]

On Wed, Jun 18, 2014 at 12:07:17PM +0100, Chris Wilson wrote:
> > Now that we have universal planes, I guess the last blocker for this
> > series is gone. Thanks for keeping at it.
> 
> Hah. We were last arguing over the property names.

But only because the property was on the CRTC and "rotation" (or
"primary-rotation") ended up in limbo? The property is now on the
(primary) plane object.

-- 
Damien

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Chris Wilson]

On Wed, Jun 18, 2014 at 12:12:31PM +0100, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 12:07:17PM +0100, Chris Wilson wrote:
> > > Now that we have universal planes, I guess the last blocker for this
> > > series is gone. Thanks for keeping at it.
> > 
> > Hah. We were last arguing over the property names.
> 
> But only because the property was on the CRTC and "rotation" (or
> "primary-rotation") ended up in limbo? The property is now on the
> (primary) plane object.

Ville also had plans for a superproperty called rotation on the CRTC
that would adjust all planes. Not quite sure what his final plans were.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Damien Lespiau]

On Wed, Jun 18, 2014 at 12:21:39PM +0100, Chris Wilson wrote:
> On Wed, Jun 18, 2014 at 12:12:31PM +0100, Damien Lespiau wrote:
> > On Wed, Jun 18, 2014 at 12:07:17PM +0100, Chris Wilson wrote:
> > > > Now that we have universal planes, I guess the last blocker for this
> > > > series is gone. Thanks for keeping at it.
> > > 
> > > Hah. We were last arguing over the property names.
> > 
> > But only because the property was on the CRTC and "rotation" (or
> > "primary-rotation") ended up in limbo? The property is now on the
> > (primary) plane object.
> 
> Ville also had plans for a superproperty called rotation on the CRTC
> that would adjust all planes. Not quite sure what his final plans were.

Sure, that would have to be some extra fun to be had for another time
though (looks useful).

-- 
Damien

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Ville Syrjälä]

On Wed, Jun 18, 2014 at 12:21:39PM +0100, Chris Wilson wrote:
> On Wed, Jun 18, 2014 at 12:12:31PM +0100, Damien Lespiau wrote:
> > On Wed, Jun 18, 2014 at 12:07:17PM +0100, Chris Wilson wrote:
> > > > Now that we have universal planes, I guess the last blocker for this
> > > > series is gone. Thanks for keeping at it.
> > > 
> > > Hah. We were last arguing over the property names.
> > 
> > But only because the property was on the CRTC and "rotation" (or
> > "primary-rotation") ended up in limbo? The property is now on the
> > (primary) plane object.
> 
> Ville also had plans for a superproperty called rotation on the CRTC
> that would adjust all planes. Not quite sure what his final plans were.

I just liked it because it was so easy to hook in at the kernel level,
and it made it possible to test rotation without having support for it
in the ddx. But I don't mind if we just drop it for now. We can revisit
later if there's a real use case for it.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/11] Support for 180 degree HW rotation

[Jindal, Sonika]

On 6/18/2014 4:30 PM, Damien Lespiau wrote:
> On Wed, Jun 18, 2014 at 02:27:16PM +0530, sonika.jindal@intel.com wrote:
>> From: Sonika Jindal <sonika.jindal@intel.com>
>>
>> Enables 180 degree rotation for sprite and primary planes.
>> Updated the primary plane rotation support as per the new universal plane
>> design.
>>
>> Most of these patches were already reviewed in intel-gfx in February 2014 thats
>> why there is version history in few of them.
>>
>> Testcase: kms_rotation_crc
>> This igt can be extended for clipped rotation cases. Right it only tests 180
>> degree rotation for sprite and primary plane with crc check.
>>
>> Sonika Jindal (2):
>>    tests/kms_rotation_crc: IGT for 180 degree HW rotation
>>    drm/i915: Add 180 degree primary plane rotation support
>>
>> Ville Syrjälä (9):
>>    drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
>>    drm: Add support_bits parameter to drm_property_create_bitmask()
>>    drm: Add drm_mode_create_rotation_property()
>>    drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
>>    drm: Add drm_rect rotation functions
>>    drm: Add drm_rotation_simplify()
>>    drm/i915: Add 180 degree sprite rotation support
>>    drm/i915: Make intel_plane_restore() return an error
>>    drm/i915: Add rotation property for sprites
>
> Now that we have universal planes, I guess the last blocker for this
> series is gone. Thanks for keeping at it.
>
> Some of these patches already had review tags. Would you mind:
>
>    - Re-add the r-b tags already gathered,
>    - Remove Greg KH from the Cc. list, let's not bother him with that
>    - Remove Dave Airlie from the Cc. list, dri-devel is enough (+ us
>      pinging him about the series if necessary)
>    - Don't resend the igt patch with the kernel ones,
>    - Add a patch on top to update the big property table
>      Documentation/DocBook/drm.tmpl - let's make sure we don't upstream
>      new properties without updating that list
>
Thanks Damien,
I have updated the patches with r-b tags.
I will update the documentation and post the patch shortly.

[PATCH 1/1] Documentation: drm: describing rotation property for i915

[sonika.jindal]

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

Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: linux-doc@vger.kernel.org (open list:DOCUMENTATION)
Cc: linux-kernel@vger.kernel.org (open list)
---
 Documentation/DocBook/drm.tmpl | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7df3134..ce11fd7 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2664,11 +2664,13 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
+	<td valign="top" >“rotation”</td>
+	<td valign="top" >BITMASK</td>
+	<td valign="top" >{ 0, "rotate-0" },
+	{ 2, "rotate-180" }</td>
+	<td valign="top" >Plane</td>
+	<td valign="top" >Used to set plane rotation. Only 0 and 180 degree
+	rotation supported as of now</td>
 	</tr>
 	<tr>
 	<td rowspan="17" valign="top" >SDVO-TV</td>
-- 
1.8.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/1] Documentation: drm: describing rotation property for i915

[Damien Lespiau]

On Thu, Jun 19, 2014 at 11:41:27AM +0530, sonika.jindal@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> Cc: linux-doc@vger.kernel.org (open list:DOCUMENTATION)
> Cc: linux-kernel@vger.kernel.org (open list)
> ---
>  Documentation/DocBook/drm.tmpl | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 7df3134..ce11fd7 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2664,11 +2664,13 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> +	<td valign="top" >“rotation”</td>
> +	<td valign="top" >BITMASK</td>
> +	<td valign="top" >{ 0, "rotate-0" },
> +	{ 2, "rotate-180" }</td>

Remove new line?

> +	<td valign="top" >Plane</td>
> +	<td valign="top" >Used to set plane rotation. Only 0 and 180 degree
> +	rotation supported as of now</td>
>  	</tr>
>  	<tr>
>  	<td rowspan="17" valign="top" >SDVO-TV</td>

So, you've spotted that weird line with "Standard * as in DRM". I agree
that it looks out of place. However if you want to make that change, you
need to fix the whole table (this line appears elsewhere as well) and
that's a separate change from adding the documentation for the rotation
propery (ie different patch).

Now can we as well:

  - Put the property under a Plane group
  - Your description doesn't add any information that isn't already in
    the previous fields, might as well say nothing there.

Thanks,

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] Documentation: drm: Removing placeholders for generic drm properties description

[sonika.jindal]

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

These property descriptions were kept as placeholder. Removing them for simplicity.

Cc: damien.lespiau@intel.com
Cc: daniel.vetter@ffwll.ch
Cc: ville.syrjala@linux.intel.com
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 Documentation/DocBook/drm.tmpl | 64 +++++++-----------------------------------
 1 file changed, 10 insertions(+), 54 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index b314a42..3adb6be 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,8 +2648,8 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="21" valign="top" >i915</td>
-	<td rowspan="3" valign="top" >Generic</td>
+	<td rowspan="20" valign="top" >i915</td>
+	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td>
@@ -2664,13 +2664,6 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
 	<td rowspan="17" valign="top" >SDVO-TV</td>
 	<td valign="top" >“mode”</td>
 	<td valign="top" >ENUM</td>
@@ -2799,8 +2792,8 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >CDV gma-500</td>
-	<td rowspan="3" valign="top" >Generic</td>
+	<td rowspan="2" valign="top" >CDV gma-500</td>
+	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ “Full”, “Limited 16:235” }</td>
@@ -2815,15 +2808,8 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
-	<td rowspan="20" valign="top" >Poulsbo</td>
-	<td rowspan="2" valign="top" >Generic</td>
+	<td rowspan="19" valign="top" >Poulsbo</td>
+	<td rowspan="1" valign="top" >Generic</td>
 	<td valign="top" >“backlight”</td>
 	<td valign="top" >RANGE</td>
 	<td valign="top" >Min=0, Max=100</td>
@@ -2831,13 +2817,6 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
 	<td rowspan="17" valign="top" >SDVO-TV</td>
 	<td valign="top" >“mode”</td>
 	<td valign="top" >ENUM</td>
@@ -3064,7 +3043,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >i2c/ch7006_drv</td>
+	<td rowspan="2" valign="top" >i2c/ch7006_drv</td>
 	<td valign="top" >Generic</td>
 	<td valign="top" >“scale”</td>
 	<td valign="top" >RANGE</td>
@@ -3073,14 +3052,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="2" valign="top" >TV</td>
-	<td valign="top" >Standard names as in DRM</td>
-	<td valign="top" >Standard types as in DRM</td>
-	<td valign="top" >Standard Values as in DRM</td>
-	<td valign="top" >Standard object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
+	<td rowspan="1" valign="top" >TV</td>
 	<td valign="top" >“mode”</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "PAL", "PAL-M","PAL-N"}, ”PAL-Nc"
@@ -3089,7 +3061,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="16" valign="top" >nouveau</td>
+	<td rowspan="15" valign="top" >nouveau</td>
 	<td rowspan="6" valign="top" >NV10 Overlay</td>
 	<td valign="top" >"colorkey"</td>
 	<td valign="top" >RANGE</td>
@@ -3198,14 +3170,6 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Generic</td>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
 	<td rowspan="2" valign="top" >omap</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >“rotation”</td>
@@ -3236,7 +3200,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="10" valign="top" >radeon</td>
+	<td rowspan="9" valign="top" >radeon</td>
 	<td valign="top" >DVI-I</td>
 	<td valign="top" >“coherent”</td>
 	<td valign="top" >RANGE</td>
@@ -3308,14 +3272,6 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td valign="top" >Generic</td>
-	<td valign="top" >Standard name as in DRM</td>
-	<td valign="top" >Standard type as in DRM</td>
-	<td valign="top" >Standard value as in DRM</td>
-	<td valign="top" >Standard Object as in DRM</td>
-	<td valign="top" >TBD</td>
-	</tr>
-	<tr>
 	<td rowspan="3" valign="top" >rcar-du</td>
 	<td rowspan="3" valign="top" >Generic</td>
 	<td valign="top" >"alpha"</td>
-- 
1.8.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] Documentation: drm: describing rotation property for i915

[sonika.jindal]

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

Cc: damien.lespiau@intel.com
Cc: daniel.vetter@ffwll.ch
Cc: ville.syrjala@linux.intel.com
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 Documentation/DocBook/drm.tmpl | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 3adb6be..3bd8ae8 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="20" valign="top" >i915</td>
+	<td rowspan="21" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2664,6 +2664,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td rowspan="1" valign="top" >Plane</td>
+	<td valign="top" >“rotation”</td>
+	<td valign="top" >BITMASK</td>
+	<td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td>
+	<td valign="top" >Plane</td>
+	<td valign="top" >TBD</td>
+	</tr>
+	<tr>
 	<td rowspan="17" valign="top" >SDVO-TV</td>
 	<td valign="top" >“mode”</td>
 	<td valign="top" >ENUM</td>
-- 
1.8.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] Documentation: drm: describing rotation property for i915

[Damien Lespiau]

On Wed, Jun 25, 2014 at 11:08:06AM +0530, sonika.jindal@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Cc: damien.lespiau@intel.com
> Cc: daniel.vetter@ffwll.ch
> Cc: ville.syrjala@linux.intel.com
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>


Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  Documentation/DocBook/drm.tmpl | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 3adb6be..3bd8ae8 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2648,7 +2648,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="20" valign="top" >i915</td>
> +	<td rowspan="21" valign="top" >i915</td>
>  	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
>  	<td valign="top" >ENUM</td>
> @@ -2664,6 +2664,14 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> +	<td rowspan="1" valign="top" >Plane</td>
> +	<td valign="top" >“rotation”</td>
> +	<td valign="top" >BITMASK</td>
> +	<td valign="top" >{ 0, "rotate-0" }, { 2, "rotate-180" }</td>
> +	<td valign="top" >Plane</td>
> +	<td valign="top" >TBD</td>
> +	</tr>
> +	<tr>
>  	<td rowspan="17" valign="top" >SDVO-TV</td>
>  	<td valign="top" >“mode”</td>
>  	<td valign="top" >ENUM</td>
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Documentation: drm: Removing placeholders for generic drm properties description

[Damien Lespiau]

On Wed, Jun 25, 2014 at 11:08:05AM +0530, sonika.jindal@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> These property descriptions were kept as placeholder. Removing them for simplicity.
> 
> Cc: damien.lespiau@intel.com
> Cc: daniel.vetter@ffwll.ch
> Cc: ville.syrjala@linux.intel.com
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

> ---
>  Documentation/DocBook/drm.tmpl | 64 +++++++-----------------------------------
>  1 file changed, 10 insertions(+), 54 deletions(-)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index b314a42..3adb6be 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2648,8 +2648,8 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="21" valign="top" >i915</td>
> -	<td rowspan="3" valign="top" >Generic</td>
> +	<td rowspan="20" valign="top" >i915</td>
> +	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td>
> @@ -2664,13 +2664,6 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
>  	<td rowspan="17" valign="top" >SDVO-TV</td>
>  	<td valign="top" >“mode”</td>
>  	<td valign="top" >ENUM</td>
> @@ -2799,8 +2792,8 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="3" valign="top" >CDV gma-500</td>
> -	<td rowspan="3" valign="top" >Generic</td>
> +	<td rowspan="2" valign="top" >CDV gma-500</td>
> +	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ “Full”, “Limited 16:235” }</td>
> @@ -2815,15 +2808,8 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
> -	<td rowspan="20" valign="top" >Poulsbo</td>
> -	<td rowspan="2" valign="top" >Generic</td>
> +	<td rowspan="19" valign="top" >Poulsbo</td>
> +	<td rowspan="1" valign="top" >Generic</td>
>  	<td valign="top" >“backlight”</td>
>  	<td valign="top" >RANGE</td>
>  	<td valign="top" >Min=0, Max=100</td>
> @@ -2831,13 +2817,6 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
>  	<td rowspan="17" valign="top" >SDVO-TV</td>
>  	<td valign="top" >“mode”</td>
>  	<td valign="top" >ENUM</td>
> @@ -3064,7 +3043,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="3" valign="top" >i2c/ch7006_drv</td>
> +	<td rowspan="2" valign="top" >i2c/ch7006_drv</td>
>  	<td valign="top" >Generic</td>
>  	<td valign="top" >“scale”</td>
>  	<td valign="top" >RANGE</td>
> @@ -3073,14 +3052,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="2" valign="top" >TV</td>
> -	<td valign="top" >Standard names as in DRM</td>
> -	<td valign="top" >Standard types as in DRM</td>
> -	<td valign="top" >Standard Values as in DRM</td>
> -	<td valign="top" >Standard object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
> +	<td rowspan="1" valign="top" >TV</td>
>  	<td valign="top" >“mode”</td>
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ "PAL", "PAL-M","PAL-N"}, ”PAL-Nc"
> @@ -3089,7 +3061,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="16" valign="top" >nouveau</td>
> +	<td rowspan="15" valign="top" >nouveau</td>
>  	<td rowspan="6" valign="top" >NV10 Overlay</td>
>  	<td valign="top" >"colorkey"</td>
>  	<td valign="top" >RANGE</td>
> @@ -3198,14 +3170,6 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Generic</td>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
>  	<td rowspan="2" valign="top" >omap</td>
>  	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >“rotation”</td>
> @@ -3236,7 +3200,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="10" valign="top" >radeon</td>
> +	<td rowspan="9" valign="top" >radeon</td>
>  	<td valign="top" >DVI-I</td>
>  	<td valign="top" >“coherent”</td>
>  	<td valign="top" >RANGE</td>
> @@ -3308,14 +3272,6 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td valign="top" >Generic</td>
> -	<td valign="top" >Standard name as in DRM</td>
> -	<td valign="top" >Standard type as in DRM</td>
> -	<td valign="top" >Standard value as in DRM</td>
> -	<td valign="top" >Standard Object as in DRM</td>
> -	<td valign="top" >TBD</td>
> -	</tr>
> -	<tr>
>  	<td rowspan="3" valign="top" >rcar-du</td>
>  	<td rowspan="3" valign="top" >Generic</td>
>  	<td valign="top" >"alpha"</td>
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx