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

[Li Peng]

On Wed, 2009-12-02 at 13:58 +0800, ykzhao wrote:
> On Wed, 2009-12-02 at 21:29 +0800, Li Peng wrote:
> > On Wed, 2009-12-02 at 11:11 +0800, Fu Michael wrote:
> > > 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.
> > > 
> > Thanks for pointing out, I will fix it.
> 
> It seems that we should also use the FIFO_MASK/FIFO_EN on 945G platform.
> 

Yeah, should add 945G.

> 
> > > 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?
> > 
> > I have tried that, just enabling SR bit and setting srwm in
> > i9xx_update_wm(), but X got frozen after boot, so I add the 
> > code to disable SR explicitly. Maybe H/W isn't that smart :)
> > 
> 
> Do you mean that the system can't be booted correctly if we enable the
> SR bit in i9xx_update_wm?
> 
> How about setting the another self-refresh watermark?

System boot correctly, I got screen freeze immediately after booting
into desktop, can't click anything, but VT switch and system network
works. 

how to calculate another self-refresh watermark ? I tried some random
value, doesn't help.

> > > 
> > > > @@ -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;
> > > >   
> > > 
> > 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>