[PATCH 72/72] drm/i915/gt: Support virtual engine queues
Chris Wilson
chris at chris-wilson.co.uk
Sat Dec 12 16:58:43 UTC 2020
Allow multiple requests to be queued unto a virtual engine, whereas
before we only allowed a single request to be queued at a time. The
advantage of keeping just one request in the queue was to ensure that we
always decided late which engine to use. However, with the introduction
of the virtual deadline we throttle submission and still only drip one
request into the sibling at a time (unless it is truly empty, but then a
second request will have an earlier deadline than the queued virtual
engine and force itself in front). This also takes advantage that a
virtual engine will remain bound while it is active, i.e. we can not
switch to a second engine until the context is completed -- such that we
cannot be as lazy as lazy can be.
By allowing a full queue, we avoid having to synchronize via the
breadcrumb interrupt everytime, letting the virtual engine reach the
full throughput of the siblings.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
.../drm/i915/gt/intel_execlists_submission.c | 266 +++++++++---------
drivers/gpu/drm/i915/i915_request.c | 3 +-
2 files changed, 134 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index be31477eea72..3237efd3b0a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -162,17 +162,6 @@ struct virtual_engine {
struct intel_context context;
struct rcu_work rcu;
- /*
- * We allow only a single request through the virtual engine at a time
- * (each request in the timeline waits for the completion fence of
- * the previous before being submitted). By restricting ourselves to
- * only submitting a single request, each request is placed on to a
- * physical to maximise load spreading (by virtue of the late greedy
- * scheduling -- each real engine takes the next available request
- * upon idling).
- */
- struct i915_request *request;
-
/*
* We keep a rbtree of available virtual engines inside each physical
* engine, sorted by priority. Here we preallocate the nodes we need
@@ -417,18 +406,49 @@ first_queue_request(struct intel_engine_cs *engine)
} while (1);
}
-static struct i915_request *
-first_virtual_request(const struct intel_engine_cs *engine)
+static struct virtual_engine *
+first_virtual_engine(struct intel_engine_cs *engine)
+{
+ return rb_entry_safe(rb_first_cached(&engine->execlists.virtual),
+ struct virtual_engine,
+ nodes[engine->id].rb);
+}
+
+static struct i915_request *__first_virtual_request(struct virtual_engine *ve)
{
struct rb_node *rb;
- rb = rb_first_cached(&engine->execlists.virtual);
- if (!rb)
- return NULL;
+ while ((rb = rb_first_cached(&ve->base.active.queue))) {
+ struct i915_priolist *p = to_priolist(rb);
- return READ_ONCE(rb_entry(rb,
- struct virtual_engine,
- nodes[engine->id].rb)->request);
+ if (list_empty(&p->requests)) {
+ rb_erase_cached(&p->node, &ve->base.active.queue);
+ i915_priolist_free(p);
+ continue;
+ }
+
+ return list_first_entry(&p->requests,
+ struct i915_request,
+ sched.link);
+ }
+
+ return NULL;
+}
+
+static const struct i915_request *
+first_virtual_request(struct intel_engine_cs *engine)
+{
+ struct i915_request *rq = NULL;
+ struct virtual_engine *ve;
+
+ ve = first_virtual_engine(engine);
+ if (ve) {
+ spin_lock(&ve->base.active.lock);
+ rq = __first_virtual_request(ve);
+ spin_unlock(&ve->base.active.lock);
+ }
+
+ return rq;
}
static const struct i915_request *
@@ -1334,7 +1354,7 @@ static inline void execlists_schedule_in(struct i915_request *rq, int idx)
trace_i915_request_in(rq, idx);
old = ce->inflight;
- if (!old)
+ if (!ptr_unmask_bits(old, 3))
old = __execlists_schedule_in(rq);
WRITE_ONCE(ce->inflight, ptr_inc(old));
@@ -1342,9 +1362,11 @@ static inline void execlists_schedule_in(struct i915_request *rq, int idx)
}
static void
-resubmit_virtual_request(struct i915_request *rq, struct virtual_engine *ve)
+__resubmit_virtual_request(struct i915_request *rq,
+ struct intel_engine_cs *engine,
+ struct virtual_engine *ve)
{
- struct intel_engine_cs *engine = rq->engine;
+ GEM_BUG_ON(rq->engine != engine);
/* Flush concurrent rcu iterators in signal_irq_work */
if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &rq->fence.flags)) {
@@ -1361,11 +1383,35 @@ resubmit_virtual_request(struct i915_request *rq, struct virtual_engine *ve)
cpu_relax();
}
- spin_lock_irq(&engine->active.lock);
-
clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
WRITE_ONCE(rq->engine, &ve->base);
ve->base.submit_request(rq);
+}
+
+static void
+resubmit_virtual_request(struct i915_request *rq, struct virtual_engine *ve)
+{
+ struct intel_engine_cs *engine = rq->engine;
+ struct intel_timeline *tl = READ_ONCE(rq->timeline);
+ struct i915_request *last = rq;
+ struct i915_request *first = rq;
+
+ spin_lock_irq(&engine->active.lock);
+
+ /* Rewind back to the start of this virtual engine queue */
+ list_for_each_entry_continue_reverse(rq, &tl->requests, link) {
+ if (i915_request_completed(rq))
+ break;
+
+ first = rq;
+ }
+
+ /* Resubmit the queue in execution order */
+ for (rq = first;; rq = list_next_entry(rq, link)) {
+ __resubmit_virtual_request(rq, engine, ve);
+ if (rq == last)
+ break;
+ }
spin_unlock_irq(&engine->active.lock);
}
@@ -1385,7 +1431,7 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
rq->execution_mask != engine->mask)
resubmit_virtual_request(rq, ve);
- if (READ_ONCE(ve->request))
+ if (!RB_EMPTY_ROOT(&ve->base.active.queue.rb_root))
i915_sched_kick(&ve->base.active);
}
@@ -1790,6 +1836,9 @@ static bool virtual_matches(const struct virtual_engine *ve,
{
const struct intel_engine_cs *inflight;
+ if (!rq)
+ return false;
+
if (!(rq->execution_mask & engine->mask)) /* We peeked too soon! */
return false;
@@ -1809,31 +1858,6 @@ static bool virtual_matches(const struct virtual_engine *ve,
return true;
}
-static struct virtual_engine *
-first_virtual_engine(struct intel_engine_cs *engine)
-{
- struct intel_engine_execlists *el = &engine->execlists;
- struct rb_node *rb = rb_first_cached(&el->virtual);
-
- while (rb) {
- struct virtual_engine *ve =
- rb_entry(rb, typeof(*ve), nodes[engine->id].rb);
- struct i915_request *rq = READ_ONCE(ve->request);
-
- /* lazily cleanup after another engine handled rq */
- if (!rq || !virtual_matches(ve, rq, engine)) {
- rb_erase_cached(rb, &el->virtual);
- RB_CLEAR_NODE(rb);
- rb = rb_first_cached(&el->virtual);
- continue;
- }
-
- return ve;
- }
-
- return NULL;
-}
-
static void virtual_xfer_context(struct virtual_engine *ve,
struct intel_engine_cs *engine)
{
@@ -1993,6 +2017,43 @@ static bool completed(const struct i915_request *rq)
return i915_request_completed(rq);
}
+static void virtual_dequeue(struct virtual_engine *ve,
+ struct intel_engine_cs *sibling)
+{
+ struct ve_node * const node = &ve->nodes[sibling->id];
+ struct rb_node **parent, *rb;
+ struct i915_request *rq;
+ u64 deadline;
+ bool first;
+
+ rb_erase_cached(&node->rb, &sibling->execlists.virtual);
+ RB_CLEAR_NODE(&node->rb);
+
+ rq = __first_virtual_request(ve);
+ if (!virtual_matches(ve, rq, sibling))
+ return;
+
+ rb = NULL;
+ first = true;
+ parent = &sibling->execlists.virtual.rb_root.rb_node;
+ deadline = rq_deadline(rq);
+ while (*parent) {
+ struct ve_node *other;
+
+ rb = *parent;
+ other = rb_entry(rb, typeof(*other), rb);
+ if (deadline <= other->deadline) {
+ parent = &rb->rb_left;
+ } else {
+ parent = &rb->rb_right;
+ first = false;
+ }
+ }
+
+ rb_link_node(&node->rb, rb, parent);
+ rb_insert_color_cached(&node->rb, &sibling->execlists.virtual, first);
+}
+
static void execlists_dequeue(struct intel_engine_cs *engine)
{
struct intel_engine_execlists * const execlists = &engine->execlists;
@@ -2122,24 +2183,22 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
spin_lock(&ve->base.active.lock);
- rq = ve->request;
- if (unlikely(!rq)) /* lost the race to a sibling */
+ rq = __first_virtual_request(ve);
+ if (unlikely(!virtual_matches(ve, rq, engine)))
+ /* lost the race to a sibling */
goto unlock;
GEM_BUG_ON(rq->engine != &ve->base);
GEM_BUG_ON(rq->context != &ve->context);
- if (unlikely(deadline_before(rq, first_queue_request(engine)))) {
+ if (unlikely(!deadline_before(first_queue_request(engine), rq))) {
spin_unlock(&ve->base.active.lock);
break;
}
- GEM_BUG_ON(!virtual_matches(ve, rq, engine));
-
if (last && !can_merge_rq(last, rq)) {
spin_unlock(&ve->base.active.lock);
- spin_unlock(&engine->active.lock);
- return; /* leave this for another sibling */
+ break; /* leave this for another sibling? */
}
ENGINE_TRACE(engine,
@@ -2150,11 +2209,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
i915_request_started(rq) ? "*" :
"",
yesno(engine != ve->siblings[0]));
- WRITE_ONCE(ve->request, NULL);
-
- rb = &ve->nodes[engine->id].rb;
- rb_erase_cached(rb, &execlists->virtual);
- RB_CLEAR_NODE(rb);
GEM_BUG_ON(!(rq->execution_mask & engine->mask));
WRITE_ONCE(rq->engine, engine);
@@ -2176,23 +2230,17 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
virtual_xfer_context(ve, engine);
GEM_BUG_ON(ve->siblings[0] != engine);
+ /* Bind this ve before we release the lock */
+ if (!rq->context->inflight)
+ WRITE_ONCE(rq->context->inflight, &ve->base);
+
submit = true;
last = rq;
}
- i915_request_put(rq);
unlock:
+ virtual_dequeue(ve, engine);
spin_unlock(&ve->base.active.lock);
-
- /*
- * Hmm, we have a bunch of virtual engine requests,
- * but the first one was already completed (thanks
- * preempt-to-busy!). Keep looking at the veng queue
- * until we have no more relevant requests (i.e.
- * the normal submit queue has higher priority).
- */
- if (submit)
- break;
}
while ((rb = rb_first_cached(&engine->active.queue))) {
@@ -4083,13 +4131,11 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine)
RB_CLEAR_NODE(rb);
spin_lock(&ve->base.active.lock);
- rq = fetch_and_zero(&ve->request);
- if (rq) {
+ while ((rq = __first_virtual_request(ve))) {
mark_eio(rq);
rq->engine = engine;
__i915_request_submit(rq);
- i915_request_put(rq);
}
spin_unlock(&ve->base.active.lock);
}
@@ -4541,11 +4587,6 @@ static int __execlists_context_alloc(struct intel_context *ce,
return ret;
}
-static struct list_head *virtual_queue(struct virtual_engine *ve)
-{
- return &ve->base.active.default_priolist.requests;
-}
-
static void rcu_virtual_context_destroy(struct work_struct *wrk)
{
struct virtual_engine *ve =
@@ -4555,17 +4596,13 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk)
GEM_BUG_ON(ve->context.inflight);
/* Preempt-to-busy may leave a stale request behind. */
- if (unlikely(ve->request)) {
+ if (unlikely(!i915_sched_is_idle(&ve->base.active))) {
struct i915_request *old;
spin_lock_irq(&ve->base.active.lock);
- old = fetch_and_zero(&ve->request);
- if (old) {
- GEM_BUG_ON(!i915_request_completed(old));
+ while ((old = __first_virtual_request(ve)))
__i915_request_submit(old);
- i915_request_put(old);
- }
spin_unlock_irq(&ve->base.active.lock);
}
@@ -4596,7 +4633,6 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk)
spin_unlock_irq(&sibling->active.lock);
}
GEM_BUG_ON(__tasklet_is_scheduled(&ve->base.active.tasklet));
- GEM_BUG_ON(!list_empty(virtual_queue(ve)));
if (ve->context.state)
__execlists_context_fini(&ve->context);
@@ -4709,7 +4745,9 @@ virtual_submission_mask(struct virtual_engine *ve, u64 *deadline)
struct i915_request *rq;
intel_engine_mask_t mask;
- rq = READ_ONCE(ve->request);
+ spin_lock_irq(&ve->base.active.lock);
+ rq = __first_virtual_request(ve);
+ spin_unlock_irq(&ve->base.active.lock);
if (!rq)
return 0;
@@ -4749,9 +4787,6 @@ static void virtual_submission_tasklet(unsigned long data)
struct rb_node **parent, *rb;
bool first;
- if (!READ_ONCE(ve->request))
- break; /* already handled by a sibling's tasklet */
-
spin_lock_irq(&sibling->active.lock);
if (unlikely(!(mask & sibling->mask))) {
@@ -4813,45 +4848,6 @@ static void virtual_submission_tasklet(unsigned long data)
}
}
-static void virtual_submit_request(struct i915_request *rq)
-{
- struct virtual_engine *ve = to_virtual_engine(rq->engine);
- unsigned long flags;
-
- ENGINE_TRACE(&ve->base, "rq=%llx:%lld\n",
- rq->fence.context,
- rq->fence.seqno);
-
- GEM_BUG_ON(ve->base.submit_request != virtual_submit_request);
-
- spin_lock_irqsave(&ve->base.active.lock, flags);
-
- /* By the time we resubmit a request, it may be completed */
- if (i915_request_completed(rq)) {
- __i915_request_submit(rq);
- goto unlock;
- }
-
- if (ve->request) { /* background completion from preempt-to-busy */
- GEM_BUG_ON(!i915_request_completed(ve->request));
- __i915_request_submit(ve->request);
- i915_request_put(ve->request);
- }
-
- rq->sched.deadline =
- min(rq->sched.deadline,
- i915_scheduler_next_virtual_deadline(rq_prio(rq)));
- ve->request = i915_request_get(rq);
-
- GEM_BUG_ON(!list_empty(virtual_queue(ve)));
- list_move_tail(&rq->sched.link, virtual_queue(ve));
-
- tasklet_hi_schedule(&ve->base.active.tasklet);
-
-unlock:
- spin_unlock_irqrestore(&ve->base.active.lock, flags);
-}
-
static struct ve_bond *
virtual_find_bond(struct virtual_engine *ve,
const struct intel_engine_cs *master)
@@ -4924,10 +4920,9 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
ve->base.cops = &virtual_context_ops;
ve->base.request_alloc = execlists_request_alloc;
- ve->base.submit_request = virtual_submit_request;
+ ve->base.submit_request = i915_request_enqueue;
ve->base.bond_execute = virtual_bond_execute;
- INIT_LIST_HEAD(virtual_queue(ve));
tasklet_init(&ve->base.active.tasklet,
virtual_submission_tasklet,
(unsigned long)ve);
@@ -5136,14 +5131,17 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine,
for (rb = rb_first_cached(&execlists->virtual); rb; rb = rb_next(rb)) {
struct virtual_engine *ve =
rb_entry(rb, typeof(*ve), nodes[engine->id].rb);
- struct i915_request *rq = READ_ONCE(ve->request);
+ struct i915_request *rq;
+ spin_lock(&ve->base.active.lock);
+ rq = __first_virtual_request(ve);
if (rq) {
if (count++ < max - 1)
show_request(m, rq, "\t\t", 0);
else
last = rq;
}
+ spin_unlock(&ve->base.active.lock);
}
if (last) {
if (count > max) {
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 4098cec91b72..c9c447801131 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1345,6 +1345,7 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
GEM_BUG_ON(to == from);
GEM_BUG_ON(to->timeline == from->timeline);
+ GEM_BUG_ON(to->context == from->context);
if (i915_request_completed(from)) {
i915_sw_fence_set_error_once(&to->submit, from->fence.error);
@@ -1530,7 +1531,7 @@ __i915_request_add_to_timeline(struct i915_request *rq)
i915_seqno_passed(prev->fence.seqno,
rq->fence.seqno));
- if (is_power_of_2(READ_ONCE(prev->engine)->mask | rq->engine->mask))
+ if (prev->context == rq->context)
i915_sw_fence_await_sw_fence(&rq->submit,
&prev->submit,
&rq->submitq);
--
2.20.1
More information about the Intel-gfx-trybot
mailing list