Re: [PATCH 3/3] drm/i915: Never read FORCEWAKE

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 24, 2012 at 12:31:11PM -0700, Ben Widawsky wrote:
> The same designer from the previous patch has told us to never read
> FORCEWAKE. We only do this for the POSTING_READ(), so simply change that
> to something within the same cacheline (for no reason in particular
> other than it sounds nice). In the _mt case we can leverage
> the gtfifodbg check for the POSTING_READ.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

This partially reverts

commit 6af2d180f82151cf3d58952e35a4f96e45bc453a
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jul 26 16:24:50 2012 +0200

    drm/i915: fix forcewake related hangs on snb

I guess the commit message should mention that. Also, please add a comment
it the _put functions that the check_fifodbg serves as a posting read (and
that we have experimental evidence suggesting that we should readback
something "nearby").

btw, can you please ask your nice designer whether he has any clue whether
that "nearby" posting_read has an effect?

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2a8468d..83ec02c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3975,7 +3975,7 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE, 1);
> -	POSTING_READ(FORCEWAKE);
> +	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
>  
>  	if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
>  			    FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -3998,7 +3998,7 @@ static void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
>  		DRM_ERROR("Force wake wait timed out\n");
>  
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_ENABLE(1));
> -	POSTING_READ(FORCEWAKE_MT);
> +	POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
>  
>  	if (wait_for_atomic((I915_READ_NOTRACE(forcewake_ack) & 1),
>  			    FORCEWAKE_ACK_TIMEOUT_MS))
> @@ -4035,14 +4035,12 @@ void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
>  static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE, 0);
> -	POSTING_READ(FORCEWAKE);
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
>  static void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
>  {
>  	I915_WRITE_NOTRACE(FORCEWAKE_MT, _MASKED_BIT_DISABLE(1));
> -	POSTING_READ(FORCEWAKE_MT);
>  	gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> -- 
> 1.7.12
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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