Re: [PATCH] drm/i915: Drop WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jun 13, 2014 at 03:46:14PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Workaround fixed in BYT. Forcing Gfx clk up not needed, and Requesting the
> min freq should bring bring the voltage Vnn.
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 40 +---------------------------------------
>  1 file changed, 1 insertion(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0b088fe..9aee28b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3198,44 +3198,6 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>  	trace_intel_gpu_freq_change(val * 50);
>  }
>  
> -/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
> - *
> - * * If Gfx is Idle, then
> - * 1. Mask Turbo interrupts
> - * 2. Bring up Gfx clock
> - * 3. Change the freq to Rpn and wait till P-Unit updates freq
> - * 4. Clear the Force GFX CLK ON bit so that Gfx can down
> - * 5. Unmask Turbo interrupts
> -*/
> -static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> -{
> -	/*
> -	 * When we are idle.  Drop to min voltage state.
> -	 */
> -
> -	if (dev_priv->rps.cur_freq <= dev_priv->rps.min_freq_softlimit)
> -		return;
> -
> -	/* Mask turbo interrupt so that they will not come in between */
> -	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> -
> -	vlv_force_gfx_clock(dev_priv, true);
> -
> -	dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
> -
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
> -					dev_priv->rps.min_freq_softlimit);
> -
> -	if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
> -				& GENFREQSTATUS) == 0, 5))
> -		DRM_ERROR("timed out waiting for Punit\n");
> -
> -	vlv_force_gfx_clock(dev_priv, false);
> -
> -	I915_WRITE(GEN6_PMINTRMSK,
> -		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> -}
> -
>  void gen6_rps_idle(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -3243,7 +3205,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	if (dev_priv->rps.enabled) {
>  		if (IS_VALLEYVIEW(dev))
> -			vlv_set_rps_idle(dev_priv);
> +			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);

This should take care of https://bugs.freedesktop.org/show_bug.cgi?id=75244

I don't know when the hardware got fixed so I'm hesitant to r-b it, but
at least my C0 works fine without this stuff, so:
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

However to avoid future mishaps I think we should have some kind of a
comment before the valleyview_set_rps() call to let the reader know that
we really need this on VLV to drop the voltage.

Also it now occurs to me that we might be leaving the GPU frequency (and
thus Vnn) high during a system suspend. I think we need an explicit
rps_idle() call in the suspend path somewhere. Runtime suspend should be
fine already since it depends on intel_mark_idle() getting called before
the last rpm reference is dropped.

>  		else
>  			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>  
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC