[Intel-gfx] [PATCH 01/10] drm/i915: Refactor activity tracking for requests

Chris Wilson chris at chris-wilson.co.uk
Mon Dec 14 03:36:26 PST 2015


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_debugfs.c     |   4 +-
 drivers/gpu/drm/i915/i915_drv.h         |  10 ---
 drivers/gpu/drm/i915/i915_gem.c         | 149 ++++++++------------------------
 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 |  39 ++++++---
 drivers/gpu/drm/i915/i915_gem_request.h |  26 +++++-
 drivers/gpu/drm/i915/i915_gpu_error.c   |   4 +-
 drivers/gpu/drm/i915/intel_lrc.c        |   1 -
 drivers/gpu/drm/i915/intel_ringbuffer.c |   7 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  12 ---
 12 files changed, 93 insertions(+), 240 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 3df88be4752e..14794370db44 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -21,7 +21,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_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index dafc58b94c9f..68310ba73d23 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -695,13 +695,13 @@ static int i915_gem_request_info(struct seq_file *m, void *data)
 		int count;
 
 		count = 0;
-		list_for_each_entry(req, &ring->request_list, list)
+		list_for_each_entry(req, &ring->request_list, link)
 			count++;
 		if (count == 0)
 			continue;
 
 		seq_printf(m, "%s requests: %d\n", ring->name, count);
-		list_for_each_entry(req, &ring->request_list, list) {
+		list_for_each_entry(req, &ring->request_list, link) {
 			struct task_struct *task;
 
 			rcu_read_lock();
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43de7c2c6b3e..dbcb7659ba2b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -444,8 +444,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;
@@ -2014,7 +2012,6 @@ struct drm_i915_gem_object {
 	struct drm_mm_node *stolen;
 	struct list_head global_list;
 
-	struct list_head ring_list[I915_NUM_RINGS];
 	/** Used in execbuf to temporarily hold a ref */
 	struct list_head obj_exec_link;
 
@@ -3079,13 +3076,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 b412b9291d91..a7c39d5fbcca 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -38,14 +38,8 @@
 #include <linux/pci.h>
 #include <linux/dma-buf.h>
 
-#define RQ_BUG_ON(expr)
-
 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)
@@ -119,7 +113,6 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	WARN_ON(i915_verify_lists(dev));
 	return 0;
 }
 
@@ -1119,27 +1112,14 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 		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);
-		}
+		ret = i915_wait_request(obj->last_write.request);
+		if (ret)
+			return ret;
 	} else {
 		for (i = 0; i < I915_NUM_RINGS; 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);
 		}
 		RQ_BUG_ON(obj->active);
 	}
@@ -1147,20 +1127,6 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
 	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);
-
-	i915_gem_request_retire__upto(req);
-}
-
 /* A nonblocking variant of the above wait. This is a highly dangerous routine
  * as the object state may change during this call.
  */
@@ -1208,7 +1174,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_unreference(requests[i]);
 	}
 
@@ -2061,35 +2027,37 @@ 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->ring_list[engine->id], &engine->active_list);
 	i915_gem_request_mark_active(req, &obj->last_read[engine->id]);
-
 	list_move_tail(&vma->mm_list, &vma->vm->active_list);
 }
 
 static void
-i915_gem_object_retire__write(struct drm_i915_gem_object *obj)
+i915_gem_object_retire__fence(struct drm_i915_gem_request_active *active,
+			      struct drm_i915_gem_request *req)
 {
-	RQ_BUG_ON(obj->last_write.request == NULL);
-	RQ_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 drm_i915_gem_request_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 drm_i915_gem_request_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;
 
-	RQ_BUG_ON(obj->last_read[ring].request == NULL);
-	RQ_BUG_ON(!(obj->active & (1 << ring)));
-
-	list_del_init(&obj->ring_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);
+	RQ_BUG_ON((obj->flags & (1 << (ring + I915_BO_ACTIVE_SHIFT))) == 0);
 
 	obj->active &= ~(1 << ring);
 	if (obj->active)
@@ -2099,15 +2067,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->base.dev)->mm.bound_list);
+	list_move_tail(&obj->global_list, &request->i915->mm.bound_list);
 
 	list_for_each_entry(vma, &obj->vma_list, vma_link) {
 		if (!list_empty(&vma->mm_list))
 			list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
 	}
 
-	i915_gem_request_assign(&obj->last_fence.request, NULL);
 	drm_gem_object_unreference(&obj->base);
 }
 
@@ -2179,7 +2145,7 @@ i915_gem_find_active_request(struct intel_engine_cs *ring)
 	if (ring->seqno_barrier)
 		ring->seqno_barrier(ring);
 
-	list_for_each_entry(request, &ring->request_list, list) {
+	list_for_each_entry(request, &ring->request_list, link) {
 		if (i915_gem_request_completed(request))
 			continue;
 
@@ -2204,7 +2170,7 @@ static void i915_gem_reset_ring_status(struct intel_engine_cs *ring)
 
 	i915_set_reset_status(dev_priv, request->ctx, ring_hung);
 
-	list_for_each_entry_continue(request, &ring->request_list, list)
+	list_for_each_entry_continue(request, &ring->request_list, link)
 		i915_set_reset_status(dev_priv, request->ctx, false);
 }
 
@@ -2212,16 +2178,6 @@ static void i915_gem_reset_ring_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,
-				       ring_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
@@ -2258,7 +2214,7 @@ static void i915_gem_reset_ring_cleanup(struct intel_engine_cs *engine)
 
 		request = list_last_entry(&engine->request_list,
 					  struct drm_i915_gem_request,
-					  list);
+					  link);
 
 		i915_gem_request_retire__upto(request);
 	}
@@ -2298,8 +2254,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));
 }
 
 /**
@@ -2308,8 +2262,6 @@ void i915_gem_reset(struct drm_device *dev)
 void
 i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 {
-	WARN_ON(i915_verify_lists(ring->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
@@ -2320,32 +2272,13 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
 
 		request = list_first_entry(&ring->request_list,
 					   struct drm_i915_gem_request,
-					   list);
+					   link);
 
 		if (!i915_gem_request_completed(request))
 			break;
 
 		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(&ring->active_list)) {
-		struct drm_i915_gem_object *obj;
-
-		obj = list_first_entry(&ring->active_list,
-				      struct drm_i915_gem_object,
-				      ring_list[ring->id]);
-
-		if (!list_empty(&obj->last_read[ring->id].request->list))
-			break;
-
-		i915_gem_object_retire__read(obj, ring->id);
-	}
-
-	WARN_ON(i915_verify_lists(ring->dev));
 }
 
 void
@@ -2446,14 +2379,8 @@ i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
 		if (req == NULL)
 			continue;
 
-		if (list_empty(&req->list))
-			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;
@@ -2519,8 +2446,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_RINGS; i++) {
 		if (obj->last_read[i].request == NULL)
 			continue;
@@ -2528,6 +2453,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		req[n++] = i915_gem_request_reference(obj->last_read[i].request);
 	}
 
+out:
+	drm_gem_object_unreference(&obj->base);
 	mutex_unlock(&dev->struct_mutex);
 
 	for (i = 0; i < n; i++) {
@@ -2538,11 +2465,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 		i915_gem_request_unreference(req[i]);
 	}
 	return ret;
-
-out:
-	drm_gem_object_unreference(&obj->base);
-	mutex_unlock(&dev->struct_mutex);
-	return ret;
 }
 
 static int
@@ -2566,7 +2488,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])
@@ -2763,7 +2685,6 @@ int i915_gpu_idle(struct drm_device *dev)
 			return ret;
 	}
 
-	WARN_ON(i915_verify_lists(dev));
 	return 0;
 }
 
@@ -3774,7 +3695,12 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->global_list);
 	for (i = 0; i < I915_NUM_RINGS; i++)
-		INIT_LIST_HEAD(&obj->ring_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);
@@ -4341,7 +4267,6 @@ i915_gem_cleanup_ringbuffer(struct drm_device *dev)
 static void
 init_ring_lists(struct intel_engine_cs *ring)
 {
-	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->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 17299d04189f..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 *ring;
-	int err = 0;
-	int i;
-
-	if (warned)
-		return 0;
-
-	for_each_ring(ring, dev_priv, i) {
-		list_for_each_entry(obj, &ring->active_list, ring_list[ring->id]) {
-			if (obj->base.dev != dev ||
-			    !atomic_read(&obj->base.refcount.refcount)) {
-				DRM_ERROR("%s: freed active obj %p\n",
-					  ring->name, obj);
-				err++;
-				break;
-			} else if (!obj->active ||
-				   obj->last_read_req[ring->id] == NULL) {
-				DRM_ERROR("%s: invalid active obj %p\n",
-					  ring->name, obj);
-				err++;
-			} else if (obj->base.write_domain) {
-				DRM_ERROR("%s: invalid write obj %p (w %x)\n",
-					  ring->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 ab29c237ffa9..ff085efcf0e5 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 79c15cdb776b..95b7a993579d 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -236,7 +236,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 		   engine->fence_context,
 		   seqno);
 
-	INIT_LIST_HEAD(&req->list);
+	INIT_LIST_HEAD(&req->active_list);
 	req->i915 = dev_priv;
 	req->engine = engine;
 	req->reset_counter = reset_counter;
@@ -323,7 +323,7 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
 
 static void __i915_gem_request_release(struct drm_i915_gem_request *request)
 {
-	list_del_init(&request->list);
+	list_del_init(&request->link);
 
 	i915_gem_request_remove_from_client(request);
 
@@ -343,6 +343,8 @@ void i915_gem_request_cancel(struct drm_i915_gem_request *req)
 
 static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 {
+	struct drm_i915_gem_request_active *node, *next;
+
 	trace_i915_gem_request_retire(request);
 
 	/* We know the GPU must have read the request to have
@@ -354,6 +356,21 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 * completion order.
 	 */
 	request->ring->last_retired_head = request->postfix;
+
+	/* 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(node, next, &request->active_list, link) {
+		/* node->retire() may free the node, so decouple it first
+		 * and pass along the auxiliary information.
+		 */
+		INIT_LIST_HEAD(&node->link);
+		node->request = NULL;
+
+		node->retire(node, request);
+	}
+
 	__i915_gem_request_release(request);
 }
 
@@ -364,18 +381,15 @@ i915_gem_request_retire__upto(struct drm_i915_gem_request *req)
 	struct drm_i915_gem_request *tmp;
 
 	lockdep_assert_held(&engine->dev->struct_mutex);
-
-	if (list_empty(&req->list))
+	if (list_empty(&req->link))
 		return;
 
 	do {
 		tmp = list_first_entry(&engine->request_list,
-				       typeof(*tmp), list);
+				       typeof(*tmp), link);
 
 		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)
@@ -467,7 +481,7 @@ void __i915_add_request(struct drm_i915_gem_request *request,
 	request->emitted_jiffies = jiffies;
 	request->previous_seqno = request->engine->last_submitted_seqno;
 	request->engine->last_submitted_seqno = request->fence.seqno;
-	list_add_tail(&request->list, &request->engine->request_list);
+	list_add_tail(&request->link, &request->engine->request_list);
 
 	trace_i915_gem_request_add(request);
 
@@ -581,9 +595,6 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 
 	might_sleep();
 
-	if (list_empty(&req->list))
-		return 0;
-
 	if (i915_gem_request_completed(req))
 		return 0;
 
@@ -740,10 +751,10 @@ i915_wait_request(struct drm_i915_gem_request *req)
 {
 	int ret;
 
-	BUG_ON(req == NULL);
-
-	BUG_ON(!mutex_is_locked(&req->i915->dev->struct_mutex));
+	if (req == NULL)
+		return 0;
 
+	lockdep_assert_held(&req->i915->dev->struct_mutex);
 	ret = __i915_wait_request(req, req->i915->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 90b188545e26..fa785ce43534 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -84,12 +84,13 @@ 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;
 
-	/** global list entry for this request */
-	struct list_head list;
+	/** engine->request_list entry for this request */
+	struct list_head link;
 
 	struct drm_i915_file_private *file_priv;
 	/** file_priv list entry for this request */
@@ -118,9 +119,16 @@ struct drm_i915_gem_request {
 	int elsp_submitted;
 };
 
+#if 0
+#define RQ_BUG_ON(expr) BUG_ON(expr)
+#else
+#define RQ_BUG_ON(expr)
+#endif
+
 struct drm_i915_gem_request *
 i915_gem_request_alloc(struct intel_engine_cs *ring,
 		       struct intel_context *ctx);
+
 void i915_gem_request_cancel(struct drm_i915_gem_request *req);
 int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
 				   struct drm_file *file);
@@ -202,13 +210,25 @@ static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req)
 
 struct drm_i915_gem_request_active {
 	struct drm_i915_gem_request *request;
+	struct list_head link;
+	void (*retire)(struct drm_i915_gem_request_active *,
+		       struct drm_i915_gem_request *);
 };
 
+inline static void init_request_active(struct drm_i915_gem_request_active *active,
+				       void (*func)(struct drm_i915_gem_request_active *,
+						    struct drm_i915_gem_request *))
+{
+	INIT_LIST_HEAD(&active->link);
+	active->retire = func;
+}
+
 static inline void
 i915_gem_request_mark_active(struct drm_i915_gem_request *request,
 			     struct drm_i915_gem_request_active *active)
 {
-	i915_gem_request_assign(&active->request, request);
+	active->request = request;
+	list_move(&active->link, &request->active_list);
 }
 
 #endif /* I915_GEM_REQUEST_H */
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 82baa370ae09..d27c07982c89 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1058,7 +1058,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
 		i915_gem_record_active_context(engine, error, &error->ring[i]);
 
 		count = 0;
-		list_for_each_entry(request, &engine->request_list, list)
+		list_for_each_entry(request, &engine->request_list, link)
 			count++;
 
 		error->ring[i].num_requests = count;
@@ -1071,7 +1071,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
 		}
 
 		count = 0;
