Re: [Intel-gfx] [PATCH 17/57] drm/i915: Extract request suspension from the execlists
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
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/57] drm/i915: Extract request suspension from the execlists
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/57] drm/i915: Extract request suspension from the execlists
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/57] drm/i915: Extract request suspension from the execlists
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
---
.../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
