In theory, the different register blocks were meant to be only ever touched when holding either the struct_mutex, mode_config.lock or even a specific localised lock. This does not seem to be the case, and the hardware reacts extremely badly if we attempt to concurrently access two registers within the same cacheline.
The HSD suggests that we only need to do this workaround for display range registers. However, upon review we need to serialize the multiple stages in our register write functions - if only for preemption protection. v2: Rebase onto uncore v3: Add wa reference, WaSerializeKmdDisplayAccess Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63914 Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Ben Widawsky <[email protected]> --- drivers/gpu/drm/i915/intel_uncore.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index e787648..789a596 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -343,23 +343,31 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) } } +/* + * Note that we need to serialise access to registers within the same + * cacheline - to do that we use a spinlock over all registers (or in the + * future perhaps register blocks). + * + * WaSerializeKmdDisplayAccess:hsw + */ + #define __i915_read(x) \ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg, bool trace) { \ + unsigned long irqflags; \ u##x val = 0; \ + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ if (IS_GEN5(dev_priv->dev)) \ ilk_dummy_write(dev_priv); \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ - unsigned long irqflags; \ - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ if (dev_priv->uncore.forcewake_count == 0) \ dev_priv->uncore.funcs.force_wake_get(dev_priv); \ val = __raw_i915_read##x(dev_priv, reg); \ if (dev_priv->uncore.forcewake_count == 0) \ dev_priv->uncore.funcs.force_wake_put(dev_priv); \ - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \ } else { \ val = __raw_i915_read##x(dev_priv, reg); \ } \ + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \ if (trace) trace_i915_reg_rw(false, reg, val, sizeof(val)); \ return val; \ } @@ -372,8 +380,10 @@ __i915_read(64) #define __i915_write(x) \ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool trace) { \ + unsigned long irqflags; \ u32 __fifo_ret = 0; \ if (trace) trace_i915_reg_rw(true, reg, val, sizeof(val)); \ + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ } \ @@ -385,6 +395,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val, bool tr gen6_gt_check_fifodbg(dev_priv); \ } \ hsw_unclaimed_reg_check(dev_priv, reg); \ + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \ } __i915_write(8) __i915_write(16) -- 1.8.3.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
