drm/i915: Refactor activity tracking for requests

With the introduction of requests, we amplified the number of atomic
refcounted objects we use and update every execbuffer; from none to
several references, and a set of references that need to be changed. We
also introduced interesting side-effects in the order of retiring
requests and objects.

Instead of independently tracking the last request for an object, track
the active objects for each request. The object will reside in the
buffer list of its most recent active request and so we reduce the kref
interchange to a list_move. Now retirements are entirely driven by the
request, dramatically simplifying activity tracking on the object
themselves, and removing the ambiguity between retiring objects and
retiring requests.

Furthermore with the consolidation of managing the activity tracking
centrally, we can look forward to using RCU to enable lockless lookup of
the current active requests for an object. In the future, we will be
able to query the status or wait upon rendering to an object without
even touching the struct_mutex BKL.

All told, less code, simpler and faster, and more extensible.

v2: Add a typedef for the function pointer for convenience later.
v3: Make the noop retirement callback explicit. Allow passing NULL to
the init_request_active() which is expanded to a common noop function.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470293567-10811-16-git-send-email-chris@chris-wilson.co.uk

1 files changed, 43 insertions, 7 deletions

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 7802156d481a..cdaaeb6dd913 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -22,6 +22,8 @@
  *
  */
 
+#include <linux/prefetch.h>
+
 #include "i915_drv.h"
 
 static const char *i915_fence_get_driver_name(struct fence *fence)
@@ -157,8 +159,16 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
 	request->pid = NULL;
 }
 
+void i915_gem_retire_noop(struct i915_gem_active *active,
+			  struct drm_i915_gem_request *request)
+{
+	/* Space left intentionally blank */
+}
+
 static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 {
+	struct i915_gem_active *active, *next;
+
 	trace_i915_gem_request_retire(request);
 	list_del_init(&request->link);
 
@@ -172,6 +182,33 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	request->ring->last_retired_head = request->postfix;
 
+	/* Walk through the active list, calling retire on each. This allows
+	 * objects to track their GPU activity and mark themselves as idle
+	 * when their *last* active request is completed (updating state
+	 * tracking lists for eviction, active references for GEM, etc).
+	 *
+	 * As the ->retire() may free the node, we decouple it first and
+	 * pass along the auxiliary information (to avoid dereferencing
+	 * the node after the callback).
+	 */
+	list_for_each_entry_safe(active, next, &request->active_list, link) {
+		/* In microbenchmarks or focusing upon time inside the kernel,
+		 * we may spend an inordinate amount of time simply handling
+		 * the retirement of requests and processing their callbacks.
+		 * Of which, this loop itself is particularly hot due to the
+		 * cache misses when jumping around the list of i915_gem_active.
+		 * So we try to keep this loop as streamlined as possible and
+		 * also prefetch the next i915_gem_active to try and hide
+		 * the likely cache miss.
+		 */
+		prefetchw(next);
+
+		INIT_LIST_HEAD(&active->link);
+		active->request = NULL;
+
+		active->retire(active, request);
+	}
+
 	i915_gem_request_remove_from_client(request);
 
 	if (request->previous_context) {
@@ -200,8 +237,6 @@ void i915_gem_request_retire_upto(struct drm_i915_gem_request *req)
 
 		i915_gem_request_retire(tmp);
 	} while (tmp != req);
-
-	WARN_ON(i915_verify_lists(engine->dev));
 }
 
 static int i915_gem_check_wedge(unsigned int reset_counter, bool interruptible)
@@ -336,6 +371,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 		   engine->fence_context,
 		   seqno);
 
+	INIT_LIST_HEAD(&req->active_list);
 	req->i915 = dev_priv;
 	req->engine = engine;
 	req->ctx = i915_gem_context_get(ctx);
@@ -570,9 +606,6 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 
 	might_sleep();
 
-	if (list_empty(&req->link))
-		return 0;
-
 	if (i915_gem_request_completed(req))
 		return 0;
 
@@ -705,10 +738,13 @@ int i915_wait_request(struct drm_i915_gem_request *req)
 {
 	int ret;
 
-	GEM_BUG_ON(!req);
 	lockdep_assert_held(&req->i915->drm.struct_mutex);
+	GEM_BUG_ON(list_empty(&req->link));
 
-	ret = __i915_wait_request(req, req->i915->mm.interruptible, NULL, NULL);
+	ret = __i915_wait_request(req,
+				  req->i915->mm.interruptible,
+				  NULL,
+				  NULL);
 	if (ret)
 		return ret;