All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc.
@ 2017-11-17 19:19 Ville Syrjala
  2017-11-17 19:19 ` [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 UTC (permalink / raw)
  To: intel-gfx

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

OK, one more time. This time with s/plane/i9xx_plane/ etc. all over.
Maybe that will make everyone happy? Unlikely, but let's try.

Patch 3 is the only one missing r-b.

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 | 519 ++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_drv.h     |   9 +-
 drivers/gpu/drm/i915/intel_fbc.c     |  35 ++-
 drivers/gpu/drm/i915/intel_pm.c      |  36 +--
 drivers/gpu/drm/i915/intel_sprite.c  |  85 +++++-
 6 files changed, 373 insertions(+), 327 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] 20+ messages in thread

* [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v2
Tested-by: Thierry Reding <thierry.reding@gmail.com> #v2
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 3b3dec1e6640..7bf8290f0343 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1190,23 +1190,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)
 {
@@ -1234,77 +1217,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)
@@ -1896,9 +1827,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
@@ -1968,9 +1897,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);
@@ -3364,6 +3291,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)
 {
@@ -4879,7 +4831,8 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
 	 * 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,
@@ -4913,7 +4866,8 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state)
 	if (!crtc_state->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));
@@ -9499,6 +9453,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)
 {
@@ -9692,6 +9663,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 = {
@@ -13279,6 +13272,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);
@@ -13289,6 +13283,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);
@@ -13296,6 +13291,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);
@@ -13303,6 +13299,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)
@@ -13392,10 +13389,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;
 	}
 
@@ -14761,8 +14760,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));
@@ -14976,20 +14975,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 69aab324aaa1..99840f3940c7 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -866,6 +866,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);
@@ -1934,6 +1935,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 ce615704982a..5baa0023964e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -325,6 +325,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)
 {
@@ -502,6 +522,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)
 {
@@ -642,6 +682,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)
 {
@@ -773,6 +832,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,
@@ -1231,6 +1309,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);
@@ -1241,6 +1320,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);
@@ -1251,6 +1331,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);
@@ -1266,6 +1347,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);
@@ -1276,6 +1358,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] 20+ messages in thread

* [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
  2017-11-17 19:19 ` [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-12-01 18:05   ` Alex Villacis Lasso
  2017-11-17 19:19 ` [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.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 7bf8290f0343..91f74c5373b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2726,6 +2726,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)
@@ -2783,12 +2800,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;
 
@@ -5869,6 +5881,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;
@@ -5877,11 +5890,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);
@@ -14773,22 +14787,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)
@@ -14844,33 +14872,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))
@@ -14978,14 +14988,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)
@@ -15193,6 +15207,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] 20+ messages in thread

* [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
  2017-11-17 19:19 ` [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
  2017-11-17 19:19 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 20:12   ` Rodrigo Vivi
  2017-11-20 17:50   ` James Ausmus
  2017-11-17 19:19 ` [PATCH v3 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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
v6: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
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 | 98 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +--
 drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
 drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
 5 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2158a758a17d..55dd602582cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -304,9 +304,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,
@@ -1145,7 +1145,7 @@ struct intel_fbc {
 
 		struct {
 			enum pipe pipe;
-			enum plane plane;
+			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
-	i915_reg_t reg = DSPCNTR(plane);
+	i915_reg_t reg = DSPCNTR(i9xx_plane);
 	int x = plane_state->main.x;
 	int y = plane_state->main.y;
 	unsigned long irqflags;
@@ -3248,34 +3248,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(i9xx_plane),
 			      ((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(i9xx_plane), 0);
+	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
+		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
 			      ((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(i9xx_plane), 0);
+		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
 	}
 
 	I915_WRITE_FW(reg, dspcntr);
 
-	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
+	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(i9xx_plane),
 			      intel_plane_ggtt_offset(plane_state) +
 			      dspaddr_offset);
-		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
+		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
 	} else if (INTEL_GEN(dev_priv) >= 4) {
-		I915_WRITE_FW(DSPSURF(plane),
+		I915_WRITE_FW(DSPSURF(i9xx_plane),
 			      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(i9xx_plane), (y << 16) | x);
+		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
 	} else {
-		I915_WRITE_FW(DSPADDR(plane),
+		I915_WRITE_FW(DSPADDR(i9xx_plane),
 			      intel_plane_ggtt_offset(plane_state) +
 			      dspaddr_offset);
 	}
@@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
+	if (INTEL_GEN(dev_priv) >= 4)
+		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
 	else
-		I915_WRITE_FW(DSPADDR(plane), 0);
-	POSTING_READ_FW(DSPCNTR(plane));
+		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
+	POSTING_READ_FW(DSPCNTR(i9xx_plane));
 
 	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 i9xx_plane = plane->i9xx_plane;
+	enum pipe pipe = plane->pipe;
 	bool ret;
 
 	/*
@@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
 
 	intel_display_power_put(dev_priv, power_domain);
 
@@ -7406,7 +7405,7 @@ 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);
 	u32 val, base, offset;
-	int pipe = crtc->pipe, plane = crtc->plane;
+	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
 	int fourcc, pixel_format;
 	unsigned int aligned_height;
 	struct drm_framebuffer *fb;
@@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
 	else
-		primary->plane = (enum plane) pipe;
+		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
 	primary->id = PLANE_PRIMARY;
 	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
 	primary->check_plane = intel_check_primary_plane;
@@ -13303,16 +13302,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;
 	}
 
@@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 					       intel_primary_formats, num_formats,
 					       modifiers,
 					       DRM_PLANE_TYPE_PRIMARY,
-					       "plane %c", plane_name(primary->plane));
+					       "plane %c",
+					       plane_name(primary->i9xx_plane));
 	if (ret)
 		goto fail;
 
@@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
 	cursor->id = PLANE_CURSOR;
 	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
 
@@ -13524,14 +13524,14 @@ 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;
+	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
+	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
 	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
@@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
+	u32 val = I915_READ(DSPCNTR(i9xx_plane));
 
 	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 99840f3940c7..d1fe7be94b62 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -799,7 +799,7 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum plane plane;
+	enum i9xx_plane_id i9xx_plane;
 	/*
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
@@ -844,7 +844,7 @@ struct intel_crtc {
 
 struct intel_plane {
 	struct drm_plane base;
-	u8 plane;
+	enum i9xx_plane_id i9xx_plane;
 	enum plane_id id;
 	enum pipe pipe;
 	bool can_scale;
@@ -1130,7 +1130,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];
 }
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1a0f5e0c8d10..3133131306a9 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 
 		/* Set it up... */
 		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
-		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
+		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
 		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
 		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
 	}
@@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
 	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
 	u32 dpfc_ctl;
 
-	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
+	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
 	if (params->fb.format->cpp[0] == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
 	else
@@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 	u32 dpfc_ctl;
 	int threshold = dev_priv->fbc.threshold;
 
-	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
+	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
 	if (params->fb.format->cpp[0] == 2)
 		threshold++;
 
@@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 
 	dpfc_ctl = 0;
 	if (IS_IVYBRIDGE(dev_priv))
-		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
+		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
 
 	if (params->fb.format->cpp[0] == 2)
 		threshold++;
@@ -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.i9xx_plane = crtc->i9xx_plane;
 	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
 
 	params->fb.format = cache->fb.format;
@@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
 			continue;
 
 		intel_crtc_state = to_intel_crtc_state(
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5baa0023964e..dd485f59eb1d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 	}
 
 	intel_plane->pipe = pipe;
-	intel_plane->plane = plane;
+	intel_plane->i9xx_plane = plane;
 	intel_plane->id = PLANE_SPRITE0 + plane;
 	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
 	intel_plane->check_plane = intel_check_sprite_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] 20+ messages in thread

* [PATCH v3 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (2 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v5 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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)
v3: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 55dd602582cb..bbeefa2c11ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -698,7 +698,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 i9xx_plane);
 	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 3c55e4026331..f22cc8468cc8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -518,38 +518,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 i9xx_plane)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
 
 	size = dsparb & 0x7f;
-	if (plane)
+	if (i9xx_plane == 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(i9xx_plane), 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 i9xx_plane)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
 
 	size = dsparb & 0x1ff;
-	if (plane)
+	if (i9xx_plane == 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(i9xx_plane), 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 i9xx_plane)
 {
 	uint32_t dsparb = I915_READ(DSPARB);
 	int size;
@@ -557,9 +560,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(i9xx_plane), size);
 
 	return size;
 }
@@ -2283,8 +2285,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;
@@ -2310,8 +2312,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;
@@ -2423,7 +2425,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] 20+ messages in thread

* [PATCH v5 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (3 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v3 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v2 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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)
v4: Rebase due to GLK/CNL PLANE_COLOR_CTL alpha stuff
v5: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-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 | 60 ++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 16ac86816f28..0c407cb0e6aa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7404,14 +7404,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 i9xx_plane = plane->i9xx_plane;
+	enum pipe pipe = crtc->pipe;
 	u32 val, base, offset;
-	int pipe = crtc->pipe, plane = crtc->i9xx_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(i9xx_plane));
 	if (!(val & DISPLAY_PLANE_ENABLE))
 		return;
 
@@ -7438,12 +7440,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(i9xx_plane));
 		else
