[PATCH 0/7] drm/i915: Per-plane rotation, fixes, and mirroring for CHV

[PATCH 0/7] drm/i915: Per-plane rotation, fixes, and mirroring for CHV

[ville.syrjala]

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

Found some patches lying about that implement the per-plane rotation
property I've been going on about occasionally. There's also a fix
for atomic to reject invalid rotation angles. I also tossed in support
for horizontal mirroring on CHV, because I could.

Series available here:
git://github.com/vsyrjala/linux.git chv_mirror

Ville Syrjälä (7):
  drm: Add drm_rotation_90_or_270()
  drm/atomic: Reject attempts to use multiple rotation angles at once
  drm: Add support for optional per-plane rotation property
  drm/i915: Use the per-plane rotation property
  drm/i915: Use & instead if == to check for rotations
  drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
  drm/i915: Add horizontal mirroring support for CHV pipe B planes

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |   4 +-
 drivers/gpu/drm/drm_atomic.c                    |   8 +-
 drivers/gpu/drm/drm_atomic_helper.c             |   2 +-
 drivers/gpu/drm/drm_crtc.c                      |   3 +-
 drivers/gpu/drm/drm_fb_helper.c                 |   6 +-
 drivers/gpu/drm/i915/intel_atomic_plane.c       |  10 ++-
 drivers/gpu/drm/i915/intel_display.c            | 104 ++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h                |  10 +--
 drivers/gpu/drm/i915/intel_fbc.c                |   2 +-
 drivers/gpu/drm/i915/intel_pm.c                 |  12 +--
 drivers/gpu/drm/i915/intel_sprite.c             |  56 +++++++++----
 drivers/gpu/drm/omapdrm/omap_plane.c            |   8 +-
 include/drm/drm_crtc.h                          |   7 ++
 13 files changed, 140 insertions(+), 92 deletions(-)

-- 
2.7.4

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

[PATCH 1/7] drm: Add drm_rotation_90_or_270()

[ville.syrjala]

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

We have intel_rotation_90_or_270() in i915 to check if the rotation is
90 or 270 degrees. Similar checks are elsewhere in drm, so let's move
the helper into a central place and use it everwhere.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 ++--
 drivers/gpu/drm/drm_atomic_helper.c             |  2 +-
 drivers/gpu/drm/drm_crtc.c                      |  3 +--
 drivers/gpu/drm/i915/intel_atomic_plane.c       |  2 +-
 drivers/gpu/drm/i915/intel_display.c            | 10 +++++-----
 drivers/gpu/drm/i915/intel_drv.h                |  6 ------
 drivers/gpu/drm/i915/intel_fbc.c                |  2 +-
 drivers/gpu/drm/i915/intel_pm.c                 | 12 ++++++------
 drivers/gpu/drm/i915/intel_sprite.c             |  2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c            |  8 ++------
 include/drm/drm_crtc.h                          |  5 +++++
 11 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 016c191221f3..f3350c80704d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -393,7 +393,7 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
 
 	if ((state->base.fb->pixel_format == DRM_FORMAT_YUV422 ||
 	     state->base.fb->pixel_format == DRM_FORMAT_NV61) &&
-	    (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))))
+	    drm_rotation_90_or_270(state->base.rotation))
 		cfg |= ATMEL_HLCDC_YUV422ROT;
 
 	atmel_hlcdc_layer_update_cfg(&plane->layer,
@@ -628,7 +628,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
 	/*
 	 * Swap width and size in case of 90 or 270 degrees rotation
 	 */
-	if (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+	if (drm_rotation_90_or_270(state->base.rotation)) {
 		tmp = state->crtc_w;
 		state->crtc_w = state->crtc_h;
 		state->crtc_h = tmp;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index de7fddce3cef..3e12e6e646ad 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2348,7 +2348,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
 	primary_state->crtc_h = vdisplay;
 	primary_state->src_x = set->x << 16;
 	primary_state->src_y = set->y << 16;
-	if (primary_state->rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
+	if (drm_rotation_90_or_270(primary_state->rotation)) {
 		primary_state->src_w = vdisplay << 16;
 		primary_state->src_h = hdisplay << 16;
 	} else {
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f1d9f0569d7f..986de180e6c2 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2804,8 +2804,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
 	drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
 
 	if (crtc->state &&
-	    crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |
-					      BIT(DRM_ROTATE_270)))
+	    drm_rotation_90_or_270(crtc->primary->state->rotation))
 		swap(hdisplay, vdisplay);
 
 	return check_src_coords(x << 16, y << 16,
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7de7721f65bc..c19eb9a0cd4a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -156,7 +156,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.y2 =
 		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
 
-	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
+	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
 			DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 51fbca756cdb..08b9f9a19df0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2138,7 +2138,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
 			const struct drm_framebuffer *fb,
 			unsigned int rotation)
 {
-	if (intel_rotation_90_or_270(rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		*view = i915_ggtt_view_rotated;
 		view->params.rotated = to_intel_framebuffer(fb)->rot_info;
 	} else {
@@ -2352,7 +2352,7 @@ u32 intel_compute_tile_offset(int *x, int *y,
 		intel_tile_dims(dev_priv, &tile_width, &tile_height,
 				fb_modifier, cpp);
 
-		if (intel_rotation_90_or_270(rotation)) {
+		if (drm_rotation_90_or_270(rotation)) {
 			pitch_tiles = pitch / tile_height;
 			swap(tile_width, tile_height);
 		} else {
@@ -3011,7 +3011,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 
 	WARN_ON(drm_rect_width(&plane_state->src) == 0);
 
-	if (intel_rotation_90_or_270(rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
 
 		/* stride = Surface height in tiles */
@@ -4179,7 +4179,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		to_intel_crtc(crtc_state->base.crtc);
 	int need_scaling;
 
-	need_scaling = intel_rotation_90_or_270(rotation) ?
+	need_scaling = drm_rotation_90_or_270(rotation) ?
 		(src_h != dst_w || src_w != dst_h):
 		(src_w != dst_w || src_h != dst_h);
 
@@ -11414,7 +11414,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
 	 * The stride is either expressed as a multiple of 64 bytes chunks for
 	 * linear buffers or in number of tiles for tiled buffers.
 	 */
-	if (intel_rotation_90_or_270(rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		/* stride = Surface height in tiles */
 		tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
 		stride = DIV_ROUND_UP(fb->height, tile_height);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e74d851868c5..907a72cfdad3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,12 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
 			       uint64_t fb_modifier, unsigned int cpp);
 
-static inline bool
-intel_rotation_90_or_270(unsigned int rotation)
-{
-	return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
-}
-
 void intel_create_rotation_property(struct drm_device *dev,
 					struct intel_plane *plane);
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 781e2f5f7cd8..f3f3322139e0 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -84,7 +84,7 @@ static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache *cache,
 {
 	int w, h;
 
-	if (intel_rotation_90_or_270(cache->plane.rotation)) {
+	if (drm_rotation_90_or_270(cache->plane.rotation)) {
 		w = cache->plane.src_h;
 		h = cache->plane.src_w;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a1bf5f8fbb1c..53116cdc43a6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3004,7 +3004,7 @@ skl_plane_downscale_amount(const struct intel_plane_state *pstate)
 	src_h = drm_rect_height(&pstate->src);
 	dst_w = drm_rect_width(&pstate->dst);
 	dst_h = drm_rect_height(&pstate->dst);
-	if (intel_rotation_90_or_270(pstate->base.rotation))
+	if (drm_rotation_90_or_270(pstate->base.rotation))
 		swap(dst_w, dst_h);
 
 	downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
@@ -3035,7 +3035,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	width = drm_rect_width(&intel_pstate->src) >> 16;
 	height = drm_rect_height(&intel_pstate->src) >> 16;
 
-	if (intel_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(pstate->rotation))
 		swap(width, height);
 
 	/* for planar format */
@@ -3137,7 +3137,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	src_w = drm_rect_width(&intel_pstate->src) >> 16;
 	src_h = drm_rect_height(&intel_pstate->src) >> 16;
 
-	if (intel_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(pstate->rotation))
 		swap(src_w, src_h);
 
 	/* Halve UV plane width and height for NV12 */
@@ -3151,7 +3151,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	else
 		plane_bpp = drm_format_plane_cpp(fb->pixel_format, 0);
 
-	if (intel_rotation_90_or_270(pstate->rotation)) {
+	if (drm_rotation_90_or_270(pstate->rotation)) {
 		switch (plane_bpp) {
 		case 1:
 			min_scanlines = 32;
@@ -3407,7 +3407,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	width = drm_rect_width(&intel_pstate->src) >> 16;
 	height = drm_rect_height(&intel_pstate->src) >> 16;
 
-	if (intel_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(pstate->rotation))
 		swap(width, height);
 
 	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
@@ -3428,7 +3428,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
 		uint32_t min_scanlines = 4;
 		uint32_t y_tile_minimum;
-		if (intel_rotation_90_or_270(pstate->rotation)) {
+		if (drm_rotation_90_or_270(pstate->rotation)) {
 			int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
 				drm_format_plane_cpp(fb->pixel_format, 1) :
 				drm_format_plane_cpp(fb->pixel_format, 0);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0de935ad01c2..cc6af1410d67 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -251,7 +251,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 
 	surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
 
-	if (intel_rotation_90_or_270(rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
 
 		/* stride: Surface height in tiles */
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 5252ab720e70..7f0d567b8d67 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -108,16 +108,12 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	win.src_x = state->src_x >> 16;
 	win.src_y = state->src_y >> 16;
 
-	switch (state->rotation & DRM_ROTATE_MASK) {
-	case BIT(DRM_ROTATE_90):
-	case BIT(DRM_ROTATE_270):
+	if (drm_rotation_90_or_270(state->rotation)) {
 		win.src_w = state->src_h >> 16;
 		win.src_h = state->src_w >> 16;
-		break;
-	default:
+	} else {
 		win.src_w = state->src_w >> 16;
 		win.src_h = state->src_h >> 16;
-		break;
 	}
 
 	/* update scanout: */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3edeaf88ebc0..b7ac7dcf52db 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -92,6 +92,11 @@ static inline uint64_t I642U64(int64_t val)
 #define DRM_REFLECT_X	4
 #define DRM_REFLECT_Y	5
 
+static inline bool drm_rotation_90_or_270(unsigned int rotation)
+{
+	return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
+}
+
 enum drm_connector_force {
 	DRM_FORCE_UNSPECIFIED,
 	DRM_FORCE_OFF,
-- 
2.7.4

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

[PATCH 2/7] drm/atomic: Reject attempts to use multiple rotation angles at once

[ville.syrjala]

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

The rotation property should only accept exactly one rotation angle
at once. Let's reject attempts to set none or multiple angles.

Testcase: igt/kms_rotation_crc/bad-rotation
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3cee084e9d28..5f0002897862 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -711,6 +711,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
 	} else if (property == config->prop_src_h) {
 		state->src_h = val;
 	} else if (property == config->rotation_property) {
+		if (!is_power_of_2(val & DRM_ROTATE_MASK))
+			return -EINVAL;
 		state->rotation = val;
 	} else if (plane->funcs->atomic_set_property) {
 		return plane->funcs->atomic_set_property(plane, state,
-- 
2.7.4

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

[PATCH 3/7] drm: Add support for optional per-plane rotation property

[ville.syrjala]

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

Not all planes on the ssytem may support the same rotations/reflections,
so make it possible to create a separate property for each plane.
This way userspace gets told exactly which rotations/reflections are
possible for each plane.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_atomic.c    | 6 ++++--
 drivers/gpu/drm/drm_fb_helper.c | 6 +++++-
 include/drm/drm_crtc.h          | 2 ++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5f0002897862..38b023da6f09 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -710,7 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
 		state->src_w = val;
 	} else if (property == config->prop_src_h) {
 		state->src_h = val;
-	} else if (property == config->rotation_property) {
+	} else if (property == config->rotation_property ||
+		   property == plane->rotation_property) {
 		if (!is_power_of_2(val & DRM_ROTATE_MASK))
 			return -EINVAL;
 		state->rotation = val;
@@ -768,7 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 		*val = state->src_w;
 	} else if (property == config->prop_src_h) {
 		*val = state->src_h;
-	} else if (property == config->rotation_property) {
+	} else if (property == config->rotation_property ||
+		   property == plane->rotation_property) {
 		*val = state->rotation;
 	} else if (plane->funcs->atomic_get_property) {
 		return plane->funcs->atomic_get_property(plane, state, property, val);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ce54e985d91b..ce536c0553e5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
 
-		if (dev->mode_config.rotation_property) {
+		if (plane->rotation_property) {
+			drm_mode_plane_set_obj_prop(plane,
+						    plane->rotation_property,
+						    BIT(DRM_ROTATE_0));
+		} else if (dev->mode_config.rotation_property) {
 			drm_mode_plane_set_obj_prop(plane,
 						    dev->mode_config.rotation_property,
 						    BIT(DRM_ROTATE_0));
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b7ac7dcf52db..8d65e85d6d91 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1737,6 +1737,8 @@ struct drm_plane {
 	const struct drm_plane_helper_funcs *helper_private;
 
 	struct drm_plane_state *state;
+
+	struct drm_property *rotation_property;
 };
 
 /**
-- 
2.7.4

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

[PATCH 4/7] drm/i915: Use the per-plane rotation property

[ville.syrjala]

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

On certain platforms not all planes support the same set of
rotations/reflections, so let's use the per-plane property
for this.

This is already a problem on SKL when we use the legay cursor plane
as it only supports 0|180 whereas the universal planes support
0|90|180|270, and it will be a problem on CHV soon.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     |  4 +--
 drivers/gpu/drm/i915/intel_sprite.c  | 12 ++++++++-
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 08b9f9a19df0..93ecb259c5ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14217,6 +14217,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	struct intel_plane *primary = NULL;
 	struct intel_plane_state *state = NULL;
 	const uint32_t *intel_primary_formats;
+	unsigned int supported_rotations;
 	unsigned int num_formats;
 	int ret;
 
@@ -14289,8 +14290,19 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	if (INTEL_INFO(dev)->gen >= 9) {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+	} else if (INTEL_INFO(dev)->gen >= 4) {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
+	} else {
+		supported_rotations = BIT(DRM_ROTATE_0);
+	}
+
 	if (INTEL_INFO(dev)->gen >= 4)
-		intel_create_rotation_property(dev, primary);
+		intel_create_rotation_property(primary, supported_rotations);
 
 	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
@@ -14303,22 +14315,20 @@ fail:
 	return NULL;
 }
 
-void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
+void intel_create_rotation_property(struct intel_plane *plane,
+				    unsigned int supported_rotations)
 {
-	if (!dev->mode_config.rotation_property) {
-		unsigned long flags = BIT(DRM_ROTATE_0) |
-			BIT(DRM_ROTATE_180);
+	struct drm_device *dev = plane->base.dev;
 
-		if (INTEL_INFO(dev)->gen >= 9)
-			flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
+	if (!plane->base.rotation_property)
+		plane->base.rotation_property =
+			drm_mode_create_rotation_property(dev,
+							  supported_rotations);
 
-		dev->mode_config.rotation_property =
-			drm_mode_create_rotation_property(dev, flags);
-	}
-	if (dev->mode_config.rotation_property)
+	if (plane->base.rotation_property)
 		drm_object_attach_property(&plane->base.base,
-				dev->mode_config.rotation_property,
-				plane->base.state->rotation);
+					   plane->base.rotation_property,
+					   plane->base.state->rotation);
 }
 
 static int
@@ -14449,17 +14459,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	if (INTEL_INFO(dev)->gen >= 4) {
-		if (!dev->mode_config.rotation_property)
-			dev->mode_config.rotation_property =
-				drm_mode_create_rotation_property(dev,
-							BIT(DRM_ROTATE_0) |
-							BIT(DRM_ROTATE_180));
-		if (dev->mode_config.rotation_property)
-			drm_object_attach_property(&cursor->base.base,
-				dev->mode_config.rotation_property,
-				state->base.rotation);
-	}
+	if (INTEL_INFO(dev)->gen >= 4)
+		intel_create_rotation_property(cursor,
+					       BIT(DRM_ROTATE_0) |
+					       BIT(DRM_ROTATE_180));
 
 	if (INTEL_INFO(dev)->gen >=9)
 		state->scaler_id = -1;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 907a72cfdad3..2715aec979fc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,8 +1255,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
 			       uint64_t fb_modifier, unsigned int cpp);
 
-void intel_create_rotation_property(struct drm_device *dev,
-					struct intel_plane *plane);
+void intel_create_rotation_property(struct intel_plane *plane,
+				    unsigned int supported_rotations);
 
 void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
 				    enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cc6af1410d67..bc41c1bba04c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	struct intel_plane_state *state = NULL;
 	unsigned long possible_crtcs;
 	const uint32_t *plane_formats;
+	unsigned int supported_rotations;
 	int num_plane_formats;
 	int ret;
 
@@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		goto fail;
 	}
 
+	if (INTEL_INFO(dev)->gen >= 9) {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+	} else {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
+	}
+
 	intel_plane->pipe = pipe;
 	intel_plane->plane = plane;
 	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
@@ -1143,7 +1153,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 	if (ret)
 		goto fail;
 
-	intel_create_rotation_property(dev, intel_plane);
+	intel_create_rotation_property(intel_plane, supported_rotations);
 
 	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
-- 
2.7.4

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

[PATCH 5/7] drm/i915: Use & instead if == to check for rotations

[ville.syrjala]

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

Using == to check for 180 degree rotation only works as long as the
reflection bits aren't set. That will change soon enough for CHV, so
let's stop doing things the wrong way.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++----
 drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 93ecb259c5ce..79c1a8b89d1d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 		intel_crtc->dspaddr_offset = linear_offset;
 	}
 
-	if (rotation == BIT(DRM_ROTATE_180)) {
+	if (rotation & BIT(DRM_ROTATE_180)) {
 		dspcntr |= DISPPLANE_ROTATE_180;
 
 		x += (crtc_state->pipe_src_w - 1);
@@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 		intel_compute_tile_offset(&x, &y, fb, 0,
 					  fb->pitches[0], rotation);
 	linear_offset -= intel_crtc->dspaddr_offset;
-	if (rotation == BIT(DRM_ROTATE_180)) {
+
+	if (rotation & BIT(DRM_ROTATE_180)) {
 		dspcntr |= DISPPLANE_ROTATE_180;
 
 		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
@@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 		if (HAS_DDI(dev))
 			cntl |= CURSOR_PIPE_CSC_ENABLE;
 
-		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
+		if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
 			cntl |= CURSOR_ROTATE_180;
 	}
 
@@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 
 		/* ILK+ do this automagically */
 		if (HAS_GMCH_DISPLAY(dev) &&
-		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
+		    plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
 			base += (plane_state->base.crtc_h *
 				 plane_state->base.crtc_w - 1) * 4;
 		}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bc41c1bba04c..6b815d57d75a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,
 						   fb->pitches[0], rotation);
 	linear_offset -= sprsurf_offset;
 
-	if (rotation == BIT(DRM_ROTATE_180)) {
+	if (rotation & BIT(DRM_ROTATE_180)) {
 		sprctl |= SP_ROTATE_180;
 
 		x += src_w;
@@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,
 						   fb->pitches[0], rotation);
 	linear_offset -= sprsurf_offset;
 
-	if (rotation == BIT(DRM_ROTATE_180)) {
+	if (rotation & BIT(DRM_ROTATE_180)) {
 		sprctl |= SPRITE_ROTATE_180;
 
 		/* HSW and BDW does this automagically in hardware */
@@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,
 						   fb->pitches[0], rotation);
 	linear_offset -= dvssurf_offset;
 
-	if (rotation == BIT(DRM_ROTATE_180)) {
+	if (rotation & BIT(DRM_ROTATE_180)) {
 		dvscntr |= DVS_ROTATE_180;
 
 		x += src_w;
-- 
2.7.4

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

[PATCH 6/7] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup

[ville.syrjala]

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

Move the plane control register rotation setup away from the
coordinate munging code. This will result in neater looking
code once we add reflection support for CHV.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++--------------
 drivers/gpu/drm/i915/intel_sprite.c  | 28 +++++++++++++++-------------
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 79c1a8b89d1d..88a7c4173715 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2674,6 +2674,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	    obj->tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
 
+	if (rotation & BIT(DRM_ROTATE_180))
+		dspcntr |= DISPPLANE_ROTATE_180;
+
 	if (IS_G4X(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
@@ -2689,8 +2692,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	}
 
 	if (rotation & BIT(DRM_ROTATE_180)) {
-		dspcntr |= DISPPLANE_ROTATE_180;
-
 		x += (crtc_state->pipe_src_w - 1);
 		y += (crtc_state->pipe_src_h - 1);
 
@@ -2783,6 +2784,9 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	if (obj->tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
 
+	if (rotation & BIT(DRM_ROTATE_180))
+		dspcntr |= DISPPLANE_ROTATE_180;
+
 	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
@@ -2792,19 +2796,15 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 					  fb->pitches[0], rotation);
 	linear_offset -= intel_crtc->dspaddr_offset;
 
-	if (rotation & BIT(DRM_ROTATE_180)) {
-		dspcntr |= DISPPLANE_ROTATE_180;
-
-		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
-			x += (crtc_state->pipe_src_w - 1);
-			y += (crtc_state->pipe_src_h - 1);
+	/* HSW and BDW does this automagically in hardware */
+	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
+	    rotation & BIT(DRM_ROTATE_180)) {
+		x += (crtc_state->pipe_src_w - 1);
+		y += (crtc_state->pipe_src_h - 1);
 
-			/* Finding the last pixel of the last line of the display
-			data and adding to linear_offset*/
-			linear_offset +=
-				(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
-				(crtc_state->pipe_src_w - 1) * cpp;
-		}
+		linear_offset +=
+			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
+			(crtc_state->pipe_src_w - 1) * cpp;
 	}
 
 	intel_crtc->adjusted_x = x;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6b815d57d75a..14173f53f520 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -433,6 +433,9 @@ vlv_update_plane(struct drm_plane *dplane,
 	if (obj->tiling_mode != I915_TILING_NONE)
 		sprctl |= SP_TILED;
 
+	if (rotation & BIT(DRM_ROTATE_180))
+		sprctl |= SP_ROTATE_180;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -445,8 +448,6 @@ vlv_update_plane(struct drm_plane *dplane,
 	linear_offset -= sprsurf_offset;
 
 	if (rotation & BIT(DRM_ROTATE_180)) {
-		sprctl |= SP_ROTATE_180;
-
 		x += src_w;
 		y += src_h;
 		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
@@ -555,6 +556,9 @@ ivb_update_plane(struct drm_plane *plane,
 	if (obj->tiling_mode != I915_TILING_NONE)
 		sprctl |= SPRITE_TILED;
 
+	if (rotation & BIT(DRM_ROTATE_180))
+		sprctl |= SPRITE_ROTATE_180;
+
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
 	else
@@ -577,15 +581,12 @@ ivb_update_plane(struct drm_plane *plane,
 						   fb->pitches[0], rotation);
 	linear_offset -= sprsurf_offset;
 
-	if (rotation & BIT(DRM_ROTATE_180)) {
-		sprctl |= SPRITE_ROTATE_180;
-
-		/* HSW and BDW does this automagically in hardware */
-		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
-			x += src_w;
-			y += src_h;
-			linear_offset += src_h * fb->pitches[0] + src_w * cpp;
-		}
+	/* HSW and BDW does this automagically in hardware */
+	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
+	    rotation & BIT(DRM_ROTATE_180)) {
+		x += src_w;
+		y += src_h;
+		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
 	}
 
 	if (key->flags) {
@@ -696,6 +697,9 @@ ilk_update_plane(struct drm_plane *plane,
 	if (obj->tiling_mode != I915_TILING_NONE)
 		dvscntr |= DVS_TILED;
 
+	if (rotation & BIT(DRM_ROTATE_180))
+		dvscntr |= DVS_ROTATE_180;
+
 	if (IS_GEN6(dev))
 		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 
@@ -715,8 +719,6 @@ ilk_update_plane(struct drm_plane *plane,
 	linear_offset -= dvssurf_offset;
 
 	if (rotation & BIT(DRM_ROTATE_180)) {
-		dvscntr |= DVS_ROTATE_180;
-
 		x += src_w;
 		y += src_h;
 		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
-- 
2.7.4

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

[PATCH 7/7] drm/i915: Add horizontal mirroring support for CHV pipe B planes

[ville.syrjala]

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

The primary and sprite planes on CHV pipe B support horizontal
mirroring. Expose it to the world.

Sadly the hardware ignores the mirror bit when the rotate bit is
set, so we'll have to reject the 180+X case.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  8 ++++++++
 drivers/gpu/drm/i915/intel_display.c      | 10 ++++++++++
 drivers/gpu/drm/i915/intel_sprite.c       | 10 ++++++++++
 3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index c19eb9a0cd4a..0a019eacfede 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -180,6 +180,14 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 		}
 	}
 
+	/* CHV ignores the mirror bit when the rotate bit is set :( */
+	if (IS_CHERRYVIEW(plane->dev) &&
+	    state->rotation & BIT(DRM_ROTATE_180) &&
+	    state->rotation & BIT(DRM_REFLECT_X)) {
+		DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
+		return -EINVAL;
+	}
+
 	intel_state->visible = false;
 	ret = intel_plane->check_plane(plane, crtc_state, intel_state);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 88a7c4173715..edb1809a642d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2677,6 +2677,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	if (rotation & BIT(DRM_ROTATE_180))
 		dspcntr |= DISPPLANE_ROTATE_180;
 
+	if (rotation & BIT(DRM_REFLECT_X))
+		dspcntr |= DISPPLANE_MIRROR;
+
 	if (IS_G4X(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
@@ -2700,6 +2703,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 		linear_offset +=
 			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
 			(crtc_state->pipe_src_w - 1) * cpp;
+	} else if (rotation & BIT(DRM_REFLECT_X)) {
+		x += (crtc_state->pipe_src_w - 1);
+		linear_offset += (crtc_state->pipe_src_w - 1) * cpp;
 	}
 
 	intel_crtc->adjusted_x = x;
@@ -14295,6 +14301,10 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 		supported_rotations =
 			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
 			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+	} else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
+			BIT(DRM_REFLECT_X);
 	} else if (INTEL_INFO(dev)->gen >= 4) {
 		supported_rotations =
 			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 14173f53f520..4d6cd1a02e34 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -436,6 +436,9 @@ vlv_update_plane(struct drm_plane *dplane,
 	if (rotation & BIT(DRM_ROTATE_180))
 		sprctl |= SP_ROTATE_180;
 
+	if (rotation & BIT(DRM_REFLECT_X))
+		sprctl |= SP_MIRROR;
+
 	/* Sizes are 0 based */
 	src_w--;
 	src_h--;
@@ -451,6 +454,9 @@ vlv_update_plane(struct drm_plane *dplane,
 		x += src_w;
 		y += src_h;
 		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
+	} else if (rotation & BIT(DRM_REFLECT_X)) {
+		x += src_w;
+		linear_offset += src_w * cpp;
 	}
 
 	if (key->flags) {
@@ -1128,6 +1134,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		supported_rotations =
 			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
 			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
+	} else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
+		supported_rotations =
+			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
+			BIT(DRM_REFLECT_X);
 	} else {
 		supported_rotations =
 			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
-- 
2.7.4

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

[PATCH i-g-t] tests/kms_rotation_crc: Add bad-rotation subtest

[ville.syrjala]

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

Add "bad-rotation" subtest to make sure the kernel rejects some
invalid rotation values (0 and specifying multiple angles at one).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_rotation_crc.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 6cc15337fff9..e10a0a770437 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -237,6 +237,31 @@ static void wait_for_pageflip(int fd)
 	igt_assert(drmHandleEvent(fd, &evctx) == 0);
 }
 
+static void test_bad_prop_value(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	int valid_tests = 0;
+	enum pipe pipe;
+	igt_plane_t *plane;
+	int ret;
+
+	for_each_pipe(display, pipe)  {
+		for_each_plane_on_pipe(display, pipe, plane) {
+			igt_require(igt_plane_supports_rotation(plane));
+
+			ret = drmModeObjectSetProperty(display->drm_fd,
+						       plane->drm_plane->plane_id,
+						       DRM_MODE_OBJECT_PLANE,
+						       plane->rotation_property,
+						       data->rotation);
+
+			igt_assert_eq(ret, -EINVAL);
+			valid_tests++;
+		}
+	}
+	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
 static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
 {
 	igt_display_t *display = &data->display;
@@ -508,6 +533,14 @@ igt_main
 
 		igt_display_init(&data.display, data.gfx_fd);
 	}
+	igt_subtest_f("bad-rotation") {
+		data.rotation = 0;
+		test_bad_prop_value(&data);
+
+		data.rotation = IGT_ROTATION_0 | IGT_ROTATION_180;
+		test_bad_prop_value(&data);
+	}
+
 	igt_subtest_f("primary-rotation-180") {
 		data.rotation = IGT_ROTATION_180;
 		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
-- 
2.7.4

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

Re: [PATCH 1/7] drm: Add drm_rotation_90_or_270()

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have intel_rotation_90_or_270() in i915 to check if the rotation is
> 90 or 270 degrees. Similar checks are elsewhere in drm, so let's move
> the helper into a central place and use it everwhere.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 ++--
>  drivers/gpu/drm/drm_atomic_helper.c             |  2 +-
>  drivers/gpu/drm/drm_crtc.c                      |  3 +--
>  drivers/gpu/drm/i915/intel_atomic_plane.c       |  2 +-
>  drivers/gpu/drm/i915/intel_display.c            | 10 +++++-----
>  drivers/gpu/drm/i915/intel_drv.h                |  6 ------
>  drivers/gpu/drm/i915/intel_fbc.c                |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c                 | 12 ++++++------
>  drivers/gpu/drm/i915/intel_sprite.c             |  2 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c            |  8 ++------
>  include/drm/drm_crtc.h                          |  5 +++++
>  11 files changed, 25 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 016c191221f3..f3350c80704d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -393,7 +393,7 @@ static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
>  
>  	if ((state->base.fb->pixel_format == DRM_FORMAT_YUV422 ||
>  	     state->base.fb->pixel_format == DRM_FORMAT_NV61) &&
> -	    (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))))
> +	    drm_rotation_90_or_270(state->base.rotation))
>  		cfg |= ATMEL_HLCDC_YUV422ROT;
>  
>  	atmel_hlcdc_layer_update_cfg(&plane->layer,
> @@ -628,7 +628,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
>  	/*
>  	 * Swap width and size in case of 90 or 270 degrees rotation
>  	 */
> -	if (state->base.rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
> +	if (drm_rotation_90_or_270(state->base.rotation)) {
>  		tmp = state->crtc_w;
>  		state->crtc_w = state->crtc_h;
>  		state->crtc_h = tmp;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index de7fddce3cef..3e12e6e646ad 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2348,7 +2348,7 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>  	primary_state->crtc_h = vdisplay;
>  	primary_state->src_x = set->x << 16;
>  	primary_state->src_y = set->y << 16;
> -	if (primary_state->rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270))) {
> +	if (drm_rotation_90_or_270(primary_state->rotation)) {
>  		primary_state->src_w = vdisplay << 16;
>  		primary_state->src_h = hdisplay << 16;
>  	} else {
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f1d9f0569d7f..986de180e6c2 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2804,8 +2804,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
>  	drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
>  
>  	if (crtc->state &&
> -	    crtc->primary->state->rotation & (BIT(DRM_ROTATE_90) |
> -					      BIT(DRM_ROTATE_270)))
> +	    drm_rotation_90_or_270(crtc->primary->state->rotation))
>  		swap(hdisplay, vdisplay);
>  
>  	return check_src_coords(x << 16, y << 16,
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7de7721f65bc..c19eb9a0cd4a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -156,7 +156,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  	intel_state->clip.y2 =
>  		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
>  
> -	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
> +	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
>  		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
>  			DRM_DEBUG_KMS("Y/Yf tiling required for 90/270!\n");
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 51fbca756cdb..08b9f9a19df0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2138,7 +2138,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
>  			const struct drm_framebuffer *fb,
>  			unsigned int rotation)
>  {
> -	if (intel_rotation_90_or_270(rotation)) {
> +	if (drm_rotation_90_or_270(rotation)) {
>  		*view = i915_ggtt_view_rotated;
>  		view->params.rotated = to_intel_framebuffer(fb)->rot_info;
>  	} else {
> @@ -2352,7 +2352,7 @@ u32 intel_compute_tile_offset(int *x, int *y,
>  		intel_tile_dims(dev_priv, &tile_width, &tile_height,
>  				fb_modifier, cpp);
>  
> -		if (intel_rotation_90_or_270(rotation)) {
> +		if (drm_rotation_90_or_270(rotation)) {
>  			pitch_tiles = pitch / tile_height;
>  			swap(tile_width, tile_height);
>  		} else {
> @@ -3011,7 +3011,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
>  
>  	WARN_ON(drm_rect_width(&plane_state->src) == 0);
>  
> -	if (intel_rotation_90_or_270(rotation)) {
> +	if (drm_rotation_90_or_270(rotation)) {
>  		int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
>  
>  		/* stride = Surface height in tiles */
> @@ -4179,7 +4179,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  		to_intel_crtc(crtc_state->base.crtc);
>  	int need_scaling;
>  
> -	need_scaling = intel_rotation_90_or_270(rotation) ?
> +	need_scaling = drm_rotation_90_or_270(rotation) ?
>  		(src_h != dst_w || src_w != dst_h):
>  		(src_w != dst_w || src_h != dst_h);
>  
> @@ -11414,7 +11414,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
>  	 * The stride is either expressed as a multiple of 64 bytes chunks for
>  	 * linear buffers or in number of tiles for tiled buffers.
>  	 */
> -	if (intel_rotation_90_or_270(rotation)) {
> +	if (drm_rotation_90_or_270(rotation)) {
>  		/* stride = Surface height in tiles */
>  		tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
>  		stride = DIV_ROUND_UP(fb->height, tile_height);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e74d851868c5..907a72cfdad3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,12 +1255,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
>  			       uint64_t fb_modifier, unsigned int cpp);
>  
> -static inline bool
> -intel_rotation_90_or_270(unsigned int rotation)
> -{
> -	return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
> -}
> -
>  void intel_create_rotation_property(struct drm_device *dev,
>  					struct intel_plane *plane);
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 781e2f5f7cd8..f3f3322139e0 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -84,7 +84,7 @@ static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache *cache,
>  {
>  	int w, h;
>  
> -	if (intel_rotation_90_or_270(cache->plane.rotation)) {
> +	if (drm_rotation_90_or_270(cache->plane.rotation)) {
>  		w = cache->plane.src_h;
>  		h = cache->plane.src_w;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a1bf5f8fbb1c..53116cdc43a6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3004,7 +3004,7 @@ skl_plane_downscale_amount(const struct intel_plane_state *pstate)
>  	src_h = drm_rect_height(&pstate->src);
>  	dst_w = drm_rect_width(&pstate->dst);
>  	dst_h = drm_rect_height(&pstate->dst);
> -	if (intel_rotation_90_or_270(pstate->base.rotation))
> +	if (drm_rotation_90_or_270(pstate->base.rotation))
>  		swap(dst_w, dst_h);
>  
>  	downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
> @@ -3035,7 +3035,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  	width = drm_rect_width(&intel_pstate->src) >> 16;
>  	height = drm_rect_height(&intel_pstate->src) >> 16;
>  
> -	if (intel_rotation_90_or_270(pstate->rotation))
> +	if (drm_rotation_90_or_270(pstate->rotation))
>  		swap(width, height);
>  
>  	/* for planar format */
> @@ -3137,7 +3137,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>  	src_w = drm_rect_width(&intel_pstate->src) >> 16;
>  	src_h = drm_rect_height(&intel_pstate->src) >> 16;
>  
> -	if (intel_rotation_90_or_270(pstate->rotation))
> +	if (drm_rotation_90_or_270(pstate->rotation))
>  		swap(src_w, src_h);
>  
>  	/* Halve UV plane width and height for NV12 */
> @@ -3151,7 +3151,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>  	else
>  		plane_bpp = drm_format_plane_cpp(fb->pixel_format, 0);
>  
> -	if (intel_rotation_90_or_270(pstate->rotation)) {
> +	if (drm_rotation_90_or_270(pstate->rotation)) {
>  		switch (plane_bpp) {
>  		case 1:
>  			min_scanlines = 32;
> @@ -3407,7 +3407,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	width = drm_rect_width(&intel_pstate->src) >> 16;
>  	height = drm_rect_height(&intel_pstate->src) >> 16;
>  
> -	if (intel_rotation_90_or_270(pstate->rotation))
> +	if (drm_rotation_90_or_270(pstate->rotation))
>  		swap(width, height);
>  
>  	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> @@ -3428,7 +3428,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
>  		uint32_t min_scanlines = 4;
>  		uint32_t y_tile_minimum;
> -		if (intel_rotation_90_or_270(pstate->rotation)) {
> +		if (drm_rotation_90_or_270(pstate->rotation)) {
>  			int cpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
>  				drm_format_plane_cpp(fb->pixel_format, 1) :
>  				drm_format_plane_cpp(fb->pixel_format, 0);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0de935ad01c2..cc6af1410d67 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -251,7 +251,7 @@ skl_update_plane(struct drm_plane *drm_plane,
>  
>  	surf_addr = intel_plane_obj_offset(intel_plane, obj, 0);
>  
> -	if (intel_rotation_90_or_270(rotation)) {
> +	if (drm_rotation_90_or_270(rotation)) {
>  		int cpp = drm_format_plane_cpp(fb->pixel_format, 0);
>  
>  		/* stride: Surface height in tiles */
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 5252ab720e70..7f0d567b8d67 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -108,16 +108,12 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
>  	win.src_x = state->src_x >> 16;
>  	win.src_y = state->src_y >> 16;
>  
> -	switch (state->rotation & DRM_ROTATE_MASK) {
> -	case BIT(DRM_ROTATE_90):
> -	case BIT(DRM_ROTATE_270):
> +	if (drm_rotation_90_or_270(state->rotation)) {
>  		win.src_w = state->src_h >> 16;
>  		win.src_h = state->src_w >> 16;
> -		break;
> -	default:
> +	} else {
>  		win.src_w = state->src_w >> 16;
>  		win.src_h = state->src_h >> 16;
> -		break;
>  	}
>  
>  	/* update scanout: */
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3edeaf88ebc0..b7ac7dcf52db 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -92,6 +92,11 @@ static inline uint64_t I642U64(int64_t val)
>  #define DRM_REFLECT_X	4
>  #define DRM_REFLECT_Y	5
>  
> +static inline bool drm_rotation_90_or_270(unsigned int rotation)
> +{
> +	return rotation & (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270));
> +}
> +
>  enum drm_connector_force {
>  	DRM_FORCE_UNSPECIFIED,
>  	DRM_FORCE_OFF,
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/7] drm/atomic: Reject attempts to use multiple rotation angles at once

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The rotation property should only accept exactly one rotation angle
> at once. Let's reject attempts to set none or multiple angles.
> 
> Testcase: igt/kms_rotation_crc/bad-rotation
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/drm_atomic.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3cee084e9d28..5f0002897862 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -711,6 +711,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  	} else if (property == config->prop_src_h) {
>  		state->src_h = val;
>  	} else if (property == config->rotation_property) {
> +		if (!is_power_of_2(val & DRM_ROTATE_MASK))
> +			return -EINVAL;
>  		state->rotation = val;
>  	} else if (plane->funcs->atomic_set_property) {
>  		return plane->funcs->atomic_set_property(plane, state,
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/7] drm/atomic: Reject attempts to use multiple rotation angles at once

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:07PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The rotation property should only accept exactly one rotation angle
> at once. Let's reject attempts to set none or multiple angles.
> 
> Testcase: igt/kms_rotation_crc/bad-rotation
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/7] drm: Add support for optional per-plane rotation property

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:08PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not all planes on the ssytem may support the same rotations/reflections,
> so make it possible to create a separate property for each plane.
> This way userspace gets told exactly which rotations/reflections are
> possible for each plane.

Hmm, can't see that it would make much difference here, but I was
expecting something like

drm_plane_lookup_rotation_propery()
{
	return plane->rotation_property ?: plane->dev->mode_config.rotation_property;
}

so that it is obvious that the plane takes precedence over the global
property when provided.

You've caught all the spots I can see so,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/i915: Use the per-plane rotation property

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:09PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On certain platforms not all planes support the same set of
> rotations/reflections, so let's use the per-plane property
> for this.
> 
> This is already a problem on SKL when we use the legay cursor plane
> as it only supports 0|180 whereas the universal planes support
> 0|90|180|270, and it will be a problem on CHV soon.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/7] drm/i915: Use & instead if == to check for rotations

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:10PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Using == to check for 180 degree rotation only works as long as the
> reflection bits aren't set. That will change soon enough for CHV, so
> let's stop doing things the wrong way.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:11PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the plane control register rotation setup away from the
> coordinate munging code. This will result in neater looking
> code once we add reflection support for CHV.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/7] drm: Add drm_rotation_90_or_270()

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:18:06PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have intel_rotation_90_or_270() in i915 to check if the rotation is
> 90 or 270 degrees. Similar checks are elsewhere in drm, so let's move
> the helper into a central place and use it everwhere.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I didn't check for any other candidates though.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm: Add support for optional per-plane rotation property

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not all planes on the ssytem may support the same rotations/reflections,

s/ssytem/system/

> so make it possible to create a separate property for each plane.
> This way userspace gets told exactly which rotations/reflections are
> possible for each plane.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c    | 6 ++++--
>  drivers/gpu/drm/drm_fb_helper.c | 6 +++++-
>  include/drm/drm_crtc.h          | 2 ++
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5f0002897862..38b023da6f09 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -710,7 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
>  		state->src_w = val;
>  	} else if (property == config->prop_src_h) {
>  		state->src_h = val;
> -	} else if (property == config->rotation_property) {
> +	} else if (property == config->rotation_property ||
> +		   property == plane->rotation_property) {
>  		if (!is_power_of_2(val & DRM_ROTATE_MASK))
>  			return -EINVAL;
>  		state->rotation = val;
> @@ -768,7 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>  		*val = state->src_w;
>  	} else if (property == config->prop_src_h) {
>  		*val = state->src_h;
> -	} else if (property == config->rotation_property) {
> +	} else if (property == config->rotation_property ||
> +		   property == plane->rotation_property) {
>  		*val = state->rotation;
>  	} else if (plane->funcs->atomic_get_property) {
>  		return plane->funcs->atomic_get_property(plane, state, property, val);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ce54e985d91b..ce536c0553e5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
>  		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>  			drm_plane_force_disable(plane);
>  
> -		if (dev->mode_config.rotation_property) {
> +		if (plane->rotation_property) {
> +			drm_mode_plane_set_obj_prop(plane,
> +						    plane->rotation_property,
> +						    BIT(DRM_ROTATE_0));
> +		} else if (dev->mode_config.rotation_property) {

Why not clear the mode_config.rotation_property too, still?

Regards, Joonas

>  			drm_mode_plane_set_obj_prop(plane,
>  						    dev->mode_config.rotation_property,
>  						    BIT(DRM_ROTATE_0));
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b7ac7dcf52db..8d65e85d6d91 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1737,6 +1737,8 @@ struct drm_plane {
>  	const struct drm_plane_helper_funcs *helper_private;
>  
>  	struct drm_plane_state *state;
> +
> +	struct drm_property *rotation_property;
>  };
>  
>  /**
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Use the per-plane rotation property

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On certain platforms not all planes support the same set of
> rotations/reflections, so let's use the per-plane property
> for this.
> 
> This is already a problem on SKL when we use the legay cursor plane
> as it only supports 0|180 whereas the universal planes support
> 0|90|180|270, and it will be a problem on CHV soon.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +--
>  drivers/gpu/drm/i915/intel_sprite.c  | 12 ++++++++-
>  3 files changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 08b9f9a19df0..93ecb259c5ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14217,6 +14217,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	struct intel_plane *primary = NULL;
>  	struct intel_plane_state *state = NULL;
>  	const uint32_t *intel_primary_formats;
> +	unsigned int supported_rotations;
>  	unsigned int num_formats;
>  	int ret;
>  
> @@ -14289,8 +14290,19 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> +			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> +	} else if (INTEL_INFO(dev)->gen >= 4) {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> +	} else {
> +		supported_rotations = BIT(DRM_ROTATE_0);
> +	}
> +

Might this be more informative if it was;

supported_rotations = BIT(DRM_ROTATE_0);

if (gen >= 4)
	supported_rotations |= BIT(DRM_ROTATE_180);

Then you could exclude too, for special platforms.

Also, use INTEL_GEN()

>  	if (INTEL_INFO(dev)->gen >= 4)
> -		intel_create_rotation_property(dev, primary);
> +		intel_create_rotation_property(primary, supported_rotations);
>  
>  	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
>  
> @@ -14303,22 +14315,20 @@ fail:
>  	return NULL;
>  }
>  
> -void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
> +void intel_create_rotation_property(struct intel_plane *plane,
> +				    unsigned int supported_rotations)
>  {
> -	if (!dev->mode_config.rotation_property) {
> -		unsigned long flags = BIT(DRM_ROTATE_0) |
> -			BIT(DRM_ROTATE_180);
> +	struct drm_device *dev = plane->base.dev;
>  
> -		if (INTEL_INFO(dev)->gen >= 9)
> -			flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
> +	if (!plane->base.rotation_property)
> +		plane->base.rotation_property =
> +			drm_mode_create_rotation_property(dev,
> +							  supported_rotations);
>  
> -		dev->mode_config.rotation_property =
> -			drm_mode_create_rotation_property(dev, flags);
> -	}
> -	if (dev->mode_config.rotation_property)
> +	if (plane->base.rotation_property)
>  		drm_object_attach_property(&plane->base.base,
> -				dev->mode_config.rotation_property,
> -				plane->base.state->rotation);
> +					   plane->base.rotation_property,
> +					   plane->base.state->rotation);
>  }
>  
>  static int
> @@ -14449,17 +14459,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> -	if (INTEL_INFO(dev)->gen >= 4) {
> -		if (!dev->mode_config.rotation_property)
> -			dev->mode_config.rotation_property =
> -				drm_mode_create_rotation_property(dev,
> -							BIT(DRM_ROTATE_0) |
> -							BIT(DRM_ROTATE_180));
> -		if (dev->mode_config.rotation_property)
> -			drm_object_attach_property(&cursor->base.base,
> -				dev->mode_config.rotation_property,
> -				state->base.rotation);
> -	}
> +	if (INTEL_INFO(dev)->gen >= 4)

INTEL_GEN() while touching it.

> +		intel_create_rotation_property(cursor,
> +					       BIT(DRM_ROTATE_0) |
> +					       BIT(DRM_ROTATE_180));
>  
>  	if (INTEL_INFO(dev)->gen >=9)
>  		state->scaler_id = -1;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 907a72cfdad3..2715aec979fc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,8 +1255,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
>  			       uint64_t fb_modifier, unsigned int cpp);
>  
> -void intel_create_rotation_property(struct drm_device *dev,
> -					struct intel_plane *plane);
> +void intel_create_rotation_property(struct intel_plane *plane,
> +				    unsigned int supported_rotations);
>  
>  void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
>  				    enum pipe pipe);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index cc6af1410d67..bc41c1bba04c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  	struct intel_plane_state *state = NULL;
>  	unsigned long possible_crtcs;
>  	const uint32_t *plane_formats;
> +	unsigned int supported_rotations;
>  	int num_plane_formats;
>  	int ret;
>  
> @@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		goto fail;
>  	}
>  
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> +			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> +	} else {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> +	}
> +

Same here, I'd put the |='s in order so that new gens add features,
making easier to get a picture of what was added at which point.

Regards, Joonas

>  	intel_plane->pipe = pipe;
>  	intel_plane->plane = plane;
>  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> @@ -1143,7 +1153,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  	if (ret)
>  		goto fail;
>  
> -	intel_create_rotation_property(dev, intel_plane);
> +	intel_create_rotation_property(intel_plane, supported_rotations);
>  
>  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm/i915: Use & instead if == to check for rotations

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Using == to check for 180 degree rotation only works as long as the
> reflection bits aren't set. That will change soon enough for CHV, so
> let's stop doing things the wrong way.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reading through the patch makes me wanna do the change;

#define DRM_ROTATE_180 BIT(x)

even more. Maybe with coccinelle.

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++----
>  drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 93ecb259c5ce..79c1a8b89d1d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  		intel_crtc->dspaddr_offset = linear_offset;
>  	}
>  
> -	if (rotation == BIT(DRM_ROTATE_180)) {
> +	if (rotation & BIT(DRM_ROTATE_180)) {
>  		dspcntr |= DISPPLANE_ROTATE_180;
>  
>  		x += (crtc_state->pipe_src_w - 1);
> @@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>  		intel_compute_tile_offset(&x, &y, fb, 0,
>  					  fb->pitches[0], rotation);
>  	linear_offset -= intel_crtc->dspaddr_offset;
> -	if (rotation == BIT(DRM_ROTATE_180)) {
> +
> +	if (rotation & BIT(DRM_ROTATE_180)) {
>  		dspcntr |= DISPPLANE_ROTATE_180;
>  
>  		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> @@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
>  		if (HAS_DDI(dev))
>  			cntl |= CURSOR_PIPE_CSC_ENABLE;
>  
> -		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
> +		if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
>  			cntl |= CURSOR_ROTATE_180;
>  	}
>  
> @@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
>  
>  		/* ILK+ do this automagically */
>  		if (HAS_GMCH_DISPLAY(dev) &&
> -		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> +		    plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
>  			base += (plane_state->base.crtc_h *
>  				 plane_state->base.crtc_w - 1) * 4;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index bc41c1bba04c..6b815d57d75a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,
>  						   fb->pitches[0], rotation);
>  	linear_offset -= sprsurf_offset;
>  
> -	if (rotation == BIT(DRM_ROTATE_180)) {
> +	if (rotation & BIT(DRM_ROTATE_180)) {
>  		sprctl |= SP_ROTATE_180;
>  
>  		x += src_w;
> @@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,
>  						   fb->pitches[0], rotation);
>  	linear_offset -= sprsurf_offset;
>  
> -	if (rotation == BIT(DRM_ROTATE_180)) {
> +	if (rotation & BIT(DRM_ROTATE_180)) {
>  		sprctl |= SPRITE_ROTATE_180;
>  
>  		/* HSW and BDW does this automagically in hardware */
> @@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,
>  						   fb->pitches[0], rotation);
>  	linear_offset -= dvssurf_offset;
>  
> -	if (rotation == BIT(DRM_ROTATE_180)) {
> +	if (rotation & BIT(DRM_ROTATE_180)) {
>  		dvscntr |= DVS_ROTATE_180;
>  
>  		x += src_w;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the plane control register rotation setup away from the
> coordinate munging code. This will result in neater looking
> code once we add reflection support for CHV.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks cleaner,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_sprite.c  | 28 +++++++++++++++-------------
>  2 files changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 79c1a8b89d1d..88a7c4173715 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2674,6 +2674,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	    obj->tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
>  
> +	if (rotation & BIT(DRM_ROTATE_180))
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> @@ -2689,8 +2692,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	}
>  
>  	if (rotation & BIT(DRM_ROTATE_180)) {
> -		dspcntr |= DISPPLANE_ROTATE_180;
> -
>  		x += (crtc_state->pipe_src_w - 1);
>  		y += (crtc_state->pipe_src_h - 1);
>  
> @@ -2783,6 +2784,9 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>  	if (obj->tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
>  
> +	if (rotation & BIT(DRM_ROTATE_180))
> +		dspcntr |= DISPPLANE_ROTATE_180;
> +
>  	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> @@ -2792,19 +2796,15 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>  					  fb->pitches[0], rotation);
>  	linear_offset -= intel_crtc->dspaddr_offset;
>  
> -	if (rotation & BIT(DRM_ROTATE_180)) {
> -		dspcntr |= DISPPLANE_ROTATE_180;
> -
> -		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> -			x += (crtc_state->pipe_src_w - 1);
> -			y += (crtc_state->pipe_src_h - 1);
> +	/* HSW and BDW does this automagically in hardware */
> +	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
> +	    rotation & BIT(DRM_ROTATE_180)) {
> +		x += (crtc_state->pipe_src_w - 1);
> +		y += (crtc_state->pipe_src_h - 1);
>  
> -			/* Finding the last pixel of the last line of the display
> -			data and adding to linear_offset*/
> -			linear_offset +=
> -				(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> -				(crtc_state->pipe_src_w - 1) * cpp;
> -		}
> +		linear_offset +=
> +			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
> +			(crtc_state->pipe_src_w - 1) * cpp;
>  	}
>  
>  	intel_crtc->adjusted_x = x;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 6b815d57d75a..14173f53f520 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -433,6 +433,9 @@ vlv_update_plane(struct drm_plane *dplane,
>  	if (obj->tiling_mode != I915_TILING_NONE)
>  		sprctl |= SP_TILED;
>  
> +	if (rotation & BIT(DRM_ROTATE_180))
> +		sprctl |= SP_ROTATE_180;
> +
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -445,8 +448,6 @@ vlv_update_plane(struct drm_plane *dplane,
>  	linear_offset -= sprsurf_offset;
>  
>  	if (rotation & BIT(DRM_ROTATE_180)) {
> -		sprctl |= SP_ROTATE_180;
> -
>  		x += src_w;
>  		y += src_h;
>  		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> @@ -555,6 +556,9 @@ ivb_update_plane(struct drm_plane *plane,
>  	if (obj->tiling_mode != I915_TILING_NONE)
>  		sprctl |= SPRITE_TILED;
>  
> +	if (rotation & BIT(DRM_ROTATE_180))
> +		sprctl |= SPRITE_ROTATE_180;
> +
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
>  	else
> @@ -577,15 +581,12 @@ ivb_update_plane(struct drm_plane *plane,
>  						   fb->pitches[0], rotation);
>  	linear_offset -= sprsurf_offset;
>  
> -	if (rotation & BIT(DRM_ROTATE_180)) {
> -		sprctl |= SPRITE_ROTATE_180;
> -
> -		/* HSW and BDW does this automagically in hardware */
> -		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> -			x += src_w;
> -			y += src_h;
> -			linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> -		}
> +	/* HSW and BDW does this automagically in hardware */
> +	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) &&
> +	    rotation & BIT(DRM_ROTATE_180)) {
> +		x += src_w;
> +		y += src_h;
> +		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
>  	}
>  
>  	if (key->flags) {
> @@ -696,6 +697,9 @@ ilk_update_plane(struct drm_plane *plane,
>  	if (obj->tiling_mode != I915_TILING_NONE)
>  		dvscntr |= DVS_TILED;
>  
> +	if (rotation & BIT(DRM_ROTATE_180))
> +		dvscntr |= DVS_ROTATE_180;
> +
>  	if (IS_GEN6(dev))
>  		dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
>  
> @@ -715,8 +719,6 @@ ilk_update_plane(struct drm_plane *plane,
>  	linear_offset -= dvssurf_offset;
>  
>  	if (rotation & BIT(DRM_ROTATE_180)) {
> -		dvscntr |= DVS_ROTATE_180;
> -
>  		x += src_w;
>  		y += src_h;
>  		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm: Add support for optional per-plane rotation property

[Ville Syrjälä]

On Wed, Jul 20, 2016 at 04:51:57PM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Not all planes on the ssytem may support the same rotations/reflections,
> 
> s/ssytem/system/
> 
> > so make it possible to create a separate property for each plane.
> > This way userspace gets told exactly which rotations/reflections are
> > possible for each plane.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c    | 6 ++++--
> >  drivers/gpu/drm/drm_fb_helper.c | 6 +++++-
> >  include/drm/drm_crtc.h          | 2 ++
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 5f0002897862..38b023da6f09 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -710,7 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> >  		state->src_w = val;
> >  	} else if (property == config->prop_src_h) {
> >  		state->src_h = val;
> > -	} else if (property == config->rotation_property) {
> > +	} else if (property == config->rotation_property ||
> > +		   property == plane->rotation_property) {
> >  		if (!is_power_of_2(val & DRM_ROTATE_MASK))
> >  			return -EINVAL;
> >  		state->rotation = val;
> > @@ -768,7 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> >  		*val = state->src_w;
> >  	} else if (property == config->prop_src_h) {
> >  		*val = state->src_h;
> > -	} else if (property == config->rotation_property) {
> > +	} else if (property == config->rotation_property ||
> > +		   property == plane->rotation_property) {
> >  		*val = state->rotation;
> >  	} else if (plane->funcs->atomic_get_property) {
> >  		return plane->funcs->atomic_get_property(plane, state, property, val);
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index ce54e985d91b..ce536c0553e5 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> >  		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> >  			drm_plane_force_disable(plane);
> >  
> > -		if (dev->mode_config.rotation_property) {
> > +		if (plane->rotation_property) {
> > +			drm_mode_plane_set_obj_prop(plane,
> > +						    plane->rotation_property,
> > +						    BIT(DRM_ROTATE_0));
> > +		} else if (dev->mode_config.rotation_property) {
> 
> Why not clear the mode_config.rotation_property too, still?

I would be cleared if any plane uses it. Though I'm not sure mixing
per-plane and global rotation props in one device makes any real sense.

Or maybe I should just nuke the global property?

> 
> Regards, Joonas
> 
> >  			drm_mode_plane_set_obj_prop(plane,
> >  						    dev->mode_config.rotation_property,
> >  						    BIT(DRM_ROTATE_0));
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index b7ac7dcf52db..8d65e85d6d91 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1737,6 +1737,8 @@ struct drm_plane {
> >  	const struct drm_plane_helper_funcs *helper_private;
> >  
> >  	struct drm_plane_state *state;
> > +
> > +	struct drm_property *rotation_property;
> >  };
> >  
> >  /**
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm: Add support for optional per-plane rotation property

[Joonas Lahtinen]

On ke, 2016-07-20 at 17:08 +0300, Ville Syrjälä wrote:
> On Wed, Jul 20, 2016 at 04:51:57PM +0300, Joonas Lahtinen wrote:
> > 
> > On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> > > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Not all planes on the ssytem may support the same rotations/reflections,
> > s/ssytem/system/
> > 
> > > 
> > > so make it possible to create a separate property for each plane.
> > > This way userspace gets told exactly which rotations/reflections are
> > > possible for each plane.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic.c    | 6 ++++--
> > >  drivers/gpu/drm/drm_fb_helper.c | 6 +++++-
> > >  include/drm/drm_crtc.h          | 2 ++
> > >  3 files changed, 11 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 5f0002897862..38b023da6f09 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -710,7 +710,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
> > >  		state->src_w = val;
> > >  	} else if (property == config->prop_src_h) {
> > >  		state->src_h = val;
> > > -	} else if (property == config->rotation_property) {
> > > +	} else if (property == config->rotation_property ||
> > > +		   property == plane->rotation_property) {
> > >  		if (!is_power_of_2(val & DRM_ROTATE_MASK))
> > >  			return -EINVAL;
> > >  		state->rotation = val;
> > > @@ -768,7 +769,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> > >  		*val = state->src_w;
> > >  	} else if (property == config->prop_src_h) {
> > >  		*val = state->src_h;
> > > -	} else if (property == config->rotation_property) {
> > > +	} else if (property == config->rotation_property ||
> > > +		   property == plane->rotation_property) {
> > >  		*val = state->rotation;
> > >  	} else if (plane->funcs->atomic_get_property) {
> > >  		return plane->funcs->atomic_get_property(plane, state, property, val);
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > > index ce54e985d91b..ce536c0553e5 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -392,7 +392,11 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> > >  		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> > >  			drm_plane_force_disable(plane);
> > >  
> > > -		if (dev->mode_config.rotation_property) {
> > > +		if (plane->rotation_property) {
> > > +			drm_mode_plane_set_obj_prop(plane,
> > > +						    plane->rotation_property,
> > > +						    BIT(DRM_ROTATE_0));
> > > +		} else if (dev->mode_config.rotation_property) {
> > Why not clear the mode_config.rotation_property too, still?
> I would be cleared if any plane uses it. Though I'm not sure mixing
> per-plane and global rotation props in one device makes any real sense.
> 
> Or maybe I should just nuke the global property?

Maybe a vote for nuking, co-existence will unnecessarily complicate the
code. Only scenario I can see it working when planes with the property
are not used, and planes without are used solely. So the legacy
behaviour might not be very useful.

Regards, Joonas

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/i915: Use the per-plane rotation property

[Ville Syrjälä]

On Wed, Jul 20, 2016 at 04:57:32PM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On certain platforms not all planes support the same set of
> > rotations/reflections, so let's use the per-plane property
> > for this.
> > 
> > This is already a problem on SKL when we use the legay cursor plane
> > as it only supports 0|180 whereas the universal planes support
> > 0|90|180|270, and it will be a problem on CHV soon.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++++++-----------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  4 +--
> >  drivers/gpu/drm/i915/intel_sprite.c  | 12 ++++++++-
> >  3 files changed, 40 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 08b9f9a19df0..93ecb259c5ce 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14217,6 +14217,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> >  	struct intel_plane *primary = NULL;
> >  	struct intel_plane_state *state = NULL;
> >  	const uint32_t *intel_primary_formats;
> > +	unsigned int supported_rotations;
> >  	unsigned int num_formats;
> >  	int ret;
> >  
> > @@ -14289,8 +14290,19 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> >  	if (ret)
> >  		goto fail;
> >  
> > +	if (INTEL_INFO(dev)->gen >= 9) {
> > +		supported_rotations =
> > +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> > +			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> > +	} else if (INTEL_INFO(dev)->gen >= 4) {
> > +		supported_rotations =
> > +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> > +	} else {
> > +		supported_rotations = BIT(DRM_ROTATE_0);
> > +	}
> > +
> 
> Might this be more informative if it was;
> 
> supported_rotations = BIT(DRM_ROTATE_0);
> 
> if (gen >= 4)
> 	supported_rotations |= BIT(DRM_ROTATE_180);
> 
> Then you could exclude too, for special platforms.

I think I prefer to have explicit "these platforms support exactly these things".
Less thinking required.

> 
> Also, use INTEL_GEN()
> 
> >  	if (INTEL_INFO(dev)->gen >= 4)
> > -		intel_create_rotation_property(dev, primary);
> > +		intel_create_rotation_property(primary, supported_rotations);
> >  
> >  	drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
> >  
> > @@ -14303,22 +14315,20 @@ fail:
> >  	return NULL;
> >  }
> >  
> > -void intel_create_rotation_property(struct drm_device *dev, struct intel_plane *plane)
> > +void intel_create_rotation_property(struct intel_plane *plane,
> > +				    unsigned int supported_rotations)
> >  {
> > -	if (!dev->mode_config.rotation_property) {
> > -		unsigned long flags = BIT(DRM_ROTATE_0) |
> > -			BIT(DRM_ROTATE_180);
> > +	struct drm_device *dev = plane->base.dev;
> >  
> > -		if (INTEL_INFO(dev)->gen >= 9)
> > -			flags |= BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270);
> > +	if (!plane->base.rotation_property)
> > +		plane->base.rotation_property =
> > +			drm_mode_create_rotation_property(dev,
> > +							  supported_rotations);
> >  
> > -		dev->mode_config.rotation_property =
> > -			drm_mode_create_rotation_property(dev, flags);
> > -	}
> > -	if (dev->mode_config.rotation_property)
> > +	if (plane->base.rotation_property)
> >  		drm_object_attach_property(&plane->base.base,
> > -				dev->mode_config.rotation_property,
> > -				plane->base.state->rotation);
> > +					   plane->base.rotation_property,
> > +					   plane->base.state->rotation);
> >  }
> >  
> >  static int
> > @@ -14449,17 +14459,10 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> >  	if (ret)
> >  		goto fail;
> >  
> > -	if (INTEL_INFO(dev)->gen >= 4) {
> > -		if (!dev->mode_config.rotation_property)
> > -			dev->mode_config.rotation_property =
> > -				drm_mode_create_rotation_property(dev,
> > -							BIT(DRM_ROTATE_0) |
> > -							BIT(DRM_ROTATE_180));
> > -		if (dev->mode_config.rotation_property)
> > -			drm_object_attach_property(&cursor->base.base,
> > -				dev->mode_config.rotation_property,
> > -				state->base.rotation);
> > -	}
> > +	if (INTEL_INFO(dev)->gen >= 4)
> 
> INTEL_GEN() while touching it.
> 
> > +		intel_create_rotation_property(cursor,
> > +					       BIT(DRM_ROTATE_0) |
> > +					       BIT(DRM_ROTATE_180));
> >  
> >  	if (INTEL_INFO(dev)->gen >=9)
> >  		state->scaler_id = -1;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 907a72cfdad3..2715aec979fc 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1255,8 +1255,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> >  unsigned int intel_tile_height(const struct drm_i915_private *dev_priv,
> >  			       uint64_t fb_modifier, unsigned int cpp);
> >  
> > -void intel_create_rotation_property(struct drm_device *dev,
> > -					struct intel_plane *plane);
> > +void intel_create_rotation_property(struct intel_plane *plane,
> > +				    unsigned int supported_rotations);
> >  
> >  void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
> >  				    enum pipe pipe);
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index cc6af1410d67..bc41c1bba04c 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1046,6 +1046,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  	struct intel_plane_state *state = NULL;
> >  	unsigned long possible_crtcs;
> >  	const uint32_t *plane_formats;
> > +	unsigned int supported_rotations;
> >  	int num_plane_formats;
> >  	int ret;
> >  
> > @@ -1121,6 +1122,15 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  		goto fail;
> >  	}
> >  
> > +	if (INTEL_INFO(dev)->gen >= 9) {
> > +		supported_rotations =
> > +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
> > +			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> > +	} else {
> > +		supported_rotations =
> > +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> > +	}
> > +
> 
> Same here, I'd put the |='s in order so that new gens add features,
> making easier to get a picture of what was added at which point.
> 
> Regards, Joonas
> 
> >  	intel_plane->pipe = pipe;
> >  	intel_plane->plane = plane;
> >  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> > @@ -1143,7 +1153,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  	if (ret)
> >  		goto fail;
> >  
> > -	intel_create_rotation_property(dev, intel_plane);
> > +	intel_create_rotation_property(intel_plane, supported_rotations);
> >  
> >  	drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> >  
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] drm/i915: Use & instead if == to check for rotations

[Ville Syrjälä]

On Wed, Jul 20, 2016 at 05:01:00PM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Using == to check for 180 degree rotation only works as long as the
> > reflection bits aren't set. That will change soon enough for CHV, so
> > let's stop doing things the wrong way.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Reading through the patch makes me wanna do the change;
> 
> #define DRM_ROTATE_180 BIT(x)
> 
> even more. Maybe with coccinelle.

Yeah, that one is annoying. One problem is the way the bits are
specified for bitmask properties (as shifts), so we still need the
names for the shifts too :(

> 
> Regards, Joonas
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 9 +++++----
> >  drivers/gpu/drm/i915/intel_sprite.c  | 6 +++---
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 93ecb259c5ce..79c1a8b89d1d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2688,7 +2688,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
> >  		intel_crtc->dspaddr_offset = linear_offset;
> >  	}
> >  
> > -	if (rotation == BIT(DRM_ROTATE_180)) {
> > +	if (rotation & BIT(DRM_ROTATE_180)) {
> >  		dspcntr |= DISPPLANE_ROTATE_180;
> >  
> >  		x += (crtc_state->pipe_src_w - 1);
> > @@ -2791,7 +2791,8 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
> >  		intel_compute_tile_offset(&x, &y, fb, 0,
> >  					  fb->pitches[0], rotation);
> >  	linear_offset -= intel_crtc->dspaddr_offset;
> > -	if (rotation == BIT(DRM_ROTATE_180)) {
> > +
> > +	if (rotation & BIT(DRM_ROTATE_180)) {
> >  		dspcntr |= DISPPLANE_ROTATE_180;
> >  
> >  		if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) {
> > @@ -10263,7 +10264,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
> >  		if (HAS_DDI(dev))
> >  			cntl |= CURSOR_PIPE_CSC_ENABLE;
> >  
> > -		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
> > +		if (plane_state->base.rotation & BIT(DRM_ROTATE_180))
> >  			cntl |= CURSOR_ROTATE_180;
> >  	}
> >  
> > @@ -10309,7 +10310,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> >  
> >  		/* ILK+ do this automagically */
> >  		if (HAS_GMCH_DISPLAY(dev) &&
> > -		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> > +		    plane_state->base.rotation & BIT(DRM_ROTATE_180)) {
> >  			base += (plane_state->base.crtc_h *
> >  				 plane_state->base.crtc_w - 1) * 4;
> >  		}
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index bc41c1bba04c..6b815d57d75a 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -444,7 +444,7 @@ vlv_update_plane(struct drm_plane *dplane,
> >  						   fb->pitches[0], rotation);
> >  	linear_offset -= sprsurf_offset;
> >  
> > -	if (rotation == BIT(DRM_ROTATE_180)) {
> > +	if (rotation & BIT(DRM_ROTATE_180)) {
> >  		sprctl |= SP_ROTATE_180;
> >  
> >  		x += src_w;
> > @@ -577,7 +577,7 @@ ivb_update_plane(struct drm_plane *plane,
> >  						   fb->pitches[0], rotation);
> >  	linear_offset -= sprsurf_offset;
> >  
> > -	if (rotation == BIT(DRM_ROTATE_180)) {
> > +	if (rotation & BIT(DRM_ROTATE_180)) {
> >  		sprctl |= SPRITE_ROTATE_180;
> >  
> >  		/* HSW and BDW does this automagically in hardware */
> > @@ -714,7 +714,7 @@ ilk_update_plane(struct drm_plane *plane,
> >  						   fb->pitches[0], rotation);
> >  	linear_offset -= dvssurf_offset;
> >  
> > -	if (rotation == BIT(DRM_ROTATE_180)) {
> > +	if (rotation & BIT(DRM_ROTATE_180)) {
> >  		dvscntr |= DVS_ROTATE_180;
> >  
> >  		x += src_w;
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Ro.CI.BAT: success for drm/i915: Per-plane rotation, fixes, and mirroring for CHV

[Patchwork]

== Series Details ==

Series: drm/i915: Per-plane rotation, fixes, and mirroring for CHV
URL   : https://patchwork.freedesktop.org/series/10093/
State : success

== Summary ==

Series 10093v1 drm/i915: Per-plane rotation, fixes, and mirroring for CHV
http://patchwork.freedesktop.org/api/1.0/series/10093/revisions/1/mbox

Test gem_sync:
        Subgroup basic-store-each:
                dmesg-fail -> PASS       (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-hsw-i7-4770k)

fi-hsw-i7-4770k  total:208  pass:189  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-qkkr      total:244  pass:179  dwarn:27  dfail:0   fail:11  skip:27 
fi-skl-i5-6260u  total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
fi-skl-i7-6700k  total:244  pass:210  dwarn:0   dfail:0   fail:8   skip:26 
fi-snb-i7-2600   total:244  pass:196  dwarn:0   dfail:0   fail:8   skip:40 
ro-bdw-i5-5250u  total:244  pass:219  dwarn:4   dfail:0   fail:8   skip:13 
ro-bdw-i7-5600u  total:244  pass:204  dwarn:0   dfail:0   fail:8   skip:32 
ro-bsw-n3050     total:218  pass:173  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:244  pass:197  dwarn:0   dfail:0   fail:9   skip:38 
ro-hsw-i3-4010u  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-hsw-i7-4770r  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-ilk-i7-620lm  total:244  pass:172  dwarn:0   dfail:0   fail:9   skip:63 
ro-ilk1-i5-650   total:239  pass:172  dwarn:0   dfail:0   fail:9   skip:58 
ro-ivb-i7-3770   total:244  pass:203  dwarn:0   dfail:0   fail:8   skip:33 
ro-skl3-i5-6260u total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
ro-snb-i7-2620M  total:244  pass:193  dwarn:0   dfail:0   fail:9   skip:42 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1545/

30920eb drm-intel-nightly: 2016y-07m-20d-13h-08m-56s UTC integration manifest
26bd7f8 drm/i915: Add horizontal mirroring support for CHV pipe B planes
d6486b7 drm/i915: Clean up rotation DSPCNTR/DVSCNTR/etc. setup
3d4af5f drm/i915: Use & instead if == to check for rotations
8db883e drm/i915: Use the per-plane rotation property
e2e77bc drm: Add support for optional per-plane rotation property
82703dc drm/atomic: Reject attempts to use multiple rotation angles at once
f11ba82 drm: Add drm_rotation_90_or_270()

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

Re: [PATCH 7/7] drm/i915: Add horizontal mirroring support for CHV pipe B planes

[Joonas Lahtinen]

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The primary and sprite planes on CHV pipe B support horizontal
> mirroring. Expose it to the world.
> 
> Sadly the hardware ignores the mirror bit when the rotate bit is
> set, so we'll have to reject the 180+X case.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Seems to match spec. Maybe add your own T-b, as you have obviously
tested on hardware.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  8 ++++++++
>  drivers/gpu/drm/i915/intel_display.c      | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_sprite.c       | 10 ++++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index c19eb9a0cd4a..0a019eacfede 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -180,6 +180,14 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  		}
>  	}
>  
> +	/* CHV ignores the mirror bit when the rotate bit is set :( */
> +	if (IS_CHERRYVIEW(plane->dev) &&
> +	    state->rotation & BIT(DRM_ROTATE_180) &&
> +	    state->rotation & BIT(DRM_REFLECT_X)) {
> +		DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
> +		return -EINVAL;
> +	}
> +
>  	intel_state->visible = false;
>  	ret = intel_plane->check_plane(plane, crtc_state, intel_state);
>  	if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 88a7c4173715..edb1809a642d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2677,6 +2677,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	if (rotation & BIT(DRM_ROTATE_180))
>  		dspcntr |= DISPPLANE_ROTATE_180;
>  
> +	if (rotation & BIT(DRM_REFLECT_X))
> +		dspcntr |= DISPPLANE_MIRROR;
> +
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
> @@ -2700,6 +2703,9 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  		linear_offset +=
>  			(crtc_state->pipe_src_h - 1) * fb->pitches[0] +
>  			(crtc_state->pipe_src_w - 1) * cpp;
> +	} else if (rotation & BIT(DRM_REFLECT_X)) {
> +		x += (crtc_state->pipe_src_w - 1);
> +		linear_offset += (crtc_state->pipe_src_w - 1) * cpp;
>  	}
>  
>  	intel_crtc->adjusted_x = x;
> @@ -14295,6 +14301,10 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  		supported_rotations =
>  			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
>  			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> +	} else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
> +			BIT(DRM_REFLECT_X);
>  	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		supported_rotations =
>  			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 14173f53f520..4d6cd1a02e34 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -436,6 +436,9 @@ vlv_update_plane(struct drm_plane *dplane,
>  	if (rotation & BIT(DRM_ROTATE_180))
>  		sprctl |= SP_ROTATE_180;
>  
> +	if (rotation & BIT(DRM_REFLECT_X))
> +		sprctl |= SP_MIRROR;
> +
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -451,6 +454,9 @@ vlv_update_plane(struct drm_plane *dplane,
>  		x += src_w;
>  		y += src_h;
>  		linear_offset += src_h * fb->pitches[0] + src_w * cpp;
> +	} else if (rotation & BIT(DRM_REFLECT_X)) {
> +		x += src_w;
> +		linear_offset += src_w * cpp;
>  	}
>  
>  	if (key->flags) {
> @@ -1128,6 +1134,10 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		supported_rotations =
>  			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
>  			BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270);
> +	} else if (IS_CHERRYVIEW(dev) && pipe == PIPE_B) {
> +		supported_rotations =
> +			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180) |
> +			BIT(DRM_REFLECT_X);
>  	} else {
>  		supported_rotations =
>  			BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/kms_rotation_crc: Add bad-rotation subtest

[Joonas Lahtinen]

Was not this implemented once and then dropped for some reason?

On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add "bad-rotation" subtest to make sure the kernel rejects some
> invalid rotation values (0 and specifying multiple angles at one).
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_rotation_crc.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 6cc15337fff9..e10a0a770437 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -237,6 +237,31 @@ static void wait_for_pageflip(int fd)
>  	igt_assert(drmHandleEvent(fd, &evctx) == 0);
>  }
>  
> +static void test_bad_prop_value(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	int valid_tests = 0;
> +	enum pipe pipe;
> +	igt_plane_t *plane;
> +	int ret;
> +
> +	for_each_pipe(display, pipe)  {
> +		for_each_plane_on_pipe(display, pipe, plane) {
> +			igt_require(igt_plane_supports_rotation(plane));
> +
> +			ret = drmModeObjectSetProperty(display->drm_fd,
> +						       plane->drm_plane->plane_id,
> +						       DRM_MODE_OBJECT_PLANE,
> +						       plane->rotation_property,
> +						       data->rotation);
> +
> +			igt_assert_eq(ret, -EINVAL);
> +			valid_tests++;
> +		}
> +	}
> +	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> +}
> +
>  static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>  {
>  	igt_display_t *display = &data->display;
> @@ -508,6 +533,14 @@ igt_main
>  
>  		igt_display_init(&data.display, data.gfx_fd);
>  	}
> +	igt_subtest_f("bad-rotation") {
> +		data.rotation = 0;
> +		test_bad_prop_value(&data);
> +
> +		data.rotation = IGT_ROTATION_0 | IGT_ROTATION_180;
> +		test_bad_prop_value(&data);
> +	}
> +
>  	igt_subtest_f("primary-rotation-180") {
>  		data.rotation = IGT_ROTATION_180;
>  		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/kms_rotation_crc: Add bad-rotation subtest

[Ville Syrjälä]

On Thu, Jul 21, 2016 at 01:32:48PM +0300, Joonas Lahtinen wrote:
> Was not this implemented once and then dropped for some reason?

Dunno.

> 
> On ke, 2016-07-20 at 16:18 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add "bad-rotation" subtest to make sure the kernel rejects some
> > invalid rotation values (0 and specifying multiple angles at one).
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/kms_rotation_crc.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index 6cc15337fff9..e10a0a770437 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -237,6 +237,31 @@ static void wait_for_pageflip(int fd)
> >  	igt_assert(drmHandleEvent(fd, &evctx) == 0);
> >  }
> >  
> > +static void test_bad_prop_value(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	int valid_tests = 0;
> > +	enum pipe pipe;
> > +	igt_plane_t *plane;
> > +	int ret;
> > +
> > +	for_each_pipe(display, pipe)  {
> > +		for_each_plane_on_pipe(display, pipe, plane) {
> > +			igt_require(igt_plane_supports_rotation(plane));
> > +
> > +			ret = drmModeObjectSetProperty(display->drm_fd,
> > +						       plane->drm_plane->plane_id,
> > +						       DRM_MODE_OBJECT_PLANE,
> > +						       plane->rotation_property,
> > +						       data->rotation);
> > +
> > +			igt_assert_eq(ret, -EINVAL);
> > +			valid_tests++;
> > +		}
> > +	}
> > +	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> > +}
> > +
> >  static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
> >  {
> >  	igt_display_t *display = &data->display;
> > @@ -508,6 +533,14 @@ igt_main
> >  
> >  		igt_display_init(&data.display, data.gfx_fd);
> >  	}
> > +	igt_subtest_f("bad-rotation") {
> > +		data.rotation = 0;
> > +		test_bad_prop_value(&data);
> > +
> > +		data.rotation = IGT_ROTATION_0 | IGT_ROTATION_180;
> > +		test_bad_prop_value(&data);
> > +	}
> > +
> >  	igt_subtest_f("primary-rotation-180") {
> >  		data.rotation = IGT_ROTATION_180;
> >  		test_plane_rotation(&data, IGT_PLANE_PRIMARY);
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/kms_rotation_crc: Add bad-rotation subtest

[Matthew Auld]

I believe you're thinking of:
https://patchwork.freedesktop.org/patch/77191/
https://patchwork.freedesktop.org/patch/77192/

Although they don't test for multiple rotation values...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] tests/kms_rotation_crc: Add bad-rotation subtest

[Ville Syrjälä]

On Fri, Jul 22, 2016 at 01:52:32PM +0100, Matthew Auld wrote:
> I believe you're thinking of:
> https://patchwork.freedesktop.org/patch/77191/
> https://patchwork.freedesktop.org/patch/77192/
> 
> Although they don't test for multiple rotation values...

I guess you could just

for (rotation = 0; rotation < 0x40; rotation++)
	if (!is_power_of_2(rotation & 0xf) ||
	    (rotation & ~supported) != 0)
	    	// expect failure
}

if you want to be really sure invalid crap won't get through.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx