| Message ID | 1457120584-26080-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2016-03-04 at 21:43 +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we set the initial GPU frequency to min_freq_softlimit > on gen9, and to efficient_freq on VLV/CHV. On all the other platforms > we set it to idle_freq. Let's use idle_freq across the board to make > sure we don't waste power. This is especially relevant for VLV since > Vnn won't drop to minimum unless the GPU is at the minimum frequency. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Yes, I think having the same logic on all platforms make sense, so: Reviewed-by: Imre Deak <imre.deak@intel.com> I noticed that rps.min_freq is always the same as rps.idle_freq, what's the reason to have both, could we remove one of them? Adding Chris. > --- > drivers/gpu/drm/i915/intel_pm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index c53b8c4d381c..2591d533a895 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4804,7 +4804,7 @@ static void gen9_enable_rps(struct drm_device *dev) > * Up/Down EI & threshold registers, as well as the RP_CONTROL, > * RP_INTERRUPT_LIMITS & RPNSWREQ registers */ > dev_priv->rps.power = HIGH_POWER; /* force a reset */ > - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit); > + gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); > > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > } > @@ -5594,10 +5594,10 @@ static void cherryview_enable_rps(struct drm_device *dev) > dev_priv->rps.cur_freq); > > DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", > - intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq), > - dev_priv->rps.efficient_freq); > + intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq), > + dev_priv->rps.idle_freq); > > - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq); > + valleyview_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); > > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > } > @@ -5684,10 +5684,10 @@ static void valleyview_enable_rps(struct drm_device *dev) > dev_priv->rps.cur_freq); > > DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", > - intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq), > - dev_priv->rps.efficient_freq); > + intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq), > + dev_priv->rps.idle_freq); > > - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq); > + valleyview_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); > > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); > }
On Wed, Mar 16, 2016 at 07:56:58PM +0200, Imre Deak wrote: > On Fri, 2016-03-04 at 21:43 +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Currently we set the initial GPU frequency to min_freq_softlimit > > on gen9, and to efficient_freq on VLV/CHV. On all the other platforms > > we set it to idle_freq. Let's use idle_freq across the board to make > > sure we don't waste power. This is especially relevant for VLV since > > Vnn won't drop to minimum unless the GPU is at the minimum frequency. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Yes, I think having the same logic on all platforms make sense, so: > Reviewed-by: Imre Deak <imre.deak@intel.com> > > I noticed that rps.min_freq is always the same as rps.idle_freq, what's > the reason to have both, could we remove one of them? Adding Chris. They have two different meanings, one is the hardware minimum and the other is the frequency to use at idle. As indicated by vlv, it was not always clear if the HW engineers thought it was a good idea to rest at minimum and so having a separate control knob would be useful. -Chris
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c53b8c4d381c..2591d533a895 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4804,7 +4804,7 @@ static void gen9_enable_rps(struct drm_device *dev) * Up/Down EI & threshold registers, as well as the RP_CONTROL, * RP_INTERRUPT_LIMITS & RPNSWREQ registers */ dev_priv->rps.power = HIGH_POWER; /* force a reset */ - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit); + gen6_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); } @@ -5594,10 +5594,10 @@ static void cherryview_enable_rps(struct drm_device *dev) dev_priv->rps.cur_freq); DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", - intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq), - dev_priv->rps.efficient_freq); + intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq), + dev_priv->rps.idle_freq); - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq); + valleyview_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); } @@ -5684,10 +5684,10 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.cur_freq); DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", - intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq), - dev_priv->rps.efficient_freq); + intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq), + dev_priv->rps.idle_freq); - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq); + valleyview_set_rps(dev_priv->dev, dev_priv->rps.idle_freq); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); }