[PATCH 1/2] drm/i915: intel_crtc_page_flip: Add intel_state local variable

[PATCH 1/2] drm/i915: intel_crtc_page_flip: Add intel_state local variable

[Hans de Goede]

Add intel_state local variable to avoid recalling to_intel_plane_state
all the time.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ed1f4f272b4f..3664707950c7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12146,6 +12146,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	struct drm_framebuffer *old_fb = crtc->primary->fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_plane_state *intel_state =
+		to_intel_plane_state(crtc->primary->state);
 	struct drm_plane *primary = crtc->primary;
 	enum pipe pipe = intel_crtc->pipe;
 	struct intel_flip_work *work;
@@ -12262,8 +12264,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 	}
 
-	work->old_vma = to_intel_plane_state(primary->state)->vma;
-	to_intel_plane_state(primary->state)->vma = vma;
+	work->old_vma = intel_state->vma;
+	intel_state->vma = vma;
 
 	work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset;
 	work->rotation = crtc->primary->state->rotation;
@@ -12276,8 +12278,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	 * be on the safe side and do this immediately before scheduling the
 	 * flip.
 	 */
-	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
-			     to_intel_plane_state(primary->state));
+	intel_fbc_pre_update(intel_crtc, intel_crtc->config, intel_state);
 
 	if (mmio_flip) {
 		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
@@ -12320,7 +12321,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 cleanup_request:
 	i915_add_request_no_flush(request);
 cleanup_unpin:
-	to_intel_plane_state(primary->state)->vma = work->old_vma;
+	intel_state->vma = work->old_vma;
 	intel_unpin_fb_vma(vma);
 cleanup_pending:
 	atomic_dec(&intel_crtc->unpin_work_count);
-- 
2.12.2

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

[PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Hans de Goede]

On some (Bay Trail) devices the LCD panel is mounted upside-down.

This commit uses the code to read back the initial rotation of the
primary plane in get_initial_plane_config from Ville Syrjala's
"drm/fb-helper: Inherit rotation wip" patch and when re-using the
initial fb it stores that in intel_crtc.initial_rotation.

It adds an intel_plane_get_rotation helper which combines this
initial_rotation with any rotation requested by userspace and
uses this in all places which look at a planes rotation, thus
transparently dealing with upside-down LCD panels without requiring
any user-space or fbcon changes.

This fixes the kernel boot messages switching from being shown the right
way up in efifb to being shown upside down as soon as a native kms driver
loads, as well as any graphics displayed by userspace being upside-down.

Note this only deals with upside-down LCD panels / 180 degrees
rotation as the hardware in question only supports 180 degrees
rotation in hardware. The one model known which has a panel mounted
with 90/270 degrees rotation will need to be fully dealt with in
userspace, even the firmware boot screen / menus are rotated 90 degrees
on this one and there simply is nothing the kernel can do about this.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  7 +--
 drivers/gpu/drm/i915/intel_display.c      | 80 +++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h          | 32 +++++++++++++
 drivers/gpu/drm/i915/intel_fbc.c          |  2 +-
 drivers/gpu/drm/i915/intel_pm.c           | 16 ++++---
 drivers/gpu/drm/i915/intel_sprite.c       | 14 +++---
 6 files changed, 113 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 41fd94e62d3c..db681bc4a9ca 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -130,6 +130,7 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(plane->dev);
 	struct drm_plane_state *state = &intel_state->base;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	unsigned int rotation = intel_plane_get_rotation(intel_state);
 	int ret;
 
 	/*
@@ -149,7 +150,7 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	intel_state->clip.y2 =
 		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
 
-	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
+	if (state->fb && drm_rotation_90_or_270(rotation)) {
 		struct drm_format_name_buf format_name;
 
 		if (state->fb->modifier != I915_FORMAT_MOD_Y_TILED &&
@@ -178,8 +179,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 
 	/* CHV ignores the mirror bit when the rotate bit is set :( */
 	if (IS_CHERRYVIEW(dev_priv) &&
-	    state->rotation & DRM_ROTATE_180 &&
-	    state->rotation & DRM_REFLECT_X) {
+	    rotation & DRM_ROTATE_180 &&
+	    rotation & DRM_REFLECT_X) {
 		DRM_DEBUG_KMS("Cannot rotate and reflect at the same time\n");
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3664707950c7..d4824702be4e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2286,7 +2286,7 @@ void intel_add_fb_offsets(int *x, int *y,
 
 {
 	const struct intel_framebuffer *intel_fb = to_intel_framebuffer(state->base.fb);
-	unsigned int rotation = state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(state);
 
 	if (drm_rotation_90_or_270(rotation)) {
 		*x += intel_fb->rotated[plane].x;
@@ -2339,7 +2339,7 @@ static u32 intel_adjust_tile_offset(int *x, int *y,
 	const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
 	const struct drm_framebuffer *fb = state->base.fb;
 	unsigned int cpp = fb->format->cpp[plane];
-	unsigned int rotation = state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(state);
 	unsigned int pitch = intel_fb_pitch(fb, plane, rotation);
 
 	WARN_ON(new_offset > old_offset);
@@ -2444,7 +2444,7 @@ u32 intel_compute_tile_offset(int *x, int *y,
 {
 	const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
 	const struct drm_framebuffer *fb = state->base.fb;
-	unsigned int rotation = state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(state);
 	int pitch = intel_fb_pitch(fb, plane, rotation);
 	u32 alignment;
 
@@ -2798,9 +2798,10 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	return;
 
 valid_fb:
+	intel_crtc->initial_rotation = plane_config->rotation;
 	mutex_lock(&dev->struct_mutex);
 	intel_state->vma =
-		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+		intel_pin_and_fence_fb_obj(fb, intel_crtc->initial_rotation);
 	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(intel_state->vma)) {
 		DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
@@ -2882,7 +2883,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 {
 	const struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 	int w = drm_rect_width(&plane_state->base.src) >> 16;
@@ -2941,7 +2942,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
 static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	int max_width = skl_max_plane_width(fb, 1, rotation);
 	int max_height = 4096;
 	int x = plane_state->base.src.x1 >> 17;
@@ -2970,7 +2971,7 @@ static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 int skl_check_plane_surface(struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	int ret;
 
 	if (!plane_state->base.visible)
@@ -3014,7 +3015,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	u32 linear_offset;
 	u32 dspcntr;
 	i915_reg_t reg = DSPCNTR(plane);
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 
@@ -3146,7 +3147,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 	u32 linear_offset;
 	u32 dspcntr;
 	i915_reg_t reg = DSPCNTR(plane);
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	int x = plane_state->base.src.x1 >> 16;
 	int y = plane_state->base.src.y1 >> 16;
 
@@ -3373,7 +3374,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	enum plane_id plane_id = to_intel_plane(plane)->id;
 	enum pipe pipe = to_intel_plane(plane)->pipe;
 	u32 plane_ctl;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	u32 stride = skl_plane_stride(fb, 0, rotation);
 	u32 surf_addr = plane_state->main.offset;
 	int scaler_id = plane_state->scaler_id;
@@ -4735,7 +4736,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	ret = skl_update_scaler(crtc_state, force_detach,
 				drm_plane_index(&intel_plane->base),
 				&plane_state->scaler_id,
-				plane_state->base.rotation,
+				intel_plane_get_rotation(plane_state),
 				drm_rect_width(&plane_state->base.src) >> 16,
 				drm_rect_height(&plane_state->base.src) >> 16,
 				drm_rect_width(&plane_state->base.dst),
@@ -8733,6 +8734,9 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 			fb->modifier = I915_FORMAT_MOD_X_TILED;
 		}
+
+		if (val & DISPPLANE_ROTATE_180)
+			plane_config->rotation = DRM_ROTATE_180;
 	}
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
@@ -9784,6 +9788,9 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 		goto error;
 	}
 
+	if ((val & PLANE_CTL_ROTATE_MASK) == PLANE_CTL_ROTATE_180)
+		plane_config->rotation = DRM_ROTATE_180;
+
 	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
 	plane_config->base = base;
 
@@ -9872,6 +9879,9 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
 			plane_config->tiling = I915_TILING_X;
 			fb->modifier = I915_FORMAT_MOD_X_TILED;
 		}
+
+		if (val & DISPPLANE_ROTATE_180)
+			plane_config->rotation = DRM_ROTATE_180;
 	}
 
 	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
@@ -10857,7 +10867,7 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 		if (HAS_DDI(dev_priv))
 			cntl |= CURSOR_PIPE_CSC_ENABLE;
 
-		if (plane_state->base.rotation & DRM_ROTATE_180)
+		if (intel_plane_get_rotation(plane_state) & DRM_ROTATE_180)
 			cntl |= CURSOR_ROTATE_180;
 	}
 
@@ -10874,6 +10884,25 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
 	intel_crtc->cursor_base = base;
 }
 
+static void intel_crtc_get_cursor_pos(struct drm_crtc *crtc,
+				      const struct drm_plane_state *cursor,
+				      int *x, int *y)
+{
+	struct drm_plane_state *primary = crtc->primary->state;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	*x = cursor->crtc_x;
+	*y = cursor->crtc_y;
+
+	if (!primary)
+		return;
+
+	if (intel_crtc->initial_rotation == DRM_ROTATE_180) {
+		*x = primary->crtc_w - cursor->crtc_w - *x;
+		*y = primary->crtc_h - cursor->crtc_h - *y;
+	}
+}
+
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
 static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 				     const struct intel_plane_state *plane_state)
@@ -10881,13 +10910,12 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
+	int x, y, pipe = intel_crtc->pipe;
 	u32 base = intel_crtc->cursor_addr;
 	u32 pos = 0;
 
 	if (plane_state) {
-		int x = plane_state->base.crtc_x;
-		int y = plane_state->base.crtc_y;
+		intel_crtc_get_cursor_pos(crtc, &plane_state->base, &x, &y);
 
 		if (x < 0) {
 			pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
@@ -10903,7 +10931,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 
 		/* ILK+ do this automagically */
 		if (HAS_GMCH_DISPLAY(dev_priv) &&
-		    plane_state->base.rotation & DRM_ROTATE_180) {
+		    intel_plane_get_rotation(plane_state) & DRM_ROTATE_180) {
 			base += (plane_state->base.crtc_h *
 				 plane_state->base.crtc_w - 1) * 4;
 		}
@@ -12152,6 +12180,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	enum pipe pipe = intel_crtc->pipe;
 	struct intel_flip_work *work;
 	struct intel_engine_cs *engine;
+	unsigned int rotation;
 	bool mmio_flip;
 	struct drm_i915_gem_request *request;
 	struct i915_vma *vma;
@@ -12258,7 +12287,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	mmio_flip = use_mmio_flip(engine, obj);
 
-	vma = intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+	rotation = intel_plane_get_rotation(intel_state);
+	vma = intel_pin_and_fence_fb_obj(fb, rotation);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto cleanup_pending;
@@ -12268,7 +12298,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	intel_state->vma = vma;
 
 	work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset;
-	work->rotation = crtc->primary->state->rotation;
+	work->rotation = rotation;
 
 	/*
 	 * There's the potential that the next frame will not be compatible with
@@ -14842,9 +14872,12 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 			return ret;
 		}
 	} else {
+		unsigned int rotation;
 		struct i915_vma *vma;
 
-		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+		rotation = intel_plane_get_rotation(
+					to_intel_plane_state(new_state));
+		vma = intel_pin_and_fence_fb_obj(fb, rotation);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
 			return PTR_ERR(vma);
@@ -15091,9 +15124,12 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 			goto out_unlock;
 		}
 	} else {
+		unsigned int rotation;
 		struct i915_vma *vma;
 
-		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
+		rotation = intel_plane_get_rotation(
+					to_intel_plane_state(new_plane_state));
+		vma = intel_pin_and_fence_fb_obj(fb, rotation);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
 
@@ -16714,7 +16750,9 @@ int intel_modeset_init(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 
 	for_each_intel_crtc(dev, crtc) {
-		struct intel_initial_plane_config plane_config = {};
+		struct intel_initial_plane_config plane_config = {
+			.rotation = DRM_ROTATE_0
+		};
 
 		if (!crtc->active)
 			continue;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 344f238b283f..4c9625b67600 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -418,6 +418,7 @@ struct intel_initial_plane_config {
 	unsigned int tiling;
 	int size;
 	u32 base;
+	uint8_t rotation; /* DRM_ROTATE_* */
 };
 
 #define SKL_MIN_SRC_W 8
@@ -704,6 +705,12 @@ struct intel_crtc {
 
 	atomic_t unpin_work_count;
 
+	/*
+	 * Initial DRM_ROTATE_* as read back from the hardware at init, this
+	 * is used to compensate for e.g. upside-down mounted lcd-panels.
+	 */
+	uint8_t initial_rotation;
+
 	/* Display surface base address adjustement for pageflips. Note that on
 	 * gen4+ this only adjusts up to a tile, offsets within a tile are
 	 * handled in the hw itself (with the TILEOFF register). */
@@ -1403,6 +1410,31 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 	return i915_ggtt_offset(state->vma);
 }
 
+static inline unsigned int
+intel_plane_get_rotation(const struct intel_plane_state *plane_state)
+{
+	unsigned int rotation = plane_state->base.rotation;
+	unsigned int new_rotation = DRM_ROTATE_0;
+	struct intel_crtc *intel_crtc;
+
+	if (!plane_state->base.crtc)
+		return rotation;
+
+	/* We only support an initial rotation of DRM_ROTATE_180 for now */
+	intel_crtc = to_intel_crtc(plane_state->base.crtc);
+	if (intel_crtc->initial_rotation != DRM_ROTATE_180)
+		return rotation;
+
+	switch (rotation & DRM_ROTATE_MASK) {
+	case DRM_ROTATE_0:	new_rotation = DRM_ROTATE_180;	break;
+	case DRM_ROTATE_90:	new_rotation = DRM_ROTATE_270;	break;
+	case DRM_ROTATE_180:	new_rotation = DRM_ROTATE_0;	break;
+	case DRM_ROTATE_270:	new_rotation = DRM_ROTATE_90;	break;
+	}
+
+	return (rotation & DRM_ROTATE_MASK) | new_rotation;
+}
+
 u32 skl_plane_ctl_format(uint32_t pixel_format);
 u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
 u32 skl_plane_ctl_rotation(unsigned int rotation);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 89fe5c8464df..7dd2a0ab56f8 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -746,7 +746,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 		cache->crtc.hsw_bdw_pixel_rate =
 			ilk_pipe_pixel_rate(crtc_state);
 
-	cache->plane.rotation = plane_state->base.rotation;
+	cache->plane.rotation = intel_plane_get_rotation(plane_state);
 	cache->plane.src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	cache->plane.src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	cache->plane.visible = plane_state->base.visible;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6a29784d2b41..cd7c63793d0f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3186,6 +3186,7 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 static uint32_t
 skl_plane_downscale_amount(const struct intel_plane_state *pstate)
 {
+	unsigned int rotation = intel_plane_get_rotation(pstate);
 	uint32_t downscale_h, downscale_w;
 	uint32_t src_w, src_h, dst_w, dst_h;
 
@@ -3197,7 +3198,7 @@ skl_plane_downscale_amount(const struct intel_plane_state *pstate)
 	src_h = drm_rect_height(&pstate->base.src);
 	dst_w = drm_rect_width(&pstate->base.dst);
 	dst_h = drm_rect_height(&pstate->base.dst);
-	if (drm_rotation_90_or_270(pstate->base.rotation))
+	if (drm_rotation_90_or_270(rotation))
 		swap(dst_w, dst_h);
 
 	downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
@@ -3213,6 +3214,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 			     int y)
 {
 	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
+	unsigned int rotation = intel_plane_get_rotation(intel_pstate);
 	uint32_t down_scale_amount, data_rate;
 	uint32_t width = 0, height = 0;
 	struct drm_framebuffer *fb;
@@ -3232,7 +3234,7 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	if (drm_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(rotation))
 		swap(width, height);
 
 	/* for planar format */
@@ -3301,6 +3303,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 {
 	struct drm_framebuffer *fb = pstate->fb;
 	struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
+	unsigned int rotation = intel_plane_get_rotation(intel_pstate);
 	uint32_t src_w, src_h;
 	uint32_t min_scanlines = 8;
 	uint8_t plane_bpp;
@@ -3320,7 +3323,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	src_w = drm_rect_width(&intel_pstate->base.src) >> 16;
 	src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	if (drm_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(rotation))
 		swap(src_w, src_h);
 
 	/* Halve UV plane width and height for NV12 */
@@ -3334,7 +3337,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
 	else
 		plane_bpp = fb->format->cpp[0];
 
-	if (drm_rotation_90_or_270(pstate->rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		switch (plane_bpp) {
 		case 1:
 			min_scanlines = 32;
@@ -3568,6 +3571,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	struct drm_plane_state *pstate = &intel_pstate->base;
 	struct drm_framebuffer *fb = pstate->fb;
 	uint32_t latency = dev_priv->wm.skl_latency[level];
+	unsigned int rotation = intel_plane_get_rotation(intel_pstate);
 	uint_fixed_16_16_t method1, method2;
 	uint_fixed_16_16_t plane_blocks_per_line;
 	uint_fixed_16_16_t selected_result;
@@ -3603,13 +3607,13 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
-	if (drm_rotation_90_or_270(pstate->rotation))
+	if (drm_rotation_90_or_270(rotation))
 		swap(width, height);
 
 	cpp = fb->format->cpp[0];
 	plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, intel_pstate);
 
-	if (drm_rotation_90_or_270(pstate->rotation)) {
+	if (drm_rotation_90_or_270(rotation)) {
 		int cpp = (fb->format->format == DRM_FORMAT_NV12) ?
 			fb->format->cpp[1] :
 			fb->format->cpp[0];
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9481ca9a3ae7..add15f0c1e10 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -208,7 +208,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	u32 plane_ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	u32 surf_addr = plane_state->main.offset;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	u32 stride = skl_plane_stride(fb, 0, rotation);
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
@@ -341,7 +341,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	enum plane_id plane_id = intel_plane->id;
 	u32 sprctl;
 	u32 sprsurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
@@ -486,7 +486,7 @@ ivb_update_plane(struct drm_plane *plane,
 	enum pipe pipe = intel_plane->pipe;
 	u32 sprctl, sprscale = 0;
 	u32 sprsurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
@@ -624,7 +624,7 @@ ilk_update_plane(struct drm_plane *plane,
 	int pipe = intel_plane->pipe;
 	u32 dvscntr, dvsscale;
 	u32 dvssurf_offset, linear_offset;
-	unsigned int rotation = plane_state->base.rotation;
+	unsigned int rotation = intel_plane_get_rotation(plane_state);
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
@@ -748,6 +748,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
+	unsigned int rotation = intel_plane_get_rotation(state);
 	int crtc_x, crtc_y;
 	unsigned int crtc_w, crtc_h;
 	uint32_t src_x, src_y, src_w, src_h;
@@ -802,8 +803,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	 * coordinates and sizes. We probably need some way to decide whether
 	 * more strict checking should be done instead.
 	 */
-	drm_rect_rotate(src, fb->width << 16, fb->height << 16,
-			state->base.rotation);
+	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
 
 	hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
 	BUG_ON(hscale < 0);
@@ -844,7 +844,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 				     drm_rect_height(dst) * vscale - drm_rect_height(src));
 
 		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
-				    state->base.rotation);
+				    rotation);
 
 		/* sanity check to make sure the src viewport wasn't enlarged */
 		WARN_ON(src->x1 < (int) state->base.src_x ||
-- 
2.12.2

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

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Jani Nikula]

On Sun, 07 May 2017, Hans de Goede <hdegoede@redhat.com> wrote:
> @@ -1403,6 +1410,31 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
>  	return i915_ggtt_offset(state->vma);
>  }
>  
> +static inline unsigned int
> +intel_plane_get_rotation(const struct intel_plane_state *plane_state)
> +{

Random drive-by bikeshed, is this really worth the inline?

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Hans de Goede]

Hi,

On 08-05-17 10:25, Jani Nikula wrote:
> On Sun, 07 May 2017, Hans de Goede <hdegoede@redhat.com> wrote:
>> @@ -1403,6 +1410,31 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
>>   	return i915_ggtt_offset(state->vma);
>>   }
>>   
>> +static inline unsigned int
>> +intel_plane_get_rotation(const struct intel_plane_state *plane_state)
>> +{
> 
> Random drive-by bikeshed, is this really worth the inline?

It is small and this seemed a convenient way to answer the
"where to put this" question, but I'm fine with having it as
a regular function instead.

I guess it should go to intel_atomic_plane.c then?

Regards,

Hans

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

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Ville Syrjälä]

On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
> On some (Bay Trail) devices the LCD panel is mounted upside-down.
> 
> This commit uses the code to read back the initial rotation of the
> primary plane in get_initial_plane_config from Ville Syrjala's
> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
> initial fb it stores that in intel_crtc.initial_rotation.
> 
> It adds an intel_plane_get_rotation helper which combines this
> initial_rotation with any rotation requested by userspace and
> uses this in all places which look at a planes rotation, thus
> transparently dealing with upside-down LCD panels without requiring
> any user-space or fbcon changes.
> 
> This fixes the kernel boot messages switching from being shown the right
> way up in efifb to being shown upside down as soon as a native kms driver
> loads, as well as any graphics displayed by userspace being upside-down.
> 
> Note this only deals with upside-down LCD panels / 180 degrees
> rotation as the hardware in question only supports 180 degrees
> rotation in hardware. The one model known which has a panel mounted
> with 90/270 degrees rotation will need to be fully dealt with in
> userspace, even the firmware boot screen / menus are rotated 90 degrees
> on this one and there simply is nothing the kernel can do about this.

This pretty much looks like a very limited attempt at full pipe
rotation. I have posted a more generic version of that in the past
but it was pretty much shot down by everyone else.

So I'm not convinced this apporach is really the way to go. There
are a few limitations even for the 180 degree rotation so trying
to hide it inside the kernel isn't 100% possible. 

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

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Hans de Goede]

Hi,

On 08-05-17 12:44, Ville Syrjälä wrote:
> On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
>> On some (Bay Trail) devices the LCD panel is mounted upside-down.
>>
>> This commit uses the code to read back the initial rotation of the
>> primary plane in get_initial_plane_config from Ville Syrjala's
>> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
>> initial fb it stores that in intel_crtc.initial_rotation.
>>
>> It adds an intel_plane_get_rotation helper which combines this
>> initial_rotation with any rotation requested by userspace and
>> uses this in all places which look at a planes rotation, thus
>> transparently dealing with upside-down LCD panels without requiring
>> any user-space or fbcon changes.
>>
>> This fixes the kernel boot messages switching from being shown the right
>> way up in efifb to being shown upside down as soon as a native kms driver
>> loads, as well as any graphics displayed by userspace being upside-down.
>>
>> Note this only deals with upside-down LCD panels / 180 degrees
>> rotation as the hardware in question only supports 180 degrees
>> rotation in hardware. The one model known which has a panel mounted
>> with 90/270 degrees rotation will need to be fully dealt with in
>> userspace, even the firmware boot screen / menus are rotated 90 degrees
>> on this one and there simply is nothing the kernel can do about this.
> 
> This pretty much looks like a very limited attempt at full pipe
> rotation.

Correct, it is limited to 180 degree rotation because that is what
almost all hardware out there which needs rotation to correct
for LCD-panel mounting needs and all pipes on GEN4+ support
180 degree rotation and on GEN < 4 we will never set initial_rotation
so this patch is a nop.

> I have posted a more generic version of that in the past
> but it was pretty much shot down by everyone else.

I agree that trying to deal with 90/270 degree rotation in the
kernel is not a good idea, but 180 degree rotation is much
easier to deal with and all devices I know of where the kernel
can even detect it needs to do rotation use 180 degree rotation.

> So I'm not convinced this apporach is really the way to go.

This ways in at about the same amount of lines added as the previous
2 patches, but unlike the previous 2 patches it actually fully solves
the problem. As my testing has shown we really should not punt this
problem to userspace because then we will get a never ending job
of needing to fix the userspace kms consumer of the week. I'm aware
of at least 6 implementations which would need fixing without
even trying: intel-ddx modesetting-ddx gnome-wayland-compositor
kde-wayland-compositor, sway-wayland-compositor, weston.

> There are a few limitations even for the 180 degree rotation so
> trying to hide it inside the kernel isn't 100% possible.

Looking at supported_rotations for all planes I don't see any
limitations, or are the maybe stride limitations or some such ?

The only limitation I see is that on Cherry Trail Pipe B
supports reflect-x but not when combined with 180 degree
rotation, this will get caught in intel_plane_atomic_check_with_state
just as before only now reflect cannot be combined with
0 degree rotation instead of 180 degree rotation.

Regards,

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

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Chris Wilson]

On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
> On some (Bay Trail) devices the LCD panel is mounted upside-down.
> 
> This commit uses the code to read back the initial rotation of the
> primary plane in get_initial_plane_config from Ville Syrjala's
> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
> initial fb it stores that in intel_crtc.initial_rotation.
> 
> It adds an intel_plane_get_rotation helper which combines this
> initial_rotation with any rotation requested by userspace and
> uses this in all places which look at a planes rotation, thus
> transparently dealing with upside-down LCD panels without requiring
> any user-space or fbcon changes.
> 
> This fixes the kernel boot messages switching from being shown the right
> way up in efifb to being shown upside down as soon as a native kms driver
> loads, as well as any graphics displayed by userspace being upside-down.
> 
> Note this only deals with upside-down LCD panels / 180 degrees
> rotation as the hardware in question only supports 180 degrees
> rotation in hardware. The one model known which has a panel mounted
> with 90/270 degrees rotation will need to be fully dealt with in
> userspace, even the firmware boot screen / menus are rotated 90 degrees
> on this one and there simply is nothing the kernel can do about this.

I shall just mention a concern with hiding the transformation on the
connection, we do expose the subpixel layout to userspace and that
should be adjusted based on this lie. There are probably other places
where the orientation of the output leaks through the current interface.

The commit message fails to explain how userspace, which should already
have the tools available to it, is unable to reapply the existing
rotation - i.e. why this is a kernel problem, and whether by hiding this
from userspace we are not trapping ourselves with warts in the ABI.
-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: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Hans de Goede]

HI,

On 08-05-17 14:27, Chris Wilson wrote:
> On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
>> On some (Bay Trail) devices the LCD panel is mounted upside-down.
>>
>> This commit uses the code to read back the initial rotation of the
>> primary plane in get_initial_plane_config from Ville Syrjala's
>> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
>> initial fb it stores that in intel_crtc.initial_rotation.
>>
>> It adds an intel_plane_get_rotation helper which combines this
>> initial_rotation with any rotation requested by userspace and
>> uses this in all places which look at a planes rotation, thus
>> transparently dealing with upside-down LCD panels without requiring
>> any user-space or fbcon changes.
>>
>> This fixes the kernel boot messages switching from being shown the right
>> way up in efifb to being shown upside down as soon as a native kms driver
>> loads, as well as any graphics displayed by userspace being upside-down.
>>
>> Note this only deals with upside-down LCD panels / 180 degrees
>> rotation as the hardware in question only supports 180 degrees
>> rotation in hardware. The one model known which has a panel mounted
>> with 90/270 degrees rotation will need to be fully dealt with in
>> userspace, even the firmware boot screen / menus are rotated 90 degrees
>> on this one and there simply is nothing the kernel can do about this.
> 
> I shall just mention a concern with hiding the transformation on the
> connection, we do expose the subpixel layout to userspace and that
> should be adjusted based on this lie. There are probably other places
> where the orientation of the output leaks through the current interface.
> 
> The commit message fails to explain how userspace, which should already
> have the tools available to it, is unable to reapply the existing
> rotation - i.e. why this is a kernel problem,

This is a kernel problem because one of the things the kernel does
is hardware abstraction, as I mentioned in my reply to Ville, there is
not single userspace to fix here, there is a large supply of userspace
consumers of the kms API which need fixing to fix this in userspace.

For touchscreens which are mounted with a wrong orientation the
input layer fixes things up, I don't see how this is any different
really. Now if it was impossible or very complicated to fix this
in the kernel that would be a different story, but as this patch
shows it is quite doable to fix this in the kernel.

Regards,

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

Re: [PATCH 2/2] drm/i915: Deal with upside-down mounted LCD panels

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 3029 bytes --]

Hans de Goede <hdegoede@redhat.com> writes:

> HI,
>
> On 08-05-17 14:27, Chris Wilson wrote:
>> On Sun, May 07, 2017 at 11:10:56AM +0200, Hans de Goede wrote:
>>> On some (Bay Trail) devices the LCD panel is mounted upside-down.
>>>
>>> This commit uses the code to read back the initial rotation of the
>>> primary plane in get_initial_plane_config from Ville Syrjala's
>>> "drm/fb-helper: Inherit rotation wip" patch and when re-using the
>>> initial fb it stores that in intel_crtc.initial_rotation.
>>>
>>> It adds an intel_plane_get_rotation helper which combines this
>>> initial_rotation with any rotation requested by userspace and
>>> uses this in all places which look at a planes rotation, thus
>>> transparently dealing with upside-down LCD panels without requiring
>>> any user-space or fbcon changes.
>>>
>>> This fixes the kernel boot messages switching from being shown the right
>>> way up in efifb to being shown upside down as soon as a native kms driver
>>> loads, as well as any graphics displayed by userspace being upside-down.
>>>
>>> Note this only deals with upside-down LCD panels / 180 degrees
>>> rotation as the hardware in question only supports 180 degrees
>>> rotation in hardware. The one model known which has a panel mounted
>>> with 90/270 degrees rotation will need to be fully dealt with in
>>> userspace, even the firmware boot screen / menus are rotated 90 degrees
>>> on this one and there simply is nothing the kernel can do about this.
>> 
>> I shall just mention a concern with hiding the transformation on the
>> connection, we do expose the subpixel layout to userspace and that
>> should be adjusted based on this lie. There are probably other places
>> where the orientation of the output leaks through the current interface.
>> 
>> The commit message fails to explain how userspace, which should already
>> have the tools available to it, is unable to reapply the existing
>> rotation - i.e. why this is a kernel problem,
>
> This is a kernel problem because one of the things the kernel does
> is hardware abstraction, as I mentioned in my reply to Ville, there is
> not single userspace to fix here, there is a large supply of userspace
> consumers of the kms API which need fixing to fix this in userspace.
>
> For touchscreens which are mounted with a wrong orientation the
> input layer fixes things up, I don't see how this is any different
> really. Now if it was impossible or very complicated to fix this
> in the kernel that would be a different story, but as this patch
> shows it is quite doable to fix this in the kernel.

FWIW, Raspberry Pi has a similar problem: The panel has an appropriate
orientation due to its viewing angle, but the cabling is such that many
people mount it upside down and use a firmware configuration to have
everything flipped.  It would be nice if I could do readback of the
panel's orientation reg (I haven't tested) and retain automatically that
after boot, as a default policy.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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