-			offset = I915_READ(DSPLINOFF(plane));
-		base = I915_READ(DSPSURF(plane)) & 0xfffff000;
+			offset = I915_READ(DSPLINOFF(i9xx_plane));
+		base = I915_READ(DSPSURF(i9xx_plane)) & 0xfffff000;
 	} else {
-		base = I915_READ(DSPADDR(plane));
+		base = I915_READ(DSPADDR(i9xx_plane));
 	}
 	plane_config->base = base;
 
@@ -7451,15 +7453,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(i9xx_plane));
 	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);
 
@@ -8428,8 +8430,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, alpha;
-	int pipe = crtc->pipe;
 	int fourcc, pixel_format;
 	unsigned int aligned_height;
 	struct drm_framebuffer *fb;
@@ -8445,14 +8449,14 @@ 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;
 
 	pixel_format = val & PLANE_CTL_FORMAT_MASK;
 
 	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
-		alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
+		alpha = I915_READ(PLANE_COLOR_CTL(pipe, plane_id));
 		alpha &= PLANE_COLOR_ALPHA_MASK;
 	} else {
 		alpha = val & PLANE_CTL_ALPHA_MASK;
@@ -8488,16 +8492,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;
 
@@ -8505,8 +8509,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);
 
@@ -8547,14 +8551,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 i9xx_plane = plane->i9xx_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(i9xx_plane));
 	if (!(val & DISPLAY_PLANE_ENABLE))
 		return;
 
@@ -8579,14 +8585,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(i9xx_plane)) & 0xfffff000;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		offset = I915_READ(DSPOFFSET(pipe));
+		offset = I915_READ(DSPOFFSET(i9xx_plane));
 	} else {
 		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(pipe));
+			offset = I915_READ(DSPTILEOFF(i9xx_plane));
 		else
-			offset = I915_READ(DSPLINOFF(pipe));
+			offset = I915_READ(DSPLINOFF(i9xx_plane));
 	}
 	plane_config->base = base;
 
@@ -8594,15 +8600,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(i9xx_plane));
 	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] 20+ messages in thread

* [PATCH v2 06/10] drm/i915: Nuke ironlake_get_initial_plane_config()
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (4 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v5 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v2 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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().

v2: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
Reviewed-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 | 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 0c407cb0e6aa..c1d7547c1457 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7438,7 +7438,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(i9xx_plane));
+		base = I915_READ(DSPSURF(i9xx_plane)) & 0xfffff000;
+	} else if (INTEL_GEN(dev_priv) >= 4) {
 		if (plane_config->tiling)
 			offset = I915_READ(DSPTILEOFF(i9xx_plane));
 		else
@@ -8545,76 +8548,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 i9xx_plane = plane->i9xx_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(i9xx_plane));
-	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(i9xx_plane)) & 0xfffff000;
-	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		offset = I915_READ(DSPOFFSET(i9xx_plane));
-	} else {
-		if (plane_config->tiling)
-			offset = I915_READ(DSPTILEOFF(i9xx_plane));
-		else
-			offset = I915_READ(DSPLINOFF(i9xx_plane));
-	}
-	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(i9xx_plane));
-	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)
 {
@@ -14217,7 +14150,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;
@@ -14225,7 +14158,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] 20+ messages in thread

* [PATCH v2 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (5 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v2 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v3 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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.

v2: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 bbeefa2c11ba..9b4857096dd2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -560,13 +560,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; \
@@ -576,7 +576,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 c1d7547c1457..b1ead3f95cde 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12080,7 +12080,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 d1fe7be94b62..3ebe62666108 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1652,7 +1652,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 3133131306a9..474234322b8b 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1054,11 +1054,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;
 
@@ -1066,7 +1066,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))
@@ -1076,13 +1076,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)
@@ -1091,10 +1089,9 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 		if (fbc_on_plane_a_only(dev_priv) && crtc->i9xx_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] 20+ messages in thread

