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

From: Daniel Vetter <daniel@ffwll.ch>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Use wait_for() to wait for Punit to change GPU freq on VLV
Date: Tue, 2 Jul 2013 16:01:33 +0200	[thread overview]
Message-ID: <20130702140133.GS18285@phenom.ffwll.local> (raw)
In-Reply-To: <20130626092625.60a08794@jbarnes-desktop>

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>

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