[ 81/99] drm/i915: Serialize almost all register access

[Greg Kroah-Hartman]

3.10-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

commit a7cd1b8fea2f341b626b255d9898a5ca5fabbf0a upstream.

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=63914
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
 drivers/gpu/drm/i915/i915_drv.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1247,21 +1247,21 @@ hsw_unclaimed_reg_check(struct drm_i915_
 
 #define __i915_read(x, y) \
 u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
+	unsigned long irqflags; \
 	u##x val = 0; \
+	spin_lock_irqsave(&dev_priv->gt_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->gt_lock, irqflags); \
 		if (dev_priv->forcewake_count == 0) \
 			dev_priv->gt.force_wake_get(dev_priv); \
 		val = read##y(dev_priv->regs + reg); \
 		if (dev_priv->forcewake_count == 0) \
 			dev_priv->gt.force_wake_put(dev_priv); \
-		spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
 	} else { \
 		val = read##y(dev_priv->regs + reg); \
 	} \
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
 	trace_i915_reg_rw(false, reg, val, sizeof(val)); \
 	return val; \
 }
@@ -1274,8 +1274,10 @@ __i915_read(64, q)
 
 #define __i915_write(x, y) \
 void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
+	unsigned long irqflags; \
 	u32 __fifo_ret = 0; \
 	trace_i915_reg_rw(true, reg, val, sizeof(val)); \
+	spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
@@ -1287,6 +1289,7 @@ void i915_write##x(struct drm_i915_priva
 		gen6_gt_check_fifodbg(dev_priv); \
 	} \
 	hsw_unclaimed_reg_check(dev_priv, reg); \
+	spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags); \
 }
 __i915_write(8, b)
 __i915_write(16, w)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/