On Tue, Feb 18, 2014 at 07:10:24PM +0200, Mika Kuoppala wrote: > Sometimes generic driver code gets forcewake explicitly by > gen6_gt_force_wake_get(), which check forcewake_count before accessing > hardware. However the register access with gen8_write function access > low level hw accessors directly, ignoring the forcewake_count. This > leads to nested forcewake get from hardware, in ring init and possibly > elsewhere, causing forcewake ack clear errors and/or hangs. > > Fix this by checking the forcewake count also in gen8_write > > v2: Read side doesn't care about shadowed registers, > Remove __needs_put funkiness from gen8_write. (Ville) > Improved commit message. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=74007 > Signed-off-by: Mika Kuoppala <[email protected]> > Cc: Ben Widawsky <[email protected]> > Cc: Ville Syrjälä <[email protected]> > Signed-off-by: Mika Kuoppala <[email protected]>
Thanks for finding this. I'd been meaning to track down the extra "Timed out" messages. I do wonder how this actually fixes a hang. Do you have any idea? Reviewed-by: Ben Widawsky <[email protected]> > --- > drivers/gpu/drm/i915/intel_uncore.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index c628414..d1e9d63 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -634,16 +634,17 @@ static bool is_gen8_shadowed(struct drm_i915_private > *dev_priv, u32 reg) > #define __gen8_write(x) \ > static void \ > gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool > trace) { \ > - bool __needs_put = reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg); \ > REG_WRITE_HEADER; \ > - if (__needs_put) { \ > - dev_priv->uncore.funcs.force_wake_get(dev_priv, \ > - FORCEWAKE_ALL); \ > - } \ > - __raw_i915_write##x(dev_priv, reg, val); \ > - if (__needs_put) { \ > - dev_priv->uncore.funcs.force_wake_put(dev_priv, \ > - FORCEWAKE_ALL); \ > + if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) { \ > + if (dev_priv->uncore.forcewake_count == 0) \ > + dev_priv->uncore.funcs.force_wake_get(dev_priv, \ > + FORCEWAKE_ALL); \ > + __raw_i915_write##x(dev_priv, reg, val); \ > + if (dev_priv->uncore.forcewake_count == 0) \ > + dev_priv->uncore.funcs.force_wake_put(dev_priv, \ > + FORCEWAKE_ALL); \ > + } else { \ > + __raw_i915_write##x(dev_priv, reg, val); \ > } \ > REG_WRITE_FOOTER; \ > } > -- > 1.7.9.5 > -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
