Commit 26208d87 authored by Chris Wilson's avatar Chris Wilson

drm/i915/gt: Fix hold/unhold recursion

In eliminating the recursion from walking the tree of signalers/waiters
for processing the hold/unhold operations, a crucial error crept in
where we looked at the parent request and not the list element when
processing the list.

Brown paper bag, much?

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1166
Fixes: 32ff621f ("drm/i915/gt: Allow temporary suspension of inflight requests")
Fixes: 74831738 ("drm/i915/execlists: Offline error capture")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200209131922.180287-1-chris@chris-wilson.co.uk
parent 48d7fb18
...@@ -2374,10 +2374,10 @@ static void __execlists_hold(struct i915_request *rq) ...@@ -2374,10 +2374,10 @@ static void __execlists_hold(struct i915_request *rq)
if (i915_request_is_active(rq)) if (i915_request_is_active(rq))
__i915_request_unsubmit(rq); __i915_request_unsubmit(rq);
RQ_TRACE(rq, "on hold\n");
clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
list_move_tail(&rq->sched.link, &rq->engine->active.hold); list_move_tail(&rq->sched.link, &rq->engine->active.hold);
i915_request_set_hold(rq); i915_request_set_hold(rq);
RQ_TRACE(rq, "on hold\n");
for_each_waiter(p, rq) { for_each_waiter(p, rq) {
struct i915_request *w = struct i915_request *w =
...@@ -2393,7 +2393,7 @@ static void __execlists_hold(struct i915_request *rq) ...@@ -2393,7 +2393,7 @@ static void __execlists_hold(struct i915_request *rq)
if (i915_request_completed(w)) if (i915_request_completed(w))
continue; continue;
if (i915_request_on_hold(rq)) if (i915_request_on_hold(w))
continue; continue;
list_move_tail(&w->sched.link, &list); list_move_tail(&w->sched.link, &list);
...@@ -2451,6 +2451,7 @@ static bool execlists_hold(struct intel_engine_cs *engine, ...@@ -2451,6 +2451,7 @@ static bool execlists_hold(struct intel_engine_cs *engine,
GEM_BUG_ON(i915_request_on_hold(rq)); GEM_BUG_ON(i915_request_on_hold(rq));
GEM_BUG_ON(rq->engine != engine); GEM_BUG_ON(rq->engine != engine);
__execlists_hold(rq); __execlists_hold(rq);
GEM_BUG_ON(list_empty(&engine->active.hold));
unlock: unlock:
spin_unlock_irq(&engine->active.lock); spin_unlock_irq(&engine->active.lock);
...@@ -2486,6 +2487,8 @@ static void __execlists_unhold(struct i915_request *rq) ...@@ -2486,6 +2487,8 @@ static void __execlists_unhold(struct i915_request *rq)
do { do {
struct i915_dependency *p; struct i915_dependency *p;
RQ_TRACE(rq, "hold release\n");
GEM_BUG_ON(!i915_request_on_hold(rq)); GEM_BUG_ON(!i915_request_on_hold(rq));
GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit));
...@@ -2494,7 +2497,6 @@ static void __execlists_unhold(struct i915_request *rq) ...@@ -2494,7 +2497,6 @@ static void __execlists_unhold(struct i915_request *rq)
i915_sched_lookup_priolist(rq->engine, i915_sched_lookup_priolist(rq->engine,
rq_prio(rq))); rq_prio(rq)));
set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
RQ_TRACE(rq, "hold release\n");
/* Also release any children on this engine that are ready */ /* Also release any children on this engine that are ready */
for_each_waiter(p, rq) { for_each_waiter(p, rq) {
...@@ -2504,11 +2506,11 @@ static void __execlists_unhold(struct i915_request *rq) ...@@ -2504,11 +2506,11 @@ static void __execlists_unhold(struct i915_request *rq)
if (w->engine != rq->engine) if (w->engine != rq->engine)
continue; continue;
if (!i915_request_on_hold(rq)) if (!i915_request_on_hold(w))
continue; continue;
/* Check that no other parents are also on hold */ /* Check that no other parents are also on hold */
if (hold_request(rq)) if (hold_request(w))
continue; continue;
list_move_tail(&w->sched.link, &list); list_move_tail(&w->sched.link, &list);
...@@ -2806,6 +2808,7 @@ static void execlists_submit_request(struct i915_request *request) ...@@ -2806,6 +2808,7 @@ static void execlists_submit_request(struct i915_request *request)
spin_lock_irqsave(&engine->active.lock, flags); spin_lock_irqsave(&engine->active.lock, flags);
if (unlikely(ancestor_on_hold(engine, request))) { if (unlikely(ancestor_on_hold(engine, request))) {
RQ_TRACE(request, "ancestor on hold\n");
list_add_tail(&request->sched.link, &engine->active.hold); list_add_tail(&request->sched.link, &engine->active.hold);
i915_request_set_hold(request); i915_request_set_hold(request);
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment