On 6/8/2021 12:17 PM, Matthew Brost wrote:

Introduce i915_sched_engine object which is lower level data structure that i915_scheduler / generic code can operate on without touching execlist specific structures. This allows additional submission backends to be added without breaking the layering.

This is a bit of detour to integrating the i915 with the DRM scheduler but this object will still exist when the DRM scheduler lands in the i915. It will however look a bit different. It will encapsulate the drm_gpu_scheduler object plus and common variables (to the backends) related to scheduling. Regardless this is a step in the right direction.

I think this needs some more explanation on how we plan to use this structure within i915, otherwise some implementation choices are unclear. E.g. i915_sched_engine_get() is unused in this patch and the whole kref logic only makes sense when you consider that with the GuC submission back-end we plan to point multiple engine_cs to the same sched_engine, so this needs to be explained.

This patch starts the aforementioned transition by moving the priolist into the i915_sched_engine object.

v3: (Jason Ekstrand) Update comment next to intel_engine_cs.virtual Add kernel doc (Checkpatch) Fix double the in commit message

Signed-off-by: Matthew Brost matthew.brost@intel.com

Documentation/gpu/i915.rst | 5 ++ drivers/gpu/drm/i915/gt/intel_engine_cs.c | 14 +++- drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 +- drivers/gpu/drm/i915/gt/intel_engine_types.h | 32 ++------ .../drm/i915/gt/intel_execlists_submission.c | 81 +++++++++++-------- drivers/gpu/drm/i915/gt/mock_engine.c | 9 ++- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 19 ++--- drivers/gpu/drm/i915/i915_scheduler.c | 51 +++++++++--- drivers/gpu/drm/i915/i915_scheduler.h | 18 +++++ drivers/gpu/drm/i915/i915_scheduler_types.h | 47 +++++++++++ 10 files changed, 190 insertions(+), 90 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 42ce0196930a..1d5ce5676d35 100644 --- a/Documentation/gpu/i915.rst +++ b/Documentation/gpu/i915.rst @@ -425,6 +425,11 @@ User Batchbuffer Execution .. kernel-doc:: drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c :doc: User command execution

+Scheduling +---------- +.. kernel-doc:: drivers/gpu/drm/i915/i915_scheduler_types.h

• :functions: i915_sched_engine

• Logical Rings, Logical Ring Contexts and Execlists

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 9ceddfbb1687..49d44c3ac055 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -585,9 +585,6 @@ void intel_engine_init_execlists(struct intel_engine_cs *engine) memset(execlists->pending, 0, sizeof(execlists->pending)); execlists->active = memset(execlists->inflight, 0, sizeof(execlists->inflight));

• execlists->queue_priority_hint = INT_MIN;

• execlists->queue = RB_ROOT_CACHED; }

  static void cleanup_status_page(struct intel_engine_cs *engine)

@@ -714,6 +711,12 @@ static int engine_setup_common(struct intel_engine_cs *engine) goto err_status; }

• engine->sched_engine = i915_sched_engine_create(ENGINE_PHYSICAL);
• if (!engine->sched_engine) {

• err = -ENOMEM;
  

• goto err_sched_engine;
  

• }
• err = intel_engine_init_cmd_parser(engine); if (err) goto err_cmd_parser;

@@ -737,6 +740,8 @@ static int engine_setup_common(struct intel_engine_cs *engine) return 0;

err_cmd_parser:

• i915_sched_engine_put(engine->sched_engine);

+err_sched_engine: intel_breadcrumbs_free(engine->breadcrumbs); err_status: cleanup_status_page(engine); @@ -960,6 +965,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) GEM_BUG_ON(!list_empty(&engine->active.requests)); tasklet_kill(&engine->execlists.tasklet); /* flush the callback */

• i915_sched_engine_put(engine->sched_engine); intel_breadcrumbs_free(engine->breadcrumbs);

  intel_engine_fini_retire(engine);

@@ -1283,7 +1289,7 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) intel_engine_flush_submission(engine);

/* ELSP is empty, but there are ready requests? E.g. after reset */

• if (!RB_EMPTY_ROOT(&engine->execlists.queue.rb_root))

• if (!RB_EMPTY_ROOT(&engine->sched_engine->queue.rb_root)) return false;

  /* Ring stopped? */

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index 47f4397095e5..b6a00dd72808 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -275,12 +275,12 @@ static int __engine_park(struct intel_wakeref *wf) intel_breadcrumbs_park(engine->breadcrumbs);

/* Must be reset upon idling, or we may miss the busy wakeup. */

• GEM_BUG_ON(engine->execlists.queue_priority_hint != INT_MIN);

• GEM_BUG_ON(engine->sched_engine->queue_priority_hint != INT_MIN);

  if (engine->park) engine->park(engine);

• engine->execlists.no_priolist = false;

• engine->sched_engine->no_priolist = false;

  /* While gt calls i915_vma_parked(), we have to break the lock cycle */ intel_gt_pm_put_async(engine->gt);

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index e113f93b3274..e41a9c3f9269 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -59,6 +59,7 @@ struct drm_i915_reg_table; struct i915_gem_context; struct i915_request; struct i915_sched_attr; +struct i915_sched_engine; struct intel_gt; struct intel_ring; struct intel_uncore; @@ -152,11 +153,6 @@ struct intel_engine_execlists { */ struct timer_list preempt;

• /**

• * @default_priolist: priority list for I915_PRIORITY_NORMAL
  

• */
  

• struct i915_priolist default_priolist;
• /**
  • @ccid: identifier for contexts submitted to this engine
  */

@@ -191,11 +187,6 @@ struct intel_engine_execlists { */ u32 reset_ccid;

• /**

• * @no_priolist: priority lists disabled
  

• */
  

• bool no_priolist;
• /**
  • @submit_reg: gen-specific execlist submission register
  • set to the ExecList Submission Port (elsp) register pre-Gen11 and to

@@ -238,23 +229,10 @@ struct intel_engine_execlists { unsigned int port_mask;

/**

• * @queue_priority_hint: Highest pending priority.
  

• *
  

• * When we add requests into the queue, or adjust the priority of
  

• * executing requests, we compute the maximum priority of those
  

• * pending requests. We can then use this value to determine if
  

• * we need to preempt the executing requests to service the queue.
  

• * However, since the we may have recorded the priority of an inflight
  

• * request we wanted to preempt but since completed, at the time of
  

• * dequeuing the priority hint may no longer may match the highest
  

• * available request priority.
  

• * @virtual: Queue of requets on a virtual engine, sorted by priority.
  

• * Each RB entry is a struct i915_priolist containing a list of requests
  

• * of the same priority.
  

  */

• int queue_priority_hint;

• /**

• * @queue: queue of requests, in priority lists
  

• */
  

• struct rb_root_cached queue; struct rb_root_cached virtual;

  /**

@@ -332,6 +310,8 @@ struct intel_engine_cs { struct list_head hold; /* ready requests, but on hold */ } active;

• struct i915_sched_engine *sched_engine;
• /* keep a request in reserve for a [pm] barrier under oom */ struct i915_request *request_pool;

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index fc77592d88a9..4f759559a792 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -273,11 +273,11 @@ static int effective_prio(const struct i915_request *rq) return prio; }

-static int queue_prio(const struct intel_engine_execlists *execlists) +static int queue_prio(const struct i915_sched_engine *sched_engine) { struct rb_node *rb;

• rb = rb_first_cached(&execlists->queue);

• rb = rb_first_cached(&sched_engine->queue); if (!rb) return INT_MIN;

@@ -318,7 +318,7 @@ static bool need_preempt(const struct intel_engine_cs *engine, * to preserve FIFO ordering of dependencies. */ last_prio = max(effective_prio(rq), I915_PRIORITY_NORMAL - 1);

• if (engine->execlists.queue_priority_hint <= last_prio)

• if (engine->sched_engine->queue_priority_hint <= last_prio) return false;

  /*

@@ -340,7 +340,7 @@ static bool need_preempt(const struct intel_engine_cs *engine, * context, it's priority would not exceed ELSP[0] aka last_prio. */ return max(virtual_prio(&engine->execlists),

•    queue_prio(&engine->execlists)) > last_prio;
  

•    queue_prio(engine->sched_engine)) > last_prio;
  

  }

  __maybe_unused static bool

@@ -384,7 +384,7 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine) prio = rq_prio(rq); pl = i915_sched_lookup_priolist(engine, prio); }

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
  

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->sched_engine->queue.rb_root));
  
  

  list_move(&rq->sched.link, pl); set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);

@@ -1139,7 +1139,7 @@ static bool needs_timeslice(const struct intel_engine_cs *engine, }

/* Otherwise, ELSP[0] is by itself, but may be waiting in the queue */

• if (!RB_EMPTY_ROOT(&engine->execlists.queue.rb_root)) {

• if (!RB_EMPTY_ROOT(&engine->sched_engine->queue.rb_root)) { ENGINE_TRACE(engine, "timeslice required for queue\n"); return true; }

@@ -1236,6 +1236,7 @@ static bool completed(const struct i915_request *rq) static void execlists_dequeue(struct intel_engine_cs *engine) { struct intel_engine_execlists * const execlists = &engine->execlists;

• struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_request **port = execlists->pending; struct i915_request ** const last_port = port + execlists->port_mask; struct i915_request *last, * const *active;

@@ -1287,7 +1288,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) last->fence.context, last->fence.seqno, last->sched.attr.priority,

• 		     execlists->queue_priority_hint);
  

• 		     sched_engine->queue_priority_hint);
  record_preemption(execlists);
  
  /*
  

@@ -1313,7 +1314,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) yesno(timer_expired(&execlists->timer)), last->fence.context, last->fence.seqno, rq_prio(last),

• 		     execlists->queue_priority_hint,
  

• 		     sched_engine->queue_priority_hint,
  	     yesno(timeslice_yield(execlists, last)));
  
  /*
  

@@ -1384,7 +1385,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(rq->engine != &ve->base); GEM_BUG_ON(rq->context != &ve->context);

• if (unlikely(rq_prio(rq) < queue_prio(execlists))) {
  

• if (unlikely(rq_prio(rq) < queue_prio(sched_engine))) {
  spin_unlock(&ve->base.active.lock);
  break;
  

  }

@@ -1405,7 +1406,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) yesno(engine != ve->siblings[0]));

WRITE_ONCE(ve->request, NULL);

• WRITE_ONCE(ve->base.execlists.queue_priority_hint, INT_MIN);
  

• WRITE_ONCE(ve->base.sched_engine->queue_priority_hint, INT_MIN);
  
  

  rb = &ve->nodes[engine->id].rb; rb_erase_cached(rb, &execlists->virtual);

@@ -1450,7 +1451,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) break; }

• while ((rb = rb_first_cached(&execlists->queue))) {

• while ((rb = rb_first_cached(&sched_engine->queue))) { struct i915_priolist *p = to_priolist(rb); struct i915_request *rq, *rn;

@@ -1529,7 +1530,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) } }

• rb_erase_cached(&p->node, &execlists->queue);
  

• rb_erase_cached(&p->node, &sched_engine->queue);
  

  i915_priolist_free(p); } done:

@@ -1551,7 +1552,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * request triggering preemption on the next dequeue (or subsequent * interrupt for secondary ports). */

• execlists->queue_priority_hint = queue_prio(execlists);

• sched_engine->queue_priority_hint = queue_prio(sched_engine); spin_unlock(&engine->active.lock);

  /*

@@ -2123,8 +2124,8 @@ static void execlists_unhold(struct intel_engine_cs *engine, */ __execlists_unhold(rq);

• if (rq_prio(rq) > engine->execlists.queue_priority_hint) {

• engine->execlists.queue_priority_hint = rq_prio(rq);
  

• 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); }

@@ -2455,12 +2456,12 @@ static void queue_request(struct intel_engine_cs *engine, static bool submit_queue(struct intel_engine_cs *engine, const struct i915_request *rq) {

• struct intel_engine_execlists *execlists = &engine->execlists;

• struct i915_sched_engine *sched_engine = engine->sched_engine;

• if (rq_prio(rq) <= execlists->queue_priority_hint)

• if (rq_prio(rq) <= sched_engine->queue_priority_hint) return false;

• execlists->queue_priority_hint = rq_prio(rq);

• sched_engine->queue_priority_hint = rq_prio(rq); return true; }

@@ -2486,7 +2487,7 @@ static void execlists_submit_request(struct i915_request *request) } else { queue_request(engine, request);

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));
  

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->sched_engine->queue.rb_root));
  

  GEM_BUG_ON(list_empty(&request->sched.link));

  if (submit_queue(engine, request))

@@ -2969,12 +2970,13 @@ static void nop_submission_tasklet(struct tasklet_struct *t) from_tasklet(engine, t, execlists.tasklet);

/* The driver is wedged; don't process any more events. */

• WRITE_ONCE(engine->execlists.queue_priority_hint, INT_MIN);

• WRITE_ONCE(engine->sched_engine->queue_priority_hint, INT_MIN); }

  static void execlists_reset_cancel(struct intel_engine_cs *engine) { struct intel_engine_execlists * const execlists = &engine->execlists;

• struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_request *rq, *rn; struct rb_node *rb; unsigned long flags;

@@ -3006,7 +3008,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) intel_engine_signal_breadcrumbs(engine);

/* Flush the queued requests to the timeline list (for retiring). */

• while ((rb = rb_first_cached(&execlists->queue))) {

• while ((rb = rb_first_cached(&sched_engine->queue))) { struct i915_priolist *p = to_priolist(rb);

  priolist_for_each_request_consume(rq, rn, p) {

@@ -3016,7 +3018,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) } }

• rb_erase_cached(&p->node, &execlists->queue);
  

• rb_erase_cached(&p->node, &sched_engine->queue);
  

  i915_priolist_free(p); }

@@ -3042,15 +3044,15 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) } i915_request_put(rq);

• 	ve->base.execlists.queue_priority_hint = INT_MIN;
  

• 	ve->base.sched_engine->queue_priority_hint = INT_MIN;
  

  } spin_unlock(&ve->base.active.lock); }

  /* Remaining _unready_ requests will be nop'ed when submitted */

• execlists->queue_priority_hint = INT_MIN;
• execlists->queue = RB_ROOT_CACHED;

• 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;

@@ -3286,7 +3288,7 @@ int intel_execlists_submission_setup(struct intel_engine_cs *engine)

static struct list_head *virtual_queue(struct virtual_engine *ve) {

• return &ve->base.execlists.default_priolist.requests;

• return &ve->base.sched_engine->default_priolist.requests; }

  static void rcu_virtual_context_destroy(struct work_struct *wrk)

@@ -3344,7 +3346,10 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk) lrc_fini(&ve->context); intel_context_fini(&ve->context);

• intel_breadcrumbs_free(ve->base.breadcrumbs);

• if (ve->base.breadcrumbs)

• intel_breadcrumbs_free(ve->base.breadcrumbs);
  

• if (ve->base.sched_engine)

• i915_sched_engine_put(ve->base.sched_engine);
  

  intel_engine_free_request_pool(&ve->base);

  kfree(ve->bonds);

@@ -3475,7 +3480,7 @@ static intel_engine_mask_t virtual_submission_mask(struct virtual_engine *ve)

ENGINE_TRACE(&ve->base, "rq=%llx:%lld, mask=%x, prio=%d\n", rq->fence.context, rq->fence.seqno,

•      mask, ve->base.execlists.queue_priority_hint);
  

•      mask, ve->base.sched_engine->queue_priority_hint);
  
  

  return mask; }

@@ -3484,7 +3489,7 @@ static void virtual_submission_tasklet(struct tasklet_struct *t) { struct virtual_engine * const ve = from_tasklet(ve, t, base.execlists.tasklet);

• const int prio = READ_ONCE(ve->base.execlists.queue_priority_hint);

• const int prio = READ_ONCE(ve->base.sched_engine->queue_priority_hint); intel_engine_mask_t mask; unsigned int n;

@@ -3552,7 +3557,7 @@ static void virtual_submission_tasklet(struct tasklet_struct *t) submit_engine: GEM_BUG_ON(RB_EMPTY_NODE(&node->rb)); node->prio = prio;

• if (first && prio > sibling->execlists.queue_priority_hint)
  

• if (first && prio > sibling->sched_engine->queue_priority_hint)
  tasklet_hi_schedule(&sibling->execlists.tasklet);
  
  

  unlock_engine:

@@ -3588,7 +3593,7 @@ static void virtual_submit_request(struct i915_request *rq) i915_request_put(ve->request); }

• ve->base.execlists.queue_priority_hint = rq_prio(rq);

• ve->base.sched_engine->queue_priority_hint = rq_prio(rq); ve->request = i915_request_get(rq);

  GEM_BUG_ON(!list_empty(virtual_queue(ve)));

@@ -3684,6 +3689,12 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, intel_engine_init_active(&ve->base, ENGINE_VIRTUAL); intel_engine_init_execlists(&ve->base);

• ve->base.sched_engine = i915_sched_engine_create(ENGINE_VIRTUAL);
• if (!ve->base.sched_engine) {

• err = -ENOMEM;
  

• goto err_put;
  

• }
• ve->base.cops = &virtual_context_ops; ve->base.request_alloc = execlists_request_alloc;

@@ -3692,7 +3703,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, ve->base.bond_execute = virtual_bond_execute;

INIT_LIST_HEAD(virtual_queue(ve));

• ve->base.execlists.queue_priority_hint = INT_MIN; tasklet_setup(&ve->base.execlists.tasklet, virtual_submission_tasklet);

  intel_context_init(&ve->context, &ve->base);

@@ -3849,6 +3859,7 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, unsigned int max) { const struct intel_engine_execlists *execlists = &engine->execlists;

• const struct i915_sched_engine *sched_engine = engine->sched_engine; struct i915_request *rq, *last; unsigned long flags; unsigned int count;

@@ -3873,13 +3884,13 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, show_request(m, last, "\t\t", 0); }

• if (execlists->queue_priority_hint != INT_MIN)

• if (sched_engine->queue_priority_hint != INT_MIN) drm_printf(m, "\t\tQueue priority hint: %d\n",

• 	   READ_ONCE(execlists->queue_priority_hint));
  

• 	   READ_ONCE(sched_engine->queue_priority_hint));
  
  

  last = NULL; count = 0;

