[Intel-gfx] [PATCH 3/3] drm/i915: Reorder hw init to avoid executing with invalid context/mm state

[Mika Kuoppala]

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

Currently we initialise the rings, add the first context switch to the
ring and execute our golden state then enable (aliasing or full) ppgtt.
However, as we enable ppgtt using direct MMIO but load the PD using
MI_LRI, we end up executing the context switch and golden render state
with an invalid PD generating page faults. To solve this issue, first do
the ppgtt PD setup, then set the default context and write the commands
to run the render state into the ring, before we activate the ring. This
allows us to be sure that the register state is valid before we begin
execution.

This was spotted when writing the seqno/request conversion, but only with
the ERROR capture did I realise that it was a necessity now.

RFC: cleanup the error handling in i915_gem_init_hw.

v2: added intel_ring_reset

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> (v1)
Signed-off-by: Mika Kuoppala <mika.kuoppala at intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 19 +++++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 26 ++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0fe313d..e154770 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4817,6 +4817,9 @@ i915_gem_init_hw(struct drm_device *dev)
 		}
 	}
 
+	for_each_ring(ring, dev_priv, i)
+		intel_ring_reset(ring);
+
 	i915_gem_init_swizzling(dev);
 
 	/*
@@ -4827,19 +4830,19 @@ i915_gem_init_hw(struct drm_device *dev)
 	 */
 	init_unused_rings(dev);
 
-	for_each_ring(ring, dev_priv, i) {
-		ret = ring->init_hw(ring);
-		if (ret)
-			goto out;
+	ret = i915_ppgtt_init_hw(dev);
+	if (ret && ret != -EIO) {
+		DRM_ERROR("PPGTT enable failed %d\n", ret);
+		i915_gem_cleanup_ringbuffer(dev);
 	}
 
 	for (i = 0; i < NUM_L3_SLICES(dev); i++)
 		i915_gem_l3_remap(&dev_priv->ring[RCS], i);
 
-	ret = i915_ppgtt_init_hw(dev);
-	if (ret && ret != -EIO) {
-		DRM_ERROR("PPGTT enable failed %d\n", ret);
-		i915_gem_cleanup_ringbuffer(dev);
+	for_each_ring(ring, dev_priv, i) {
+		ret = ring->init_hw(ring);
+		if (ret)
+			goto out;
 	}
 
 	ret = i915_gem_context_enable(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 441e250..0f97588 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -560,6 +560,22 @@ static bool stop_ring(struct intel_engine_cs *ring)
 	return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0;
 }
 
+void intel_ring_reset(struct intel_engine_cs *ring)
+{
+	struct intel_ringbuffer *ringbuf = ring->buffer;
+
+	WARN_ON(!stop_ring(ring));
+
+	if (WARN_ON(ringbuf == NULL))
+		return;
+
+	ringbuf->head = 0;
+	ringbuf->tail = 0;
+
+	ringbuf->last_retired_head = -1;
+}
+
+
 static int init_ring_common(struct intel_engine_cs *ring)
 {
 	struct drm_device *dev = ring->dev;
@@ -614,6 +630,9 @@ static int init_ring_common(struct intel_engine_cs *ring)
 	I915_WRITE_HEAD(ring, 0);
 	(void)I915_READ_HEAD(ring);
 
+	I915_WRITE_HEAD(ring, ringbuf->head);
+	I915_WRITE_TAIL(ring, ringbuf->head);
+
 	I915_WRITE_CTL(ring,
 			((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES)
 			| RING_VALID);
@@ -621,7 +640,7 @@ static int init_ring_common(struct intel_engine_cs *ring)
 	/* If the head is still not zero, the ring is dead */
 	if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 &&
 		     I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj) &&
-		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
+		     (I915_READ_HEAD(ring) & HEAD_ADDR) == ringbuf->head, 50)) {
 		DRM_ERROR("%s initialization failed "
 			  "ctl %08x (valid? %d) head %08x tail %08x start %08x [expected %08lx]\n",
 			  ring->name,
@@ -632,13 +651,12 @@ static int init_ring_common(struct intel_engine_cs *ring)
 		goto out;
 	}
 
+	I915_WRITE_TAIL(ring, ringbuf->tail);
+
 	ringbuf->last_retired_head = -1;
-	ringbuf->head = I915_READ_HEAD(ring);
-	ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR;
 	intel_ring_update_space(ringbuf);
 
 	memset(&ring->hangcheck, 0, sizeof(ring->hangcheck));
-
 out:
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c761fe0..168a670 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -408,6 +408,7 @@ int __intel_ring_space(int head, int tail, int size);
 void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
 int intel_ring_space(struct intel_ringbuffer *ringbuf);
 bool intel_ring_stopped(struct intel_engine_cs *ring);
+void intel_ring_reset(struct intel_engine_cs *ring);
 void __intel_ring_advance(struct intel_engine_cs *ring);
 
 int __must_check intel_ring_idle(struct intel_engine_cs *ring);
-- 
1.9.1