diff options
author | Daniel Vetter <daniel.vetter@ffwll.ch> | 2014-03-14 19:08:55 -0400 |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2014-03-28 13:25:18 -0400 |
commit | 88fe429db0d3866ecc355c2b72056690b0ce8f57 (patch) | |
tree | 10dd0def65d63839a771c157cbbe6b2e583bf85c /drivers/gpu | |
parent | 4da98541d898d45255edf874b16687aad0ff90cd (diff) |
drm/i915: fix up semaphore_waits_for
There's an entire pile of issues in here:
- Use the main RING_HEAD register, not ACTHD. ACTHD points at the gtt
offset of the batch buffer when a batch is executed. Semaphores are
always emitted to the main ring, so we always want to look at that.
- Mask the obtained HEAD pointer with the actual ring size, which is
much smaller. Together with the above issue this resulted us in
trying to dereference a pointer way outside of the ring mmio
mapping. The resulting invalid access in interrupt context
(hangcheck is executed from timers) lead to a full blown kernel
panic. The fbcon panic handler then tried to frob our driver harder,
resulting in a full machine hang at least on my snb here where I've
stumbled over this.
- Handle ring wrapping correctly and be a bit more explicit about how
many dwords we're scanning. We probably should also scan more than
just 4 ...
- Space out some of teh computations for readability.
This reduces hard-hangs on my snb here. Mika and QA both say that it
doesn't completel remove them, but at least for me it's a clear
improvement in stability.
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=74100
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Diffstat (limited to 'drivers/gpu')
-rw-r--r-- | drivers/gpu/drm/i915/i915_irq.c | 38 |
1 files changed, 26 insertions, 12 deletions
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index acf1ab3ff0d9..9ef241fb86b3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c | |||
@@ -2530,29 +2530,43 @@ static struct intel_ring_buffer * | |||
2530 | semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno) | 2530 | semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno) |
2531 | { | 2531 | { |
2532 | struct drm_i915_private *dev_priv = ring->dev->dev_private; | 2532 | struct drm_i915_private *dev_priv = ring->dev->dev_private; |
2533 | u32 cmd, ipehr, acthd, acthd_min; | 2533 | u32 cmd, ipehr, head; |
2534 | int i; | ||
2534 | 2535 | ||
2535 | ipehr = I915_READ(RING_IPEHR(ring->mmio_base)); | 2536 | ipehr = I915_READ(RING_IPEHR(ring->mmio_base)); |
2536 | if ((ipehr & ~(0x3 << 16)) != | 2537 | if ((ipehr & ~(0x3 << 16)) != |
2537 | (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER)) | 2538 | (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER)) |
2538 | return NULL; | 2539 | return NULL; |
2539 | 2540 | ||
2540 | /* ACTHD is likely pointing to the dword after the actual command, | 2541 | /* |
2541 | * so scan backwards until we find the MBOX. | 2542 | * HEAD is likely pointing to the dword after the actual command, |
2543 | * so scan backwards until we find the MBOX. But limit it to just 3 | ||
2544 | * dwords. Note that we don't care about ACTHD here since that might | ||
2545 | * point at at batch, and semaphores are always emitted into the | ||
2546 | * ringbuffer itself. | ||
2542 | */ | 2547 | */ |
2543 | acthd = intel_ring_get_active_head(ring) & HEAD_ADDR; | 2548 | head = I915_READ_HEAD(ring) & HEAD_ADDR; |
2544 | acthd_min = max((int)acthd - 3 * 4, 0); | 2549 | |
2545 | do { | 2550 | for (i = 4; i; --i) { |
2546 | cmd = ioread32(ring->virtual_start + acthd); | 2551 | /* |
2552 | * Be paranoid and presume the hw has gone off into the wild - | ||
2553 | * our ring is smaller than what the hardware (and hence | ||
2554 | * HEAD_ADDR) allows. Also handles wrap-around. | ||
2555 | */ | ||
2556 | head &= ring->size - 1; | ||
2557 | |||
2558 | /* This here seems to blow up */ | ||
2559 | cmd = ioread32(ring->virtual_start + head); | ||
2547 | if (cmd == ipehr) | 2560 | if (cmd == ipehr) |
2548 | break; | 2561 | break; |
2549 | 2562 | ||
2550 | acthd -= 4; | 2563 | head -= 4; |
2551 | if (acthd < acthd_min) | 2564 | } |
2552 | return NULL; | 2565 | |
2553 | } while (1); | 2566 | if (!i) |
2567 | return NULL; | ||
2554 | 2568 | ||
2555 | *seqno = ioread32(ring->virtual_start+acthd+4)+1; | 2569 | *seqno = ioread32(ring->virtual_start + head + 4) + 1; |
2556 | return &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3]; | 2570 | return &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3]; |
2557 | } | 2571 | } |
2558 | 2572 | ||