Re: [PATCH 2/3] drm/i915: Allow sharing the idle-barrier from other kernel requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 30/07/2019 12:21, Chris Wilson wrote:
By placing our idle-barriers in the i915_active fence tree, we expose
those for reuse by other components that are issuing requests along the
kernel_context. Reusing the proto-barrier active_node is perfectly fine
as the new request implies a context-switch, and so an opportune point
to run the idle-barrier. However, the proto-barrier is not equivalent
to a normal active_node and care must be taken to avoid dereferencing the
ERR_PTR used as its request marker.

Reported-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch")
Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/intel_context.c       |  40 ++-
  drivers/gpu/drm/i915/gt/intel_context.h       |  13 +-
  drivers/gpu/drm/i915/gt/intel_engine_pm.c     |   2 +-
  drivers/gpu/drm/i915/gt/selftest_context.c    | 310 ++++++++++++++++++
  drivers/gpu/drm/i915/i915_active.c            | 246 +++++++++++---
  drivers/gpu/drm/i915/i915_active.h            |   2 +-
  drivers/gpu/drm/i915/i915_active_types.h      |   2 +-
  .../drm/i915/selftests/i915_live_selftests.h  |   3 +-
  8 files changed, 555 insertions(+), 63 deletions(-)
  create mode 100644 drivers/gpu/drm/i915/gt/selftest_context.c

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index d64b45f7ec6d..211ac6568a5d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -162,23 +162,41 @@ static int __intel_context_active(struct i915_active *active)
  	if (err)
  		goto err_ring;
+ return 0;
+
+err_ring:
+	intel_ring_unpin(ce->ring);
+err_put:
+	intel_context_put(ce);
+	return err;
+}
+
+int intel_context_active_acquire(struct intel_context *ce)
+{
+	int err;
+
+	err = i915_active_acquire(&ce->active);
+	if (err)
+		return err;
+
  	/* Preallocate tracking nodes */
  	if (!i915_gem_context_is_kernel(ce->gem_context)) {
  		err = i915_active_acquire_preallocate_barrier(&ce->active,
  							      ce->engine);
-		if (err)
-			goto err_state;
+		if (err) {
+			i915_active_release(&ce->active);
+			return err;
+		}
  	}
return 0;
+}
-err_state:
-	__context_unpin_state(ce->state);
-err_ring:
-	intel_ring_unpin(ce->ring);
-err_put:
-	intel_context_put(ce);
-	return err;
+void intel_context_active_release(struct intel_context *ce)
+{
+	/* Nodes preallocated in intel_context_active() */
+	i915_active_acquire_barrier(&ce->active);
+	i915_active_release(&ce->active);
  }
void
@@ -297,3 +315,7 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)
return rq;
  }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_context.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 23c7e4c0ce7c..07f9924de48f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -104,17 +104,8 @@ static inline void intel_context_exit(struct intel_context *ce)
  		ce->ops->exit(ce);
  }
-static inline int intel_context_active_acquire(struct intel_context *ce)
-{
-	return i915_active_acquire(&ce->active);
-}
-
-static inline void intel_context_active_release(struct intel_context *ce)
-{
-	/* Nodes preallocated in intel_context_active() */
-	i915_active_acquire_barrier(&ce->active);
-	i915_active_release(&ce->active);
-}
+int intel_context_active_acquire(struct intel_context *ce);
+void intel_context_active_release(struct intel_context *ce);
static inline struct intel_context *intel_context_get(struct intel_context *ce)
  {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index e74fbf04a68d..ce54092475da 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -90,7 +90,7 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
  	/* Check again on the next retirement. */
  	engine->wakeref_serial = engine->serial + 1;
- i915_request_add_barriers(rq);
+	i915_request_add_active_barriers(rq);
  	__i915_request_commit(rq);
return false;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
new file mode 100644
index 000000000000..d39b5594cb02
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -0,0 +1,310 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "intel_gt.h"
+
+#include "gem/selftests/mock_context.h"
+#include "selftests/igt_flush_test.h"
+#include "selftests/mock_drm.h"
+
+static int request_sync(struct i915_request *rq)
+{
+	long timeout;
+	int err = 0;
+
+	i915_request_get(rq);
+
+	i915_request_add(rq);
+	timeout = i915_request_wait(rq, 0, HZ / 10);
+	if (timeout < 0)
+		err = timeout;
+	else
+		i915_request_retire_upto(rq);
+
+	i915_request_put(rq);
+
+	return err;
+}
+
+static int context_sync(struct intel_context *ce)
+{
+	struct intel_timeline *tl = ce->ring->timeline;
+	int err = 0;
+
+	do {
+		struct i915_request *rq;
+		long timeout;
+
+		rcu_read_lock();
+		rq = rcu_dereference(tl->last_request.request);
+		if (rq)
+			rq = i915_request_get_rcu(rq);
+		rcu_read_unlock();
+		if (!rq)
+			break;
+
+		timeout = i915_request_wait(rq, 0, HZ / 10);
+		if (timeout < 0)
+			err = timeout;
+		else
+			i915_request_retire_upto(rq);
+
+		i915_request_put(rq);
+	} while (!err);
+
+	return err;
+}
+
+static int __live_active_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *ce;
+	int pass;
+	int err;
+
+	/*
+	 * We keep active contexts alive until after a subsequent context
+	 * switch as the final write from the context-save will be after
+	 * we retire the final request. We track when we unpin the context,
+	 * under the presumption that the final pin is from the last request,
+	 * and instead of immediately unpinning the context, we add a task
+	 * to unpin the context from the next idle-barrier.
+	 *
+	 * This test makes sure that the context is kept alive until a
+	 * subsequent idle-barrier (emitted when the engine wakeref hits 0
+	 * with no more outstanding requests).
+	 */
+
+	if (intel_engine_pm_is_awake(engine)) {
+		pr_err("%s is awake before starting %s!\n",
+		       engine->name, __func__);
+		return -EINVAL;
+	}
+
+	ce = intel_context_create(fixme, engine);
+	if (!ce)
+		return -ENOMEM;
+
+	for (pass = 0; pass <= 2; pass++) {
+		struct i915_request *rq;
+
+		rq = intel_context_create_request(ce);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			goto err;
+		}
+
+		err = request_sync(rq);
+		if (err)
+			goto err;
+
+		/* Context will be kept active until after an idle-barrier. */
+		if (i915_active_is_idle(&ce->active)) {
+			pr_err("context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			goto err;
+		}
+
+		if (!intel_engine_pm_is_awake(engine)) {
+			pr_err("%s is asleep before idle-barrier\n",
+			       engine->name);
+			err = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* Now make sure our idle-barriers are flushed */
+	err = context_sync(engine->kernel_context);
+	if (err)
+		goto err;
+
+	if (!i915_active_is_idle(&ce->active)) {
+		pr_err("context is still active!");
+		err = -EINVAL;
+	}
+
+	if (intel_engine_pm_is_awake(engine)) {
+		struct drm_printer p = drm_debug_printer(__func__);
+
+		intel_engine_dump(engine, &p,
+				  "%s is still awake after idle-barriers\n",
+				  engine->name);
+		GEM_TRACE_DUMP();
+
+		err = -EINVAL;
+		goto err;
+	}
+
+err:
+	intel_context_put(ce);
+	return err;
+}
+
+static int live_active_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_active_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+static int __remote_sync(struct intel_context *ce, struct intel_context *remote)
+{
+	struct i915_request *rq;
+	int err;
+
+	err = intel_context_pin(remote);
+	if (err)
+		return err;
+
+	rq = intel_context_create_request(ce);
+	if (IS_ERR(rq)) {
+		err = PTR_ERR(rq);
+		goto unpin;
+	}
+
+	err = intel_context_prepare_remote_request(remote, rq);
+	if (err) {
+		i915_request_add(rq);
+		goto unpin;
+	}
+
+	err = request_sync(rq);
+
+unpin:
+	intel_context_unpin(remote);
+	return err;
+}
+
+static int __live_remote_context(struct intel_engine_cs *engine,
+				 struct i915_gem_context *fixme)
+{
+	struct intel_context *local, *remote;
+	int pass;
+	int err;
+
+	/*
+	 * Check that our idle barriers do not interfere with normal
+	 * activity tracking. In particular, check that operating
+	 * on the context image remotely (intel_context_prepare_remote_request),
+	 * which inserts foreign fences into intel_context.active, does not
+	 * clobber the idle-barrier.
+	 */
+
+	remote = intel_context_create(fixme, engine);
+	if (!remote)
+		return -ENOMEM;
+
+	local = intel_context_create(fixme, engine);
+	if (!local) {
+		err = -ENOMEM;
+		goto err_remote;
+	}
+
+	for (pass = 0; pass <= 2; pass++) {
+		err = __remote_sync(local, remote);
+		if (err)
+			break;
+
+		err = __remote_sync(engine->kernel_context, remote);
+		if (err)
+			break;
+
+		if (i915_active_is_idle(&remote->active)) {
+			pr_err("remote context is not active; expected idle-barrier (%s pass %d)\n",
+			       engine->name, pass);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	intel_context_put(local);
+err_remote:
+	intel_context_put(remote);
+	return err;
+}
+
+static int live_remote_context(void *arg)
+{
+	struct intel_gt *gt = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context *fixme;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	int err = 0;
+
+	file = mock_file(gt->i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&gt->i915->drm.struct_mutex);
+
+	fixme = live_context(gt->i915, file);
+	if (!fixme) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	for_each_engine(engine, gt->i915, id) {
+		err = __live_remote_context(engine, fixme);
+		if (err)
+			break;
+
+		err = igt_flush_test(gt->i915, I915_WAIT_LOCKED);
+		if (err)
+			break;
+	}
+
+unlock:
+	mutex_unlock(&gt->i915->drm.struct_mutex);
+	mock_file_free(gt->i915, file);
+	return err;
+}
+
+int intel_context_live_selftests(struct drm_i915_private *i915)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(live_active_context),
+		SUBTEST(live_remote_context),
+	};
+	struct intel_gt *gt = &i915->gt;
+
+	if (intel_gt_is_wedged(gt))
+		return 0;
+
+	return intel_gt_live_subtests(tests, gt);
+}
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index d32db8a4db5c..3d50a27ed16c 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -33,6 +33,38 @@ struct active_node {
  	u64 timeline;
  };
+static inline struct active_node *
+node_from_active(struct i915_active_request *active)
+{
+	return container_of(active, struct active_node, base);
+}
+
+#define get_preallocated_barriers(x) llist_del_all(&(x)->preallocated_barriers)
+
+static inline bool is_barrier(const struct i915_active_request *active)
+{
+	return IS_ERR(rcu_access_pointer(active->request));
+}
+
+static inline struct llist_node *barrier_to_ll(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct llist_node *)&node->base.link;
+}
+
+static inline struct intel_engine_cs *
+barrier_to_engine(struct active_node *node)
+{
+	GEM_BUG_ON(!is_barrier(&node->base));
+	return (struct intel_engine_cs *)node->base.link.prev;
+}
+
+static inline struct active_node *barrier_from_ll(struct llist_node *x)
+{
+	return container_of((struct list_head *)x,
+			    struct active_node, base.link);
+}
+
  #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && IS_ENABLED(CONFIG_DEBUG_OBJECTS)
static void *active_debug_hint(void *addr)
@@ -127,7 +159,7 @@ active_retire(struct i915_active *ref)
  static void
  node_retire(struct i915_active_request *base, struct i915_request *rq)
  {
-	active_retire(container_of(base, struct active_node, base)->ref);
+	active_retire(node_from_active(base)->ref);
  }
static struct i915_active_request *
@@ -184,6 +216,7 @@ active_instance(struct i915_active *ref, u64 idx)
  	ref->cache = node;
  	mutex_unlock(&ref->mutex);
+ BUILD_BUG_ON(offsetof(typeof(*node), base));
  	return &node->base;
  }
@@ -201,11 +234,37 @@ void __i915_active_init(struct drm_i915_private *i915,
  	ref->retire = retire;
  	ref->tree = RB_ROOT;
  	ref->cache = NULL;
-	init_llist_head(&ref->barriers);
+	init_llist_head(&ref->preallocated_barriers);
  	atomic_set(&ref->count, 0);
  	__mutex_init(&ref->mutex, "i915_active", key);
  }
+static bool __active_del_barrier(struct i915_active *ref,
+				 struct active_node *node)
+{
+	struct intel_engine_cs *engine = barrier_to_engine(node);
+	struct llist_node *head = NULL, *tail = NULL;
+	struct llist_node *pos, *next;
+
+	GEM_BUG_ON(node->timeline != engine->kernel_context->ring->timeline->fence_context);
+
+	llist_for_each_safe(pos, next, llist_del_all(&engine->barrier_tasks)) {
+		if (node == barrier_from_ll(pos)) {
+			node = NULL;
+			continue;
+		}
+
+		pos->next = head;
+		head = pos;
+		if (!tail)
+			tail = pos;
+	}
+	if (head)
+		llist_add_batch(head, tail, &engine->barrier_tasks);
+
+	return !node;
+}
+
  int i915_active_ref(struct i915_active *ref,
  		    u64 timeline,
  		    struct i915_request *rq)
@@ -224,8 +283,15 @@ int i915_active_ref(struct i915_active *ref,
  		goto out;
  	}
- if (!i915_active_request_isset(active))
-		atomic_inc(&ref->count);
+	if (is_barrier(active)) { /* proto-node used by our idle barrier */
+		__active_del_barrier(ref, node_from_active(active));
+		RCU_INIT_POINTER(active->request, NULL);
+		INIT_LIST_HEAD(&active->link);

So this, when prepare_remote_request calls it on ce->active, will remove the idle barrier from the engine->barriers_list and immediately transfer it to the rq->active_list? Then when this request is retired we know the context is idle? But what if it is the same context? Then it is not idle yet.

+	} else {
+		if (!i915_active_request_isset(active))
+			atomic_inc(&ref->count);
+	}
+	GEM_BUG_ON(!atomic_read(&ref->count));
  	__i915_active_request_set(active, rq);
out:
@@ -312,6 +378,11 @@ int i915_active_wait(struct i915_active *ref)
  	}
rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) {
+		if (is_barrier(&it->base)) { /* unconnected idle-barrier */
+			err = -EBUSY;
+			break;
+		}
+
  		err = i915_active_request_retire(&it->base, BKL(ref));
  		if (err)
  			break;
@@ -374,6 +445,79 @@ void i915_active_fini(struct i915_active *ref)
  }
  #endif
+static inline bool is_idle_barrier(struct active_node *node, u64 idx)
+{
+	return node->timeline == idx && !i915_active_request_isset(&node->base);
+}
+
+static struct active_node *idle_barrier(struct i915_active *ref, u64 idx)
+{
+	struct rb_node *prev, *p;
+
+	if (RB_EMPTY_ROOT(&ref->tree))
+		return NULL;
+
+	mutex_lock(&ref->mutex);
+	GEM_BUG_ON(i915_active_is_idle(ref));
+
+	/*
+	 * Try to reuse any existing barrier nodes already allocated for this
+	 * i915_active, due to overlapping active phases there is likely a

For this i915_active or for this idx? It is this i915_active by definition, no? And idx also has to match in both loops below.

+	 * node kept alive (as we reuse before parking). We prefer to reuse
+	 * completely idle barriers (less hassle in manipulating the llists),
+	 * but otherwise any will do.
+	 */
+	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+		p = &ref->cache->node;
+		goto match;
+	}
+
+	prev = NULL;
+	p = ref->tree.rb_node;
+	while (p) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (is_idle_barrier(node, idx))
+			goto match;
+
+		prev = p;
+		if (node->timeline < idx)
+			p = p->rb_right;
+		else
+			p = p->rb_left;
+	}
+
+	for (p = prev; p; p = rb_next(p)) {
+		struct active_node *node =
+			rb_entry(p, struct active_node, node);
+
+		if (node->timeline > idx)
+			break;
+
+		if (node->timeline < idx)
+			continue;
+
+		if (!i915_active_request_isset(&node->base))
+			goto match;

Isn't this idle barrier, so same as above? How can above not find it, and find it here?

+
+		if (is_barrier(&node->base) && __active_del_barrier(ref, node))
+			goto match;

And this is yet unused barrier with the right idx. Under what circumstances can __active_del_barrier fail then? I think a comment is needed.

+	}
+
+	mutex_unlock(&ref->mutex);
+
+	return NULL;
+
+match:
+	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */
+	if (p == &ref->cache->node)
+		ref->cache = NULL;
+	mutex_unlock(&ref->mutex);
+
+	return rb_entry(p, struct active_node, node);
+}
+
  int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
  					    struct intel_engine_cs *engine)
  {
@@ -382,39 +526,52 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
  	struct llist_node *pos, *next;
  	int err;
- GEM_BUG_ON(!mask);
+	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
+
+	/*
+	 * Preallocate a node for each physical engine supporting the target
+	 * engine (remember virtual engines have more than one sibling).
+	 * We can then use the preallocated nodes in
+	 * i915_active_acquire_barrier()
+	 */
  	for_each_engine_masked(engine, i915, mask, tmp) {
-		struct intel_context *kctx = engine->kernel_context;
+		u64 idx = engine->kernel_context->ring->timeline->fence_context;
  		struct active_node *node;
- node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
-		if (unlikely(!node)) {
-			err = -ENOMEM;
-			goto unwind;
+		node = idle_barrier(ref, idx);
+		if (!node) {
+			node = kmem_cache_alloc(global.slab_cache, GFP_KERNEL);
+			if (!node) {
+				err = ENOMEM;
+				goto unwind;
+			}
+
+			RCU_INIT_POINTER(node->base.request, NULL);
+			node->base.retire = node_retire;
+			node->timeline = idx;
+			node->ref = ref;
  		}
- i915_active_request_init(&node->base,
-					 (void *)engine, node_retire);
-		node->timeline = kctx->ring->timeline->fence_context;
-		node->ref = ref;
-		atomic_inc(&ref->count);
+		if (!i915_active_request_isset(&node->base)) {
+			RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
+			node->base.link.prev = (void *)engine;

Comment here please - what for and why it is safe.

+			atomic_inc(&ref->count);
+		}
+ GEM_BUG_ON(barrier_to_engine(node) != engine);
+		llist_add(barrier_to_ll(node), &ref->preallocated_barriers);
  		intel_engine_pm_get(engine);
-		llist_add((struct llist_node *)&node->base.link,
-			  &ref->barriers);
  	}
return 0; unwind:
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct active_node *node;
+	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {

s/get/take/ or remove, extract?

+		struct active_node *node = barrier_from_ll(pos);
- node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-		engine = (void *)rcu_access_pointer(node->base.request);
+		atomic_dec(&ref->count);
+		intel_engine_pm_put(barrier_to_engine(node));
- intel_engine_pm_put(engine);
  		kmem_cache_free(global.slab_cache, node);
  	}
  	return err;
@@ -426,25 +583,27 @@ void i915_active_acquire_barrier(struct i915_active *ref)
GEM_BUG_ON(i915_active_is_idle(ref)); + /*
+	 * Transfer the list of preallocated barriers into the
+	 * i915_active rbtree, but only as proto-nodes. They will be
+	 * populated by i915_request_add_active_barrier() to point to the
+	 * request that will eventually release them.
+	 */
  	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);
-	llist_for_each_safe(pos, next, llist_del_all(&ref->barriers)) {
-		struct intel_engine_cs *engine;
-		struct active_node *node;
+	llist_for_each_safe(pos, next, get_preallocated_barriers(ref)) {
+		struct active_node *node = barrier_from_ll(pos);
+		struct intel_engine_cs *engine = barrier_to_engine(node);
  		struct rb_node **p, *parent;
- node = container_of((struct list_head *)pos,
-				    typeof(*node), base.link);
-
-		engine = (void *)rcu_access_pointer(node->base.request);
-		RCU_INIT_POINTER(node->base.request, ERR_PTR(-EAGAIN));
-
  		parent = NULL;
  		p = &ref->tree.rb_node;
  		while (*p) {
+			struct active_node *it;
+
  			parent = *p;
-			if (rb_entry(parent,
-				     struct active_node,
-				     node)->timeline < node->timeline)
+
+			it = rb_entry(parent, struct active_node, node);
+			if (it->timeline < node->timeline)
  				p = &parent->rb_right;
  			else
  				p = &parent->rb_left;
@@ -452,20 +611,29 @@ void i915_active_acquire_barrier(struct i915_active *ref)
  		rb_link_node(&node->node, parent, p);
  		rb_insert_color(&node->node, &ref->tree);
- llist_add((struct llist_node *)&node->base.link,
-			  &engine->barrier_tasks);
+		llist_add(barrier_to_ll(node), &engine->barrier_tasks);
  		intel_engine_pm_put(engine);
  	}
  	mutex_unlock(&ref->mutex);
  }
-void i915_request_add_barriers(struct i915_request *rq)
+void i915_request_add_active_barriers(struct i915_request *rq)
  {
  	struct intel_engine_cs *engine = rq->engine;
  	struct llist_node *node, *next;
- llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks))
+	GEM_BUG_ON(intel_engine_is_virtual(engine));
+	GEM_BUG_ON(rq->timeline != engine->kernel_context->ring->timeline);
+
+	/*
+	 * Attach the list of proto-fences to the in-flight request such
+	 * that the parent i915_active will be released when this request
+	 * is retired.
+	 */
+	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
+		RCU_INIT_POINTER(barrier_from_ll(node)->base.request, rq);
  		list_add_tail((struct list_head *)node, &rq->active_list);
+	}
  }
int i915_active_request_set(struct i915_active_request *active,
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index ba68b077ec6c..566336c99ed7 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -413,6 +413,6 @@ static inline void i915_active_fini(struct i915_active *ref) { }
  int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
  					    struct intel_engine_cs *engine);
  void i915_active_acquire_barrier(struct i915_active *ref);
-void i915_request_add_barriers(struct i915_request *rq);
+void i915_request_add_active_barriers(struct i915_request *rq);
#endif /* _I915_ACTIVE_H_ */
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index 74743dd0d5f0..ae3ee441c114 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -42,7 +42,7 @@ struct i915_active {
  	int (*active)(struct i915_active *ref);
  	void (*retire)(struct i915_active *ref);
- struct llist_head barriers;
+	struct llist_head preallocated_barriers;
  };
#endif /* _I915_ACTIVE_TYPES_H_ */
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index 2b31a4ee0b4c..a841d3f9bedc 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -15,6 +15,7 @@ selftest(workarounds, intel_workarounds_live_selftests)
  selftest(timelines, intel_timeline_live_selftests)
  selftest(requests, i915_request_live_selftests)
  selftest(active, i915_active_live_selftests)
+selftest(gt_contexts, intel_context_live_selftests)
  selftest(objects, i915_gem_object_live_selftests)
  selftest(mman, i915_gem_mman_live_selftests)
  selftest(dmabuf, i915_gem_dmabuf_live_selftests)
@@ -24,7 +25,7 @@ selftest(gtt, i915_gem_gtt_live_selftests)
  selftest(gem, i915_gem_live_selftests)
  selftest(evict, i915_gem_evict_live_selftests)
  selftest(hugepages, i915_gem_huge_page_live_selftests)
-selftest(contexts, i915_gem_context_live_selftests)
+selftest(gem_contexts, i915_gem_context_live_selftests)
  selftest(blt, i915_gem_object_blt_live_selftests)
  selftest(client, i915_gem_client_blt_live_selftests)
  selftest(reset, intel_reset_live_selftests)


It's extremely complicated!

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux