drm/i915: Fix startup failure in LRC mode after recent init changes

A previous commit introduced engine init changes:

    commit 372ee59699d9 ("drm/i915: Only init engines once")

This broke execlists as intel_lr_context_render_state_init was trying to emit commands to the RCS for the default context before the ring->init_hw was called.

Made a new gen8_init_rcs_context function and assign in to render ring init_context. Moved call to intel_logical_ring_workarounds_emit into gen8_init_rcs_context to maintain previous functionality.

Moved call to render_state_init from lr_context_deferred_create into gen8_init_rcs_context, and modified deferred_create to call ring->init_context for non-default contexts.

Modified i915_gem_context_enable to call ring->init_context for the default context.

So init_context will now always be called when the hw is ready - in i915_gem_context_enable for the default context and in lr_context_deferred_create for other contexts.

Signed-off-by Thomas Daniel <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_context.c |   25 ++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_lrc.c        |   30 +++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 18 deletions(-)

drivers/gpu/drm/i915/i915_gem_context.c +18 -7

index 3c3a9ff..5cd2b97 100644
@@ -408,14 +408,25 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
 
 	BUG_ON(!dev_priv->ring[RCS].default_context);
 
-	if (i915.enable_execlists)
-		return 0;
+	if (i915.enable_execlists) {
+		for_each_ring(ring, dev_priv, i) {
+			if (ring->init_context) {
+				ret = ring->init_context(ring,
+						ring->default_context);
+				if (ret) {
+					DRM_ERROR("ring init context: %d\n",
+							ret);
+					return ret;
+				}
+			}
+		}
 
-	for_each_ring(ring, dev_priv, i) {
-		ret = i915_switch_context(ring, ring->default_context);
-		if (ret)
-			return ret;
-	}
+	} else
+		for_each_ring(ring, dev_priv, i) {
+			ret = i915_switch_context(ring, ring->default_context);
+			if (ret)
+				return ret;
+		}
 
 	return 0;
 }

drivers/gpu/drm/i915/intel_lrc.c +19 -11

index 4ffb08c..79ef40c 100644
@@ -1336,6 +1336,18 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
 	return 0;
 }
 
+static int gen8_init_rcs_context(struct intel_engine_cs *ring,
+		       struct intel_context *ctx)
+{
+	int ret;
+
+	ret = intel_logical_ring_workarounds_emit(ring, ctx);
+	if (ret)
+		return ret;
+
+	return intel_lr_context_render_state_init(ring, ctx);
+}
+
 /**
  * intel_logical_ring_cleanup() - deallocate the Engine Command Streamer
  *

@@ -1409,7 +1421,7 @@ static int logical_render_ring_init(struct drm_device *dev)
 		ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
 	ring->init_hw = gen8_init_render_ring;
-	ring->init_context = intel_logical_ring_workarounds_emit;
+	ring->init_context = gen8_init_rcs_context;
 	ring->cleanup = intel_fini_pipe_control;
 	ring->get_seqno = gen8_get_seqno;
 	ring->set_seqno = gen8_set_seqno;

@@ -1905,21 +1917,17 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 
 	if (ctx == ring->default_context)
 		lrc_setup_hardware_status_page(ring, ctx_obj);
-
-	if (ring->id == RCS && !ctx->rcs_initialized) {
+	else if (ring->id == RCS && !ctx->rcs_initialized) {
 		if (ring->init_context) {
 			ret = ring->init_context(ring, ctx);
-			if (ret)
+			if (ret) {
 				DRM_ERROR("ring init context: %d\n", ret);
+				ctx->engine[ring->id].ringbuf = NULL;
+				ctx->engine[ring->id].state = NULL;
+				goto error;
+			}
 		}
 
-		ret = intel_lr_context_render_state_init(ring, ctx);
-		if (ret) {
-			DRM_ERROR("Init render state failed: %d\n", ret);
-			ctx->engine[ring->id].ringbuf = NULL;
-			ctx->engine[ring->id].state = NULL;
-			goto error;
-		}
 		ctx->rcs_initialized = true;
 	}
 

Signature · 6 lines

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Tuesday, December 02, 2014 2:45 PM
To: Daniel, Thomas
Cc: [email protected]
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix startup failure in LRC mode after recent init changes

Quoted 16 lines · Thomas Daniel

A previous commit introduced engine init changes:

    commit 372ee59699d9 ("drm/i915: Only init engines once")

This broke execlists as intel_lr_context_render_state_init was trying to emit commands to the RCS for the default context before the ring- init_hw was called.

Made a new gen8_init_rcs_context function and assign in to render ring init_context. Moved call to intel_logical_ring_workarounds_emit into gen8_init_rcs_context to maintain previous functionality.

Moved call to render_state_init from lr_context_deferred_create into gen8_init_rcs_context, and modified deferred_create to call ring->init_context for non-default contexts.

Quoted 8 lines

Modified i915_gem_context_enable to call ring->init_context for the default context.

So init_context will now always be called when the hw is ready - in i915_gem_context_enable for the default context and in lr_context_deferred_create for other contexts.

Signed-off-by Thomas Daniel <[email protected]>

Oops, sorry for breaking things I didn't realize that we bash things into the hw in the deferred create. So merged this patch right away to get the regression out of the way.

It's not quite there yet for lrc context init though. I think we need to
- split intel_lr_context_deferred_create into _alloc and _init functions.
- Call the _alloc part from logical_ring_init init (which is once at driver load now)
- only call _init from ->init_context
- then move all the default context special-case handling out of _alloc into logical_ring_init (i.e. the pinning and similar stuff) and into ->init_hw (lrc hw setup, status page).
- Shuffle the code in i915_gem_context_enable into ring->init_hw functions.

I think this would reduce the confusion we have a lot here. And also remove a bunch of execlist special cases.

Thoughts? Signed up?

Cheers, Daniel

Quoted 2 lines

---
 drivers/gpu/drm/i915/i915_gem_context.c |   25 ++++++++++++++++++--

-----

Quoted 1 line

 drivers/gpu/drm/i915/intel_lrc.c        |   30 +++++++++++++++++++----------

-

Quoted 11 lines

 2 files changed, 37 insertions(+), 18 deletions(-)

drivers/gpu/drm/i915/i915_gem_context.c +0 -0

index 3c3a9ff..5cd2b97 100644
@@ -408,14 +408,25 @@ int i915_gem_context_enable(struct
 drm_i915_private *dev_priv)
 
 	BUG_ON(!dev_priv->ring[RCS].default_context);

drivers/gpu/drm/i915/i915_gem_context.c int i915_gem_context_enable(struct · line 418

Quoted 10 lines

drivers/gpu/drm/i915/i915_gem_context.c +7 -2

 
-	if (i915.enable_execlists)
-		return 0;
+	if (i915.enable_execlists) {
+		for_each_ring(ring, dev_priv, i) {
+			if (ring->init_context) {
+				ret = ring->init_context(ring,
+						ring->default_context);
+				if (ret) {
+					DRM_ERROR("ring init context:

%d\n",

drivers/gpu/drm/i915/intel_lrc.c static int gen8_emit_request(struct

Quoted 27 lines

drivers/gpu/drm/i915/i915_gem_context.c +11 -5

+							ret);
+					return ret;
+				}
+			}
+		}
 
-	for_each_ring(ring, dev_priv, i) {
-		ret = i915_switch_context(ring, ring->default_context);
-		if (ret)
-			return ret;
-	}
+	} else
+		for_each_ring(ring, dev_priv, i) {
+			ret = i915_switch_context(ring, ring-
 default_context);
+			if (ret)
+				return ret;
+		}
 

drivers/gpu/drm/i915/i915_gem_context.c +0 -0

 	return 0;
 }

drivers/gpu/drm/i915/intel_lrc.c +0 -0

index 4ffb08c..79ef40c 100644
@@ -1336,6 +1336,18 @@ static int gen8_emit_request(struct

intel_ringbuffer *ringbuf)

drivers/gpu/drm/i915/intel_lrc.c static int logical_render_ring_init(struct

Quoted 18 lines

drivers/gpu/drm/i915/intel_lrc.c +11 -0

 	return 0;
 }
 
+static int gen8_init_rcs_context(struct intel_engine_cs *ring,
+		       struct intel_context *ctx)
+{
+	int ret;
+
+	ret = intel_logical_ring_workarounds_emit(ring, ctx);
+	if (ret)
+		return ret;
+
+	return intel_lr_context_render_state_init(ring, ctx); }
+
 /**
  * intel_logical_ring_cleanup() - deallocate the Engine Command Streamer
  *

@@ -1409,7 +1421,7 @@ static int logical_render_ring_init(struct

drm_device *dev)

drivers/gpu/drm/i915/intel_lrc.c static int logical_render_ring_init(struct · line 1421

Quoted 1 line

drivers/gpu/drm/i915/intel_lrc.c +0 -0

 		ring->irq_keep_mask |=

GT_RENDER_L3_PARITY_ERROR_INTERRUPT;

Quoted 39 lines

drivers/gpu/drm/i915/intel_lrc.c +7 -12

 	ring->init_hw = gen8_init_render_ring;
-	ring->init_context = intel_logical_ring_workarounds_emit;
+	ring->init_context = gen8_init_rcs_context;
 	ring->cleanup = intel_fini_pipe_control;
 	ring->get_seqno = gen8_get_seqno;
 	ring->set_seqno = gen8_set_seqno;

@@ -1905,21 +1917,17 @@ int intel_lr_context_deferred_create(struct
 intel_context *ctx,
 
 	if (ctx == ring->default_context)
 		lrc_setup_hardware_status_page(ring, ctx_obj);
-
-	if (ring->id == RCS && !ctx->rcs_initialized) {
+	else if (ring->id == RCS && !ctx->rcs_initialized) {
 		if (ring->init_context) {
 			ret = ring->init_context(ring, ctx);
-			if (ret)
+			if (ret) {
 				DRM_ERROR("ring init context: %d\n", ret);
+				ctx->engine[ring->id].ringbuf = NULL;
+				ctx->engine[ring->id].state = NULL;
+				goto error;
+			}
 		}
 
-		ret = intel_lr_context_render_state_init(ring, ctx);
-		if (ret) {
-			DRM_ERROR("Init render state failed: %d\n", ret);
-			ctx->engine[ring->id].ringbuf = NULL;
-			ctx->engine[ring->id].state = NULL;
-			goto error;
-		}
 		ctx->rcs_initialized = true;
 	}
 
--

1.7.9.5

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch