On Thu, Oct 22, 2015 at 03:34:57PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxx>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
