[PATCH 85/98] drm/i915: Refactor activity tracking for requests

Chris Wilson chris at chris-wilson.co.uk
Wed Apr 20 21:56:58 UTC 2016


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.

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

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile           |   1 -
 drivers/gpu/drm/i915/i915_drv.h         |  10 --
 drivers/gpu/drm/i915/i915_gem.c         | 206 ++++++++++----------------------
 drivers/gpu/drm/i915/i915_gem_debug.c   |  70 -----------
 drivers/gpu/drm/i915/i915_gem_fence.c   |  10 +-
 drivers/gpu/drm/i915/i915_gem_request.c |  37 ++++--
 drivers/gpu/drm/i915/i915_gem_request.h |  16 ++-
 drivers/gpu/drm/i915/intel_lrc.c        |   1 -
 drivers/gpu/drm/i915/intel_ringbuffer.c |   1 -
 drivers/gpu/drm/i915/intel_ringbuffer.h |  12 --
 10 files changed, 107 insertions(+), 257 deletions(-)
 delete mode 100644 drivers/gpu/drm/i915/i915_gem_debug.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 450310fcc093..23be1754f1f3 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -23,7 +23,6 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
 i915-y += i915_cmd_parser.o \
 	  i915_gem_batch_pool.o \
 	  i915_gem_context.o \
-	  i915_gem_debug.o \
 	  i915_gem_dmabuf.o \
 	  i915_gem_evict.o \
 	  i915_gem_execbuffer.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 043b66636f77..cb4802cdb86f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -411,8 +411,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
 #define DRIVER_MINOR		6
 #define DRIVER_PATCHLEVEL	0
 
-#define WATCH_LISTS	0
-
 struct opregion_header;
 struct opregion_acpi;
 struct opregion_swsci;
@@ -2108,7 +2106,6 @@ struct drm_i915_gem_object {
 	struct drm_mm_node *stolen;
 	struct list_head global_list;
 
-	struct list_head engine_list[I915_NUM_ENGINES];
 	/** Used in execbuf to temporarily hold a ref */
 	struct list_head obj_exec_link;
 
@@ -3201,13 +3198,6 @@ static inline bool i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_objec
 		obj->tiling_mode != I915_TILING_NONE;
 }
 
-/* i915_gem_debug.c */
-#if WATCH_LISTS
-int i915_verify_lists(struct drm_device *dev);
-#else
-#define i915_verify_lists(dev) 0
-#endif
-
 /* i915_debugfs.c */
 int i915_debugfs_init(struct drm_minor *minor);
 void i915_debugfs_cleanup(struct drm_minor *minor);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9f98fedad4e2..eb3dc49c3c77 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -41,10 +41,6 @@
 
 static void i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj);
 static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj);
-static void
-i915_gem_object_retire__write(struct drm_i915_gem_object *obj);
-static void
-i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring);
 
 static bool cpu_cache_is_coherent(struct drm_device *dev,
 				  enum i915_cache_level level)
@@ -118,7 +114,6 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	WARN_ON(i915_verify_lists(dev));
 	return 0;
 }
 
@@ -1111,52 +1106,30 @@ int
 i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 			       bool readonly)
 {
-	int ret, i;
+	struct i915_gem_active *active;
+	int nactive;
+
+	lockdep_assert_held(&obj->base.dev->struct_mutex);
 
 	if (!obj->active)
 		return 0;
 
 	if (readonly) {
-		if (obj->last_write.request != NULL) {
-			ret = i915_wait_request(obj->last_write.request);
-			if (ret)
-				return ret;
-
-			i = obj->last_write.request->engine->id;
-			if (obj->last_read[i].request == obj->last_write.request)
-				i915_gem_object_retire__read(obj, i);
-			else
-				i915_gem_object_retire__write(obj);
-		}
+		active = &obj->last_write;
+		nactive = 1;
 	} else {
-		for (i = 0; i < I915_NUM_ENGINES; i++) {
-			if (obj->last_read[i].request == NULL)
-				continue;
-
-			ret = i915_wait_request(obj->last_read[i].request);
-			if (ret)
-				return ret;
-
-			i915_gem_object_retire__read(obj, i);
-		}
-		GEM_BUG_ON(obj->active);
+		active = obj->last_read;
+		nactive = ARRAY_SIZE(obj->last_read);
 	}
 
-	return 0;
-}
-
-static void
-i915_gem_object_retire_request(struct drm_i915_gem_object *obj,
-			       struct drm_i915_gem_request *req)
-{
-	int ring = req->engine->id;
-
-	if (obj->last_read[ring].request == req)
-		i915_gem_object_retire__read(obj, ring);
-	else if (obj->last_write.request == req)
-		i915_gem_object_retire__write(obj);
+	do {
+		int ret = i915_wait_request(active->request);
+		if (ret)
+			return ret;
+	} while (active++, --nactive);
+	GEM_BUG_ON(!readonly && obj->active);
 
-	i915_gem_request_retire_upto(req);
+	return 0;
 }
 
 /* A nonblocking variant of the above wait. This is a highly dangerous routine
@@ -1206,7 +1179,7 @@ i915_gem_object_wait_rendering__nonblocking(struct drm_i915_gem_object *obj,
 
 	for (i = 0; i < n; i++) {
 		if (ret == 0)
-			i915_gem_object_retire_request(obj, requests[i]);
+			i915_gem_request_retire_upto(requests[i]);
 		i915_gem_request_put(requests[i]);
 	}
 
@@ -2090,35 +2063,38 @@ void i915_vma_move_to_active(struct i915_vma *vma,
 		drm_gem_object_reference(&obj->base);
 	obj->active |= intel_engine_flag(engine);
 
-	list_move_tail(&obj->engine_list[engine->id], &engine->active_list);
 	i915_gem_active_set(&obj->last_read[engine->id], req);
 
 	list_move_tail(&vma->vm_link, &vma->vm->active_list);
 }
 
 static void
-i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
+i915_gem_object_retire__fence(struct i915_gem_active *active,
+			      struct drm_i915_gem_request *req)
 {
-	GEM_BUG_ON(obj->last_write.request == NULL);
-	GEM_BUG_ON(!(obj->active & intel_engine_flag(obj->last_write.request->engine)));
+}
 
-	i915_gem_request_assign(&obj->last_write.request, NULL);
-	intel_fb_obj_flush(obj, true, ORIGIN_CS);
+static void
+i915_gem_object_retire__write(struct i915_gem_active *active,
+			      struct drm_i915_gem_request *request)
+{
+	intel_fb_obj_flush(container_of(active,
+					struct drm_i915_gem_object,
+					last_write),
+			   true,
+			   ORIGIN_CS);
 }
 
 static void
-i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
+i915_gem_object_retire__read(struct i915_gem_active *active,
+			     struct drm_i915_gem_request *request)
 {
+	int ring = request->engine->id;
+	struct drm_i915_gem_object *obj =
+		container_of(active, struct drm_i915_gem_object, last_read[ring]);
 	struct i915_vma *vma;
 
-	GEM_BUG_ON(obj->last_read[ring].request == NULL);
-	GEM_BUG_ON(!(obj->active & (1 << ring)));
-
-	list_del_init(&obj->engine_list[ring]);
-	i915_gem_request_assign(&obj->last_read[ring].request, NULL);
-
-	if (obj->last_write.request && obj->last_write.request->engine->id == ring)
-		i915_gem_object_retire__write(obj);
+	GEM_BUG_ON((obj->active & (1 << ring)) == 0);
 
 	obj->active &= ~(1 << ring);
 	if (obj->active)
@@ -2128,14 +2104,13 @@ i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
 	 * so that we don't steal from recently used but inactive objects
 	 * (unless we are forced to ofc!)
 	 */
-	list_move_tail(&obj->global_list, &to_i915(obj)->mm.bound_list);
+	list_move_tail(&obj->global_list, &to_i915(request)->mm.bound_list);
 
 	list_for_each_entry(vma, &obj->vma_list, obj_link) {
 		if (!list_empty(&vma->vm_link))
 			list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
 	}
 
-	i915_gem_request_assign(&obj->last_fence.request, NULL);
 	drm_gem_object_unreference(&obj->base);
 }
 