-		list_for_each_entry(request, &engine->request_list, list) {
+		list_for_each_entry(request, &engine->request_list, link) {
 			struct drm_i915_error_request *erq;
 
 			if (count >= error->ring[i].num_requests) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3c1d96b83491..db7061fac0db 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1527,7 +1527,6 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
 	ring->dev = dev;
 	ring->i915 = to_i915(dev);
 	ring->fence_context = fence_context_alloc(1);
-	INIT_LIST_HEAD(&ring->active_list);
 	INIT_LIST_HEAD(&ring->request_list);
 	i915_gem_batch_pool_init(dev, &ring->batch_pool);
 	intel_engine_init_breadcrumbs(ring);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3c0c97e0dff5..83641dc0125a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1976,7 +1976,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);
@@ -2060,7 +2059,7 @@ int intel_engine_idle(struct intel_engine_cs *ring)
 
 	req = list_entry(ring->request_list.prev,
 			struct drm_i915_gem_request,
-			list);
+			link);
 
 	/* Make sure we do not trigger any retires */
 	return __i915_wait_request(req,
@@ -2135,7 +2134,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 	/* The whole point of reserving space is to not wait! */
 	WARN_ON(ring->reserved_in_use);
 
-	list_for_each_entry(target, &engine->request_list, list) {
+	list_for_each_entry(target, &engine->request_list, link) {
 		/*
 		 * The request queue is per-engine, so can contain requests
 		 * from multiple ringbuffers. Here, we must ignore any that
@@ -2151,7 +2150,7 @@ static int wait_for_space(struct drm_i915_gem_request *req, int bytes)
 			break;
 	}
 
-	if (WARN_ON(&target->list == &engine->request_list))
+	if (WARN_ON(&target->link == &engine->request_list))
 		return -ENOSPC;
 
 	ret = i915_wait_request(target);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 004054bf85a7..968c1db71da0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -299,18 +299,6 @@ struct intel_engine_cs {
 #define I915_DISPATCH_RS     0x4
 
 	/**
-	 * 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.6.4



More information about the Intel-gfx mailing list