[PATCH] drm/i915: Update comment in vlv_set_rps_idle()

[PATCH] drm/i915: Update comment in vlv_set_rps_idle()

[Chris Wilson]

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.
+	 */
 	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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Update comment in vlv_set_rps_idle()

[Patchwork]

== Series Details ==

Series: drm/i915: Update comment in vlv_set_rps_idle()
URL   : https://patchwork.freedesktop.org/series/17375/
State : warning

== Summary ==

Series 17375v1 drm/i915: Update comment in vlv_set_rps_idle()
https://patchwork.freedesktop.org/api/1.0/series/17375/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-snb-2520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-snb-2520m     total:246  pass:214  dwarn:1   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

847638b14a477e423033b31d01483bf15e9a8d4f drm-tip: 2017y-01m-02d-13h-23m-55s UTC integration manifest
b0fe6928 drm/i915: Update comment in vlv_set_rps_idle()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3420/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Update comment in vlv_set_rps_idle()

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Update comment in vlv_set_rps_idle()

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx