Re: [PATCH] drm/i915: Use wait_for() to wait for Punit to change GPU freq on VLV

Daniel Vetter
2013-07-02T13:41:26Z

On Wed, Jun 26, 2013 at 09:26:25AM -0700, Jesse Barnes wrote:
> On Wed, 26 Jun 2013 17:43:24 +0300
> ville.syrjala@linux.intel.com wrote:
>
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Use wait_for() instead of the open coded loop to avoid spreading the
> > same old timeout related bugs.
> >
> > This changes the loop to use msleep(1) instead of udelay(10) when the
> > Punit had not yet completed the frequency change. In practice that
> > doesn't seem to hurt performance as the Punit appears to be ready pretty
> > much always.
> >
> > Also give the status bit a name, instead of using the magic number 1.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
> > 2 files changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 10ac3d5..d5199a3 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -363,6 +363,7 @@
> > #define PUNIT_REG_GPU_LFM 0xd3
> > #define PUNIT_REG_GPU_FREQ_REQ 0xd4
> > #define PUNIT_REG_GPU_FREQ_STS 0xd8
> > +#define GENFREQSTATUS (1<<0)
> > #define PUNIT_REG_MEDIA_TURBO_FREQ_REQ 0xdc
> >
> > #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index aa48fc6..bff5709 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3074,19 +3074,12 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> > */
> > static void vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv)
> > {
> > - unsigned long timeout = jiffies + msecs_to_jiffies(10);
> > u32 pval;
> >
> > WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> >
> > - do {
> > - pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > - if (time_after(jiffies, timeout)) {
> > - DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> > - break;
> > - }
> > - udelay(10);
> > - } while (pval & 1);
> > + if (wait_for(((pval = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) & GENFREQSTATUS) == 0, 10))
> > + DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
> >
> > pval >>= 8;
> >
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch