[Intel-gfx] [PATCH resend] drm/i915: enable memory self refresh on 9xx

[Li Peng]

On Mon, Jan 25, 2010 at 09:02:24AM -0800, Eric Anholt wrote:
> On Tue, 19 Jan 2010 21:32:57 +0800, Li Peng <peng.li at linux.intel.com> wrote:
> > Enabling memory self refresh (SR) on 9xx needs to set additional
> > register bits. On 945, we need bit 31 of FW_BLC_SELF to enable the
> > write to self refresh bit and bit 16 to enable the write of self
> > refresh watermark. On 915, bit 12 of INSTPM is used to enable SR.
> > 
> > SR will take effect when CPU enters C3+ state and its entry/exit
> > should be automatically controlled by H/W, driver only needs to set
> > SR enable bits in wm update. But this isn't safe in my test on 945
> > because GPU is hung. So this patch explicitly enables SR when GPU
> > is idle, and disables SR when it is busy. In my test on a netbook of
> > 945GSE chipset, it saves about 0.8W idle power.
> > 
> > Signed-off-by: Li Peng <peng.li at intel.com>
> 
> Applied to -next.  Thanks!

Hi, Eric

The patch you committed at
http://git.kernel.org/?p=linux/kernel/git/anholt/drm-intel.git;a=commit;h=72ddbd39f39b3cc5f1f661f97e67f2a497c4d57a
is an old version. It still has a bug that may cause GPU hang in my test. Because
that so far we have two idle timers, one is gpu idle timer and the other is 
crtc idle timer, we should disable SR when either of them changing to busy, 
not just for crtc idle timer. Here is the update version. Please review.

>From 7373cc1452c551f7c3ef7523483d76d2dd16373e Mon Sep 17 00:00:00 2001
From: Li Peng <peng.li at intel.com>
Date: Tue, 19 Jan 2010 02:13:00 -0500
Subject: drm/i915: enable memory self refresh on 9xx

Enabling memory self refresh (SR) on 9xx needs to set additional
register bits. On 945, we need bit 31 of FW_BLC_SELF to enable the
write to self refresh bit and bit 16 to enable the write of self
refresh watermark. On 915, bit 12 of INSTPM is used to enable SR.

SR will take effect when CPU enters C3+ state and its entry/exit
should be automatically controlled by H/W, driver only needs to set
SR enable bits in wm update. But this isn't safe in my test on 945
because GPU is hung. So this patch explicitly enables SR when GPU
is idle, and disables SR when it is busy. In my test on a netbook of
945GSE chipset, it saves about 0.8W idle power.

Signed-off-by: Li Peng <peng.li at intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    5 ++++-
 drivers/gpu/drm/i915/intel_display.c |   22 +++++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 847006c..3921e5a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -306,11 +306,14 @@
 #define   I915_ERROR_MEMORY_REFRESH			(1<<1)
 #define   I915_ERROR_INSTRUCTION			(1<<0)
 #define INSTPM	        0x020c0
+#define   INSTPM_SELF_EN (1<<12) /* 915GM only */
 #define ACTHD	        0x020c8
 #define FW_BLC		0x020d8
 #define FW_BLC2	 	0x020dc
 #define FW_BLC_SELF	0x020e0 /* 915+ only */
-#define   FW_BLC_SELF_EN (1<<15)
+#define   FW_BLC_SELF_EN_MASK      (1<<31)
+#define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
+#define   FW_BLC_SELF_EN           (1<<15) /* 945 only */
 #define MM_BURST_LENGTH     0x00700000
 #define MM_FIFO_WATERMARK   0x0001F000
 #define LM_BURST_LENGTH     0x00000700
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45da78e..d042955 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2629,7 +2629,13 @@ static void i9xx_update_wm(struct drm_device *dev, int planea_clock,
 		srwm = total_size - sr_entries;
 		if (srwm < 0)
 			srwm = 1;
-		I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN | (srwm & 0x3f));
+		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, srwm & 0x3f);
+			I915_WRITE(INSTPM, I915_READ(INSTPM) | INSTPM_SELF_EN);
+		}
 	}
 
 	DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
@@ -3879,6 +3885,11 @@ static void intel_idle_update(struct work_struct *work)
 
 	mutex_lock(&dev->struct_mutex);
 
+	if (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);
+	}
+
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		/* Skip inactive CRTCs */
 		if (!crtc->fb)
@@ -3912,6 +3923,15 @@ void intel_mark_busy(struct drm_device *dev, struct drm_gem_object *obj)
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
 
+	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
-- 
1.6.1.3