* [PATCH v3 08/10] drm/i915: Nuke crtc->plane
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (6 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v2 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v3 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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
v4: s/plane/i9xx_plane/ etc. (James)

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-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 | 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 b1ead3f95cde..62cc2a600205 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13463,14 +13463,13 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 		goto fail;
 
 	intel_crtc->pipe = pipe;
-	intel_crtc->i9xx_plane = primary->i9xx_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->i9xx_plane] != NULL);
-	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
+	       dev_priv->plane_to_crtc_mapping[primary->i9xx_plane] != NULL);
+	dev_priv->plane_to_crtc_mapping[primary->i9xx_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 3ebe62666108..635a96fcd788 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -799,7 +799,6 @@ struct intel_crtc_state {
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
-	enum i9xx_plane_id i9xx_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 474234322b8b..4aefc658a5cf 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.i9xx_plane = crtc->i9xx_plane;
+	params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_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->i9xx_plane != PLANE_A)
+		if (fbc_on_plane_a_only(dev_priv) && plane->i9xx_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] 20+ messages in thread

* [PATCH v3 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (7 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v3 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:19 ` [PATCH v3 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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.

v2: s/plane/i9xx_plane/ etc. (James)

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 62cc2a600205..ed58311e8da0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7413,8 +7413,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 	struct drm_framebuffer *fb;
 	struct intel_framebuffer *intel_fb;
 
-	val = I915_READ(DSPCNTR(i9xx_plane));
-	if (!(val & DISPLAY_PLANE_ENABLE))
+	if (!plane->get_hw_state(plane))
 		return;
 
 	intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
@@ -7427,6 +7426,8 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
 
 	fb->dev = dev;
 
+	val = I915_READ(DSPCNTR(i9xx_plane));
+
 	if (INTEL_GEN(dev_priv) >= 4) {
 		if (val & DISPPLANE_TILED) {
 			plane_config->tiling = I915_TILING_X;
@@ -8442,6 +8443,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");
@@ -8453,8 +8457,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;
 
-- 
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] 20+ messages in thread

* [PATCH v3 10/10] drm/i915: Add rudimentary plane state verification
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (8 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v3 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
@ 2017-11-17 19:19 ` Ville Syrjala
  2017-11-17 19:54 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev9) Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-11-17 19:19 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 thorough verification.

v2: Verify all planes part of the state as long as at least
    one crtc is doing a modeset (Daniel)
v3: Fix typoes (James)

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: James Ausmus <james.ausmus@intel.com>
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 ed58311e8da0..5ca7b33554ef 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11601,6 +11601,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,
@@ -12393,6 +12405,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] 20+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev9)
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (9 preceding siblings ...)
  2017-11-17 19:19 ` [PATCH v3 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
@ 2017-11-17 19:54 ` Patchwork
  2017-11-17 20:08 ` [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Rodrigo Vivi
  2017-11-17 20:40 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev9) Patchwork
  12 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-17 19:54 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:454s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:379s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:554s
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:500s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:506s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:487s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:266s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:541s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:438s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:426s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:482s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:527s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:578s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:469s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:566s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:524s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:499s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:459s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:558s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:416s
Blacklisted hosts:
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:607s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:548s
fi-glk-dsi       total:156  pass:133  dwarn:0   dfail:0   fail:0   skip:22 

74ae8acff97c1739330154fa34bf5a64e28d608f drm-tip: 2017y-11m-17d-17h-53m-01s UTC integration manifest
52af1bc14617 drm/i915: Add rudimentary plane state verification
29561aa75bfd drm/i915: Use plane->get_hw_state() for initial plane fb readout
ee2987349d8c drm/i915: Nuke crtc->plane
2aeaf2574af6 drm/i915: Switch fbc over to for_each_new_intel_plane_in_state()
d02162cc6092 drm/i915: Nuke ironlake_get_initial_plane_config()
481703c326e8 drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout
a2bc7475c85d drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks
a99138d4f433 drm/i915: s/enum plane/enum i9xx_plane_id/
9b752b27b383 drm/i915: Redo plane sanitation during readout
3f6b3bc03e92 drm/i915: Add .get_hw_state() method for planes

== Logs ==

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

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

* Re: [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc.
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (10 preceding siblings ...)
  2017-11-17 19:54 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev9) Patchwork
@ 2017-11-17 20:08 ` Rodrigo Vivi
  2017-11-17 20:40 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev9) Patchwork
  12 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2017-11-17 20:08 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Nov 17, 2017 at 07:19:07PM +0000, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> OK, one more time. This time with s/plane/i9xx_plane/ etc. all over.

why not intel_plane?!

> Maybe that will make everyone happy? Unlikely, but let's try.

:)

> 
> Patch 3 is the only one missing r-b.
> 
> 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 | 519 ++++++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_drv.h     |   9 +-
>  drivers/gpu/drm/i915/intel_fbc.c     |  35 ++-
>  drivers/gpu/drm/i915/intel_pm.c      |  36 +--
>  drivers/gpu/drm/i915/intel_sprite.c  |  85 +++++-
>  6 files changed, 373 insertions(+), 327 deletions(-)
> 
> -- 
> 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] 20+ messages in thread

* Re: [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-17 19:19 ` [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
@ 2017-11-17 20:12   ` Rodrigo Vivi
  2017-11-17 20:32     ` Ville Syrjälä
  2017-11-20 17:50   ` James Ausmus
  1 sibling, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2017-11-17 20:12 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Nov 17, 2017 at 07:19:10PM +0000, 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.

Oh! I should had read this before commenting on the cover letter... sorry...

Anyway I don't believe it make clear that it is not skl... because it means
it started on i9xx, but not that ended on skl... just "plane" was already
good enough imho... but it is really just a comment...
don't take this as a nack anyhow... ;)

> 
> 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
> v6: s/plane/i9xx_plane/ etc. (James)
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> 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 | 98 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
>  drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
>  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>  5 files changed, 62 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2158a758a17d..55dd602582cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -304,9 +304,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,
> @@ -1145,7 +1145,7 @@ struct intel_fbc {
>  
>  		struct {
>  			enum pipe pipe;
> -			enum plane plane;
> +			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
>  	u32 linear_offset;
>  	u32 dspcntr = plane_state->ctl;
> -	i915_reg_t reg = DSPCNTR(plane);
> +	i915_reg_t reg = DSPCNTR(i9xx_plane);
>  	int x = plane_state->main.x;
>  	int y = plane_state->main.y;
>  	unsigned long irqflags;
> @@ -3248,34 +3248,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(i9xx_plane),
>  			      ((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(i9xx_plane), 0);
> +	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> +		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
>  			      ((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(i9xx_plane), 0);
> +		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
>  	}
>  
>  	I915_WRITE_FW(reg, dspcntr);
>  
> -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> +	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(i9xx_plane),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
> -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> +		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
>  	} else if (INTEL_GEN(dev_priv) >= 4) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(i9xx_plane),
>  			      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(i9xx_plane), (y << 16) | x);
> +		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
>  	} else {
> -		I915_WRITE_FW(DSPADDR(plane),
> +		I915_WRITE_FW(DSPADDR(i9xx_plane),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
>  	}
> @@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
> +	if (INTEL_GEN(dev_priv) >= 4)
> +		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
>  	else
> -		I915_WRITE_FW(DSPADDR(plane), 0);
> -	POSTING_READ_FW(DSPCNTR(plane));
> +		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> +	POSTING_READ_FW(DSPCNTR(i9xx_plane));
>  
>  	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 i9xx_plane = plane->i9xx_plane;
> +	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
>  	/*
> @@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -7406,7 +7405,7 @@ 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);
>  	u32 val, base, offset;
> -	int pipe = crtc->pipe, plane = crtc->plane;
> +	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
>  	int fourcc, pixel_format;
>  	unsigned int aligned_height;
>  	struct drm_framebuffer *fb;
> @@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
>  	else
> -		primary->plane = (enum plane) pipe;
> +		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
>  	primary->id = PLANE_PRIMARY;
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
> @@ -13303,16 +13302,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;
>  	}
>  
> @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  					       intel_primary_formats, num_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_PRIMARY,
> -					       "plane %c", plane_name(primary->plane));
> +					       "plane %c",
> +					       plane_name(primary->i9xx_plane));
>  	if (ret)
>  		goto fail;
>  
> @@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
>  	cursor->id = PLANE_CURSOR;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  
> @@ -13524,14 +13524,14 @@ 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;
> +	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
> +	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
>  
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> @@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
> +	u32 val = I915_READ(DSPCNTR(i9xx_plane));
>  
>  	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 99840f3940c7..d1fe7be94b62 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -799,7 +799,7 @@ struct intel_crtc_state {
>  struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
> -	enum plane plane;
> +	enum i9xx_plane_id i9xx_plane;
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
>  	 * that crtc->enabled is set, i.e. the current mode configuration has
> @@ -844,7 +844,7 @@ struct intel_crtc {
>  
>  struct intel_plane {
>  	struct drm_plane base;
> -	u8 plane;
> +	enum i9xx_plane_id i9xx_plane;
>  	enum plane_id id;
>  	enum pipe pipe;
>  	bool can_scale;
> @@ -1130,7 +1130,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];
>  }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 1a0f5e0c8d10..3133131306a9 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  		/* Set it up... */
>  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> -		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> +		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
>  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
>  		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
>  	}
> @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
>  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
>  	u32 dpfc_ctl;
>  
> -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
>  	if (params->fb.format->cpp[0] == 2)
>  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
>  	else
> @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  	u32 dpfc_ctl;
>  	int threshold = dev_priv->fbc.threshold;
>  
> -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
>  	if (params->fb.format->cpp[0] == 2)
>  		threshold++;
>  
> @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  	dpfc_ctl = 0;
>  	if (IS_IVYBRIDGE(dev_priv))
> -		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
>  
>  	if (params->fb.format->cpp[0] == 2)
>  		threshold++;
> @@ -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.i9xx_plane = crtc->i9xx_plane;
>  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
>  
>  	params->fb.format = cache->fb.format;
> @@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
>  			continue;
>  
>  		intel_crtc_state = to_intel_crtc_state(
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5baa0023964e..dd485f59eb1d 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  	}
>  
>  	intel_plane->pipe = pipe;
> -	intel_plane->plane = plane;
> +	intel_plane->i9xx_plane = plane;
>  	intel_plane->id = PLANE_SPRITE0 + plane;
>  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>  	intel_plane->check_plane = intel_check_sprite_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] 20+ messages in thread

* Re: [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-17 20:12   ` Rodrigo Vivi
@ 2017-11-17 20:32     ` Ville Syrjälä
  2017-11-17 21:04       ` Rodrigo Vivi
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2017-11-17 20:32 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Fri, Nov 17, 2017 at 12:12:15PM -0800, Rodrigo Vivi wrote:
> On Fri, Nov 17, 2017 at 07:19:10PM +0000, 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.
> 
> Oh! I should had read this before commenting on the cover letter... sorry...
> 
> Anyway I don't believe it make clear that it is not skl... because it means
> it started on i9xx, but not that ended on skl... just "plane" was already
> good enough imho... but it is really just a comment...
> don't take this as a nack anyhow... ;)

I think our unwritten rule is that i9xx often refers to
gen2/3/4, sometimes including g4x sometimes not, and sometimes
even vlv/chv gets included.

And sometimes we just call things by the name i915 instead of
i9xx, meaning gmch. But on other occasions i915 means just gen3,
and i9xx means anything else gmch not otherwise named more
specifically.

Got it? Good :)

In this case that "pattern" is a bit more broken since here
i9xx extends all the way to bdw. But it does match the fact that
the primary plane functions are now called i9xx_plane_whatever().

I think I need to include the cursor planes in the i9xx_plane_id
soon enough, which will make it partially extend all the way to cnl
and beyond. We really need to tell the hw people to redesign the
cursor planes so that we have an excuse to split the code :P

> 
> > 
> > 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
> > v6: s/plane/i9xx_plane/ etc. (James)
> > 
> > Cc: James Ausmus <james.ausmus@intel.com>
> > 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 | 98 ++++++++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
> >  drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
> >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> >  5 files changed, 62 insertions(+), 62 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2158a758a17d..55dd602582cb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -304,9 +304,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,
> > @@ -1145,7 +1145,7 @@ struct intel_fbc {
> >  
> >  		struct {
> >  			enum pipe pipe;
> > -			enum plane plane;
> > +			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
> >  	u32 linear_offset;
> >  	u32 dspcntr = plane_state->ctl;
> > -	i915_reg_t reg = DSPCNTR(plane);
> > +	i915_reg_t reg = DSPCNTR(i9xx_plane);
> >  	int x = plane_state->main.x;
> >  	int y = plane_state->main.y;
> >  	unsigned long irqflags;
> > @@ -3248,34 +3248,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(i9xx_plane),
> >  			      ((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(i9xx_plane), 0);
> > +	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> > +		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
> >  			      ((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(i9xx_plane), 0);
> > +		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
> >  	}
> >  
> >  	I915_WRITE_FW(reg, dspcntr);
> >  
> > -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> > +	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> > -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> > +		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
> >  	} else if (INTEL_GEN(dev_priv) >= 4) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> >  			      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(i9xx_plane), (y << 16) | x);
> > +		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
> >  	} else {
> > -		I915_WRITE_FW(DSPADDR(plane),
> > +		I915_WRITE_FW(DSPADDR(i9xx_plane),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> >  	}
> > @@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
> > +	if (INTEL_GEN(dev_priv) >= 4)
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
> >  	else
> > -		I915_WRITE_FW(DSPADDR(plane), 0);
> > -	POSTING_READ_FW(DSPCNTR(plane));
> > +		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> > +	POSTING_READ_FW(DSPCNTR(i9xx_plane));
> >  
> >  	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 i9xx_plane = plane->i9xx_plane;
> > +	enum pipe pipe = plane->pipe;
> >  	bool ret;
> >  
> >  	/*
> > @@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
> >  
> >  	intel_display_power_put(dev_priv, power_domain);
> >  
> > @@ -7406,7 +7405,7 @@ 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);
> >  	u32 val, base, offset;
> > -	int pipe = crtc->pipe, plane = crtc->plane;
> > +	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
> >  	int fourcc, pixel_format;
> >  	unsigned int aligned_height;
> >  	struct drm_framebuffer *fb;
> > @@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
> >  	else
> > -		primary->plane = (enum plane) pipe;
> > +		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> >  	primary->id = PLANE_PRIMARY;
> >  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> >  	primary->check_plane = intel_check_primary_plane;
> > @@ -13303,16 +13302,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;
> >  	}
> >  
> > @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  					       intel_primary_formats, num_formats,
> >  					       modifiers,
> >  					       DRM_PLANE_TYPE_PRIMARY,
> > -					       "plane %c", plane_name(primary->plane));
> > +					       "plane %c",
> > +					       plane_name(primary->i9xx_plane));
> >  	if (ret)
> >  		goto fail;
> >  
> > @@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
> >  	cursor->id = PLANE_CURSOR;
> >  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> >  
> > @@ -13524,14 +13524,14 @@ 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;
> > +	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
> > +	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
> >  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
> >  
> >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> > @@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
> > +	u32 val = I915_READ(DSPCNTR(i9xx_plane));
> >  
> >  	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 99840f3940c7..d1fe7be94b62 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -799,7 +799,7 @@ struct intel_crtc_state {
> >  struct intel_crtc {
> >  	struct drm_crtc base;
> >  	enum pipe pipe;
> > -	enum plane plane;
> > +	enum i9xx_plane_id i9xx_plane;
> >  	/*
> >  	 * Whether the crtc and the connected output pipeline is active. Implies
> >  	 * that crtc->enabled is set, i.e. the current mode configuration has
> > @@ -844,7 +844,7 @@ struct intel_crtc {
> >  
> >  struct intel_plane {
> >  	struct drm_plane base;
> > -	u8 plane;
> > +	enum i9xx_plane_id i9xx_plane;
> >  	enum plane_id id;
> >  	enum pipe pipe;
> >  	bool can_scale;
> > @@ -1130,7 +1130,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];
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 1a0f5e0c8d10..3133131306a9 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
> >  
> >  		/* Set it up... */
> >  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> > -		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> > +		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
> >  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> >  		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
> >  	}
> > @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
> >  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> >  	u32 dpfc_ctl;
> >  
> > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
> >  	if (params->fb.format->cpp[0] == 2)
> >  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> >  	else
> > @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> >  	u32 dpfc_ctl;
> >  	int threshold = dev_priv->fbc.threshold;
> >  
> > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> >  	if (params->fb.format->cpp[0] == 2)
> >  		threshold++;
> >  
> > @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
> >  
> >  	dpfc_ctl = 0;
> >  	if (IS_IVYBRIDGE(dev_priv))
> > -		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> > +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> >  
> >  	if (params->fb.format->cpp[0] == 2)
> >  		threshold++;
> > @@ -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.i9xx_plane = crtc->i9xx_plane;
> >  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
> >  
> >  	params->fb.format = cache->fb.format;
> > @@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
> >  			continue;
> >  
> >  		intel_crtc_state = to_intel_crtc_state(
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 5baa0023964e..dd485f59eb1d 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> >  	}
> >  
> >  	intel_plane->pipe = pipe;
> > -	intel_plane->plane = plane;
> > +	intel_plane->i9xx_plane = plane;
> >  	intel_plane->id = PLANE_SPRITE0 + plane;
> >  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> >  	intel_plane->check_plane = intel_check_sprite_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] 20+ messages in thread

* ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev9)
  2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
                   ` (11 preceding siblings ...)
  2017-11-17 20:08 ` [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Rodrigo Vivi
@ 2017-11-17 20:40 ` Patchwork
  12 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-11-17 20:40 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_plane:
        Subgroup plane-panning-top-left-pipe-c-planes:
                dmesg-fail -> PASS       (shard-hsw)
Test kms_cursor_crc:
        Subgroup cursor-64x64-onscreen:
                pass       -> SKIP       (shard-snb)
Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> SKIP       (shard-snb)

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

shard-hsw        total:2585 pass:1472 dwarn:3   dfail:1   fail:10  skip:1099 time:9406s
shard-snb        total:2585 pass:1257 dwarn:1   dfail:1   fail:13  skip:1313 time:7879s
Blacklisted hosts:
shard-apl        total:2563 pass:1601 dwarn:1   dfail:0   fail:24  skip:936 time:12896s
shard-kbl        total:2565 pass:1701 dwarn:4   dfail:1   fail:26  skip:832 time:10491s

== Logs ==

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

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

* Re: [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-17 20:32     ` Ville Syrjälä
@ 2017-11-17 21:04       ` Rodrigo Vivi
  0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2017-11-17 21:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Fri, Nov 17, 2017 at 08:32:19PM +0000, Ville Syrjälä wrote:
> On Fri, Nov 17, 2017 at 12:12:15PM -0800, Rodrigo Vivi wrote:
> > On Fri, Nov 17, 2017 at 07:19:10PM +0000, 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.
> > 
> > Oh! I should had read this before commenting on the cover letter... sorry...
> > 
> > Anyway I don't believe it make clear that it is not skl... because it means
> > it started on i9xx, but not that ended on skl... just "plane" was already
> > good enough imho... but it is really just a comment...
> > don't take this as a nack anyhow... ;)
> 
> I think our unwritten rule is that i9xx often refers to
> gen2/3/4, sometimes including g4x sometimes not, and sometimes
> even vlv/chv gets included.
> 
> And sometimes we just call things by the name i915 instead of
> i9xx, meaning gmch. But on other occasions i915 means just gen3,
> and i9xx means anything else gmch not otherwise named more
> specifically.
> 
> Got it? Good :)

hahaha I think so! :)

> 
> In this case that "pattern" is a bit more broken since here
> i9xx extends all the way to bdw.

I was wondering something like that...

> But it does match the fact that
> the primary plane functions are now called i9xx_plane_whatever().

oh! I see...

> 
> I think I need to include the cursor planes in the i9xx_plane_id
> soon enough, which will make it partially extend all the way to cnl
> and beyond. We really need to tell the hw people to redesign the
> cursor planes so that we have an excuse to split the code :P

in the hope that it wouldn't get it more fuzzy! :)

Thanks a lot for the explanation!

> 
> > 
> > > 
> > > 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
> > > v6: s/plane/i9xx_plane/ etc. (James)
> > > 
> > > Cc: James Ausmus <james.ausmus@intel.com>
> > > 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 | 98 ++++++++++++++++++------------------
> > >  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
> > >  drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
> > >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> > >  5 files changed, 62 insertions(+), 62 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 2158a758a17d..55dd602582cb 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -304,9 +304,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,
> > > @@ -1145,7 +1145,7 @@ struct intel_fbc {
> > >  
> > >  		struct {
> > >  			enum pipe pipe;
> > > -			enum plane plane;
> > > +			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
> > >  	u32 linear_offset;
> > >  	u32 dspcntr = plane_state->ctl;
> > > -	i915_reg_t reg = DSPCNTR(plane);
> > > +	i915_reg_t reg = DSPCNTR(i9xx_plane);
> > >  	int x = plane_state->main.x;
> > >  	int y = plane_state->main.y;
> > >  	unsigned long irqflags;
> > > @@ -3248,34 +3248,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(i9xx_plane),
> > >  			      ((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(i9xx_plane), 0);
> > > +	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> > > +		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
> > >  			      ((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(i9xx_plane), 0);
> > > +		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
> > >  	}
> > >  
> > >  	I915_WRITE_FW(reg, dspcntr);
> > >  
> > > -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> > > +	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
> > >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > -		I915_WRITE_FW(DSPSURF(plane),
> > > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> > >  			      intel_plane_ggtt_offset(plane_state) +
> > >  			      dspaddr_offset);
> > > -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> > > +		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
> > >  	} else if (INTEL_GEN(dev_priv) >= 4) {
> > > -		I915_WRITE_FW(DSPSURF(plane),
> > > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> > >  			      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(i9xx_plane), (y << 16) | x);
> > > +		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
> > >  	} else {
> > > -		I915_WRITE_FW(DSPADDR(plane),
> > > +		I915_WRITE_FW(DSPADDR(i9xx_plane),
> > >  			      intel_plane_ggtt_offset(plane_state) +
> > >  			      dspaddr_offset);
> > >  	}
> > > @@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
> > > +	if (INTEL_GEN(dev_priv) >= 4)
> > > +		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
> > >  	else
> > > -		I915_WRITE_FW(DSPADDR(plane), 0);
> > > -	POSTING_READ_FW(DSPCNTR(plane));
> > > +		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> > > +	POSTING_READ_FW(DSPCNTR(i9xx_plane));
> > >  
> > >  	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 i9xx_plane = plane->i9xx_plane;
> > > +	enum pipe pipe = plane->pipe;
> > >  	bool ret;
> > >  
> > >  	/*
> > > @@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
> > >  
> > >  	intel_display_power_put(dev_priv, power_domain);
> > >  
> > > @@ -7406,7 +7405,7 @@ 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);
> > >  	u32 val, base, offset;
> > > -	int pipe = crtc->pipe, plane = crtc->plane;
> > > +	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
> > >  	int fourcc, pixel_format;
> > >  	unsigned int aligned_height;
> > >  	struct drm_framebuffer *fb;
> > > @@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
> > >  	else
> > > -		primary->plane = (enum plane) pipe;
> > > +		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> > >  	primary->id = PLANE_PRIMARY;
> > >  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> > >  	primary->check_plane = intel_check_primary_plane;
> > > @@ -13303,16 +13302,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;
> > >  	}
> > >  
> > > @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >  					       intel_primary_formats, num_formats,
> > >  					       modifiers,
> > >  					       DRM_PLANE_TYPE_PRIMARY,
> > > -					       "plane %c", plane_name(primary->plane));
> > > +					       "plane %c",
> > > +					       plane_name(primary->i9xx_plane));
> > >  	if (ret)
> > >  		goto fail;
> > >  
> > > @@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
> > >  	cursor->id = PLANE_CURSOR;
> > >  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> > >  
> > > @@ -13524,14 +13524,14 @@ 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;
> > > +	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
> > > +	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
> > >  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
> > >  
> > >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> > > @@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
> > > +	u32 val = I915_READ(DSPCNTR(i9xx_plane));
> > >  
> > >  	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 99840f3940c7..d1fe7be94b62 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -799,7 +799,7 @@ struct intel_crtc_state {
> > >  struct intel_crtc {
> > >  	struct drm_crtc base;
> > >  	enum pipe pipe;
> > > -	enum plane plane;
> > > +	enum i9xx_plane_id i9xx_plane;
> > >  	/*
> > >  	 * Whether the crtc and the connected output pipeline is active. Implies
> > >  	 * that crtc->enabled is set, i.e. the current mode configuration has
> > > @@ -844,7 +844,7 @@ struct intel_crtc {
> > >  
> > >  struct intel_plane {
> > >  	struct drm_plane base;
> > > -	u8 plane;
> > > +	enum i9xx_plane_id i9xx_plane;
> > >  	enum plane_id id;
> > >  	enum pipe pipe;
> > >  	bool can_scale;
> > > @@ -1130,7 +1130,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];
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 1a0f5e0c8d10..3133131306a9 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
> > >  
> > >  		/* Set it up... */
> > >  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> > > -		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> > > +		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
> > >  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> > >  		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
> > >  	}
> > > @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
> > >  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> > >  	u32 dpfc_ctl;
> > >  
> > > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> > > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
> > >  	if (params->fb.format->cpp[0] == 2)
> > >  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> > >  	else
> > > @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> > >  	u32 dpfc_ctl;
> > >  	int threshold = dev_priv->fbc.threshold;
> > >  
> > > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> > > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> > >  	if (params->fb.format->cpp[0] == 2)
> > >  		threshold++;
> > >  
> > > @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
> > >  
> > >  	dpfc_ctl = 0;
> > >  	if (IS_IVYBRIDGE(dev_priv))
> > > -		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> > > +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> > >  
> > >  	if (params->fb.format->cpp[0] == 2)
> > >  		threshold++;
> > > @@ -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.i9xx_plane = crtc->i9xx_plane;
> > >  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
> > >  
> > >  	params->fb.format = cache->fb.format;
> > > @@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
> > >  			continue;
> > >  
> > >  		intel_crtc_state = to_intel_crtc_state(
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 5baa0023964e..dd485f59eb1d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> > >  	}
> > >  
> > >  	intel_plane->pipe = pipe;
> > > -	intel_plane->plane = plane;
> > > +	intel_plane->i9xx_plane = plane;
> > >  	intel_plane->id = PLANE_SPRITE0 + plane;
> > >  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> > >  	intel_plane->check_plane = intel_check_sprite_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] 20+ messages in thread

* Re: [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/
  2017-11-17 19:19 ` [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
  2017-11-17 20:12   ` Rodrigo Vivi
@ 2017-11-20 17:50   ` James Ausmus
  2017-11-21 19:42     ` Ville Syrjälä
  1 sibling, 1 reply; 20+ messages in thread
From: James Ausmus @ 2017-11-20 17:50 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Fri, Nov 17, 2017 at 09:19:10PM +0200, 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
> v6: s/plane/i9xx_plane/ etc. (James)
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

And here you had me hoping for plane->bikeshed ;)

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

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  6 +--
>  drivers/gpu/drm/i915/intel_display.c | 98 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
>  drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
>  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
>  5 files changed, 62 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2158a758a17d..55dd602582cb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -304,9 +304,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,
> @@ -1145,7 +1145,7 @@ struct intel_fbc {
>  
>  		struct {
>  			enum pipe pipe;
> -			enum plane plane;
> +			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
>  	u32 linear_offset;
>  	u32 dspcntr = plane_state->ctl;
> -	i915_reg_t reg = DSPCNTR(plane);
> +	i915_reg_t reg = DSPCNTR(i9xx_plane);
>  	int x = plane_state->main.x;
>  	int y = plane_state->main.y;
>  	unsigned long irqflags;
> @@ -3248,34 +3248,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(i9xx_plane),
>  			      ((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(i9xx_plane), 0);
> +	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> +		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
>  			      ((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(i9xx_plane), 0);
> +		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
>  	}
>  
>  	I915_WRITE_FW(reg, dspcntr);
>  
> -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> +	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(i9xx_plane),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
> -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> +		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
>  	} else if (INTEL_GEN(dev_priv) >= 4) {
> -		I915_WRITE_FW(DSPSURF(plane),
> +		I915_WRITE_FW(DSPSURF(i9xx_plane),
>  			      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(i9xx_plane), (y << 16) | x);
> +		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
>  	} else {
> -		I915_WRITE_FW(DSPADDR(plane),
> +		I915_WRITE_FW(DSPADDR(i9xx_plane),
>  			      intel_plane_ggtt_offset(plane_state) +
>  			      dspaddr_offset);
>  	}
> @@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
> +	if (INTEL_GEN(dev_priv) >= 4)
> +		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
>  	else
> -		I915_WRITE_FW(DSPADDR(plane), 0);
> -	POSTING_READ_FW(DSPCNTR(plane));
> +		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> +	POSTING_READ_FW(DSPCNTR(i9xx_plane));
>  
>  	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 i9xx_plane = plane->i9xx_plane;
> +	enum pipe pipe = plane->pipe;
>  	bool ret;
>  
>  	/*
> @@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  
> @@ -7406,7 +7405,7 @@ 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);
>  	u32 val, base, offset;
> -	int pipe = crtc->pipe, plane = crtc->plane;
> +	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
>  	int fourcc, pixel_format;
>  	unsigned int aligned_height;
>  	struct drm_framebuffer *fb;
> @@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
>  	else
> -		primary->plane = (enum plane) pipe;
> +		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
>  	primary->id = PLANE_PRIMARY;
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
> @@ -13303,16 +13302,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;
>  	}
>  
> @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  					       intel_primary_formats, num_formats,
>  					       modifiers,
>  					       DRM_PLANE_TYPE_PRIMARY,
> -					       "plane %c", plane_name(primary->plane));
> +					       "plane %c",
> +					       plane_name(primary->i9xx_plane));
>  	if (ret)
>  		goto fail;
>  
> @@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
>  	cursor->id = PLANE_CURSOR;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  
> @@ -13524,14 +13524,14 @@ 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;
> +	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
> +	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
>  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
>  
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> @@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
> +	u32 val = I915_READ(DSPCNTR(i9xx_plane));
>  
>  	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 99840f3940c7..d1fe7be94b62 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -799,7 +799,7 @@ struct intel_crtc_state {
>  struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
> -	enum plane plane;
> +	enum i9xx_plane_id i9xx_plane;
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
>  	 * that crtc->enabled is set, i.e. the current mode configuration has
> @@ -844,7 +844,7 @@ struct intel_crtc {
>  
>  struct intel_plane {
>  	struct drm_plane base;
> -	u8 plane;
> +	enum i9xx_plane_id i9xx_plane;
>  	enum plane_id id;
>  	enum pipe pipe;
>  	bool can_scale;
> @@ -1130,7 +1130,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];
>  }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 1a0f5e0c8d10..3133131306a9 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  		/* Set it up... */
>  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> -		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> +		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
>  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
>  		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
>  	}
> @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
>  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
>  	u32 dpfc_ctl;
>  
> -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
>  	if (params->fb.format->cpp[0] == 2)
>  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
>  	else
> @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  	u32 dpfc_ctl;
>  	int threshold = dev_priv->fbc.threshold;
>  
> -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
>  	if (params->fb.format->cpp[0] == 2)
>  		threshold++;
>  
> @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  
>  	dpfc_ctl = 0;
>  	if (IS_IVYBRIDGE(dev_priv))
> -		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
>  
>  	if (params->fb.format->cpp[0] == 2)
>  		threshold++;
> @@ -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.i9xx_plane = crtc->i9xx_plane;
>  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
>  
>  	params->fb.format = cache->fb.format;
> @@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
>  			continue;
>  
>  		intel_crtc_state = to_intel_crtc_state(
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5baa0023964e..dd485f59eb1d 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  	}
>  
>  	intel_plane->pipe = pipe;
> -	intel_plane->plane = plane;
> +	intel_plane->i9xx_plane = plane;
>  	intel_plane->id = PLANE_SPRITE0 + plane;
>  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>  	intel_plane->check_plane = intel_check_sprite_plane;
> -- 
> 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] 20+ messages in thread

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

On Mon, Nov 20, 2017 at 09:50:28AM -0800, James Ausmus wrote:
> On Fri, Nov 17, 2017 at 09:19:10PM +0200, 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
> > v6: s/plane/i9xx_plane/ etc. (James)
> > 
> > Cc: James Ausmus <james.ausmus@intel.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> And here you had me hoping for plane->bikeshed ;)

Sorry to disappoint :)

Series pushed to dinq. Thanks for the reviews.

> 
> Reviewed-by: James Ausmus <james.ausmus@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  6 +--
> >  drivers/gpu/drm/i915/intel_display.c | 98 ++++++++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  6 +--
> >  drivers/gpu/drm/i915/intel_fbc.c     | 12 ++---
> >  drivers/gpu/drm/i915/intel_sprite.c  |  2 +-
> >  5 files changed, 62 insertions(+), 62 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2158a758a17d..55dd602582cb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -304,9 +304,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,
> > @@ -1145,7 +1145,7 @@ struct intel_fbc {
> >  
> >  		struct {
> >  			enum pipe pipe;
> > -			enum plane plane;
> > +			enum i9xx_plane_id i9xx_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 91f74c5373b3..16ac86816f28 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3220,16 +3220,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 i9xx_plane = plane->i9xx_plane;
> >  	u32 linear_offset;
> >  	u32 dspcntr = plane_state->ctl;
> > -	i915_reg_t reg = DSPCNTR(plane);
> > +	i915_reg_t reg = DSPCNTR(i9xx_plane);
> >  	int x = plane_state->main.x;
> >  	int y = plane_state->main.y;
> >  	unsigned long irqflags;
> > @@ -3248,34 +3248,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(i9xx_plane),
> >  			      ((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(i9xx_plane), 0);
> > +	} else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) {
> > +		I915_WRITE_FW(PRIMSIZE(i9xx_plane),
> >  			      ((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(i9xx_plane), 0);
> > +		I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0);
> >  	}
> >  
> >  	I915_WRITE_FW(reg, dspcntr);
> >  
> > -	I915_WRITE_FW(DSPSTRIDE(plane), fb->pitches[0]);
> > +	I915_WRITE_FW(DSPSTRIDE(i9xx_plane), fb->pitches[0]);
> >  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> > -		I915_WRITE_FW(DSPOFFSET(plane), (y << 16) | x);
> > +		I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x);
> >  	} else if (INTEL_GEN(dev_priv) >= 4) {
> > -		I915_WRITE_FW(DSPSURF(plane),
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane),
> >  			      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(i9xx_plane), (y << 16) | x);
> > +		I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset);
> >  	} else {
> > -		I915_WRITE_FW(DSPADDR(plane),
> > +		I915_WRITE_FW(DSPADDR(i9xx_plane),
> >  			      intel_plane_ggtt_offset(plane_state) +
> >  			      dspaddr_offset);
> >  	}
> > @@ -3284,32 +3284,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 i9xx_plane = plane->i9xx_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(i9xx_plane), 0);
> > +	if (INTEL_GEN(dev_priv) >= 4)
> > +		I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
> >  	else
> > -		I915_WRITE_FW(DSPADDR(plane), 0);
> > -	POSTING_READ_FW(DSPCNTR(plane));
> > +		I915_WRITE_FW(DSPADDR(i9xx_plane), 0);
> > +	POSTING_READ_FW(DSPCNTR(i9xx_plane));
> >  
> >  	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 i9xx_plane = plane->i9xx_plane;
> > +	enum pipe pipe = plane->pipe;
> >  	bool ret;
> >  
> >  	/*
> > @@ -3321,7 +3320,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(i9xx_plane)) & DISPLAY_PLANE_ENABLE;
> >  
> >  	intel_display_power_put(dev_priv, power_domain);
> >  
> > @@ -7406,7 +7405,7 @@ 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);
> >  	u32 val, base, offset;
> > -	int pipe = crtc->pipe, plane = crtc->plane;
> > +	int pipe = crtc->pipe, plane = crtc->i9xx_plane;
> >  	int fourcc, pixel_format;
> >  	unsigned int aligned_height;
> >  	struct drm_framebuffer *fb;
> > @@ -13272,9 +13271,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->i9xx_plane = (enum i9xx_plane_id) !pipe;
> >  	else
> > -		primary->plane = (enum plane) pipe;
> > +		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> >  	primary->id = PLANE_PRIMARY;
> >  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> >  	primary->check_plane = intel_check_primary_plane;
> > @@ -13303,16 +13302,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;
> >  	}
> >  
> > @@ -13336,7 +13335,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  					       intel_primary_formats, num_formats,
> >  					       modifiers,
> >  					       DRM_PLANE_TYPE_PRIMARY,
> > -					       "plane %c", plane_name(primary->plane));
> > +					       "plane %c",
> > +					       plane_name(primary->i9xx_plane));
> >  	if (ret)
> >  		goto fail;
> >  
> > @@ -13396,7 +13396,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->i9xx_plane = (enum i9xx_plane_id) pipe;
> >  	cursor->id = PLANE_CURSOR;
> >  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> >  
> > @@ -13524,14 +13524,14 @@ 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;
> > +	intel_crtc->i9xx_plane = primary->i9xx_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[intel_crtc->i9xx_plane] != NULL);
> > +	dev_priv->plane_to_crtc_mapping[intel_crtc->i9xx_plane] = intel_crtc;
> >  	dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc;
> >  
> >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> > @@ -14788,11 +14788,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 i9xx_plane = plane->i9xx_plane;
> > +	u32 val = I915_READ(DSPCNTR(i9xx_plane));
> >  
> >  	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 99840f3940c7..d1fe7be94b62 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -799,7 +799,7 @@ struct intel_crtc_state {
> >  struct intel_crtc {
> >  	struct drm_crtc base;
> >  	enum pipe pipe;
> > -	enum plane plane;
> > +	enum i9xx_plane_id i9xx_plane;
> >  	/*
> >  	 * Whether the crtc and the connected output pipeline is active. Implies
> >  	 * that crtc->enabled is set, i.e. the current mode configuration has
> > @@ -844,7 +844,7 @@ struct intel_crtc {
> >  
> >  struct intel_plane {
> >  	struct drm_plane base;
> > -	u8 plane;
> > +	enum i9xx_plane_id i9xx_plane;
> >  	enum plane_id id;
> >  	enum pipe pipe;
> >  	bool can_scale;
> > @@ -1130,7 +1130,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];
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 1a0f5e0c8d10..3133131306a9 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -151,7 +151,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
> >  
> >  		/* Set it up... */
> >  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> > -		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.plane);
> > +		fbc_ctl2 |= FBC_CTL_PLANE(params->crtc.i9xx_plane);
> >  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> >  		I915_WRITE(FBC_FENCE_OFF, params->crtc.fence_y_offset);
> >  	}
> > @@ -177,7 +177,7 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
> >  	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
> >  	u32 dpfc_ctl;
> >  
> > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
> > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane) | DPFC_SR_EN;
> >  	if (params->fb.format->cpp[0] == 2)
> >  		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> >  	else
> > @@ -224,7 +224,7 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
> >  	u32 dpfc_ctl;
> >  	int threshold = dev_priv->fbc.threshold;
> >  
> > -	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
> > +	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> >  	if (params->fb.format->cpp[0] == 2)
> >  		threshold++;
> >  
> > @@ -306,7 +306,7 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
> >  
> >  	dpfc_ctl = 0;
> >  	if (IS_IVYBRIDGE(dev_priv))
> > -		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
> > +		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.i9xx_plane);
> >  
> >  	if (params->fb.format->cpp[0] == 2)
> >  		threshold++;
> > @@ -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.i9xx_plane = crtc->i9xx_plane;
> >  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(fbc);
> >  
> >  	params->fb.format = cache->fb.format;
> > @@ -1088,7 +1088,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) && crtc->i9xx_plane != PLANE_A)
> >  			continue;
> >  
> >  		intel_crtc_state = to_intel_crtc_state(
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 5baa0023964e..dd485f59eb1d 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1384,7 +1384,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> >  	}
> >  
> >  	intel_plane->pipe = pipe;
> > -	intel_plane->plane = plane;
> > +	intel_plane->i9xx_plane = plane;
> >  	intel_plane->id = PLANE_SPRITE0 + plane;
> >  	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> >  	intel_plane->check_plane = intel_check_sprite_plane;
> > -- 
> > 2.13.6
> > 

-- 
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] 20+ messages in thread

* Re: [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout
  2017-11-17 19:19 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
@ 2017-12-01 18:05   ` Alex Villacis Lasso
  0 siblings, 0 replies; 20+ messages in thread
From: Alex Villacis Lasso @ 2017-12-01 18:05 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

El 17/11/17 a las 14:19, Ville Syrjala escribió:
> 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
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Tested-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 114 ++++++++++++++++++++---------------
>   1 file changed, 65 insertions(+), 49 deletions(-)

Where can I check whether the patches are progressing into mainline kernel? Last time I checked, 4.15.0-rc1 still did not have the patches.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-12-02 22:41 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 19:19 [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Ville Syrjala
2017-11-17 19:19 ` [PATCH v4 01/10] drm/i915: Add .get_hw_state() method for planes Ville Syrjala
2017-11-17 19:19 ` [PATCH v3 02/10] drm/i915: Redo plane sanitation during readout Ville Syrjala
2017-12-01 18:05   ` Alex Villacis Lasso
2017-11-17 19:19 ` [PATCH v6 03/10] drm/i915: s/enum plane/enum i9xx_plane_id/ Ville Syrjala
2017-11-17 20:12   ` Rodrigo Vivi
2017-11-17 20:32     ` Ville Syrjälä
2017-11-17 21:04       ` Rodrigo Vivi
2017-11-20 17:50   ` James Ausmus
2017-11-21 19:42     ` Ville Syrjälä
2017-11-17 19:19 ` [PATCH v3 04/10] drm/i915: Use enum i9xx_plane_id for the .get_fifo_size() hooks Ville Syrjala
2017-11-17 19:19 ` [PATCH v5 05/10] drm/i915: Cleanup enum pipe/enum plane_id/enum i9xx_plane_id in initial fb readout Ville Syrjala
2017-11-17 19:19 ` [PATCH v2 06/10] drm/i915: Nuke ironlake_get_initial_plane_config() Ville Syrjala
2017-11-17 19:19 ` [PATCH v2 07/10] drm/i915: Switch fbc over to for_each_new_intel_plane_in_state() Ville Syrjala
2017-11-17 19:19 ` [PATCH v3 08/10] drm/i915: Nuke crtc->plane Ville Syrjala
2017-11-17 19:19 ` [PATCH v3 09/10] drm/i915: Use plane->get_hw_state() for initial plane fb readout Ville Syrjala
2017-11-17 19:19 ` [PATCH v3 10/10] drm/i915: Add rudimentary plane state verification Ville Syrjala
2017-11-17 19:54 ` ✓ Fi.CI.BAT: success for drm/i915: Plane assert/readout cleanups etc. (rev9) Patchwork
2017-11-17 20:08 ` [PATCH v3 00/10] drm/i915: Plane assert/readout cleanups etc Rodrigo Vivi
2017-11-17 20:40 ` ✗ Fi.CI.IGT: warning for drm/i915: Plane assert/readout cleanups etc. (rev9) 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.