All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc.
@ 2017-10-13 13:58 Ville Syrjala
  2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
                   ` (17 more replies)
  0 siblings, 18 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter, Alex Villacís Lasso

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

Respin of the plane readout/sanitation series. I tried to address all of
Daniel's review comments, and tossed in the requested plane state
verification patch on top. Also dropped the primary plane windowing patch
for now.

Entire series available here:
git://github.com/vsyrjala/linux.git plane_sanitation_3

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Ville Syrjälä (10):
  drm/i915: Add .get_hw_state() method for planes
  drm/i915: Redo plane sanitation during readout
  drm/i915: s/enum plane/enum i9xx_plane_id/
  drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
  drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in
    initial fb readout
  drm/i915: Nuke ironlake_get_initial_plane_config()
  drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
  drm/i915: Nuke crtc->plane
  drm/i915: Use plane->get_hw_state() for initial plane fb readout
  drm/i915: Add rudimentary plane state verification

 drivers/gpu/drm/i915/i915_drv.h      |  16 +-
 drivers/gpu/drm/i915/intel_display.c | 489 +++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |   8 +-
 drivers/gpu/drm/i915/intel_fbc.c     |  27 +-
 drivers/gpu/drm/i915/intel_pm.c      |  36 +--
 drivers/gpu/drm/i915/intel_sprite.c  |  43 +++
 6 files changed, 299 insertions(+), 320 deletions(-)

-- 
2.13.6

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 17:36   ` [PATCH v3 " Ville Syrjala
  2017-10-23 14:50   ` [PATCH v4 " Ville Syrjala
  2017-10-13 13:58 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
                   ` (16 subsequent siblings)
  17 siblings, 2 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Alex Villacís Lasso

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

Add a .get_hw_state() method for planes, returning true or false
depending on whether the plane is enabled. Use it to rewrite the
plane enabled/disabled asserts in platform agnostic fashion.

We do lose the pre-gen4 plane<->pipe mapping checks, but since we're
supposed sanitize that anyway it doesn't really matter.

v2: Reoder patches to not depend on enum old_plane_id
    Just call assert_plane_disabled() from assert_planes_disabled()

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 155 +++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 drivers/gpu/drm/i915/intel_sprite.c  |  43 ++++++++++
 3 files changed, 101 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7e91dc9a0fcf..212595fe6100 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1192,23 +1192,6 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 	     pipe_name(pipe));
 }
 
-static void assert_cursor(struct drm_i915_private *dev_priv,
-			  enum pipe pipe, bool state)
-{
-	bool cur_state;
-
-	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		cur_state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
-	else
-		cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
-
-	I915_STATE_WARN(cur_state != state,
-	     "cursor on pipe %c assertion failure (expected %s, current %s)\n",
-			pipe_name(pipe), onoff(state), onoff(cur_state));
-}
-#define assert_cursor_enabled(d, p) assert_cursor(d, p, true)
-#define assert_cursor_disabled(d, p) assert_cursor(d, p, false)
-
 void assert_pipe(struct drm_i915_private *dev_priv,
 		 enum pipe pipe, bool state)
 {
@@ -1236,77 +1219,25 @@ void assert_pipe(struct drm_i915_private *dev_priv,
 			pipe_name(pipe), onoff(state), onoff(cur_state));
 }
 
-static void assert_plane(struct drm_i915_private *dev_priv,
-			 enum plane plane, bool state)
+static void assert_plane(struct intel_plane *plane, bool state)
 {
-	u32 val;
-	bool cur_state;
+	bool cur_state = plane->get_hw_state(plane);
 
-	val = I915_READ(DSPCNTR(plane));
-	cur_state = !!(val & DISPLAY_PLANE_ENABLE);
 	I915_STATE_WARN(cur_state != state,
-	     "plane %c assertion failure (expected %s, current %s)\n",
-			plane_name(plane), onoff(state), onoff(cur_state));
+			"%s assertion failure (expected %s, current %s)\n",
+			plane->base.name, onoff(state), onoff(cur_state));
 }
 
-#define assert_plane_enabled(d, p) assert_plane(d, p, true)
-#define assert_plane_disabled(d, p) assert_plane(d, p, false)
+#define assert_plane_enabled(p) assert_plane(p, true)
+#define assert_plane_disabled(p) assert_plane(p, false)
 
-static void assert_planes_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe)
+static void assert_planes_disabled(struct intel_crtc *crtc)
 {
-	int i;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_plane *plane;
 
-	/* Primary planes are fixed to pipes on gen4+ */
-	if (INTEL_GEN(dev_priv) >= 4) {
-		u32 val = I915_READ(DSPCNTR(pipe));
-		I915_STATE_WARN(val & DISPLAY_PLANE_ENABLE,
-		     "plane %c assertion failure, should be disabled but not\n",
-		     plane_name(pipe));
-		return;
-	}
-
-	/* Need to check both planes against the pipe */
-	for_each_pipe(dev_priv, i) {
-		u32 val = I915_READ(DSPCNTR(i));
-		enum pipe cur_pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
-			DISPPLANE_SEL_PIPE_SHIFT;
-		I915_STATE_WARN((val & DISPLAY_PLANE_ENABLE) && pipe == cur_pipe,
-		     "plane %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(i), pipe_name(pipe));
-	}
-}
-
-static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
-				    enum pipe pipe)
-{
-	int sprite;
-
-	if (INTEL_GEN(dev_priv) >= 9) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(PLANE_CTL(pipe, sprite));
-			I915_STATE_WARN(val & PLANE_CTL_ENABLE,
-			     "plane %d assertion failure, should be off on pipe %c but is still active\n",
-			     sprite, pipe_name(pipe));
-		}
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(SPCNTR(pipe, PLANE_SPRITE0 + sprite));
-			I915_STATE_WARN(val & SP_ENABLE,
-			     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-			     sprite_name(pipe, sprite), pipe_name(pipe));
-		}
-	} else if (INTEL_GEN(dev_priv) >= 7) {
-		u32 val = I915_READ(SPRCTL(pipe));
-		I915_STATE_WARN(val & SPRITE_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	} else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
-		u32 val = I915_READ(DVSCNTR(pipe));
-		I915_STATE_WARN(val & DVS_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	}
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane)
+		assert_plane_disabled(plane);
 }
 
 static void assert_vblank_disabled(struct drm_crtc *crtc)
@@ -1899,9 +1830,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 
 	DRM_DEBUG_KMS("enabling pipe %c\n", pipe_name(pipe));
 
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	/*
 	 * A pipe without a PLL won't actually be able to drive bits from
@@ -1971,9 +1900,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	 * Make sure planes won't keep trying to pump pixels to us,
 	 * or we might hang the display.
 	 */
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
@@ -3370,6 +3297,14 @@ static void i9xx_disable_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+{
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	enum plane plane = primary->plane;
+
+	return I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+}
+
 static u32
 intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
 {
@@ -3638,6 +3573,15 @@ static void skylake_disable_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool skylake_primary_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
+	enum plane_id plane_id = plane->id;
+
+	return I915_READ(PLANE_CTL(pipe, plane_id)) & PLANE_CTL_ENABLE;
+}
+
 static int
 __intel_display_resume(struct drm_device *dev,
 		       struct drm_atomic_state *state,
@@ -4944,7 +4888,8 @@ void hsw_enable_ips(struct intel_crtc *crtc)
 	 * a vblank wait.
 	 */
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
@@ -4977,7 +4922,8 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	if (!crtc->config->ips_enabled)
 		return;
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0));
@@ -9557,6 +9503,13 @@ static void i845_disable_cursor(struct intel_plane *plane,
 	i845_update_cursor(plane, NULL, NULL);
 }
 
+static bool i845_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+
+	return I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
+}
+
 static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9750,6 +9703,13 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
 	i9xx_update_cursor(plane, NULL, NULL);
 }
 
+static bool i9xx_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
+
+	return I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
+}
 
 /* VESA 640x480x72Hz mode to set on the pipe */
 static const struct drm_display_mode load_detect_mode = {
@@ -13231,6 +13191,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
+		primary->get_hw_state = skylake_primary_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 9) {
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
@@ -13241,6 +13202,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
+		primary->get_hw_state = skylake_primary_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
@@ -13248,6 +13210,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
@@ -13255,6 +13218,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 9)
@@ -13344,10 +13308,12 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
 		cursor->update_plane = i845_update_cursor;
 		cursor->disable_plane = i845_disable_cursor;
+		cursor->get_hw_state = i845_cursor_get_hw_state;
 		cursor->check_plane = i845_check_cursor;
 	} else {
 		cursor->update_plane = i9xx_update_cursor;
 		cursor->disable_plane = i9xx_disable_cursor;
+		cursor->get_hw_state = i9xx_cursor_get_hw_state;
 		cursor->check_plane = i9xx_check_cursor;
 	}
 
@@ -14695,8 +14661,8 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
 		      pipe_name(pipe));
 
-	assert_plane_disabled(dev_priv, PLANE_A);
-	assert_plane_disabled(dev_priv, PLANE_B);
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
 
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
@@ -14910,20 +14876,13 @@ void i915_redisable_vga(struct drm_i915_private *dev_priv)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_VGA);
 }
 
-static bool primary_get_hw_state(struct intel_plane *plane)
-{
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-
-	return I915_READ(DSPCNTR(plane->plane)) & DISPLAY_PLANE_ENABLE;
-}
-
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
 	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
 	bool visible;
 
-	visible = crtc->active && primary_get_hw_state(primary);
+	visible = crtc->active && primary->get_hw_state(primary);
 
 	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
 				to_intel_plane_state(primary->base.state),
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d61985f93d40..508b8e2eb4cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -871,6 +871,7 @@ struct intel_plane {
 			     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct intel_plane *plane,
 			      struct intel_crtc *crtc);
+	bool (*get_hw_state)(struct intel_plane *plane);
 	int (*check_plane)(struct intel_plane *plane,
 			   struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 86fc9b529f2d..62fe84d3679f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -329,6 +329,16 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+skl_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+
+	return I915_READ(PLANE_CTL(pipe, plane_id)) & PLANE_CTL_ENABLE;
+}
+
 static void
 chv_update_csc(struct intel_plane *plane, uint32_t format)
 {
@@ -506,6 +516,16 @@ vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+vlv_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+
+	return I915_READ(SPCNTR(pipe, plane_id)) & SP_ENABLE;
+}
+
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -646,6 +666,15 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+ivb_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
+
+	return I915_READ(SPRCTL(pipe)) & SPRITE_ENABLE;
+}
+
 static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -777,6 +806,15 @@ g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+g4x_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum pipe pipe = plane->pipe;