• for (rb = rb_first_cached(&execlists->queue); rb; rb = rb_next(rb)) {

• for (rb = rb_first_cached(&sched_engine->queue); rb; rb = rb_next(rb)) { struct i915_priolist *p = rb_entry(rb, typeof(*p), node);

  priolist_for_each_request(rq, p) {

diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index 32589c6625e1..b1fdba13e900 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -283,6 +283,7 @@ static void mock_engine_release(struct intel_engine_cs *engine)

GEM_BUG_ON(timer_pending(&mock->hw_delay));

• i915_sched_engine_put(engine->sched_engine); intel_breadcrumbs_free(engine->breadcrumbs);

  intel_context_unpin(engine->kernel_context);

@@ -345,6 +346,10 @@ int mock_engine_init(struct intel_engine_cs *engine) { struct intel_context *ce;

• engine->sched_engine = i915_sched_engine_create(ENGINE_MOCK);
• if (!engine->sched_engine)

• return -ENOMEM;
  

• intel_engine_init_active(engine, ENGINE_MOCK); intel_engine_init_execlists(engine); intel_engine_init__pm(engine);

@@ -352,7 +357,7 @@ int mock_engine_init(struct intel_engine_cs *engine)

engine->breadcrumbs = intel_breadcrumbs_create(NULL); if (!engine->breadcrumbs)

• return -ENOMEM;
  

• goto err_schedule;
  
  

  ce = create_kernel_context(engine); if (IS_ERR(ce))

@@ -366,6 +371,8 @@ int mock_engine_init(struct intel_engine_cs *engine)

err_breadcrumbs: intel_breadcrumbs_free(engine->breadcrumbs); +err_schedule:

• i915_sched_engine_put(engine->sched_engine); return -ENOMEM; }

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 7c8ff9792f7b..5c5f33f40055 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -182,6 +182,7 @@ static void schedule_out(struct i915_request *rq) static void __guc_dequeue(struct intel_engine_cs *engine) { struct intel_engine_execlists * const execlists = &engine->execlists;

• struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_request **first = execlists->inflight; struct i915_request ** const last_port = first + execlists->port_mask; struct i915_request *last = first[0];

@@ -204,7 +205,7 @@ static void __guc_dequeue(struct intel_engine_cs *engine) * event. */ port = first;

• while ((rb = rb_first_cached(&execlists->queue))) {

• while ((rb = rb_first_cached(&sched_engine->queue))) { struct i915_priolist *p = to_priolist(rb); struct i915_request *rq, *rn;

@@ -224,11 +225,11 @@ static void __guc_dequeue(struct intel_engine_cs *engine) last = rq; }

• rb_erase_cached(&p->node, &execlists->queue);
  

• rb_erase_cached(&p->node, &sched_engine->queue);
  

  i915_priolist_free(p); } done:

• execlists->queue_priority_hint =

• sched_engine->queue_priority_hint = rb ? to_priolist(rb)->priority : INT_MIN; if (submit) { *port = schedule_in(last, port - execlists->inflight);

@@ -338,7 +339,7 @@ static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled)

static void guc_reset_cancel(struct intel_engine_cs *engine) {

• struct intel_engine_execlists * const execlists = &engine->execlists;

• struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_request *rq, *rn; struct rb_node *rb; unsigned long flags;

@@ -368,7 +369,7 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) }

/* Flush the queued requests to the timeline list (for retiring). */

• while ((rb = rb_first_cached(&execlists->queue))) {

• while ((rb = rb_first_cached(&sched_engine->queue))) { struct i915_priolist *p = to_priolist(rb);

  priolist_for_each_request_consume(rq, rn, p) {

@@ -378,14 +379,14 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) i915_request_mark_complete(rq); }

• rb_erase_cached(&p->node, &execlists->queue);
  

• rb_erase_cached(&p->node, &sched_engine->queue);
  

  i915_priolist_free(p); }

  /* Remaining _unready_ requests will be nop'ed when submitted */

• execlists->queue_priority_hint = INT_MIN;
• execlists->queue = RB_ROOT_CACHED;

• sched_engine->queue_priority_hint = INT_MIN;

• sched_engine->queue = RB_ROOT_CACHED;

  spin_unlock_irqrestore(&engine->active.lock, flags); }

@@ -514,7 +515,7 @@ static void guc_submit_request(struct i915_request *rq)

queue_request(engine, rq, rq_prio(rq));

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->execlists.queue.rb_root));

• GEM_BUG_ON(RB_EMPTY_ROOT(&engine->sched_engine->queue.rb_root)); GEM_BUG_ON(list_empty(&rq->sched.link));

  tasklet_hi_schedule(&engine->execlists.tasklet);

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index efa638c3acc7..4953874a9ba6 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -40,7 +40,7 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) return rb_entry(rb, struct i915_priolist, node); }

-static void assert_priolists(struct intel_engine_execlists * const execlists) +static void assert_priolists(struct i915_sched_engine * const sched_engine) { struct rb_node *rb; long last_prio; @@ -48,11 +48,11 @@ static void assert_priolists(struct intel_engine_execlists * const execlists) if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) return;

• GEM_BUG_ON(rb_first_cached(&execlists->queue) !=

•    rb_first(&execlists->queue.rb_root));
  

• GEM_BUG_ON(rb_first_cached(&sched_engine->queue) !=

•    rb_first(&sched_engine->queue.rb_root));
  
  

  last_prio = INT_MAX;

• for (rb = rb_first_cached(&execlists->queue); rb; rb = rb_next(rb)) {

• for (rb = rb_first_cached(&sched_engine->queue); rb; rb = rb_next(rb)) { const struct i915_priolist *p = to_priolist(rb);

  GEM_BUG_ON(p->priority > last_prio);

@@ -63,21 +63,21 @@ static void assert_priolists(struct intel_engine_execlists * const execlists) struct list_head * i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio) {

• struct intel_engine_execlists * const execlists = &engine->execlists;

• struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_priolist *p; struct rb_node **parent, *rb; bool first = true;

  lockdep_assert_held(&engine->active.lock);

• assert_priolists(execlists);

• assert_priolists(sched_engine);

• if (unlikely(execlists->no_priolist))

• if (unlikely(sched_engine->no_priolist)) prio = I915_PRIORITY_NORMAL;

  find_priolist: /* most positive priority is scheduled first, equal priorities fifo */ rb = NULL;

• parent = &execlists->queue.rb_root.rb_node;

• parent = &sched_engine->queue.rb_root.rb_node; while (*parent) { rb = *parent; p = to_priolist(rb);

@@ -92,7 +92,7 @@ i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio) }

if (prio == I915_PRIORITY_NORMAL) {

• p = &execlists->default_priolist;
  

• p = &sched_engine->default_priolist;
  

  } else { p = kmem_cache_alloc(global.slab_priorities, GFP_ATOMIC); /* Convert an allocation failure to a priority bump */

@@ -107,7 +107,7 @@ i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio) * requests, so if userspace lied about their * dependencies that reordering may be visible. */

• 	execlists->no_priolist = true;
  

• 	sched_engine->no_priolist = true;
  goto find_priolist;
  

  } }

@@ -116,7 +116,7 @@ i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio) INIT_LIST_HEAD(&p->requests);

rb_link_node(&p->node, rb, parent);

• rb_insert_color_cached(&p->node, &execlists->queue, first);

• rb_insert_color_cached(&p->node, &sched_engine->queue, first);

  return &p->requests; }

@@ -184,7 +184,7 @@ static void kick_submission(struct intel_engine_cs *engine, * We only need to kick the tasklet once for the high priority * new context we add into the queue. */

• if (prio <= engine->execlists.queue_priority_hint)

• if (prio <= engine->sched_engine->queue_priority_hint) return;

  rcu_read_lock();

@@ -208,7 +208,7 @@ static void kick_submission(struct intel_engine_cs *engine, inflight->fence.context, inflight->fence.seqno, inflight->sched.attr.priority);

• engine->execlists.queue_priority_hint = prio;

• engine->sched_engine->queue_priority_hint = prio; if (need_preempt(prio, rq_prio(inflight))) tasklet_hi_schedule(&engine->execlists.tasklet);

@@ -489,6 +489,31 @@ void i915_request_show_with_schedule(struct drm_printer *m, rcu_read_unlock(); }

+void i915_sched_engine_free(struct kref *kref) +{

• struct i915_sched_engine *sched_engine =

• container_of(kref, typeof(*sched_engine), ref);
  

• kfree(sched_engine);

+}

+struct i915_sched_engine * +i915_sched_engine_create(unsigned int subclass)

subclass is unused here. My preference would be to move adding this param to the patch that actually makes use of it (i.e. patch 4), but if you want to keep it here because it makes the series split easier at least add a mention in the commit message of what's this for and why it is here and not in the patch that uses it.

Apart from these minor comments, the mechanical replacement looks fine. I'd prefer if we tried to avoid accessing sched_engine internals from the backends now, but that can come as a follow up (and I see some of it is already in patch 2).

With the comment addressed:

Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

Daniele

+{

• struct i915_sched_engine *sched_engine;
• sched_engine = kzalloc(sizeof(*sched_engine), GFP_KERNEL);
• if (!sched_engine)

• return NULL;
  

• kref_init(&sched_engine->ref);
• sched_engine->queue = RB_ROOT_CACHED;
• sched_engine->queue_priority_hint = INT_MIN;
• return sched_engine;

+}

• static void i915_global_scheduler_shrink(void) { kmem_cache_shrink(global.slab_dependencies);

diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h index 858a0938f47a..91a04e34cac5 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.h +++ b/drivers/gpu/drm/i915/i915_scheduler.h @@ -48,6 +48,24 @@ static inline void i915_priolist_free(struct i915_priolist *p) __i915_priolist_free(p); }

+struct i915_sched_engine * +i915_sched_engine_create(unsigned int subclass);

+void i915_sched_engine_free(struct kref *kref);

+static inline struct i915_sched_engine * +i915_sched_engine_get(struct i915_sched_engine *sched_engine) +{

• kref_get(&sched_engine->ref);
• return sched_engine;

+}

+static inline void +i915_sched_engine_put(struct i915_sched_engine *sched_engine) +{

• kref_put(&sched_engine->ref, i915_sched_engine_free);

+}

• 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 343ed44d5ed4..4a7c9f06b40b 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -91,4 +91,51 @@ struct i915_dependency { &(rq__)->sched.signalers_list, \ signal_link)

+/**

• • struct i915_sched_engine - scheduler engine
• • A schedule engine represents a submission queue with different priority
• • bands. It contains all the common state (relative to the backend) to queue,
• • track, and submit a request.
• • This object at the moment is quite i915 specific but will transition into a
• • container for the drm_gpu_scheduler plus a few other variables once the i915
• • is integrated with the DRM scheduler.
• */

+struct i915_sched_engine {

• /**

• * @ref: reference count of schedule engine object
  

• */
  

• struct kref ref;
• /**

• * @default_priolist: priority list for I915_PRIORITY_NORMAL
  

• */
  

• struct i915_priolist default_priolist;
• /**

• * @queue_priority_hint: Highest pending priority.
  

• *
  

• * When we add requests into the queue, or adjust the priority of
  

• * executing requests, we compute the maximum priority of those
  

• * pending requests. We can then use this value to determine if
  

• * we need to preempt the executing requests to service the queue.
  

• * However, since the we may have recorded the priority of an inflight
  

• * request we wanted to preempt but since completed, at the time of
  

• * dequeuing the priority hint may no longer may match the highest
  

• * available request priority.
  

• */
  

• int queue_priority_hint;
• /**

• * @queue: queue of requests, in priority lists
  

• */
  

• struct rb_root_cached queue;
• /**

• * @no_priolist: priority lists disabled
  

• */
  

• bool no_priolist;

+};

• #endif /* _I915_SCHEDULER_TYPES_H_ */

On 6/8/2021 12:17 PM, Matthew Brost wrote:

Move active request tracking and its lock to i915_sched_engine. This lock is also the submission lock so having it in the i915_sched_engine is the correct place.

v3: (Jason Ekstrand) Add kernel doc

Signed-off-by: Matthew Brost matthew.brost@intel.com

Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

Daniele

---

drivers/gpu/drm/i915/gt/intel_engine.h | 2 - drivers/gpu/drm/i915/gt/intel_engine_cs.c | 43 +++----- drivers/gpu/drm/i915/gt/intel_engine_types.h | 6 -- .../drm/i915/gt/intel_execlists_submission.c | 98 ++++++++++--------- .../gpu/drm/i915/gt/intel_ring_submission.c | 12 +-- drivers/gpu/drm/i915/gt/mock_engine.c | 7 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 4 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 20 ++-- drivers/gpu/drm/i915/i915_gpu_error.c | 4 +- drivers/gpu/drm/i915/i915_request.c | 32 +++--- drivers/gpu/drm/i915/i915_request.h | 2 +- drivers/gpu/drm/i915/i915_scheduler.c | 30 ++++-- drivers/gpu/drm/i915/i915_scheduler_types.h | 16 +++ 13 files changed, 141 insertions(+), 135 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 8d9184920c51..a8b2174b4395 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -257,8 +257,6 @@ intel_engine_find_active_request(struct intel_engine_cs *engine);

u32 intel_engine_context_size(struct intel_gt *gt, u8 class);

-void intel_engine_init_active(struct intel_engine_cs *engine,

• 	      unsigned int subclass);
  

  #define ENGINE_PHYSICAL 0 #define ENGINE_MOCK 1 #define ENGINE_VIRTUAL 2

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 75a773cd4e24..3c3f09a4acdd 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -721,7 +721,6 @@ static int engine_setup_common(struct intel_engine_cs *engine) if (err) goto err_cmd_parser;

• intel_engine_init_active(engine, ENGINE_PHYSICAL); intel_engine_init_execlists(engine); intel_engine_init__pm(engine); intel_engine_init_retire(engine);

@@ -780,11 +779,11 @@ static int measure_breadcrumb_dw(struct intel_context *ce) frame->rq.ring = &frame->ring;

mutex_lock(&ce->timeline->mutex);

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock);

  dw = engine->emit_fini_breadcrumb(&frame->rq, frame->cs) - frame->cs;

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); mutex_unlock(&ce->timeline->mutex);

  GEM_BUG_ON(dw & 1); /* RING_TAIL must be qword aligned */

@@ -793,28 +792,6 @@ static int measure_breadcrumb_dw(struct intel_context *ce) return dw; }

-void -intel_engine_init_active(struct intel_engine_cs *engine, unsigned int subclass) -{

• INIT_LIST_HEAD(&engine->active.requests);
• INIT_LIST_HEAD(&engine->active.hold);
• spin_lock_init(&engine->active.lock);
• lockdep_set_subclass(&engine->active.lock, subclass);
• /*

• * Due to an interesting quirk in lockdep's internal debug tracking,
  

• * after setting a subclass we must ensure the lock is used. Otherwise,
  

• * nr_unused_locks is incremented once too often.
  

• */
  

-#ifdef CONFIG_DEBUG_LOCK_ALLOC

• local_irq_disable();
• lock_map_acquire(&engine->active.lock.dep_map);
• lock_map_release(&engine->active.lock.dep_map);
• local_irq_enable();

-#endif -}

• static struct intel_context * create_pinned_context(struct intel_engine_cs *engine, unsigned int hwsp,

@@ -962,7 +939,7 @@ int intel_engines_init(struct intel_gt *gt) */ void intel_engine_cleanup_common(struct intel_engine_cs *engine) {

• GEM_BUG_ON(!list_empty(&engine->active.requests));

• GEM_BUG_ON(!list_empty(&engine->sched_engine->requests)); tasklet_kill(&engine->execlists.tasklet); /* flush the callback */

  i915_sched_engine_put(engine->sched_engine);

@@ -1355,7 +1332,7 @@ static struct intel_timeline *get_timeline(struct i915_request *rq) struct intel_timeline *tl;

/*

• * Even though we are holding the engine->active.lock here, there
  

• * Even though we are holding the engine->sched_engine->lock here, there
  

  • is no control over the submission queue per-se and we are
  • inspecting the active state at a random point in time, with an
  • unknown queue. Play safe and make sure the timeline remains valid.

@@ -1702,7 +1679,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,

drm_printf(m, "\tRequests:\n");

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags); rq = intel_engine_find_active_request(engine); if (rq) { struct intel_timeline *tl = get_timeline(rq);

@@ -1733,8 +1710,9 @@ void intel_engine_dump(struct intel_engine_cs *engine, hexdump(m, rq->context->lrc_reg_state, PAGE_SIZE); } }

• drm_printf(m, "\tOn hold?: %lu\n", list_count(&engine->active.hold));
• spin_unlock_irqrestore(&engine->active.lock, flags);

• drm_printf(m, "\tOn hold?: %lu\n",

•    list_count(&engine->sched_engine->hold));
  

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags);

  drm_printf(m, "\tMMIO base: 0x%08x\n", engine->mmio_base); wakeref = intel_runtime_pm_get_if_in_use(engine->uncore->rpm);

@@ -1814,7 +1792,7 @@ intel_engine_find_active_request(struct intel_engine_cs *engine) * At all other times, we must assume the GPU is still running, but * we only care about the snapshot of this moment. */

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&engine->sched_engine->lock);

  rcu_read_lock(); request = execlists_active(&engine->execlists);

@@ -1832,7 +1810,8 @@ intel_engine_find_active_request(struct intel_engine_cs *engine) if (active) return active;

• list_for_each_entry(request, &engine->active.requests, sched.link) {

• list_for_each_entry(request, &engine->sched_engine->requests,

• 	    sched.link) {
  

  if (__i915_request_is_complete(request)) continue;

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index e41a9c3f9269..5e0f39d202ef 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -304,12 +304,6 @@ struct intel_engine_cs {

struct intel_sseu sseu;

• struct {

• spinlock_t lock;
  

• struct list_head requests;
  

• struct list_head hold; /* ready requests, but on hold */
  

• } active;

• struct i915_sched_engine *sched_engine;

  /* keep a request in reserve for a [pm] barrier under oom */

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 47a43aafa39f..0591698573e8 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -325,7 +325,7 @@ static bool need_preempt(const struct intel_engine_cs *engine, * Check against the first request in ELSP[1], it will, thanks to the * power of PI, be the highest priority of that context. */

• if (!list_is_last(&rq->sched.link, &engine->active.requests) &&

• if (!list_is_last(&rq->sched.link, &engine->sched_engine->requests) && rq_prio(list_next_entry(rq, sched.link)) > last_prio) return true;

@@ -367,10 +367,10 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine) struct list_head *pl; int prio = I915_PRIORITY_INVALID;

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&engine->sched_engine->lock);

  list_for_each_entry_safe_reverse(rq, rn,

• 			 &engine->active.requests,
  

• 			 &engine->sched_engine->requests,
  		 sched.link) {
  

  if (__i915_request_is_complete(rq)) { list_del_init(&rq->sched.link);

@@ -534,13 +534,13 @@ resubmit_virtual_request(struct i915_request *rq, struct virtual_engine *ve) { struct intel_engine_cs *engine = rq->engine;

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock);

  clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); WRITE_ONCE(rq->engine, &ve->base); ve->base.submit_request(rq);

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); }

  static void kick_siblings(struct i915_request *rq, struct intel_context *ce)

@@ -579,7 +579,7 @@ static void __execlists_schedule_out(struct i915_request * const rq, unsigned int ccid;

/*

• * NB process_csb() is not under the engine->active.lock and hence
  

• * NB process_csb() is not under the engine->sched_engine->lock and hence
  

  • schedule_out can race with schedule_in meaning that we should
  • refrain from doing non-trivial work here.
  */

@@ -1133,7 +1133,8 @@ static bool needs_timeslice(const struct intel_engine_cs *engine, return false;

/* If ELSP[1] is occupied, always check to see if worth slicing */

• if (!list_is_last_rcu(&rq->sched.link, &engine->active.requests)) {

• if (!list_is_last_rcu(&rq->sched.link,

• 	      &engine->sched_engine->requests)) {
  

  ENGINE_TRACE(engine, "timeslice required for second inflight context\n"); return true; }

@@ -1266,7 +1267,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * and context switches) submission. */

• spin_lock(&engine->active.lock);

• spin_lock(&sched_engine->lock);

  /*

  • If the queue is higher priority than the last

@@ -1366,7 +1367,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * Even if ELSP[1] is occupied and not worthy * of timeslices, our queue might be. */

• 		spin_unlock(&engine->active.lock);
  

• 		spin_unlock(&sched_engine->lock);
  	return;
  }
  

  }

@@ -1376,7 +1377,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) while ((ve = first_virtual_engine(engine))) { struct i915_request *rq;

• spin_lock(&ve->base.active.lock);
  

• spin_lock(&ve->base.sched_engine->lock);
  
  

  rq = ve->request; if (unlikely(!virtual_matches(ve, rq, engine)))

@@ -1386,13 +1387,13 @@ static void execlists_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(rq->context != &ve->context);

if (unlikely(rq_prio(rq) < queue_prio(sched_engine))) {

• 	spin_unlock(&ve->base.active.lock);
  

• 	spin_unlock(&ve->base.sched_engine->lock);
  break;
  

  }

  if (last && !can_merge_rq(last, rq)) {

• 	spin_unlock(&ve->base.active.lock);
  

• 	spin_unlock(&engine->active.lock);
  

• 	spin_unlock(&ve->base.sched_engine->lock);
  

• 	spin_unlock(&engine->sched_engine->lock);
  return; /* leave this for another sibling */
  

  }

@@ -1438,7 +1439,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)

i915_request_put(rq);

unlock:

• spin_unlock(&ve->base.active.lock);
  

• spin_unlock(&ve->base.sched_engine->lock);
  
  

  /*

  • Hmm, we have a bunch of virtual engine requests,

@@ -1554,7 +1555,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) */ sched_engine->queue_priority_hint = queue_prio(sched_engine); i915_sched_engine_reset_on_empty(sched_engine);

• spin_unlock(&engine->active.lock);

• spin_unlock(&sched_engine->lock);

  /*

  • We can skip poking the HW if we ended up with exactly the same set

@@ -1981,7 +1982,8 @@ static void __execlists_hold(struct i915_request *rq) __i915_request_unsubmit(rq);

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->sched_engine->hold);
  

  i915_request_set_hold(rq); RQ_TRACE(rq, "on hold\n");

@@ -2018,7 +2020,7 @@ static bool execlists_hold(struct intel_engine_cs *engine, if (i915_request_on_hold(rq)) return false;

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock);

  if (__i915_request_is_complete(rq)) { /* too late! */ rq = NULL;

@@ -2034,10 +2036,10 @@ static bool execlists_hold(struct intel_engine_cs *engine, 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));

• GEM_BUG_ON(list_empty(&engine->sched_engine->hold));

  unlock:

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); return rq; }

