[PATCH 00/11] Enabling 180 degree rotation for sprite and crtc planes

[PATCH 00/11] Enabling 180 degree rotation for sprite and crtc planes

[sagar.a.kamble]

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

With these patches 180 degree rotation for crtc and sprite planes will be
exposed through drm rotation properties implemented by Ville few days back.
I have added CRTC rotation support on top of Ville's 9 patches that includes
sprite rotation. Omapdrm changes, rotation_simplify, drm_rect functions are
also included in this list. Functionality has been tested with i-g-t test
"intel_plane_rotate".


Sagar Kamble (11):
  drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
  drm: Add drm_mode_create_rotation_property()
  drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
  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
  drm: Add support_bits parameter to drm_property_create_bitmask()
  drm: Add drm_rect rotation functions
  drm/i915: Add 180 degree primary plane rotation support
  drm: Set property to return invalid for unsupported arguments for
    bitmask property

 drivers/gpu/drm/drm_crtc.c           |  72 +++++++++++++++++-
 drivers/gpu/drm/drm_rect.c           | 140 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |   4 +
 drivers/gpu/drm/i915/intel_display.c |  54 +++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   5 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  90 ++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_drv.h   |   7 --
 drivers/gpu/drm/omapdrm/omap_plane.c |  17 ++---
 include/drm/drm_crtc.h               |  16 +++-
 include/drm/drm_rect.h               |   6 ++
 11 files changed, 382 insertions(+), 30 deletions(-)

-- 
1.8.5

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

[sagar.a.kamble]

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

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

[sagar.a.kamble]

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

Add a function to create a standards compliant rotation property.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@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 3b7d32d..87744d6 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4114,3 +4114,21 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	idr_destroy(&dev->mode_config.crtc_idr);
 }
 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 d5c46c1..b7d0f3c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1182,6 +1182,8 @@ extern int drm_format_plane_cpp(uint32_t format, int plane);
 extern int drm_format_horz_chroma_subsampling(uint32_t format);
 extern int drm_format_vert_chroma_subsampling(uint32_t format);
 extern const char *drm_get_format_name(uint32_t format);
+extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
+							      unsigned int supported_rotations);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

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

[sagar.a.kamble]

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

Use the new drm_mode_create_rotation_property() in omapdrm.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

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

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 30 ++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 87744d6..b0a2889 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4002,6 +4002,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 b7d0f3c..5a6c345 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1184,6 +1184,9 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
 extern const char *drm_get_format_name(uint32_t format);
 extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 							      unsigned int supported_rotations);
+extern unsigned int drm_rotation_simplify(unsigned int rotation,
+					  unsigned int supported_rotations);
+
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-- 
1.8.5

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala<ville.syrjala@linux.intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  3 +++
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

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

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

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

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_sprite.c | 42 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fa37dfd..ea2efc3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1548,6 +1548,7 @@ typedef struct drm_i915_private {
 
 	struct drm_property *broadcast_rgb_property;
 	struct drm_property *force_audio_property;
+	struct drm_property *rotation_property;
 
 	uint32_t hw_context_size;
 	struct list_head context_list;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 11560a6..d84f2fb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1044,6 +1044,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);
@@ -1070,6 +1094,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[] = {
@@ -1106,6 +1131,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;
@@ -1180,8 +1206,22 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 			     &intel_plane_funcs,
 			     plane_formats, num_plane_formats,
 			     false);
-	if (ret)
+	if (ret) {
 		kfree(intel_plane);
+		goto out;
+	}
+
+	if (!dev_priv->rotation_property)
+		dev_priv->rotation_property =
+			drm_mode_create_rotation_property(dev,
+							  BIT(DRM_ROTATE_0) |
+							  BIT(DRM_ROTATE_180));
+
+	if (dev_priv->rotation_property)
+		drm_object_attach_property(&intel_plane->base.base,
+					   dev_priv->rotation_property,
+					   intel_plane->rotation);
 