+
+	return I915_READ(DVSCNTR(pipe)) & DVS_ENABLE;
+}
+
 static int
 intel_check_sprite_plane(struct intel_plane *plane,
 			 struct intel_crtc_state *crtc_state,
@@ -1232,6 +1270,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1242,6 +1281,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1252,6 +1292,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = vlv_update_plane;
 		intel_plane->disable_plane = vlv_disable_plane;
+		intel_plane->get_hw_state = vlv_plane_get_hw_state;
 
 		plane_formats = vlv_plane_formats;
 		num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
@@ -1267,6 +1308,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = ivb_update_plane;
 		intel_plane->disable_plane = ivb_disable_plane;
+		intel_plane->get_hw_state = ivb_plane_get_hw_state;
 
 		plane_formats = snb_plane_formats;
 		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
@@ -1277,6 +1319,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = g4x_update_plane;
 		intel_plane->disable_plane = g4x_disable_plane;
+		intel_plane->get_hw_state = g4x_plane_get_hw_state;
 
 		modifiers = i9xx_plane_format_modifiers;
 		if (IS_GEN6(dev_priv)) {
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
  2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 13:58 ` [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Alex Villacís Lasso

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

Unify the plane disabling during state readout by pulling the code into
a new helper intel_plane_disable_noatomic(). We'll also read out the
state of all planes, so that we know which planes really need to be
diabled.

Additonally we change the plane<->pipe mapping sanitation to work by
simply disabling the offending planes instead of entire pipes. And
we do it before we otherwise sanitize the crtcs, which means we don't
have to worry about misassigned planes during crtc sanitation anymore.

v2: Reoder patches to not depend on enum old_plane_id
v3: s/for_each_pipe/for_each_intel_crtc/

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103223
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/gpu/drm/i915/intel_display.c | 114 ++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 212595fe6100..1daec8b967a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2729,6 +2729,23 @@ intel_set_plane_visible(struct intel_crtc_state *crtc_state,
 		      crtc_state->active_planes);
 }
 
+static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
+					 struct intel_plane *plane)
+{
+	struct intel_crtc_state *crtc_state =
+		to_intel_crtc_state(crtc->base.state);
+	struct intel_plane_state *plane_state =
+		to_intel_plane_state(plane->base.state);
+
+	intel_set_plane_visible(crtc_state, plane_state, false);
+
+	if (plane->id == PLANE_PRIMARY)
+		intel_pre_disable_primary_noatomic(&crtc->base);
+
+	trace_intel_disable_plane(&plane->base, crtc);
+	plane->disable_plane(plane, crtc);
+}
+
 static void
 intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 			     struct intel_initial_plane_config *plane_config)
@@ -2786,12 +2803,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * simplest solution is to just disable the primary plane now and
 	 * pretend the BIOS never had it enabled.
 	 */
-	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
-				to_intel_plane_state(plane_state),
-				false);
-	intel_pre_disable_primary_noatomic(&intel_crtc->base);
-	trace_intel_disable_plane(primary, intel_crtc);
-	intel_plane->disable_plane(intel_plane, intel_crtc);
+	intel_plane_disable_noatomic(intel_crtc, intel_plane);
 
 	return;
 
@@ -5923,6 +5935,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	enum intel_display_power_domain domain;
+	struct intel_plane *plane;
 	u64 domains;
 	struct drm_atomic_state *state;
 	struct intel_crtc_state *crtc_state;
@@ -5931,11 +5944,12 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	if (!intel_crtc->active)
 		return;
 
-	if (crtc->primary->state->visible) {
-		intel_pre_disable_primary_noatomic(crtc);
+	for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, plane) {
+		const struct intel_plane_state *plane_state =
+			to_intel_plane_state(plane->base.state);
 
-		intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary));
-		crtc->primary->state->visible = false;
+		if (plane_state->base.visible)
+			intel_plane_disable_noatomic(intel_crtc, plane);
 	}
 
 	state = drm_atomic_state_alloc(crtc->dev);
@@ -14674,22 +14688,36 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	POSTING_READ(DPLL(pipe));
 }
 
-static bool
-intel_check_plane_mapping(struct intel_crtc *crtc)
+static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
+				   struct intel_plane *primary)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 val;
+	enum plane plane = primary->plane;
+	u32 val = I915_READ(DSPCNTR(plane));
 
-	if (INTEL_INFO(dev_priv)->num_pipes == 1)
-		return true;
+	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
+		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
+}
 
-	val = I915_READ(DSPCNTR(!crtc->plane));
+static void
+intel_sanitize_plane_mapping(struct drm_i915_private *dev_priv)
+{
+	struct intel_crtc *crtc;
 
-	if ((val & DISPLAY_PLANE_ENABLE) &&
-	    (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
-		return false;
+	if (INTEL_GEN(dev_priv) >= 4)
+		return;
 
-	return true;
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		struct intel_plane *plane =
+			to_intel_plane(crtc->base.primary);
+
+		if (intel_plane_mapping_ok(crtc, plane))
+			continue;
+
+		DRM_DEBUG_KMS("%s attached to the wrong pipe, disabling plane\n",
+			      plane->base.name);
+		intel_plane_disable_noatomic(crtc, plane);
+	}
 }
 
 static bool intel_crtc_has_encoders(struct intel_crtc *crtc)
@@ -14745,33 +14773,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 
 		/* Disable everything but the primary plane */
 		for_each_intel_plane_on_crtc(dev, crtc, plane) {
-			if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
-				continue;
+			const struct intel_plane_state *plane_state =
+				to_intel_plane_state(plane->base.state);
 
-			trace_intel_disable_plane(&plane->base, crtc);
-			plane->disable_plane(plane, crtc);
+			if (plane_state->base.visible &&
+			    plane->base.type != DRM_PLANE_TYPE_PRIMARY)
+				intel_plane_disable_noatomic(crtc, plane);
 		}
 	}
 
-	/* We need to sanitize the plane -> pipe mapping first because this will
-	 * disable the crtc (and hence change the state) if it is wrong. Note
-	 * that gen4+ has a fixed plane -> pipe mapping.  */
-	if (INTEL_GEN(dev_priv) < 4 && !intel_check_plane_mapping(crtc)) {
-		bool plane;
-
-		DRM_DEBUG_KMS("[CRTC:%d:%s] wrong plane connection detected!\n",
-			      crtc->base.base.id, crtc->base.name);
-
-		/* Pipe has the wrong plane attached and the plane is active.
-		 * Temporarily change the plane mapping and disable everything
-		 * ...  */
-		plane = crtc->plane;
-		crtc->base.primary->state->visible = true;
-		crtc->plane = !plane;
-		intel_crtc_disable_noatomic(&crtc->base, ctx);
-		crtc->plane = plane;
-	}
-
 	/* Adjust the state of the output pipe according to whether we
 	 * have active connectors/encoders. */
 	if (crtc->active && !intel_crtc_has_encoders(crtc))
@@ -14879,14 +14889,18 @@ void i915_redisable_vga(struct drm_i915_private *dev_priv)
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
-	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
-	bool visible;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_crtc_state *crtc_state =
+		to_intel_crtc_state(crtc->base.state);
+	struct intel_plane *plane;
 
-	visible = crtc->active && primary->get_hw_state(primary);
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+		struct intel_plane_state *plane_state =
+			to_intel_plane_state(plane->base.state);
+		bool visible = plane->get_hw_state(plane);
 
-	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
-				to_intel_plane_state(primary->base.state),
-				visible);
+		intel_set_plane_visible(crtc_state, plane_state, visible);
+	}
 }
 
 static void intel_modeset_readout_hw_state(struct drm_device *dev)
@@ -15075,6 +15089,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	/* HW state is read out, now we need to sanitize this mess. */
 	get_encoder_power_domains(dev_priv);
 
+	intel_sanitize_plane_mapping(dev_priv);
+
 	for_each_intel_encoder(dev, encoder) {
 		intel_sanitize_encoder(encoder);
 	}
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
  2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
  2017-10-13 13:58 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 17:37   ` [PATCH v4 " Ville Syrjala
  2017-10-23 14:50   ` [PATCH v5 " Ville Syrjala
  2017-10-13 13:58 ` [PATCH v2 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
                   ` (14 subsequent siblings)
  17 siblings, 2 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx

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

Rename enum plane to enum i9xx_plane_id to make it clear that it only
applies to pre-SKL platforms.

enum i9xx_plane_id is a global identifier, whereas enum plane_id is
per-pipe. We need the old global identifier to index the primary plane
(and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
platforms.

v2: Reorder patches
v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
    Pimp the commit message a bit
    Note that i9xx_plane_id doesn't apply to SKL+

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 +--
 drivers/gpu/drm/i915/intel_display.c | 84 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +--
 3 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c7b2ca6aff05..2a89cbbb7a56 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -305,9 +305,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
 
 /*
  * Global legacy plane identifier. Valid only for primary/sprite
- * planes on pre-g4x, and only for primary planes on g4x+.
+ * planes on pre-g4x, and only for primary planes on g4x-bdw.
  */
-enum plane {
+enum i9xx_plane_id {
 	PLANE_A,
 	PLANE_B,
 	PLANE_C,
@@ -1128,7 +1128,7 @@ struct intel_fbc {
 
 		struct {
 			enum pipe pipe;
-			enum plane plane;
+			enum i9xx_plane_id plane;
 			unsigned int fence_y_offset;
 		} crtc;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1daec8b967a7..75d8fc78aa89 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3223,17 +3223,17 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct intel_plane *primary,
-				      const struct intel_crtc_state *crtc_state,
-				      const struct intel_plane_state *plane_state)
+static void i9xx_update_plane(struct intel_plane *plane,
+			      const struct intel_crtc_state *crtc_state,
+			      const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	enum plane plane = primary->plane;
+	enum i9xx_plane_id plane_id = plane->plane;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
-	i915_reg_t reg = DSPCNTR(plane);
+	i915_reg_t reg = DSPCNTR(plane_id);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
 	unsigned long irqflags;
@@ -3254,34 +3254,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
 		 */
-		I915_WRITE_FW(DSPSIZE(plane),
+		I915_WRITE_FW(DSPSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(DSPPOS(plane), 0);
-	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
-		I915_WRITE_FW(PRIMSIZE(plane),
+		I915_WRITE_FW(DSPPOS(plane_id), 0);
+	} else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
+		I915_WRITE_FW(PRIMSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(PRIMPOS(plane), 0);
-		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
+		I915_WRITE_FW(PRIMPOS(plane_id), 0);
+		I915_WRITE_FW(PRIMCNSTALPHA(plane_id), 0);
 	}
 
 	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSTRIDE(plane_id), fb->pitches[0]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
-		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPOFFSET(plane_id), (y << 16) | x);
 	} else if (INTEL_GEN(dev_priv) >= 4) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
-		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
+		I915_WRITE_FW(DSPTILEOFF(plane_id), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(plane_id), linear_offset);
 	} else {
-		I915_WRITE_FW(DSPADDR(plane),
+		I915_WRITE_FW(DSPADDR(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
 	}
@@ -3290,31 +3290,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void i9xx_disable_primary_plane(struct intel_plane *primary,
-				       struct intel_crtc *crtc)
+static void i9xx_disable_plane(struct intel_plane *plane,
+			       struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
-	enum plane plane = primary->plane;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum i9xx_plane_id plane_id = plane->plane;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE_FW(DSPCNTR(plane), 0);
-	if (INTEL_INFO(dev_priv)->gen >= 4)
-		I915_WRITE_FW(DSPSURF(plane), 0);
+	I915_WRITE_FW(DSPCNTR(plane_id), 0);
+	if (INTEL_GEN(dev_priv) >= 4)
+		I915_WRITE_FW(DSPSURF(plane_id), 0);
 	else
-		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
+		I915_WRITE_FW(DSPADDR(plane_id), 0);
+	POSTING_READ_FW(DSPCNTR(plane_id));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
-	enum plane plane = primary->plane;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum i9xx_plane_id plane_id = plane->plane;
 
-	return I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+	return I915_READ(DSPCNTR(plane_id)) & DISPLAY_PLANE_ENABLE;
 }
 
 static u32
@@ -13191,9 +13191,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
 	 */
 	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
-		primary->plane = (enum plane) !pipe;
+		primary->plane = (enum i9xx_plane_id) !pipe;
 	else
-		primary->plane = (enum plane) pipe;
+		primary->plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
@@ -13222,16 +13222,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		num_formats = ARRAY_SIZE(i965_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
@@ -13315,7 +13315,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	cursor->can_scale = false;
 	cursor->max_downscale = 1;
 	cursor->pipe = pipe;
-	cursor->plane = pipe;
+	cursor->plane = (enum i9xx_plane_id) pipe;
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 
@@ -14689,11 +14689,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
-				   struct intel_plane *primary)
+				   struct intel_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum plane plane = primary->plane;
-	u32 val = I915_READ(DSPCNTR(plane));
+	enum i9xx_plane_id plane_id = plane->plane;
+	u32 val = I915_READ(DSPCNTR(plane_id));
 
 	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
 		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 508b8e2eb4cf..dc0b60a7f13f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -796,7 +796,7 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum plane plane;
+	enum i9xx_plane_id plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
@@ -849,7 +849,7 @@ struct intel_crtc {
 
 struct intel_plane {
 	struct drm_plane base;
-	u8 plane;
+	enum i9xx_plane_id plane;
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
@@ -1136,7 +1136,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static inline struct intel_crtc *
-intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
+intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
 {
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (2 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 13:58 ` [PATCH v3 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Replace the 0 and 1 with PLANE_A and PLANE_B in the pre-g4x wm code.

v2: s/old_plane_id/i9xx_plane_id/ (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 ++-
 drivers/gpu/drm/i915/intel_pm.c | 36 +++++++++++++++++++-----------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a89cbbb7a56..81c1ece8b3ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -699,7 +699,8 @@ struct drm_i915_display_funcs {
 			  struct intel_cdclk_state *cdclk_state);
 	void (*set_cdclk)(struct drm_i915_private *dev_priv,
 			  const struct intel_cdclk_state *cdclk_state);
-	int (*get_fifo_size)(struct drm_i915_private *dev_priv, int plane);
+	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
+			     enum i9xx_plane_id plane_id);
 	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
 	int (*compute_intermediate_wm)(struct drm_device *dev,
 				       struct intel_crtc *intel_crtc,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2fcff9788b6f..0e4e4eb4bfcf 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -521,38 +521,41 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
 	fifo_state->plane[PLANE_CURSOR] = 63;
 }
 
-static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
+static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv,
+			      enum i9xx_plane_id plane_id)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
-	if (plane)
+	if (plane_id == PLANE_B)
 		size = ((dsparb >> DSPARB_CSTART_SHIFT) & 0x7f) - size;
 
-	DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
-		      plane ? "B" : "A", size);
+	DRM_DEBUG_KMS("FIFO size - (0x%08x) %c: %d\n",
+		      dsparb, plane_name(plane_id), size);
 
 	return size;
 }
 
-static int i830_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
+static int i830_get_fifo_size(struct drm_i915_private *dev_priv,
+			      enum i9xx_plane_id plane_id)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
 
 	size = dsparb & 0x1ff;
-	if (plane)
+	if (plane_id == PLANE_B)
 		size = ((dsparb >> DSPARB_BEND_SHIFT) & 0x1ff) - size;
 	size >>= 1; /* Convert to cachelines */
 
-	DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
-		      plane ? "B" : "A", size);
+	DRM_DEBUG_KMS("FIFO size - (0x%08x) %c: %d\n",
+		      dsparb, plane_name(plane_id), size);
 
 	return size;
 }
 
-static int i845_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
+static int i845_get_fifo_size(struct drm_i915_private *dev_priv,
+			      enum i9xx_plane_id plane_id)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
@@ -560,9 +563,8 @@ static int i845_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
 	size = dsparb & 0x7f;
 	size >>= 2; /* Convert to cachelines */
 
-	DRM_DEBUG_KMS("FIFO size - (0x%08x) %s: %d\n", dsparb,
-		      plane ? "B" : "A",
-		      size);
+	DRM_DEBUG_KMS("FIFO size - (0x%08x) %c: %d\n",
+		      dsparb, plane_name(plane_id), size);
 
 	return size;
 }
@@ -2261,8 +2263,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
 	else
 		wm_info = &i830_a_wm_info;
 
-	fifo_size = dev_priv->display.get_fifo_size(dev_priv, 0);
-	crtc = intel_get_crtc_for_plane(dev_priv, 0);
+	fifo_size = dev_priv->display.get_fifo_size(dev_priv, PLANE_A);
+	crtc = intel_get_crtc_for_plane(dev_priv, PLANE_A);
 	if (intel_crtc_active(crtc)) {
 		const struct drm_display_mode *adjusted_mode =
 			&crtc->config->base.adjusted_mode;
@@ -2288,8 +2290,8 @@ static void i9xx_update_wm(struct intel_crtc *unused_crtc)
 	if (IS_GEN2(dev_priv))
 		wm_info = &i830_bc_wm_info;
 
-	fifo_size = dev_priv->display.get_fifo_size(dev_priv, 1);
-	crtc = intel_get_crtc_for_plane(dev_priv, 1);
+	fifo_size = dev_priv->display.get_fifo_size(dev_priv, PLANE_B);
+	crtc = intel_get_crtc_for_plane(dev_priv, PLANE_B);
 	if (intel_crtc_active(crtc)) {
 		const struct drm_display_mode *adjusted_mode =
 			&crtc->config->base.adjusted_mode;
@@ -2401,7 +2403,7 @@ static void i845_update_wm(struct intel_crtc *unused_crtc)
 	adjusted_mode = &crtc->config->base.adjusted_mode;
 	planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
 				       &i845_wm_info,
-				       dev_priv->display.get_fifo_size(dev_priv, 0),
+				       dev_priv->display.get_fifo_size(dev_priv, PLANE_A),
 				       4, pessimal_latency_ns);
 	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
 	fwater_lo |= (3<<8) | planea_wm;
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (3 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH v2 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 13:58 ` [PATCH 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Use enum pipe, enum plane_id, and enum i9xx_plane_id consistently in the
initial framebuffe readout.

v2: Use old_plane_id in the ilk code
v3: s/old_plane_id/i9xx_plane_id/ (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75d8fc78aa89..2f3571efa246 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7456,14 +7456,16 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+	enum i9xx_plane_id plane_id = plane->plane;
+	enum pipe pipe = crtc->pipe;
 	u32 val, base, offset;
-	int pipe = crtc->pipe, plane = crtc->plane;
 	int fourcc, pixel_format;
 	unsigned int aligned_height;
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 
-	val = I915_READ(DSPCNTR(plane));
+	val = I915_READ(DSPCNTR(plane_id));
 	if (!(val & DISPLAY_PLANE_ENABLE))
 		return;
 
@@ -7490,12 +7492,12 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 
 	if (INTEL_GEN(dev_priv) >= 4) {
 		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(plane));
+			offset = I915_READ(DSPTILEOFF(plane_id));
 		else
-			offset = I915_READ(DSPLINOFF(plane));
-		base = I915_READ(DSPSURF(plane)) & 0xfffff000;
+			offset = I915_READ(DSPLINOFF(plane_id));
+		base = I915_READ(DSPSURF(plane_id)) & 0xfffff000;
 	} else {
-		base = I915_READ(DSPADDR(plane));
+		base = I915_READ(DSPADDR(plane_id));
 	}
 	plane_config->base = base;
 
@@ -7503,15 +7505,15 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->width = ((val >> 16) & 0xfff) + 1;
 	fb->height = ((val >> 0) & 0xfff) + 1;
 
-	val = I915_READ(DSPSTRIDE(pipe));
+	val = I915_READ(DSPSTRIDE(plane_id));
 	fb->pitches[0] = val & 0xffffffc0;
 
 	aligned_height = intel_fb_align_height(fb, 0, fb->height);
 
 	plane_config->size = fb->pitches[0] * aligned_height;
 
-	DRM_DEBUG_KMS("pipe/plane %c/%d with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe_name(pipe), plane, fb->width, fb->height,
+	DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      crtc->base.name, plane->base.name, fb->width, fb->height,
 		      fb->format->cpp[0] * 8, base, fb->pitches[0],
 		      plane_config->size);
 
@@ -8482,8 +8484,10 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = crtc->pipe;
 	u32 val, base, offset, stride_mult, tiling;
-	int pipe = crtc->pipe;
 	int fourcc, pixel_format;
 	unsigned int aligned_height;
 	struct drm_framebuffer *fb;
@@ -8499,7 +8503,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 
 	fb->dev = dev;
 
-	val = I915_READ(PLANE_CTL(pipe, 0));
+	val = I915_READ(PLANE_CTL(pipe, plane_id));
 	if (!(val & PLANE_CTL_ENABLE))
 		goto error;
 
@@ -8535,16 +8539,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 		goto error;
 	}
 
-	base = I915_READ(PLANE_SURF(pipe, 0)) & 0xfffff000;
+	base = I915_READ(PLANE_SURF(pipe, plane_id)) & 0xfffff000;
 	plane_config->base = base;
 
-	offset = I915_READ(PLANE_OFFSET(pipe, 0));
+	offset = I915_READ(PLANE_OFFSET(pipe, plane_id));
 
-	val = I915_READ(PLANE_SIZE(pipe, 0));
+	val = I915_READ(PLANE_SIZE(pipe, plane_id));
 	fb->height = ((val >> 16) & 0xfff) + 1;
 	fb->width = ((val >> 0) & 0x1fff) + 1;
 
-	val = I915_READ(PLANE_STRIDE(pipe, 0));
+	val = I915_READ(PLANE_STRIDE(pipe, plane_id));
 	stride_mult = intel_fb_stride_alignment(fb, 0);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
 
@@ -8552,8 +8556,8 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 
 	plane_config->size = fb->pitches[0] * aligned_height;
 
-	DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe_name(pipe), fb->width, fb->height,
+	DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      crtc->base.name, plane->base.name, fb->width, fb->height,
 		      fb->format->cpp[0] * 8, base, fb->pitches[0],
 		      plane_config->size);
 
@@ -8594,14 +8598,16 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+	enum i9xx_plane_id plane_id = plane->plane;
+	enum pipe pipe = crtc->pipe;
 	u32 val, base, offset;
-	int pipe = crtc->pipe;
 	int fourcc, pixel_format;
 	unsigned int aligned_height;
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 
-	val = I915_READ(DSPCNTR(pipe));
+	val = I915_READ(DSPCNTR(plane_id));
 	if (!(val & DISPLAY_PLANE_ENABLE))
 		return;
 
@@ -8626,14 +8632,14 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
 	fourcc = i9xx_format_to_fourcc(pixel_format);
 	fb->format = drm_format_info(fourcc);
 
-	base = I915_READ(DSPSURF(pipe)) & 0xfffff000;
+	base = I915_READ(DSPSURF(plane_id)) & 0xfffff000;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		offset = I915_READ(DSPOFFSET(pipe));
+		offset = I915_READ(DSPOFFSET(plane_id));
 	} else {
 		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(pipe));
+			offset = I915_READ(DSPTILEOFF(plane_id));
 		else
-			offset = I915_READ(DSPLINOFF(pipe));
+			offset = I915_READ(DSPLINOFF(plane_id));
 	}
 	plane_config->base = base;
 
@@ -8641,15 +8647,15 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->width = ((val >> 16) & 0xfff) + 1;
 	fb->height = ((val >> 0) & 0xfff) + 1;
 
-	val = I915_READ(DSPSTRIDE(pipe));
+	val = I915_READ(DSPSTRIDE(plane_id));
 	fb->pitches[0] = val & 0xffffffc0;
 
 	aligned_height = intel_fb_align_height(fb, 0, fb->height);
 
 	plane_config->size = fb->pitches[0] * aligned_height;
 
-	DRM_DEBUG_KMS("pipe %c with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      pipe_name(pipe), fb->width, fb->height,
+	DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
+		      crtc->base.name, plane->base.name, fb->width, fb->height,
 		      fb->format->cpp[0] * 8, base, fb->pitches[0],
 		      plane_config->size);
 
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/10] drm/i915: Nuke ironlake_get_initial_plane_config()
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (4 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH v3 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 13:58 ` [PATCH 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx

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

The only relevant difference between i9xx_get_initial_plane_config() and
ironlake_get_initial_plane_config() is the HSW/BDW TILEOFF handling.
Add that to i9xx_get_initial_plane_config() and nuke
ironlake_get_initial_plane_config().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 79 +++---------------------------------
 1 file changed, 6 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2f3571efa246..5b01245d8f0f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7490,7 +7490,10 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 	fourcc = i9xx_format_to_fourcc(pixel_format);
 	fb->format = drm_format_info(fourcc);
 
-	if (INTEL_GEN(dev_priv) >= 4) {
+	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
+		offset = I915_READ(DSPOFFSET(plane_id));
+		base = I915_READ(DSPSURF(plane_id)) & 0xfffff000;
+	} else if (INTEL_GEN(dev_priv) >= 4) {
 		if (plane_config->tiling)
 			offset = I915_READ(DSPTILEOFF(plane_id));
 		else
@@ -8592,76 +8595,6 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc,
 	}
 }
 
-static void
-ironlake_get_initial_plane_config(struct intel_crtc *crtc,
-				  struct intel_initial_plane_config *plane_config)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
-	enum i9xx_plane_id plane_id = plane->plane;
-	enum pipe pipe = crtc->pipe;
-	u32 val, base, offset;
-	int fourcc, pixel_format;
-	unsigned int aligned_height;
-	struct drm_framebuffer *fb;
-	struct intel_framebuffer *intel_fb;
-
-	val = I915_READ(DSPCNTR(plane_id));
-	if (!(val & DISPLAY_PLANE_ENABLE))
-		return;
-
-	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
-	if (!intel_fb) {
-		DRM_DEBUG_KMS("failed to alloc fb\n");
-		return;
-	}
-
-	fb = &intel_fb->base;
-
-	fb->dev = dev;
-
-	if (INTEL_GEN(dev_priv) >= 4) {
-		if (val & DISPPLANE_TILED) {
-			plane_config->tiling = I915_TILING_X;
-			fb->modifier = I915_FORMAT_MOD_X_TILED;
-		}
-	}
-
-	pixel_format = val & DISPPLANE_PIXFORMAT_MASK;
-	fourcc = i9xx_format_to_fourcc(pixel_format);
-	fb->format = drm_format_info(fourcc);
-
-	base = I915_READ(DSPSURF(plane_id)) & 0xfffff000;
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		offset = I915_READ(DSPOFFSET(plane_id));
-	} else {
-		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(plane_id));
-		else
-			offset = I915_READ(DSPLINOFF(plane_id));
-	}
-	plane_config->base = base;
-
-	val = I915_READ(PIPESRC(pipe));
-	fb->width = ((val >> 16) & 0xfff) + 1;
-	fb->height = ((val >> 0) & 0xfff) + 1;
-
-	val = I915_READ(DSPSTRIDE(plane_id));
-	fb->pitches[0] = val & 0xffffffc0;
-
-	aligned_height = intel_fb_align_height(fb, 0, fb->height);
-
-	plane_config->size = fb->pitches[0] * aligned_height;
-
-	DRM_DEBUG_KMS("%s/%s with fb: size=%dx%d@%d, offset=%x, pitch %d, size 0x%x\n",
-		      crtc->base.name, plane->base.name, fb->width, fb->height,
-		      fb->format->cpp[0] * 8, base, fb->pitches[0],
-		      plane_config->size);
-
-	plane_config->fb = intel_fb;
-}
-
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 				     struct intel_crtc_state *pipe_config)
 {
@@ -14136,7 +14069,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	} else if (HAS_DDI(dev_priv)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
-			ironlake_get_initial_plane_config;
+			i9xx_get_initial_plane_config;
 		dev_priv->display.crtc_compute_clock =
 			haswell_crtc_compute_clock;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
@@ -14144,7 +14077,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
-			ironlake_get_initial_plane_config;
+			i9xx_get_initial_plane_config;
 		dev_priv->display.crtc_compute_clock =
 			ironlake_crtc_compute_clock;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (5 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-13 13:58 ` [PATCH v2 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx

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

Stop using the old for_each_intel_plane_in_state() type iteration
macro and replace it with for_each_new_intel_plane_in_state().
And similarly replace drm_atomic_get_existing_crtc_state() with
intel_atomic_get_new_crtc_state(). Switch over to intel_ types
as well to make the code less cluttered.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h      |  7 +++----
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_fbc.c     | 23 ++++++++++-------------
 4 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81c1ece8b3ee..3b6a2a1bd089 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -561,13 +561,13 @@ struct i915_hotplug {
 	for_each_power_well_rev(__dev_priv, __power_well)		        \
 		for_each_if ((__power_well)->domains & (__domain_mask))
 
-#define for_each_intel_plane_in_state(__state, plane, plane_state, __i) \
+#define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \
 	for ((__i) = 0; \
 	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
 		     ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \
-		      (plane_state) = to_intel_plane_state((__state)->base.planes[__i].state), 1); \
+		      (new_plane_state) = to_intel_plane_state((__state)->base.planes[__i].new_state), 1); \
 	     (__i)++) \
-		for_each_if (plane_state)
+		for_each_if (plane)
 
 #define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
 	for ((__i) = 0; \
@@ -577,7 +577,6 @@ struct i915_hotplug {
 	     (__i)++) \
 		for_each_if (crtc)
 
-
 #define for_each_oldnew_intel_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \
 	for ((__i) = 0; \
 	     (__i) < (__state)->base.dev->mode_config.num_total_plane && \
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5b01245d8f0f..558d6a04b76c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12021,7 +12021,7 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	intel_fbc_choose_crtc(dev_priv, state);
+	intel_fbc_choose_crtc(dev_priv, intel_state);
 	return calc_watermark_data(state);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dc0b60a7f13f..b020c27bb1cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1649,7 +1649,7 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
 
 /* intel_fbc.c */
 void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
-			   struct drm_atomic_state *state);
+			   struct intel_atomic_state *state);
 bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
 void intel_fbc_pre_update(struct intel_crtc *crtc,
 			  struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 8e3a05505f49..0b40b89f8e2b 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1051,11 +1051,11 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
  * enable FBC for the chosen CRTC. If it does, it will set dev_priv->fbc.crtc.
  */
 void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
-			   struct drm_atomic_state *state)
+			   struct intel_atomic_state *state)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
-	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
+	struct intel_plane *plane;
+	struct intel_plane_state *plane_state;
 	bool crtc_chosen = false;
 	int i;
 
@@ -1063,7 +1063,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 
 	/* Does this atomic commit involve the CRTC currently tied to FBC? */
 	if (fbc->crtc &&
-	    !drm_atomic_get_existing_crtc_state(state, &fbc->crtc->base))
+	    !intel_atomic_get_new_crtc_state(state, fbc->crtc))
 		goto out;
 
 	if (!intel_fbc_can_enable(dev_priv))
@@ -1073,13 +1073,11 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 	 * plane. We could go for fancier schemes such as checking the plane
 	 * size, but this would just affect the few platforms that don't tie FBC
 	 * to pipe or plane A. */
-	for_each_new_plane_in_state(state, plane, plane_state, i) {
-		struct intel_plane_state *intel_plane_state =
-			to_intel_plane_state(plane_state);
-		struct intel_crtc_state *intel_crtc_state;
-		struct intel_crtc *crtc = to_intel_crtc(plane_state->crtc);
+	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
+		struct intel_crtc_state *crtc_state;
+		struct intel_crtc *crtc = to_intel_crtc(plane_state->base.crtc);
 
-		if (!intel_plane_state->base.visible)
+		if (!plane_state->base.visible)
 			continue;
 
 		if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
@@ -1088,10 +1086,9 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A)
 			continue;
 
-		intel_crtc_state = to_intel_crtc_state(
-			drm_atomic_get_existing_crtc_state(state, &crtc->base));
+		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
 
-		intel_crtc_state->enable_fbc = true;
+		crtc_state->enable_fbc = true;
 		crtc_chosen = true;
 		break;
 	}
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 08/10] drm/i915: Nuke crtc->plane
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (6 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-10-23 14:51   ` [PATCH v3 " Ville Syrjala
  2017-10-13 13:58 ` [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Eliminate crtc->plane since it's pretty much a layering violation.
We can always get the plane via crtc->primary if we actually need it.

The only ugly thing left is plane_to_crtc_mapping[], but that's
still needed by the pre-g4x watermark code.

v2: Removed a misplaced comment change (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 drivers/gpu/drm/i915/intel_drv.h     | 1 -
 drivers/gpu/drm/i915/intel_fbc.c     | 4 ++--
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 558d6a04b76c..2da670628e35 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13382,14 +13382,13 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		goto fail;
 
 	intel_crtc->pipe = pipe;
-	intel_crtc->plane = primary->plane;
 
 	/* initialize shared scalers */
 	intel_crtc_init_scalers(intel_crtc, crtc_state);
 
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
-	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
-	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
+	       dev_priv->plane_to_crtc_mapping[primary->plane] != NULL);
+	dev_priv->plane_to_crtc_mapping[primary->plane] = intel_crtc;
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b020c27bb1cd..fd10856d8f66 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -796,7 +796,6 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum i9xx_plane_id plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 0b40b89f8e2b..567f10380a0f 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -887,7 +887,7 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	params->vma = cache->vma;
 
 	params->crtc.pipe = crtc->pipe;
-	params->crtc.plane = crtc->plane;
+	params->crtc.plane = to_intel_plane(crtc->base.primary)->plane;
 	params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
 
 	params->fb.format = cache->fb.format;
@@ -1083,7 +1083,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
 			continue;
 
-		if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A)
+		if (fbc_on_plane_a_only(dev_priv) && plane->plane != PLANE_A)
 			continue;
 
 		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (7 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH v2 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-11-16 23:49   ` James Ausmus
  2017-10-13 13:58 ` [PATCH 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Since we now have a ->get_hw_state() method for planes, let's use
that during the initial plane fb readout.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2da670628e35..268d320690f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7465,19 +7465,20 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 
+	if (!plane->get_hw_state(plane))
+		return;
+
+	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
+	if (!intel_fb) {
+		DRM_DEBUG_KMS("failed to alloc fb\n");
+		return;
+	}
+
+	fb = &intel_fb->base;
+
+	fb->dev = dev;
+
 	val = I915_READ(DSPCNTR(plane_id));
-	if (!(val & DISPLAY_PLANE_ENABLE))
-		return;
-
-	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
-	if (!intel_fb) {
-		DRM_DEBUG_KMS("failed to alloc fb\n");
-		return;
-	}
-
-	fb = &intel_fb->base;
-
-	fb->dev = dev;
 
 	if (INTEL_GEN(dev_priv) >= 4) {
 		if (val & DISPPLANE_TILED) {
@@ -8496,6 +8497,9 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 
+	if (!plane->get_hw_state(plane))
+		return;
+
 	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
 	if (!intel_fb) {
 		DRM_DEBUG_KMS("failed to alloc fb\n");
@@ -8507,8 +8511,6 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 	fb->dev = dev;
 
 	val = I915_READ(PLANE_CTL(pipe, plane_id));
-	if (!(val & PLANE_CTL_ENABLE))
-		goto error;
 
 	pixel_format = val & PLANE_CTL_FORMAT_MASK;
 	fourcc = skl_format_to_fourcc(pixel_format,
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/10] drm/i915: Add rudimentary plane state verification
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (8 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
@ 2017-10-13 13:58 ` Ville Syrjala
  2017-11-02 16:38   ` [PATCH v2 " Ville Syrjala
  2017-10-13 16:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 13:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Check that the planes on each crtc undergoing a modeset are in the
state we expect them to be. For now we can only check whether each
plane is correctly enabled or disabled. In the future we may want
to expand the plane state readout to support a more through
verification.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 268d320690f4..b0f666e086cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11545,6 +11545,26 @@ verify_crtc_state(struct drm_crtc *crtc,
 }
 
 static void
+verify_plane_state(struct intel_atomic_state *state,
+		   struct intel_crtc *crtc)
+{
+	struct intel_plane *plane;
+	const struct intel_plane_state *old_plane_state;
+	const struct intel_plane_state *new_plane_state;
+	int i;
+
+	for_each_oldnew_intel_plane_in_state(state, plane,
+					     old_plane_state,
+					     new_plane_state, i) {
+		if (old_plane_state->base.crtc != &crtc->base &&
+		    new_plane_state->base.crtc != &crtc->base)
+			continue;
+
+		assert_plane(plane, new_plane_state->base.visible);
+	}
+}
+
+static void
 verify_single_dpll_state(struct drm_i915_private *dev_priv,
 			 struct intel_shared_dpll *pll,
 			 struct drm_crtc *crtc,
@@ -11638,6 +11658,8 @@ intel_modeset_verify_crtc(struct drm_crtc *crtc,
 	verify_wm_state(crtc, new_state);
 	verify_connector_state(crtc->dev, state, crtc);
 	verify_crtc_state(crtc, old_state, new_state);
+	verify_plane_state(to_intel_atomic_state(state),
+			   to_intel_crtc(crtc));
 	verify_shared_dpll_state(crtc->dev, crtc, old_state, new_state);
 }
 
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (9 preceding siblings ...)
  2017-10-13 13:58 ` [PATCH 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
@ 2017-10-13 16:24 ` Patchwork
  2017-10-13 16:53   ` Ville Syrjälä
  2017-10-13 18:05 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 30+ messages in thread
From: Patchwork @ 2017-10-13 16:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev2)
URL   : https://patchwork.freedesktop.org/series/31758/
State : warning

== Summary ==

Series 31758v2 drm/i915: Plane assert/readout cleanups etc.
https://patchwork.freedesktop.org/api/1.0/series/31758/revisions/2/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup basic-flip-b:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-flip-c:
                pass       -> DMESG-WARN (fi-bdw-gvtdvm)
                pass       -> DMESG-WARN (fi-bsw-n3050)
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6700hq)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
                pass       -> DMESG-WARN (fi-bxt-dsi)
                pass       -> DMESG-WARN (fi-bxt-j4205) fdo#102035 +1
                pass       -> DMESG-WARN (fi-kbl-7567u)
                pass       -> DMESG-WARN (fi-kbl-r)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-gdg-551) fdo#102618
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> DMESG-WARN (fi-byt-j1900)
        Subgroup force-load-detect:
                pass       -> DMESG-WARN (fi-byt-j1900)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-bsw-n3050)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
        Subgroup hang-read-crc-pipe-c:
WARNING: Long output truncated

005c15a2795854ab64b6ce63dcb099d2eea4a889 drm-tip: 2017y-10m-13d-15h-39m-54s UTC integration manifest
649aae97b03f drm/i915: Add rudimentary plane state verification
3b62710ae8d7 drm/i915: Use plane->get_hw_state() for initial plane fb readout
091ccde11e5d drm/i915: Nuke crtc->plane
35c2d556152b drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
b0e0e3b92854 drm/i915: Nuke ironlake_get_initial_plane_config()
e4a00c7c1f3c drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout
f8d9df6701ea drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
8099a24ba745 drm/i915: s/enum plane/enum i9xx_plane_id/
532f661025c8 drm/i915: Redo plane sanitation during readout
d738145d91a4 drm/i915: Add .get_hw_state() method for planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6024/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2)
  2017-10-13 16:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
@ 2017-10-13 16:53   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2017-10-13 16:53 UTC (permalink / raw)
  To: intel-gfx

On Fri, Oct 13, 2017 at 04:24:45PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Plane assert/readout cleanups etc. (rev2)
> URL   : https://patchwork.freedesktop.org/series/31758/
> State : warning
> 
> == Summary ==
> 
> Series 31758v2 drm/i915: Plane assert/readout cleanups etc.
> https://patchwork.freedesktop.org/api/1.0/series/31758/revisions/2/mbox/
> 
> Test chamelium:
>         Subgroup dp-crc-fast:
>                 fail       -> PASS       (fi-kbl-7500u) fdo#102514
> Test debugfs_test:
>         Subgroup read_all_entries:
>                 pass       -> DMESG-WARN (fi-byt-j1900)
>                 pass       -> DMESG-WARN (fi-byt-n2820)
>                 pass       -> DMESG-WARN (fi-bsw-n3050)
<snip tons more>

All of these seem to be due to the power well being off when we read the
plane state. I tried to protect against that by making sure we check
only if the plane's crtc is undergoing a modeset, but apparently that's
not good enough.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 01/10] drm/i915: Add .get_hw_state() method for planes
  2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
@ 2017-10-13 17:36   ` Ville Syrjala
  2017-10-23 14:50   ` [PATCH v4 " Ville Syrjala
  1 sibling, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 17:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Alex Villacís Lasso

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

Add a .get_hw_state() method for planes, returning true or false
depending on whether the plane is enabled. Use it to rewrite the
plane enabled/disabled asserts in platform agnostic fashion.

We do lose the pre-gen4 plane<->pipe mapping checks, but since we're
supposed sanitize that anyway it doesn't really matter.

v2: Reoder patches to not depend on enum old_plane_id
    Just call assert_plane_disabled() from assert_planes_disabled()
v3: Deal with disabled power wells in .get_hw_state()

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v2
Tested-by: Thierry Reding <thierry.reding@gmail.com> #v2
---
 drivers/gpu/drm/i915/intel_display.c | 207 ++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 drivers/gpu/drm/i915/intel_sprite.c  |  83 ++++++++++++++
 3 files changed, 193 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 185be5726b5e..b577e00b1db2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1192,23 +1192,6 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 	     pipe_name(pipe));
 }
 
-static void assert_cursor(struct drm_i915_private *dev_priv,
-			  enum pipe pipe, bool state)
-{
-	bool cur_state;
-
-	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		cur_state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
-	else
-		cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
-
-	I915_STATE_WARN(cur_state != state,
-	     "cursor on pipe %c assertion failure (expected %s, current %s)\n",
-			pipe_name(pipe), onoff(state), onoff(cur_state));
-}
-#define assert_cursor_enabled(d, p) assert_cursor(d, p, true)
-#define assert_cursor_disabled(d, p) assert_cursor(d, p, false)
-
 void assert_pipe(struct drm_i915_private *dev_priv,
 		 enum pipe pipe, bool state)
 {
@@ -1236,77 +1219,25 @@ void assert_pipe(struct drm_i915_private *dev_priv,
 			pipe_name(pipe), onoff(state), onoff(cur_state));
 }
 
-static void assert_plane(struct drm_i915_private *dev_priv,
-			 enum plane plane, bool state)
+static void assert_plane(struct intel_plane *plane, bool state)
 {
-	u32 val;
-	bool cur_state;
+	bool cur_state = plane->get_hw_state(plane);
 
-	val = I915_READ(DSPCNTR(plane));
-	cur_state = !!(val & DISPLAY_PLANE_ENABLE);
 	I915_STATE_WARN(cur_state != state,
-	     "plane %c assertion failure (expected %s, current %s)\n",
-			plane_name(plane), onoff(state), onoff(cur_state));
+			"%s assertion failure (expected %s, current %s)\n",
+			plane->base.name, onoff(state), onoff(cur_state));
 }
 
-#define assert_plane_enabled(d, p) assert_plane(d, p, true)
-#define assert_plane_disabled(d, p) assert_plane(d, p, false)
+#define assert_plane_enabled(p) assert_plane(p, true)
+#define assert_plane_disabled(p) assert_plane(p, false)
 
-static void assert_planes_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe)
+static void assert_planes_disabled(struct intel_crtc *crtc)
 {
-	int i;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_plane *plane;
 
-	/* Primary planes are fixed to pipes on gen4+ */
-	if (INTEL_GEN(dev_priv) >= 4) {
-		u32 val = I915_READ(DSPCNTR(pipe));
-		I915_STATE_WARN(val & DISPLAY_PLANE_ENABLE,
-		     "plane %c assertion failure, should be disabled but not\n",
-		     plane_name(pipe));
-		return;
-	}
-
-	/* Need to check both planes against the pipe */
-	for_each_pipe(dev_priv, i) {
-		u32 val = I915_READ(DSPCNTR(i));
-		enum pipe cur_pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
-			DISPPLANE_SEL_PIPE_SHIFT;
-		I915_STATE_WARN((val & DISPLAY_PLANE_ENABLE) && pipe == cur_pipe,
-		     "plane %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(i), pipe_name(pipe));
-	}
-}
-
-static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
-				    enum pipe pipe)
-{
-	int sprite;
-
-	if (INTEL_GEN(dev_priv) >= 9) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(PLANE_CTL(pipe, sprite));
-			I915_STATE_WARN(val & PLANE_CTL_ENABLE,
-			     "plane %d assertion failure, should be off on pipe %c but is still active\n",
-			     sprite, pipe_name(pipe));
-		}
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(SPCNTR(pipe, PLANE_SPRITE0 + sprite));
-			I915_STATE_WARN(val & SP_ENABLE,
-			     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-			     sprite_name(pipe, sprite), pipe_name(pipe));
-		}
-	} else if (INTEL_GEN(dev_priv) >= 7) {
-		u32 val = I915_READ(SPRCTL(pipe));
-		I915_STATE_WARN(val & SPRITE_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	} else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
-		u32 val = I915_READ(DVSCNTR(pipe));
-		I915_STATE_WARN(val & DVS_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	}
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane)
+		assert_plane_disabled(plane);
 }
 
 static void assert_vblank_disabled(struct drm_crtc *crtc)
@@ -1899,9 +1830,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 
 	DRM_DEBUG_KMS("enabling pipe %c\n", pipe_name(pipe));
 
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	/*
 	 * A pipe without a PLL won't actually be able to drive bits from
@@ -1971,9 +1900,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	 * Make sure planes won't keep trying to pump pixels to us,
 	 * or we might hang the display.
 	 */
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
@@ -3370,6 +3297,31 @@ static void i9xx_disable_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+{
+
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane plane = primary->plane;
+	enum pipe pipe = primary->pipe;
+	bool ret;
+
+	/*
+	 * Not 100% correct for planes that can move between pipes,
+	 * but that's only the case for gen2-4 which don't have any
+	 * display power wells.
+	 */
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32
 intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
 {
@@ -3638,6 +3590,25 @@ static void skylake_disable_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool skylake_primary_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(PLANE_CTL(pipe, plane_id)) & PLANE_CTL_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static int
 __intel_display_resume(struct drm_device *dev,
 		       struct drm_atomic_state *state,
@@ -4944,7 +4915,8 @@ void hsw_enable_ips(struct intel_crtc *crtc)
 	 * a vblank wait.
 	 */
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
@@ -4977,7 +4949,8 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	if (!crtc->config->ips_enabled)
 		return;
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0));
@@ -9557,6 +9530,23 @@ static void i845_disable_cursor(struct intel_plane *plane,
 	i845_update_cursor(plane, NULL, NULL);
 }
 
+static bool i845_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(PIPE_A);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9750,6 +9740,28 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
 	i9xx_update_cursor(plane, NULL, NULL);
 }
 
+static bool i9xx_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	/*
+	 * Not 100% correct for planes that can move between pipes,
+	 * but that's only the case for gen2-3 which don't have any
+	 * display power wells.
+	 */
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
 
 /* VESA 640x480x72Hz mode to set on the pipe */
 static const struct drm_display_mode load_detect_mode = {
@@ -13231,6 +13243,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
+		primary->get_hw_state = skylake_primary_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 9) {
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
@@ -13241,6 +13254,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
+		primary->get_hw_state = skylake_primary_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
@@ -13248,6 +13262,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
@@ -13255,6 +13270,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 9)
@@ -13344,10 +13360,12 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
 		cursor->update_plane = i845_update_cursor;
 		cursor->disable_plane = i845_disable_cursor;
+		cursor->get_hw_state = i845_cursor_get_hw_state;
 		cursor->check_plane = i845_check_cursor;
 	} else {
 		cursor->update_plane = i9xx_update_cursor;
 		cursor->disable_plane = i9xx_disable_cursor;
+		cursor->get_hw_state = i9xx_cursor_get_hw_state;
 		cursor->check_plane = i9xx_check_cursor;
 	}
 
@@ -14695,8 +14713,8 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
 		      pipe_name(pipe));
 
-	assert_plane_disabled(dev_priv, PLANE_A);
-	assert_plane_disabled(dev_priv, PLANE_B);
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
 
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
@@ -14910,20 +14928,13 @@ void i915_redisable_vga(struct drm_i915_private *dev_priv)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_VGA);
 }
 
-static bool primary_get_hw_state(struct intel_plane *plane)
-{
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-
-	return I915_READ(DSPCNTR(plane->plane)) & DISPLAY_PLANE_ENABLE;
-}
-
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
 	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
 	bool visible;
 
-	visible = crtc->active && primary_get_hw_state(primary);
+	visible = crtc->active && primary->get_hw_state(primary);
 
 	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
 				to_intel_plane_state(primary->base.state),
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d61985f93d40..508b8e2eb4cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -871,6 +871,7 @@ struct intel_plane {
 			     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct intel_plane *plane,
 			      struct intel_crtc *crtc);
