Message ID | 20210201085715.27435-17-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [01/57] drm/i915/gt: Restrict the GT clock override to just Icelake | expand |
On 01/02/2021 08:56, Chris Wilson wrote: > Make the ability to suspend and resume a request and its dependents > generic. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > .../drm/i915/gt/intel_execlists_submission.c | 167 +----------------- > drivers/gpu/drm/i915/gt/selftest_execlists.c | 8 +- > drivers/gpu/drm/i915/i915_scheduler.c | 153 ++++++++++++++++ > drivers/gpu/drm/i915/i915_scheduler.h | 10 ++ > 4 files changed, 169 insertions(+), 169 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index b6dea80da533..853021314786 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > @@ -1921,169 +1921,6 @@ static void post_process_csb(struct i915_request **port, > execlists_schedule_out(*port++); > } > > -static void __execlists_hold(struct i915_request *rq) > -{ > - LIST_HEAD(list); > - > - do { > - struct i915_dependency *p; > - > - if (i915_request_is_active(rq)) > - __i915_request_unsubmit(rq); > - > - clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); > - list_move_tail(&rq->sched.link, &rq->engine->active.hold); > - i915_request_set_hold(rq); > - RQ_TRACE(rq, "on hold\n"); > - > - for_each_waiter(p, rq) { > - struct i915_request *w = > - container_of(p->waiter, typeof(*w), sched); > - > - if (p->flags & I915_DEPENDENCY_WEAK) > - continue; > - > - /* Leave semaphores spinning on the other engines */ > - if (w->engine != rq->engine) > - continue; > - > - if (!i915_request_is_ready(w)) > - continue; > - > - if (__i915_request_is_complete(w)) > - continue; > - > - if (i915_request_on_hold(w)) > - continue; > - > - list_move_tail(&w->sched.link, &list); > - } > - > - rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); > - } while (rq); > -} > - > -static bool execlists_hold(struct intel_engine_cs *engine, > - struct i915_request *rq) > -{ > - if (i915_request_on_hold(rq)) > - return false; > - > - spin_lock_irq(&engine->active.lock); > - > - if (__i915_request_is_complete(rq)) { /* too late! */ > - rq = NULL; > - goto unlock; > - } > - > - /* > - * Transfer this request onto the hold queue to prevent it > - * being resumbitted to HW (and potentially completed) before we have > - * released it. Since we may have already submitted following > - * requests, we need to remove those as well. > - */ > - GEM_BUG_ON(i915_request_on_hold(rq)); > - GEM_BUG_ON(rq->engine != engine); > - __execlists_hold(rq); > - GEM_BUG_ON(list_empty(&engine->active.hold)); > - > -unlock: > - spin_unlock_irq(&engine->active.lock); > - return rq; > -} > - > -static bool hold_request(const struct i915_request *rq) > -{ > - struct i915_dependency *p; > - bool result = false; > - > - /* > - * If one of our ancestors is on hold, we must also be on hold, > - * otherwise we will bypass it and execute before it. > - */ > - rcu_read_lock(); > - for_each_signaler(p, rq) { > - const struct i915_request *s = > - container_of(p->signaler, typeof(*s), sched); > - > - if (s->engine != rq->engine) > - continue; > - > - result = i915_request_on_hold(s); > - if (result) > - break; > - } > - rcu_read_unlock(); > - > - return result; > -} > - > -static void __execlists_unhold(struct i915_request *rq) > -{ > - LIST_HEAD(list); > - > - do { > - struct i915_dependency *p; > - > - RQ_TRACE(rq, "hold release\n"); > - > - GEM_BUG_ON(!i915_request_on_hold(rq)); > - GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); > - > - i915_request_clear_hold(rq); > - list_move_tail(&rq->sched.link, > - i915_sched_lookup_priolist(rq->engine, > - rq_prio(rq))); > - set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); > - > - /* Also release any children on this engine that are ready */ > - for_each_waiter(p, rq) { > - struct i915_request *w = > - container_of(p->waiter, typeof(*w), sched); > - > - if (p->flags & I915_DEPENDENCY_WEAK) > - continue; > - > - /* Propagate any change in error status */ > - if (rq->fence.error) > - i915_request_set_error_once(w, rq->fence.error); > - > - if (w->engine != rq->engine) > - continue; > - > - if (!i915_request_on_hold(w)) > - continue; > - > - /* Check that no other parents are also on hold */ > - if (hold_request(w)) > - continue; > - > - list_move_tail(&w->sched.link, &list); > - } > - > - rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); > - } while (rq); > -} > - > -static void execlists_unhold(struct intel_engine_cs *engine, > - struct i915_request *rq) > -{ > - spin_lock_irq(&engine->active.lock); > - > - /* > - * Move this request back to the priority queue, and all of its > - * children and grandchildren that were suspended along with it. > - */ > - __execlists_unhold(rq); > - > - if (rq_prio(rq) > engine->execlists.queue_priority_hint) { > - engine->execlists.queue_priority_hint = rq_prio(rq); > - tasklet_hi_schedule(&engine->execlists.tasklet); > - } > - > - spin_unlock_irq(&engine->active.lock); > -} > - > struct execlists_capture { > struct work_struct work; > struct i915_request *rq; > @@ -2116,7 +1953,7 @@ static void execlists_capture_work(struct work_struct *work) > i915_gpu_coredump_put(cap->error); > > /* Return this request and all that depend upon it for signaling */ > - execlists_unhold(engine, cap->rq); > + i915_sched_resume_request(engine, cap->rq); > i915_request_put(cap->rq); > > kfree(cap); > @@ -2250,7 +2087,7 @@ static void execlists_capture(struct intel_engine_cs *engine) > * simply hold that request accountable for being non-preemptible > * long enough to force the reset. > */ > - if (!execlists_hold(engine, cap->rq)) > + if (!i915_sched_suspend_request(engine, cap->rq)) > goto err_rq; > > INIT_WORK(&cap->work, execlists_capture_work); > diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c > index 73340a96548f..64f6a49a5c22 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c > +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c > @@ -608,7 +608,7 @@ static int live_hold_reset(void *arg) > GEM_BUG_ON(execlists_active(&engine->execlists) != rq); > > i915_request_get(rq); > - execlists_hold(engine, rq); > + i915_sched_suspend_request(engine, rq); > GEM_BUG_ON(!i915_request_on_hold(rq)); > > __intel_engine_reset_bh(engine, NULL); > @@ -630,7 +630,7 @@ static int live_hold_reset(void *arg) > GEM_BUG_ON(!i915_request_on_hold(rq)); > > /* But is resubmitted on release */ > - execlists_unhold(engine, rq); > + i915_sched_resume_request(engine, rq); > if (i915_request_wait(rq, 0, HZ / 5) < 0) { > pr_err("%s: held request did not complete!\n", > engine->name); > @@ -4606,7 +4606,7 @@ static int reset_virtual_engine(struct intel_gt *gt, > GEM_BUG_ON(rq->engine != engine); > > /* Reset the engine while keeping our active request on hold */ > - execlists_hold(engine, rq); > + i915_sched_suspend_request(engine, rq); > GEM_BUG_ON(!i915_request_on_hold(rq)); > > __intel_engine_reset_bh(engine, NULL); > @@ -4629,7 +4629,7 @@ static int reset_virtual_engine(struct intel_gt *gt, > GEM_BUG_ON(!i915_request_on_hold(rq)); > > /* But is resubmitted on release */ > - execlists_unhold(engine, rq); > + i915_sched_resume_request(engine, rq); > if (i915_request_wait(rq, 0, HZ / 5) < 0) { > pr_err("%s: held request did not complete!\n", > engine->name); > diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c > index 9fcfbf303ce0..351c0c0055b5 100644 > --- a/drivers/gpu/drm/i915/i915_scheduler.c > +++ b/drivers/gpu/drm/i915/i915_scheduler.c > @@ -586,6 +586,159 @@ __i915_sched_rewind_requests(struct intel_engine_cs *engine) > return active; > } > > +bool __i915_sched_suspend_request(struct intel_engine_cs *engine, > + struct i915_request *rq) > +{ > + LIST_HEAD(list); > + > + lockdep_assert_held(&engine->active.lock); > + GEM_BUG_ON(rq->engine != engine); > + > + if (__i915_request_is_complete(rq)) /* too late! */ > + return false; > + > + if (i915_request_on_hold(rq)) > + return false; This was a GEM_BUG_ON before so not pure extraction / code movement. > + > + ENGINE_TRACE(engine, "suspending request %llx:%lld\n", > + rq->fence.context, rq->fence.seqno); > + > + /* > + * Transfer this request onto the hold queue to prevent it > + * being resumbitted to HW (and potentially completed) before we have > + * released it. Since we may have already submitted following > + * requests, we need to remove those as well. > + */ > + do { > + struct i915_dependency *p; > + > + if (i915_request_is_active(rq)) > + __i915_request_unsubmit(rq); > + > + list_move_tail(&rq->sched.link, &engine->active.hold); > + clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); > + i915_request_set_hold(rq); > + RQ_TRACE(rq, "on hold\n"); > + > + for_each_waiter(p, rq) { > + struct i915_request *w = > + container_of(p->waiter, typeof(*w), sched); > + > + if (p->flags & I915_DEPENDENCY_WEAK) > + continue; > + > + /* Leave semaphores spinning on the other engines */ > + if (w->engine != engine) > + continue; > + > + if (!i915_request_is_ready(w)) > + continue; > + > + if (__i915_request_is_complete(w)) > + continue; > + > + if (i915_request_on_hold(w)) /* acts as a visited bit */ > + continue; > + > + list_move_tail(&w->sched.link, &list); > + } > + > + rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); > + } while (rq); > + > + GEM_BUG_ON(list_empty(&engine->active.hold)); > + > + return true; > +} > + > +bool i915_sched_suspend_request(struct intel_engine_cs *engine, > + struct i915_request *rq) > +{ > + bool result; > + > + if (i915_request_on_hold(rq)) > + return false; > + > + spin_lock_irq(&engine->active.lock); > + result = __i915_sched_suspend_request(engine, rq); > + spin_unlock_irq(&engine->active.lock); > + > + return result; > +} > + > +void __i915_sched_resume_request(struct intel_engine_cs *engine, > + struct i915_request *rq) > +{ > + LIST_HEAD(list); > + > + lockdep_assert_held(&engine->active.lock); > + > + if (rq_prio(rq) > engine->execlists.queue_priority_hint) { > + engine->execlists.queue_priority_hint = rq_prio(rq); > + tasklet_hi_schedule(&engine->execlists.tasklet); > + } > + > + if (!i915_request_on_hold(rq)) > + return; > + > + ENGINE_TRACE(engine, "resuming request %llx:%lld\n", > + rq->fence.context, rq->fence.seqno); > + > + /* > + * Move this request back to the priority queue, and all of its > + * children and grandchildren that were suspended along with it. > + */ > + do { > + struct i915_dependency *p; > + > + RQ_TRACE(rq, "hold release\n"); > + > + GEM_BUG_ON(!i915_request_on_hold(rq)); > + GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); > + > + i915_request_clear_hold(rq); > + list_del_init(&rq->sched.link); > + > + queue_request(engine, rq); > + > + /* Also release any children on this engine that are ready */ > + for_each_waiter(p, rq) { > + struct i915_request *w = > + container_of(p->waiter, typeof(*w), sched); > + > + if (p->flags & I915_DEPENDENCY_WEAK) > + continue; > + > + /* Propagate any change in error status */ > + if (rq->fence.error) > + i915_request_set_error_once(w, rq->fence.error); > + > + if (w->engine != engine) > + continue; > + > + /* We also treat the on-hold status as a visited bit */ > + if (!i915_request_on_hold(w)) > + continue; > + > + /* Check that no other parents are also on hold [BFS] */ > + if (hold_request(w)) > + continue; hold_request() appears deleted in the patch so possible rebase error. Regards, Tvrtko > + > + list_move_tail(&w->sched.link, &list); > + } > + > + rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); > + } while (rq); > +} > + > +void i915_sched_resume_request(struct intel_engine_cs *engine, > + struct i915_request *rq) > +{ > + spin_lock_irq(&engine->active.lock); > + __i915_sched_resume_request(engine, rq); > + spin_unlock_irq(&engine->active.lock); > +} > + > void i915_sched_node_init(struct i915_sched_node *node) > { > spin_lock_init(&node->lock); > diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h > index d3984f65b3a6..9860459fedb1 100644 > --- a/drivers/gpu/drm/i915/i915_scheduler.h > +++ b/drivers/gpu/drm/i915/i915_scheduler.h > @@ -45,6 +45,16 @@ void i915_request_enqueue(struct i915_request *request); > struct i915_request * > __i915_sched_rewind_requests(struct intel_engine_cs *engine); > > +bool __i915_sched_suspend_request(struct intel_engine_cs *engine, > + struct i915_request *rq); > +void __i915_sched_resume_request(struct intel_engine_cs *engine, > + struct i915_request *request); > + > +bool i915_sched_suspend_request(struct intel_engine_cs *engine, > + struct i915_request *request); > +void i915_sched_resume_request(struct intel_engine_cs *engine, > + struct i915_request *rq); > + > struct list_head * > i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio); > >
Quoting Tvrtko Ursulin (2021-02-02 13:15:52) > > On 01/02/2021 08:56, Chris Wilson wrote: > > +void __i915_sched_resume_request(struct intel_engine_cs *engine, > > + struct i915_request *rq) > > +{ > > + LIST_HEAD(list); > > + > > + lockdep_assert_held(&engine->active.lock); > > + > > + if (rq_prio(rq) > engine->execlists.queue_priority_hint) { > > + engine->execlists.queue_priority_hint = rq_prio(rq); > > + tasklet_hi_schedule(&engine->execlists.tasklet); > > + } > > + > > + if (!i915_request_on_hold(rq)) > > + return; > > + > > + ENGINE_TRACE(engine, "resuming request %llx:%lld\n", > > + rq->fence.context, rq->fence.seqno); > > + > > + /* > > + * Move this request back to the priority queue, and all of its > > + * children and grandchildren that were suspended along with it. > > + */ > > + do { > > + struct i915_dependency *p; > > + > > + RQ_TRACE(rq, "hold release\n"); > > + > > + GEM_BUG_ON(!i915_request_on_hold(rq)); > > + GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); > > + > > + i915_request_clear_hold(rq); > > + list_del_init(&rq->sched.link); > > + > > + queue_request(engine, rq); > > + > > + /* Also release any children on this engine that are ready */ > > + for_each_waiter(p, rq) { > > + struct i915_request *w = > > + container_of(p->waiter, typeof(*w), sched); > > + > > + if (p->flags & I915_DEPENDENCY_WEAK) > > + continue; > > + > > + /* Propagate any change in error status */ > > + if (rq->fence.error) > > + i915_request_set_error_once(w, rq->fence.error); > > + > > + if (w->engine != engine) > > + continue; > > + > > + /* We also treat the on-hold status as a visited bit */ > > + if (!i915_request_on_hold(w)) > > + continue; > > + > > + /* Check that no other parents are also on hold [BFS] */ > > + if (hold_request(w)) > > + continue; > > hold_request() appears deleted in the patch so possible rebase error. The secret is we get to de-duplicate after having duplicated hold_request() in i915_scheduler in an earlier patch, drm/i915: Extract request submission from execlists -Chris
Quoting Tvrtko Ursulin (2021-02-02 13:15:52) > > On 01/02/2021 08:56, Chris Wilson wrote: > > Make the ability to suspend and resume a request and its dependents > > generic. > > +bool __i915_sched_suspend_request(struct intel_engine_cs *engine, > > + struct i915_request *rq) > > +{ > > + LIST_HEAD(list); > > + > > + lockdep_assert_held(&engine->active.lock); > > + GEM_BUG_ON(rq->engine != engine); > > + > > + if (__i915_request_is_complete(rq)) /* too late! */ > > + return false; > > + > > + if (i915_request_on_hold(rq)) > > + return false; > > This was a GEM_BUG_ON before so not pure extraction / code movement. It was part of making it generic to allow other callers. -Chris
On 02/02/2021 13:26, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2021-02-02 13:15:52) >> >> On 01/02/2021 08:56, Chris Wilson wrote: >>> +void __i915_sched_resume_request(struct intel_engine_cs *engine, >>> + struct i915_request *rq) >>> +{ >>> + LIST_HEAD(list); >>> + >>> + lockdep_assert_held(&engine->active.lock); >>> + >>> + if (rq_prio(rq) > engine->execlists.queue_priority_hint) { >>> + engine->execlists.queue_priority_hint = rq_prio(rq); >>> + tasklet_hi_schedule(&engine->execlists.tasklet); >>> + } >>> + >>> + if (!i915_request_on_hold(rq)) >>> + return; >>> + >>> + ENGINE_TRACE(engine, "resuming request %llx:%lld\n", >>> + rq->fence.context, rq->fence.seqno); >>> + >>> + /* >>> + * Move this request back to the priority queue, and all of its >>> + * children and grandchildren that were suspended along with it. >>> + */ >>> + do { >>> + struct i915_dependency *p; >>> + >>> + RQ_TRACE(rq, "hold release\n"); >>> + >>> + GEM_BUG_ON(!i915_request_on_hold(rq)); >>> + GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); >>> + >>> + i915_request_clear_hold(rq); >>> + list_del_init(&rq->sched.link); >>> + >>> + queue_request(engine, rq); >>> + >>> + /* Also release any children on this engine that are ready */ >>> + for_each_waiter(p, rq) { >>> + struct i915_request *w = >>> + container_of(p->waiter, typeof(*w), sched); >>> + >>> + if (p->flags & I915_DEPENDENCY_WEAK) >>> + continue; >>> + >>> + /* Propagate any change in error status */ >>> + if (rq->fence.error) >>> + i915_request_set_error_once(w, rq->fence.error); >>> + >>> + if (w->engine != engine) >>> + continue; >>> + >>> + /* We also treat the on-hold status as a visited bit */ >>> + if (!i915_request_on_hold(w)) >>> + continue; >>> + >>> + /* Check that no other parents are also on hold [BFS] */ >>> + if (hold_request(w)) >>> + continue; >> >> hold_request() appears deleted in the patch so possible rebase error. > > The secret is we get to de-duplicate after having duplicated > hold_request() in i915_scheduler in an earlier patch, > drm/i915: Extract request submission from execlists Pfft ancient history long forgotten.. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index b6dea80da533..853021314786 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -1921,169 +1921,6 @@ static void post_process_csb(struct i915_request **port, execlists_schedule_out(*port++); } -static void __execlists_hold(struct i915_request *rq) -{ - LIST_HEAD(list); - - do { - struct i915_dependency *p; - - if (i915_request_is_active(rq)) - __i915_request_unsubmit(rq); - - clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); - list_move_tail(&rq->sched.link, &rq->engine->active.hold); - i915_request_set_hold(rq); - RQ_TRACE(rq, "on hold\n"); - - for_each_waiter(p, rq) { - struct i915_request *w = - container_of(p->waiter, typeof(*w), sched); - - if (p->flags & I915_DEPENDENCY_WEAK) - continue; - - /* Leave semaphores spinning on the other engines */ - if (w->engine != rq->engine) - continue; - - if (!i915_request_is_ready(w)) - continue; - - if (__i915_request_is_complete(w)) - continue; - - if (i915_request_on_hold(w)) - continue; - - list_move_tail(&w->sched.link, &list); - } - - rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); - } while (rq); -} - -static bool execlists_hold(struct intel_engine_cs *engine, - struct i915_request *rq) -{ - if (i915_request_on_hold(rq)) - return false; - - spin_lock_irq(&engine->active.lock); - - if (__i915_request_is_complete(rq)) { /* too late! */ - rq = NULL; - goto unlock; - } - - /* - * Transfer this request onto the hold queue to prevent it - * being resumbitted to HW (and potentially completed) before we have - * released it. Since we may have already submitted following - * requests, we need to remove those as well. - */ - GEM_BUG_ON(i915_request_on_hold(rq)); - GEM_BUG_ON(rq->engine != engine); - __execlists_hold(rq); - GEM_BUG_ON(list_empty(&engine->active.hold)); - -unlock: - spin_unlock_irq(&engine->active.lock); - return rq; -} - -static bool hold_request(const struct i915_request *rq) -{ - struct i915_dependency *p; - bool result = false; - - /* - * If one of our ancestors is on hold, we must also be on hold, - * otherwise we will bypass it and execute before it. - */ - rcu_read_lock(); - for_each_signaler(p, rq) { - const struct i915_request *s = - container_of(p->signaler, typeof(*s), sched); - - if (s->engine != rq->engine) - continue; - - result = i915_request_on_hold(s); - if (result) - break; - } - rcu_read_unlock(); - - return result; -} - -static void __execlists_unhold(struct i915_request *rq) -{ - LIST_HEAD(list); - - do { - struct i915_dependency *p; - - RQ_TRACE(rq, "hold release\n"); - - GEM_BUG_ON(!i915_request_on_hold(rq)); - GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); - - i915_request_clear_hold(rq); - list_move_tail(&rq->sched.link, - i915_sched_lookup_priolist(rq->engine, - rq_prio(rq))); - set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); - - /* Also release any children on this engine that are ready */ - for_each_waiter(p, rq) { - struct i915_request *w = - container_of(p->waiter, typeof(*w), sched); - - if (p->flags & I915_DEPENDENCY_WEAK) - continue; - - /* Propagate any change in error status */ - if (rq->fence.error) - i915_request_set_error_once(w, rq->fence.error); - - if (w->engine != rq->engine) - continue; - - if (!i915_request_on_hold(w)) - continue; - - /* Check that no other parents are also on hold */ - if (hold_request(w)) - continue; - - list_move_tail(&w->sched.link, &list); - } - - rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); - } while (rq); -} - -static void execlists_unhold(struct intel_engine_cs *engine, - struct i915_request *rq) -{ - spin_lock_irq(&engine->active.lock); - - /* - * Move this request back to the priority queue, and all of its - * children and grandchildren that were suspended along with it. - */ - __execlists_unhold(rq); - - if (rq_prio(rq) > engine->execlists.queue_priority_hint) { - engine->execlists.queue_priority_hint = rq_prio(rq); - tasklet_hi_schedule(&engine->execlists.tasklet); - } - - spin_unlock_irq(&engine->active.lock); -} - struct execlists_capture { struct work_struct work; struct i915_request *rq; @@ -2116,7 +1953,7 @@ static void execlists_capture_work(struct work_struct *work) i915_gpu_coredump_put(cap->error); /* Return this request and all that depend upon it for signaling */ - execlists_unhold(engine, cap->rq); + i915_sched_resume_request(engine, cap->rq); i915_request_put(cap->rq); kfree(cap); @@ -2250,7 +2087,7 @@ static void execlists_capture(struct intel_engine_cs *engine) * simply hold that request accountable for being non-preemptible * long enough to force the reset. */ - if (!execlists_hold(engine, cap->rq)) + if (!i915_sched_suspend_request(engine, cap->rq)) goto err_rq; INIT_WORK(&cap->work, execlists_capture_work); diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index 73340a96548f..64f6a49a5c22 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -608,7 +608,7 @@ static int live_hold_reset(void *arg) GEM_BUG_ON(execlists_active(&engine->execlists) != rq); i915_request_get(rq); - execlists_hold(engine, rq); + i915_sched_suspend_request(engine, rq); GEM_BUG_ON(!i915_request_on_hold(rq)); __intel_engine_reset_bh(engine, NULL); @@ -630,7 +630,7 @@ static int live_hold_reset(void *arg) GEM_BUG_ON(!i915_request_on_hold(rq)); /* But is resubmitted on release */ - execlists_unhold(engine, rq); + i915_sched_resume_request(engine, rq); if (i915_request_wait(rq, 0, HZ / 5) < 0) { pr_err("%s: held request did not complete!\n", engine->name); @@ -4606,7 +4606,7 @@ static int reset_virtual_engine(struct intel_gt *gt, GEM_BUG_ON(rq->engine != engine); /* Reset the engine while keeping our active request on hold */ - execlists_hold(engine, rq); + i915_sched_suspend_request(engine, rq); GEM_BUG_ON(!i915_request_on_hold(rq)); __intel_engine_reset_bh(engine, NULL); @@ -4629,7 +4629,7 @@ static int reset_virtual_engine(struct intel_gt *gt, GEM_BUG_ON(!i915_request_on_hold(rq)); /* But is resubmitted on release */ - execlists_unhold(engine, rq); + i915_sched_resume_request(engine, rq); if (i915_request_wait(rq, 0, HZ / 5) < 0) { pr_err("%s: held request did not complete!\n", engine->name); diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index 9fcfbf303ce0..351c0c0055b5 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -586,6 +586,159 @@ __i915_sched_rewind_requests(struct intel_engine_cs *engine) return active; } +bool __i915_sched_suspend_request(struct intel_engine_cs *engine, + struct i915_request *rq) +{ + LIST_HEAD(list); + + lockdep_assert_held(&engine->active.lock); + GEM_BUG_ON(rq->engine != engine); + + if (__i915_request_is_complete(rq)) /* too late! */ + return false; + + if (i915_request_on_hold(rq)) + return false; + + ENGINE_TRACE(engine, "suspending request %llx:%lld\n", + rq->fence.context, rq->fence.seqno); + + /* + * Transfer this request onto the hold queue to prevent it + * being resumbitted to HW (and potentially completed) before we have + * released it. Since we may have already submitted following + * requests, we need to remove those as well. + */ + do { + struct i915_dependency *p; + + if (i915_request_is_active(rq)) + __i915_request_unsubmit(rq); + + list_move_tail(&rq->sched.link, &engine->active.hold); + clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); + i915_request_set_hold(rq); + RQ_TRACE(rq, "on hold\n"); + + for_each_waiter(p, rq) { + struct i915_request *w = + container_of(p->waiter, typeof(*w), sched); + + if (p->flags & I915_DEPENDENCY_WEAK) + continue; + + /* Leave semaphores spinning on the other engines */ + if (w->engine != engine) + continue; + + if (!i915_request_is_ready(w)) + continue; + + if (__i915_request_is_complete(w)) + continue; + + if (i915_request_on_hold(w)) /* acts as a visited bit */ + continue; + + list_move_tail(&w->sched.link, &list); + } + + rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); + } while (rq); + + GEM_BUG_ON(list_empty(&engine->active.hold)); + + return true; +} + +bool i915_sched_suspend_request(struct intel_engine_cs *engine, + struct i915_request *rq) +{ + bool result; + + if (i915_request_on_hold(rq)) + return false; + + spin_lock_irq(&engine->active.lock); + result = __i915_sched_suspend_request(engine, rq); + spin_unlock_irq(&engine->active.lock); + + return result; +} + +void __i915_sched_resume_request(struct intel_engine_cs *engine, + struct i915_request *rq) +{ + LIST_HEAD(list); + + lockdep_assert_held(&engine->active.lock); + + if (rq_prio(rq) > engine->execlists.queue_priority_hint) { + engine->execlists.queue_priority_hint = rq_prio(rq); + tasklet_hi_schedule(&engine->execlists.tasklet); + } + + if (!i915_request_on_hold(rq)) + return; + + ENGINE_TRACE(engine, "resuming request %llx:%lld\n", + rq->fence.context, rq->fence.seqno); + + /* + * Move this request back to the priority queue, and all of its + * children and grandchildren that were suspended along with it. + */ + do { + struct i915_dependency *p; + + RQ_TRACE(rq, "hold release\n"); + + GEM_BUG_ON(!i915_request_on_hold(rq)); + GEM_BUG_ON(!i915_sw_fence_signaled(&rq->submit)); + + i915_request_clear_hold(rq); + list_del_init(&rq->sched.link); + + queue_request(engine, rq); + + /* Also release any children on this engine that are ready */ + for_each_waiter(p, rq) { + struct i915_request *w = + container_of(p->waiter, typeof(*w), sched); + + if (p->flags & I915_DEPENDENCY_WEAK) + continue; + + /* Propagate any change in error status */ + if (rq->fence.error) + i915_request_set_error_once(w, rq->fence.error); + + if (w->engine != engine) + continue; + + /* We also treat the on-hold status as a visited bit */ + if (!i915_request_on_hold(w)) + continue; + + /* Check that no other parents are also on hold [BFS] */ + if (hold_request(w)) + continue; + + list_move_tail(&w->sched.link, &list); + } + + rq = list_first_entry_or_null(&list, typeof(*rq), sched.link); + } while (rq); +} + +void i915_sched_resume_request(struct intel_engine_cs *engine, + struct i915_request *rq) +{ + spin_lock_irq(&engine->active.lock); + __i915_sched_resume_request(engine, rq); + spin_unlock_irq(&engine->active.lock); +} + void i915_sched_node_init(struct i915_sched_node *node) { spin_lock_init(&node->lock); diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index d3984f65b3a6..9860459fedb1 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -45,6 +45,16 @@ void i915_request_enqueue(struct i915_request *request); struct i915_request * __i915_sched_rewind_requests(struct intel_engine_cs *engine); +bool __i915_sched_suspend_request(struct intel_engine_cs *engine, + struct i915_request *rq); +void __i915_sched_resume_request(struct intel_engine_cs *engine, + struct i915_request *request); + +bool i915_sched_suspend_request(struct intel_engine_cs *engine, + struct i915_request *request); +void i915_sched_resume_request(struct intel_engine_cs *engine, + struct i915_request *rq); + struct list_head * i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio);
Make the ability to suspend and resume a request and its dependents generic. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- .../drm/i915/gt/intel_execlists_submission.c | 167 +----------------- drivers/gpu/drm/i915/gt/selftest_execlists.c | 8 +- drivers/gpu/drm/i915/i915_scheduler.c | 153 ++++++++++++++++ drivers/gpu/drm/i915/i915_scheduler.h | 10 ++ 4 files changed, 169 insertions(+), 169 deletions(-)