| Message ID | 1372185491-9688-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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); }