[Intel-gfx] [PATCH 8/9] drm/i915: Move submission tasklet to i915_sched_engine

Jason Ekstrand jason at jlekstrand.net
Fri Jun 4 19:26:38 UTC 2021


On Thu, Jun 3, 2021 at 4:09 PM Matthew Brost <matthew.brost at intel.com> wrote:
>
> The submission tasklet operates on i915_sched_engine, thus it is the
> correct place for it.
>
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h        | 14 ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     | 12 +--
>  drivers/gpu/drm/i915/gt/intel_engine_types.h  |  5 --
>  .../drm/i915/gt/intel_execlists_submission.c  | 86 ++++++++++---------
>  drivers/gpu/drm/i915/gt/mock_engine.c         |  1 +
>  drivers/gpu/drm/i915/gt/selftest_execlists.c  | 16 ++--
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  2 +-
>  drivers/gpu/drm/i915/gt/selftest_lrc.c        |  6 +-
>  drivers/gpu/drm/i915/gt/selftest_reset.c      |  2 +-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 25 +++---
>  drivers/gpu/drm/i915/i915_scheduler.c         |  1 +
>  drivers/gpu/drm/i915/i915_scheduler.h         | 14 +++
>  drivers/gpu/drm/i915/i915_scheduler_types.h   |  8 ++
>  13 files changed, 99 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index a8b2174b4395..988d9688ae4d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -123,20 +123,6 @@ execlists_active(const struct intel_engine_execlists *execlists)
>         return active;
>  }
>
> -static inline void
> -execlists_active_lock_bh(struct intel_engine_execlists *execlists)
> -{
> -       local_bh_disable(); /* prevent local softirq and lock recursion */
> -       tasklet_lock(&execlists->tasklet);
> -}
> -
> -static inline void
> -execlists_active_unlock_bh(struct intel_engine_execlists *execlists)
> -{
> -       tasklet_unlock(&execlists->tasklet);
> -       local_bh_enable(); /* restore softirq, and kick ksoftirqd! */
> -}
> -
>  struct i915_request *
>  execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists);
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index b480fcb1aad1..837d2132e31b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -711,6 +711,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>                 err = -ENOMEM;
>                 goto err_sched_engine;
>         }
> +       engine->sched_engine->engine = engine;

This bothers me.  If we're trying to separate things, why do we have a
back pointer?  Yes, I know it's because we need to access our engine
back-end somehow.  Makes me wonder if it should be some sort of a
void* private data pointer instead of the engine.  That's probably
less safe but it seems more "right" in some sense?  I don't know.

Also, dumb question but what's a tasklet?

--Jason

