From patchwork Sat Sep 5 17:07:06 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 45938 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n85H7aw5026196 for ; Sat, 5 Sep 2009 17:07:36 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BCDD19E777; Sat, 5 Sep 2009 10:07:35 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (unknown [94.30.43.99]) by gabe.freedesktop.org (Postfix) with ESMTP id 7E8F19E744 for ; Sat, 5 Sep 2009 10:07:33 -0700 (PDT) Received: from localhost.localdomain (unverified [78.156.66.37]) by fireflyinternet.com (Firefly Internet SMTP) with ESMTP id 49638855-1948518 for multiple; Sat, 05 Sep 2009 18:07:24 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Sat, 5 Sep 2009 18:07:06 +0100 Message-Id: <1252170426-1056-1-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 1.6.3.3 MIME-Version: 1.0 X-Originating-IP: 78.156.66.37 Subject: [Intel-gfx] [PATCH] drm/i915: Pad ringbuffer with NOOPs before wrapping X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.9 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org According to the docs, the ringbuffer is not allowed to wrap in the middle of an instruction. G45 PRM, Vol 1b, p101: While the “free space” wrap may allow commands to be wrapped around the end of the Ring Buffer, the wrap should only occur between commands. Padding (with NOP) may be required to follow this restriction. Do as commanded. [Having seen bug reports where there is evidence of split commands, but apparently the GPU has continued on merrily before a bizarre and untimely death, this may or may not fix a few random hangs.] Signed-off-by: Chris Wilson CC: Eric Anholt --- drivers/gpu/drm/i915/i915_debugfs.c | 4 +-- drivers/gpu/drm/i915/i915_dma.c | 29 ++++++++++++++++++++++- drivers/gpu/drm/i915/i915_drv.h | 43 +++++++++++++++++------------------ drivers/gpu/drm/i915/i915_gem.c | 1 - 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9acc1da..4d293c1 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -346,15 +346,13 @@ static int i915_ringbuffer_info(struct seq_file *m, void *data) struct drm_info_node *node = (struct drm_info_node *) m->private; struct drm_device *dev = node->minor->dev; drm_i915_private_t *dev_priv = dev->dev_private; - unsigned int head, tail, mask; + unsigned int head, tail; head = I915_READ(PRB0_HEAD) & HEAD_ADDR; tail = I915_READ(PRB0_TAIL) & TAIL_ADDR; - mask = dev_priv->ring.tail_mask; seq_printf(m, "RingHead : %08x\n", head); seq_printf(m, "RingTail : %08x\n", tail); - seq_printf(m, "RingMask : %08x\n", mask); seq_printf(m, "RingSize : %08lx\n", dev_priv->ring.Size); seq_printf(m, "Acthd : %08x\n", I915_READ(IS_I965G(dev) ? ACTHD_I965 : ACTHD)); diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 50d1f78..f135bdc 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -80,6 +80,34 @@ int i915_wait_ring(struct drm_device * dev, int n, const char *caller) return -EBUSY; } +/* As a ringbuffer is only allowed to wrap between instructions, fill + * the tail with NOOPs. + */ +int i915_wrap_ring(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + volatile unsigned int *virt; + int rem; + + rem = dev_priv->ring.Size - dev_priv->ring.tail; + if (dev_priv->ring.space < rem) { + int ret = i915_wait_ring(dev, rem, __func__); + if (ret) + return ret; + } + dev_priv->ring.space -= rem; + + virt = (unsigned int *) + (dev_priv->ring.virtual_start + dev_priv->ring.tail); + rem /= 4; + while (rem--) + *virt++ = MI_NOOP; + + dev_priv->ring.tail = 0; + + return 0; +} + /** * Sets up the hardware status page for devices that need a physical address * in the register. @@ -200,7 +228,6 @@ static int i915_initialize(struct drm_device * dev, drm_i915_init_t * init) } dev_priv->ring.Size = init->ring_size; - dev_priv->ring.tail_mask = dev_priv->ring.Size - 1; dev_priv->ring.map.offset = init->ring_start; dev_priv->ring.map.size = init->ring_size; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 76914ae..2d5bce6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -85,7 +85,6 @@ struct drm_i915_gem_phys_object { }; typedef struct _drm_i915_ring_buffer { - int tail_mask; unsigned long Size; u8 *virtual_start; int head; @@ -790,33 +789,32 @@ extern void intel_modeset_cleanup(struct drm_device *dev); #define I915_VERBOSE 0 -#define RING_LOCALS unsigned int outring, ringmask, outcount; \ - volatile char *virt; - -#define BEGIN_LP_RING(n) do { \ - if (I915_VERBOSE) \ - DRM_DEBUG("BEGIN_LP_RING(%d)\n", (n)); \ - if (dev_priv->ring.space < (n)*4) \ - i915_wait_ring(dev, (n)*4, __func__); \ - outcount = 0; \ - outring = dev_priv->ring.tail; \ - ringmask = dev_priv->ring.tail_mask; \ - virt = dev_priv->ring.virtual_start; \ +#define RING_LOCALS volatile unsigned int *ring_virt__; + +#define BEGIN_LP_RING(n) do { \ + int bytes__ = 4*(n); \ + if (I915_VERBOSE) DRM_DEBUG("BEGIN_LP_RING(%d)\n", (n)); \ + /* a wrap must occur between instructions so pad beforehand */ \ + if (unlikely (dev_priv->ring.tail + bytes__ > dev_priv->ring.Size)) \ + i915_wrap_ring(dev); \ + if (unlikely (dev_priv->ring.space < bytes__)) \ + i915_wait_ring(dev, bytes__, __func__); \ + ring_virt__ = (unsigned int *) \ + (dev_priv->ring.virtual_start + dev_priv->ring.tail); \ + dev_priv->ring.tail += bytes__; \ + dev_priv->ring.tail &= dev_priv->ring.Size - 1; \ + dev_priv->ring.space -= bytes__; \ } while (0) -#define OUT_RING(n) do { \ +#define OUT_RING(n) do { \ if (I915_VERBOSE) DRM_DEBUG(" OUT_RING %x\n", (int)(n)); \ - *(volatile unsigned int *)(virt + outring) = (n); \ - outcount++; \ - outring += 4; \ - outring &= ringmask; \ + *ring_virt__++ = (n); \ } while (0) #define ADVANCE_LP_RING() do { \ - if (I915_VERBOSE) DRM_DEBUG("ADVANCE_LP_RING %x\n", outring); \ - dev_priv->ring.tail = outring; \ - dev_priv->ring.space -= outcount * 4; \ - I915_WRITE(PRB0_TAIL, outring); \ + if (I915_VERBOSE) \ + DRM_DEBUG("ADVANCE_LP_RING %x\n", dev_priv->ring.tail); \ + I915_WRITE(PRB0_TAIL, dev_priv->ring.tail); \ } while(0) /** @@ -839,6 +837,7 @@ extern void intel_modeset_cleanup(struct drm_device *dev); #define I915_GEM_HWS_INDEX 0x20 #define I915_BREADCRUMB_INDEX 0x21 +extern int i915_wrap_ring(struct drm_device * dev); extern int i915_wait_ring(struct drm_device * dev, int n, const char *caller); #define IS_I830(dev) ((dev)->pci_device == 0x3577) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bb816ec..f52dab0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4094,7 +4094,6 @@ i915_gem_init_ringbuffer(struct drm_device *dev) /* Set up the kernel mapping for the ring. */ ring->Size = obj->size; - ring->tail_mask = obj->size - 1; ring->map.offset = dev->agp->base + obj_priv->gtt_offset; ring->map.size = obj->size;