+ out:
 	return ret;
 }
-- 
1.8.5

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 6 +++++-
 include/drm/drm_crtc.h     | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b0a2889..4f5e408 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2906,7 +2906,8 @@ EXPORT_SYMBOL(drm_property_create_enum);
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values)
+					 int num_values,
+					 unsigned int supported_bits)
 {
 	struct drm_property *property;
 	int i, ret;
@@ -2918,6 +2919,9 @@ struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 		return NULL;
 
 	for (i = 0; i < num_values; i++) {
+		if (!(supported_bits & (1 << i)))
+			continue;
+
 		ret = drm_property_add_enum(property, i,
 				      props[i].type,
 				      props[i].name);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5a6c345..e227e85 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1070,7 +1070,8 @@ extern struct drm_property *drm_property_create_enum(struct drm_device *dev, int
 struct drm_property *drm_property_create_bitmask(struct drm_device *dev,
 					 int flags, const char *name,
 					 const struct drm_prop_enum_list *props,
-					 int num_values);
+					 int num_values,
+					 unsigned int supported_bits);
 struct drm_property *drm_property_create_range(struct drm_device *dev, int flags,
 					 const char *name,
 					 uint64_t min, uint64_t max);
-- 
1.8.5

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

[sagar.a.kamble]

From: Sagar Kamble <sagar.a.kamble@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.

Signed-off-by: Ville Syrjala <ville.syrjala@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.8.5

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

[sagar.a.kamble]

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

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

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 54 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 57906c5..d3000c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3553,6 +3553,7 @@
 #define   DISPPLANE_NO_LINE_DOUBLE		0
 #define   DISPPLANE_STEREO_POLARITY_FIRST	0
 #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
+#define   DISPPLANE_ROTATE_180			(1<<15)
 #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
 #define   DISPPLANE_TILED			(1<<10)
 #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..483de59 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	unsigned long linear_offset;
 	u32 dspcntr;
 	u32 reg;
+	int pixel_size;
 
 	switch (plane) {
 	case 0:
@@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		return -EINVAL;
 	}
 
+	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
@@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	dspcntr = I915_READ(reg);
 	/* Mask out pixel format bits in case we change it */
 	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
+	dspcntr &= ~DISPPLANE_ROTATE_180;
+
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
@@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	if (IS_G4X(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
-	I915_WRITE(reg, dspcntr);
-
 	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
 
 	if (INTEL_INFO(dev)->gen >= 4) {
@@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
+	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
+		dspcntr |= DISPPLANE_ROTATE_180;
+
+		x += (fb->width - 1);
+		y += (fb->height - 1);
+		linear_offset += (fb->height - 1) * fb->pitches[0] +
+                                   fb->width * pixel_size;
+	}
+
+	I915_WRITE(reg, dspcntr);
+
 	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
 		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
 		      fb->pitches[0]);
@@ -8748,6 +8761,31 @@ free_work:
 	return ret;
 }
 
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+				    struct drm_property *prop,
+				    uint64_t val)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	uint64_t old_val;
+	int ret = -ENOENT;
+
+	if (prop == dev_priv->rotation_property) {
+		/* exactly one rotation angle please */
+		if (hweight32(val & 0xf) != 1)
+			return -EINVAL;
+
+		old_val = intel_crtc->rotation;
+		intel_crtc->rotation = val;
+		ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
+		if (ret)
+			intel_crtc->rotation = old_val;
+	}
+
+	return ret;
+}
+
 static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
 	.load_lut = intel_crtc_load_lut,
@@ -10160,6 +10198,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = intel_crtc_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = intel_crtc_set_property
 };
 
 static void intel_cpu_pll_init(struct drm_device *dev)
@@ -10288,6 +10327,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	 */
 	intel_crtc->pipe = pipe;
 	intel_crtc->plane = pipe;
+	intel_crtc->rotation = BIT(DRM_ROTATE_0);
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
 		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
 		intel_crtc->plane = !pipe;
