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

Ville Syrjälä
2014-06-13T11:57:31Z

On Fri, Jun 13, 2014 at 02:33:44PM +0300, Ville Syrjälä wrote:
> 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.

Maybe this is all we need?

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3768199..fabd852 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4530,7 +4530,7 @@ i915_gem_suspend(struct drm_device *dev)

del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
cancel_delayed_work_sync(&dev_priv->mm.retire_work);
- cancel_delayed_work_sync(&dev_priv->mm.idle_work);
+ flush_delayed_work(&dev_priv->mm.idle_work);

return 0;

--
Ville Syrjälä
Intel OTC