Commit a7cd1b8f authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter

drm/i915: Serialize almost all register access

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.

Irrespective of the hardware requirements, the current io functions are
a little too loose with respect to the combination of pre- and
post-condition testing that we do in conjunction with the actual io. As
a result, we may be pre-empted and generate both false-postive and
false-negative errors.

Note well that this is a "90%" solution, there remains a few direct
users of ioread/iowrite which will be fixed up in the next few patches.
Since they are more invasive and that this simple change will prevent
almost all lockups on Haswell, we kept this patch simple to facilitate
backporting to stable.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63914Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent e85843be
...@@ -1253,21 +1253,21 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) ...@@ -1253,21 +1253,21 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
#define __i915_read(x, y) \ #define __i915_read(x, y) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \ u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
unsigned long irqflags; \
u##x val = 0; \ u##x val = 0; \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (IS_GEN5(dev_priv->dev)) \ if (IS_GEN5(dev_priv->dev)) \
ilk_dummy_write(dev_priv); \ ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
unsigned long irqflags; \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (dev_priv->forcewake_count == 0) \ if (dev_priv->forcewake_count == 0) \
dev_priv->gt.force_wake_get(dev_priv); \ dev_priv->gt.force_wake_get(dev_priv); \
val = read##y(dev_priv->regs + reg); \ val = read##y(dev_priv->regs + reg); \
if (dev_priv->forcewake_count == 0) \ if (dev_priv->forcewake_count == 0) \
dev_priv->gt.force_wake_put(dev_priv); \ dev_priv->gt.force_wake_put(dev_priv); \
spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
} else { \ } else { \
val = read##y(dev_priv->regs + reg); \ val = read##y(dev_priv->regs + reg); \
} \ } \
spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
trace_i915_reg_rw(false, reg, val, sizeof(val)); \ trace_i915_reg_rw(false, reg, val, sizeof(val)); \
return val; \ return val; \
} }
...@@ -1280,8 +1280,10 @@ __i915_read(64, q) ...@@ -1280,8 +1280,10 @@ __i915_read(64, q)
#define __i915_write(x, y) \ #define __i915_write(x, y) \
void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
unsigned long irqflags; \
u32 __fifo_ret = 0; \ u32 __fifo_ret = 0; \
trace_i915_reg_rw(true, reg, val, sizeof(val)); \ trace_i915_reg_rw(true, reg, val, sizeof(val)); \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \ } \
...@@ -1293,6 +1295,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \ ...@@ -1293,6 +1295,7 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
gen6_gt_check_fifodbg(dev_priv); \ gen6_gt_check_fifodbg(dev_priv); \
} \ } \
hsw_unclaimed_reg_check(dev_priv, reg); \ hsw_unclaimed_reg_check(dev_priv, reg); \
spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
} }
__i915_write(8, b) __i915_write(8, b)
__i915_write(16, w) __i915_write(16, w)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment