[Intel-gfx] [PATCH] drm/i915: allow 945 to control self refresh (CxSR) automatically

[Chris Wilson]

From: Alexander Lam <lambchop468 at gmail.com>

I changed 945's self refresh to work without the need for the driver to
enable/disable self refresh manually based on the idle state of the gpu.
This is much better than enabling/disabling self refresh for various
reasons, including staying in a lower power state for more time and
avoiding the need for cpu cycles.

This was originally done manually to workaround issues with the hardware
hanging. However, since 944001201: drm/i915: enable low power render
writes on GEN3 hardware, automatic CxSR seems stable.

Signed-off-by: Alexander Lam <lambchop468 at gmail.com>
Acked-by : Li Peng <peng.li at linux.intel.com>
[ickle: play safe with the ordering and disable CxSR before tweaking any
watermark and enable afterwards.]
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   62 +++++++++++++---------------------
 1 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 14ac352..09d3eaa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3316,7 +3316,7 @@ static void i9xx_update_wm(struct drm_device *dev, int planea_clock,
 	int planea_wm, planeb_wm;
 	struct intel_watermark_params planea_params, planeb_params;
 	unsigned long line_time_us;
-	int sr_clock, sr_entries = 0;
+	int sr_clock, sr_entries = 0, sr_enabled = 0;
 
 	/* Create copies of the base settings for each pipe */
 	if (IS_CRESTLINE(dev) || IS_I945GM(dev))
@@ -3345,6 +3345,12 @@ static void i9xx_update_wm(struct drm_device *dev, int planea_clock,
 	 */
 	cwm = 2;
 
+	/* Play safe and disable self-refresh before adjusting watermarks. */
+	if (IS_I945G(dev) || IS_I945GM(dev))
+		I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | 0);
+	else if (IS_I915GM(dev))
+		I915_WRITE(INSTPM, I915_READ(INSTPM) & ~INSTPM_SELF_EN);
+
 	/* Calc sr entries for one plane configs */
 	if (HAS_FW_BLC(dev) && sr_hdisplay &&
 	    (!planea_clock || !planeb_clock)) {
@@ -3364,20 +3370,12 @@ static void i9xx_update_wm(struct drm_device *dev, int planea_clock,
 			srwm = 1;
 
 		if (IS_I945G(dev) || IS_I945GM(dev))
-			I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
-		else if (IS_I915GM(dev)) {
-			/* 915M has a smaller SRWM field */
+			I915_WRITE(FW_BLC_SELF,
+				   FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
+		else if (IS_I915GM(dev))
 			I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
-			I915_WRITE(INSTPM, I915_READ(INSTPM) | INSTPM_SELF_EN);
-		}
-	} else {
-		/* Turn off self refresh if both pipes are enabled */
-		if (IS_I945G(dev) || IS_I945GM(dev)) {
-			I915_WRITE(FW_BLC_SELF, I915_READ(FW_BLC_SELF)
-				   & ~FW_BLC_SELF_EN);
-		} else if (IS_I915GM(dev)) {
-			I915_WRITE(INSTPM, I915_READ(INSTPM) & ~INSTPM_SELF_EN);
-		}
+
+		sr_enabled = 1;
 	}
 
 	DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
@@ -3392,6 +3390,16 @@ static void i9xx_update_wm(struct drm_device *dev, int planea_clock,
 
 	I915_WRITE(FW_BLC, fwater_lo);
 	I915_WRITE(FW_BLC2, fwater_hi);
+
+	if (sr_enabled) {
+		if (IS_I945G(dev) || IS_I945GM(dev))
+			I915_WRITE(FW_BLC_SELF,
+				   FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN);
+		else if (IS_I915GM(dev))
+			I915_WRITE(INSTPM, I915_READ(INSTPM) | INSTPM_SELF_EN);
+		DRM_DEBUG_KMS("memory self refresh enabled\n");
+	} else
+		DRM_DEBUG_KMS("memory self refresh disabled\n");
 }
 
 static void i830_update_wm(struct drm_device *dev, int planea_clock, int unused,
@@ -5132,7 +5140,6 @@ static void intel_idle_update(struct work_struct *work)
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
-	int enabled = 0;
 
 	if (!i915_powersave)
 		return;
@@ -5146,16 +5153,11 @@ static void intel_idle_update(struct work_struct *work)
 		if (!crtc->fb)
 			continue;
 
-		enabled++;
 		intel_crtc = to_intel_crtc(crtc);
 		if (!intel_crtc->busy)
 			intel_decrease_pllclock(crtc);
 	}
 
-	if ((enabled == 1) && (IS_I945G(dev) || IS_I945GM(dev))) {
-		DRM_DEBUG_DRIVER("enable memory self refresh on 945\n");
-		I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN);
-	}
 
 	mutex_unlock(&dev->struct_mutex);
 }
@@ -5180,17 +5182,9 @@ void intel_mark_busy(struct drm_device *dev, struct drm_i915_gem_object *obj)
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
 
-	if (!dev_priv->busy) {
-		if (IS_I945G(dev) || IS_I945GM(dev)) {
-			u32 fw_blc_self;
-
-			DRM_DEBUG_DRIVER("disable memory self refresh on 945\n");
-			fw_blc_self = I915_READ(FW_BLC_SELF);
-			fw_blc_self &= ~FW_BLC_SELF_EN;
-			I915_WRITE(FW_BLC_SELF, fw_blc_self | FW_BLC_SELF_EN_MASK);
-		}
+	if (!dev_priv->busy)
 		dev_priv->busy = true;
-	} else
+	else
 		mod_timer(&dev_priv->idle_timer, jiffies +
 			  msecs_to_jiffies(GPU_IDLE_TIMEOUT));
 
@@ -5202,14 +5196,6 @@ void intel_mark_busy(struct drm_device *dev, struct drm_i915_gem_object *obj)
 		intel_fb = to_intel_framebuffer(crtc->fb);
 		if (intel_fb->obj == obj) {
 			if (!intel_crtc->busy) {
-				if (IS_I945G(dev) || IS_I945GM(dev)) {
-					u32 fw_blc_self;
-
-					DRM_DEBUG_DRIVER("disable memory self refresh on 945\n");
-					fw_blc_self = I915_READ(FW_BLC_SELF);
-					fw_blc_self &= ~FW_BLC_SELF_EN;
-					I915_WRITE(FW_BLC_SELF, fw_blc_self | FW_BLC_SELF_EN_MASK);
-				}
 				/* Non-busy -> busy, upclock */
 				intel_increase_pllclock(crtc);
 				intel_crtc->busy = true;
-- 
1.7.2.3