[PATCH 77/80] drm/i915/gt: Support virtual engine queues
Chris Wilson
chris at chris-wilson.co.uk
Fri Aug 7 23:23:28 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>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 212 +++++++++++++---------------
drivers/gpu/drm/i915/i915_request.c | 3 +-
2 files changed, 100 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index bf70117dc679..4b46b112cd42 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -185,17 +185,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
@@ -1353,14 +1342,31 @@ static void
resubmit_virtual_request(struct i915_request *rq, struct virtual_engine *ve)
{
struct intel_engine_cs *engine = rq->engine;
+ struct intel_timeline *tl;
+ struct i915_request *rn;
+ LIST_HEAD(resubmit);
unsigned long flags;
spin_lock_irqsave(&engine->active.lock, flags);
+ spin_lock(&ve->base.active.lock);
+
+ tl = i915_request_active_timeline(rq);
+ do {
+ list_move(&rq->sched.link, &resubmit);
- clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
- WRITE_ONCE(rq->engine, &ve->base);
- ve->base.submit_request(rq);
+ if (list_is_first(&rq->link, &tl->requests))
+ break;
+
+ rq = list_prev_entry(rq, link);
+ } while (i915_request_in_priority_queue(rq));
+ list_for_each_entry_safe(rq, rn, &resubmit, sched.link) {
+ GEM_BUG_ON(rq->engine != engine);
+ WRITE_ONCE(rq->engine, &ve->base);
+ __intel_engine_queue_request(&ve->base, rq);
+ }
+
+ spin_unlock(&ve->base.active.lock);
spin_unlock_irqrestore(&engine->active.lock, flags);
}
@@ -1368,9 +1374,6 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
{
struct virtual_engine *ve = container_of(ce, typeof(*ve), context);
- if (READ_ONCE(ve->request))
- tasklet_hi_schedule(&ve->base.execlists.tasklet);
-
/*
* This engine is now too busy to run this virtual request, so
* see if we can find an alternative engine for it to execute on.
@@ -1380,6 +1383,9 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
if (i915_request_in_priority_queue(rq) &&
rq->execution_mask != rq->engine->mask)
resubmit_virtual_request(rq, ve);
+
+ if (!RB_EMPTY_ROOT(&ve->base.execlists.queue.rb_root))
+ tasklet_hi_schedule(&ve->base.execlists.tasklet);
}
static inline void __execlists_schedule_out(struct i915_request *rq)
@@ -1780,6 +1786,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;
@@ -1799,31 +1808,36 @@ static bool virtual_matches(const struct virtual_engine *ve,
return true;
}
-static struct virtual_engine *
-first_virtual_engine(struct intel_engine_cs *engine)
+static struct i915_request *first_virtual_request(struct virtual_engine *ve)
{
- struct intel_engine_execlists *el = &engine->execlists;
- struct rb_node *rb = rb_first_cached(&el->virtual);
+ struct intel_engine_execlists *el = &ve->base.execlists;
+ struct rb_node *rb;
- while (rb) {
- struct virtual_engine *ve =
- rb_entry(rb, typeof(*ve), nodes[engine->id].rb);
- struct i915_request *rq = READ_ONCE(ve->request);
+ while ((rb = rb_first_cached(&el->queue))) {
+ struct i915_priolist *p = to_priolist(rb);
- /* 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);
+ if (list_empty(&p->requests)) {
+ rb_erase_cached(&p->node, &el->queue);
+ i915_priolist_free(p);
continue;
}
- return ve;
+ return list_first_entry(&p->requests,
+ struct i915_request,
+ sched.link);
}
return NULL;
}
+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 void virtual_xfer_context(struct virtual_engine *ve,
struct intel_engine_cs *engine)
{
@@ -1975,6 +1989,43 @@ static void set_preempt_timeout(struct intel_engine_cs *engine,
active_preempt_timeout(engine, 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;
@@ -2111,8 +2162,9 @@ 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);
@@ -2123,12 +2175,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
break;
}
- GEM_BUG_ON(!virtual_matches(ve, rq, engine));
-
if (last && !can_merge_rq(last, rq)) {
spin_unlock(&ve->base.active.lock);
- spin_unlock_irq(&engine->active.lock);
- return; /* leave this for another sibling */
+ break; /* leave this for another sibling? */
}
ENGINE_TRACE(engine,
@@ -2139,11 +2188,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);
@@ -2169,19 +2213,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
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(&execlists->queue))) {
@@ -4014,13 +4048,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);
}
@@ -5026,11 +5058,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.execlists.default_priolist.requests;
-}
-
static void rcu_virtual_context_destroy(struct work_struct *wrk)
{
struct virtual_engine *ve =
@@ -5040,17 +5067,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(!RB_EMPTY_ROOT(&ve->base.execlists.queue.rb_root))) {
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);
}
@@ -5081,7 +5104,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.execlists.tasklet));
- GEM_BUG_ON(!list_empty(virtual_queue(ve)));
if (ve->context.state)
__execlists_context_fini(&ve->context);
@@ -5189,7 +5211,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;
@@ -5229,9 +5253,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))) {
@@ -5293,45 +5314,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.execlists.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)
@@ -5404,10 +5386,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.execlists.tasklet,
virtual_submission_tasklet,
(unsigned long)ve);
@@ -5615,14 +5596,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\tV ");
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 02f1dd89bd60..e85ad287950a 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1342,6 +1342,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);
@@ -1527,7 +1528,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