On Thu, Oct 22, 2015 at 03:37:33PM +0200, Daniel Vetter wrote:
> 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?
Yes.
> 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