@@ -2212,16 +2187,6 @@ static void i915_gem_reset_engine_cleanup(struct intel_engine_cs *engine)
 {
 	struct intel_ring *ring;
 
-	while (!list_empty(&engine->active_list)) {
-		struct drm_i915_gem_object *obj;
-
-		obj = list_first_entry(&engine->active_list,
-				       struct drm_i915_gem_object,
-				       engine_list[engine->id]);
-
-		i915_gem_object_retire__read(obj, engine->id);
-	}
-
 	/*
 	 * Clear the execlists queue up before freeing the requests, as those
 	 * are the ones that keep the context and ringbuffer backing objects
@@ -2286,8 +2251,6 @@ void i915_gem_reset(struct drm_device *dev)
 	i915_gem_context_reset(dev);
 
 	i915_gem_restore_fences(dev);
-
-	WARN_ON(i915_verify_lists(dev));
 }
 
 /**
@@ -2296,13 +2259,6 @@ void i915_gem_reset(struct drm_device *dev)
 void
 i915_gem_retire_requests_ring(struct intel_engine_cs *engine)
 {
-	WARN_ON(i915_verify_lists(engine->dev));
-
-	/* Retire requests first as we use it above for the early return.
-	 * If we retire requests last, we may use a later seqno and so clear
-	 * the requests lists without clearing the active list, leading to
-	 * confusion.
-	 */
 	while (!list_empty(&engine->request_list)) {
 		struct drm_i915_gem_request *request;
 
@@ -2315,25 +2271,6 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *engine)
 
 		i915_gem_request_retire_upto(request);
 	}
-
-	/* 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.
-	 */
-	while (!list_empty(&engine->active_list)) {
-		struct drm_i915_gem_object *obj;
-
-		obj = list_first_entry(&engine->active_list,
-				       struct drm_i915_gem_object,
-				       engine_list[engine->id]);
-
-		if (!list_empty(&obj->last_read[engine->id].request->link))
-			break;
-
-		i915_gem_object_retire__read(obj, engine->id);
-	}
-
-	WARN_ON(i915_verify_lists(engine->dev));
 }
 
 void
@@ -2424,13 +2361,13 @@ out:
  * write domains, emitting any outstanding lazy request and retiring and
  * completed requests.
  */
-static int
+static void
 i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
 {
 	int i;
 
 	if (!obj->active)
-		return 0;
+		return;
 
 	for (i = 0; i < I915_NUM_ENGINES; i++) {
 		struct drm_i915_gem_request *req;
@@ -2439,17 +2376,9 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
 		if (req == NULL)
 			continue;
 
-		if (list_empty(&req->link))
-			goto retire;
-
-		if (i915_gem_request_completed(req)) {
+		if (i915_gem_request_completed(req))
 			i915_gem_request_retire_upto(req);
-retire:
-			i915_gem_object_retire__read(obj, i);
-		}
 	}
-
-	return 0;
 }
 
 /**
@@ -2497,10 +2426,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 	}
 
 	/* Need to make sure the object gets inactive eventually. */
-	ret = i915_gem_object_flush_active(obj);
-	if (ret)
-		goto out;
-
+	i915_gem_object_flush_active(obj);
 	if (!obj->active)
 		goto out;
 
@@ -2512,8 +2438,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		goto out;
 	}
 
-	drm_gem_object_unreference(&obj->base);
-
 	for (i = 0; i < I915_NUM_ENGINES; i++) {
 		if (obj->last_read[i].request == NULL)
 			continue;
@@ -2521,6 +2445,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		req[n++] = i915_gem_request_get(obj->last_read[i].request);
 	}
 
+out:
+	drm_gem_object_unreference(&obj->base);
 	mutex_unlock(&dev->struct_mutex);
 
 	for (i = 0; i < n; i++) {
@@ -2531,11 +2457,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		i915_gem_request_put(req[i]);
 	}
 	return ret;
-
-out:
-	drm_gem_object_unreference(&obj->base);
-	mutex_unlock(&dev->struct_mutex);
-	return ret;
 }
 
 static int
@@ -2559,7 +2480,7 @@ __i915_gem_object_sync(struct drm_i915_gem_object *obj,
 		if (ret)
 			return ret;
 
-		i915_gem_object_retire_request(obj, from);
+		i915_gem_request_retire_upto(from);
 	} else {
 		int idx = intel_engine_sync_index(from->engine, to->engine);
 		if (from->fence.seqno <= from->engine->semaphore.sync_seqno[idx])
@@ -2600,27 +2521,30 @@ int
 i915_gem_object_sync(struct drm_i915_gem_object *obj,
 		     struct drm_i915_gem_request *to)
 {
-	const bool readonly = obj->base.pending_write_domain == 0;
-	struct drm_i915_gem_request *req[I915_NUM_ENGINES];
-	int ret, i, n;
+	struct i915_gem_active *active;
+	int nactive;
 
 	if (!obj->active)
 		return 0;
 
-	n = 0;
-	if (readonly) {
-		if (obj->last_write.request)
-			req[n++] = obj->last_write.request;
+	if (obj->base.pending_write_domain) {
+		active = obj->last_read;
+		nactive = ARRAY_SIZE(obj->last_read);
 	} else {
-		for (i = 0; i < I915_NUM_ENGINES; i++)
-			if (obj->last_read[i].request)
-				req[n++] = obj->last_read[i].request;
+		active = &obj->last_write;
+		nactive = 1;
 	}
-	for (i = 0; i < n; i++) {
-		ret = __i915_gem_object_sync(obj, to, req[i]);
+
+	do {
+		int ret;
+
+		if (active->request == NULL)
+			continue;
+
+		ret = __i915_gem_object_sync(obj, to, active->request);
 		if (ret)
 			return ret;
-	}
+	} while (active++, --nactive);
 
 	return 0;
 }
@@ -2762,7 +2686,6 @@ int i915_gpu_idle(struct drm_device *dev)
 			return ret;
 	}
 
-	WARN_ON(i915_verify_lists(dev));
 	return 0;
 }
 
