[Intel-gfx] [PATCH] drm/i915: enable memory self refresh on 945GM

[Fu Michael]

Li Peng wrote:
> I did some test and found that memory self refresh on 945GM
> isn't really enabled in commit 652c39. On 945, we need to use
> 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.
>
> This patch enables memory self refresh on 945GM when graphics
> is idle, and disable self refresh when it is busy. It can save
> about 0.2W idle power on the netbook with 945GSE chipset.
>
> Signed-off-by: Li Peng <peng.li at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    4 +++-
>  drivers/gpu/drm/i915/intel_display.c |   23 ++++++++++++++++++++++-
>  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 1687edf..93b5077 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -301,7 +301,9 @@
>  #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)
> +#define   FW_BLC_SELF_EN           (1<<15)
>  #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 3ba6546..9b96439 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2580,7 +2580,10 @@ 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_I945GM(dev))
> +			I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_FIFO_MASK | (srwm & 0xff));
> +		else
> +			I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN | (srwm & 0x3f));
>  	}
>  
>  	DRM_DEBUG("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
>   
915GM's SR_EN bit is in INSTPM:12 rather than in FW_BLC_SELF reg.

There seems no need to explicitly disable SR bit. system should be able 
to use it automatically when condition is met. Do you see any issue 
without below hunk?

> @@ -3671,11 +3674,23 @@ static void intel_gpu_idle_timer(unsigned long arg)
>  void intel_increase_renderclock(struct drm_device *dev, bool schedule)
>  {
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> +	u32 fw_blc_self;
> +	int need_schedule = 0;
>  
>  	if (IS_IGDNG(dev))
>  		return;
>  
> +	if (IS_I945GM(dev)) {
> +		DRM_DEBUG("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);
> +		need_schedule = 1;
> +	}
> +
>  	if (!dev_priv->render_reclock_avail) {
> +		if (need_schedule)
> +			goto schedule_timer;
>  		DRM_DEBUG("not reclocking render clock\n");
>  		return;
>  	}
> @@ -3688,6 +3703,7 @@ void intel_increase_renderclock(struct drm_device *dev, bool schedule)
>  	DRM_DEBUG("increasing render clock frequency\n");
>  
>  	/* Schedule downclock */
> +schedule_timer:
>  	if (schedule)
>  		mod_timer(&dev_priv->idle_timer, jiffies +
>  			  msecs_to_jiffies(GPU_IDLE_TIMEOUT));
> @@ -3700,6 +3716,11 @@ void intel_decrease_renderclock(struct drm_device *dev)
>  	if (IS_IGDNG(dev))
>  		return;
>  
> +	if (IS_I945GM(dev)) {
> +		DRM_DEBUG("enable memory self refresh on 945\n");
> +		I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN);
> +	}
> +
>  	if (!dev_priv->render_reclock_avail) {
>  		DRM_DEBUG("not reclocking render clock\n");
>  		return;
>