@@ -2117,7 +2119,7 @@ static void __execlists_unhold(struct i915_request *rq) static void execlists_unhold(struct intel_engine_cs *engine, struct i915_request *rq) {

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock);

  /*

  • Move this request back to the priority queue, and all of its

@@ -2130,7 +2132,7 @@ static void execlists_unhold(struct intel_engine_cs *engine, tasklet_hi_schedule(&engine->execlists.tasklet); }

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); }

  struct execlists_capture {

@@ -2260,13 +2262,13 @@ static void execlists_capture(struct intel_engine_cs *engine) if (!cap) return;

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock); cap->rq = active_context(engine, active_ccid(engine)); if (cap->rq) { cap->rq = active_request(cap->rq->context->timeline, cap->rq); cap->rq = i915_request_get_rcu(cap->rq); }

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); if (!cap->rq) goto err_free;

@@ -2470,7 +2472,7 @@ static bool ancestor_on_hold(const struct intel_engine_cs *engine, const struct i915_request *rq) { GEM_BUG_ON(i915_request_on_hold(rq));

• return !list_empty(&engine->active.hold) && hold_request(rq);

• return !list_empty(&engine->sched_engine->hold) && hold_request(rq); }

  static void execlists_submit_request(struct i915_request *request)

@@ -2479,11 +2481,12 @@ static void execlists_submit_request(struct i915_request *request) unsigned long flags;

/* Will be called from irq-context when using foreign fences. */

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  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->sched_engine->hold);
  

  i915_request_set_hold(request); } else { queue_request(engine, request);

@@ -2495,7 +2498,7 @@ static void execlists_submit_request(struct i915_request *request) __execlists_kick(&engine->execlists); }

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static int

@@ -2959,9 +2962,9 @@ static void execlists_reset_rewind(struct intel_engine_cs *engine, bool stalled)

/* Push back any incomplete requests for replay after the reset. */ rcu_read_lock();

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags); __unwind_incomplete_requests(engine);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); rcu_read_unlock(); }

@@ -3001,10 +3004,10 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) execlists_reset_csb(engine, true);

rcu_read_lock();

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  /* Mark all executing requests as skipped. */

• list_for_each_entry(rq, &engine->active.requests, sched.link)

• list_for_each_entry(rq, &engine->sched_engine->requests, sched.link) i915_request_put(i915_request_mark_eio(rq)); intel_engine_signal_breadcrumbs(engine);

@@ -3024,7 +3027,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) }

/* On-hold requests will be flushed to timeline upon their release */

• list_for_each_entry(rq, &engine->active.hold, sched.link)

• list_for_each_entry(rq, &sched_engine->hold, sched.link) i915_request_put(i915_request_mark_eio(rq));

  /* Cancel all attached virtual engines */

@@ -3035,7 +3038,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) rb_erase_cached(rb, &execlists->virtual); RB_CLEAR_NODE(rb);

• spin_lock(&ve->base.active.lock);
  

• spin_lock(&ve->base.sched_engine->lock);
  

  rq = fetch_and_zero(&ve->request); if (rq) { if (i915_request_mark_eio(rq)) {

@@ -3047,7 +3050,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine)

	ve->base.sched_engine->queue_priority_hint = INT_MIN;
}

• spin_unlock(&ve->base.active.lock);
  

• spin_unlock(&ve->base.sched_engine->lock);
  

  }

  /* Remaining _unready_ requests will be nop'ed when submitted */

@@ -3058,7 +3061,7 @@ static void execlists_reset_cancel(struct intel_engine_cs *engine) GEM_BUG_ON(__tasklet_is_enabled(&execlists->tasklet)); execlists->tasklet.callback = nop_submission_tasklet;

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); rcu_read_unlock(); }

@@ -3304,7 +3307,7 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk) if (unlikely(ve->request)) { struct i915_request *old;

• spin_lock_irq(&ve->base.active.lock);
  

• spin_lock_irq(&ve->base.sched_engine->lock);
  
  

  old = fetch_and_zero(&ve->request); if (old) {

@@ -3313,7 +3316,7 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk) i915_request_put(old); }

• spin_unlock_irq(&ve->base.active.lock);
  

• spin_unlock_irq(&ve->base.sched_engine->lock);
  

  }

  /*

@@ -3333,13 +3336,13 @@ static void rcu_virtual_context_destroy(struct work_struct *wrk) if (RB_EMPTY_NODE(node)) continue;

• spin_lock_irq(&sibling->active.lock);
  

• spin_lock_irq(&sibling->sched_engine->lock);
  
  

  /* Detachment is lazily performed in the execlists tasklet */ if (!RB_EMPTY_NODE(node)) rb_erase_cached(node, &sibling->execlists.virtual);

• spin_unlock_irq(&sibling->active.lock);
  

• spin_unlock_irq(&sibling->sched_engine->lock);
  

  } GEM_BUG_ON(__tasklet_is_scheduled(&ve->base.execlists.tasklet)); GEM_BUG_ON(!list_empty(virtual_queue(ve)));

@@ -3509,7 +3512,7 @@ static void virtual_submission_tasklet(struct tasklet_struct *t) if (!READ_ONCE(ve->request)) break; /* already handled by a sibling's tasklet */

• spin_lock_irq(&sibling->active.lock);
  

• spin_lock_irq(&sibling->sched_engine->lock);
  
  

  if (unlikely(!(mask & sibling->mask))) { if (!RB_EMPTY_NODE(&node->rb)) {

@@ -3562,7 +3565,7 @@ static void virtual_submission_tasklet(struct tasklet_struct *t) tasklet_hi_schedule(&sibling->execlists.tasklet);

unlock_engine:

• spin_unlock_irq(&sibling->active.lock);
  

• spin_unlock_irq(&sibling->sched_engine->lock);
  
  

  if (intel_context_inflight(&ve->context)) break;

@@ -3580,7 +3583,7 @@ static void virtual_submit_request(struct i915_request *rq)

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

• spin_lock_irqsave(&ve->base.active.lock, flags);

• spin_lock_irqsave(&ve->base.sched_engine->lock, flags);

  /* By the time we resubmit a request, it may be completed */ if (__i915_request_is_complete(rq)) {

@@ -3603,7 +3606,7 @@ static void virtual_submit_request(struct i915_request *rq) tasklet_hi_schedule(&ve->base.execlists.tasklet);

unlock:

• spin_unlock_irqrestore(&ve->base.active.lock, flags);

• spin_unlock_irqrestore(&ve->base.sched_engine->lock, flags); }

  static struct ve_bond *

@@ -3687,7 +3690,6 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings,

snprintf(ve->base.name, sizeof(ve->base.name), "virtual");

• intel_engine_init_active(&ve->base, ENGINE_VIRTUAL); intel_engine_init_execlists(&ve->base);

  ve->base.sched_engine = i915_sched_engine_create(ENGINE_VIRTUAL);

@@ -3860,17 +3862,17 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, unsigned int max) { const struct intel_engine_execlists *execlists = &engine->execlists;

• const struct i915_sched_engine *sched_engine = engine->sched_engine;

• struct i915_sched_engine *sched_engine = engine->sched_engine; struct i915_request *rq, *last; unsigned long flags; unsigned int count; struct rb_node *rb;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&sched_engine->lock, flags);

  last = NULL; count = 0;

• list_for_each_entry(rq, &engine->active.requests, sched.link) {

• list_for_each_entry(rq, &sched_engine->requests, sched.link) { if (count++ < max - 1) show_request(m, rq, "\t\t", 0); else

@@ -3933,7 +3935,7 @@ void intel_execlists_show_requests(struct intel_engine_cs *engine, show_request(m, last, "\t\t", 0); }

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&sched_engine->lock, flags); }

  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 0c423f096e2b..5d42a12ef3d6 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -339,9 +339,9 @@ static void reset_rewind(struct intel_engine_cs *engine, bool stalled) u32 head;

rq = NULL;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags); rcu_read_lock();

• list_for_each_entry(pos, &engine->active.requests, sched.link) {

• list_for_each_entry(pos, &engine->sched_engine->requests, sched.link) { if (!__i915_request_is_complete(pos)) { rq = pos; break;

@@ -396,7 +396,7 @@ static void reset_rewind(struct intel_engine_cs *engine, bool stalled) } engine->legacy.ring->head = intel_ring_wrap(engine->legacy.ring, head);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void reset_finish(struct intel_engine_cs *engine)

@@ -408,16 +408,16 @@ static void reset_cancel(struct intel_engine_cs *engine) struct i915_request *request; unsigned long flags;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  /* Mark all submitted requests as skipped. */

• list_for_each_entry(request, &engine->active.requests, sched.link)

• list_for_each_entry(request, &engine->sched_engine->requests, sched.link) i915_request_put(i915_request_mark_eio(request)); intel_engine_signal_breadcrumbs(engine);

  /* Remaining _unready_ requests will be nop'ed when submitted */

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void i9xx_submit_request(struct i915_request *request)

diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index b1fdba13e900..a49fd3039f13 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -253,10 +253,10 @@ static void mock_reset_cancel(struct intel_engine_cs *engine)

del_timer_sync(&mock->hw_delay);

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  /* Mark all submitted requests as skipped. */

• list_for_each_entry(rq, &engine->active.requests, sched.link)

• list_for_each_entry(rq, &engine->sched_engine->requests, sched.link) i915_request_put(i915_request_mark_eio(rq)); intel_engine_signal_breadcrumbs(engine);

@@ -269,7 +269,7 @@ static void mock_reset_cancel(struct intel_engine_cs *engine) } INIT_LIST_HEAD(&mock->hw_queue);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void mock_reset_finish(struct intel_engine_cs *engine)

@@ -350,7 +350,6 @@ int mock_engine_init(struct intel_engine_cs *engine) if (!engine->sched_engine) return -ENOMEM;

• intel_engine_init_active(engine, ENGINE_MOCK); intel_engine_init_execlists(engine); intel_engine_init__pm(engine); intel_engine_init_retire(engine);

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index ea2203af0764..d213fc4df411 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -4599,9 +4599,9 @@ static int reset_virtual_engine(struct intel_gt *gt, GEM_BUG_ON(execlists_active(&engine->execlists) != rq);

/* Fake a preemption event; failed of course */

• spin_lock_irq(&engine->active.lock);

• spin_lock_irq(&engine->sched_engine->lock); __unwind_incomplete_requests(engine);

• spin_unlock_irq(&engine->active.lock);

• spin_unlock_irq(&engine->sched_engine->lock); GEM_BUG_ON(rq->engine != engine);

  /* Reset the engine while keeping our active request on hold */

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 9887a514a4d5..6b8f6544d91a 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -190,7 +190,7 @@ static void __guc_dequeue(struct intel_engine_cs *engine) bool submit = false; struct rb_node *rb;

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&sched_engine->lock);

  if (last) { if (*++first)

@@ -247,7 +247,7 @@ static void guc_submission_tasklet(struct tasklet_struct *t) struct i915_request **port, *rq; unsigned long flags;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  for (port = execlists->inflight; (rq = *port); port++) { if (!i915_request_completed(rq))

@@ -265,7 +265,7 @@ static void guc_submission_tasklet(struct tasklet_struct *t)

i915_sched_engine_reset_on_empty(engine->sched_engine);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void cs_irq_handler(struct intel_engine_cs *engine, u16 iir)

@@ -322,7 +322,7 @@ static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled) struct i915_request *rq; unsigned long flags;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  /* Push back any incomplete requests for replay after the reset. */ rq = execlists_unwind_incomplete_requests(execlists);

@@ -336,7 +336,7 @@ static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled) guc_reset_state(rq->context, engine, rq->head, stalled);

out_unlock:

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void guc_reset_cancel(struct intel_engine_cs *engine)

@@ -362,10 +362,10 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) * submission's irq state, we also wish to remind ourselves that * it is irq state.) */

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&sched_engine->lock, flags);

  /* Mark all executing requests as skipped. */

• list_for_each_entry(rq, &engine->active.requests, sched.link) {

• list_for_each_entry(rq, &sched_engine->requests, sched.link) { i915_request_set_error_once(rq, -EIO); i915_request_mark_complete(rq); }

@@ -390,7 +390,7 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) sched_engine->queue_priority_hint = INT_MIN; sched_engine->queue = RB_ROOT_CACHED;

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&sched_engine->lock, flags); }

  static void guc_reset_finish(struct intel_engine_cs *engine)

@@ -513,7 +513,7 @@ static void guc_submit_request(struct i915_request *rq) unsigned long flags;

/* Will be called from irq-context when using foreign fences. */

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  queue_request(engine, rq, rq_prio(rq));

@@ -522,7 +522,7 @@ static void guc_submit_request(struct i915_request *rq)

tasklet_hi_schedule(&engine->execlists.tasklet);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void sanitize_hwsp(struct intel_engine_cs *engine)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 35c97c39f125..cb182c6d265a 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1436,12 +1436,12 @@ capture_engine(struct intel_engine_cs *engine, if (!ee) return NULL;

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags); rq = intel_engine_find_active_request(engine); if (rq) capture = intel_engine_coredump_add_request(ee, rq, ATOMIC_MAYFAIL);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); if (!capture) { kfree(ee); return NULL;

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 1014c71cf7f5..bd58198735dc 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -272,11 +272,11 @@ i915_request_active_engine(struct i915_request *rq, * check that we have acquired the lock on the final engine. */ locked = READ_ONCE(rq->engine);

• spin_lock_irq(&locked->active.lock);

• spin_lock_irq(&locked->sched_engine->lock); while (unlikely(locked != (engine = READ_ONCE(rq->engine)))) {

• spin_unlock(&locked->active.lock);
  

• spin_unlock(&locked->sched_engine->lock);
  

  locked = engine;

• spin_lock(&locked->active.lock);
  

• spin_lock(&locked->sched_engine->lock);
  

  }

  if (i915_request_is_active(rq)) {

@@ -285,7 +285,7 @@ i915_request_active_engine(struct i915_request *rq, ret = true; }

• spin_unlock_irq(&locked->active.lock);

• spin_unlock_irq(&locked->sched_engine->lock);

  return ret; }

@@ -302,10 +302,10 @@ static void remove_from_engine(struct i915_request *rq) * check that the rq still belongs to the newly locked engine. */ locked = READ_ONCE(rq->engine);

• spin_lock_irq(&locked->active.lock);

• spin_lock_irq(&locked->sched_engine->lock); while (unlikely(locked != (engine = READ_ONCE(rq->engine)))) {

• spin_unlock(&locked->active.lock);
  

• spin_lock(&engine->active.lock);
  

• spin_unlock(&locked->sched_engine->lock);
  

• spin_lock(&engine->sched_engine->lock);
  

  locked = engine; } list_del_init(&rq->sched.link);

@@ -316,7 +316,7 @@ static void remove_from_engine(struct i915_request *rq) /* Prevent further __await_execution() registering a cb, then flush */ set_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags);

• spin_unlock_irq(&locked->active.lock);

• spin_unlock_irq(&locked->sched_engine->lock);

  __notify_execute_cb_imm(rq); }

@@ -637,7 +637,7 @@ bool __i915_request_submit(struct i915_request *request) RQ_TRACE(request, "\n");

GEM_BUG_ON(!irqs_disabled());

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&engine->sched_engine->lock);

  /*

  • With the advent of preempt-to-busy, we frequently encounter

@@ -649,7 +649,7 @@ bool __i915_request_submit(struct i915_request *request) * * We must remove the request from the caller's priority queue, * and the caller must only call us when the request is in their

• * priority queue, under the active.lock. This ensures that the
  

• * priority queue, under the sched_engine->lock. This ensures that the
  

  • request has *not* yet been retired and we can safely move
  • the request into the engine->active.list where it will be
  • dropped upon retiring. (Otherwise if resubmit a *retired*

@@ -694,7 +694,7 @@ bool __i915_request_submit(struct i915_request *request) result = true;

GEM_BUG_ON(test_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags));

• list_move_tail(&request->sched.link, &engine->active.requests);

• list_move_tail(&request->sched.link, &engine->sched_engine->requests); active: clear_bit(I915_FENCE_FLAG_PQUEUE, &request->fence.flags); set_bit(I915_FENCE_FLAG_ACTIVE, &request->fence.flags);

@@ -724,11 +724,11 @@ void i915_request_submit(struct i915_request *request) unsigned long flags;

/* Will be called from irq-context when using foreign fences. */

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  __i915_request_submit(request);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  void __i915_request_unsubmit(struct i915_request *request)

@@ -742,7 +742,7 @@ void __i915_request_unsubmit(struct i915_request *request) RQ_TRACE(request, "\n");

GEM_BUG_ON(!irqs_disabled());

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&engine->sched_engine->lock);

  /*

  • Before we remove this breadcrumb from the signal list, we have

@@ -775,11 +775,11 @@ void i915_request_unsubmit(struct i915_request *request) unsigned long flags;

/* Will be called from irq-context when using foreign fences. */

• spin_lock_irqsave(&engine->active.lock, flags);

• spin_lock_irqsave(&engine->sched_engine->lock, flags);

  __i915_request_unsubmit(request);

• spin_unlock_irqrestore(&engine->active.lock, flags);

• spin_unlock_irqrestore(&engine->sched_engine->lock, flags); }

  static void __cancel_request(struct i915_request *rq)

diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h index 270f6cd37650..239964bec1fa 100644 --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -613,7 +613,7 @@ i915_request_active_timeline(const struct i915_request *rq) * this submission. */ return rcu_dereference_protected(rq->timeline,

• 			 lockdep_is_held(&rq->engine->active.lock));
  

• 			 lockdep_is_held(&rq->engine->sched_engine->lock));
  

  }

  static inline u32

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index 4953874a9ba6..4bc6969f6a97 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -68,7 +68,7 @@ i915_sched_lookup_priolist(struct intel_engine_cs *engine, int prio) struct rb_node **parent, *rb; bool first = true;

• lockdep_assert_held(&engine->active.lock);

• lockdep_assert_held(&engine->sched_engine->lock); assert_priolists(sched_engine);

  if (unlikely(sched_engine->no_priolist))

@@ -147,9 +147,9 @@ sched_lock_engine(const struct i915_sched_node *node, * check that the rq still belongs to the newly locked engine. */ while (locked != (engine = READ_ONCE(rq->engine))) {

• spin_unlock(&locked->active.lock);
  

• spin_unlock(&locked->sched_engine->lock);
  

  memset(cache, 0, sizeof(*cache));

• spin_lock(&engine->active.lock);
  

• spin_lock(&engine->sched_engine->lock);
  

  locked = engine; }

@@ -296,7 +296,7 @@ static void __i915_schedule(struct i915_sched_node *node,

memset(&cache, 0, sizeof(cache)); engine = node_to_request(node)->engine;

• spin_lock(&engine->active.lock);

• spin_lock(&engine->sched_engine->lock);

  /* Fifo and depth-first replacement ensure our deps execute before us */ engine = sched_lock_engine(node, engine, &cache);

@@ -305,7 +305,7 @@ static void __i915_schedule(struct i915_sched_node *node,

node = dep->signaler;
engine = sched_lock_engine(node, engine, &cache);

• lockdep_assert_held(&engine->active.lock);
  

• lockdep_assert_held(&engine->sched_engine->lock);
  
  

  /* Recheck after acquiring the engine->timeline.lock */ if (prio <= node->attr.priority || node_signaled(node))

@@ -338,7 +338,7 @@ static void __i915_schedule(struct i915_sched_node *node, kick_submission(engine, node_to_request(node), prio); }

• spin_unlock(&engine->active.lock);

• spin_unlock(&engine->sched_engine->lock); }

  void i915_schedule(struct i915_request *rq, const struct i915_sched_attr *attr)

@@ -511,6 +511,24 @@ i915_sched_engine_create(unsigned int subclass) sched_engine->queue = RB_ROOT_CACHED; sched_engine->queue_priority_hint = INT_MIN;

• INIT_LIST_HEAD(&sched_engine->requests);
• INIT_LIST_HEAD(&sched_engine->hold);
• spin_lock_init(&sched_engine->lock);
• lockdep_set_subclass(&sched_engine->lock, subclass);
• /*

• * Due to an interesting quirk in lockdep's internal debug tracking,
  

• * after setting a subclass we must ensure the lock is used. Otherwise,
  

• * nr_unused_locks is incremented once too often.
  

• */
  

+#ifdef CONFIG_DEBUG_LOCK_ALLOC

• local_irq_disable();
• lock_map_acquire(&sched_engine->lock.dep_map);
• lock_map_release(&sched_engine->lock.dep_map);
• local_irq_enable();

+#endif

• return sched_engine; }

diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 4a7c9f06b40b..74e3bb2b2a65 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -108,6 +108,22 @@ struct i915_sched_engine { */ struct kref ref;

• /**

• * @lock: protects requests in priority lists, requests, hold and
  

• * tasklet while running
  

• */
  

• spinlock_t lock;
• /**

• * @requests: list of requests inflight on this schedule engine
  

• */
  

• struct list_head requests;
• /**

• * @hold: list of ready requests, but on hold
  

• */
  

• struct list_head hold;
• /**
  • @default_priolist: priority list for I915_PRIORITY_NORMAL
  */

On 6/8/2021 12:17 PM, Matthew Brost wrote:

The schedule function should be in the schedule object.

v3: (Jason Ekstrand) Add kernel doc

Signed-off-by: Matthew Brost matthew.brost@intel.com

Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

Daniele

---

drivers/gpu/drm/i915/gem/i915_gem_wait.c | 4 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 --- drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++-- drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 -------- drivers/gpu/drm/i915/gt/intel_engine_user.c | 2 +- .../gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++-- drivers/gpu/drm/i915/gt/selftest_execlists.c | 16 ++++++++-------- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++-- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +- drivers/gpu/drm/i915/i915_request.c | 10 +++++----- drivers/gpu/drm/i915/i915_scheduler_types.h | 10 ++++++++++ 11 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c index 1e97520c62b2..1070d3afdce7 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c @@ -104,8 +104,8 @@ static void fence_set_priority(struct dma_fence *fence, engine = rq->engine;

rcu_read_lock(); /* RCU serialisation for set-wedged protection */

• if (engine->schedule)

• engine->schedule(rq, attr);
  

• if (engine->sched_engine->schedule)

• engine->sched_engine->schedule(rq, attr);
  

  rcu_read_unlock(); }

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 3c3f09a4acdd..7ff2640aa74a 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -328,9 +328,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id) if (engine->context_size) DRIVER_CAPS(i915)->has_logical_contexts = true;

• /* Nothing to do here, execute in order of dependencies */
• engine->schedule = NULL;
• ewma__engine_latency_init(&engine->latency); seqcount_init(&engine->stats.lock);

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c index b99ac41695f3..b6a305e6a974 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c @@ -121,7 +121,7 @@ static void heartbeat(struct work_struct *wrk) * but all other contexts, including the kernel * context are stuck waiting for the signal. */

• } else if (engine->schedule &&
  

• } else if (engine->sched_engine->schedule &&
     rq->sched.attr.priority < I915_PRIORITY_BARRIER) {
  /*
   * Gradually raise the priority of the heartbeat to
  

@@ -136,7 +136,7 @@ static void heartbeat(struct work_struct *wrk) attr.priority = I915_PRIORITY_BARRIER;

	local_bh_disable();

• 	engine->schedule(rq, &attr);
  

• 	engine->sched_engine->schedule(rq, &attr);
  local_bh_enable();
  

  } else { if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 5e0f39d202ef..0bb65c57d274 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -428,14 +428,6 @@ struct intel_engine_cs { void (*bond_execute)(struct i915_request *rq, struct dma_fence *signal);

• /*

• * Call when the priority on a request has changed and it and its
  

• * dependencies may need rescheduling. Note the request itself may
  

• * not be ready to run!
  

• */
  

• void (*schedule)(struct i915_request *request,

• 		    const struct i915_sched_attr *attr);
  

• void (*release)(struct intel_engine_cs *engine);

  struct intel_engine_execlists execlists;

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 3cca7ea2d6ea..84142127ebd8 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -108,7 +108,7 @@ static void set_scheduler_caps(struct drm_i915_private *i915) for_each_uabi_engine(engine, i915) { /* all engines must agree! */ int i;

• if (engine->schedule)
  

• if (engine->sched_engine->schedule)
  enabled |= (I915_SCHEDULER_CAP_ENABLED |
  	    I915_SCHEDULER_CAP_PRIORITY);
  

  else

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 0591698573e8..8a3d4014fd2c 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -3119,7 +3119,7 @@ static bool can_preempt(struct intel_engine_cs *engine) static void execlists_set_default_submission(struct intel_engine_cs *engine) { engine->submit_request = execlists_submit_request;

• engine->schedule = i915_schedule;

• engine->sched_engine->schedule = i915_schedule; engine->execlists.tasklet.callback = execlists_submission_tasklet; }

@@ -3701,7 +3701,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, ve->base.cops = &virtual_context_ops; ve->base.request_alloc = execlists_request_alloc;

• ve->base.schedule = i915_schedule;

• ve->base.sched_engine->schedule = i915_schedule; ve->base.submit_request = virtual_submit_request; ve->base.bond_execute = virtual_bond_execute;

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index d213fc4df411..6c0583e7a4c8 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -273,7 +273,7 @@ static int live_unlite_restore(struct intel_gt *gt, int prio) };

	/* Alternatively preempt the spinner with ce[1] */

• 	engine->schedule(rq[1], &attr);
  

• 	engine->sched_engine->schedule(rq[1], &attr);
  

  }

  /* And switch back to ce[0] for good measure */

@@ -900,7 +900,7 @@ release_queue(struct intel_engine_cs *engine, i915_request_add(rq);

local_bh_disable();

• engine->schedule(rq, &attr);

• engine->sched_engine->schedule(rq, &attr); local_bh_enable(); /* kick tasklet */

  i915_request_put(rq);

@@ -1325,7 +1325,7 @@ static int live_timeslice_queue(void *arg) err = PTR_ERR(rq); goto err_heartbeat; }

• engine->schedule(rq, &attr);
  

• engine->sched_engine->schedule(rq, &attr);
  

  err = wait_for_submit(engine, rq, HZ / 2); if (err) { pr_err("%s: Timed out trying to submit semaphores\n",

@@ -1867,7 +1867,7 @@ static int live_late_preempt(void *arg) }

attr.priority = I915_PRIORITY_MAX;

• engine->schedule(rq, &attr);
  

• engine->sched_engine->schedule(rq, &attr);
  
  

  if (!igt_wait_for_spinner(&spin_hi, rq)) { pr_err("High priority context failed to preempt the low priority context\n");

@@ -2480,7 +2480,7 @@ static int live_suppress_self_preempt(void *arg) i915_request_add(rq_b);

	GEM_BUG_ON(i915_request_completed(rq_a));

• 	engine->schedule(rq_a, &attr);
  

• 	engine->sched_engine->schedule(rq_a, &attr);
  igt_spinner_end(&a.spin);
  
  if (!igt_wait_for_spinner(&b.spin, rq_b)) {
  

@@ -2612,7 +2612,7 @@ static int live_chain_preempt(void *arg)

	i915_request_get(rq);
	i915_request_add(rq);

• 	engine->schedule(rq, &attr);
  

• 	engine->sched_engine->schedule(rq, &attr);
  
  igt_spinner_end(&hi.spin);
  if (i915_request_wait(rq, 0, HZ / 5) < 0) {
  

@@ -2971,7 +2971,7 @@ static int live_preempt_gang(void *arg) break;

	/* Submit each spinner at increasing priority */

• 	engine->schedule(rq, &attr);
  

• 	engine->sched_engine->schedule(rq, &attr);
  

  } while (prio <= I915_PRIORITY_MAX && !__igt_timeout(end_time, NULL)); pr_debug("%s: Preempt chain of %d requests\n",

@@ -3219,7 +3219,7 @@ static int preempt_user(struct intel_engine_cs *engine, i915_request_get(rq); i915_request_add(rq);

• engine->schedule(rq, &attr);

• engine->sched_engine->schedule(rq, &attr);

  if (i915_request_wait(rq, 0, HZ / 2) < 0) err = -ETIME;

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c index 853246fad05f..cec4b9977c9b 100644 --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c @@ -858,12 +858,12 @@ static int active_engine(void *data) rq[idx] = i915_request_get(new); i915_request_add(new);

• if (engine->schedule && arg->flags & TEST_PRIORITY) {
  

• if (engine->sched_engine->schedule && arg->flags & TEST_PRIORITY) {
  struct i915_sched_attr attr = {
  	.priority =
  		i915_prandom_u32_max_state(512, &prng),
  };
  

• 	engine->schedule(rq[idx], &attr);
  

• 	engine->sched_engine->schedule(rq[idx], &attr);
  

  }

  err = active_request_put(old);

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 6b8f6544d91a..60121809e6e2 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -615,7 +615,7 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) engine->cops = &guc_context_ops; engine->request_alloc = guc_request_alloc;

• engine->schedule = i915_schedule;

• engine->sched_engine->schedule = i915_schedule;

  engine->reset.prepare = guc_reset_prepare; engine->reset.rewind = guc_reset_rewind;

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index bd58198735dc..c5989c0b83d3 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1343,7 +1343,7 @@ __i915_request_await_execution(struct i915_request *to, }

/* Couple the dependency tree for PI on this exposed to->fence */

• if (to->engine->schedule) {

• if (to->engine->sched_engine->schedule) { err = i915_sched_node_add_dependency(&to->sched, &from->sched, I915_DEPENDENCY_WEAK);

@@ -1484,7 +1484,7 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from) return 0; }

• if (to->engine->schedule) {

• if (to->engine->sched_engine->schedule) { ret = i915_sched_node_add_dependency(&to->sched, &from->sched, I915_DEPENDENCY_EXTERNAL);

@@ -1671,7 +1671,7 @@ __i915_request_add_to_timeline(struct i915_request *rq) __i915_sw_fence_await_dma_fence(&rq->submit, &prev->fence, &rq->dmaq);

• if (rq->engine->schedule)
  

• if (rq->engine->sched_engine->schedule)
  __i915_sched_node_add_dependency(&rq->sched,
  				 &prev->sched,
  				 &rq->dep,
  

@@ -1743,8 +1743,8 @@ void __i915_request_queue(struct i915_request *rq, * decide whether to preempt the entire chain so that it is ready to * run at the earliest possible convenience. */

• if (attr && rq->engine->schedule)

• rq->engine->schedule(rq, attr);
  

• if (attr && rq->engine->sched_engine->schedule)

• rq->engine->sched_engine->schedule(rq, attr);
  
  

  local_bh_disable(); __i915_request_queue_bh(rq);

diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 74e3bb2b2a65..0c1e417b0164 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -152,6 +152,16 @@ struct i915_sched_engine { * @no_priolist: priority lists disabled */ bool no_priolist;

• /**

• * @schedule: adjust priority of request
  

• *
  

• * Call when the priority on a request has changed and it and its
  

• * dependencies may need rescheduling. Note the request itself may
  

• * not be ready to run!
  

• */
  

• void (*schedule)(struct i915_request *request,

• 	    const struct i915_sched_attr *attr);
  

  };

  #endif /* _I915_SCHEDULER_TYPES_H_ */

On 6/8/2021 12:17 PM, Matthew Brost wrote:

Rather than touching execlist specific structures in the generic scheduling code, add a callback function in the backend.

I think this could do with a better wording to explain the reasoning more, something like: "Not all back-ends require a kick after a scheduling update, so make the kick a call-back function that the back-end can opt-in to. Also move the current kick function from the scheduler to the execlists file as it is specific to that back-end". With something like that:

Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

Daniele

Signed-off-by: Matthew Brost matthew.brost@intel.com

.../drm/i915/gt/intel_execlists_submission.c | 52 ++++++++++++++++ drivers/gpu/drm/i915/i915_scheduler.c | 62 +------------------ drivers/gpu/drm/i915/i915_scheduler_types.h | 6 ++ 3 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 8a3d4014fd2c..9487d9e0be62 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -3116,10 +3116,61 @@ static bool can_preempt(struct intel_engine_cs *engine) return engine->class != RENDER_CLASS; }

+static void kick_execlists(const struct i915_request *rq, int prio) +{

• struct intel_engine_cs *engine = rq->engine;
• struct i915_sched_engine *sched_engine = engine->sched_engine;
• const struct i915_request *inflight;
• /*

• * We only need to kick the tasklet once for the high priority
  

• * new context we add into the queue.
  

• */
  

• if (prio <= sched_engine->queue_priority_hint)

• return;
  

• rcu_read_lock();
• /* Nothing currently active? We're overdue for a submission! */
• inflight = execlists_active(&engine->execlists);
• if (!inflight)

• goto unlock;
  

• /*

• * If we are already the currently executing context, don't
  

• * bother evaluating if we should preempt ourselves.
  

• */
  

• if (inflight->context == rq->context)

• goto unlock;
  

• ENGINE_TRACE(engine,

•      "bumping queue-priority-hint:%d for rq:%llx:%lld, inflight:%llx:%lld prio %d\n",
  

•      prio,
  

•      rq->fence.context, rq->fence.seqno,
  

•      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
  

• * tasks under the impression that latency for low priority
  

• * tasks does not matter (as much as background throughput),
  

• * so kiss.
  

• */
  

• if (prio >= max(I915_PRIORITY_NORMAL, rq_prio(inflight)))

• tasklet_hi_schedule(&engine->execlists.tasklet);
  

+unlock:

• rcu_read_unlock();

+}

• 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; }

@@ -3702,6 +3753,7 @@ intel_execlists_create_virtual(struct intel_engine_cs **siblings, ve->base.request_alloc = execlists_request_alloc;

ve->base.sched_engine->schedule = i915_schedule;

• ve->base.sched_engine->kick_backend = kick_execlists; ve->base.submit_request = virtual_submit_request; ve->base.bond_execute = virtual_bond_execute;

diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c index 4bc6969f6a97..035b88f2e4aa 100644 --- a/drivers/gpu/drm/i915/i915_scheduler.c +++ b/drivers/gpu/drm/i915/i915_scheduler.c @@ -157,65 +157,6 @@ sched_lock_engine(const struct i915_sched_node *node, return locked; }

-static inline int rq_prio(const struct i915_request *rq) -{

• return rq->sched.attr.priority;

-}

-static inline bool need_preempt(int prio, int active) -{

• /*

• * Allow preemption of low -> normal -> high, but we do
  

• * not allow low priority tasks to preempt other low priority
  

• * tasks under the impression that latency for low priority
  

• * tasks does not matter (as much as background throughput),
  

• * so kiss.
  

• */
  

• return prio >= max(I915_PRIORITY_NORMAL, active);

-}

-static void kick_submission(struct intel_engine_cs *engine,

• 	    const struct i915_request *rq,
  

• 	    int prio)
  

-{

• const struct i915_request *inflight;
• /*

• * We only need to kick the tasklet once for the high priority
  

• * new context we add into the queue.
  

• */
  

• if (prio <= engine->sched_engine->queue_priority_hint)

• return;
  

• rcu_read_lock();
• /* Nothing currently active? We're overdue for a submission! */
• inflight = execlists_active(&engine->execlists);
• if (!inflight)

• goto unlock;
  

• /*

• * If we are already the currently executing context, don't
  

• * bother evaluating if we should preempt ourselves.
  

• */
  

• if (inflight->context == rq->context)

• goto unlock;
  

• ENGINE_TRACE(engine,

•      "bumping queue-priority-hint:%d for rq:%llx:%lld, inflight:%llx:%lld prio %d\n",
  

•      prio,
  

•      rq->fence.context, rq->fence.seqno,
  

•      inflight->fence.context, inflight->fence.seqno,
  

•      inflight->sched.attr.priority);
  

• engine->sched_engine->queue_priority_hint = prio;
• if (need_preempt(prio, rq_prio(inflight)))

• tasklet_hi_schedule(&engine->execlists.tasklet);
  

-unlock:

• rcu_read_unlock();

-}

• static void __i915_schedule(struct i915_sched_node *node, const struct i915_sched_attr *attr) {

@@ -335,7 +276,8 @@ static void __i915_schedule(struct i915_sched_node *node, }

/* Defer (tasklet) submission until after all of our updates. */

• kick_submission(engine, node_to_request(node), prio);
  

• if (engine->sched_engine->kick_backend)
  

• 	engine->sched_engine->kick_backend(node_to_request(node), prio);
  

  }

  spin_unlock(&engine->sched_engine->lock);

diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h index 0c1e417b0164..8bd07d0c27e1 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -153,6 +153,12 @@ struct i915_sched_engine { */ bool no_priolist;

• /**

• * @kick_backend: kick backend after a request's priority has changed
  

• */
  

• void (*kick_backend)(const struct i915_request *rq,

• 		int prio);
  

• /**
  • @schedule: adjust priority of request

On Mon, Jun 14, 2021 at 06:05:19PM -0700, Daniele Ceraolo Spurio wrote:

On 6/8/2021 12:17 PM, Matthew Brost wrote:

...

The submission tasklet operates on i915_sched_engine, thus it is the correct place for it.

v3: (Jason Ekstrand) Change sched_engine->engine to a void* private data pointer Add kernel doc

Signed-off-by: Matthew Brost matthew.brost@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 | 10 +++ 13 files changed, 101 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 7ff2640aa74a..67939ee0d68f 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -713,6 +713,7 @@ static int engine_setup_common(struct intel_engine_cs *engine) err = -ENOMEM; goto err_sched_engine; }

• engine->sched_engine->private_data = engine;

Given that the private_data is back-end specific, IMO it should be set in one of the back-end functions, like set_default_submission().

That likely works.

...

err = intel_engine_init_cmd_parser(engine); if (err) @@ -937,7 +938,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);

@@ -1223,7 +1223,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;

@@ -1484,8 +1484,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));
  

@@ -1509,7 +1509,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];

@@ -1540,7 +1540,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 (GRAPHICS_VER(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 0bb65c57d274..5b91068ab277 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 ffad4d98cec0..79b40a96937c 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->private_data; 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->private_data; /* 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;

why is this being removed?

This looks like a mistake. Will fix.

...

/* * 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->private_data;
  

• 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->private_data = &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..68970398e4ef 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->private_data = 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 6c0583e7a4c8..74ada241e4f0 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 cec4b9977c9b..6a0b04bdac58 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 3119016d9910..b0977a3b699b 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 cb13cc586c67..e9c237b18692 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->private_data; 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));
  

  } /*

@@ -520,7 +517,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); }

@@ -600,7 +597,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);

@@ -679,7 +676,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine) */ GEM_BUG_ON(GRAPHICS_VER(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 fa8863df9513..3a58a9130309 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 8bd07d0c27e1..b26335e9e3fa 100644 --- a/drivers/gpu/drm/i915/i915_scheduler_types.h +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h @@ -124,6 +124,11 @@ struct i915_sched_engine { */ struct list_head hold;

• /**

• * @tasklet: softirq tasklet for submission
  

• */
  

• struct tasklet_struct tasklet;
• /**
  • @default_priolist: priority list for I915_PRIORITY_NORMAL
  */

@@ -153,6 +158,11 @@ struct i915_sched_engine { */ bool no_priolist;

• /**

• * @private_data: private data
  

Here I'd highlight who owns the private data, something like:

• @private_data: private data of the submission backend

Will do.

Matt

Daniele

...

• */
  

• void *private_data;
• /**
  • @kick_backend: kick backend after a request's priority has changed
  */