@@ -3688,9 +3611,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	 * become non-busy without any further actions, therefore emit any
 	 * necessary flushes here.
 	 */
-	ret = i915_gem_object_flush_active(obj);
-	if (ret)
-		goto unref;
+	i915_gem_object_flush_active(obj);
 
 	args->busy = 0;
 	if (obj->active) {
@@ -3707,7 +3628,6 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 			args->busy |= obj->last_write.request->engine->exec_id;
 	}
 
-unref:
 	drm_gem_object_unreference(&obj->base);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
@@ -3785,7 +3705,12 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->global_list);
 	for (i = 0; i < I915_NUM_ENGINES; i++)
-		INIT_LIST_HEAD(&obj->engine_list[i]);
+		init_request_active(&obj->last_read[i],
+				    i915_gem_object_retire__read);
+	init_request_active(&obj->last_write,
+			    i915_gem_object_retire__write);
+	init_request_active(&obj->last_fence,
+			    i915_gem_object_retire__fence);
 	INIT_LIST_HEAD(&obj->obj_exec_link);
 	INIT_LIST_HEAD(&obj->vma_list);
 	INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4317,7 +4242,6 @@ i915_gem_cleanup_engines(struct drm_device *dev)
 static void
 init_engine_lists(struct intel_engine_cs *engine)
 {
-	INIT_LIST_HEAD(&engine->active_list);
 	INIT_LIST_HEAD(&engine->request_list);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_debug.c b/drivers/gpu/drm/i915/i915_gem_debug.c
deleted file mode 100644
index a56516482394..000000000000
--- a/drivers/gpu/drm/i915/i915_gem_debug.c
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- * Copyright © 2008 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *    Keith Packard <keithp at keithp.com>
- *
- */
-
-#include <drm/drmP.h>
-#include <drm/i915_drm.h>
-#include "i915_drv.h"
-
-#if WATCH_LISTS
-int
-i915_verify_lists(struct drm_device *dev)
-{
-	static int warned;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct drm_i915_gem_object *obj;
-	struct intel_engine_cs *engine;
-	int err = 0;
-
-	if (warned)
-		return 0;
-
-	for_each_engine(engine, dev_priv) {
-		list_for_each_entry(obj, &engine->active_list,
-				    engine_list[engine->id]) {
-			if (obj->base.dev != dev ||
-			    !atomic_read(&obj->base.refcount.refcount)) {
-				DRM_ERROR("%s: freed active obj %p\n",
-					  engine->name, obj);
-				err++;
-				break;
-			} else if (!obj->active ||
-				   obj->last_read_req[engine->id] == NULL) {
-				DRM_ERROR("%s: invalid active obj %p\n",
-					  engine->name, obj);
-				err++;
-			} else if (obj->base.write_domain) {
-				DRM_ERROR("%s: invalid write obj %p (w %x)\n",
-					  engine->name,
-					  obj, obj->base.write_domain);
-				err++;
-			}
-		}
-	}
-
-	return warned = err;
-}
-#endif /* WATCH_LIST */
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c b/drivers/gpu/drm/i915/i915_gem_fence.c
index 4b949ee04c33..fd1b3fc3db4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -261,15 +261,7 @@ static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
 static int
 i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
 {
-	if (obj->last_fence.request) {
-		int ret = i915_wait_request(obj->last_fence.request);
-		if (ret)
-			return ret;
-
-		i915_gem_request_assign(&obj->last_fence.request, NULL);
-	}
-
-	return 0;
+	return i915_wait_request(obj->last_fence.request);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 718a2ec14145..6566607a253b 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -232,6 +232,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->reset_counter = reset_counter;
@@ -319,6 +320,8 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
 
 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);
 
@@ -332,6 +335,22 @@ 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) {
+		INIT_LIST_HEAD(&active->link);
+		active->request = NULL;
+
+		active->retire(active, request);
+	}
+
 	i915_gem_request_remove_from_client(request);
 
 	if (request->previous_context) {
@@ -349,8 +368,7 @@ void i915_gem_request_retire_upto(struct drm_i915_gem_request *req)
 	struct intel_engine_cs *engine = req->engine;
 	struct drm_i915_gem_request *tmp;
 
-	lockdep_assert_held(&engine->dev->struct_mutex);
-
+	lockdep_assert_held(&to_i915(req)->dev->struct_mutex);
 	if (list_empty(&req->link))
 		return;
 
@@ -360,8 +378,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 void i915_gem_mark_busy(struct drm_i915_private *dev_priv,
@@ -560,9 +576,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;
 
@@ -691,11 +704,13 @@ int i915_wait_request(struct drm_i915_gem_request *req)
 {
 	int ret;
 
-	BUG_ON(req == NULL);
-	BUG_ON(!mutex_is_locked(&to_i915(req)->dev->struct_mutex));
+	if (req == NULL)
+		return 0;
+
+	lockdep_assert_held(&to_i915(req)->dev->struct_mutex);
+	GEM_BUG_ON(list_empty(&req->link));
 
-	ret = __i915_wait_request(req, to_i915(req)->mm.interruptible,
-				  NULL, NULL);
+	ret = __i915_wait_request(req, to_i915(req)->mm.interruptible, NULL, NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index cb4bb7fd756d..fb07ee388a51 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -96,6 +96,7 @@ struct drm_i915_gem_request {
 	/** Batch buffer related to this request if any (used for
 	    error state dump only) */
 	struct drm_i915_gem_object *batch_obj;
+	struct list_head active_list;
 
 	/** Time at which this request was emitted, in jiffies. */
 	unsigned long emitted_jiffies;
@@ -259,13 +260,26 @@ static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req)
  */
 struct i915_gem_active {
 	struct drm_i915_gem_request *request;
+	struct list_head link;
+	void (*retire)(struct i915_gem_active *,
+		       struct drm_i915_gem_request *);
 };
 
 static inline void
+init_request_active(struct i915_gem_active *active,
+		    void (*func)(struct i915_gem_active *,
+				 struct drm_i915_gem_request *))
+{
+	INIT_LIST_HEAD(&active->link);
+	active->retire = func;
+}
+
+static inline void
 i915_gem_active_set(struct i915_gem_active *active,
 		    struct drm_i915_gem_request *request)
 {
-	i915_gem_request_assign(&active->request, request);
+	list_move(&active->link, &request->active_list);
+	active->request = request;
 }
 
 #endif /* I915_GEM_REQUEST_H */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b62030a045b7..237684faf32c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1723,7 +1723,6 @@ logical_ring_setup(struct drm_device *dev, int id,
 
 	engine->fw_domains = fw_domains;
 
-	INIT_LIST_HEAD(&engine->active_list);
 	INIT_LIST_HEAD(&engine->request_list);
 	INIT_LIST_HEAD(&engine->buffers);
 	INIT_LIST_HEAD(&engine->execlist_queue);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c825df8fa5fd..05f6bdb11aa5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2004,7 +2004,6 @@ static int intel_init_engine(struct drm_device *dev,
 	engine->dev = dev;
 	engine->i915 = to_i915(dev);
 	engine->fence_context = fence_context_alloc(1);
-	INIT_LIST_HEAD(&engine->active_list);
 	INIT_LIST_HEAD(&engine->request_list);
 	INIT_LIST_HEAD(&engine->execlist_queue);
 	INIT_LIST_HEAD(&engine->buffers);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 80c21668f6f5..cf8c4c77d7d6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -298,18 +298,6 @@ struct intel_engine_cs {
 	u32 ctx_desc_template;
 
 	/**
-	 * List of objects currently involved in rendering from the
-	 * ringbuffer.
-	 *
-	 * Includes buffers having the contents of their GPU caches
-	 * flushed, not necessarily primitives.  last_read_req
-	 * represents when the rendering involved will be completed.
-	 *
-	 * A reference is held on the buffer while on this list.
-	 */
-	struct list_head active_list;
-
-	/**
 	 * List of breadcrumbs associated with GPU requests currently
 	 * outstanding.
 	 */
-- 
2.8.1



More information about the Intel-gfx-trybot mailing list