Skip to content

Commit

Permalink
drm/i915: Fixup preempt-to-busy vs resubmission of a virtual request
Browse files Browse the repository at this point in the history
As preempt-to-busy leaves the request on the HW as the resubmission is
processed, that request may complete in the background and even cause a
second virtual request to enter queue. This second virtual request
breaks our "single request in the virtual pipeline" assumptions.
Furthermore, as the virtual request may be completed and retired, we
lose the reference the virtual engine assumes is held. Normally, just
removing the request from the scheduler queue removes it from the
engine, but the virtual engine keeps track of its singleton request via
its ve->request. This pointer needs protecting with a reference.

v2: Drop unnecessary motion of rq->engine = owner

Fixes: 22b7a42 ("drm/i915/execlists: Preempt-to-busy")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190923152844.8914-1-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Sep 23, 2019
1 parent 27ffe6e commit b647c7d
Showing 1 changed file with 26 additions and 6 deletions.
32 changes: 26 additions & 6 deletions drivers/gpu/drm/i915/gt/intel_lrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
submit = true;
last = rq;
}
i915_request_put(rq);

/*
* Hmm, we have a bunch of virtual engine requests,
Expand Down Expand Up @@ -2556,6 +2557,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)

rq->engine = engine;
__i915_request_submit(rq);
i915_request_put(rq);

ve->base.execlists.queue_priority_hint = INT_MIN;
}
Expand Down Expand Up @@ -3809,6 +3811,8 @@ 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);
struct i915_request *old;
unsigned long flags;

GEM_TRACE("%s: rq=%llx:%lld\n",
ve->base.name,
Expand All @@ -3817,15 +3821,31 @@ static void virtual_submit_request(struct i915_request *rq)

GEM_BUG_ON(ve->base.submit_request != virtual_submit_request);

GEM_BUG_ON(ve->request);
GEM_BUG_ON(!list_empty(virtual_queue(ve)));
spin_lock_irqsave(&ve->base.active.lock, flags);

old = ve->request;
if (old) { /* background completion event from preempt-to-busy */
GEM_BUG_ON(!i915_request_completed(old));
__i915_request_submit(old);
i915_request_put(old);
}

ve->base.execlists.queue_priority_hint = rq_prio(rq);
WRITE_ONCE(ve->request, rq);
if (i915_request_completed(rq)) {
__i915_request_submit(rq);

list_move_tail(&rq->sched.link, virtual_queue(ve));
ve->base.execlists.queue_priority_hint = INT_MIN;
ve->request = NULL;
} else {
ve->base.execlists.queue_priority_hint = 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_schedule(&ve->base.execlists.tasklet);
}

tasklet_schedule(&ve->base.execlists.tasklet);
spin_unlock_irqrestore(&ve->base.active.lock, flags);
}

static struct ve_bond *
Expand Down

0 comments on commit b647c7d

Please sign in to comment.