[PATCH 1/2] drm/i915: Don't emit semaphore wait if wrap happened

[PATCH 1/2] drm/i915: Don't emit semaphore wait if wrap happened

[Mika Kuoppala]

If wrap just happened we need to prevent emitting waits for
pre wrap values. Detect this and emit no-ops instead.

v2: Use olr > seqno to detect wrap instead of *seqno == 0
as suggested by Chris Wilson.

v3: Use last used seqno to detect the wraparound. From
Chris Wilson

References: https://bugs.freedesktop.org/show_bug.cgi?id=57967
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |    2 +-
 drivers/gpu/drm/i915/i915_gem.c         |    3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   26 +++++++++++++++++++++-----
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ab476d..9da6782 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -671,7 +671,7 @@ typedef struct drm_i915_private {
 
 	struct pci_dev *bridge_dev;
 	struct intel_ring_buffer ring[I915_NUM_RINGS];
-	uint32_t next_seqno;
+	uint32_t last_seqno, next_seqno;
 
 	drm_dma_handle_t *status_page_dmah;
 	struct resource mch_res;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4b233d..e414507 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1969,7 +1969,7 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 		dev_priv->next_seqno = 1;
 	}
 
-	*seqno = dev_priv->next_seqno++;
+	*seqno = dev_priv->last_seqno = dev_priv->next_seqno++;
 	return 0;
 }
 
@@ -2032,6 +2032,7 @@ i915_add_request(struct intel_ring_buffer *ring,
 	}
 
 	trace_i915_gem_request_add(ring, request->seqno);
+	dev_priv->last_seqno = request->seqno;
 	ring->outstanding_lazy_request = 0;
 
 	if (!dev_priv->mm.suspended) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0d03dc6..69bbe7b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -596,6 +596,13 @@ gen6_add_request(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static inline bool i915_gem_has_seqno_wrapped(struct drm_device *dev,
+					      u32 seqno)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	return dev_priv->last_seqno < seqno;
+}
+
 /**
  * intel_ring_sync - sync the waiter to the signaller on seqno
  *
@@ -626,11 +633,20 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
 	if (ret)
 		return ret;
 
-	intel_ring_emit(waiter,
-			dw1 | signaller->semaphore_register[waiter->id]);
-	intel_ring_emit(waiter, seqno);
-	intel_ring_emit(waiter, 0);
-	intel_ring_emit(waiter, MI_NOOP);
+	/* If seqno wrap happened, omit the wait with no-ops */
+	if (likely(!i915_gem_has_seqno_wrapped(waiter->dev, seqno))) {
+		intel_ring_emit(waiter,
+				dw1 |
+				signaller->semaphore_register[waiter->id]);
+		intel_ring_emit(waiter, seqno);
+		intel_ring_emit(waiter, 0);
+		intel_ring_emit(waiter, MI_NOOP);
+	} else {
+		intel_ring_emit(waiter, MI_NOOP);
+		intel_ring_emit(waiter, MI_NOOP);
+		intel_ring_emit(waiter, MI_NOOP);
+		intel_ring_emit(waiter, MI_NOOP);
+	}
 	intel_ring_advance(waiter);
 
 	return 0;
-- 
1.7.9.5

[PATCH 2/2] drm/i915: Open-code i915_gpu_idle() for handling seqno wrapping

[Mika Kuoppala]

From: Chris Wilson <chris@chris-wilson.co.uk>

The complication is that during seqno wrapping we must be extremely
careful not to write to any ring as that will require a new seqno, and
so would recurse back into the seqno wrap handler. So we cannot call
i915_gpu_idle() as that does additional work beyond simply retiring the
current set of requests, and instead must do the minimal work ourselves
during seqno wrapping.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e414507..e753cfc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1938,11 +1938,15 @@ i915_gem_handle_seqno_wrap(struct drm_device *dev)
 	if (ret == 0)
 		return ret;
 
-	ret = i915_gpu_idle(dev);
-	if (ret)
-		return ret;
-
+	/* Carefully retire all requests without writing to the rings */
+	for_each_ring(ring, dev_priv, i) {
+		ret = intel_ring_idle(ring);
+		if (ret)
+			return ret;
+	}
 	i915_gem_retire_requests(dev);
+
+	/* Finally reset hw state */
 	for_each_ring(ring, dev_priv, i) {
 		ret = intel_ring_handle_seqno_wrap(ring);
 		if (ret)
-- 
1.7.9.5

Re: [PATCH 2/2] drm/i915: Open-code i915_gpu_idle() for handling seqno wrapping

[Chris Wilson]

On Mon, 10 Dec 2012 13:56:17 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> @@ -2032,6 +2032,7 @@ i915_add_request(struct intel_ring_buffer *ring,
>  	}
>  
>  	trace_i915_gem_request_add(ring, request->seqno);
> +	dev_priv->last_seqno = request->seqno;

An unnecessary addition to my patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] drm/i915: Open-code i915_gpu_idle() for handling seqno wrapping

[Mika Kuoppala]

On Mon, 10 Dec 2012 11:59:44 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, 10 Dec 2012 13:56:17 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > @@ -2032,6 +2032,7 @@ i915_add_request(struct intel_ring_buffer *ring,
> >  	}
> >  
> >  	trace_i915_gem_request_add(ring, request->seqno);
> > +	dev_priv->last_seqno = request->seqno;
> 
> An unnecessary addition to my patch.

Indeed, that cruft got in somehow. Fix sent but msg id was eaten
by bash as it contained '$'

> -Chris
> -- 
> Chris Wilson, Intel Open Source Technology Centre