gen7 missed IRQ workaround series.

gen7 missed IRQ workaround series.

[Eric Anholt]

This is the minimal patchset I have for working around the missed
IRQs.  I've been running it since Monday doing test runs for other
work, and it appears to be stable.

[PATCH 1/3] drm/i915: Do the fallback non-IRQ wait in ring throttle, too.

[Eric Anholt]

As a workaround for IRQ synchronization issues in the gen7 BLT ring,
we want to turn the two wait functions into polling loops.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/i915_gem.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8359dc7..66e0a55 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3309,6 +3309,10 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 
 			if (ret == 0 && atomic_read(&dev_priv->mm.wedged))
 				ret = -EIO;
+		} else if (wait_for(i915_seqno_passed(ring->get_seqno(ring),
+						      seqno) ||
+				    atomic_read(&dev_priv->mm.wedged), 3000)) {
+			ret = -EBUSY;
 		}
 	}
 
-- 
1.7.7.3

[PATCH 2/3] drm/i915: Work around gen7 BLT ring synchronization issues.

[Eric Anholt]

Previous to this commit, testing easily reproduced a failure where the
seqno would apparently arrive after the IRQ associated with it, with test programs as simple as:

for (;;) {
    glCopyPixels(0, 0, 1, 1);
    glFinish();
}

Various workarounds we've seen for previous generations didn't work to
fix this issue, so until new information comes in, replace the IRQ
waits on the BLT ring with polling.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c3f0c7b..51ba68b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -793,6 +793,17 @@ ring_add_request(struct intel_ring_buffer *ring,
 }
 
 static bool
+gen7_blt_ring_get_irq(struct intel_ring_buffer *ring)
+{
+	/* The BLT ring on IVB appears to have broken synchronization
+	 * between the seqno write and the interrupt, so that the
+	 * interrupt appears first.  Returning false here makes
+	 * i915_wait_request() do a polling loop, instead.
+	 */
+	return false;
+}
+
+static bool
 gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
 {
 	struct drm_device *dev = ring->dev;
@@ -1558,5 +1569,8 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 
 	*ring = gen6_blt_ring;
 
+	if (IS_GEN7(dev))
+		ring->irq_get = gen7_blt_ring_get_irq;
+
 	return intel_init_ring_buffer(dev, ring);
 }
-- 
1.7.7.3

[PATCH 3/3] drm/i915: Make the fallback IRQ wait not sleep.

[Eric Anholt]

The waits we do here are generally so short that sleeping is a bad
idea unless we have an IRQ to wake us up.  Improves regression test
performance from 18 minutes to 3.5 minutes on gen7, which is now
consistent with the previous generation.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/i915_gem.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 66e0a55..e55badb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2006,9 +2006,9 @@ i915_wait_request(struct intel_ring_buffer *ring,
 					   || atomic_read(&dev_priv->mm.wedged));
 
 			ring->irq_put(ring);
-		} else if (wait_for(i915_seqno_passed(ring->get_seqno(ring),
-						      seqno) ||
-				    atomic_read(&dev_priv->mm.wedged), 3000))
+		} else if (wait_for_atomic(i915_seqno_passed(ring->get_seqno(ring),
+							     seqno) ||
+					   atomic_read(&dev_priv->mm.wedged), 3000))
 			ret = -EBUSY;
 		ring->waiting_seqno = 0;
 
@@ -3309,8 +3309,8 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 
 			if (ret == 0 && atomic_read(&dev_priv->mm.wedged))
 				ret = -EIO;
-		} else if (wait_for(i915_seqno_passed(ring->get_seqno(ring),
-						      seqno) ||
+		} else if (wait_for_atomic(i915_seqno_passed(ring->get_seqno(ring),
+							     seqno) ||
 				    atomic_read(&dev_priv->mm.wedged), 3000)) {
 			ret = -EBUSY;
 		}
-- 
1.7.7.3

Re: gen7 missed IRQ workaround series.

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 896 bytes --]

On Thu, Dec 22, 2011 at 20:54, Eric Anholt <eric@anholt.net> wrote:

> This is the minimal patchset I have for working around the missed
> IRQs.  I've been running it since Monday doing test runs for other
> work, and it appears to be stable.
>

Works for me, no more missed IRQs nor hangs/delays with this patchset.

I have one questions though - would it be possible to control the
interrupts behavior via a module parameter or debugfs entry to simplify
debugging of this issue going forward?

