Re: [RFC][PATCH 1/2] drm/i915: Don't increase the GPU frequency from the delayed VLV rps timer

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 25, 2013 at 11:53:28AM -0700, Jesse Barnes wrote:
> On Tue, 25 Jun 2013 21:38:10 +0300
> ville.syrjala@linux.intel.com wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > There's little point in increasing the GPU frequency from the delayed
> > rps work on VLV. Now when the GPU is idle, the GPU frequency actually
> > keeps dropping gradually until it hits the minimum, whereas previously
> > it just ping-ponged constantly between RPe and RPe-1.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 96cfb3e..eaf0fa2 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3464,7 +3464,8 @@ static void vlv_rps_timer_work(struct work_struct *work)
> >  	 * min freq available.
> >  	 */
> >  	mutex_lock(&dev_priv->rps.hw_lock);
> > -	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
> > +	if (dev_priv->rps.cur_delay > dev_priv->rps.rpe_delay)
> > +		valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
> >  	mutex_unlock(&dev_priv->rps.hw_lock);
> >  }
> >  
> 
> Hm that's not what I saw here; I stopped getting the timer interrupts
> when the GPU went idle.  But that could be explained by punit fw
> differences.
> 
> So this change looks ok to me.
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Queued for -next, thanks for the patch. In case you noodle around in here
some more, I'd also vote for a s/timer_work/delayed_work/ color change:
The execution enviroment of a time is _much_ different than from a work,
so sticking to consistent naming helps in reading code.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch