[Intel-gfx] [RFC 10/44] drm/i915: Prepare retire_requests to handle out-of-order seqnos

[John.C.Harrison at Intel.com]

From: John Harrison <John.C.Harrison at Intel.com>

A major point of the GPU scheduler is that it re-orders batch buffers after they
have been submitted to the driver. Rather than attempting to re-assign seqno
values, it is much simpler to have each batch buffer keep its initially assigned
number and modify the rest of the driver to cope with seqnos being returned out
of order. In practice, very little code actually needs updating to cope.

One such place is the retire request handler. Rather than stopping as soon as an
uncompleted seqno is found, it must now keep iterating through the requests in
case later seqnos have completed. There is also a problem with doing the free of
the request before the move to inactive. Thus the requests are now moved to a
temporary list first, then the objects de-activated and finally the requests on
the temporary list are freed.
---
 drivers/gpu/drm/i915/i915_gem.c |   60 +++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b784eb2..7e53446 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2602,7 +2602,10 @@ void i915_gem_reset(struct drm_device *dev)
 void
 i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 {
+	struct drm_i915_gem_object *obj, *obj_next;
+	struct drm_i915_gem_request *req, *req_next;
 	uint32_t seqno;
+	LIST_HEAD(deferred_request_free);
 
 	if (list_empty(&ring->request_list))
 		return;
@@ -2611,43 +2614,35 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 
 	seqno = ring->get_seqno(ring, true);
 
-	/* Move any buffers on the active list that are no longer referenced
-	 * by the ringbuffer to the flushing/inactive lists as appropriate,
-	 * before we free the context associated with the requests.
+	/* Note that seqno values might be out of order due to rescheduling and
+	 * pre-emption. Thus both lists must be processed in their entirety
+	 * rather than stopping at the first 'non-passed' entry.
 	 */
-	while (!list_empty(&ring->active_list)) {
-		struct drm_i915_gem_object *obj;
-
-		obj = list_first_entry(&ring->active_list,
-				      struct drm_i915_gem_object,
-				      ring_list);
-
-		if (!i915_seqno_passed(seqno, obj->last_read_seqno))
-			break;
 
-		i915_gem_object_move_to_inactive(obj);
-	}
-
-
-	while (!list_empty(&ring->request_list)) {
-		struct drm_i915_gem_request *request;
-
-		request = list_first_entry(&ring->request_list,
-					   struct drm_i915_gem_request,
-					   list);
-
-		if (!i915_seqno_passed(seqno, request->seqno))
-			break;
+	list_for_each_entry_safe(req, req_next, &ring->request_list, list) {
+		if (!i915_seqno_passed(seqno, req->seqno))
+			continue;
 
-		trace_i915_gem_request_retire(ring, request->seqno);
+		trace_i915_gem_request_retire(ring, req->seqno);
 		/* We know the GPU must have read the request to have
 		 * sent us the seqno + interrupt, so use the position
 		 * of tail of the request to update the last known position
 		 * of the GPU head.
 		 */
-		ring->buffer->last_retired_head = request->tail;
+		ring->buffer->last_retired_head = req->tail;
 
-		i915_gem_free_request(request);
+		list_move_tail(&req->list, &deferred_request_free);
+	}
+
+	/* Move any buffers on the active list that are no longer referenced
+	 * by the ringbuffer to the flushing/inactive lists as appropriate,
+	 * before we free the context associated with the requests.
+	 */
+	list_for_each_entry_safe(obj, obj_next, &ring->active_list, ring_list) {
+		if (!i915_seqno_passed(seqno, obj->last_read_seqno))
+			continue;
+
+		i915_gem_object_move_to_inactive(obj);
 	}
 
 	if (unlikely(ring->trace_irq_seqno &&
@@ -2656,6 +2651,15 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 		ring->trace_irq_seqno = 0;
 	}
 
+	/* Finish processing active list before freeing request */
+	while (!list_empty(&deferred_request_free)) {
+		req = list_first_entry(&deferred_request_free,
+	                               struct drm_i915_gem_request,
+	                               list);
+
+		i915_gem_free_request(req);
+	}
+
 	WARN_ON(i915_verify_lists(ring->dev));
 }
 
-- 
1.7.9.5