For example, something like /sys/kernel/debug/dri/0/i915_irq_polling -
writing '1' there would enable polling, and '0' would use the usual
gen6_ring_get_irq. This would default to '1' on gen7, and '0' on older
architectures.

Other than that,
Tested-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1356 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: gen7 missed IRQ workaround series.

[Kenneth Graunke]

On 12/22/2011 02:54 PM, Eric Anholt wrote:
> This is the minimal patchset I have for working around the missed
> IRQs.  I've been running it since Monday doing test runs for other
> work, and it appears to be stable.

These all look okay to me, but this really isn't my area of expertise.

Acked-by: Kenneth Graunke <kenneth@whitecape.org>

Re: gen7 missed IRQ workaround series.

[Daniel Vetter]

On Thu, Dec 22, 2011 at 02:54:58PM -0800, Eric Anholt wrote:
> This is the minimal patchset I have for working around the missed
> IRQs.  I've been running it since Monday doing test runs for other
> work, and it appears to be stable.

I'm a bit late to the party with two things:
- The bsd ring is similarly broken and
- we can easily wait for a few msec (probably not when running piglit
  though) here, so busylooping is imo not appropriate.

I kinda prefer Chris' approach of sticking with irqs, but backing it up
with a timer in the msec range. Can't find that patch though atm (iirc
it's in bugzilla somewhere).

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: gen7 missed IRQ workaround series.

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 979 bytes --]

On Wed, 4 Jan 2012 00:04:18 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:

> I kinda prefer Chris' approach of sticking with irqs, but backing it up
> with a timer in the msec range. Can't find that patch though atm (iirc
> it's in bugzilla somewhere).

I think we've resolved that the interrupt arrives, but that it is not
serialized wrt the memory write. So, what I'd love to see is whether
waiting for the irq and then busy looping for 'a while' works. This
would allow for long-running operations to idle the CPU and then hit the
interrupt and spin until the memory write is recognized.

Any solution which can leave operations unacknowledged for 'ms'
timeframes seems like a potential for serious performance
problems. Eric's simple spinning approach seems fine for the BLT ring
which we don't use that often; something more complicated and yet not
entirely timer-based might be more efficient for longer-running operations.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: gen7 missed IRQ workaround series.

[Daniel Vetter]

On Tue, Jan 03, 2012 at 03:38:25PM -0800, Keith Packard wrote:
> On Wed, 4 Jan 2012 00:04:18 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > I kinda prefer Chris' approach of sticking with irqs, but backing it up
> > with a timer in the msec range. Can't find that patch though atm (iirc
> > it's in bugzilla somewhere).
> 
> I think we've resolved that the interrupt arrives, but that it is not
> serialized wrt the memory write. So, what I'd love to see is whether
> waiting for the irq and then busy looping for 'a while' works. This
> would allow for long-running operations to idle the CPU and then hit the
> interrupt and spin until the memory write is recognized.
> 
> Any solution which can leave operations unacknowledged for 'ms'
> timeframes seems like a potential for serious performance
> problems. Eric's simple spinning approach seems fine for the BLT ring
> which we don't use that often; something more complicated and yet not
> entirely timer-based might be more efficient for longer-running operations.

Afaics we only spin shortly on the blt ring if
- semaphores are not enabled and
- mesa is the client.
For all other cases it's pretty easy to come up with examples where we
busy-loop for a rather long time. So I really don't like the busy-looping,
especially now that semaphores are enabled by default for ivb.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: gen7 missed IRQ workaround series.

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 305 bytes --]

On Wed, 4 Jan 2012 18:02:21 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:

> So I really don't like the busy-looping, especially now that
> semaphores are enabled by default for ivb.

Alternatives are welcome; we have no other known fix for missing
sequence writes.

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: gen7 missed IRQ workaround series.

[Daniel Vetter]

On Wed, Jan 04, 2012 at 09:05:51AM -0800, Keith Packard wrote:
> On Wed, 4 Jan 2012 18:02:21 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > So I really don't like the busy-looping, especially now that
> > semaphores are enabled by default for ivb.
> 
> Alternatives are welcome; we have no other known fix for missing
> sequence writes.

See my patch, I'm pretty positive that I've successfully papered over it,
at least on my machine here. Also, Chris has posted an alternative using
irqs + a short timer as a backup a while ago. I like that much more.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48