[PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[ville.syrjala]

Back on the watermark horse. This series moves ILK/SNB/IVB over to the HSW
watermark code.

This series still has some underrun issues since the safe watermark
update mechanism is yet to be introduced. However it shouldn't really
be any worse than what we have ATM, and I'd like to have all the platforms
converted so that we have more systems that will excercise the new update
mechanism when it arrives.

Ville Syrjälä (14):
      drm/i915: Add IVB DDB partitioning control
      drm/i915: Add ILK/SNB/IVB WM latency field support
      drm/i915: Avoid computing invalid WM levels when sprites/scaling is enabled
      Revert "drm/i915/sprite: Always enable the scaler on IronLake"
      drm/i915: Fix LP1 sprite watermarks for ILK/SNB
      drm/i915: Fix LP1+ watermark disabling ILK
      drm/i915: Don't merge LP1+ watermarks on ILK/SNB/IVB when multiple pipes are enabled
      drm/i915: Disable FBC WM on ILK, and disable LP2+ when FBC is enabled
      drm/i915: Linetime watermarks are a HSW feature
      drm/i915: Disable LP1+ watermarks safely in init
      drm/i915: Move ILK/SNB/IVB over to the HSW WM code
      drm/i915: Try to fix the messy IVB sprite scaling workaround
      drm/i915: Don't disable primary when color keying is used
      drm/i915: Avoid underruns when disabling sprites

 drivers/gpu/drm/i915/i915_drv.h      |   1 -
 drivers/gpu/drm/i915/i915_reg.h      |   2 +
 drivers/gpu/drm/i915/intel_display.c |   2 +-
 drivers/gpu/drm/i915/intel_pm.c      | 857 +++++++----------------------------
 drivers/gpu/drm/i915/intel_sprite.c  |  50 +-
 5 files changed, 195 insertions(+), 717 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/14] drm/i915: Add IVB DDB partitioning control

[ville.syrjala]

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

On IVB the display data buffer partitioning control lives in the
DISP_ARB_CTL2 register. Add the relevant defines/code for it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c | 30 ++++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2d20390..53a6470 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4145,6 +4145,8 @@
 #define DISP_ARB_CTL	0x45000
 #define  DISP_TILE_SURFACE_SWIZZLING	(1<<13)
 #define  DISP_FBC_WM_DIS		(1<<15)
+#define DISP_ARB_CTL2	0x45004
+#define  DISP_DATA_PARTITION_5_6	(1<<6)
 #define GEN7_MSG_CTL	0x45010
 #define  WAIT_FOR_PCH_RESET_ACK		(1<<1)
 #define  WAIT_FOR_PCH_FLR_ACK		(1<<0)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a23eb27..940f159 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2865,6 +2865,7 @@ static unsigned int ilk_compute_wm_dirty(struct drm_device *dev,
 static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 				struct hsw_wm_values *results)
 {
+	struct drm_device *dev = dev_priv->dev;
 	struct hsw_wm_values *previous = &dev_priv->wm.hw;
 	unsigned int dirty;
 	uint32_t val;
@@ -2895,12 +2896,21 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 		I915_WRITE(PIPE_WM_LINETIME(PIPE_C), results->wm_linetime[2]);
 
 	if (dirty & WM_DIRTY_DDB) {
-		val = I915_READ(WM_MISC);
-		if (results->partitioning == INTEL_DDB_PART_1_2)
-			val &= ~WM_MISC_DATA_PARTITION_5_6;
-		else
-			val |= WM_MISC_DATA_PARTITION_5_6;
-		I915_WRITE(WM_MISC, val);
+		if (IS_HASWELL(dev)) {
+			val = I915_READ(WM_MISC);
+			if (results->partitioning == INTEL_DDB_PART_1_2)
+				val &= ~WM_MISC_DATA_PARTITION_5_6;
+			else
+				val |= WM_MISC_DATA_PARTITION_5_6;
+			I915_WRITE(WM_MISC, val);
+		} else {
+			val = I915_READ(DISP_ARB_CTL2);
+			if (results->partitioning == INTEL_DDB_PART_1_2)
+				val &= ~DISP_DATA_PARTITION_5_6;
+			else
+				val |= DISP_DATA_PARTITION_5_6;
+			I915_WRITE(DISP_ARB_CTL2, val);
+		}
 	}
 
 	if (dirty & WM_DIRTY_FBC) {
@@ -3211,8 +3221,12 @@ void ilk_wm_get_hw_state(struct drm_device *dev)
 	hw->wm_lp_spr[1] = I915_READ(WM2S_LP_IVB);
 	hw->wm_lp_spr[2] = I915_READ(WM3S_LP_IVB);
 
-	hw->partitioning = (I915_READ(WM_MISC) & WM_MISC_DATA_PARTITION_5_6) ?
-		INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
+	if (IS_HASWELL(dev))
+		hw->partitioning = (I915_READ(WM_MISC) & WM_MISC_DATA_PARTITION_5_6) ?
+			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
+	else if (IS_IVYBRIDGE(dev))
+		hw->partitioning = (I915_READ(DISP_ARB_CTL2) & DISP_DATA_PARTITION_5_6) ?
+			INTEL_DDB_PART_5_6 : INTEL_DDB_PART_1_2;
 
 	hw->enable_fbc_wm =
 		!(I915_READ(DISP_ARB_CTL) & DISP_FBC_WM_DIS);
-- 
1.8.3.2

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

[PATCH 02/14] drm/i915: Add ILK/SNB/IVB WM latency field support

[ville.syrjala]

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

Add a new function  ilk_wm_lp_latency() which will tell us what to write
into the WM_LPx register latency field. HSW is different from erlier
gens in this regard.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 940f159..f2cd539 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2716,6 +2716,17 @@ static int ilk_wm_lp_to_level(int wm_lp, const struct intel_pipe_wm *pipe_wm)
 	return wm_lp + (wm_lp >= 2 && pipe_wm->wm[4].enable);
 }
 
+/* The value we need to program into the WM_LPx latency field */
+static unsigned int ilk_wm_lp_latency(struct drm_device *dev, int level)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (IS_HASWELL(dev))
+		return 2 * level;
+	else
+		return dev_priv->wm.pri_latency[level];
+}
+
 static void hsw_compute_wm_results(struct drm_device *dev,
 				   const struct intel_pipe_wm *merged,
 				   enum intel_ddb_partitioning partitioning,
@@ -2738,7 +2749,7 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 			break;
 
 		results->wm_lp[wm_lp - 1] = WM3_LP_EN |
-			((level * 2) << WM1_LP_LATENCY_SHIFT) |
+			(ilk_wm_lp_latency(dev, level) << WM1_LP_LATENCY_SHIFT) |
 			(r->pri_val << WM1_LP_SR_SHIFT) |
 			r->cur_val;
 
-- 
1.8.3.2

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

[PATCH 03/14] drm/i915: Avoid computing invalid WM levels when sprites/scaling is enabled

[ville.syrjala]

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

On ILK/SNB only LP0/1 watermarks can be enabled when sprites are
enabled, and on ILK/SNB/IVB sprite scaling is limited to LP0 only.

So we can avoid computing the extra levels we're never going to use.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f2cd539..4b3ccac 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2644,6 +2644,14 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
 	/* LP0 watermarks always use 1/2 DDB partitioning */
 	ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
 
+	/* ILK/SNB: LP2+ watermarks only w/o sprites */
+	if (INTEL_INFO(dev)->gen <= 6 && params->spr.enabled)
+		max_level = 1;
+
+	/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
+	if (params->spr.scaled)
+		max_level = 0;
+
 	for (level = 0; level <= max_level; level++)
 		ilk_compute_wm_level(dev_priv, level, params,
 				     &pipe_wm->wm[level]);
-- 
1.8.3.2

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

[PATCH 04/14] Revert "drm/i915/sprite: Always enable the scaler on IronLake"

[ville.syrjala]

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

Apparently always enabling the sprite scaler magically made
sprites work on ILK in the past.

I think the real reason for the failure was missing sprite
watermark programming, and enabling the scaler effectively
disabled LP1+ watermarks, which was enough to keep things going.
Or it might be that the hardware more or less ignores watermarks
for scaled sprites since things seem to work even if I leave
sprite watermarks at 0 and disable all other planes except the
sprite.

In any case, we left the scaler always on but then failed to
check whether we might be exceeding the scaler's source size
limits. That caused the sprite to fail when a sufficiently
large unscaled image was being displayed.

Now that we're getting proper watermark programming for ILK, we
can keep the scaler disabled unless we need to do actual scaling.

This reverts commit 8aaa81a166d80ac9bf2813984e5b4c2503d0fe08.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 90a3f6d..6bfebfb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -488,7 +488,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	crtc_h--;
 
 	dvsscale = 0;
-	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
+	if (crtc_w != src_w || crtc_h != src_h)
 		dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
 
 	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
-- 
1.8.3.2

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

Re: [PATCH 04/14] Revert "drm/i915/sprite: Always enable the scaler on IronLake"

[Imre Deak]

On Thu, 2013-12-05 at 15:51 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Apparently always enabling the sprite scaler magically made
> sprites work on ILK in the past.
> 
> I think the real reason for the failure was missing sprite
> watermark programming, and enabling the scaler effectively
> disabled LP1+ watermarks, which was enough to keep things going.
> Or it might be that the hardware more or less ignores watermarks
> for scaled sprites since things seem to work even if I leave
> sprite watermarks at 0 and disable all other planes except the
> sprite.
> 
> In any case, we left the scaler always on but then failed to
> check whether we might be exceeding the scaler's source size
> limits. That caused the sprite to fail when a sufficiently
> large unscaled image was being displayed.
> 
> Now that we're getting proper watermark programming for ILK, we
> can keep the scaler disabled unless we need to do actual scaling.
> 
> This reverts commit 8aaa81a166d80ac9bf2813984e5b4c2503d0fe08.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since we switch to the new calculation only at patch 11/14, it'd be
better to apply this after that, so we don't re-introduce the issue in
between.

--Imre

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 90a3f6d..6bfebfb 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -488,7 +488,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	crtc_h--;
>  
>  	dvsscale = 0;
> -	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
> +	if (crtc_w != src_w || crtc_h != src_h)
>  		dvsscale = DVS_SCALE_ENABLE | (src_w << 16) | src_h;
>  
>  	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);


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

[PATCH 05/14] drm/i915: Fix LP1 sprite watermarks for ILK/SNB

[ville.syrjala]

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

ILK/SNB don't have LP2+ watermarks for sprites. Also the LP1 sprite
watermark register has its own enable bit. Take these differences
into account when programming the LP1+ registers.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4b3ccac..db7f3a6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2768,7 +2768,11 @@ static void hsw_compute_wm_results(struct drm_device *dev,
 			results->wm_lp[wm_lp - 1] |=
 				r->fbc_val << WM1_LP_FBC_SHIFT;
 
-		results->wm_lp_spr[wm_lp - 1] = r->spr_val;
+		if (INTEL_INFO(dev)->gen <= 6 && r->spr_val) {
+			WARN_ON(wm_lp != 1);
+			results->wm_lp_spr[wm_lp - 1] = WM1S_LP_EN | r->spr_val;
+		} else
+			results->wm_lp_spr[wm_lp - 1] = r->spr_val;
 	}
 
 	/* LP0 register values */
@@ -2900,6 +2904,10 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != 0)
 		I915_WRITE(WM1_LP_ILK, 0);
 
+	if (INTEL_INFO(dev)->gen <= 6 &&
+	    dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != 0)
+		I915_WRITE(WM1S_LP_ILK, 0);
+
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
 		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
@@ -2941,12 +2949,17 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 		I915_WRITE(DISP_ARB_CTL, val);
 	}
 
-	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])
-		I915_WRITE(WM1S_LP_ILK, results->wm_lp_spr[0]);
-	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp_spr[1] != results->wm_lp_spr[1])
-		I915_WRITE(WM2S_LP_IVB, results->wm_lp_spr[1]);
-	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp_spr[2] != results->wm_lp_spr[2])
-		I915_WRITE(WM3S_LP_IVB, results->wm_lp_spr[2]);
+	if (INTEL_INFO(dev)->gen <= 6) {
+		if (dirty & WM_DIRTY_LP(1) && results->wm_lp_spr[0] != 0)
+			I915_WRITE(WM1S_LP_ILK, results->wm_lp_spr[0]);
+	} else {
+		if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])
+			I915_WRITE(WM1S_LP_ILK, results->wm_lp_spr[0]);
+		if (dirty & WM_DIRTY_LP(2) && previous->wm_lp_spr[1] != results->wm_lp_spr[1])
+			I915_WRITE(WM2S_LP_IVB, results->wm_lp_spr[1]);
+		if (dirty & WM_DIRTY_LP(3) && previous->wm_lp_spr[2] != results->wm_lp_spr[2])
+			I915_WRITE(WM3S_LP_IVB, results->wm_lp_spr[2]);
+	}
 
 	if (dirty & WM_DIRTY_LP(1) && results->wm_lp[0] != 0)
 		I915_WRITE(WM1_LP_ILK, results->wm_lp[0]);
-- 
1.8.3.2

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

[PATCH 06/14] drm/i915: Fix LP1+ watermark disabling ILK

[ville.syrjala]

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

On ILK disabling LP1+ watermarks must be done carefully to avoid
underruns. If we just write 0 to the register in the middle of the scan
cycle we often get an underrun. So instead we have to leave the actual
watermark levels in the register intact, and just toggle the enable bit.

Presumably the hardware takes a while to get out of low power mode, and
so the watermark level need to stay valid until that time.

We also have to be careful with the WM1S_LP_EN bit. It seems the
hardware more or less treats it like the actual watermarks numbers, and
so we must not toggle it too soon. Just leave it alone when disabling
the LP1+ watermarks.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index db7f3a6..1aa6ea9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2897,16 +2897,23 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 	if (!dirty)
 		return;
 
-	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != 0)
-		I915_WRITE(WM3_LP_ILK, 0);
-	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != 0)
-		I915_WRITE(WM2_LP_ILK, 0);
-	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != 0)
-		I915_WRITE(WM1_LP_ILK, 0);
+	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {
+		previous->wm_lp[2] &= ~WM1_LP_SR_EN;
+		I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);
+	}
+	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {
+		previous->wm_lp[1] &= ~WM1_LP_SR_EN;
+		I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);
+	}
+	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {
+		previous->wm_lp[0] &= ~WM1_LP_SR_EN;
+		I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);
+	}
 
-	if (INTEL_INFO(dev)->gen <= 6 &&
-	    dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != 0)
-		I915_WRITE(WM1S_LP_ILK, 0);
+	/*
+	 * Don't touch WM1S_LP_EN here.
+	 * Doing so could cause underruns.
+	 */
 
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
 		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
@@ -2950,7 +2957,7 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 	}
 
 	if (INTEL_INFO(dev)->gen <= 6) {
-		if (dirty & WM_DIRTY_LP(1) && results->wm_lp_spr[0] != 0)
+		if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])
 			I915_WRITE(WM1S_LP_ILK, results->wm_lp_spr[0]);
 	} else {
 		if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])
@@ -2961,11 +2968,11 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 			I915_WRITE(WM3S_LP_IVB, results->wm_lp_spr[2]);
 	}
 
-	if (dirty & WM_DIRTY_LP(1) && results->wm_lp[0] != 0)
+	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != results->wm_lp[0])
 		I915_WRITE(WM1_LP_ILK, results->wm_lp[0]);
-	if (dirty & WM_DIRTY_LP(2) && results->wm_lp[1] != 0)
+	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != results->wm_lp[1])
 		I915_WRITE(WM2_LP_ILK, results->wm_lp[1]);
-	if (dirty & WM_DIRTY_LP(3) && results->wm_lp[2] != 0)
+	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != results->wm_lp[2])
 		I915_WRITE(WM3_LP_ILK, results->wm_lp[2]);
 
 	dev_priv->wm.hw = *results;
-- 
1.8.3.2

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

Re: [PATCH 06/14] drm/i915: Fix LP1+ watermark disabling ILK

[Imre Deak]

On Thu, 2013-12-05 at 15:51 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On ILK disabling LP1+ watermarks must be done carefully to avoid
> underruns. If we just write 0 to the register in the middle of the scan
> cycle we often get an underrun. So instead we have to leave the actual
> watermark levels in the register intact, and just toggle the enable bit.
> 
> Presumably the hardware takes a while to get out of low power mode, and
> so the watermark level need to stay valid until that time.
> 
> We also have to be careful with the WM1S_LP_EN bit. It seems the
> hardware more or less treats it like the actual watermarks numbers, and
> so we must not toggle it too soon. Just leave it alone when disabling
> the LP1+ watermarks.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index db7f3a6..1aa6ea9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2897,16 +2897,23 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
>  	if (!dirty)
>  		return;
>  
> -	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != 0)
> -		I915_WRITE(WM3_LP_ILK, 0);
> -	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != 0)
> -		I915_WRITE(WM2_LP_ILK, 0);
> -	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != 0)
> -		I915_WRITE(WM1_LP_ILK, 0);
> +	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {
> +		previous->wm_lp[2] &= ~WM1_LP_SR_EN;
> +		I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);
> +	}
> +	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {
> +		previous->wm_lp[1] &= ~WM1_LP_SR_EN;
> +		I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);
> +	}
> +	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {
> +		previous->wm_lp[0] &= ~WM1_LP_SR_EN;
> +		I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);
> +	}
>  
> -	if (INTEL_INFO(dev)->gen <= 6 &&
> -	    dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != 0)
> -		I915_WRITE(WM1S_LP_ILK, 0);
> +	/*
> +	 * Don't touch WM1S_LP_EN here.
> +	 * Doing so could cause underruns.
> +	 */
>  
>  	if (dirty & WM_DIRTY_PIPE(PIPE_A))
>  		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
> @@ -2950,7 +2957,7 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (INTEL_INFO(dev)->gen <= 6) {
> -		if (dirty & WM_DIRTY_LP(1) && results->wm_lp_spr[0] != 0)
> +		if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])
>  			I915_WRITE(WM1S_LP_ILK, results->wm_lp_spr[0]);

This and the below "else" version are the same now, so it could be moved
before the if.

--Imre

>  	} else {
>  		if (dirty & WM_DIRTY_LP(1) && previous->wm_lp_spr[0] != results->wm_lp_spr[0])



> @@ -2961,11 +2968,11 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
>  			I915_WRITE(WM3S_LP_IVB, results->wm_lp_spr[2]);
>  	}
>  
> -	if (dirty & WM_DIRTY_LP(1) && results->wm_lp[0] != 0)
> +	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] != results->wm_lp[0])
>  		I915_WRITE(WM1_LP_ILK, results->wm_lp[0]);
> -	if (dirty & WM_DIRTY_LP(2) && results->wm_lp[1] != 0)
> +	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] != results->wm_lp[1])
>  		I915_WRITE(WM2_LP_ILK, results->wm_lp[1]);
> -	if (dirty & WM_DIRTY_LP(3) && results->wm_lp[2] != 0)
> +	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] != results->wm_lp[2])
>  		I915_WRITE(WM3_LP_ILK, results->wm_lp[2]);
>  
>  	dev_priv->wm.hw = *results;


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

[PATCH 07/14] drm/i915: Don't merge LP1+ watermarks on ILK/SNB/IVB when multiple pipes are enabled

[ville.syrjala]

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

Multi-pipe LP1+ watermarks are a HSW+ feature, so let's not do it on
earlier generations.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1aa6ea9..29c333d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2691,11 +2691,17 @@ static void ilk_merge_wm_level(struct drm_device *dev,
  * Merge all low power watermarks for all active pipes.
  */
 static void ilk_wm_merge(struct drm_device *dev,
+			 const struct intel_wm_config *config,
 			 const struct hsw_wm_maximums *max,
 			 struct intel_pipe_wm *merged)
 {
 	int level, max_level = ilk_wm_max_level(dev);
 
+	/* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */
+	if ((INTEL_INFO(dev)->gen <= 6 || IS_IVYBRIDGE(dev)) &&
+	    config->num_pipes_active > 1)
+		return;
+
 	merged->fbc_wm_enabled = true;
 
 	/* merge each WM1+ level */
@@ -3001,13 +3007,13 @@ static void haswell_update_wm(struct drm_crtc *crtc)
 	intel_crtc->wm.active = pipe_wm;
 
 	ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max);
-	ilk_wm_merge(dev, &max, &lp_wm_1_2);
+	ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
 
 	/* 5/6 split only in single pipe config on IVB+ */
 	if (INTEL_INFO(dev)->gen >= 7 &&
 	    config.num_pipes_active == 1 && config.sprites_enabled) {
 		ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, &max);
-		ilk_wm_merge(dev, &max, &lp_wm_5_6);
+		ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
 
 		best_lp_wm = hsw_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6);
 	} else {
-- 
1.8.3.2

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

[PATCH 08/14] drm/i915: Disable FBC WM on ILK, and disable LP2+ when FBC is enabled

[ville.syrjala]

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

ILK has a bunch of issues with FBC. First of all, BSpec tells us that
FBC WM should never be enabled. Secondly when FBC is enabled
with FBC WM disabled, LP2+ watermarks must be disabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 29c333d..023be16 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2702,7 +2702,8 @@ static void ilk_wm_merge(struct drm_device *dev,
 	    config->num_pipes_active > 1)
 		return;
 
-	merged->fbc_wm_enabled = true;
+	/* ILK: FBC WM must be disabled always */
+	merged->fbc_wm_enabled = INTEL_INFO(dev)->gen >= 6;
 
 	/* merge each WM1+ level */
 	for (level = 1; level <= max_level; level++) {
@@ -2722,6 +2723,20 @@ static void ilk_wm_merge(struct drm_device *dev,
 			wm->fbc_val = 0;
 		}
 	}
+
+	/* ILK: LP2+ must be disabled when FBC WM is disabled but FBC enabled */
+	/*
+	 * FIXME this is racy. FBC might get enabled later.
+	 * What we should check here is whether FBC can be
+	 * enabled sometime later.
+	 */
+	if (IS_GEN5(dev) && !merged->fbc_wm_enabled && intel_fbc_enabled(dev)) {
+		for (level = 2; level <= max_level; level++) {
+			struct intel_wm_level *wm = &merged->wm[level];
+
+			wm->enable = false;
+		}
+	}
 }
 
 static int ilk_wm_lp_to_level(int wm_lp, const struct intel_pipe_wm *pipe_wm)
