Re: [PATCH 1/2] drm/i915: align vlv forcewake with common lore

From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	"Widawsky, Benjamin" <benjamin.widawsky@intel.com>
Subject: Re: [PATCH 1/2] drm/i915: align vlv forcewake with common lore
Date: Mon, 3 Sep 2012 09:30:59 +0200	[thread overview]
Message-ID: <20120903073059.GA5225@phenom.ffwll.local> (raw)
In-Reply-To: <20120902154339.1ba688e7@bwidawsk.net>

On Sun, Sep 02, 2012 at 03:43:39PM -0700, Ben Widawsky wrote:
> On Fri, 24 Aug 2012 17:26:20 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > For some odd reasons, the vlv forcewake code is rather different from
> > all other platforms, with no clear justification. Adjust things:
> > 
> > - Don't check whether the gt is awake already (and bail out early), we
> >   need to grab a forcewake anyway. Otherwise the chip might go to
> >   sleep too early. And this would also screw up our forcewake
> >   accounting.
> > - Like all other platforms, check whether the gt has cleared the
> >   forcewake bit in the _ACK register before setting it again.
> > - Use _MASKED_BIT_ENABLE/DISABLE macros
> > - Only use bit0 of the forcewake reg, not all 16 bits.
> > - check the gtfifodb reg like on all other platforms in _put.
> > - Drop the POSTING_READs for consistency.
> > 
> > v2: Failure to git add ... again.
> > 
> > v3: Fixup the spelling fail a bit.
> > 
> > Tested-by: "Purushothaman, Vijay A" <vijay.a.purushothaman@intel.com>
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> I'd like to see the FORCEWAKE patches I've recently submitted get
> merged, and the same magic in those get applied here.
> 
> Regarding docs, I can't find the right ones to verify any of the
> forcewake stuff, so no r-b. Maybe Jesse can help. For an a-b I'd like
> you to take the forcewake fixes, and considering squashing the vlv
> specific functions into the main functions, since you've removed most of
> the differences anyway.

I've figured I'll just apply it and let Jesse yell it at me ;-) After all,
this patch has been floating for a few weeks already in some form or
another, and seems to no break 2 of the 3 vlv machines we have access too
...

> In any case, it is:
> Tested-by: Ben Widawsky <ben@bwidawsk.net>

Thanks, I've merged both patches for -next.
-Daniel

> 
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c |   14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index c0721ff..1a197da 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4075,12 +4075,10 @@ int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> >  
> >  static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> >  {
> > -	/* Already awake? */
> > -	if ((I915_READ(0x130094) & 0xa1) == 0xa1)
> > -		return;
> > +	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1) == 0, 500))
> > +		DRM_ERROR("Force wake wait timed out\n");
> 
> WTF?

I hope that wtf is for the old code, not the new ...

> 
> >  
> > -	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffffffff);
> > -	POSTING_READ(FORCEWAKE_VLV);
> > +	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_ENABLE(1));
> >  
> >  	if (wait_for_atomic_us((I915_READ_NOTRACE(FORCEWAKE_ACK_VLV) & 1), 500))
> >  		DRM_ERROR("Force wake wait timed out\n");
> > @@ -4090,9 +4088,9 @@ static void vlv_force_wake_get(struct drm_i915_private *dev_priv)
> >  
> >  static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> >  {
> > -	I915_WRITE_NOTRACE(FORCEWAKE_VLV, 0xffff0000);
> > -	/* FIXME: confirm VLV behavior with Punit folks */
> > -	POSTING_READ(FORCEWAKE_VLV);
> > +	I915_WRITE_NOTRACE(FORCEWAKE_VLV, _MASKED_BIT_DISABLE(1));
> > +	/* The below doubles as a POSTING_READ */
> > +	gen6_gt_check_fifodbg(dev_priv);
> >  }
> >  
> >  void intel_gt_init(struct drm_device *dev)
> 
> 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48