>
>         err = intel_engine_init_cmd_parser(engine);
>         if (err)
> @@ -935,7 +936,6 @@ int intel_engines_init(struct intel_gt *gt)
>  void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>  {
>         GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
> -       tasklet_kill(&engine->execlists.tasklet); /* flush the callback */
>
>         i915_sched_engine_put(engine->sched_engine);
>         intel_breadcrumbs_free(engine->breadcrumbs);
> @@ -1221,7 +1221,7 @@ static bool ring_is_idle(struct intel_engine_cs *engine)
>
>  void __intel_engine_flush_submission(struct intel_engine_cs *engine, bool sync)
>  {
> -       struct tasklet_struct *t = &engine->execlists.tasklet;
> +       struct tasklet_struct *t = &engine->sched_engine->tasklet;
>
>         if (!t->callback)
>                 return;
> @@ -1482,8 +1482,8 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
>
>                 drm_printf(m, "\tExeclist tasklet queued? %s (%s), preempt? %s, timeslice? %s\n",
>                            yesno(test_bit(TASKLET_STATE_SCHED,
> -                                         &engine->execlists.tasklet.state)),
> -                          enableddisabled(!atomic_read(&engine->execlists.tasklet.count)),
> +                                         &engine->sched_engine->tasklet.state)),
> +                          enableddisabled(!atomic_read(&engine->sched_engine->tasklet.count)),
>                            repr_timer(&engine->execlists.preempt),
>                            repr_timer(&engine->execlists.timer));
>
> @@ -1507,7 +1507,7 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
>                                    idx, hws[idx * 2], hws[idx * 2 + 1]);
>                 }
>
> -               execlists_active_lock_bh(execlists);
> +               i915_sched_engine_active_lock_bh(engine->sched_engine);
>                 rcu_read_lock();
>                 for (port = execlists->active; (rq = *port); port++) {
>                         char hdr[160];
> @@ -1538,7 +1538,7 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine,
>                         i915_request_show(m, rq, hdr, 0);
>                 }
>                 rcu_read_unlock();
> -               execlists_active_unlock_bh(execlists);
> +               i915_sched_engine_active_unlock_bh(engine->sched_engine);
>         } else if (INTEL_GEN(dev_priv) > 6) {
>                 drm_printf(m, "\tPP_DIR_BASE: 0x%08x\n",
>                            ENGINE_READ(engine, RING_PP_DIR_BASE));
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index f1b14aff5118..6f2fd6f13ac4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -138,11 +138,6 @@ struct st_preempt_hang {
>   * driver and the hardware state for execlist mode of submission.
>   */
>  struct intel_engine_execlists {
> -       /**
> -        * @tasklet: softirq tasklet for bottom handler
> -        */
> -       struct tasklet_struct tasklet;
> -
>         /**
>          * @timer: kick the current context if its timeslice expires
>          */
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 7240c153be35..7a93aec3f6a9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -570,7 +570,7 @@ static void kick_siblings(struct i915_request *rq, struct intel_context *ce)
>                 resubmit_virtual_request(rq, ve);
>
>         if (READ_ONCE(ve->request))
> -               tasklet_hi_schedule(&ve->base.execlists.tasklet);
> +               tasklet_hi_schedule(&ve->base.sched_engine->tasklet);
>  }
>
>  static void __execlists_schedule_out(struct i915_request * const rq,
> @@ -739,9 +739,9 @@ trace_ports(const struct intel_engine_execlists *execlists,
>  }
>
>  static bool
> -reset_in_progress(const struct intel_engine_execlists *execlists)
> +reset_in_progress(const struct intel_engine_cs *engine)
>  {
> -       return unlikely(!__tasklet_is_enabled(&execlists->tasklet));
> +       return unlikely(!__tasklet_is_enabled(&engine->sched_engine->tasklet));
>  }
>
>  static __maybe_unused noinline bool
> @@ -757,7 +757,7 @@ assert_pending_valid(const struct intel_engine_execlists *execlists,
>         trace_ports(execlists, msg, execlists->pending);
>
>         /* We may be messing around with the lists during reset, lalala */
> -       if (reset_in_progress(execlists))
> +       if (reset_in_progress(engine))
>                 return true;
>
>         if (!execlists->pending[0]) {
> @@ -1190,7 +1190,7 @@ static void start_timeslice(struct intel_engine_cs *engine)
>                          * its timeslice, so recheck.
>                          */
>                         if (!timer_pending(&el->timer))
> -                               tasklet_hi_schedule(&el->tasklet);
> +                               tasklet_hi_schedule(&engine->sched_engine->tasklet);
>                         return;
>                 }
>
> @@ -1772,8 +1772,8 @@ process_csb(struct intel_engine_cs *engine, struct i915_request **inactive)
>          * access. Either we are inside the tasklet, or the tasklet is disabled
>          * and we assume that is only inside the reset paths and so serialised.
>          */
> -       GEM_BUG_ON(!tasklet_is_locked(&execlists->tasklet) &&
> -                  !reset_in_progress(execlists));
> +       GEM_BUG_ON(!tasklet_is_locked(&engine->sched_engine->tasklet) &&
> +                  !reset_in_progress(engine));
>
>         /*
>          * Note that csb_write, csb_status may be either in HWSP or mmio.
> @@ -2131,7 +2131,7 @@ static void execlists_unhold(struct intel_engine_cs *engine,
>
>         if (rq_prio(rq) > engine->sched_engine->queue_priority_hint) {
>                 engine->sched_engine->queue_priority_hint = rq_prio(rq);
> -               tasklet_hi_schedule(&engine->execlists.tasklet);
> +               tasklet_hi_schedule(&engine->sched_engine->tasklet);
>         }
>
>         spin_unlock_irq(&engine->sched_engine->lock);
> @@ -2322,13 +2322,13 @@ static void execlists_reset(struct intel_engine_cs *engine, const char *msg)
>         ENGINE_TRACE(engine, "reset for %s\n", msg);
>
>         /* Mark this tasklet as disabled to avoid waiting for it to complete */
> -       tasklet_disable_nosync(&engine->execlists.tasklet);
> +       tasklet_disable_nosync(&engine->sched_engine->tasklet);
>
>         ring_set_paused(engine, 1); /* Freeze the current request in place */
>         execlists_capture(engine);
>         intel_engine_reset(engine, msg);
>
> -       tasklet_enable(&engine->execlists.tasklet);
> +       tasklet_enable(&engine->sched_engine->tasklet);
>         clear_and_wake_up_bit(bit, lock);
>  }
>
> @@ -2351,8 +2351,9 @@ static bool preempt_timeout(const struct intel_engine_cs *const engine)
>   */
>  static void execlists_submission_tasklet(struct tasklet_struct *t)
>  {
> -       struct intel_engine_cs * const engine =
> -               from_tasklet(engine, t, execlists.tasklet);
> +       struct i915_sched_engine *sched_engine =
> +               from_tasklet(sched_engine, t, tasklet);
> +       struct intel_engine_cs * const engine = sched_engine->engine;
>         struct i915_request *post[2 * EXECLIST_MAX_PORTS];
>         struct i915_request **inactive;
>
> @@ -2427,13 +2428,16 @@ static void execlists_irq_handler(struct intel_engine_cs *engine, u16 iir)
>                 intel_engine_signal_breadcrumbs(engine);
>
>         if (tasklet)
> -               tasklet_hi_schedule(&engine->execlists.tasklet);
> +               tasklet_hi_schedule(&engine->sched_engine->tasklet);
>  }
>
>  static void __execlists_kick(struct intel_engine_execlists *execlists)
>  {
> +       struct intel_engine_cs *engine =
> +               container_of(execlists, typeof(*engine), execlists);
> +
>         /* Kick the tasklet for some interrupt coalescing and reset handling */
> -       tasklet_hi_schedule(&execlists->tasklet);
> +       tasklet_hi_schedule(&engine->sched_engine->tasklet);
>  }
>
>  #define execlists_kick(t, member) \
> @@ -2808,10 +2812,8 @@ static int execlists_resume(struct intel_engine_cs *engine)
>
>  static void execlists_reset_prepare(struct intel_engine_cs *engine)
>  {
> -       struct intel_engine_execlists * const execlists = &engine->execlists;
> -
>         ENGINE_TRACE(engine, "depth<-%d\n",
> -                    atomic_read(&execlists->tasklet.count));
> +                    atomic_read(&engine->sched_engine->tasklet.count));
>
>         /*
>          * Prevent request submission to the hardware until we have
> @@ -2822,8 +2824,8 @@ static void execlists_reset_prepare(struct intel_engine_cs *engine)
>          * Turning off the execlists->tasklet until the reset is over
>          * prevents the race.
>          */
> -       __tasklet_disable_sync_once(&execlists->tasklet);
> -       GEM_BUG_ON(!reset_in_progress(execlists));
> +       __tasklet_disable_sync_once(&engine->sched_engine->tasklet);
> +       GEM_BUG_ON(!reset_in_progress(engine));
>
>         /*
>          * We stop engines, otherwise we might get failed reset and a
> @@ -2973,8 +2975,9 @@ static void execlists_reset_rewind(struct intel_engine_cs *engine, bool stalled)
>
>  static void nop_submission_tasklet(struct tasklet_struct *t)
>  {
> -       struct intel_engine_cs * const engine =
> -               from_tasklet(engine, t, execlists.tasklet);
> +       struct i915_sched_engine *sched_engine =
> +               from_tasklet(sched_engine, t, tasklet);
> +       struct intel_engine_cs * const engine = sched_engine->engine;
>
>         /* The driver is wedged; don't process any more events. */
>         WRITE_ONCE(engine->sched_engine->queue_priority_hint, INT_MIN);
> @@ -3061,8 +3064,8 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine)
>         sched_engine->queue_priority_hint = INT_MIN;
>         sched_engine->queue = RB_ROOT_CACHED;
>
> -       GEM_BUG_ON(__tasklet_is_enabled(&execlists->tasklet));
> -       execlists->tasklet.callback = nop_submission_tasklet;
> +       GEM_BUG_ON(__tasklet_is_enabled(&engine->sched_engine->tasklet));
> +       engine->sched_engine->tasklet.callback = nop_submission_tasklet;
>
>         spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
>         rcu_read_unlock();
> @@ -3082,14 +3085,14 @@ static void execlists_reset_finish(struct intel_engine_cs *engine)
>          * reset as the next level of recovery, and as a final resort we
>          * will declare the device wedged.
>          */
> -       GEM_BUG_ON(!reset_in_progress(execlists));
> +       GEM_BUG_ON(!reset_in_progress(engine));
>
>         /* And kick in case we missed a new request submission. */
> -       if (__tasklet_enable(&execlists->tasklet))
> +       if (__tasklet_enable(&engine->sched_engine->tasklet))
>                 __execlists_kick(execlists);
>
>         ENGINE_TRACE(engine, "depth->%d\n",
> -                    atomic_read(&execlists->tasklet.count));
> +                    atomic_read(&engine->sched_engine->tasklet.count));
>  }
>
>  static void gen8_logical_ring_enable_irq(struct intel_engine_cs *engine)
> @@ -3153,8 +3156,6 @@ static void kick_execlists(const struct i915_request *rq, int prio)
>                      inflight->fence.context, inflight->fence.seqno,
>                      inflight->sched.attr.priority);
>
> -       sched_engine->queue_priority_hint = prio;
> -
>         /*
>          * Allow preemption of low -> normal -> high, but we do
>          * not allow low priority tasks to preempt other low priority
> @@ -3163,7 +3164,7 @@ static void kick_execlists(const struct i915_request *rq, int prio)
>          * so kiss.
>          */
>         if (prio >= max(I915_PRIORITY_NORMAL, rq_prio(inflight)))
> -               tasklet_hi_schedule(&engine->execlists.tasklet);
> +               tasklet_hi_schedule(&sched_engine->tasklet);
>
>  unlock:
>         rcu_read_unlock();
> @@ -3174,7 +3175,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
>         engine->submit_request = execlists_submit_request;
>         engine->sched_engine->schedule = i915_schedule;
>         engine->sched_engine->kick_backend = kick_execlists;
> -       engine->execlists.tasklet.callback = execlists_submission_tasklet;
> +       engine->sched_engine->tasklet.callback = execlists_submission_tasklet;
>  }
>
>  static void execlists_shutdown(struct intel_engine_cs *engine)
> @@ -3182,7 +3183,7 @@ static void execlists_shutdown(struct intel_engine_cs *engine)
>         /* Synchronise with residual timers and any softirq they raise */
>         del_timer_sync(&engine->execlists.timer);
>         del_timer_sync(&engine->execlists.preempt);
> -       tasklet_kill(&engine->execlists.tasklet);
> +       tasklet_kill(&engine->sched_engine->tasklet);
>  }
>
>  static void execlists_release(struct intel_engine_cs *engine)
> @@ -3298,7 +3299,7 @@ int intel_execlists_submission_setup(struct intel_engine_cs *engine)
>         struct intel_uncore *uncore = engine->uncore;
>         u32 base = engine->mmio_base;
>
> -       tasklet_setup(&engine->execlists.tasklet, execlists_submission_tasklet);
> +       tasklet_setup(&engine->sched_engine->tasklet, execlists_submission_tasklet);
>         timer_setup(&engine->execlists.timer, execlists_timeslice, 0);
>         timer_setup(&engine->execlists.preempt, execlists_preempt, 0);
>
> @@ -3380,7 +3381,7 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk)
>          * rbtrees as in the case it is running in parallel, it may reinsert
>          * the rb_node into a sibling.
>          */
> -       tasklet_kill(&ve->base.execlists.tasklet);
> +       tasklet_kill(&ve->base.sched_engine->tasklet);
>
>         /* Decouple ourselves from the siblings, no more access allowed. */
>         for (n = 0; n < ve->num_siblings; n++) {
> @@ -3392,13 +3393,13 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk)
>
>                 spin_lock_irq(&sibling->sched_engine->lock);
>
> -               /* Detachment is lazily performed in the execlists tasklet */
> +               /* Detachment is lazily performed in the sched_engine->tasklet */
>                 if (!RB_EMPTY_NODE(node))
>                         rb_erase_cached(node, &sibling->execlists.virtual);
>
>                 spin_unlock_irq(&sibling->sched_engine->lock);
>         }
> -       GEM_BUG_ON(__tasklet_is_scheduled(&ve->base.execlists.tasklet));
> +       GEM_BUG_ON(__tasklet_is_scheduled(&ve->base.sched_engine->tasklet));
>         GEM_BUG_ON(!list_empty(virtual_queue(ve)));
>
>         lrc_fini(&ve->context);
> @@ -3545,9 +3546,11 @@ static intel_engine_mask_t virtual_submission_mask(struct virtual_engine *ve)
>
>  static void virtual_submission_tasklet(struct tasklet_struct *t)
>  {
> +       struct i915_sched_engine *sched_engine =
> +               from_tasklet(sched_engine, t, tasklet);
>         struct virtual_engine * const ve =
> -               from_tasklet(ve, t, base.execlists.tasklet);
> -       const int prio = READ_ONCE(ve->base.sched_engine->queue_priority_hint);
> +               (struct virtual_engine *)sched_engine->engine;
> +       const int prio = READ_ONCE(sched_engine->queue_priority_hint);
>         intel_engine_mask_t mask;
>         unsigned int n;
>
> @@ -3616,7 +3619,7 @@ static void virtual_submission_tasklet(struct tasklet_struct *t)
>                 GEM_BUG_ON(RB_EMPTY_NODE(&node->rb));
>                 node->prio = prio;
>                 if (first && prio > sibling->sched_engine->queue_priority_hint)
> -                       tasklet_hi_schedule(&sibling->execlists.tasklet);
> +                       tasklet_hi_schedule(&sibling->sched_engine->tasklet);
>
>  unlock_engine:
>                 spin_unlock_irq(&sibling->sched_engine->lock);
> @@ -3657,7 +3660,7 @@ static void virtual_submit_request(struct i915_request *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);
> +       tasklet_hi_schedule(&ve->base.sched_engine->tasklet);
>
>  unlock:
>         spin_unlock_irqrestore(&ve->base.sched_engine->lock, flags);
> @@ -3751,6 +3754,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
>                 err = -ENOMEM;
>                 goto err_put;
>         }
> +       ve->base.sched_engine->engine = &ve->base;
>
>         ve->base.cops = &virtual_context_ops;
>         ve->base.request_alloc = execlists_request_alloc;
> @@ -3761,7 +3765,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
>         ve->base.bond_execute = virtual_bond_execute;
>
>         INIT_LIST_HEAD(virtual_queue(ve));
> -       tasklet_setup(&ve->base.execlists.tasklet, virtual_submission_tasklet);
> +       tasklet_setup(&ve->base.sched_engine->tasklet, virtual_submission_tasklet);
>
>         intel_context_init(&ve->context, &ve->base);
>
> @@ -3789,7 +3793,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,
>                  * layering if we handle cloning of the requests and
>                  * submitting a copy into each backend.
>                  */
> -               if (sibling->execlists.tasklet.callback !=
> +               if (sibling->sched_engine->tasklet.callback !=
>                     execlists_submission_tasklet) {
>                         err = -ENODEV;
>                         goto err_put;
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> index a49fd3039f13..bd005c1b6fd5 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -349,6 +349,7 @@ int mock_engine_init(struct intel_engine_cs *engine)
>         engine->sched_engine = i915_sched_engine_create(ENGINE_MOCK);
>         if (!engine->sched_engine)
>                 return -ENOMEM;
> +       engine->sched_engine->engine = engine;
>
>         intel_engine_init_execlists(engine);
>         intel_engine_init__pm(engine);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> index 5cc7648d1e5a..2554a2f343b4 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
> @@ -43,7 +43,7 @@ static int wait_for_submit(struct intel_engine_cs *engine,
>                            unsigned long timeout)
>  {
>         /* Ignore our own attempts to suppress excess tasklets */
> -       tasklet_hi_schedule(&engine->execlists.tasklet);
> +       tasklet_hi_schedule(&engine->sched_engine->tasklet);
>
>         timeout += jiffies;
>         do {
> @@ -606,9 +606,9 @@ static int live_hold_reset(void *arg)
>                         err = -EBUSY;
>                         goto out;
>                 }
> -               tasklet_disable(&engine->execlists.tasklet);
> +               tasklet_disable(&engine->sched_engine->tasklet);
>
> -               engine->execlists.tasklet.callback(&engine->execlists.tasklet);
> +               engine->sched_engine->tasklet.callback(&engine->sched_engine->tasklet);
>                 GEM_BUG_ON(execlists_active(&engine->execlists) != rq);
>
>                 i915_request_get(rq);
> @@ -618,7 +618,7 @@ static int live_hold_reset(void *arg)
>                 __intel_engine_reset_bh(engine, NULL);
>                 GEM_BUG_ON(rq->fence.error != -EIO);
>
> -               tasklet_enable(&engine->execlists.tasklet);
> +               tasklet_enable(&engine->sched_engine->tasklet);
>                 clear_and_wake_up_bit(I915_RESET_ENGINE + id,
>                                       &gt->reset.flags);
>                 local_bh_enable();
> @@ -1183,7 +1183,7 @@ static int live_timeslice_rewind(void *arg)
>                 while (i915_request_is_active(rq[A2])) { /* semaphore yield! */
>                         /* Wait for the timeslice to kick in */
>                         del_timer(&engine->execlists.timer);
> -                       tasklet_hi_schedule(&engine->execlists.tasklet);
> +                       tasklet_hi_schedule(&engine->sched_engine->tasklet);
>                         intel_engine_flush_submission(engine);
>                 }
>                 /* -> ELSP[] = { { A:rq1 }, { B:rq1 } } */
> @@ -4593,9 +4593,9 @@ static int reset_virtual_engine(struct intel_gt *gt,
>                 err = -EBUSY;
>                 goto out_heartbeat;
>         }
> -       tasklet_disable(&engine->execlists.tasklet);
> +       tasklet_disable(&engine->sched_engine->tasklet);
>
> -       engine->execlists.tasklet.callback(&engine->execlists.tasklet);
> +       engine->sched_engine->tasklet.callback(&engine->sched_engine->tasklet);
>         GEM_BUG_ON(execlists_active(&engine->execlists) != rq);
>
>         /* Fake a preemption event; failed of course */
> @@ -4612,7 +4612,7 @@ static int reset_virtual_engine(struct intel_gt *gt,
>         GEM_BUG_ON(rq->fence.error != -EIO);
>
>         /* Release our grasp on the engine, letting CS flow again */
> -       tasklet_enable(&engine->execlists.tasklet);
> +       tasklet_enable(&engine->sched_engine->tasklet);
>         clear_and_wake_up_bit(I915_RESET_ENGINE + engine->id, &gt->reset.flags);
>         local_bh_enable();
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index e57dc900ae8d..cbcb800e2ca0 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -1702,7 +1702,7 @@ static int __igt_atomic_reset_engine(struct intel_engine_cs *engine,
>                                      const struct igt_atomic_section *p,
>                                      const char *mode)
>  {
> -       struct tasklet_struct * const t = &engine->execlists.tasklet;
> +       struct tasklet_struct * const t = &engine->sched_engine->tasklet;
>         int err;
>
>         GEM_TRACE("i915_reset_engine(%s:%s) under %s\n",
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index d8f6623524e8..6a3a0d89dcd2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -49,7 +49,7 @@ static int wait_for_submit(struct intel_engine_cs *engine,
>                            unsigned long timeout)
>  {
>         /* Ignore our own attempts to suppress excess tasklets */
> -       tasklet_hi_schedule(&engine->execlists.tasklet);
> +       tasklet_hi_schedule(&engine->sched_engine->tasklet);
>
>         timeout += jiffies;
>         do {
> @@ -1613,12 +1613,12 @@ static void garbage_reset(struct intel_engine_cs *engine,
>
>         local_bh_disable();
>         if (!test_and_set_bit(bit, lock)) {
> -               tasklet_disable(&engine->execlists.tasklet);
> +               tasklet_disable(&engine->sched_engine->tasklet);
>
>                 if (!rq->fence.error)
>                         __intel_engine_reset_bh(engine, NULL);
>
> -               tasklet_enable(&engine->execlists.tasklet);
> +               tasklet_enable(&engine->sched_engine->tasklet);
>                 clear_and_wake_up_bit(bit, lock);
>         }
>         local_bh_enable();
> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 8784257ec808..7a50c9f4071b 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
> @@ -321,7 +321,7 @@ static int igt_atomic_engine_reset(void *arg)
>                 goto out_unlock;
>
>         for_each_engine(engine, gt, id) {
> -               struct tasklet_struct *t = &engine->execlists.tasklet;
> +               struct tasklet_struct *t = &engine->sched_engine->tasklet;
>
>                 if (t->func)
>                         tasklet_disable(t);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 7ed21670ef14..95c6f6af4047 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -241,8 +241,9 @@ static void __guc_dequeue(struct intel_engine_cs *engine)
>
>  static void guc_submission_tasklet(struct tasklet_struct *t)
>  {
> -       struct intel_engine_cs * const engine =
> -               from_tasklet(engine, t, execlists.tasklet);
> +       struct i915_sched_engine *sched_engine =
> +               from_tasklet(sched_engine, t, tasklet);
> +       struct intel_engine_cs * const engine = sched_engine->engine;
>         struct intel_engine_execlists * const execlists = &engine->execlists;
>         struct i915_request **port, *rq;
>         unsigned long flags;
> @@ -272,14 +273,12 @@ static void cs_irq_handler(struct intel_engine_cs *engine, u16 iir)
>  {
>         if (iir & GT_RENDER_USER_INTERRUPT) {
>                 intel_engine_signal_breadcrumbs(engine);
> -               tasklet_hi_schedule(&engine->execlists.tasklet);
> +               tasklet_hi_schedule(&engine->sched_engine->tasklet);
>         }
>  }
>
>  static void guc_reset_prepare(struct intel_engine_cs *engine)
>  {
> -       struct intel_engine_execlists * const execlists = &engine->execlists;
> -
>         ENGINE_TRACE(engine, "\n");
>
>         /*
> @@ -291,7 +290,7 @@ static void guc_reset_prepare(struct intel_engine_cs *engine)
>          * Turning off the execlists->tasklet until the reset is over
>          * prevents the race.
>          */
> -       __tasklet_disable_sync_once(&execlists->tasklet);
> +       __tasklet_disable_sync_once(&engine->sched_engine->tasklet);
>  }
>
>  static void guc_reset_state(struct intel_context *ce,
> @@ -395,14 +394,12 @@ static void guc_reset_cancel(struct intel_engine_cs *engine)
>
>  static void guc_reset_finish(struct intel_engine_cs *engine)
>  {
> -       struct intel_engine_execlists * const execlists = &engine->execlists;
> -
> -       if (__tasklet_enable(&execlists->tasklet))
> +       if (__tasklet_enable(&engine->sched_engine->tasklet))
>                 /* And kick in case we missed a new request submission. */
> -               tasklet_hi_schedule(&execlists->tasklet);
> +               tasklet_hi_schedule(&engine->sched_engine->tasklet);
>
>         ENGINE_TRACE(engine, "depth->%d\n",
> -                    atomic_read(&execlists->tasklet.count));
> +                    atomic_read(&engine->sched_engine->tasklet.count));
>  }
>
>  /*
> @@ -546,7 +543,7 @@ static void guc_submit_request(struct i915_request *rq)
>         GEM_BUG_ON(i915_sched_engine_is_empty(engine->sched_engine));
>         GEM_BUG_ON(list_empty(&rq->sched.link));
>
> -       tasklet_hi_schedule(&engine->execlists.tasklet);
> +       tasklet_hi_schedule(&engine->sched_engine->tasklet);
>
>         spin_unlock_irqrestore(&engine->sched_engine->lock, flags);
>  }
> @@ -626,7 +623,7 @@ static void guc_release(struct intel_engine_cs *engine)
>  {
>         engine->sanitize = NULL; /* no longer in control, nothing to sanitize */
>
> -       tasklet_kill(&engine->execlists.tasklet);
> +       tasklet_kill(&engine->sched_engine->tasklet);
>
>         intel_engine_cleanup_common(engine);
>         lrc_fini_wa_ctx(engine);
> @@ -705,7 +702,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine)
>          */
>         GEM_BUG_ON(INTEL_GEN(i915) < 11);
>
> -       tasklet_setup(&engine->execlists.tasklet, guc_submission_tasklet);
> +       tasklet_setup(&engine->sched_engine->tasklet, guc_submission_tasklet);
>
>         guc_default_vfuncs(engine);
>         guc_default_irqs(engine);
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index 3d36e4447b5d..6f082579ee9e 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -436,6 +436,7 @@ void i915_sched_engine_free(struct kref *kref)
>         struct i915_sched_engine *sched_engine =
>                 container_of(kref, typeof(*sched_engine), ref);
>
> +       tasklet_kill(&sched_engine->tasklet); /* flush the callback */
>         kfree(sched_engine);
>  }
>
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
> index 0014745bda30..650ab8e0db9f 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.h
> +++ b/drivers/gpu/drm/i915/i915_scheduler.h
> @@ -79,6 +79,20 @@ i915_sched_engine_reset_on_empty(struct i915_sched_engine *sched_engine)
>                 sched_engine->no_priolist = false;
>  }
>
> +static inline void
> +i915_sched_engine_active_lock_bh(struct i915_sched_engine *sched_engine)
> +{
> +       local_bh_disable(); /* prevent local softirq and lock recursion */
> +       tasklet_lock(&sched_engine->tasklet);
> +}
> +
> +static inline void
> +i915_sched_engine_active_unlock_bh(struct i915_sched_engine *sched_engine)
> +{
> +       tasklet_unlock(&sched_engine->tasklet);
> +       local_bh_enable(); /* restore softirq, and kick ksoftirqd! */
> +}
> +
>  void i915_request_show_with_schedule(struct drm_printer *m,
>                                      const struct i915_request *rq,
>                                      const char *prefix,
> diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
> index 8b3af536e6cf..9d79514450de 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler_types.h
> +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
> @@ -103,6 +103,11 @@ struct i915_sched_engine {
>         struct list_head requests;
>         struct list_head hold; /* ready requests, but on hold */
>
> +       /**
> +        * @tasklet: softirq tasklet for bottom handler
> +        */
> +       struct tasklet_struct tasklet;
> +
>         /**
>          * @default_priolist: priority list for I915_PRIORITY_NORMAL
>          */
> @@ -132,6 +137,9 @@ struct i915_sched_engine {
>          */
>         bool no_priolist;
>
> +       /* Back pointer to engine */
> +       struct intel_engine_cs *engine;
> +
>         /* Kick backend */
>         void    (*kick_backend)(const struct i915_request *rq,
>                                 int prio);
> --
> 2.28.0
>


More information about the Intel-gfx mailing list