[Intel-gfx] [PATCH 1/2] drm/i915: align vlv forcewake with common lore

[Ben Widawsky]

On Fri, 24 Aug 2012 17:26:20 +0200
Daniel Vetter <daniel.vetter at 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 at intel.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter at 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.

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



> ---
>  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?

>  
> -	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