Re: [PATCH 2/5] drm/i915: Read FORCEWAKE registers with I915_READ_FW()

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Oct 22, 2015 at 03:34:57PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change FORCEWAKE & co. reads for the error state to use I915_READ_FW().
> Reading a FORCEWAKE register using a function that can frob forcewake
> just seems wrong.
> 
> There is a check to skip grabbing the forcewake for accessing FORCEWAKE
> in intel_uncore.c, but there's no such check for FORCEWAKE_MT. So no
> idea what is currently happening with FORCEWAKE_MT reads. FORCEWAKE_VLV
> is fortunately outside the forcewake range anyway, so no actual issue
> with that one.
> 
> So let's just make the rule that you can access FORCEWAKE registers with

s/can/cannot/ I presume? With that changed

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> the normal I915_READ() stuff, and we can drop the extra FORCEWAKE check
> from NEEDS_FORCEWAKE(). While at it use NEEDS_FORCEWAKE() on BDW, where
> it was skipped for whatever bikeshed reason that I've already forgotten.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 6 +++---
>  drivers/gpu/drm/i915/intel_uncore.c   | 5 ++---
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 2f04e4f..de86f26 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1181,7 +1181,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	if (IS_VALLEYVIEW(dev)) {
>  		error->gtier[0] = I915_READ(GTIER);
>  		error->ier = I915_READ(VLV_IER);
> -		error->forcewake = I915_READ(FORCEWAKE_VLV);
> +		error->forcewake = I915_READ_FW(FORCEWAKE_VLV);
>  	}
>  
>  	if (IS_GEN7(dev))
> @@ -1193,14 +1193,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	}
>  
>  	if (IS_GEN6(dev)) {
> -		error->forcewake = I915_READ(FORCEWAKE);
> +		error->forcewake = I915_READ_FW(FORCEWAKE);
>  		error->gab_ctl = I915_READ(GAB_CTL);
>  		error->gfx_mode = I915_READ(GFX_MODE);
>  	}
>  
>  	/* 2: Registers which belong to multiple generations */
>  	if (INTEL_INFO(dev)->gen >= 7)
> -		error->forcewake = I915_READ(FORCEWAKE_MT);
> +		error->forcewake = I915_READ_FW(FORCEWAKE_MT);
>  
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		error->derrmr = I915_READ(DERRMR);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8dfeac9..dca0979 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -513,8 +513,7 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
>  }
>  
>  /* We give fast paths for the really cool registers */
> -#define NEEDS_FORCE_WAKE(reg) \
> -	 ((reg) < 0x40000 && (reg) != FORCEWAKE)
> +#define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000)
>  
>  #define REG_RANGE(reg, start, end) ((reg) >= (start) && (reg) < (end))
>  
> @@ -918,7 +917,7 @@ static void \
>  gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
>  	GEN6_WRITE_HEADER; \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
> -	if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
> +	if (NEEDS_FORCE_WAKE(reg) && !is_gen8_shadowed(dev_priv, reg)) \
>  		__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
>  	__raw_i915_write##x(dev_priv, reg, val); \
>  	hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx