[Intel-gfx] [PATCH 3/4] drm/i915: Skip cancelled requests in flight

[Chris Wilson]

Check the error of the fence upon submission and skip the request
(clearing out the dispatch and only processing the breadcrumb update) if
we are propagating an earlier failure. This means that we cancel
requests inflight without having to override the engine->submit_request
callback, eliminating the need to stop the machine to do so and allows
us to recover afterwards.

The caveat is that we do depend upon error propagation along the
dependency chains to skip pending requests following the wedged contexts.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Mika Kuoppala <mika.kuoppala at intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 44 +++------------------------------
 drivers/gpu/drm/i915/i915_gem_request.c | 21 ++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_request.h |  2 ++
 3 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e29f9400c9d1..d2e2dc19bc8f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2778,20 +2778,7 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 
 static void skip_request(struct drm_i915_gem_request *request)
 {
-	void *vaddr = request->ring->vaddr;
-	u32 head;
-
-	/* As this request likely depends on state from the lost
-	 * context, clear out all the user operations leaving the
-	 * breadcrumb at the end (so we get the fence notifications).
-	 */
-	head = request->head;
-	if (request->postfix < head) {
-		memset(vaddr + head, 0, request->ring->size - head);
-		head = 0;
-	}
-	memset(vaddr + head, 0, request->postfix - head);
-
+	i915_gem_request_skip(request);
 	dma_fence_set_error(&request->fence, -EIO);
 }
 
@@ -2915,26 +2902,11 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void nop_submit_request(struct drm_i915_gem_request *request)
-{
-	dma_fence_set_error(&request->fence, -EIO);
-	i915_gem_request_submit(request);
-	intel_engine_init_global_seqno(request->engine, request->global_seqno);
-}
-
 static void engine_set_wedged(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *request;
 	unsigned long flags;
 
-	/* We need to be sure that no thread is running the old callback as
-	 * we install the nop handler (otherwise we would submit a request
-	 * to hardware that will never complete). In order to prevent this
-	 * race, we wait until the machine is idle before making the swap
-	 * (using stop_machine()).
-	 */
-	engine->submit_request = nop_submit_request;
-
 	/* Mark all executing requests as skipped */
 	spin_lock_irqsave(&engine->timeline->lock, flags);
 	list_for_each_entry(request, &engine->timeline->requests, link)
@@ -2969,24 +2941,16 @@ static void engine_set_wedged(struct intel_engine_cs *engine)
 	}
 }
 
-static int __i915_gem_set_wedged_BKL(void *data)
+void i915_gem_set_wedged(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *i915 = data;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	for_each_engine(engine, i915, id)
-		engine_set_wedged(engine);
-
-	return 0;
-}
-
-void i915_gem_set_wedged(struct drm_i915_private *dev_priv)
-{
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 	set_bit(I915_WEDGED, &dev_priv->gpu_error.flags);
 
-	stop_machine(__i915_gem_set_wedged_BKL, dev_priv, NULL);
+	for_each_engine(engine, dev_priv, id)
+		engine_set_wedged(engine);
 
 	i915_gem_context_lost(dev_priv);
 	i915_gem_retire_requests(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 1e1d9f2072cd..e76c6e83a794 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -477,6 +477,23 @@ void i915_gem_request_unsubmit(struct drm_i915_gem_request *request)
 	spin_unlock_irqrestore(&engine->timeline->lock, flags);
 }
 
+void i915_gem_request_skip(struct drm_i915_gem_request *request)
+{
+	void *vaddr = request->ring->vaddr;
+	u32 head;
+
+	/* As this request likely depends on state from the lost
+	 * context, clear out all the user operations leaving the
+	 * breadcrumb at the end (so we get the fence notifications).
+	 */
+	head = request->head;
+	if (request->postfix < head) {
+		memset(vaddr + head, 0, request->ring->size - head);
+		head = 0;
+	}
+	memset(vaddr + head, 0, request->postfix - head);
+}
+
 static int __i915_sw_fence_call
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
@@ -486,6 +503,10 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 	switch (state) {
 	case FENCE_COMPLETE:
 		trace_i915_gem_request_submit(request);
+		if (unlikely(fence->error == -EIO)) {
+			i915_gem_request_skip(request);
+			dma_fence_set_error(&request->fence, fence->error);
+		}
 		request->engine->submit_request(request);
 		break;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 5018e55922f0..6fdfb801bcee 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -352,6 +352,8 @@ static inline bool i915_spin_request(const struct drm_i915_gem_request *request,
 		__i915_spin_request(request, seqno, state, timeout_us));
 }
 
+void i915_gem_request_skip(struct drm_i915_gem_request *request);
+
 /* We treat requests as fences. This is not be to confused with our
  * "fence registers" but pipeline synchronisation objects ala GL_ARB_sync.
  * We use the fences to synchronize access from the CPU with activity on the
-- 
2.11.0