| Message ID | 20170102152845.32352-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 02, 2017 at 03:28:45PM +0000, Chris Wilson wrote: > Ville explained that the wakelock was being acquired during set-idle in > order to flush the voltage change from the punit. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com > --- > drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 4406359c5f81..4c9a1b12dfee 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5011,8 +5011,18 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > if (dev_priv->rps.cur_freq <= val) > return; > > - /* Wake up the media well, as that takes a lot less > - * power than the Render well. */ > + /* The punit delays the write of the frequency and voltage until it > + * determines the GPU is awake. During normal usage we don't want to > + * waste power changing the frequency if the GPU is sleeping (rc6). > + * However, the GPU and driver is now idle and we do not want to delay > + * switching to minimum voltage (reducing power whilst idle) as we do > + * not expect to be woken in the near future and so must flush the > + * change by waking the device. > + * > + * We choose to take the media powerwell (either would do to trick the > + * punit into commiting the voltage change) as that takes a lot less > + * power than the render powerwell. > + */ lgtm Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA); > valleyview_set_rps(dev_priv, val); > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA); > -- > 2.11.0
On Mon, Jan 02, 2017 at 05:56:51PM +0200, Ville Syrjälä wrote: > On Mon, Jan 02, 2017 at 03:28:45PM +0000, Chris Wilson wrote: > > Ville explained that the wakelock was being acquired during set-idle in > > order to flush the voltage change from the punit. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com > > --- > > drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 4406359c5f81..4c9a1b12dfee 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -5011,8 +5011,18 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) > > if (dev_priv->rps.cur_freq <= val) > > return; > > > > - /* Wake up the media well, as that takes a lot less > > - * power than the Render well. */ > > + /* The punit delays the write of the frequency and voltage until it > > + * determines the GPU is awake. During normal usage we don't want to > > + * waste power changing the frequency if the GPU is sleeping (rc6). > > + * However, the GPU and driver is now idle and we do not want to delay > > + * switching to minimum voltage (reducing power whilst idle) as we do > > + * not expect to be woken in the near future and so must flush the > > + * change by waking the device. > > + * > > + * We choose to take the media powerwell (either would do to trick the > > + * punit into commiting the voltage change) as that takes a lot less > > + * power than the render powerwell. > > + */ > > lgtm > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Ta, fixed a spelling mistake and pushed. -Chris
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4406359c5f81..4c9a1b12dfee 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5011,8 +5011,18 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) if (dev_priv->rps.cur_freq <= val) return; - /* Wake up the media well, as that takes a lot less - * power than the Render well. */ + /* The punit delays the write of the frequency and voltage until it + * determines the GPU is awake. During normal usage we don't want to + * waste power changing the frequency if the GPU is sleeping (rc6). + * However, the GPU and driver is now idle and we do not want to delay + * switching to minimum voltage (reducing power whilst idle) as we do + * not expect to be woken in the near future and so must flush the + * change by waking the device. + * + * We choose to take the media powerwell (either would do to trick the + * punit into commiting the voltage change) as that takes a lot less + * power than the render powerwell. + */ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA); valleyview_set_rps(dev_priv, val); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA);
Ville explained that the wakelock was being acquired during set-idle in order to flush the voltage change from the punit. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)