+	bool (*get_hw_state)(struct intel_plane *plane);
 	int (*check_plane)(struct intel_plane *plane,
 			   struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 86fc9b529f2d..2ab002312a4e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -329,6 +329,26 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+skl_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(PLANE_CTL(pipe, plane_id)) & PLANE_CTL_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static void
 chv_update_csc(struct intel_plane *plane, uint32_t format)
 {
@@ -506,6 +526,26 @@ vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+vlv_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(SPCNTR(pipe, plane_id)) & SP_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -646,6 +686,25 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+ivb_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret =  I915_READ(SPRCTL(pipe)) & SPRITE_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -777,6 +836,25 @@ g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+g4x_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(DVSCNTR(pipe)) & DVS_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static int
 intel_check_sprite_plane(struct intel_plane *plane,
 			 struct intel_crtc_state *crtc_state,
@@ -1232,6 +1310,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1242,6 +1321,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1252,6 +1332,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = vlv_update_plane;
 		intel_plane->disable_plane = vlv_disable_plane;
+		intel_plane->get_hw_state = vlv_plane_get_hw_state;
 
 		plane_formats = vlv_plane_formats;
 		num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
@@ -1267,6 +1348,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = ivb_update_plane;
 		intel_plane->disable_plane = ivb_disable_plane;
+		intel_plane->get_hw_state = ivb_plane_get_hw_state;
 
 		plane_formats = snb_plane_formats;
 		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
@@ -1277,6 +1359,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = g4x_update_plane;
 		intel_plane->disable_plane = g4x_disable_plane;
+		intel_plane->get_hw_state = g4x_plane_get_hw_state;
 
 		modifiers = i9xx_plane_format_modifiers;
 		if (IS_GEN6(dev_priv)) {
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v4 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-10-13 13:58 ` [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
@ 2017-10-13 17:37   ` Ville Syrjala
  2017-10-23 14:50   ` [PATCH v5 " Ville Syrjala
  1 sibling, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-13 17:37 UTC (permalink / raw)
  To: intel-gfx

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

Rename enum plane to enum i9xx_plane_id to make it clear that it only
applies to pre-SKL platforms.

enum i9xx_plane_id is a global identifier, whereas enum plane_id is
per-pipe. We need the old global identifier to index the primary plane
(and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
platforms.

v2: Reorder patches
v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
    Pimp the commit message a bit
    Note that i9xx_plane_id doesn't apply to SKL+
v4: Rebase due to power domain handling in plane readout

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 +--
 drivers/gpu/drm/i915/intel_display.c | 87 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +--
 3 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c7b2ca6aff05..2a89cbbb7a56 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -305,9 +305,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
 
 /*
  * Global legacy plane identifier. Valid only for primary/sprite
- * planes on pre-g4x, and only for primary planes on g4x+.
+ * planes on pre-g4x, and only for primary planes on g4x-bdw.
  */
-enum plane {
+enum i9xx_plane_id {
 	PLANE_A,
 	PLANE_B,
 	PLANE_C,
@@ -1128,7 +1128,7 @@ struct intel_fbc {
 
 		struct {
 			enum pipe pipe;
-			enum plane plane;
+			enum i9xx_plane_id plane;
 			unsigned int fence_y_offset;
 		} crtc;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac2bd7461c8d..79ea38d6d72e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3223,17 +3223,17 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct intel_plane *primary,
-				      const struct intel_crtc_state *crtc_state,
-				      const struct intel_plane_state *plane_state)
+static void i9xx_update_plane(struct intel_plane *plane,
+			      const struct intel_crtc_state *crtc_state,
+			      const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	enum plane plane = primary->plane;
+	enum i9xx_plane_id plane_id = plane->plane;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
-	i915_reg_t reg = DSPCNTR(plane);
+	i915_reg_t reg = DSPCNTR(plane_id);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
 	unsigned long irqflags;
@@ -3254,34 +3254,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
 		 */
-		I915_WRITE_FW(DSPSIZE(plane),
+		I915_WRITE_FW(DSPSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(DSPPOS(plane), 0);
-	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
-		I915_WRITE_FW(PRIMSIZE(plane),
+		I915_WRITE_FW(DSPPOS(plane_id), 0);
+	} else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
+		I915_WRITE_FW(PRIMSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(PRIMPOS(plane), 0);
-		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
+		I915_WRITE_FW(PRIMPOS(plane_id), 0);
+		I915_WRITE_FW(PRIMCNSTALPHA(plane_id), 0);
 	}
 
 	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSTRIDE(plane_id), fb->pitches[0]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
-		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPOFFSET(plane_id), (y << 16) | x);
 	} else if (INTEL_GEN(dev_priv) >= 4) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
-		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
+		I915_WRITE_FW(DSPTILEOFF(plane_id), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(plane_id), linear_offset);
 	} else {
-		I915_WRITE_FW(DSPADDR(plane),
+		I915_WRITE_FW(DSPADDR(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      crtc->dspaddr_offset);
 	}
@@ -3290,32 +3290,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void i9xx_disable_primary_plane(struct intel_plane *primary,
-				       struct intel_crtc *crtc)
+static void i9xx_disable_plane(struct intel_plane *plane,
+			       struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
-	enum plane plane = primary->plane;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum i9xx_plane_id plane_id = plane->plane;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE_FW(DSPCNTR(plane), 0);
-	if (INTEL_INFO(dev_priv)->gen >= 4)
-		I915_WRITE_FW(DSPSURF(plane), 0);
+	I915_WRITE_FW(DSPCNTR(plane_id), 0);
+	if (INTEL_GEN(dev_priv) >= 4)
+		I915_WRITE_FW(DSPSURF(plane_id), 0);
 	else
-		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
+		I915_WRITE_FW(DSPADDR(plane_id), 0);
+	POSTING_READ_FW(DSPCNTR(plane_id));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
 {
-
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum intel_display_power_domain power_domain;
-	enum plane plane = primary->plane;
-	enum pipe pipe = primary->pipe;
+	enum i9xx_plane_id plane_id = plane->plane;
+	enum pipe pipe = plane->pipe;
 	bool ret;
 
 	/*
@@ -3327,7 +3326,7 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
-	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+	ret = I915_READ(DSPCNTR(plane_id)) & DISPLAY_PLANE_ENABLE;
 
 	intel_display_power_put(dev_priv, power_domain);
 
@@ -13243,9 +13242,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
 	 */
 	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
-		primary->plane = (enum plane) !pipe;
+		primary->plane = (enum i9xx_plane_id) !pipe;
 	else
-		primary->plane = (enum plane) pipe;
+		primary->plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
@@ -13274,16 +13273,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		num_formats = ARRAY_SIZE(i965_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
@@ -13367,7 +13366,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	cursor->can_scale = false;
 	cursor->max_downscale = 1;
 	cursor->pipe = pipe;
-	cursor->plane = pipe;
+	cursor->plane = (enum i9xx_plane_id) pipe;
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 
@@ -14741,11 +14740,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
-				   struct intel_plane *primary)
+				   struct intel_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum plane plane = primary->plane;
-	u32 val = I915_READ(DSPCNTR(plane));
+	enum i9xx_plane_id plane_id = plane->plane;
+	u32 val = I915_READ(DSPCNTR(plane_id));
 
 	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
 		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 508b8e2eb4cf..dc0b60a7f13f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -796,7 +796,7 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum plane plane;
+	enum i9xx_plane_id plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
@@ -849,7 +849,7 @@ struct intel_crtc {
 
 struct intel_plane {
 	struct drm_plane base;
-	u8 plane;
+	enum i9xx_plane_id plane;
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
@@ -1136,7 +1136,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static inline struct intel_crtc *
-intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
+intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
 {
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev4)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (10 preceding siblings ...)
  2017-10-13 16:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
@ 2017-10-13 18:05 ` Patchwork
  2017-10-13 23:20 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-10-13 18:05 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev4)
URL   : https://patchwork.freedesktop.org/series/31758/
State : success

== Summary ==

Series 31758v4 drm/i915: Plane assert/readout cleanups etc.
https://patchwork.freedesktop.org/api/1.0/series/31758/revisions/4/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-bdw-5557u) fdo#102473
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102473 https://bugs.freedesktop.org/show_bug.cgi?id=102473
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:289  pass:267  dwarn:1   dfail:0   fail:0   skip:21  time:454s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:475s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:386s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:574s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:284s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:525s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:519s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:534s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:520s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:570s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:426s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:271s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:602s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:442s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:459s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:471s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:502s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:595s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:653s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:469s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:658s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:535s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:519s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:471s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:583s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:430s

005c15a2795854ab64b6ce63dcb099d2eea4a889 drm-tip: 2017y-10m-13d-15h-39m-54s UTC integration manifest
885ca98014cf drm/i915: Add rudimentary plane state verification
28669af1dae1 drm/i915: Use plane->get_hw_state() for initial plane fb readout
73648186d8ad drm/i915: Nuke crtc->plane
524ab247a435 drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
8c4a96a59500 drm/i915: Nuke ironlake_get_initial_plane_config()
f77a4c8760ce drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout
c4249765df62 drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
5741360dfc4f drm/i915: s/enum plane/enum i9xx_plane_id/
2d6333548ccc drm/i915: Redo plane sanitation during readout
2f5ce2d0cbec drm/i915: Add .get_hw_state() method for planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6030/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (11 preceding siblings ...)
  2017-10-13 18:05 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
@ 2017-10-13 23:20 ` Patchwork
  2017-10-14  2:35 ` ✗ Fi.CI.IGT: failure for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-10-13 23:20 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev2)
URL   : https://patchwork.freedesktop.org/series/31758/
State : warning

== Summary ==

Test kms_atomic_transition:
        Subgroup plane-all-transition:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup plane-all-modeset-transition:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_cursor_crc:
        Subgroup cursor-64x64-sliding:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-64x21-random:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-64x64-rapid-movement:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-64x64-suspend:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-128x128-random:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-128x128-suspend:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup cursor-64x21-onscreen:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_fbc_crc:
        Subgroup mmap_cpu:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-B:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249
Test kms_flip:
        Subgroup flip-vs-dpms-off-vs-modeset-interruptible:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup flip-vs-expired-vblank:
                pass       -> DMESG-WARN (shard-hsw) fdo#102367
        Subgroup flip-vs-panning-vs-hang-interruptible:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup vblank-vs-suspend:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup wf_vblank:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup flip-vs-fences-interruptible:
                pass       -> DMESG-WARN (shard-hsw) fdo#102946
        Subgroup vblank-vs-modeset-rpm:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-B:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup hang-read-crc-pipe-C:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_mmio_vs_cs_flip:
        Subgroup setplane_vs_cs_flip:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_flip_event_leak:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_render:
        Subgroup direct-render:
                pass       -> DMESG-WARN (shard-hsw) fdo#102614

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#102367 https://bugs.freedesktop.org/show_bug.cgi?id=102367
fdo#102946 https://bugs.freedesktop.org/show_bug.cgi?id=102946
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614

shard-hsw        total:2553 pass:1417 dwarn:25  dfail:0   fail:8   skip:1103 time:9641s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6024/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915: Plane assert/readout cleanups etc. (rev4)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (12 preceding siblings ...)
  2017-10-13 23:20 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
@ 2017-10-14  2:35 ` Patchwork
  2017-10-23 15:10 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev7) Patchwork
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-10-14  2:35 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev4)
URL   : https://patchwork.freedesktop.org/series/31758/
State : failure

== Summary ==

Test perf:
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2553 pass:1439 dwarn:1   dfail:0   fail:10  skip:1103 time:9629s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6030/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes
  2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
  2017-10-13 17:36   ` [PATCH v3 " Ville Syrjala
@ 2017-10-23 14:50   ` Ville Syrjala
  1 sibling, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-23 14:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Alex Villacís Lasso

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

Add a .get_hw_state() method for planes, returning true or false
depending on whether the plane is enabled. Use it to rewrite the
plane enabled/disabled asserts in platform agnostic fashion.

We do lose the pre-gen4 plane<->pipe mapping checks, but since we're
supposed sanitize that anyway it doesn't really matter.

v2: Reoder patches to not depend on enum old_plane_id
    Just call assert_plane_disabled() from assert_planes_disabled()
v3: Deal with disabled power wells in .get_hw_state()
v4: Rebase due skl primary plane code removal

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v2
Tested-by: Thierry Reding <thierry.reding@gmail.com> #v2
---
 drivers/gpu/drm/i915/intel_display.c | 188 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_sprite.c  |  83 ++++++++++++++++
 3 files changed, 175 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e2ac976844d8..8638d8fd7721 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1192,23 +1192,6 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 	     pipe_name(pipe));
 }
 
-static void assert_cursor(struct drm_i915_private *dev_priv,
-			  enum pipe pipe, bool state)
-{
-	bool cur_state;
-
-	if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
-		cur_state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
-	else
-		cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
-
-	I915_STATE_WARN(cur_state != state,
-	     "cursor on pipe %c assertion failure (expected %s, current %s)\n",
-			pipe_name(pipe), onoff(state), onoff(cur_state));
-}
-#define assert_cursor_enabled(d, p) assert_cursor(d, p, true)
-#define assert_cursor_disabled(d, p) assert_cursor(d, p, false)
-
 void assert_pipe(struct drm_i915_private *dev_priv,
 		 enum pipe pipe, bool state)
 {
@@ -1236,77 +1219,25 @@ void assert_pipe(struct drm_i915_private *dev_priv,
 			pipe_name(pipe), onoff(state), onoff(cur_state));
 }
 
-static void assert_plane(struct drm_i915_private *dev_priv,
-			 enum plane plane, bool state)
+static void assert_plane(struct intel_plane *plane, bool state)
 {
-	u32 val;
-	bool cur_state;
+	bool cur_state = plane->get_hw_state(plane);
 
-	val = I915_READ(DSPCNTR(plane));
-	cur_state = !!(val & DISPLAY_PLANE_ENABLE);
 	I915_STATE_WARN(cur_state != state,
-	     "plane %c assertion failure (expected %s, current %s)\n",
-			plane_name(plane), onoff(state), onoff(cur_state));
+			"%s assertion failure (expected %s, current %s)\n",
+			plane->base.name, onoff(state), onoff(cur_state));
 }
 
-#define assert_plane_enabled(d, p) assert_plane(d, p, true)
-#define assert_plane_disabled(d, p) assert_plane(d, p, false)
+#define assert_plane_enabled(p) assert_plane(p, true)
+#define assert_plane_disabled(p) assert_plane(p, false)
 
-static void assert_planes_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe)
+static void assert_planes_disabled(struct intel_crtc *crtc)
 {
-	int i;
-
-	/* Primary planes are fixed to pipes on gen4+ */
-	if (INTEL_GEN(dev_priv) >= 4) {
-		u32 val = I915_READ(DSPCNTR(pipe));
-		I915_STATE_WARN(val & DISPLAY_PLANE_ENABLE,
-		     "plane %c assertion failure, should be disabled but not\n",
-		     plane_name(pipe));
-		return;
-	}
-
-	/* Need to check both planes against the pipe */
-	for_each_pipe(dev_priv, i) {
-		u32 val = I915_READ(DSPCNTR(i));
-		enum pipe cur_pipe = (val & DISPPLANE_SEL_PIPE_MASK) >>
-			DISPPLANE_SEL_PIPE_SHIFT;
-		I915_STATE_WARN((val & DISPLAY_PLANE_ENABLE) && pipe == cur_pipe,
-		     "plane %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(i), pipe_name(pipe));
-	}
-}
-
-static void assert_sprites_disabled(struct drm_i915_private *dev_priv,
-				    enum pipe pipe)
-{
-	int sprite;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_plane *plane;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(PLANE_CTL(pipe, sprite));
-			I915_STATE_WARN(val & PLANE_CTL_ENABLE,
-			     "plane %d assertion failure, should be off on pipe %c but is still active\n",
-			     sprite, pipe_name(pipe));
-		}
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		for_each_sprite(dev_priv, pipe, sprite) {
-			u32 val = I915_READ(SPCNTR(pipe, PLANE_SPRITE0 + sprite));
-			I915_STATE_WARN(val & SP_ENABLE,
-			     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-			     sprite_name(pipe, sprite), pipe_name(pipe));
-		}
-	} else if (INTEL_GEN(dev_priv) >= 7) {
-		u32 val = I915_READ(SPRCTL(pipe));
-		I915_STATE_WARN(val & SPRITE_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	} else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
-		u32 val = I915_READ(DVSCNTR(pipe));
-		I915_STATE_WARN(val & DVS_ENABLE,
-		     "sprite %c assertion failure, should be off on pipe %c but is still active\n",
-		     plane_name(pipe), pipe_name(pipe));
-	}
+	for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane)
+		assert_plane_disabled(plane);
 }
 
 static void assert_vblank_disabled(struct drm_crtc *crtc)
@@ -1899,9 +1830,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 
 	DRM_DEBUG_KMS("enabling pipe %c\n", pipe_name(pipe));
 
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	/*
 	 * A pipe without a PLL won't actually be able to drive bits from
@@ -1971,9 +1900,7 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	 * Make sure planes won't keep trying to pump pixels to us,
 	 * or we might hang the display.
 	 */
-	assert_planes_disabled(dev_priv, pipe);
-	assert_cursor_disabled(dev_priv, pipe);
-	assert_sprites_disabled(dev_priv, pipe);
+	assert_planes_disabled(crtc);
 
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
@@ -3367,6 +3294,31 @@ static void i9xx_disable_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+{
+
+	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane plane = primary->plane;
+	enum pipe pipe = primary->pipe;
+	bool ret;
+
+	/*
+	 * Not 100% correct for planes that can move between pipes,
+	 * but that's only the case for gen2-4 which don't have any
+	 * display power wells.
+	 */
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32
 intel_fb_stride_alignment(const struct drm_framebuffer *fb, int plane)
 {
@@ -4847,7 +4799,8 @@ void hsw_enable_ips(struct intel_crtc *crtc)
 	 * a vblank wait.
 	 */
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
@@ -4880,7 +4833,8 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	if (!crtc->config->ips_enabled)
 		return;
 
-	assert_plane_enabled(dev_priv, crtc->plane);
+	assert_plane_enabled(to_intel_plane(crtc->base.primary));
+
 	if (IS_BROADWELL(dev_priv)) {
 		mutex_lock(&dev_priv->pcu_lock);
 		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0));
@@ -9458,6 +9412,23 @@ static void i845_disable_cursor(struct intel_plane *plane,
 	i845_update_cursor(plane, NULL, NULL);
 }
 
+static bool i845_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(PIPE_A);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 			   const struct intel_plane_state *plane_state)
 {
@@ -9651,6 +9622,28 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
 	i9xx_update_cursor(plane, NULL, NULL);
 }
 
+static bool i9xx_cursor_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	/*
+	 * Not 100% correct for planes that can move between pipes,
+	 * but that's only the case for gen2-3 which don't have any
+	 * display power wells.
+	 */
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
 
 /* VESA 640x480x72Hz mode to set on the pipe */
 static const struct drm_display_mode load_detect_mode = {
@@ -13183,6 +13176,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skl_update_plane;
 		primary->disable_plane = skl_disable_plane;
+		primary->get_hw_state = skl_plane_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 9) {
 		intel_primary_formats = skl_primary_formats;
 		num_formats = ARRAY_SIZE(skl_primary_formats);
@@ -13193,6 +13187,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = skl_update_plane;
 		primary->disable_plane = skl_disable_plane;
+		primary->get_hw_state = skl_plane_get_hw_state;
 	} else if (INTEL_GEN(dev_priv) >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
@@ -13200,6 +13195,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
@@ -13207,6 +13203,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 		primary->update_plane = i9xx_update_primary_plane;
 		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
 	if (INTEL_GEN(dev_priv) >= 9)
@@ -13296,10 +13293,12 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
 		cursor->update_plane = i845_update_cursor;
 		cursor->disable_plane = i845_disable_cursor;
+		cursor->get_hw_state = i845_cursor_get_hw_state;
 		cursor->check_plane = i845_check_cursor;
 	} else {
 		cursor->update_plane = i9xx_update_cursor;
 		cursor->disable_plane = i9xx_disable_cursor;
+		cursor->get_hw_state = i9xx_cursor_get_hw_state;
 		cursor->check_plane = i9xx_check_cursor;
 	}
 
@@ -14647,8 +14646,8 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
 		      pipe_name(pipe));
 
-	assert_plane_disabled(dev_priv, PLANE_A);
-	assert_plane_disabled(dev_priv, PLANE_B);
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
+	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
 
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
@@ -14862,20 +14861,13 @@ void i915_redisable_vga(struct drm_i915_private *dev_priv)
 	intel_display_power_put(dev_priv, POWER_DOMAIN_VGA);
 }
 
-static bool primary_get_hw_state(struct intel_plane *plane)
-{
-	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
-
-	return I915_READ(DSPCNTR(plane->plane)) & DISPLAY_PLANE_ENABLE;
-}
-
 /* FIXME read out full plane state for all planes */
 static void readout_plane_state(struct intel_crtc *crtc)
 {
 	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
 	bool visible;
 
-	visible = crtc->active && primary_get_hw_state(primary);
+	visible = crtc->active && primary->get_hw_state(primary);
 
 	intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
 				to_intel_plane_state(primary->base.state),
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47d022d48718..07ba376c6fff 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -863,6 +863,7 @@ struct intel_plane {
 			     const struct intel_plane_state *plane_state);
 	void (*disable_plane)(struct intel_plane *plane,
 			      struct intel_crtc *crtc);
+	bool (*get_hw_state)(struct intel_plane *plane);
 	int (*check_plane)(struct intel_plane *plane,
 			   struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *state);
@@ -1925,6 +1926,7 @@ void skl_update_plane(struct intel_plane *plane,
 		      const struct intel_crtc_state *crtc_state,
 		      const struct intel_plane_state *plane_state);
 void skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc);
+bool skl_plane_get_hw_state(struct intel_plane *plane);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4fcf80ca91dd..4a8a5d918a83 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -329,6 +329,26 @@ skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+bool
+skl_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(PLANE_CTL(pipe, plane_id)) & PLANE_CTL_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static void
 chv_update_csc(struct intel_plane *plane, uint32_t format)
 {
@@ -506,6 +526,26 @@ vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+vlv_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum plane_id plane_id = plane->id;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(SPCNTR(pipe, plane_id)) & SP_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -646,6 +686,25 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+ivb_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret =  I915_READ(SPRCTL(pipe)) & SPRITE_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 			  const struct intel_plane_state *plane_state)
 {
@@ -777,6 +836,25 @@ g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static bool
+g4x_plane_get_hw_state(struct intel_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum intel_display_power_domain power_domain;
+	enum pipe pipe = plane->pipe;
+	bool ret;
+
+	power_domain = POWER_DOMAIN_PIPE(pipe);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+
+	ret = I915_READ(DVSCNTR(pipe)) & DVS_ENABLE;
+
+	intel_display_power_put(dev_priv, power_domain);
+
+	return ret;
+}
+
 static int
 intel_check_sprite_plane(struct intel_plane *plane,
 			 struct intel_crtc_state *crtc_state,
@@ -1232,6 +1310,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1242,6 +1321,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
+		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
 		plane_formats = skl_plane_formats;
 		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
@@ -1252,6 +1332,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = vlv_update_plane;
 		intel_plane->disable_plane = vlv_disable_plane;
+		intel_plane->get_hw_state = vlv_plane_get_hw_state;
 
 		plane_formats = vlv_plane_formats;
 		num_plane_formats = ARRAY_SIZE(vlv_plane_formats);
@@ -1267,6 +1348,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = ivb_update_plane;
 		intel_plane->disable_plane = ivb_disable_plane;
+		intel_plane->get_hw_state = ivb_plane_get_hw_state;
 
 		plane_formats = snb_plane_formats;
 		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
@@ -1277,6 +1359,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		intel_plane->update_plane = g4x_update_plane;
 		intel_plane->disable_plane = g4x_disable_plane;
+		intel_plane->get_hw_state = g4x_plane_get_hw_state;
 
 		modifiers = i9xx_plane_format_modifiers;
 		if (IS_GEN6(dev_priv)) {
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v5 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-10-13 13:58 ` [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
  2017-10-13 17:37   ` [PATCH v4 " Ville Syrjala
@ 2017-10-23 14:50   ` Ville Syrjala
  2017-11-16 23:21     ` James Ausmus
  1 sibling, 1 reply; 30+ messages in thread
From: Ville Syrjala @ 2017-10-23 14:50 UTC (permalink / raw)
  To: intel-gfx

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

Rename enum plane to enum i9xx_plane_id to make it clear that it only
applies to pre-SKL platforms.

enum i9xx_plane_id is a global identifier, whereas enum plane_id is
per-pipe. We need the old global identifier to index the primary plane
(and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
platforms.

v2: Reorder patches
v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
    Pimp the commit message a bit
    Note that i9xx_plane_id doesn't apply to SKL+
v4: Rebase due to power domain handling in plane readout
v5: Rebase due to crtc->dspaddr_offset removal

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 +--
 drivers/gpu/drm/i915/intel_display.c | 87 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +--
 3 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54b5d4c582b6..a6b912c646f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -305,9 +305,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
 
 /*
  * Global legacy plane identifier. Valid only for primary/sprite
- * planes on pre-g4x, and only for primary planes on g4x+.
+ * planes on pre-g4x, and only for primary planes on g4x-bdw.
  */
-enum plane {
+enum i9xx_plane_id {
 	PLANE_A,
 	PLANE_B,
 	PLANE_C,
@@ -1137,7 +1137,7 @@ struct intel_fbc {
 
 		struct {
 			enum pipe pipe;
-			enum plane plane;
+			enum i9xx_plane_id plane;
 			unsigned int fence_y_offset;
 		} crtc;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4ea0f1ef249e..e726b65588aa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3223,16 +3223,16 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 	return 0;
 }
 
-static void i9xx_update_primary_plane(struct intel_plane *primary,
-				      const struct intel_crtc_state *crtc_state,
-				      const struct intel_plane_state *plane_state)
+static void i9xx_update_plane(struct intel_plane *plane,
+			      const struct intel_crtc_state *crtc_state,
+			      const struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
-	enum plane plane = primary->plane;
+	enum i9xx_plane_id plane_id = plane->plane;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
-	i915_reg_t reg = DSPCNTR(plane);
+	i915_reg_t reg = DSPCNTR(plane_id);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
 	unsigned long irqflags;
@@ -3251,34 +3251,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 		/* pipesrc and dspsize control the size that is scaled from,
 		 * which should always be the user's requested size.
 		 */
-		I915_WRITE_FW(DSPSIZE(plane),
+		I915_WRITE_FW(DSPSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(DSPPOS(plane), 0);
-	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
-		I915_WRITE_FW(PRIMSIZE(plane),
+		I915_WRITE_FW(DSPPOS(plane_id), 0);
+	} else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
+		I915_WRITE_FW(PRIMSIZE(plane_id),
 			      ((crtc_state->pipe_src_h - 1) << 16) |
 			      (crtc_state->pipe_src_w - 1));
-		I915_WRITE_FW(PRIMPOS(plane), 0);
-		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
+		I915_WRITE_FW(PRIMPOS(plane_id), 0);
+		I915_WRITE_FW(PRIMCNSTALPHA(plane_id), 0);
 	}
 
 	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSTRIDE(plane_id), fb->pitches[0]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      dspaddr_offset);
-		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPOFFSET(plane_id), (y << 16) | x);
 	} else if (INTEL_GEN(dev_priv) >= 4) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      dspaddr_offset);
-		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
-		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
+		I915_WRITE_FW(DSPTILEOFF(plane_id), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(plane_id), linear_offset);
 	} else {
-		I915_WRITE_FW(DSPADDR(plane),
+		I915_WRITE_FW(DSPADDR(plane_id),
 			      intel_plane_ggtt_offset(plane_state) +
 			      dspaddr_offset);
 	}
@@ -3287,32 +3287,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static void i9xx_disable_primary_plane(struct intel_plane *primary,
-				       struct intel_crtc *crtc)
+static void i9xx_disable_plane(struct intel_plane *plane,
+			       struct intel_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
-	enum plane plane = primary->plane;
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+	enum i9xx_plane_id plane_id = plane->plane;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
 
-	I915_WRITE_FW(DSPCNTR(plane), 0);
-	if (INTEL_INFO(dev_priv)->gen >= 4)
-		I915_WRITE_FW(DSPSURF(plane), 0);
+	I915_WRITE_FW(DSPCNTR(plane_id), 0);
+	if (INTEL_GEN(dev_priv) >= 4)
+		I915_WRITE_FW(DSPSURF(plane_id), 0);
 	else
-		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
+		I915_WRITE_FW(DSPADDR(plane_id), 0);
+	POSTING_READ_FW(DSPCNTR(plane_id));
 
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
+static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
 {
-
-	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	enum intel_display_power_domain power_domain;
-	enum plane plane = primary->plane;
-	enum pipe pipe = primary->pipe;
+	enum i9xx_plane_id plane_id = plane->plane;
+	enum pipe pipe = plane->pipe;
 	bool ret;
 
 	/*
@@ -3324,7 +3323,7 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
-	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
+	ret = I915_READ(DSPCNTR(plane_id)) & DISPLAY_PLANE_ENABLE;
 
 	intel_display_power_put(dev_priv, power_domain);
 
@@ -13176,9 +13175,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
 	 */
 	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
-		primary->plane = (enum plane) !pipe;
+		primary->plane = (enum i9xx_plane_id) !pipe;
 	else
-		primary->plane = (enum plane) pipe;
+		primary->plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
@@ -13207,16 +13206,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		num_formats = ARRAY_SIZE(i965_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	} else {
 		intel_primary_formats = i8xx_primary_formats;
 		num_formats = ARRAY_SIZE(i8xx_primary_formats);
 		modifiers = i9xx_format_modifiers;
 
-		primary->update_plane = i9xx_update_primary_plane;
-		primary->disable_plane = i9xx_disable_primary_plane;
+		primary->update_plane = i9xx_update_plane;
+		primary->disable_plane = i9xx_disable_plane;
 		primary->get_hw_state = i9xx_plane_get_hw_state;
 	}
 
@@ -13300,7 +13299,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 	cursor->can_scale = false;
 	cursor->max_downscale = 1;
 	cursor->pipe = pipe;
-	cursor->plane = pipe;
+	cursor->plane = (enum i9xx_plane_id) pipe;
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 
@@ -14674,11 +14673,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
-				   struct intel_plane *primary)
+				   struct intel_plane *plane)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum plane plane = primary->plane;
-	u32 val = I915_READ(DSPCNTR(plane));
+	enum i9xx_plane_id plane_id = plane->plane;
+	u32 val = I915_READ(DSPCNTR(plane_id));
 
 	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
 		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 07ba376c6fff..8e20924ab9df 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -796,7 +796,7 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum plane plane;
+	enum i9xx_plane_id plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
@@ -841,7 +841,7 @@ struct intel_crtc {
 
 struct intel_plane {
 	struct drm_plane base;
-	u8 plane;
+	enum i9xx_plane_id plane;
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
@@ -1128,7 +1128,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 }
 
 static inline struct intel_crtc *
-intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
+intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
 {
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 08/10] drm/i915: Nuke crtc->plane
  2017-10-13 13:58 ` [PATCH v2 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
@ 2017-10-23 14:51   ` Ville Syrjala
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjala @ 2017-10-23 14:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Eliminate crtc->plane since it's pretty much a layering violation.
We can always get the plane via crtc->primary if we actually need it.

The only ugly thing left is plane_to_crtc_mapping[], but that's
still needed by the pre-g4x watermark code.

v2: Removed a misplaced comment change (Daniel)
v3: Rebase due to fbc crtc->y usage removal

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 drivers/gpu/drm/i915/intel_drv.h     | 1 -
 drivers/gpu/drm/i915/intel_fbc.c     | 4 ++--
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8854d8ccadb0..4cdd180bcc55 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13366,14 +13366,13 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		goto fail;
 
 	intel_crtc->pipe = pipe;
-	intel_crtc->plane = primary->plane;
 
 	/* initialize shared scalers */
 	intel_crtc_init_scalers(intel_crtc, crtc_state);
 
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
-	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
-	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
+	       dev_priv->plane_to_crtc_mapping[primary->plane] != NULL);
+	dev_priv->plane_to_crtc_mapping[primary->plane] = intel_crtc;
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index caa91f248f03..52b9321c9788 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -796,7 +796,6 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum i9xx_plane_id plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 89518846aa8b..e3cfdc2a8fef 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -890,7 +890,7 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	params->vma = cache->vma;
 
 	params->crtc.pipe = crtc->pipe;
-	params->crtc.plane = crtc->plane;
+	params->crtc.plane = to_intel_plane(crtc->base.primary)->plane;
 	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
 
 	params->fb.format = cache->fb.format;
@@ -1086,7 +1086,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		if (fbc_on_pipe_a_only(dev_priv) && crtc->pipe != PIPE_A)
 			continue;
 
-		if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A)
+		if (fbc_on_plane_a_only(dev_priv) && plane->plane != PLANE_A)
 			continue;
 
 		crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev7)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (13 preceding siblings ...)
  2017-10-14  2:35 ` ✗ Fi.CI.IGT: failure for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
@ 2017-10-23 15:10 ` Patchwork
  2017-10-23 16:09 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-10-23 15:10 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev7)
URL   : https://patchwork.freedesktop.org/series/31758/
State : success

== Summary ==

Series 31758v7 drm/i915: Plane assert/readout cleanups etc.
https://patchwork.freedesktop.org/api/1.0/series/31758/revisions/7/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618
                incomplete -> PASS       (fi-skl-6700hq) fdo#103410 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-kbl-7560u) fdo#102846

fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fdo#103410 https://bugs.freedesktop.org/show_bug.cgi?id=103410
fdo#102846 https://bugs.freedesktop.org/show_bug.cgi?id=102846

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:452s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:530s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:263s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:499s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:493s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:471s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:554s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:405s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:575s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:455s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:432s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:495s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:457s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:477s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:572s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:583s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:541s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:644s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:516s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:456s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:568s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:413s

d2ec28c53833297976d0754e0e82c3d7490b149c drm-tip: 2017y-10m-23d-08h-55m-00s UTC integration manifest
32b9109d5b22 drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
7571f3f46a81 drm/i915: s/enum plane/enum i9xx_plane_id/
7b45bbf79907 drm/i915: Redo plane sanitation during readout
bf23f7597723 drm/i915: Add .get_hw_state() method for planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6142/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Plane assert/readout cleanups etc. (rev7)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (14 preceding siblings ...)
  2017-10-23 15:10 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev7) Patchwork
@ 2017-10-23 16:09 ` Patchwork
  2017-11-02 17:06 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev8) Patchwork
  2017-11-02 18:13 ` ✓ Fi.CI.IGT: " Patchwork
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-10-23 16:09 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev7)
URL   : https://patchwork.freedesktop.org/series/31758/
State : success

== Summary ==

Test kms_flip:
        Subgroup flip-vs-absolute-wf_vblank:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-A:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-B-planes:
                skip       -> PASS       (shard-hsw)
Test kms_plane_multiple:
        Subgroup atomic-pipe-B-tiling-x:
                skip       -> PASS       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2540 pass:1432 dwarn:2   dfail:0   fail:9   skip:1097 time:9144s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6142/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 10/10] drm/i915: Add rudimentary plane state verification
  2017-10-13 13:58 ` [PATCH 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
@ 2017-11-02 16:38   ` Ville Syrjala
  2017-11-17  0:07     ` James Ausmus
  0 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjala @ 2017-11-02 16:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

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

Check that the planes are in the state we expect them to be. For
now we can only check whether each plane is correctly enabled or
disabled. In the future we may want to expand the plane state
readout to support a more through verification.

v2: Verify all planes part of the state as long as at lest
    one crtc is doing a modeset (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c23dad6d3c24..96e0a5fd69cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11537,6 +11537,18 @@ verify_crtc_state(struct drm_crtc *crtc,
 }
 
 static void
+intel_verify_planes(struct intel_atomic_state *state)
+{
+	struct intel_plane *plane;
+	const struct intel_plane_state *plane_state;
+	int i;
+
+	for_each_new_intel_plane_in_state(state, plane,
+					  plane_state, i)
+		assert_plane(plane, plane_state->base.visible);
+}
+
+static void
 verify_single_dpll_state(struct drm_i915_private *dev_priv,
 			 struct intel_shared_dpll *pll,
 			 struct drm_crtc *crtc,
@@ -12329,6 +12341,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state);
 	}
 
+	if (intel_state->modeset)
+		intel_verify_planes(intel_state);
+
 	if (intel_state->modeset && intel_can_enable_sagv(state))
 		intel_enable_sagv(dev_priv);
 
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev8)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (15 preceding siblings ...)
  2017-10-23 16:09 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-11-02 17:06 ` Patchwork
  2017-11-02 18:13 ` ✓ Fi.CI.IGT: " Patchwork
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-11-02 17:06 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev8)
URL   : https://patchwork.freedesktop.org/series/31758/
State : success

== Summary ==

Series 31758v8 drm/i915: Plane assert/readout cleanups etc.
https://patchwork.freedesktop.org/api/1.0/series/31758/revisions/8/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_cursor_legacy:
        Subgroup basic-flip-before-cursor-varying-size:
                skip       -> PASS       (fi-hsw-4770r)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (fi-skl-6700k) fdo#103546 +1

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103546 https://bugs.freedesktop.org/show_bug.cgi?id=103546

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:438s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:540s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:277s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:504s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:504s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:484s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:606s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:262s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:587s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:493s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:434s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:424s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:492s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:464s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:492s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:566s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:597s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:652s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:526s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:505s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:579s

fca2506bc5d492609e3f1b6e59d667e376a1eb3f drm-tip: 2017y-11m-02d-13h-10m-58s UTC integration manifest
5dddaaf97854 drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
33b9349ae2c9 drm/i915: s/enum plane/enum i9xx_plane_id/
894968974f00 drm/i915: Redo plane sanitation during readout
27e9f7a84447 drm/i915: Add .get_hw_state() method for planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6932/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Plane assert/readout cleanups etc. (rev8)
  2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (16 preceding siblings ...)
  2017-11-02 17:06 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev8) Patchwork
@ 2017-11-02 18:13 ` Patchwork
  17 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2017-11-02 18:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Plane assert/readout cleanups etc. (rev8)
URL   : https://patchwork.freedesktop.org/series/31758/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_flip:
        Subgroup vblank-vs-suspend:
                pass       -> SKIP       (shard-hsw) fdo#103375
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-A:
                dmesg-warn -> PASS       (shard-hsw)
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2539 pass:1430 dwarn:2   dfail:0   fail:9   skip:1098 time:9202s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6932/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v5 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-10-23 14:50   ` [PATCH v5 " Ville Syrjala
@ 2017-11-16 23:21     ` James Ausmus
  2017-11-17 15:41       ` Ville Syrjälä
  0 siblings, 1 reply; 30+ messages in thread
From: James Ausmus @ 2017-11-16 23:21 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Mon, Oct 23, 2017 at 05:50:32PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rename enum plane to enum i9xx_plane_id to make it clear that it only
> applies to pre-SKL platforms.
> 
> enum i9xx_plane_id is a global identifier, whereas enum plane_id is
> per-pipe. We need the old global identifier to index the primary plane
> (and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
> platforms.
> 
> v2: Reorder patches
> v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
>     Pimp the commit message a bit
>     Note that i9xx_plane_id doesn't apply to SKL+
> v4: Rebase due to power domain handling in plane readout
> v5: Rebase due to crtc->dspaddr_offset removal
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  6 +--
>  drivers/gpu/drm/i915/intel_display.c | 87 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
>  3 files changed, 49 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 54b5d4c582b6..a6b912c646f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -305,9 +305,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
>  
>  /*
>   * Global legacy plane identifier. Valid only for primary/sprite
> - * planes on pre-g4x, and only for primary planes on g4x+.
> + * planes on pre-g4x, and only for primary planes on g4x-bdw.
>   */
> -enum plane {
> +enum i9xx_plane_id {
>  	PLANE_A,
>  	PLANE_B,
>  	PLANE_C,
> @@ -1137,7 +1137,7 @@ struct intel_fbc {
>  
>  		struct {
>  			enum pipe pipe;
> -			enum plane plane;
> +			enum i9xx_plane_id plane;
>  			unsigned int fence_y_offset;
>  		} crtc;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4ea0f1ef249e..e726b65588aa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3223,16 +3223,16 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
>  	return 0;
>  }
>  
> -static void i9xx_update_primary_plane(struct intel_plane *primary,
> -				      const struct intel_crtc_state *crtc_state,
> -				      const struct intel_plane_state *plane_state)
> +static void i9xx_update_plane(struct intel_plane *plane,
> +			      const struct intel_crtc_state *crtc_state,
> +			      const struct intel_plane_state *plane_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
> -	enum plane plane = primary->plane;
> +	enum i9xx_plane_id plane_id = plane->plane;

It feels a bit ugly and counter-intuitive to have the two "plane"s in
"plane->plane" be different types - since i9xx_plane_id is a global id,
would it make sense to change the member naming to plane_gid or some
such (both in struct intel_plane and in struct intel_fbc->crtc)? It
feels like struct intel_plane should continue to be "plane", but we need
something else for enum i9xx_plane_id just for clarity's sake.

>  	u32 linear_offset;
>  	u32 dspcntr = plane_state->ctl;
> -	i915_reg_t reg = DSPCNTR(plane);
> +	i915_reg_t reg = DSPCNTR(plane_id);
>  	int x = plane_state->main.x;
>  	int y = plane_state->main.y;
>  	unsigned long irqflags;
> @@ -3251,34 +3251,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
>  		/* pipesrc and dspsize control the size that is scaled from,
>  		 * which should always be the user's requested size.
>  		 */
> -		I915_WRITE_FW(DSPSIZE(plane),
> +		I915_WRITE_FW(DSPSIZE(plane_id),
>  			      ((crtc_state->pipe_src_h - 1) << 16) |
>  			      (crtc_state->pipe_src_w - 1));
> -		I915_WRITE_FW(DSPPOS(plane), 0);
> -	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
> -		I915_WRITE_FW(PRIMSIZE(plane),
> +		I915_WRITE_FW(DSPPOS(plane_id), 0);
> +	} else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
> +		I915_WRITE_FW(PRIMSIZE(plane_id),
>  			      ((crtc_state->pipe_src_h - 1) << 16) |
>  			      (crtc_state->pipe_src_w - 1));
> -		I915_WRITE_FW(PRIMPOS(plane), 0);
> -		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
> +		I915_WRITE_FW(PRIMPOS(plane_id), 0);
> +		I915_WRITE_FW(PRIMCNSTALPHA(plane_id), 0);
>  	}
>  
>  	I915_WRITE_FW(reg, dspcntr);
>  
> -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> +	I915_WRITE_FW(DSPSTRIDE(plane_id), fb->pitches[0]);
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(plane_id),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
> -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> +		I915_WRITE_FW(DSPOFFSET(plane_id), (y << 16) | x);
>  	} else if (INTEL_GEN(dev_priv) >= 4) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(plane_id),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
> -		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
> -		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
> +		I915_WRITE_FW(DSPTILEOFF(plane_id), (y << 16) | x);
> +		I915_WRITE_FW(DSPLINOFF(plane_id), linear_offset);
>  	} else {
> -		I915_WRITE_FW(DSPADDR(plane),
> +		I915_WRITE_FW(DSPADDR(plane_id),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
>  	}
> @@ -3287,32 +3287,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> -static void i9xx_disable_primary_plane(struct intel_plane *primary,
> -				       struct intel_crtc *crtc)
> +static void i9xx_disable_plane(struct intel_plane *plane,
> +			       struct intel_crtc *crtc)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> -	enum plane plane = primary->plane;
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> +	enum i9xx_plane_id plane_id = plane->plane;
>  	unsigned long irqflags;
>  
>  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -	I915_WRITE_FW(DSPCNTR(plane), 0);
> -	if (INTEL_INFO(dev_priv)->gen >= 4)
> -		I915_WRITE_FW(DSPSURF(plane), 0);
> +	I915_WRITE_FW(DSPCNTR(plane_id), 0);
> +	if (INTEL_GEN(dev_priv) >= 4)

Nit: unrelated change/cleanup that's not mentioned in the commit message

> +		I915_WRITE_FW(DSPSURF(plane_id), 0);
>  	else
> -		I915_WRITE_FW(DSPADDR(plane), 0);
> -	POSTING_READ_FW(DSPCNTR(plane));
> +		I915_WRITE_FW(DSPADDR(plane_id), 0);
> +	POSTING_READ_FW(DSPCNTR(plane_id));
>  
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> -static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
> +static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
>  {
> -
> -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum intel_display_power_domain power_domain;
> -	enum plane plane = primary->plane;
> -	enum pipe pipe = primary->pipe;
> +	enum i9xx_plane_id plane_id = plane->plane;
> +	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
>  	/*
> @@ -3324,7 +3323,7 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
>  
> -	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
> +	ret = I915_READ(DSPCNTR(plane_id)) & DISPLAY_PLANE_ENABLE;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -13176,9 +13175,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
>  	 */
>  	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
> -		primary->plane = (enum plane) !pipe;
> +		primary->plane = (enum i9xx_plane_id) !pipe;
>  	else
> -		primary->plane = (enum plane) pipe;
> +		primary->plane = (enum i9xx_plane_id) pipe;
>  	primary->id = PLANE_PRIMARY;
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
> @@ -13207,16 +13206,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  		num_formats = ARRAY_SIZE(i965_primary_formats);
>  		modifiers = i9xx_format_modifiers;
>  
> -		primary->update_plane = i9xx_update_primary_plane;
> -		primary->disable_plane = i9xx_disable_primary_plane;
> +		primary->update_plane = i9xx_update_plane;
> +		primary->disable_plane = i9xx_disable_plane;
>  		primary->get_hw_state = i9xx_plane_get_hw_state;
>  	} else {
>  		intel_primary_formats = i8xx_primary_formats;
>  		num_formats = ARRAY_SIZE(i8xx_primary_formats);
>  		modifiers = i9xx_format_modifiers;
>  
> -		primary->update_plane = i9xx_update_primary_plane;
> -		primary->disable_plane = i9xx_disable_primary_plane;
> +		primary->update_plane = i9xx_update_plane;
> +		primary->disable_plane = i9xx_disable_plane;
>  		primary->get_hw_state = i9xx_plane_get_hw_state;
>  	}
>  
> @@ -13300,7 +13299,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
>  	cursor->can_scale = false;
>  	cursor->max_downscale = 1;
>  	cursor->pipe = pipe;
> -	cursor->plane = pipe;
> +	cursor->plane = (enum i9xx_plane_id) pipe;
>  	cursor->id = PLANE_CURSOR;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  
> @@ -14674,11 +14673,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>  }
>  
>  static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
> -				   struct intel_plane *primary)
> +				   struct intel_plane *plane)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum plane plane = primary->plane;
> -	u32 val = I915_READ(DSPCNTR(plane));
> +	enum i9xx_plane_id plane_id = plane->plane;
> +	u32 val = I915_READ(DSPCNTR(plane_id));
>  
>  	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
>  		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 07ba376c6fff..8e20924ab9df 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -796,7 +796,7 @@ struct intel_crtc_state {
>  struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
> -	enum plane plane;
> +	enum i9xx_plane_id plane;
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
>  	 * that crtc->enabled is set, i.e. the current mode configuration has
> @@ -841,7 +841,7 @@ struct intel_crtc {
>  
>  struct intel_plane {
>  	struct drm_plane base;
> -	u8 plane;
> +	enum i9xx_plane_id plane;
>  	enum plane_id id;
>  	enum pipe pipe;
>  	bool can_scale;
> @@ -1128,7 +1128,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>  }
>  
>  static inline struct intel_crtc *
> -intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
> +intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
>  {
>  	return dev_priv->plane_to_crtc_mapping[plane];
>  }
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout
  2017-10-13 13:58 ` [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
@ 2017-11-16 23:49   ` James Ausmus
  0 siblings, 0 replies; 30+ messages in thread
From: James Ausmus @ 2017-11-16 23:49 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Daniel Vetter, intel-gfx

On Fri, Oct 13, 2017 at 04:58:38PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since we now have a ->get_hw_state() method for planes, let's use
> that during the initial plane fb readout.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2da670628e35..268d320690f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7465,19 +7465,20 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  
> +	if (!plane->get_hw_state(plane))
> +		return;
> +
> +	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
> +	if (!intel_fb) {
> +		DRM_DEBUG_KMS("failed to alloc fb\n");
> +		return;
> +	}
> +
> +	fb = &intel_fb->base;
> +
> +	fb->dev = dev;
> +
>  	val = I915_READ(DSPCNTR(plane_id));
> -	if (!(val & DISPLAY_PLANE_ENABLE))
> -		return;
> -
> -	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
> -	if (!intel_fb) {
> -		DRM_DEBUG_KMS("failed to alloc fb\n");
> -		return;
> -	}
> -
> -	fb = &intel_fb->base;
> -
> -	fb->dev = dev;
>  
>  	if (INTEL_GEN(dev_priv) >= 4) {
>  		if (val & DISPPLANE_TILED) {
> @@ -8496,6 +8497,9 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  	struct drm_framebuffer *fb;
>  	struct intel_framebuffer *intel_fb;
>  
> +	if (!plane->get_hw_state(plane))
> +		return;
> +
>  	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
>  	if (!intel_fb) {
>  		DRM_DEBUG_KMS("failed to alloc fb\n");
> @@ -8507,8 +8511,6 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  	fb->dev = dev;
>  
>  	val = I915_READ(PLANE_CTL(pipe, plane_id));
> -	if (!(val & PLANE_CTL_ENABLE))
> -		goto error;
>  
>  	pixel_format = val & PLANE_CTL_FORMAT_MASK;
>  	fourcc = skl_format_to_fourcc(pixel_format,
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 10/10] drm/i915: Add rudimentary plane state verification
  2017-11-02 16:38   ` [PATCH v2 " Ville Syrjala
@ 2017-11-17  0:07     ` James Ausmus
  0 siblings, 0 replies; 30+ messages in thread
From: James Ausmus @ 2017-11-17  0:07 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: Daniel Vetter, intel-gfx

On Thu, Nov 02, 2017 at 06:38:32PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Check that the planes are in the state we expect them to be. For
> now we can only check whether each plane is correctly enabled or
> disabled. In the future we may want to expand the plane state
> readout to support a more through verification.

s/through/thorough/

> 
> v2: Verify all planes part of the state as long as at lest

s/lest/least

>     one crtc is doing a modeset (Daniel)
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

With those nits fixed:

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c23dad6d3c24..96e0a5fd69cf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11537,6 +11537,18 @@ verify_crtc_state(struct drm_crtc *crtc,
>  }
>  
>  static void
> +intel_verify_planes(struct intel_atomic_state *state)
> +{
> +	struct intel_plane *plane;
> +	const struct intel_plane_state *plane_state;
> +	int i;
> +
> +	for_each_new_intel_plane_in_state(state, plane,
> +					  plane_state, i)
> +		assert_plane(plane, plane_state->base.visible);
> +}
> +
> +static void
>  verify_single_dpll_state(struct drm_i915_private *dev_priv,
>  			 struct intel_shared_dpll *pll,
>  			 struct drm_crtc *crtc,
> @@ -12329,6 +12341,9 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state);
>  	}
>  
> +	if (intel_state->modeset)
> +		intel_verify_planes(intel_state);
> +
>  	if (intel_state->modeset && intel_can_enable_sagv(state))
>  		intel_enable_sagv(dev_priv);
>  
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v5 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-16 23:21     ` James Ausmus
@ 2017-11-17 15:41       ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2017-11-17 15:41 UTC (permalink / raw)
  To: James Ausmus; +Cc: intel-gfx

On Thu, Nov 16, 2017 at 03:21:32PM -0800, James Ausmus wrote:
> On Mon, Oct 23, 2017 at 05:50:32PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Rename enum plane to enum i9xx_plane_id to make it clear that it only
> > applies to pre-SKL platforms.
> > 
> > enum i9xx_plane_id is a global identifier, whereas enum plane_id is
> > per-pipe. We need the old global identifier to index the primary plane
> > (and the pre-g4x sprite C if we ever expose it) registers on pre-SKL
> > platforms.
> > 
> > v2: Reorder patches
> > v3: s/old_plane_id/i9xx_plane_id/ (Daniel)
> >     Pimp the commit message a bit
> >     Note that i9xx_plane_id doesn't apply to SKL+
> > v4: Rebase due to power domain handling in plane readout
> > v5: Rebase due to crtc->dspaddr_offset removal
> > 
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  6 +--
> >  drivers/gpu/drm/i915/intel_display.c | 87 ++++++++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
> >  3 files changed, 49 insertions(+), 50 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 54b5d4c582b6..a6b912c646f9 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -305,9 +305,9 @@ static inline bool transcoder_is_dsi(enum transcoder transcoder)
> >  
> >  /*
> >   * Global legacy plane identifier. Valid only for primary/sprite
> > - * planes on pre-g4x, and only for primary planes on g4x+.
> > + * planes on pre-g4x, and only for primary planes on g4x-bdw.
> >   */
> > -enum plane {
> > +enum i9xx_plane_id {
> >  	PLANE_A,
> >  	PLANE_B,
> >  	PLANE_C,
> > @@ -1137,7 +1137,7 @@ struct intel_fbc {
> >  
> >  		struct {
> >  			enum pipe pipe;
> > -			enum plane plane;
> > +			enum i9xx_plane_id plane;
> >  			unsigned int fence_y_offset;
> >  		} crtc;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4ea0f1ef249e..e726b65588aa 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3223,16 +3223,16 @@ int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
> >  	return 0;
> >  }
> >  
> > -static void i9xx_update_primary_plane(struct intel_plane *primary,
> > -				      const struct intel_crtc_state *crtc_state,
> > -				      const struct intel_plane_state *plane_state)
> > +static void i9xx_update_plane(struct intel_plane *plane,
> > +			      const struct intel_crtc_state *crtc_state,
> > +			      const struct intel_plane_state *plane_state)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> >  	const struct drm_framebuffer *fb = plane_state->base.fb;
> > -	enum plane plane = primary->plane;
> > +	enum i9xx_plane_id plane_id = plane->plane;
> 
> It feels a bit ugly and counter-intuitive to have the two "plane"s in
> "plane->plane"

It's always been like that. Well, ever since we had planes.
Nothing new there. At least I got rid of the magic
'plane->plane + 1' from the sprite code.

> be different types - since i9xx_plane_id is a global id,
> would it make sense to change the member naming to plane_gid or some

"gid" would confuse me more. It makes me think of uid/gid. We could name
it to plane->i9xx_plane[_id] I suppose to match the type.

> such (both in struct intel_plane and in struct intel_fbc->crtc)?

The fbc mess is going away thankfully. At which point the uses of
i9xx_plane_id will be tucked away neatly in platform specific code
instead of leaking too badly to common code.

> It
> feels like struct intel_plane should continue to be "plane", but we need
> something else for enum i9xx_plane_id just for clarity's sake.

This is I think the third attempt at coming up with something.

I might just have to rename plane->plane to plane->bikeshed to more
accurately reflect its role in i915 development ;)

> 
> >  	u32 linear_offset;
> >  	u32 dspcntr = plane_state->ctl;
> > -	i915_reg_t reg = DSPCNTR(plane);
> > +	i915_reg_t reg = DSPCNTR(plane_id);
> >  	int x = plane_state->main.x;
> >  	int y = plane_state->main.y;
> >  	unsigned long irqflags;
> > @@ -3251,34 +3251,34 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
> >  		/* pipesrc and dspsize control the size that is scaled from,
> >  		 * which should always be the user's requested size.
> >  		 */
> > -		I915_WRITE_FW(DSPSIZE(plane),
> > +		I915_WRITE_FW(DSPSIZE(plane_id),
> >  			      ((crtc_state->pipe_src_h - 1) << 16) |
> >  			      (crtc_state->pipe_src_w - 1));
> > -		I915_WRITE_FW(DSPPOS(plane), 0);
> > -	} else if (IS_CHERRYVIEW(dev_priv) && plane == PLANE_B) {
> > -		I915_WRITE_FW(PRIMSIZE(plane),
> > +		I915_WRITE_FW(DSPPOS(plane_id), 0);
> > +	} else if (IS_CHERRYVIEW(dev_priv) && plane_id == PLANE_B) {
> > +		I915_WRITE_FW(PRIMSIZE(plane_id),
> >  			      ((crtc_state->pipe_src_h - 1) << 16) |
> >  			      (crtc_state->pipe_src_w - 1));
> > -		I915_WRITE_FW(PRIMPOS(plane), 0);
> > -		I915_WRITE_FW(PRIMCNSTALPHA(plane), 0);
> > +		I915_WRITE_FW(PRIMPOS(plane_id), 0);
> > +		I915_WRITE_FW(PRIMCNSTALPHA(plane_id), 0);
> >  	}
> >  
> >  	I915_WRITE_FW(reg, dspcntr);
> >  
> > -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> > +	I915_WRITE_FW(DSPSTRIDE(plane_id), fb->pitches[0]);
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(plane_id),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> > -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> > +		I915_WRITE_FW(DSPOFFSET(plane_id), (y << 16) | x);
> >  	} else if (INTEL_GEN(dev_priv) >= 4) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(plane_id),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> > -		I915_WRITE_FW(DSPTILEOFF(plane), (y << 16) | x);
> > -		I915_WRITE_FW(DSPLINOFF(plane), linear_offset);
> > +		I915_WRITE_FW(DSPTILEOFF(plane_id), (y << 16) | x);
> > +		I915_WRITE_FW(DSPLINOFF(plane_id), linear_offset);
> >  	} else {
> > -		I915_WRITE_FW(DSPADDR(plane),
> > +		I915_WRITE_FW(DSPADDR(plane_id),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> >  	}
> > @@ -3287,32 +3287,31 @@ static void i9xx_update_primary_plane(struct intel_plane *primary,
> >  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> >  }
> >  
> > -static void i9xx_disable_primary_plane(struct intel_plane *primary,
> > -				       struct intel_crtc *crtc)
> > +static void i9xx_disable_plane(struct intel_plane *plane,
> > +			       struct intel_crtc *crtc)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > -	enum plane plane = primary->plane;
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> > +	enum i9xx_plane_id plane_id = plane->plane;
> >  	unsigned long irqflags;
> >  
> >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> > -	I915_WRITE_FW(DSPCNTR(plane), 0);
> > -	if (INTEL_INFO(dev_priv)->gen >= 4)
> > -		I915_WRITE_FW(DSPSURF(plane), 0);
> > +	I915_WRITE_FW(DSPCNTR(plane_id), 0);
> > +	if (INTEL_GEN(dev_priv) >= 4)
> 
> Nit: unrelated change/cleanup that's not mentioned in the commit message
> 
> > +		I915_WRITE_FW(DSPSURF(plane_id), 0);
> >  	else
> > -		I915_WRITE_FW(DSPADDR(plane), 0);
> > -	POSTING_READ_FW(DSPCNTR(plane));
> > +		I915_WRITE_FW(DSPADDR(plane_id), 0);
> > +	POSTING_READ_FW(DSPCNTR(plane_id));
> >  
> >  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> >  }
> >  
> > -static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
> > +static bool i9xx_plane_get_hw_state(struct intel_plane *plane)
> >  {
> > -
> > -	struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
> > +	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
> >  	enum intel_display_power_domain power_domain;
> > -	enum plane plane = primary->plane;
> > -	enum pipe pipe = primary->pipe;
> > +	enum i9xx_plane_id plane_id = plane->plane;
> > +	enum pipe pipe = plane->pipe;
> >  	bool ret;
> >  
> >  	/*
> > @@ -3324,7 +3323,7 @@ static bool i9xx_plane_get_hw_state(struct intel_plane *primary)
> >  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >  		return false;
> >  
> > -	ret = I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE;
> > +	ret = I915_READ(DSPCNTR(plane_id)) & DISPLAY_PLANE_ENABLE;
> >  
> >  	intel_display_power_put(dev_priv, power_domain);
> >  
> > @@ -13176,9 +13175,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  	 * port is hooked to pipe B. Hence we want plane A feeding pipe B.
> >  	 */
> >  	if (HAS_FBC(dev_priv) && INTEL_GEN(dev_priv) < 4)
> > -		primary->plane = (enum plane) !pipe;
> > +		primary->plane = (enum i9xx_plane_id) !pipe;
> >  	else
> > -		primary->plane = (enum plane) pipe;
> > +		primary->plane = (enum i9xx_plane_id) pipe;
> >  	primary->id = PLANE_PRIMARY;
> >  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> >  	primary->check_plane = intel_check_primary_plane;
> > @@ -13207,16 +13206,16 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  		num_formats = ARRAY_SIZE(i965_primary_formats);
> >  		modifiers = i9xx_format_modifiers;
> >  
> > -		primary->update_plane = i9xx_update_primary_plane;
> > -		primary->disable_plane = i9xx_disable_primary_plane;
> > +		primary->update_plane = i9xx_update_plane;
> > +		primary->disable_plane = i9xx_disable_plane;
> >  		primary->get_hw_state = i9xx_plane_get_hw_state;
> >  	} else {
> >  		intel_primary_formats = i8xx_primary_formats;
> >  		num_formats = ARRAY_SIZE(i8xx_primary_formats);
> >  		modifiers = i9xx_format_modifiers;
> >  
> > -		primary->update_plane = i9xx_update_primary_plane;
> > -		primary->disable_plane = i9xx_disable_primary_plane;
> > +		primary->update_plane = i9xx_update_plane;
> > +		primary->disable_plane = i9xx_disable_plane;
> >  		primary->get_hw_state = i9xx_plane_get_hw_state;
> >  	}
> >  
> > @@ -13300,7 +13299,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
> >  	cursor->can_scale = false;
> >  	cursor->max_downscale = 1;
> >  	cursor->pipe = pipe;
> > -	cursor->plane = pipe;
> > +	cursor->plane = (enum i9xx_plane_id) pipe;
> >  	cursor->id = PLANE_CURSOR;
> >  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> >  
> > @@ -14674,11 +14673,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  }
> >  
> >  static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
> > -				   struct intel_plane *primary)
> > +				   struct intel_plane *plane)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > -	enum plane plane = primary->plane;
> > -	u32 val = I915_READ(DSPCNTR(plane));
> > +	enum i9xx_plane_id plane_id = plane->plane;
> > +	u32 val = I915_READ(DSPCNTR(plane_id));
> >  
> >  	return (val & DISPLAY_PLANE_ENABLE) == 0 ||
> >  		(val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 07ba376c6fff..8e20924ab9df 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -796,7 +796,7 @@ struct intel_crtc_state {
> >  struct intel_crtc {
> >  	struct drm_crtc base;
> >  	enum pipe pipe;
> > -	enum plane plane;
> > +	enum i9xx_plane_id plane;
> >  	/*
> >  	 * Whether the crtc and the connected output pipeline is active. Implies
> >  	 * that crtc->enabled is set, i.e. the current mode configuration has
> > @@ -841,7 +841,7 @@ struct intel_crtc {
> >  
> >  struct intel_plane {
> >  	struct drm_plane base;
> > -	u8 plane;
> > +	enum i9xx_plane_id plane;
> >  	enum plane_id id;
> >  	enum pipe pipe;
> >  	bool can_scale;
> > @@ -1128,7 +1128,7 @@ intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  }
> >  
> >  static inline struct intel_crtc *
> > -intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum plane plane)
> > +intel_get_crtc_for_plane(struct drm_i915_private *dev_priv, enum i9xx_plane_id plane)
> >  {
> >  	return dev_priv->plane_to_crtc_mapping[plane];
> >  }
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2017-11-17 15:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-13 13:58 [PATCH v2 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
2017-10-13 13:58 ` [PATCH v2 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
2017-10-13 17:36   ` [PATCH v3 " Ville Syrjala
2017-10-23 14:50   ` [PATCH v4 " Ville Syrjala
2017-10-13 13:58 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
2017-10-13 13:58 ` [PATCH v3 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
2017-10-13 17:37   ` [PATCH v4 " Ville Syrjala
2017-10-23 14:50   ` [PATCH v5 " Ville Syrjala
2017-11-16 23:21     ` James Ausmus
2017-11-17 15:41       ` Ville Syrjälä
2017-10-13 13:58 ` [PATCH v2 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
2017-10-13 13:58 ` [PATCH v3 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
2017-10-13 13:58 ` [PATCH 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
2017-10-13 13:58 ` [PATCH 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
2017-10-13 13:58 ` [PATCH v2 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
2017-10-23 14:51   ` [PATCH v3 " Ville Syrjala
2017-10-13 13:58 ` [PATCH 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
2017-11-16 23:49   ` James Ausmus
2017-10-13 13:58 ` [PATCH 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
2017-11-02 16:38   ` [PATCH v2 " Ville Syrjala
2017-11-17  0:07     ` James Ausmus
2017-10-13 16:24 ` ✗ Fi.CI.BAT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
2017-10-13 16:53   ` Ville Syrjälä
2017-10-13 18:05 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
2017-10-13 23:20 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev2) Patchwork
2017-10-14  2:35 ` ✗ Fi.CI.IGT: failure for drm/i915: Plane assert/readout cleanups etc. (rev4) Patchwork
2017-10-23 15:10 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev7) Patchwork
2017-10-23 16:09 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-02 17:06 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev8) Patchwork
2017-11-02 18:13 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.