[Intel-gfx] [PATCH 46/50] drm/i915/bdw: Avoid non-lite-restore preemptions

[oscar.mateo at intel.com]

From: Oscar Mateo <oscar.mateo at intel.com>

In the current Execlists feeding mechanism, full preemption is not
supported yet: only lite-restores are allowed (this is: the GPU
simply samples a new tail pointer for the context currently in
execution).

But we have identified an scenario in which a full preemption occurs:
1) We submit two contexts for execution (A & B).
2) The GPU finishes with the first one (A), switches to the second one
(B) and informs us.
3) We submit B again (hoping to cause a lite restore) together with C,
but in the time we spend writing to the ELSP, the GPU finishes B.
4) The GPU start executing B again (since we told it so).
5) We receive a B finished interrupt and, mistakenly, we submit C (again)
and D, causing a full preemption of B.

By keeping a better track of our submissions, we can avoid the scenario
described above.

Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  3 +++
 drivers/gpu/drm/i915/intel_lrc.c | 28 ++++++++++++++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 07b8bdc..c797e63 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1750,6 +1750,9 @@ struct drm_i915_gem_request {
 	struct list_head execlist_link;
 	/** Struct to handle this request in the bottom half of an interrupt */
 	struct work_struct work;
+
+	/** No. of times this request has been sent to the ELSP */
+	int elsp_submitted;
 };
 
 struct drm_i915_file_private {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 49f6c9d..a13a570 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -294,6 +294,7 @@ static void gen8_switch_context_unqueue(struct intel_engine *ring)
 		else if (req0->ctx == cursor->ctx) {
 			/* Same ctx: ignore first request, as second request
 			 * will update tail past first request's workload */
+			cursor->elsp_submitted = req0->elsp_submitted;
 			list_del(&req0->execlist_link);
 			queue_work(dev_priv->wq, &req0->work);
 			req0 = cursor;
@@ -303,8 +304,14 @@ static void gen8_switch_context_unqueue(struct intel_engine *ring)
 		}
 	}
 
+	WARN_ON(req1 && req1->elsp_submitted);
+
 	BUG_ON(gen8_switch_context(ring, req0->ctx, req0->tail,
 			req1? req1->ctx : NULL, req1? req1->tail : 0));
+
+	req0->elsp_submitted++;
+	if (req1)
+		req1->elsp_submitted++;
 }
 
 static bool check_remove_request(struct intel_engine *ring, u32 request_id)
@@ -320,9 +327,13 @@ static bool check_remove_request(struct intel_engine *ring, u32 request_id)
 		struct drm_i915_gem_object *ctx_obj =
 				head_req->ctx->engine[ring->id].obj;
 		if (intel_get_lr_contextid(ctx_obj) == request_id) {
-			list_del(&head_req->execlist_link);
-			queue_work(dev_priv->wq, &head_req->work);
-			return true;
+			WARN(head_req->elsp_submitted == 0,
+					"Never submitted head request\n");
+			if (--head_req->elsp_submitted <= 0) {
+				list_del(&head_req->execlist_link);
+				queue_work(dev_priv->wq, &head_req->work);
+				return true;
+			}
 		}
 	}
 
@@ -355,7 +366,16 @@ void gen8_handle_context_events(struct intel_engine *ring)
 		status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
 				(read_pointer % 6) * 8 + 4);
 
-		if (status & GEN8_CTX_STATUS_COMPLETE) {
+		if (status & GEN8_CTX_STATUS_PREEMPTED) {
+			if (status & GEN8_CTX_STATUS_LITE_RESTORE) {
+				if (check_remove_request(ring, status_id))
+					WARN(1, "Lite Restored request removed from queue\n");
+			} else
+				WARN(1, "Preemption without Lite Restore\n");
+		}
+
+		 if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) ||
+		     (status & GEN8_CTX_STATUS_ELEMENT_SWITCH)) {
 			if (check_remove_request(ring, status_id))
 				submit_contexts++;
 		}
-- 
1.9.0