-- 
1.8.3.2

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

[PATCH 09/14] drm/i915: Linetime watermarks are a HSW feature

[ville.syrjala]

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

Linetime watermarks don't exist on ILK/SNB/IVB, so don't compute them
except on HSW.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 023be16..954c1fb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2656,7 +2656,8 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
 		ilk_compute_wm_level(dev_priv, level, params,
 				     &pipe_wm->wm[level]);
 
-	pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
+	if (IS_HASWELL(dev))
+		pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
 
 	/* At least LP0 must be valid */
 	return ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]);
@@ -3235,7 +3236,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 	};
 
 	hw->wm_pipe[pipe] = I915_READ(wm0_pipe_reg[pipe]);
-	hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
+	if (IS_HASWELL(dev))
+		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
 
 	if (intel_crtc_active(crtc)) {
 		u32 tmp = hw->wm_pipe[pipe];
-- 
1.8.3.2

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

[PATCH 10/14] drm/i915: Disable LP1+ watermarks safely in init

[ville.syrjala]

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

ILK doesn't like if we just write the LP1+ watermarks registers with 0.
We need to just disable the watermarks by clearing the enable bit. Use
that method also when disabling LP1+ watermarks in init_clock_gating.

It looks like disabling the sprite LP1 watermarks can cause underruns
even if we just toggle the WM1S_LP_EN bit. So treat that bit like the
actual watermark numbers and avoid setting it to 0 immediately.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 954c1fb..420be2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5048,6 +5048,20 @@ static void g4x_disable_trickle_feed(struct drm_device *dev)
 	}
 }
 
+static void ilk_init_lp_watermarks(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+	I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+	I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+	/*
+	 * Don't touch WM1S_LP_EN here.
+	 * Doing so could cause underruns.
+	 */
+}
+
 static void ironlake_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5081,9 +5095,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(DISP_ARB_CTL,
 		   (I915_READ(DISP_ARB_CTL) |
 		    DISP_FBC_WM_DIS));
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
+
+	ilk_init_lp_watermarks(dev);
 
 	/*
 	 * Based on the document from hardware guys the following bits
@@ -5190,9 +5203,7 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		I915_WRITE(GEN6_GT_MODE,
 			   _MASKED_BIT_ENABLE(GEN6_TD_FOUR_ROW_DISPATCH_DISABLE));
 
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
+	ilk_init_lp_watermarks(dev);
 
 	I915_WRITE(CACHE_MODE_0,
 		   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
@@ -5352,9 +5363,7 @@ static void haswell_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
+	ilk_init_lp_watermarks(dev);
 
 	/* According to the spec, bit 13 (RCZUNIT) must be set on IVB.
 	 * This implements the WaDisableRCZUnitClockGating:hsw workaround.
@@ -5403,9 +5412,7 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t snpcr;
 
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
+	ilk_init_lp_watermarks(dev);
 
 	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
 
-- 
1.8.3.2

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

Re: [PATCH 10/14] drm/i915: Disable LP1+ watermarks safely in init

[Chris Wilson]

On Thu, Dec 05, 2013 at 03:51:37PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ILK doesn't like if we just write the LP1+ watermarks registers with 0.
> We need to just disable the watermarks by clearing the enable bit. Use
> that method also when disabling LP1+ watermarks in init_clock_gating.

Mind clarifying what error is reported under the current method? I
presume this patch removes another cause of underruns.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 10/14] drm/i915: Disable LP1+ watermarks safely in init

[Ville Syrjälä]

On Thu, Dec 05, 2013 at 02:27:14PM +0000, Chris Wilson wrote:
> On Thu, Dec 05, 2013 at 03:51:37PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > ILK doesn't like if we just write the LP1+ watermarks registers with 0.
> > We need to just disable the watermarks by clearing the enable bit. Use
> > that method also when disabling LP1+ watermarks in init_clock_gating.
> 
> Mind clarifying what error is reported under the current method? I
> presume this patch removes another cause of underruns.

Yeah you may get an underrun and a nice glitch on the screen to go with
it.

-- 
Ville Syrjälä
Intel OTC

[PATCH 11/14] drm/i915: Move ILK/SNB/IVB over to the HSW WM code

[ville.syrjala]

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

The new HSW watermark code can now handle ILK/SNB/IVB as well, so
switch them over. Kill the old code.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3c59b67..24066cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11208,7 +11208,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 		pll->on = false;
 	}
 
-	if (IS_HASWELL(dev))
+	if (HAS_PCH_SPLIT(dev))
 		ilk_wm_get_hw_state(dev);
 
 	if (force_restore) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 420be2f..8b97893 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -965,65 +965,6 @@ static const struct intel_watermark_params i830_wm_info = {
 	I830_FIFO_LINE_SIZE
 };
 
-static const struct intel_watermark_params ironlake_display_wm_info = {
-	ILK_DISPLAY_FIFO,
-	ILK_DISPLAY_MAXWM,
-	ILK_DISPLAY_DFTWM,
-	2,
-	ILK_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params ironlake_cursor_wm_info = {
-	ILK_CURSOR_FIFO,
-	ILK_CURSOR_MAXWM,
-	ILK_CURSOR_DFTWM,
-	2,
-	ILK_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params ironlake_display_srwm_info = {
-	ILK_DISPLAY_SR_FIFO,
-	ILK_DISPLAY_MAX_SRWM,
-	ILK_DISPLAY_DFT_SRWM,
-	2,
-	ILK_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params ironlake_cursor_srwm_info = {
-	ILK_CURSOR_SR_FIFO,
-	ILK_CURSOR_MAX_SRWM,
-	ILK_CURSOR_DFT_SRWM,
-	2,
-	ILK_FIFO_LINE_SIZE
-};
-
-static const struct intel_watermark_params sandybridge_display_wm_info = {
-	SNB_DISPLAY_FIFO,
-	SNB_DISPLAY_MAXWM,
-	SNB_DISPLAY_DFTWM,
-	2,
-	SNB_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params sandybridge_cursor_wm_info = {
-	SNB_CURSOR_FIFO,
-	SNB_CURSOR_MAXWM,
-	SNB_CURSOR_DFTWM,
-	2,
-	SNB_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params sandybridge_display_srwm_info = {
-	SNB_DISPLAY_SR_FIFO,
-	SNB_DISPLAY_MAX_SRWM,
-	SNB_DISPLAY_DFT_SRWM,
-	2,
-	SNB_FIFO_LINE_SIZE
-};
-static const struct intel_watermark_params sandybridge_cursor_srwm_info = {
-	SNB_CURSOR_SR_FIFO,
-	SNB_CURSOR_MAX_SRWM,
-	SNB_CURSOR_DFT_SRWM,
-	2,
-	SNB_FIFO_LINE_SIZE
-};
-
-
 /**
  * intel_calculate_wm - calculate watermark level
  * @clock_in_khz: pixel clock
@@ -1707,423 +1648,6 @@ static void i830_update_wm(struct drm_crtc *unused_crtc)
 	I915_WRITE(FW_BLC, fwater_lo);
 }
 
-/*
- * Check the wm result.
- *
- * If any calculated watermark values is larger than the maximum value that
- * can be programmed into the associated watermark register, that watermark
- * must be disabled.
- */
-static bool ironlake_check_srwm(struct drm_device *dev, int level,
-				int fbc_wm, int display_wm, int cursor_wm,
-				const struct intel_watermark_params *display,
-				const struct intel_watermark_params *cursor)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	DRM_DEBUG_KMS("watermark %d: display plane %d, fbc lines %d,"
-		      " cursor %d\n", level, display_wm, fbc_wm, cursor_wm);
-
-	if (fbc_wm > SNB_FBC_MAX_SRWM) {
-		DRM_DEBUG_KMS("fbc watermark(%d) is too large(%d), disabling wm%d+\n",
-			      fbc_wm, SNB_FBC_MAX_SRWM, level);
-
-		/* fbc has it's own way to disable FBC WM */
-		I915_WRITE(DISP_ARB_CTL,
-			   I915_READ(DISP_ARB_CTL) | DISP_FBC_WM_DIS);
-		return false;
-	} else if (INTEL_INFO(dev)->gen >= 6) {
-		/* enable FBC WM (except on ILK, where it must remain off) */
-		I915_WRITE(DISP_ARB_CTL,
-			   I915_READ(DISP_ARB_CTL) & ~DISP_FBC_WM_DIS);
-	}
-
-	if (display_wm > display->max_wm) {
-		DRM_DEBUG_KMS("display watermark(%d) is too large(%d), disabling wm%d+\n",
-			      display_wm, SNB_DISPLAY_MAX_SRWM, level);
-		return false;
-	}
-
-	if (cursor_wm > cursor->max_wm) {
-		DRM_DEBUG_KMS("cursor watermark(%d) is too large(%d), disabling wm%d+\n",
-			      cursor_wm, SNB_CURSOR_MAX_SRWM, level);
-		return false;
-	}
-
-	if (!(fbc_wm || display_wm || cursor_wm)) {
-		DRM_DEBUG_KMS("latency %d is 0, disabling wm%d+\n", level, level);
-		return false;
-	}
-
-	return true;
-}
-
-/*
- * Compute watermark values of WM[1-3],
- */
-static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
-				  int latency_ns,
-				  const struct intel_watermark_params *display,
-				  const struct intel_watermark_params *cursor,
-				  int *fbc_wm, int *display_wm, int *cursor_wm)
-{
-	struct drm_crtc *crtc;
-	const struct drm_display_mode *adjusted_mode;
-	unsigned long line_time_us;
-	int hdisplay, htotal, pixel_size, clock;
-	int line_count, line_size;
-	int small, large;
-	int entries;
-
-	if (!latency_ns) {
-		*fbc_wm = *display_wm = *cursor_wm = 0;
-		return false;
-	}
-
-	crtc = intel_get_crtc_for_plane(dev, plane);
-	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
-	clock = adjusted_mode->crtc_clock;
-	htotal = adjusted_mode->crtc_htotal;
-	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
-	pixel_size = crtc->fb->bits_per_pixel / 8;
-
-	line_time_us = (htotal * 1000) / clock;
-	line_count = (latency_ns / line_time_us + 1000) / 1000;
-	line_size = hdisplay * pixel_size;
-
-	/* Use the minimum of the small and large buffer method for primary */
-	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
-	large = line_count * line_size;
-
-	entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
-	*display_wm = entries + display->guard_size;
-
-	/*
-	 * Spec says:
-	 * FBC WM = ((Final Primary WM * 64) / number of bytes per line) + 2
-	 */
-	*fbc_wm = DIV_ROUND_UP(*display_wm * 64, line_size) + 2;
-
-	/* calculate the self-refresh watermark for display cursor */
-	entries = line_count * pixel_size * 64;
-	entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
-	*cursor_wm = entries + cursor->guard_size;
-
-	return ironlake_check_srwm(dev, level,
-				   *fbc_wm, *display_wm, *cursor_wm,
-				   display, cursor);
-}
-
-static void ironlake_update_wm(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int fbc_wm, plane_wm, cursor_wm;
-	unsigned int enabled;
-
-	enabled = 0;
-	if (g4x_compute_wm0(dev, PIPE_A,
-			    &ironlake_display_wm_info,
-			    dev_priv->wm.pri_latency[0] * 100,
-			    &ironlake_cursor_wm_info,
-			    dev_priv->wm.cur_latency[0] * 100,
-			    &plane_wm, &cursor_wm)) {
-		I915_WRITE(WM0_PIPEA_ILK,
-			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
-		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
-			      " plane %d, " "cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_A;
-	}
-
-	if (g4x_compute_wm0(dev, PIPE_B,
-			    &ironlake_display_wm_info,
-			    dev_priv->wm.pri_latency[0] * 100,
-			    &ironlake_cursor_wm_info,
-			    dev_priv->wm.cur_latency[0] * 100,
-			    &plane_wm, &cursor_wm)) {
-		I915_WRITE(WM0_PIPEB_ILK,
-			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
-		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
-			      " plane %d, cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_B;
-	}
-
-	/*
-	 * Calculate and update the self-refresh watermark only when one
-	 * display plane is used.
-	 */
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
-
-	if (!single_plane_enabled(enabled))
-		return;
-	enabled = ffs(enabled) - 1;
-
-	/* WM1 */
-	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   dev_priv->wm.pri_latency[1] * 500,
-				   &ironlake_display_srwm_info,
-				   &ironlake_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM1_LP_ILK,
-		   WM1_LP_SR_EN |
-		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/* WM2 */
-	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   dev_priv->wm.pri_latency[2] * 500,
-				   &ironlake_display_srwm_info,
-				   &ironlake_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM2_LP_ILK,
-		   WM2_LP_EN |
-		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/*
-	 * WM3 is unsupported on ILK, probably because we don't have latency
-	 * data for that power state
-	 */
-}
-
-static void sandybridge_update_wm(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
-	u32 val;
-	int fbc_wm, plane_wm, cursor_wm;
-	unsigned int enabled;
-
-	enabled = 0;
-	if (g4x_compute_wm0(dev, PIPE_A,
-			    &sandybridge_display_wm_info, latency,
-			    &sandybridge_cursor_wm_info, latency,
-			    &plane_wm, &cursor_wm)) {
-		val = I915_READ(WM0_PIPEA_ILK);
-		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
-		I915_WRITE(WM0_PIPEA_ILK, val |
-			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
-		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
-			      " plane %d, " "cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_A;
-	}
-
-	if (g4x_compute_wm0(dev, PIPE_B,
-			    &sandybridge_display_wm_info, latency,
-			    &sandybridge_cursor_wm_info, latency,
-			    &plane_wm, &cursor_wm)) {
-		val = I915_READ(WM0_PIPEB_ILK);
-		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
-		I915_WRITE(WM0_PIPEB_ILK, val |
-			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
-		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
-			      " plane %d, cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_B;
-	}
-
-	/*
-	 * Calculate and update the self-refresh watermark only when one
-	 * display plane is used.
-	 *
-	 * SNB support 3 levels of watermark.
-	 *
-	 * WM1/WM2/WM2 watermarks have to be enabled in the ascending order,
-	 * and disabled in the descending order
-	 *
-	 */
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
-
-	if (!single_plane_enabled(enabled) ||
-	    dev_priv->sprite_scaling_enabled)
-		return;
-	enabled = ffs(enabled) - 1;
-
-	/* WM1 */
-	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   dev_priv->wm.pri_latency[1] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM1_LP_ILK,
-		   WM1_LP_SR_EN |
-		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/* WM2 */
-	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   dev_priv->wm.pri_latency[2] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM2_LP_ILK,
-		   WM2_LP_EN |
-		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/* WM3 */
-	if (!ironlake_compute_srwm(dev, 3, enabled,
-				   dev_priv->wm.pri_latency[3] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM3_LP_ILK,
-		   WM3_LP_EN |
-		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-}
-
-static void ivybridge_update_wm(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
-	u32 val;
-	int fbc_wm, plane_wm, cursor_wm;
-	int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
-	unsigned int enabled;
-
-	enabled = 0;
-	if (g4x_compute_wm0(dev, PIPE_A,
-			    &sandybridge_display_wm_info, latency,
-			    &sandybridge_cursor_wm_info, latency,
-			    &plane_wm, &cursor_wm)) {
-		val = I915_READ(WM0_PIPEA_ILK);
-		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
-		I915_WRITE(WM0_PIPEA_ILK, val |
-			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
-		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
-			      " plane %d, " "cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_A;
-	}
-
-	if (g4x_compute_wm0(dev, PIPE_B,
-			    &sandybridge_display_wm_info, latency,
-			    &sandybridge_cursor_wm_info, latency,
-			    &plane_wm, &cursor_wm)) {
-		val = I915_READ(WM0_PIPEB_ILK);
-		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
-		I915_WRITE(WM0_PIPEB_ILK, val |
-			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
-		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
-			      " plane %d, cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_B;
-	}
-
-	if (g4x_compute_wm0(dev, PIPE_C,
-			    &sandybridge_display_wm_info, latency,
-			    &sandybridge_cursor_wm_info, latency,
-			    &plane_wm, &cursor_wm)) {
-		val = I915_READ(WM0_PIPEC_IVB);
-		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
-		I915_WRITE(WM0_PIPEC_IVB, val |
-			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
-		DRM_DEBUG_KMS("FIFO watermarks For pipe C -"
-			      " plane %d, cursor: %d\n",
-			      plane_wm, cursor_wm);
-		enabled |= 1 << PIPE_C;
-	}
-
-	/*
-	 * Calculate and update the self-refresh watermark only when one
-	 * display plane is used.
-	 *
-	 * SNB support 3 levels of watermark.
-	 *
-	 * WM1/WM2/WM2 watermarks have to be enabled in the ascending order,
-	 * and disabled in the descending order
-	 *
-	 */
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
-
-	if (!single_plane_enabled(enabled) ||
-	    dev_priv->sprite_scaling_enabled)
-		return;
-	enabled = ffs(enabled) - 1;
-
-	/* WM1 */
-	if (!ironlake_compute_srwm(dev, 1, enabled,
-				   dev_priv->wm.pri_latency[1] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM1_LP_ILK,
-		   WM1_LP_SR_EN |
-		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/* WM2 */
-	if (!ironlake_compute_srwm(dev, 2, enabled,
-				   dev_priv->wm.pri_latency[2] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM2_LP_ILK,
-		   WM2_LP_EN |
-		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-
-	/* WM3, note we have to correct the cursor latency */
-	if (!ironlake_compute_srwm(dev, 3, enabled,
-				   dev_priv->wm.pri_latency[3] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
-	    !ironlake_compute_srwm(dev, 3, enabled,
-				   dev_priv->wm.cur_latency[3] * 500,
-				   &sandybridge_display_srwm_info,
-				   &sandybridge_cursor_srwm_info,
-				   &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
-		return;
-
-	I915_WRITE(WM3_LP_ILK,
-		   WM3_LP_EN |
-		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
-		   (fbc_wm << WM1_LP_FBC_SHIFT) |
-		   (plane_wm << WM1_LP_SR_SHIFT) |
-		   cursor_wm);
-}
-
 static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 				    struct drm_crtc *crtc)
 {
@@ -3059,168 +2583,6 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
 	haswell_update_wm(crtc);
 }
 
-static bool
-sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
-			      uint32_t sprite_width, int pixel_size,
-			      const struct intel_watermark_params *display,
-			      int display_latency_ns, int *sprite_wm)
-{
-	struct drm_crtc *crtc;
-	int clock;
-	int entries, tlb_miss;
-
-	crtc = intel_get_crtc_for_plane(dev, plane);
-	if (!intel_crtc_active(crtc)) {
-		*sprite_wm = display->guard_size;
-		return false;
-	}
-
-	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
-
-	/* Use the small buffer method to calculate the sprite watermark */
-	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
-	tlb_miss = display->fifo_size*display->cacheline_size -
-		sprite_width * 8;
-	if (tlb_miss > 0)
-		entries += tlb_miss;
-	entries = DIV_ROUND_UP(entries, display->cacheline_size);
-	*sprite_wm = entries + display->guard_size;
-	if (*sprite_wm > (int)display->max_wm)
-		*sprite_wm = display->max_wm;
-
-	return true;
-}
-
-static bool
-sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
-				uint32_t sprite_width, int pixel_size,
-				const struct intel_watermark_params *display,
-				int latency_ns, int *sprite_wm)
-{
-	struct drm_crtc *crtc;
-	unsigned long line_time_us;
-	int clock;
-	int line_count, line_size;
-	int small, large;
-	int entries;
-
-	if (!latency_ns) {
-		*sprite_wm = 0;
-		return false;
-	}
-
-	crtc = intel_get_crtc_for_plane(dev, plane);
-	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
-	if (!clock) {
-		*sprite_wm = 0;
-		return false;
-	}
-
-	line_time_us = (sprite_width * 1000) / clock;
-	if (!line_time_us) {
-		*sprite_wm = 0;
-		return false;
-	}
-
-	line_count = (latency_ns / line_time_us + 1000) / 1000;
-	line_size = sprite_width * pixel_size;
-
-	/* Use the minimum of the small and large buffer method for primary */
-	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
-	large = line_count * line_size;
-
-	entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
-	*sprite_wm = entries + display->guard_size;
-
-	return *sprite_wm > 0x3ff ? false : true;
-}
-
-static void sandybridge_update_sprite_wm(struct drm_plane *plane,
-					 struct drm_crtc *crtc,
-					 uint32_t sprite_width, int pixel_size,
-					 bool enabled, bool scaled)
-{
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int pipe = to_intel_plane(plane)->pipe;
-	int latency = dev_priv->wm.spr_latency[0] * 100;	/* In unit 0.1us */
-	u32 val;
-	int sprite_wm, reg;
-	int ret;
-
-	if (!enabled)
-		return;
-
-	switch (pipe) {
-	case 0:
-		reg = WM0_PIPEA_ILK;
-		break;
-	case 1:
-		reg = WM0_PIPEB_ILK;
-		break;
-	case 2:
-		reg = WM0_PIPEC_IVB;
-		break;
-	default:
-		return; /* bad pipe */
-	}
-
-	ret = sandybridge_compute_sprite_wm(dev, pipe, sprite_width, pixel_size,
-					    &sandybridge_display_wm_info,
-					    latency, &sprite_wm);
-	if (!ret) {
-		DRM_DEBUG_KMS("failed to compute sprite wm for pipe %c\n",
-			      pipe_name(pipe));
-		return;
-	}
-
-	val = I915_READ(reg);
-	val &= ~WM0_PIPE_SPRITE_MASK;
-	I915_WRITE(reg, val | (sprite_wm << WM0_PIPE_SPRITE_SHIFT));
-	DRM_DEBUG_KMS("sprite watermarks For pipe %c - %d\n", pipe_name(pipe), sprite_wm);
-
-
-	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
-					      pixel_size,
-					      &sandybridge_display_srwm_info,
-					      dev_priv->wm.spr_latency[1] * 500,
-					      &sprite_wm);
-	if (!ret) {
-		DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
-			      pipe_name(pipe));
-		return;
-	}
-	I915_WRITE(WM1S_LP_ILK, sprite_wm);
-
-	/* Only IVB has two more LP watermarks for sprite */
-	if (!IS_IVYBRIDGE(dev))
-		return;
-
-	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
-					      pixel_size,
-					      &sandybridge_display_srwm_info,
-					      dev_priv->wm.spr_latency[2] * 500,
-					      &sprite_wm);
-	if (!ret) {
-		DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
-			      pipe_name(pipe));
-		return;
-	}
-	I915_WRITE(WM2S_LP_IVB, sprite_wm);
-
-	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
-					      pixel_size,
-					      &sandybridge_display_srwm_info,
-					      dev_priv->wm.spr_latency[3] * 500,
-					      &sprite_wm);
-	if (!ret) {
-		DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
-			      pipe_name(pipe));
-		return;
-	}
-	I915_WRITE(WM3S_LP_IVB, sprite_wm);
-}
-
 static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -6072,9 +5434,11 @@ void intel_init_pm(struct drm_device *dev)
 		if (IS_GEN5(dev)) {
 			if (dev_priv->wm.pri_latency[1] &&
 			    dev_priv->wm.spr_latency[1] &&
-			    dev_priv->wm.cur_latency[1])
-				dev_priv->display.update_wm = ironlake_update_wm;
-			else {
+			    dev_priv->wm.cur_latency[1]) {
+				dev_priv->display.update_wm = haswell_update_wm;
+				dev_priv->display.update_sprite_wm =
+					haswell_update_sprite_wm;
+			} else {
 				DRM_DEBUG_KMS("Failed to get proper latency. "
 					      "Disable CxSR\n");
 				dev_priv->display.update_wm = NULL;
@@ -6084,8 +5448,9 @@ void intel_init_pm(struct drm_device *dev)
 			if (dev_priv->wm.pri_latency[0] &&
 			    dev_priv->wm.spr_latency[0] &&
 			    dev_priv->wm.cur_latency[0]) {
-				dev_priv->display.update_wm = sandybridge_update_wm;
-				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
+				dev_priv->display.update_wm = haswell_update_wm;
+				dev_priv->display.update_sprite_wm =
+					haswell_update_sprite_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
@@ -6096,8 +5461,9 @@ void intel_init_pm(struct drm_device *dev)
 			if (dev_priv->wm.pri_latency[0] &&
 			    dev_priv->wm.spr_latency[0] &&
 			    dev_priv->wm.cur_latency[0]) {
-				dev_priv->display.update_wm = ivybridge_update_wm;
-				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
+				dev_priv->display.update_wm = haswell_update_wm;
+				dev_priv->display.update_sprite_wm =
+					haswell_update_sprite_wm;
 			} else {
 				DRM_DEBUG_KMS("Failed to read display plane latency. "
 					      "Disable CxSR\n");
-- 
1.8.3.2

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

Re: [PATCH 11/14] drm/i915: Move ILK/SNB/IVB over to the HSW WM code

[Imre Deak]

On Thu, 2013-12-05 at 15:51 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The new HSW watermark code can now handle ILK/SNB/IVB as well, so
> switch them over. Kill the old code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 656 +----------------------------------
>  2 files changed, 12 insertions(+), 646 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3c59b67..24066cc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11208,7 +11208,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  		pll->on = false;
>  	}
>  
> -	if (IS_HASWELL(dev))
> +	if (HAS_PCH_SPLIT(dev))
>  		ilk_wm_get_hw_state(dev);
>  
>  	if (force_restore) {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 420be2f..8b97893 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -965,65 +965,6 @@ static const struct intel_watermark_params i830_wm_info = {
>  	I830_FIFO_LINE_SIZE
>  };
>  
> -static const struct intel_watermark_params ironlake_display_wm_info = {
> -	ILK_DISPLAY_FIFO,
> -	ILK_DISPLAY_MAXWM,
> -	ILK_DISPLAY_DFTWM,
> -	2,
> -	ILK_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params ironlake_cursor_wm_info = {
> -	ILK_CURSOR_FIFO,
> -	ILK_CURSOR_MAXWM,
> -	ILK_CURSOR_DFTWM,
> -	2,
> -	ILK_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params ironlake_display_srwm_info = {
> -	ILK_DISPLAY_SR_FIFO,
> -	ILK_DISPLAY_MAX_SRWM,
> -	ILK_DISPLAY_DFT_SRWM,
> -	2,
> -	ILK_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params ironlake_cursor_srwm_info = {
> -	ILK_CURSOR_SR_FIFO,
> -	ILK_CURSOR_MAX_SRWM,
> -	ILK_CURSOR_DFT_SRWM,
> -	2,
> -	ILK_FIFO_LINE_SIZE
> -};
> -
> -static const struct intel_watermark_params sandybridge_display_wm_info = {
> -	SNB_DISPLAY_FIFO,
> -	SNB_DISPLAY_MAXWM,
> -	SNB_DISPLAY_DFTWM,
> -	2,
> -	SNB_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params sandybridge_cursor_wm_info = {
> -	SNB_CURSOR_FIFO,
> -	SNB_CURSOR_MAXWM,
> -	SNB_CURSOR_DFTWM,
> -	2,
> -	SNB_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params sandybridge_display_srwm_info = {
> -	SNB_DISPLAY_SR_FIFO,
> -	SNB_DISPLAY_MAX_SRWM,
> -	SNB_DISPLAY_DFT_SRWM,
> -	2,
> -	SNB_FIFO_LINE_SIZE
> -};
> -static const struct intel_watermark_params sandybridge_cursor_srwm_info = {
> -	SNB_CURSOR_SR_FIFO,
> -	SNB_CURSOR_MAX_SRWM,
> -	SNB_CURSOR_DFT_SRWM,
> -	2,
> -	SNB_FIFO_LINE_SIZE
> -};

We could also remove now the above macros.

> -
> -
>  /**
>   * intel_calculate_wm - calculate watermark level
>   * @clock_in_khz: pixel clock
> @@ -1707,423 +1648,6 @@ static void i830_update_wm(struct drm_crtc *unused_crtc)
>  	I915_WRITE(FW_BLC, fwater_lo);
>  }
>  
> -/*
> - * Check the wm result.
> - *
> - * If any calculated watermark values is larger than the maximum value that
> - * can be programmed into the associated watermark register, that watermark
> - * must be disabled.
> - */
> -static bool ironlake_check_srwm(struct drm_device *dev, int level,
> -				int fbc_wm, int display_wm, int cursor_wm,
> -				const struct intel_watermark_params *display,
> -				const struct intel_watermark_params *cursor)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	DRM_DEBUG_KMS("watermark %d: display plane %d, fbc lines %d,"
> -		      " cursor %d\n", level, display_wm, fbc_wm, cursor_wm);
> -
> -	if (fbc_wm > SNB_FBC_MAX_SRWM) {
> -		DRM_DEBUG_KMS("fbc watermark(%d) is too large(%d), disabling wm%d+\n",
> -			      fbc_wm, SNB_FBC_MAX_SRWM, level);
> -
> -		/* fbc has it's own way to disable FBC WM */
> -		I915_WRITE(DISP_ARB_CTL,
> -			   I915_READ(DISP_ARB_CTL) | DISP_FBC_WM_DIS);
> -		return false;
> -	} else if (INTEL_INFO(dev)->gen >= 6) {
> -		/* enable FBC WM (except on ILK, where it must remain off) */
> -		I915_WRITE(DISP_ARB_CTL,
> -			   I915_READ(DISP_ARB_CTL) & ~DISP_FBC_WM_DIS);
> -	}
> -
> -	if (display_wm > display->max_wm) {
> -		DRM_DEBUG_KMS("display watermark(%d) is too large(%d), disabling wm%d+\n",
> -			      display_wm, SNB_DISPLAY_MAX_SRWM, level);
> -		return false;
> -	}
> -
> -	if (cursor_wm > cursor->max_wm) {
> -		DRM_DEBUG_KMS("cursor watermark(%d) is too large(%d), disabling wm%d+\n",
> -			      cursor_wm, SNB_CURSOR_MAX_SRWM, level);
> -		return false;
> -	}
> -
> -	if (!(fbc_wm || display_wm || cursor_wm)) {
> -		DRM_DEBUG_KMS("latency %d is 0, disabling wm%d+\n", level, level);
> -		return false;
> -	}
> -
> -	return true;
> -}
> -
> -/*
> - * Compute watermark values of WM[1-3],
> - */
> -static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
> -				  int latency_ns,
> -				  const struct intel_watermark_params *display,
> -				  const struct intel_watermark_params *cursor,
> -				  int *fbc_wm, int *display_wm, int *cursor_wm)
> -{
> -	struct drm_crtc *crtc;
> -	const struct drm_display_mode *adjusted_mode;
> -	unsigned long line_time_us;
> -	int hdisplay, htotal, pixel_size, clock;
> -	int line_count, line_size;
> -	int small, large;
> -	int entries;
> -
> -	if (!latency_ns) {
> -		*fbc_wm = *display_wm = *cursor_wm = 0;
> -		return false;
> -	}
> -
> -	crtc = intel_get_crtc_for_plane(dev, plane);
> -	adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> -	clock = adjusted_mode->crtc_clock;
> -	htotal = adjusted_mode->crtc_htotal;
> -	hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
> -	pixel_size = crtc->fb->bits_per_pixel / 8;
> -
> -	line_time_us = (htotal * 1000) / clock;
> -	line_count = (latency_ns / line_time_us + 1000) / 1000;
> -	line_size = hdisplay * pixel_size;
> -
> -	/* Use the minimum of the small and large buffer method for primary */
> -	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
> -	large = line_count * line_size;
> -
> -	entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
> -	*display_wm = entries + display->guard_size;
> -
> -	/*
> -	 * Spec says:
> -	 * FBC WM = ((Final Primary WM * 64) / number of bytes per line) + 2
> -	 */
> -	*fbc_wm = DIV_ROUND_UP(*display_wm * 64, line_size) + 2;
> -
> -	/* calculate the self-refresh watermark for display cursor */
> -	entries = line_count * pixel_size * 64;
> -	entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
> -	*cursor_wm = entries + cursor->guard_size;
> -
> -	return ironlake_check_srwm(dev, level,
> -				   *fbc_wm, *display_wm, *cursor_wm,
> -				   display, cursor);
> -}
> -
> -static void ironlake_update_wm(struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int fbc_wm, plane_wm, cursor_wm;
> -	unsigned int enabled;
> -
> -	enabled = 0;
> -	if (g4x_compute_wm0(dev, PIPE_A,
> -			    &ironlake_display_wm_info,
> -			    dev_priv->wm.pri_latency[0] * 100,
> -			    &ironlake_cursor_wm_info,
> -			    dev_priv->wm.cur_latency[0] * 100,
> -			    &plane_wm, &cursor_wm)) {
> -		I915_WRITE(WM0_PIPEA_ILK,
> -			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
> -			      " plane %d, " "cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_A;
> -	}
> -
> -	if (g4x_compute_wm0(dev, PIPE_B,
> -			    &ironlake_display_wm_info,
> -			    dev_priv->wm.pri_latency[0] * 100,
> -			    &ironlake_cursor_wm_info,
> -			    dev_priv->wm.cur_latency[0] * 100,
> -			    &plane_wm, &cursor_wm)) {
> -		I915_WRITE(WM0_PIPEB_ILK,
> -			   (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
> -			      " plane %d, cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_B;
> -	}
> -
> -	/*
> -	 * Calculate and update the self-refresh watermark only when one
> -	 * display plane is used.
> -	 */
> -	I915_WRITE(WM3_LP_ILK, 0);
> -	I915_WRITE(WM2_LP_ILK, 0);
> -	I915_WRITE(WM1_LP_ILK, 0);
> -
> -	if (!single_plane_enabled(enabled))
> -		return;
> -	enabled = ffs(enabled) - 1;
> -
> -	/* WM1 */
> -	if (!ironlake_compute_srwm(dev, 1, enabled,
> -				   dev_priv->wm.pri_latency[1] * 500,
> -				   &ironlake_display_srwm_info,
> -				   &ironlake_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM1_LP_ILK,
> -		   WM1_LP_SR_EN |
> -		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/* WM2 */
> -	if (!ironlake_compute_srwm(dev, 2, enabled,
> -				   dev_priv->wm.pri_latency[2] * 500,
> -				   &ironlake_display_srwm_info,
> -				   &ironlake_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM2_LP_ILK,
> -		   WM2_LP_EN |
> -		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/*
> -	 * WM3 is unsupported on ILK, probably because we don't have latency
> -	 * data for that power state
> -	 */
> -}
> -
> -static void sandybridge_update_wm(struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
> -	u32 val;
> -	int fbc_wm, plane_wm, cursor_wm;
> -	unsigned int enabled;
> -
> -	enabled = 0;
> -	if (g4x_compute_wm0(dev, PIPE_A,
> -			    &sandybridge_display_wm_info, latency,
> -			    &sandybridge_cursor_wm_info, latency,
> -			    &plane_wm, &cursor_wm)) {
> -		val = I915_READ(WM0_PIPEA_ILK);
> -		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> -		I915_WRITE(WM0_PIPEA_ILK, val |
> -			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
> -			      " plane %d, " "cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_A;
> -	}
> -
> -	if (g4x_compute_wm0(dev, PIPE_B,
> -			    &sandybridge_display_wm_info, latency,
> -			    &sandybridge_cursor_wm_info, latency,
> -			    &plane_wm, &cursor_wm)) {
> -		val = I915_READ(WM0_PIPEB_ILK);
> -		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> -		I915_WRITE(WM0_PIPEB_ILK, val |
> -			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
> -			      " plane %d, cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_B;
> -	}
> -
> -	/*
> -	 * Calculate and update the self-refresh watermark only when one
> -	 * display plane is used.
> -	 *
> -	 * SNB support 3 levels of watermark.
> -	 *
> -	 * WM1/WM2/WM2 watermarks have to be enabled in the ascending order,
> -	 * and disabled in the descending order
> -	 *
> -	 */
> -	I915_WRITE(WM3_LP_ILK, 0);
> -	I915_WRITE(WM2_LP_ILK, 0);
> -	I915_WRITE(WM1_LP_ILK, 0);
> -
> -	if (!single_plane_enabled(enabled) ||
> -	    dev_priv->sprite_scaling_enabled)
> -		return;
> -	enabled = ffs(enabled) - 1;
> -
> -	/* WM1 */
> -	if (!ironlake_compute_srwm(dev, 1, enabled,
> -				   dev_priv->wm.pri_latency[1] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM1_LP_ILK,
> -		   WM1_LP_SR_EN |
> -		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/* WM2 */
> -	if (!ironlake_compute_srwm(dev, 2, enabled,
> -				   dev_priv->wm.pri_latency[2] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM2_LP_ILK,
> -		   WM2_LP_EN |
> -		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/* WM3 */
> -	if (!ironlake_compute_srwm(dev, 3, enabled,
> -				   dev_priv->wm.pri_latency[3] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM3_LP_ILK,
> -		   WM3_LP_EN |
> -		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -}
> -
> -static void ivybridge_update_wm(struct drm_crtc *crtc)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int latency = dev_priv->wm.pri_latency[0] * 100;	/* In unit 0.1us */
> -	u32 val;
> -	int fbc_wm, plane_wm, cursor_wm;
> -	int ignore_fbc_wm, ignore_plane_wm, ignore_cursor_wm;
> -	unsigned int enabled;
> -
> -	enabled = 0;
> -	if (g4x_compute_wm0(dev, PIPE_A,
> -			    &sandybridge_display_wm_info, latency,
> -			    &sandybridge_cursor_wm_info, latency,
> -			    &plane_wm, &cursor_wm)) {
> -		val = I915_READ(WM0_PIPEA_ILK);
> -		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> -		I915_WRITE(WM0_PIPEA_ILK, val |
> -			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
> -			      " plane %d, " "cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_A;
> -	}
> -
> -	if (g4x_compute_wm0(dev, PIPE_B,
> -			    &sandybridge_display_wm_info, latency,
> -			    &sandybridge_cursor_wm_info, latency,
> -			    &plane_wm, &cursor_wm)) {
> -		val = I915_READ(WM0_PIPEB_ILK);
> -		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> -		I915_WRITE(WM0_PIPEB_ILK, val |
> -			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
> -			      " plane %d, cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_B;
> -	}
> -
> -	if (g4x_compute_wm0(dev, PIPE_C,
> -			    &sandybridge_display_wm_info, latency,
> -			    &sandybridge_cursor_wm_info, latency,
> -			    &plane_wm, &cursor_wm)) {
> -		val = I915_READ(WM0_PIPEC_IVB);
> -		val &= ~(WM0_PIPE_PLANE_MASK | WM0_PIPE_CURSOR_MASK);
> -		I915_WRITE(WM0_PIPEC_IVB, val |
> -			   ((plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm));
> -		DRM_DEBUG_KMS("FIFO watermarks For pipe C -"
> -			      " plane %d, cursor: %d\n",
> -			      plane_wm, cursor_wm);
> -		enabled |= 1 << PIPE_C;
> -	}
> -
> -	/*
> -	 * Calculate and update the self-refresh watermark only when one
> -	 * display plane is used.
> -	 *
> -	 * SNB support 3 levels of watermark.
> -	 *
> -	 * WM1/WM2/WM2 watermarks have to be enabled in the ascending order,
> -	 * and disabled in the descending order
> -	 *
> -	 */
> -	I915_WRITE(WM3_LP_ILK, 0);
> -	I915_WRITE(WM2_LP_ILK, 0);
> -	I915_WRITE(WM1_LP_ILK, 0);
> -
> -	if (!single_plane_enabled(enabled) ||
> -	    dev_priv->sprite_scaling_enabled)
> -		return;
> -	enabled = ffs(enabled) - 1;
> -
> -	/* WM1 */
> -	if (!ironlake_compute_srwm(dev, 1, enabled,
> -				   dev_priv->wm.pri_latency[1] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM1_LP_ILK,
> -		   WM1_LP_SR_EN |
> -		   (dev_priv->wm.pri_latency[1] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/* WM2 */
> -	if (!ironlake_compute_srwm(dev, 2, enabled,
> -				   dev_priv->wm.pri_latency[2] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM2_LP_ILK,
> -		   WM2_LP_EN |
> -		   (dev_priv->wm.pri_latency[2] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -
> -	/* WM3, note we have to correct the cursor latency */
> -	if (!ironlake_compute_srwm(dev, 3, enabled,
> -				   dev_priv->wm.pri_latency[3] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &fbc_wm, &plane_wm, &ignore_cursor_wm) ||
> -	    !ironlake_compute_srwm(dev, 3, enabled,
> -				   dev_priv->wm.cur_latency[3] * 500,
> -				   &sandybridge_display_srwm_info,
> -				   &sandybridge_cursor_srwm_info,
> -				   &ignore_fbc_wm, &ignore_plane_wm, &cursor_wm))
> -		return;
> -
> -	I915_WRITE(WM3_LP_ILK,
> -		   WM3_LP_EN |
> -		   (dev_priv->wm.pri_latency[3] << WM1_LP_LATENCY_SHIFT) |
> -		   (fbc_wm << WM1_LP_FBC_SHIFT) |
> -		   (plane_wm << WM1_LP_SR_SHIFT) |
> -		   cursor_wm);
> -}
> -
>  static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>  				    struct drm_crtc *crtc)
>  {
> @@ -3059,168 +2583,6 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
>  	haswell_update_wm(crtc);
>  }
>  
> -static bool
> -sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
> -			      uint32_t sprite_width, int pixel_size,
> -			      const struct intel_watermark_params *display,
> -			      int display_latency_ns, int *sprite_wm)
> -{
> -	struct drm_crtc *crtc;
> -	int clock;
> -	int entries, tlb_miss;
> -
> -	crtc = intel_get_crtc_for_plane(dev, plane);
> -	if (!intel_crtc_active(crtc)) {
> -		*sprite_wm = display->guard_size;
> -		return false;
> -	}
> -
> -	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> -
> -	/* Use the small buffer method to calculate the sprite watermark */
> -	entries = ((clock * pixel_size / 1000) * display_latency_ns) / 1000;
> -	tlb_miss = display->fifo_size*display->cacheline_size -
> -		sprite_width * 8;
> -	if (tlb_miss > 0)
> -		entries += tlb_miss;
> -	entries = DIV_ROUND_UP(entries, display->cacheline_size);
> -	*sprite_wm = entries + display->guard_size;
> -	if (*sprite_wm > (int)display->max_wm)
> -		*sprite_wm = display->max_wm;
> -
> -	return true;
> -}
> -
> -static bool
> -sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
> -				uint32_t sprite_width, int pixel_size,
> -				const struct intel_watermark_params *display,
> -				int latency_ns, int *sprite_wm)
> -{
> -	struct drm_crtc *crtc;
> -	unsigned long line_time_us;
> -	int clock;
> -	int line_count, line_size;
> -	int small, large;
> -	int entries;
> -
> -	if (!latency_ns) {
> -		*sprite_wm = 0;
> -		return false;
> -	}
> -
> -	crtc = intel_get_crtc_for_plane(dev, plane);
> -	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> -	if (!clock) {
> -		*sprite_wm = 0;
> -		return false;
> -	}
> -
> -	line_time_us = (sprite_width * 1000) / clock;
> -	if (!line_time_us) {
> -		*sprite_wm = 0;
> -		return false;
> -	}
> -
> -	line_count = (latency_ns / line_time_us + 1000) / 1000;
> -	line_size = sprite_width * pixel_size;
> -
> -	/* Use the minimum of the small and large buffer method for primary */
> -	small = ((clock * pixel_size / 1000) * latency_ns) / 1000;
> -	large = line_count * line_size;
> -
> -	entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
> -	*sprite_wm = entries + display->guard_size;
> -
> -	return *sprite_wm > 0x3ff ? false : true;
> -}
> -
> -static void sandybridge_update_sprite_wm(struct drm_plane *plane,
> -					 struct drm_crtc *crtc,
> -					 uint32_t sprite_width, int pixel_size,
> -					 bool enabled, bool scaled)
> -{
> -	struct drm_device *dev = plane->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int pipe = to_intel_plane(plane)->pipe;
> -	int latency = dev_priv->wm.spr_latency[0] * 100;	/* In unit 0.1us */
> -	u32 val;
> -	int sprite_wm, reg;
> -	int ret;
> -
> -	if (!enabled)
> -		return;
> -
> -	switch (pipe) {
> -	case 0:
> -		reg = WM0_PIPEA_ILK;
> -		break;
> -	case 1:
> -		reg = WM0_PIPEB_ILK;
> -		break;
> -	case 2:
> -		reg = WM0_PIPEC_IVB;
> -		break;
> -	default:
> -		return; /* bad pipe */
> -	}
> -
> -	ret = sandybridge_compute_sprite_wm(dev, pipe, sprite_width, pixel_size,
> -					    &sandybridge_display_wm_info,
> -					    latency, &sprite_wm);
> -	if (!ret) {
> -		DRM_DEBUG_KMS("failed to compute sprite wm for pipe %c\n",
> -			      pipe_name(pipe));
> -		return;
> -	}
> -
> -	val = I915_READ(reg);
> -	val &= ~WM0_PIPE_SPRITE_MASK;
> -	I915_WRITE(reg, val | (sprite_wm << WM0_PIPE_SPRITE_SHIFT));
> -	DRM_DEBUG_KMS("sprite watermarks For pipe %c - %d\n", pipe_name(pipe), sprite_wm);
> -
> -
> -	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> -					      pixel_size,
> -					      &sandybridge_display_srwm_info,
> -					      dev_priv->wm.spr_latency[1] * 500,
> -					      &sprite_wm);
> -	if (!ret) {
> -		DRM_DEBUG_KMS("failed to compute sprite lp1 wm on pipe %c\n",
> -			      pipe_name(pipe));
> -		return;
> -	}
> -	I915_WRITE(WM1S_LP_ILK, sprite_wm);
> -
> -	/* Only IVB has two more LP watermarks for sprite */
> -	if (!IS_IVYBRIDGE(dev))
> -		return;
> -
> -	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> -					      pixel_size,
> -					      &sandybridge_display_srwm_info,
> -					      dev_priv->wm.spr_latency[2] * 500,
> -					      &sprite_wm);
> -	if (!ret) {
> -		DRM_DEBUG_KMS("failed to compute sprite lp2 wm on pipe %c\n",
> -			      pipe_name(pipe));
> -		return;
> -	}
> -	I915_WRITE(WM2S_LP_IVB, sprite_wm);
> -
> -	ret = sandybridge_compute_sprite_srwm(dev, pipe, sprite_width,
> -					      pixel_size,
> -					      &sandybridge_display_srwm_info,
> -					      dev_priv->wm.spr_latency[3] * 500,
> -					      &sprite_wm);
> -	if (!ret) {
> -		DRM_DEBUG_KMS("failed to compute sprite lp3 wm on pipe %c\n",
> -			      pipe_name(pipe));
> -		return;
> -	}
> -	I915_WRITE(WM3S_LP_IVB, sprite_wm);
> -}
> -
>  static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -6072,9 +5434,11 @@ void intel_init_pm(struct drm_device *dev)
>  		if (IS_GEN5(dev)) {
>  			if (dev_priv->wm.pri_latency[1] &&
>  			    dev_priv->wm.spr_latency[1] &&
> -			    dev_priv->wm.cur_latency[1])
> -				dev_priv->display.update_wm = ironlake_update_wm;
> -			else {
> +			    dev_priv->wm.cur_latency[1]) {
> +				dev_priv->display.update_wm = haswell_update_wm;
> +				dev_priv->display.update_sprite_wm =
> +					haswell_update_sprite_wm;

ironlake would be a better prefix now.

--Imre

> +			} else {



>  				DRM_DEBUG_KMS("Failed to get proper latency. "
>  					      "Disable CxSR\n");
>  				dev_priv->display.update_wm = NULL;
> @@ -6084,8 +5448,9 @@ void intel_init_pm(struct drm_device *dev)
>  			if (dev_priv->wm.pri_latency[0] &&
>  			    dev_priv->wm.spr_latency[0] &&
>  			    dev_priv->wm.cur_latency[0]) {
> -				dev_priv->display.update_wm = sandybridge_update_wm;
> -				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
> +				dev_priv->display.update_wm = haswell_update_wm;
> +				dev_priv->display.update_sprite_wm =
> +					haswell_update_sprite_wm;
>  			} else {
>  				DRM_DEBUG_KMS("Failed to read display plane latency. "
>  					      "Disable CxSR\n");
> @@ -6096,8 +5461,9 @@ void intel_init_pm(struct drm_device *dev)
>  			if (dev_priv->wm.pri_latency[0] &&
>  			    dev_priv->wm.spr_latency[0] &&
>  			    dev_priv->wm.cur_latency[0]) {
> -				dev_priv->display.update_wm = ivybridge_update_wm;
> -				dev_priv->display.update_sprite_wm = sandybridge_update_sprite_wm;
> +				dev_priv->display.update_wm = haswell_update_wm;
> +				dev_priv->display.update_sprite_wm =
> +					haswell_update_sprite_wm;
>  			} else {
>  				DRM_DEBUG_KMS("Failed to read display plane latency. "
>  					      "Disable CxSR\n");


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

[PATCH 12/14] drm/i915: Try to fix the messy IVB sprite scaling workaround

[ville.syrjala]

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

We now have a very clear method of disabling LP1+ wartermarks,
and we can actually detect if we actually did disable them, or
if they were already disabled. Use that to clean up the
WaCxSRDisabledForSpriteScaling:ivb handling.

I was hoping to apply the workaround in a way that wouldn't
require a blocking wait, but sadly IVB really does appear to
require LP1+ watermarks to be off for an entire frame before
enabling sprite scaling. Simply disabling LP1+ watermarks
during the previous frame is not enough, no matter how early
in the frame we do it :(

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 -
 drivers/gpu/drm/i915/intel_pm.c     | 58 ++++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_sprite.c | 27 +----------------
 3 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 780f815..79aff69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1393,7 +1393,6 @@ typedef struct drm_i915_private {
 
 	/* overlay */
 	struct intel_overlay *overlay;
-	unsigned int sprite_scaling_enabled;
 
 	/* backlight registers and fields in struct intel_panel */
 	spinlock_t backlight_lock;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8b97893..1958de0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2427,33 +2427,26 @@ static unsigned int ilk_compute_wm_dirty(struct drm_device *dev,
 	return dirty;
 }
 
-/*
- * The spec says we shouldn't write when we don't need, because every write
- * causes WMs to be re-evaluated, expending some power.
- */
-static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
-				struct hsw_wm_values *results)
+static bool _ilk_disable_lp_wm(struct drm_i915_private *dev_priv,
+			       unsigned int dirty)
 {
-	struct drm_device *dev = dev_priv->dev;
 	struct hsw_wm_values *previous = &dev_priv->wm.hw;
-	unsigned int dirty;
-	uint32_t val;
-
-	dirty = ilk_compute_wm_dirty(dev_priv->dev, previous, results);
-	if (!dirty)
-		return;
+	bool changed = false;
 
 	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {
 		previous->wm_lp[2] &= ~WM1_LP_SR_EN;
 		I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);
+		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {
 		previous->wm_lp[1] &= ~WM1_LP_SR_EN;
 		I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);
+		changed = true;
 	}
 	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {
 		previous->wm_lp[0] &= ~WM1_LP_SR_EN;
 		I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);
+		changed = true;
 	}
 
 	/*
@@ -2461,6 +2454,27 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 	 * Doing so could cause underruns.
 	 */
 
+	return changed;
+}
+
+/*
+ * The spec says we shouldn't write when we don't need, because every write
+ * causes WMs to be re-evaluated, expending some power.
+ */
+static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
+				struct hsw_wm_values *results)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct hsw_wm_values *previous = &dev_priv->wm.hw;
+	unsigned int dirty;
+	uint32_t val;
+
+	dirty = ilk_compute_wm_dirty(dev, previous, results);
+	if (!dirty)
+		return;
+
+	_ilk_disable_lp_wm(dev_priv, dirty);
+
 	if (dirty & WM_DIRTY_PIPE(PIPE_A))
 		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);
 	if (dirty & WM_DIRTY_PIPE(PIPE_B))
@@ -2524,6 +2538,13 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
 	dev_priv->wm.hw = *results;
 }
 
+static bool ilk_disable_lp_wm(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
+}
+
 static void haswell_update_wm(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2573,6 +2594,7 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
 				     uint32_t sprite_width, int pixel_size,
 				     bool enabled, bool scaled)
 {
+	struct drm_device *dev = plane->dev;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 
 	intel_plane->wm.enabled = enabled;
@@ -2580,6 +2602,16 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
 	intel_plane->wm.horiz_pixels = sprite_width;
 	intel_plane->wm.bytes_per_pixel = pixel_size;
 
+	/*
+	 * IVB workaround: must disable low power watermarks for at least
+	 * one frame before enabling scaling.  LP watermarks can be re-enabled
+	 * when scaling is disabled.
+	 *
+	 * WaCxSRDisabledForSpriteScaling:ivb
+	 */
+	if (IS_IVYBRIDGE(dev) && scaled && ilk_disable_lp_wm(dev))
+		intel_wait_for_vblank(dev, intel_plane->pipe);
+
 	haswell_update_wm(crtc);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6bfebfb..3ce328e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -230,7 +230,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	u32 sprctl, sprscale = 0;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
-	bool scaling_was_enabled = dev_priv->sprite_scaling_enabled;
 
 	sprctl = I915_READ(SPRCTL(pipe));
 
@@ -291,21 +290,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	crtc_w--;
 	crtc_h--;
 
-	/*
-	 * IVB workaround: must disable low power watermarks for at least
-	 * one frame before enabling scaling.  LP watermarks can be re-enabled
-	 * when scaling is disabled.
-	 */
-	if (crtc_w != src_w || crtc_h != src_h) {
-		dev_priv->sprite_scaling_enabled |= 1 << pipe;
-
-		if (!scaling_was_enabled) {
-			intel_update_watermarks(crtc);
-			intel_wait_for_vblank(dev, pipe);
-		}
+	if (crtc_w != src_w || crtc_h != src_h)
 		sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
-	} else
-		dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
 
 	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
@@ -332,10 +318,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	I915_MODIFY_DISPBASE(SPRSURF(pipe),
 			     i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
 	POSTING_READ(SPRSURF(pipe));
-
-	/* potentially re-enable LP watermarks */
-	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
-		intel_update_watermarks(crtc);
 }
 
 static void
@@ -345,7 +327,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
-	bool scaling_was_enabled = dev_priv->sprite_scaling_enabled;
 
 	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
 	/* Can't leave the scaler enabled... */
@@ -355,13 +336,7 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);
 	POSTING_READ(SPRSURF(pipe));
 
-	dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
-
 	intel_update_sprite_watermarks(plane, crtc, 0, 0, false, false);
-
-	/* potentially re-enable LP watermarks */
-	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
-		intel_update_watermarks(crtc);
 }
 
 static int
-- 
1.8.3.2

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

[PATCH 13/14] drm/i915: Don't disable primary when color keying is used

[ville.syrjala]

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

When color keying is used, the primary may not be invisible even though
the sprite fully covers it. So check for color keying before deciding to
disable the primary plane.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 3ce328e..a872129 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -636,6 +636,15 @@ format_is_yuv(uint32_t format)
 	}
 }
 
+static bool colorkey_enabled(struct intel_plane *intel_plane)
+{
+	struct drm_intel_sprite_colorkey key;
+
+	intel_plane->get_colorkey(&intel_plane->base, &key);
+
+	return key.flags != I915_SET_COLORKEY_NONE;
+}
+
 static int
 intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
@@ -821,7 +830,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * If the sprite is completely covering the primary plane,
 	 * we can disable the primary and save power.
 	 */
-	disable_primary = drm_rect_equals(&dst, &clip);
+	disable_primary = drm_rect_equals(&dst, &clip) && !colorkey_enabled(intel_plane);
 	WARN_ON(disable_primary && !visible && intel_crtc->active);
 
 	mutex_lock(&dev->struct_mutex);
-- 
1.8.3.2

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

[PATCH 14/14] drm/i915: Avoid underruns when disabling sprites

[ville.syrjala]

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

As the watermark registers aren't double bufferd, clearing the
watermarks immediately after writing the sprite registers can be
hazardous.

Until we have something better, add a wait for vblank between the
two steps to make sure the sprite no longer needs the watermark
levels before we clear them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a872129..fe4de89 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -336,6 +336,12 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);
 	POSTING_READ(SPRSURF(pipe));
 
+	/*
+	 * Avoid underruns when disabling the sprite.
+	 * FIXME remove once watermark updates are done properly.
+	 */
+	intel_wait_for_vblank(dev, pipe);
+
 	intel_update_sprite_watermarks(plane, crtc, 0, 0, false, false);
 }
 
@@ -503,6 +509,12 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_MODIFY_DISPBASE(DVSSURF(pipe), 0);
 	POSTING_READ(DVSSURF(pipe));
 
+	/*
+	 * Avoid underruns when disabling the sprite.
+	 * FIXME remove once watermark updates are done properly.
+	 */
+	intel_wait_for_vblank(dev, pipe);
+
 	intel_update_sprite_watermarks(plane, crtc, 0, 0, false, false);
 }
 
-- 
1.8.3.2

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

Re: [PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[Daniel Vetter]

On Thu, Dec 5, 2013 at 2:51 PM,  <ville.syrjala@linux.intel.com> wrote:
> This series still has some underrun issues since the safe watermark
> update mechanism is yet to be introduced. However it shouldn't really
> be any worse than what we have ATM, and I'd like to have all the platforms
> converted so that we have more systems that will excercise the new update
> mechanism when it arrives.

How bad are the underrun occurences? I'd really like to make the
underrun reporting much louder asap ... If it only happens rarely when
enabling/disabling sprites then I think we should up them to
DRM_ERROR.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[Ville Syrjälä]

On Thu, Dec 05, 2013 at 03:30:33PM +0100, Daniel Vetter wrote:
> On Thu, Dec 5, 2013 at 2:51 PM,  <ville.syrjala@linux.intel.com> wrote:
> > This series still has some underrun issues since the safe watermark
> > update mechanism is yet to be introduced. However it shouldn't really
> > be any worse than what we have ATM, and I'd like to have all the platforms
> > converted so that we have more systems that will excercise the new update
> > mechanism when it arrives.
> 
> How bad are the underrun occurences? I'd really like to make the
> underrun reporting much louder asap ... If it only happens rarely when
> enabling/disabling sprites then I think we should up them to
> DRM_ERROR.

You can also get them when resizing the sprite. But since the current
logic will disable underrun reporting after the first occurence, making
them DRM_ERROR might be OK even now. I have such a patch in my tree
actually.

Once the safe watermark update mechanism is in place, I think we want
to make them even louder. I'm also using a patch that arms a timer to
re-enable the underrun reporting after 2 seconds of it getting disabled.
Although one extra problem is that we still don't properly check the
cdclk related scaling limitations, so it's currently possible to
configure the sprite(s) to cause constant underruns.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[Imre Deak]

On Thu, 2013-12-05 at 15:51 +0200, ville.syrjala@linux.intel.com wrote:
> Back on the watermark horse. This series moves ILK/SNB/IVB over to the HSW
> watermark code.
> 
> This series still has some underrun issues since the safe watermark
> update mechanism is yet to be introduced. However it shouldn't really
> be any worse than what we have ATM, and I'd like to have all the platforms
> converted so that we have more systems that will excercise the new update
> mechanism when it arrives.

Looks ok to me. I have a couple of bikesheds inlined, regardless of
those on the series:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> 
> Ville Syrjälä (14):
>       drm/i915: Add IVB DDB partitioning control
>       drm/i915: Add ILK/SNB/IVB WM latency field support
>       drm/i915: Avoid computing invalid WM levels when sprites/scaling is enabled
>       Revert "drm/i915/sprite: Always enable the scaler on IronLake"
>       drm/i915: Fix LP1 sprite watermarks for ILK/SNB
>       drm/i915: Fix LP1+ watermark disabling ILK
>       drm/i915: Don't merge LP1+ watermarks on ILK/SNB/IVB when multiple pipes are enabled
>       drm/i915: Disable FBC WM on ILK, and disable LP2+ when FBC is enabled
>       drm/i915: Linetime watermarks are a HSW feature
>       drm/i915: Disable LP1+ watermarks safely in init
>       drm/i915: Move ILK/SNB/IVB over to the HSW WM code
>       drm/i915: Try to fix the messy IVB sprite scaling workaround
>       drm/i915: Don't disable primary when color keying is used
>       drm/i915: Avoid underruns when disabling sprites
> 
>  drivers/gpu/drm/i915/i915_drv.h      |   1 -
>  drivers/gpu/drm/i915/i915_reg.h      |   2 +
>  drivers/gpu/drm/i915/intel_display.c |   2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 857 +++++++----------------------------
>  drivers/gpu/drm/i915/intel_sprite.c  |  50 +-
>  5 files changed, 195 insertions(+), 717 deletions(-)
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

Re: [PATCH 00/14] drm/i915: Make ILK/SNB/IVB use HSW watermark code

[Daniel Vetter]

On Mon, Dec 16, 2013 at 11:38:38PM +0200, Imre Deak wrote:
> On Thu, 2013-12-05 at 15:51 +0200, ville.syrjala@linux.intel.com wrote:
> > Back on the watermark horse. This series moves ILK/SNB/IVB over to the HSW
> > watermark code.
> > 
> > This series still has some underrun issues since the safe watermark
> > update mechanism is yet to be introduced. However it shouldn't really
> > be any worse than what we have ATM, and I'd like to have all the platforms
> > converted so that we have more systems that will excercise the new update
> > mechanism when it arrives.
> 
> Looks ok to me. I have a couple of bikesheds inlined, regardless of
> those on the series:

Yeah, that looks like good cleanups on top, but since Ville is already on
vacation I've decided to pull the patches in already.
-Daniel

> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > 
> > Ville Syrjälä (14):
> >       drm/i915: Add IVB DDB partitioning control
> >       drm/i915: Add ILK/SNB/IVB WM latency field support
> >       drm/i915: Avoid computing invalid WM levels when sprites/scaling is enabled
> >       Revert "drm/i915/sprite: Always enable the scaler on IronLake"
> >       drm/i915: Fix LP1 sprite watermarks for ILK/SNB
> >       drm/i915: Fix LP1+ watermark disabling ILK
> >       drm/i915: Don't merge LP1+ watermarks on ILK/SNB/IVB when multiple pipes are enabled
> >       drm/i915: Disable FBC WM on ILK, and disable LP2+ when FBC is enabled
> >       drm/i915: Linetime watermarks are a HSW feature
> >       drm/i915: Disable LP1+ watermarks safely in init
> >       drm/i915: Move ILK/SNB/IVB over to the HSW WM code
> >       drm/i915: Try to fix the messy IVB sprite scaling workaround
> >       drm/i915: Don't disable primary when color keying is used
> >       drm/i915: Avoid underruns when disabling sprites
> > 
> >  drivers/gpu/drm/i915/i915_drv.h      |   1 -
> >  drivers/gpu/drm/i915/i915_reg.h      |   2 +
> >  drivers/gpu/drm/i915/intel_display.c |   2 +-
> >  drivers/gpu/drm/i915/intel_pm.c      | 857 +++++++----------------------------
> >  drivers/gpu/drm/i915/intel_sprite.c  |  50 +-
> >  5 files changed, 195 insertions(+), 717 deletions(-)
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch