[intel] Add debug code to verify the cached ring tail pointer.

Recording the tail pointer in a local variable improves performance, but if
someone messes up and fails to reload at the right time, the driver will
write commands to the wrong part of the ring and scramble execution badly.

This change (available by setting I915_RING_VALIDATE to 1) checks to make
sure the cached tail pointer matches the hardware tail pointer at each ring
buffer addition, calling BUG_ON when that's not true.

diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c
index fa73cd2..1fc617d 100644 (file)
--- a/shared-core/i915_dma.c
+++ b/shared-core/i915_dma.c
@@ -69,6 +69,30 @@ int i915_wait_ring(struct drm_device * dev, int n, const char *caller)
        return -EBUSY;
 }
 
+#if I915_RING_VALIDATE
+/**
+ * Validate the cached ring tail value
+ *
+ * If the X server writes to the ring and DRM doesn't
+ * reload the head and tail pointers, it will end up writing
+ * data to the wrong place in the ring, causing havoc.
+ */
+void i915_ring_validate(struct drm_device *dev, const char *func, int line)
+{
+       drm_i915_private_t *dev_priv = dev->dev_private;
+       drm_i915_ring_buffer_t *ring = &(dev_priv->ring);
+       u32     tail = I915_READ(LP_RING+RING_TAIL) & HEAD_ADDR;
+       u32     head = I915_READ(LP_RING+RING_HEAD) & HEAD_ADDR;
+
+       if (tail != ring->tail) {
+               DRM_ERROR("%s:%d head sw %x, hw %x. tail sw %x hw %x\n",
+                         func, line,
+                         ring->head, head, ring->tail, tail);
+               BUG_ON(1);
+       }
+}
+#endif
+
 void i915_kernel_lost_context(struct drm_device * dev)
 {
        drm_i915_private_t *dev_priv = dev->dev_private;

diff --git a/shared-core/i915_drv.h b/shared-core/i915_drv.h
index 029c39a..e217b78 100644 (file)
--- a/shared-core/i915_drv.h
+++ b/shared-core/i915_drv.h
@@ -473,14 +473,23 @@ extern void intel_fini_chipset_flush_compat(struct drm_device *dev);
 #define I915_WRITE16(reg,val)  DRM_WRITE16(dev_priv->mmio_map, (reg), (val))
 
 #define I915_VERBOSE 0
+#define I915_RING_VALIDATE 0
 
 #define RING_LOCALS    unsigned int outring, ringmask, outcount; \
                        volatile char *virt;
 
+#if I915_RING_VALIDATE
+void i915_ring_validate(struct drm_device *dev, const char *func, int line);
+#define I915_RING_DO_VALIDATE(dev) i915_ring_validate(dev, __FUNCTION__, __LINE__)
+#else
+#define I915_RING_DO_VALIDATE(dev)
+#endif
+
 #define BEGIN_LP_RING(n) do {                          \
        if (I915_VERBOSE)                               \
                DRM_DEBUG("BEGIN_LP_RING(%d)\n",        \
                                 (n));                  \
+       I915_RING_DO_VALIDATE(dev);                     \
        if (dev_priv->ring.space < (n)*4)                      \
                i915_wait_ring(dev, (n)*4, __FUNCTION__);      \
        outcount = 0;                                   \
@@ -499,6 +508,7 @@ extern void intel_fini_chipset_flush_compat(struct drm_device *dev);
 
 #define ADVANCE_LP_RING() do {                                         \
        if (I915_VERBOSE) DRM_DEBUG("ADVANCE_LP_RING %x\n", outring);   \
+       I915_RING_DO_VALIDATE(dev);                                     \
        dev_priv->ring.tail = outring;                                  \
        dev_priv->ring.space -= outcount * 4;                           \
        I915_WRITE(LP_RING + RING_TAIL, outring);                       \