@@ -10298,6 +10338,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
 
+	if (!dev_priv->rotation_property)
+		dev_priv->rotation_property =
+			drm_mode_create_rotation_property(dev,
+							BIT(DRM_ROTATE_0) |
+							BIT(DRM_ROTATE_180));
+	if (dev_priv->rotation_property)
+		drm_object_attach_property(&intel_crtc->base.base,
+					dev_priv->rotation_property,
+					intel_crtc->rotation);
+
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7a79b8e..02dfdb6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -331,6 +331,8 @@ struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
 	enum plane plane;
+	unsigned int rotation;
+
 	u8 lut_r[256], lut_g[256], lut_b[256];
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
-- 
1.8.5

[PATCH 11/11] drm: Set property to return invalid for unsupported arguments for bitmask property

[sagar.a.kamble]

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

DRM will not propagate the set_property call for bitmask drm
properties if they are not supported by underlying driver.

Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4f5e408..4c92741 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3420,6 +3420,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 	struct drm_mode_object *arg_obj;
 	struct drm_mode_object *prop_obj;
 	struct drm_property *property;
+	struct drm_property_enum *prop_enum;
+	bool supported_val = false;
 	int ret = -EINVAL;
 	int i;
 
@@ -3451,6 +3453,22 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 	}
 	property = obj_to_property(prop_obj);
 
+	if (property->flags | DRM_MODE_PROP_BITMASK) {
+		if (!list_empty(&property->enum_blob_list)) {
+			list_for_each_entry(prop_enum,
+					&property->enum_blob_list, head) {
+				if (BIT(prop_enum->value) == arg->value) {
+					supported_val = true;
+					break;
+				}
+			}
+		}
+		if (!supported_val) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
 	if (!drm_property_change_is_valid(property, arg->value))
 		goto out;
 
-- 
1.8.5

Re: [PATCH 00/11] Enabling 180 degree rotation for sprite and crtc planes

[Ville Syrjälä]

On Sat, Feb 01, 2014 at 12:40:36AM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> With these patches 180 degree rotation for crtc and sprite planes will be
> exposed through drm rotation properties implemented by Ville few days back.
> I have added CRTC rotation support on top of Ville's 9 patches that includes
> sprite rotation. Omapdrm changes, rotation_simplify, drm_rect functions are
> also included in this list. Functionality has been tested with i-g-t test
> "intel_plane_rotate".
> 
> 
> Sagar Kamble (11):

Somehow you've managed to change the authorship information of most of
the patches. Please don't do that.

>   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
>   drm: Add drm_mode_create_rotation_property()
>   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
>   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
>   drm: Add support_bits parameter to drm_property_create_bitmask()
>   drm: Add drm_rect rotation functions
>   drm/i915: Add 180 degree primary plane rotation support
>   drm: Set property to return invalid for unsupported arguments for
>     bitmask property
> 
>  drivers/gpu/drm/drm_crtc.c           |  72 +++++++++++++++++-
>  drivers/gpu/drm/drm_rect.c           | 140 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |   4 +
>  drivers/gpu/drm/i915/intel_display.c |  54 +++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |   5 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |  90 ++++++++++++++++++++--
>  drivers/gpu/drm/omapdrm/omap_drv.h   |   7 --
>  drivers/gpu/drm/omapdrm/omap_plane.c |  17 ++---
>  include/drm/drm_crtc.h               |  16 +++-
>  include/drm/drm_rect.h               |   6 ++
>  11 files changed, 382 insertions(+), 30 deletions(-)
> 
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

[Ville Syrjälä]

On Sat, Feb 01, 2014 at 12:40:41AM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@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.
> 
> Signed-off-by: Ville Syrjala<ville.syrjala@linux.intel.com>
> Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>

I actually posted a v2 of this patch, which fixes HSW. But I guess now
we really need a v3 which extends that fix for BDW. Assuming BDW is like
HSW here.

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

-- 
Ville Syrjälä
Intel OTC

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

[Ville Syrjälä]

On Sat, Feb 01, 2014 at 12:40:46AM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Primary planes support 180 degree rotation. Expose the feature
> through rotation drm property.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 54 ++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57906c5..d3000c4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3553,6 +3553,7 @@
>  #define   DISPPLANE_NO_LINE_DOUBLE		0
>  #define   DISPPLANE_STEREO_POLARITY_FIRST	0
>  #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
> +#define   DISPPLANE_ROTATE_180			(1<<15)
>  #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
>  #define   DISPPLANE_TILED			(1<<10)
>  #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d4a0d9..483de59 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
>  	switch (plane) {
>  	case 0:
> @@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		return -EINVAL;
>  	}
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	dspcntr = I915_READ(reg);
>  	/* Mask out pixel format bits in case we change it */
>  	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> +	dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> -	I915_WRITE(reg, dspcntr);
> -
>  	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> @@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> +	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
> +		x += (fb->width - 1);
> +		y += (fb->height - 1);
> +		linear_offset += (fb->height - 1) * fb->pitches[0] +
> +                                   fb->width * pixel_size;
> +	}

This isn't quite right. We need something like:
x += pipe_src_w - 1;
y += pipe_src_h - 1;

That suggests your tests don't check if panning works correctly when
rotated.

I was also going to say we need to do this before stashing the result
into intel_crtc->dspaddr_offset on gen2/3, but it actually looks like
rotation is a gen4+ feature, so this place looks OK for this code.

You've also forgotten to add the rotation bits to
ironlake_update_plane(). That one will need additional HSW/BDW checks
I think.

> +
> +	I915_WRITE(reg, dspcntr);
> +
>  	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
>  		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
>  		      fb->pitches[0]);
> @@ -8748,6 +8761,31 @@ free_work:
>  	return ret;
>  }
>  
> +static int intel_crtc_set_property(struct drm_crtc *crtc,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	uint64_t old_val;
> +	int ret = -ENOENT;
> +
> +	if (prop == dev_priv->rotation_property) {
> +		/* exactly one rotation angle please */
> +		if (hweight32(val & 0xf) != 1)
> +			return -EINVAL;
> +
> +		old_val = intel_crtc->rotation;
> +		intel_crtc->rotation = val;
> +		ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
> +		if (ret)
> +			intel_crtc->rotation = old_val;
> +	}
> +
> +	return ret;
> +}
> +
>  static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
>  	.load_lut = intel_crtc_load_lut,
> @@ -10160,6 +10198,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = intel_crtc_set_config,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
> +	.set_property = intel_crtc_set_property
>  };
>  
>  static void intel_cpu_pll_init(struct drm_device *dev)
> @@ -10288,6 +10327,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	 */
>  	intel_crtc->pipe = pipe;
>  	intel_crtc->plane = pipe;
> +	intel_crtc->rotation = BIT(DRM_ROTATE_0);
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
>  		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
>  		intel_crtc->plane = !pipe;
> @@ -10298,6 +10338,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
>  
> +	if (!dev_priv->rotation_property)
> +		dev_priv->rotation_property =
> +			drm_mode_create_rotation_property(dev,
> +							BIT(DRM_ROTATE_0) |
> +							BIT(DRM_ROTATE_180));
> +	if (dev_priv->rotation_property)
> +		drm_object_attach_property(&intel_crtc->base.base,
> +					dev_priv->rotation_property,
> +					intel_crtc->rotation);

As I mentioned it looks like rotation is only supported starting from
gen4. So we need to check for that here before we add the props.

> +
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7a79b8e..02dfdb6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -331,6 +331,8 @@ struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
>  	enum plane plane;
> +	unsigned int rotation;
> +
>  	u8 lut_r[256], lut_g[256], lut_b[256];
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/11] Enabling 180 degree rotation for sprite and crtc planes

[Sagar Arun Kamble]

On Fri, 2014-01-31 at 22:38 +0200, Ville Syrjälä wrote:
> On Sat, Feb 01, 2014 at 12:40:36AM +0530, sagar.a.kamble@intel.com wrote:
> > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > 
> > With these patches 180 degree rotation for crtc and sprite planes will be
> > exposed through drm rotation properties implemented by Ville few days back.
> > I have added CRTC rotation support on top of Ville's 9 patches that includes
> > sprite rotation. Omapdrm changes, rotation_simplify, drm_rect functions are
> > also included in this list. Functionality has been tested with i-g-t test
> > "intel_plane_rotate".
> > 
> > 
> > Sagar Kamble (11):
> 
> Somehow you've managed to change the authorship information of most of
> the patches. Please don't do that.

Sorry for this mistake. I am new to this. I will send the revised
patches for addressing the review comments.
> 
> >   drm: Move DRM_ROTATE bits out of omapdrm into drm_crtc.h
> >   drm: Add drm_mode_create_rotation_property()
> >   drm/omap: Switch omapdrm over to drm_mode_create_rotation_property()
> >   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
> >   drm: Add support_bits parameter to drm_property_create_bitmask()
> >   drm: Add drm_rect rotation functions
> >   drm/i915: Add 180 degree primary plane rotation support
> >   drm: Set property to return invalid for unsupported arguments for
> >     bitmask property
> > 
> >  drivers/gpu/drm/drm_crtc.c           |  72 +++++++++++++++++-
> >  drivers/gpu/drm/drm_rect.c           | 140 +++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_drv.h      |   1 +
> >  drivers/gpu/drm/i915/i915_reg.h      |   4 +
> >  drivers/gpu/drm/i915/intel_display.c |  54 +++++++++++++-
> >  drivers/gpu/drm/i915/intel_drv.h     |   5 +-
> >  drivers/gpu/drm/i915/intel_sprite.c  |  90 ++++++++++++++++++++--
> >  drivers/gpu/drm/omapdrm/omap_drv.h   |   7 --
> >  drivers/gpu/drm/omapdrm/omap_plane.c |  17 ++---
> >  include/drm/drm_crtc.h               |  16 +++-
> >  include/drm/drm_rect.h               |   6 ++
> >  11 files changed, 382 insertions(+), 30 deletions(-)
> > 
> > -- 
> > 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 10/11] drm/i915: Add 180 degree primary plane rotation support

[Ville Syrjälä]

On Sat, Feb 01, 2014 at 12:40:46AM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Primary planes support 180 degree rotation. Expose the feature
> through rotation drm property.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>
> Tested-by: Sagar Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 54 ++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 57906c5..d3000c4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3553,6 +3553,7 @@
>  #define   DISPPLANE_NO_LINE_DOUBLE		0
>  #define   DISPPLANE_STEREO_POLARITY_FIRST	0
>  #define   DISPPLANE_STEREO_POLARITY_SECOND	(1<<18)
> +#define   DISPPLANE_ROTATE_180			(1<<15)
>  #define   DISPPLANE_TRICKLE_FEED_DISABLE	(1<<14) /* Ironlake */
>  #define   DISPPLANE_TILED			(1<<10)
>  #define _DSPAADDR		(dev_priv->info->display_mmio_offset + 0x70184)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d4a0d9..483de59 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	unsigned long linear_offset;
>  	u32 dspcntr;
>  	u32 reg;
> +	int pixel_size;
>  
>  	switch (plane) {
>  	case 0:
> @@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		return -EINVAL;
>  	}
>  
> +	pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> @@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	dspcntr = I915_READ(reg);
>  	/* Mask out pixel format bits in case we change it */
>  	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
> +	dspcntr &= ~DISPPLANE_ROTATE_180;
> +
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> -	I915_WRITE(reg, dspcntr);
> -
>  	linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> @@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> +	if (intel_crtc->rotation == BIT(DRM_ROTATE_180)) {
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
> +		x += (fb->width - 1);
> +		y += (fb->height - 1);
> +		linear_offset += (fb->height - 1) * fb->pitches[0] +
> +                                   fb->width * pixel_size;
> +	}
> +
> +	I915_WRITE(reg, dspcntr);
> +
>  	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
>  		      i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
>  		      fb->pitches[0]);
> @@ -8748,6 +8761,31 @@ free_work:
>  	return ret;
>  }
>  
> +static int intel_crtc_set_property(struct drm_crtc *crtc,
> +				    struct drm_property *prop,
> +				    uint64_t val)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	uint64_t old_val;
> +	int ret = -ENOENT;
> +
> +	if (prop == dev_priv->rotation_property) {
> +		/* exactly one rotation angle please */
> +		if (hweight32(val & 0xf) != 1)
> +			return -EINVAL;
> +
> +		old_val = intel_crtc->rotation;
> +		intel_crtc->rotation = val;

Couple of other things we'd need to do here is check if the crtc is even
active, since calling .update_plane() on an inactive crtc would oops.

Also we need to wait for pending page flips to make sure we don't
overtake them.

Additionall the FBC code would need some rotation checks since FBC
doesn't work with a rotated plane on certain generations.

> +		ret = dev_priv->display.update_plane(crtc, crtc->fb, 0, 0);
> +		if (ret)
> +			intel_crtc->rotation = old_val;
> +	}
> +
> +	return ret;
> +}
> +
>  static struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
>  	.load_lut = intel_crtc_load_lut,
> @@ -10160,6 +10198,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = intel_crtc_set_config,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
> +	.set_property = intel_crtc_set_property
>  };
>  
>  static void intel_cpu_pll_init(struct drm_device *dev)
> @@ -10288,6 +10327,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	 */
>  	intel_crtc->pipe = pipe;
>  	intel_crtc->plane = pipe;
> +	intel_crtc->rotation = BIT(DRM_ROTATE_0);
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
>  		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
>  		intel_crtc->plane = !pipe;
> @@ -10298,6 +10338,16 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
>  
> +	if (!dev_priv->rotation_property)
> +		dev_priv->rotation_property =
> +			drm_mode_create_rotation_property(dev,
> +							BIT(DRM_ROTATE_0) |
> +							BIT(DRM_ROTATE_180));
> +	if (dev_priv->rotation_property)
> +		drm_object_attach_property(&intel_crtc->base.base,
> +					dev_priv->rotation_property,
> +					intel_crtc->rotation);
> +
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7a79b8e..02dfdb6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -331,6 +331,8 @@ struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
>  	enum plane plane;
> +	unsigned int rotation;
> +
>  	u8 lut_r[256], lut_g[256], lut_b[256];
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
> -- 
> 1.8.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/11] Enabling 180 degree rotation for sprite and crtc planes

[Daniel Vetter]

On Mon, Feb 03, 2014 at 11:59:05AM +0530, Sagar Arun Kamble wrote:
> On Fri, 2014-01-31 at 22:38 +0200, Ville Syrjälä wrote:
> > On Sat, Feb 01, 2014 at 12:40:36AM +0530, sagar.a.kamble@intel.com wrote:
> > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > 
> > > With these patches 180 degree rotation for crtc and sprite planes will be
> > > exposed through drm rotation properties implemented by Ville few days back.
> > > I have added CRTC rotation support on top of Ville's 9 patches that includes
> > > sprite rotation. Omapdrm changes, rotation_simplify, drm_rect functions are
> > > also included in this list. Functionality has been tested with i-g-t test
> > > "intel_plane_rotate".
> > > 
> > > 
> > > Sagar Kamble (11):
> > 
> > Somehow you've managed to change the authorship information of most of
> > the patches. Please don't do that.
> 
> Sorry for this mistake. I am new to this. I will send the revised
> patches for addressing the review comments.

git gui does by default change the author, git commit --amend doesn't. You
can fix this up with git commit --amend --author="..."
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[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

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

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

[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