On Mon, Apr 10, 2017 at 04:55:31PM +0100, Chris Wilson wrote: > We acquire the forcewake and use I915_READ_FW instead for the atomic > wait within intel_uncore_wait_for_register. However, this still leaves > us vulnerable to concurrent mmio access to the register, which can cause > system hangs on gen7. The protection is to acquire uncore.lock around > each register, so lets add it back. > > v2: Wrap __intel_wait_for_register_fw() to re-use its atomic wait_for > loop and spare adding another for ourselves. > > Signed-off-by: Chris Wilson <[email protected]> > Cc: Michal Wajdeczko <[email protected]> > Cc: Joonas Lahtinen <[email protected]> > --- > drivers/gpu/drm/i915/intel_uncore.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 53c8457869f6..01cea3b7a704 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1661,14 +1661,20 @@ int intel_wait_for_register(struct drm_i915_private > *dev_priv, > u32 value, > unsigned int timeout_ms) > { > - > unsigned fw = > intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ); > int ret; > > - intel_uncore_forcewake_get(dev_priv, fw); > - ret = wait_for_us((I915_READ_FW(reg) & mask) == value, 2); > - intel_uncore_forcewake_put(dev_priv, fw); > + spin_lock_irq(&dev_priv->uncore.lock); > + intel_uncore_forcewake_get__locked(dev_priv, fw); > + > + ret = __intel_wait_for_register_fw(dev_priv, > + reg, mask, value, > + 2, 0, NULL); > + > + intel_uncore_forcewake_put__locked(dev_priv, fw); > + spin_unlock_irq(&dev_priv->uncore.lock);
Hmm, shouldn't we use spin_lock_irqsave/spin_unlock_irqrestore here? Like in GEN6_READ_HEADER/GEN6_READ_FOOTER. -Michal > + > if (ret) > ret = wait_for((I915_READ_NOTRACE(reg) & mask) == value, > timeout_ms); > -- > 2.11.0 > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
