On 10/13/2021 13:42, Matthew Brost wrote:

Move guc_id allocation under submission state sub-struct as a future patch will reuse the spin lock as a global submission state lock. Moving this into sub-struct makes ownership of fields / lock clear.

v2: (Docs)

• Add comment for submission_state sub-structure

v3: (John Harrison)

• Fixup a few comments

Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: John Harrison John.C.Harrison@Intel.com

---

drivers/gpu/drm/i915/gt/intel_context_types.h | 6 +-- drivers/gpu/drm/i915/gt/uc/intel_guc.h | 28 +++++++---- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 48 ++++++++++--------- 3 files changed, 47 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 12252c411159..e7e3984aab78 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -197,18 +197,18 @@ struct intel_context { struct { /** * @id: handle which is used to uniquely identify this context

•  * with the GuC, protected by guc->contexts_lock
  

•  * with the GuC, protected by guc->submission_state.lock
  

  */ u16 id; /**
  • @ref: the number of references to the guc_id, when
  • transitioning in and out of zero protected by

•  * guc->contexts_lock
  

•  * guc->submission_state.lock
  

  */ atomic_t ref; /**
  • @link: in guc->guc_id_list when the guc_id has no refs but is

•  * still valid, protected by guc->contexts_lock
  

•  * still valid, protected by guc->submission_state.lock
  

  */ struct list_head link; } guc_id;

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 5dd174babf7a..82e248c2290c 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -71,16 +71,26 @@ struct intel_guc { } interrupts;

/**

• * @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id.id, and
  

• * ce->guc_id.ref when transitioning in and out of zero
  

• * @submission_state: sub-structure for submission state protected by
  

• * single lock
  

  */

• spinlock_t contexts_lock;
• /** @guc_ids: used to allocate unique ce->guc_id.id values */
• struct ida guc_ids;
• /**

• * @guc_id_list: list of intel_context with valid guc_ids but no refs
  

• */
  

• struct list_head guc_id_list;

• struct {

• /**
  

•  * @lock: protects everything in submission_state,
  

•  * ce->guc_id.id, and ce->guc_id.ref when transitioning in and
  

•  * out of zero
  

•  */
  

• spinlock_t lock;
  

• /**
  

•  * @guc_ids: used to allocate new guc_ids
  

•  */
  

• struct ida guc_ids;
  

• /**
  

•  * @guc_id_list: list of intel_context with valid guc_ids but no
  

•  * refs
  

•  */
  

• struct list_head guc_id_list;
  

• } submission_state;

  /**

  • @submission_supported: tracks whether we support GuC submission on

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 ba0de35f6323..b2646b088c7f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -68,14 +68,14 @@

• fence is used to stall all requests associated with this guc_id until the
• corresponding G2H returns indicating the guc_id has been deregistered.

• • guc_ids:

• • submission_state.guc_ids:
  • Unique number associated with private GuC context data passed in during
  • context registration / submission / deregistration. 64k available. Simple ida
  • is used for allocation.
  • Stealing guc_ids:
  • If no guc_ids are available they can be stolen from another context at

• • request creation time if that context is unpinned. If a guc_id can't be found

• • request creation time if that context is unpinned. If a guc_id an't be found

Oops?

John.

• we punt this problem to the user as we believe this is near impossible to hit
• during normal use cases.

@@ -89,7 +89,7 @@

• sched_engine can be submitting at a time. Currently only one sched_engine is
• used for all of GuC submission but that could change in the future.

• • guc->contexts_lock

• • guc->submission_state.lock
  • Protects guc_id allocation for the given GuC, i.e. only one context can be
  • doing guc_id allocation operations at a time for each GuC in the system.

@@ -103,7 +103,7 @@

• Lock ordering rules:
• sched_engine->lock -> ce->guc_state.lock

• • guc->contexts_lock -> ce->guc_state.lock

• • guc->submission_state.lock -> ce->guc_state.lock
  • Reset races:
  • When a full GT reset is triggered it is assumed that some G2H responses to

@@ -1148,9 +1148,9 @@ int intel_guc_submission_init(struct intel_guc *guc)

xa_init_flags(&guc->context_lookup, XA_FLAGS_LOCK_IRQ);

• spin_lock_init(&guc->contexts_lock);
• INIT_LIST_HEAD(&guc->guc_id_list);
• ida_init(&guc->guc_ids);

• spin_lock_init(&guc->submission_state.lock);

• INIT_LIST_HEAD(&guc->submission_state.guc_id_list);

• ida_init(&guc->submission_state.guc_ids);

  return 0; }

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

static int new_guc_id(struct intel_guc *guc) {

• return ida_simple_get(&guc->guc_ids, 0,

• return ida_simple_get(&guc->submission_state.guc_ids, 0, GUC_MAX_LRC_DESCRIPTORS, GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN); }

@@ -1223,7 +1223,8 @@ static int new_guc_id(struct intel_guc *guc) static void __release_guc_id(struct intel_guc *guc, struct intel_context *ce) { if (!context_guc_id_invalid(ce)) {

• ida_simple_remove(&guc->guc_ids, ce->guc_id.id);
  

• ida_simple_remove(&guc->submission_state.guc_ids,
  

• 		  ce->guc_id.id);
  

  reset_lrc_desc(guc, ce->guc_id.id); set_context_guc_id_invalid(ce); }

@@ -1235,9 +1236,9 @@ static void release_guc_id(struct intel_guc *guc, struct intel_context *ce) { unsigned long flags;

• spin_lock_irqsave(&guc->contexts_lock, flags);

• spin_lock_irqsave(&guc->submission_state.lock, flags); __release_guc_id(guc, ce);

• spin_unlock_irqrestore(&guc->contexts_lock, flags);

• spin_unlock_irqrestore(&guc->submission_state.lock, flags); }

  static int steal_guc_id(struct intel_guc *guc)

@@ -1245,10 +1246,10 @@ static int steal_guc_id(struct intel_guc *guc) struct intel_context *ce; int guc_id;

• lockdep_assert_held(&guc->contexts_lock);

• lockdep_assert_held(&guc->submission_state.lock);

• if (!list_empty(&guc->guc_id_list)) {

• ce = list_first_entry(&guc->guc_id_list,
  

• if (!list_empty(&guc->submission_state.guc_id_list)) {

• ce = list_first_entry(&guc->submission_state.guc_id_list,
  	      struct intel_context,
  	      guc_id.link);
  
  

@@ -1273,7 +1274,7 @@ static int assign_guc_id(struct intel_guc *guc, u16 *out) { int ret;

• lockdep_assert_held(&guc->contexts_lock);

• lockdep_assert_held(&guc->submission_state.lock);

  ret = new_guc_id(guc); if (unlikely(ret < 0)) {

@@ -1295,7 +1296,7 @@ static int pin_guc_id(struct intel_guc *guc, struct intel_context *ce) GEM_BUG_ON(atomic_read(&ce->guc_id.ref));

try_again:

• spin_lock_irqsave(&guc->contexts_lock, flags);

• spin_lock_irqsave(&guc->submission_state.lock, flags);

  might_lock(&ce->guc_state.lock);

@@ -1310,7 +1311,7 @@ static int pin_guc_id(struct intel_guc *guc, struct intel_context *ce) atomic_inc(&ce->guc_id.ref);

out_unlock:

• spin_unlock_irqrestore(&guc->contexts_lock, flags);

• spin_unlock_irqrestore(&guc->submission_state.lock, flags);

  /*

  • -EAGAIN indicates no guc_id are available, let's retire any

@@ -1346,11 +1347,12 @@ static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce) if (unlikely(context_guc_id_invalid(ce))) return;

• spin_lock_irqsave(&guc->contexts_lock, flags);

• spin_lock_irqsave(&guc->submission_state.lock, flags); if (!context_guc_id_invalid(ce) && list_empty(&ce->guc_id.link) && !atomic_read(&ce->guc_id.ref))

• list_add_tail(&ce->guc_id.link, &guc->guc_id_list);
  

• spin_unlock_irqrestore(&guc->contexts_lock, flags);

• list_add_tail(&ce->guc_id.link,
  

• 	      &guc->submission_state.guc_id_list);
  

• spin_unlock_irqrestore(&guc->submission_state.lock, flags); }

  static int __guc_action_register_context(struct intel_guc *guc,

@@ -1921,16 +1923,16 @@ static void guc_context_destroy(struct kref *kref) * returns indicating this context has been deregistered the guc_id is * returned to the pool of available guc_id. */

• spin_lock_irqsave(&guc->contexts_lock, flags);

• spin_lock_irqsave(&guc->submission_state.lock, flags); if (context_guc_id_invalid(ce)) {

• spin_unlock_irqrestore(&guc->contexts_lock, flags);
  

• spin_unlock_irqrestore(&guc->submission_state.lock, flags);
  

  __guc_context_destroy(ce); return; }

  if (!list_empty(&ce->guc_id.link)) list_del_init(&ce->guc_id.link);

• spin_unlock_irqrestore(&guc->contexts_lock, flags);

• spin_unlock_irqrestore(&guc->submission_state.lock, flags);

  /* Seal race with Reset */ spin_lock_irqsave(&ce->guc_state.lock, flags);

On 10/13/2021 13:42, Matthew Brost wrote:

Taking a PM reference to prevent intel_gt_wait_for_idle from short circuiting while a deregister context H2G is in flight. To do this must issue the deregister H2G from a worker as context can be destroyed from an atomic context and taking GT PM ref blows up. Previously we took a runtime PM from this atomic context which worked but will stop working once runtime pm autosuspend in enabled.

So this patch is two fold, stop intel_gt_wait_for_idle from short circuting and fix runtime pm autosuspend.

v2: (John Harrison)

• Split structure changes out in different patch

(Tvrtko)

• Don't drop lock in deregister_destroyed_contexts

v3: (John Harrison)

• Flush destroyed contexts before destroying context reg pool

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

Reviewed-by: John Harrison John.C.Harrison@Intel.com

---

drivers/gpu/drm/i915/gt/intel_context.c | 2 + drivers/gpu/drm/i915/gt/intel_context_types.h | 7 + drivers/gpu/drm/i915/gt/intel_engine_pm.h | 5 + drivers/gpu/drm/i915/gt/intel_gt_pm.h | 4 + drivers/gpu/drm/i915/gt/uc/intel_guc.h | 11 ++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 146 +++++++++++------- 6 files changed, 121 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 35babd02ddfe..d008ef8623ce 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -400,6 +400,8 @@ intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine) ce->guc_id.id = GUC_INVALID_LRC_ID; INIT_LIST_HEAD(&ce->guc_id.link);

• INIT_LIST_HEAD(&ce->destroyed_link);
• /*
  • Initialize fence to be complete as this is expected to be complete
  • unless there is a pending schedule disable outstanding.

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index e7e3984aab78..4613d027cbc3 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -213,6 +213,13 @@ struct intel_context { struct list_head link; } guc_id;

• /**

• * @destroyed_link: link in guc->submission_state.destroyed_contexts, in
  

• * list when context is pending to be destroyed (deregistered with the
  

• * GuC), protected by guc->submission_state.lock
  

• */
  

• struct list_head destroyed_link;
• #ifdef CONFIG_DRM_I915_SELFTEST /**
  • @drop_schedule_enable: Force drop of schedule enable G2H for selftest

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h index 8520c595f5e1..6fdeae668e6e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h @@ -16,6 +16,11 @@ intel_engine_pm_is_awake(const struct intel_engine_cs *engine) return intel_wakeref_is_active(&engine->wakeref); }

+static inline void __intel_engine_pm_get(struct intel_engine_cs *engine) +{

• __intel_wakeref_get(&engine->wakeref);

+}

• static inline void intel_engine_pm_get(struct intel_engine_cs *engine) { intel_wakeref_get(&engine->wakeref);

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index d0588d8aaa44..05de6c1af25b 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -41,6 +41,10 @@ static inline void intel_gt_pm_put_async(struct intel_gt *gt) intel_wakeref_put_async(&gt->wakeref); }

+#define with_intel_gt_pm(gt, tmp) \

• for (tmp = 1, intel_gt_pm_get(gt); tmp; \

•     intel_gt_pm_put(gt), tmp = 0)
  

• static inline int intel_gt_pm_wait_for_idle(struct intel_gt *gt) { return intel_wakeref_wait_for_idle(&gt->wakeref);

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 82e248c2290c..74f071a0b6d5 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -90,6 +90,17 @@ struct intel_guc { * refs */ struct list_head guc_id_list;

• /**
  

•  * @destroyed_contexts: list of contexts waiting to be destroyed
  

•  * (deregistered with the GuC)
  

•  */
  

• struct list_head destroyed_contexts;
  

• /**
  

•  * @destroyed_worker: worker to deregister contexts, need as we
  

•  * need to take a GT PM reference and can't from destroy
  

•  * function as it might be in an atomic context (no sleeping)
  

•  */
  

• struct work_struct destroyed_worker;
  

  } submission_state;

  /**

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 b2646b088c7f..d2ce47b5541e 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -90,8 +90,8 @@

• used for all of GuC submission but that could change in the future.
• guc->submission_state.lock

• • Protects guc_id allocation for the given GuC, i.e. only one context can be
• • doing guc_id allocation operations at a time for each GuC in the system.

• • Global lock for GuC submission state. Protects guc_ids and destroyed contexts
• • list.
  • ce->guc_state.lock
  • Protects everything under ce->guc_state. Ensures that a context is in the

@@ -719,6 +719,7 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc) if (deregister) guc_signal_context_fence(ce); if (destroyed) {

• 		intel_gt_pm_put_async(guc_to_gt(guc));
  	release_guc_id(guc, ce);
  	__guc_context_destroy(ce);
  }
  

@@ -797,6 +798,8 @@ static void guc_flush_submissions(struct intel_guc *guc) spin_unlock_irqrestore(&sched_engine->lock, flags); }

+static void guc_flush_destroyed_contexts(struct intel_guc *guc);

• void intel_guc_submission_reset_prepare(struct intel_guc *guc) { int i;

@@ -815,6 +818,7 @@ void intel_guc_submission_reset_prepare(struct intel_guc *guc) spin_unlock_irq(&guc_to_gt(guc)->irq_lock);

guc_flush_submissions(guc);

• guc_flush_destroyed_contexts(guc);

  /*

  • Handle any outstanding G2Hs before reset. Call IRQ handler directly

@@ -1126,6 +1130,8 @@ void intel_guc_submission_reset_finish(struct intel_guc *guc) intel_gt_unpark_heartbeats(guc_to_gt(guc)); }

+static void destroyed_worker_func(struct work_struct *w);

• /*
  • Set up the memory resources to be shared with the GuC (via the GGTT)
  • at firmware loading time.

@@ -1151,6 +1157,9 @@ int intel_guc_submission_init(struct intel_guc *guc) spin_lock_init(&guc->submission_state.lock); INIT_LIST_HEAD(&guc->submission_state.guc_id_list); ida_init(&guc->submission_state.guc_ids);

• INIT_LIST_HEAD(&guc->submission_state.destroyed_contexts);

• INIT_WORK(&guc->submission_state.destroyed_worker,

•   destroyed_worker_func);
  
  

  return 0; }

@@ -1160,6 +1169,7 @@ void intel_guc_submission_fini(struct intel_guc *guc) if (!guc->lrc_desc_pool) return;

• guc_flush_destroyed_contexts(guc); guc_lrc_desc_pool_destroy(guc); i915_sched_engine_put(guc->sched_engine); }

@@ -1859,11 +1869,30 @@ static void guc_context_sched_disable(struct intel_context *ce) static inline void guc_lrc_desc_unpin(struct intel_context *ce) { struct intel_guc *guc = ce_to_guc(ce);

• struct intel_gt *gt = guc_to_gt(guc);

• unsigned long flags;

• bool disabled;

• GEM_BUG_ON(!intel_gt_pm_is_awake(gt)); GEM_BUG_ON(!lrc_desc_registered(guc, ce->guc_id.id)); GEM_BUG_ON(ce != __get_context(guc, ce->guc_id.id)); GEM_BUG_ON(context_enabled(ce));

• /* Seal race with Reset */

• spin_lock_irqsave(&ce->guc_state.lock, flags);

• disabled = submission_disabled(guc);

• if (likely(!disabled)) {

• __intel_gt_pm_get(gt);
  

• set_context_destroyed(ce);
  

• clr_context_registered(ce);
  

• }

• spin_unlock_irqrestore(&ce->guc_state.lock, flags);

• if (unlikely(disabled)) {

• release_guc_id(guc, ce);
  

• __guc_context_destroy(ce);
  

• return;
  

• }

• deregister_context(ce, ce->guc_id.id); }

@@ -1891,78 +1920,86 @@ static void __guc_context_destroy(struct intel_context *ce) } }

+static void guc_flush_destroyed_contexts(struct intel_guc *guc) +{

• struct intel_context *ce, *cn;
• unsigned long flags;
• GEM_BUG_ON(!submission_disabled(guc) &&

•    guc_submission_initialized(guc));
  

• spin_lock_irqsave(&guc->submission_state.lock, flags);
• list_for_each_entry_safe(ce, cn,

• 		 &guc->submission_state.destroyed_contexts,
  

• 		 destroyed_link) {
  

• list_del_init(&ce->destroyed_link);
  

• __release_guc_id(guc, ce);
  

• __guc_context_destroy(ce);
  

• }
• spin_unlock_irqrestore(&guc->submission_state.lock, flags);

+}

+static void deregister_destroyed_contexts(struct intel_guc *guc) +{

• struct intel_context *ce, *cn;
• unsigned long flags;
• spin_lock_irqsave(&guc->submission_state.lock, flags);
• list_for_each_entry_safe(ce, cn,

• 		 &guc->submission_state.destroyed_contexts,
  

• 		 destroyed_link) {
  

• list_del_init(&ce->destroyed_link);
  

• guc_lrc_desc_unpin(ce);
  

• }
• spin_unlock_irqrestore(&guc->submission_state.lock, flags);

+}

+static void destroyed_worker_func(struct work_struct *w) +{

• struct intel_guc *guc = container_of(w, struct intel_guc,

• 			     submission_state.destroyed_worker);
  

• struct intel_gt *gt = guc_to_gt(guc);
• int tmp;
• with_intel_gt_pm(gt, tmp)

• deregister_destroyed_contexts(guc);
  

+}

• static void guc_context_destroy(struct kref *kref) { struct intel_context *ce = container_of(kref, typeof(*ce), ref);

• struct intel_runtime_pm *runtime_pm = ce->engine->uncore->rpm; struct intel_guc *guc = ce_to_guc(ce);
• intel_wakeref_t wakeref; unsigned long flags;
• bool disabled;

• bool destroy;

  /*

  • If the guc_id is invalid this context has been stolen and we can free
  • it immediately. Also can be freed immediately if the context is not
  • registered with the GuC or the GuC is in the middle of a reset.

  */

• if (context_guc_id_invalid(ce)) {

• __guc_context_destroy(ce);
  

• return;
  

• } else if (submission_disabled(guc) ||

•    !lrc_desc_registered(guc, ce->guc_id.id)) {
  

• release_guc_id(guc, ce);
  

• __guc_context_destroy(ce);
  

• return;
  

• }
• /*

• * We have to acquire the context spinlock and check guc_id again, if it
  

• * is valid it hasn't been stolen and needs to be deregistered. We
  

• * delete this context from the list of unpinned guc_id available to
  

• * steal to seal a race with guc_lrc_desc_pin(). When the G2H CTB
  

• * returns indicating this context has been deregistered the guc_id is
  

• * returned to the pool of available guc_id.
  

• */
  

  spin_lock_irqsave(&guc->submission_state.lock, flags);
• if (context_guc_id_invalid(ce)) {

• spin_unlock_irqrestore(&guc->submission_state.lock, flags);
  

• __guc_context_destroy(ce);
  

• return;
  

• destroy = submission_disabled(guc) || context_guc_id_invalid(ce) ||

• !lrc_desc_registered(guc, ce->guc_id.id);
  

• if (likely(!destroy)) {

• if (!list_empty(&ce->guc_id.link))
  

• 	list_del_init(&ce->guc_id.link);
  

• list_add_tail(&ce->destroyed_link,
  

• 	      &guc->submission_state.destroyed_contexts);
  

• } else {

• __release_guc_id(guc, ce);
  

  }

• if (!list_empty(&ce->guc_id.link))

• list_del_init(&ce->guc_id.link);
  

  spin_unlock_irqrestore(&guc->submission_state.lock, flags);
• /* Seal race with Reset */
• spin_lock_irqsave(&ce->guc_state.lock, flags);
• disabled = submission_disabled(guc);
• if (likely(!disabled)) {

• set_context_destroyed(ce);
  

• clr_context_registered(ce);
  

• }
• spin_unlock_irqrestore(&ce->guc_state.lock, flags);
• if (unlikely(disabled)) {

• release_guc_id(guc, ce);
  

• if (unlikely(destroy)) { __guc_context_destroy(ce); return; }

  /*

• * We defer GuC context deregistration until the context is destroyed
  

• * in order to save on CTBs. With this optimization ideally we only need
  

• * 1 CTB to register the context during the first pin and 1 CTB to
  

• * deregister the context when the context is destroyed. Without this
  

• * optimization, a CTB would be needed every pin & unpin.
  

• *
  

• * XXX: Need to acqiure the runtime wakeref as this can be triggered
  

• * from context_free_worker when runtime wakeref is not held.
  

• * guc_lrc_desc_unpin requires the runtime as a GuC register is written
  

• * in H2G CTB to deregister the context. A future patch may defer this
  

• * H2G CTB if the runtime wakeref is zero.
  

• * We use a worker to issue the H2G to deregister the context as we can
  

• * take the GT PM for the first time which isn't allowed from an atomic
  

• * context.
  

  */

• with_intel_runtime_pm(runtime_pm, wakeref)

• guc_lrc_desc_unpin(ce);
  

• queue_work(system_unbound_wq, &guc->submission_state.destroyed_worker); }

  static int guc_context_alloc(struct intel_context *ce)

@@ -2798,6 +2835,7 @@ int intel_guc_deregister_done_process_msg(struct intel_guc *guc, intel_context_put(ce); } else if (context_destroyed(ce)) { /* Context has been destroyed */

• intel_gt_pm_put_async(guc_to_gt(guc));
  

  release_guc_id(guc, ce); __guc_context_destroy(ce); }

On 10/13/2021 13:42, Matthew Brost wrote:

Taking a PM reference to prevent intel_gt_wait_for_idle from short circuiting while any user context has scheduling enabled. Returning GT idle when it is not can cause all sorts of issues throughout the stack.

v2: (Daniel Vetter)

• Add might_lock annotations to pin / unpin function

v3: (CI)

• Drop intel_engine_pm_might_put from unpin path as an async put is used

v4: (John Harrison)

• Make intel_engine_pm_might_get/put work with GuC virtual engines
• Update commit message

v5:

• Update commit message again

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

Reviewed-by: John Harrison John.C.Harrison@Intel.com

---

drivers/gpu/drm/i915/gt/intel_context.c | 2 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.h | 32 +++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt_pm.h | 10 ++++++ .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 36 +++++++++++++++++-- drivers/gpu/drm/i915/intel_wakeref.h | 12 +++++++ 5 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index d008ef8623ce..f98c9f470ba1 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -240,6 +240,8 @@ int __intel_context_do_pin_ww(struct intel_context *ce, if (err) goto err_post_unpin;

• intel_engine_pm_might_get(ce->engine);
• if (unlikely(intel_context_is_closed(ce))) { err = -ENOENT; goto err_unlock;

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h index 6fdeae668e6e..d68675925b79 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h @@ -6,9 +6,11 @@ #ifndef INTEL_ENGINE_PM_H #define INTEL_ENGINE_PM_H

+#include "i915_drv.h" #include "i915_request.h" #include "intel_engine_types.h" #include "intel_wakeref.h" +#include "intel_gt_pm.h"

static inline bool intel_engine_pm_is_awake(const struct intel_engine_cs *engine) @@ -31,6 +33,21 @@ static inline bool intel_engine_pm_get_if_awake(struct intel_engine_cs *engine) return intel_wakeref_get_if_active(&engine->wakeref); }

+static inline void intel_engine_pm_might_get(struct intel_engine_cs *engine) +{

• if (!intel_engine_is_virtual(engine)) {

• intel_wakeref_might_get(&engine->wakeref);
  

• } else {

• struct intel_gt *gt = engine->gt;
  

• struct intel_engine_cs *tengine;
  

• intel_engine_mask_t tmp, mask = engine->mask;
  

• for_each_engine_masked(tengine, gt, mask, tmp)
  

• 	intel_wakeref_might_get(&tengine->wakeref);
  

• }
• intel_gt_pm_might_get(engine->gt);

+}

• static inline void intel_engine_pm_put(struct intel_engine_cs *engine) { intel_wakeref_put(&engine->wakeref);

@@ -52,6 +69,21 @@ static inline void intel_engine_pm_flush(struct intel_engine_cs *engine) intel_wakeref_unlock_wait(&engine->wakeref); }

+static inline void intel_engine_pm_might_put(struct intel_engine_cs *engine) +{

• if (!intel_engine_is_virtual(engine)) {

• intel_wakeref_might_put(&engine->wakeref);
  

• } else {

• struct intel_gt *gt = engine->gt;
  

• struct intel_engine_cs *tengine;
  

• intel_engine_mask_t tmp, mask = engine->mask;
  

• for_each_engine_masked(tengine, gt, mask, tmp)
  

• 	intel_wakeref_might_put(&tengine->wakeref);
  

• }
• intel_gt_pm_might_put(engine->gt);

+}

• static inline struct i915_request * intel_engine_create_kernel_request(struct intel_engine_cs *engine) {

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index 05de6c1af25b..bc898df7a48c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -31,6 +31,11 @@ static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) return intel_wakeref_get_if_active(&gt->wakeref); }

+static inline void intel_gt_pm_might_get(struct intel_gt *gt) +{

• intel_wakeref_might_get(&gt->wakeref);

+}

• static inline void intel_gt_pm_put(struct intel_gt *gt) { intel_wakeref_put(&gt->wakeref);

@@ -41,6 +46,11 @@ static inline void intel_gt_pm_put_async(struct intel_gt *gt) intel_wakeref_put_async(&gt->wakeref); }

+static inline void intel_gt_pm_might_put(struct intel_gt *gt) +{

• intel_wakeref_might_put(&gt->wakeref);

+}

• #define with_intel_gt_pm(gt, tmp) \ for (tmp = 1, intel_gt_pm_get(gt); tmp; \ intel_gt_pm_put(gt), tmp = 0)

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 d2ce47b5541e..51d3963cebbf 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1571,7 +1571,12 @@ static int guc_context_pre_pin(struct intel_context *ce,

static int guc_context_pin(struct intel_context *ce, void *vaddr) {

• return __guc_context_pin(ce, ce->engine, vaddr);

• int ret = __guc_context_pin(ce, ce->engine, vaddr);

• if (likely(!ret && !intel_context_is_barrier(ce)))

• intel_engine_pm_get(ce->engine);
  

• return ret; }

  static void guc_context_unpin(struct intel_context *ce)

@@ -1580,6 +1585,9 @@ static void guc_context_unpin(struct intel_context *ce)

unpin_guc_id(guc, ce); lrc_unpin(ce);

• if (likely(!intel_context_is_barrier(ce)))

• intel_engine_pm_put_async(ce->engine);
  

  }

  static void guc_context_post_unpin(struct intel_context *ce)

@@ -2341,8 +2349,30 @@ static int guc_virtual_context_pre_pin(struct intel_context *ce, static int guc_virtual_context_pin(struct intel_context *ce, void *vaddr) { struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0);

• int ret = __guc_context_pin(ce, engine, vaddr);
• intel_engine_mask_t tmp, mask = ce->engine->mask;
• if (likely(!ret))

• for_each_engine_masked(engine, ce->engine->gt, mask, tmp)
  

• 	intel_engine_pm_get(engine);
  
  

• return __guc_context_pin(ce, engine, vaddr);

• return ret;

+}

+static void guc_virtual_context_unpin(struct intel_context *ce) +{

• intel_engine_mask_t tmp, mask = ce->engine->mask;

• struct intel_engine_cs *engine;

• struct intel_guc *guc = ce_to_guc(ce);

• GEM_BUG_ON(context_enabled(ce));

• GEM_BUG_ON(intel_context_is_barrier(ce));

• unpin_guc_id(guc, ce);

• lrc_unpin(ce);

• for_each_engine_masked(engine, ce->engine->gt, mask, tmp)

• intel_engine_pm_put_async(engine);
  

  }

  static void guc_virtual_context_enter(struct intel_context *ce)

@@ -2379,7 +2409,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {

.pre_pin = guc_virtual_context_pre_pin, .pin = guc_virtual_context_pin,

• .unpin = guc_context_unpin,

• .unpin = guc_virtual_context_unpin, .post_unpin = guc_context_post_unpin,

  .ban = guc_context_ban,

diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h index 545c8f277c46..4f4c2e15e736 100644 --- a/drivers/gpu/drm/i915/intel_wakeref.h +++ b/drivers/gpu/drm/i915/intel_wakeref.h @@ -123,6 +123,12 @@ enum { __INTEL_WAKEREF_PUT_LAST_BIT__ };

+static inline void +intel_wakeref_might_get(struct intel_wakeref *wf) +{

• might_lock(&wf->mutex);

+}

• /**
  • intel_wakeref_put_flags: Release the wakeref
  • @wf: the wakeref

@@ -170,6 +176,12 @@ intel_wakeref_put_delay(struct intel_wakeref *wf, unsigned long delay) FIELD_PREP(INTEL_WAKEREF_PUT_DELAY, delay)); }

+static inline void +intel_wakeref_might_put(struct intel_wakeref *wf) +{

• might_lock(&wf->mutex);

+}

• /**
  • intel_wakeref_lock: Lock the wakeref (mutex)
  • @wf: the wakeref

On 10/13/2021 13:42, Matthew Brost wrote:

Add multi-lrc context registration H2G. In addition a workqueue and process descriptor are setup during multi-lrc context registration as these data structures are needed for multi-lrc submission.

v2: (John Harrison)

• Move GuC specific fields into sub-struct
• Clean up WQ defines
• Add comment explaining math to derive WQ / PD address

v3: (John Harrison)

• Add PARENT_SCRATCH_SIZE define
• Update comment explaining multi-lrc register

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

drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++ drivers/gpu/drm/i915/gt/intel_lrc.c | 5 + .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 - .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 116 +++++++++++++++++- 5 files changed, 133 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 76dfca57cb45..48decb5ee954 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -239,6 +239,18 @@ struct intel_context { struct intel_context *parent; /** @number_children: number of children if parent */ u8 number_children;

• /** @guc: GuC specific members for parallel submission */
  

• struct {
  

• 	/** @wqi_head: head pointer in work queue */
  

• 	u16 wqi_head;
  

• 	/** @wqi_tail: tail pointer in work queue */
  

• 	u16 wqi_tail;
  

• 	/**
  

• 	 * @parent_page: page in context state (ce->state) used
  

• 	 * by parent for work queue, process descriptor
  

• 	 */
  

• 	u8 parent_page;
  

• } guc;
  

  } parallel;

  #ifdef CONFIG_DRM_I915_SELFTEST

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 3ef9eaf8c50e..57339d5c1fc8 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -942,6 +942,11 @@ __lrc_alloc_state(struct intel_context *ce, struct intel_engine_cs *engine) context_size += PAGE_SIZE; }

• if (intel_context_is_parent(ce) && intel_engine_uses_guc(engine)) {

• ce->parallel.guc.parent_page = context_size / PAGE_SIZE;
  

• context_size += PAGE_SIZE;
  

This needs to be += PARENT_SCRATCH_SIZE.

John.

• }
• obj = i915_gem_object_create_lmem(engine->i915, context_size, I915_BO_ALLOC_PM_VOLATILE); if (IS_ERR(obj))

diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index 8ff582222aff..ba10bd374cee 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -142,6 +142,7 @@ enum intel_guc_action { INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505, INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER = 0x4506, INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE = 0x4600,

• INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, INTEL_GUC_ACTION_RESET_CLIENT = 0x5507, INTEL_GUC_ACTION_LIMIT };

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h index fa4be13c8854..0eeb2a9feeed 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h @@ -52,8 +52,6 @@

#define GUC_DOORBELL_INVALID 256

-#define GUC_WQ_SIZE (PAGE_SIZE * 2)

• /* Work queue item header definitions */ #define WQ_STATUS_ACTIVE 1 #define WQ_STATUS_SUSPENDED 2

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 84b8e64b148f..58a6f494be8f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -344,6 +344,47 @@ static inline struct i915_priolist *to_priolist(struct rb_node *rb) return rb_entry(rb, struct i915_priolist, node); }

+/*

• • When using multi-lrc submission a scratch memory area is reserved in the
• • parent's context state for the process descriptor and work queue. Currently
• • the scratch area is sized to a page.
• • The layout of this scratch area is below:
• • 0 guc_process_desc
• • ... unused
• • PARENT_SCRATCH_SIZE / 2 work queue start
• • ... work queue
• • PARENT_SCRATCH_SIZE - 1 work queue end
• */

+#define PARENT_SCRATCH_SIZE PAGE_SIZE +#define WQ_SIZE (PARENT_SCRATCH_SIZE / 2) +#define WQ_OFFSET (PARENT_SCRATCH_SIZE - WQ_SIZE) +static u32 __get_process_desc_offset(struct intel_context *ce) +{

• GEM_BUG_ON(!ce->parallel.guc.parent_page);
• return ce->parallel.guc.parent_page * PAGE_SIZE;

+}

+static u32 __get_wq_offset(struct intel_context *ce) +{

• return __get_process_desc_offset(ce) + WQ_OFFSET;

+}

+static struct guc_process_desc * +__get_process_desc(struct intel_context *ce) +{

• /*

• * Need to subtract LRC_STATE_OFFSET here as the
  

• * parallel.guc.parent_page is the offset into ce->state while
  

• * ce->lrc_reg_reg is ce->state + LRC_STATE_OFFSET.
  

• */
  

• return (struct guc_process_desc *)

• (ce->lrc_reg_state +
  

•  ((__get_process_desc_offset(ce) -
  

•    LRC_STATE_OFFSET) / sizeof(u32)));
  

+}

• static struct guc_lrc_desc *__get_lrc_desc(struct intel_guc *guc, u32 index) { struct guc_lrc_desc *base = guc->lrc_desc_pool_vaddr;

@@ -1365,6 +1406,30 @@ static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce) spin_unlock_irqrestore(&guc->submission_state.lock, flags); }

+static int __guc_action_register_multi_lrc(struct intel_guc *guc,

• 			   struct intel_context *ce,
  

• 			   u32 guc_id,
  

• 			   u32 offset,
  

• 			   bool loop)
  

+{

• struct intel_context *child;
• u32 action[4 + MAX_ENGINE_INSTANCE];
• int len = 0;
• GEM_BUG_ON(ce->parallel.number_children > MAX_ENGINE_INSTANCE);
• action[len++] = INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC;
• action[len++] = guc_id;
• action[len++] = ce->parallel.number_children + 1;
• action[len++] = offset;
• for_each_child(ce, child) {

• offset += sizeof(struct guc_lrc_desc);
  

• action[len++] = offset;
  

• }
• return guc_submission_send_busy_loop(guc, action, len, 0, loop);

+}

• static int __guc_action_register_context(struct intel_guc *guc, u32 guc_id, u32 offset,

@@ -1387,9 +1452,15 @@ static int register_context(struct intel_context *ce, bool loop) ce->guc_id.id * sizeof(struct guc_lrc_desc); int ret;

• GEM_BUG_ON(intel_context_is_child(ce)); trace_intel_context_register(ce);

• ret = __guc_action_register_context(guc, ce->guc_id.id, offset, loop);

• if (intel_context_is_parent(ce))

• ret = __guc_action_register_multi_lrc(guc, ce, ce->guc_id.id,
  

• 				      offset, loop);
  

• else

• ret = __guc_action_register_context(guc, ce->guc_id.id, offset,
  

• 				    loop);
  

  if (likely(!ret)) { unsigned long flags;

@@ -1418,6 +1489,7 @@ static int deregister_context(struct intel_context *ce, u32 guc_id) { struct intel_guc *guc = ce_to_guc(ce);

• GEM_BUG_ON(intel_context_is_child(ce)); trace_intel_context_deregister(ce);

  return __guc_action_deregister_context(guc, guc_id);

@@ -1445,6 +1517,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) struct guc_lrc_desc *desc; bool context_registered; intel_wakeref_t wakeref;

• struct intel_context *child; int ret = 0;

  GEM_BUG_ON(!engine->mask);

@@ -1470,6 +1543,41 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD; guc_context_policy_init(engine, desc);

• /*

• * If context is a parent, we need to register a process descriptor
  

• * describing a work queue and register all child contexts.
  

• */
  

• if (intel_context_is_parent(ce)) {

• struct guc_process_desc *pdesc;
  

• ce->parallel.guc.wqi_tail = 0;
  

• ce->parallel.guc.wqi_head = 0;
  

• desc->process_desc = i915_ggtt_offset(ce->state) +
  

• 	__get_process_desc_offset(ce);
  

• desc->wq_addr = i915_ggtt_offset(ce->state) +
  

• 	__get_wq_offset(ce);
  

• desc->wq_size = WQ_SIZE;
  

• pdesc = __get_process_desc(ce);
  

• memset(pdesc, 0, sizeof(*(pdesc)));
  

• pdesc->stage_id = ce->guc_id.id;
  

• pdesc->wq_base_addr = desc->wq_addr;
  

• pdesc->wq_size_bytes = desc->wq_size;
  

• pdesc->wq_status = WQ_STATUS_ACTIVE;
  

• for_each_child(ce, child) {
  

• 	desc = __get_lrc_desc(guc, child->guc_id.id);
  

• 	desc->engine_class =
  

• 		engine_class_to_guc_class(engine->class);
  

• 	desc->hw_context_desc = child->lrc.lrca;
  

• 	desc->priority = ce->guc_state.prio;
  

• 	desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
  

• 	guc_context_policy_init(engine, desc);
  

• }
  

• }
• /*
  • The context_lookup xarray is used to determine if the hardware
  • context is currently registered. There are two cases in which it

@@ -2804,6 +2912,12 @@ g2h_context_lookup(struct intel_guc *guc, u32 desc_idx) return NULL; }

• if (unlikely(intel_context_is_child(ce))) {

• drm_err(&guc_to_gt(guc)->i915->drm,
  

• 	"Context is child, desc_idx %u", desc_idx);
  

• return NULL;
  

• }
• return ce; }

On 10/13/2021 13:42, Matthew Brost wrote:

Update context and full GPU reset to work with multi-lrc. The idea is parent context tracks all the active requests inflight for itself and its children. The parent context owns the reset replaying / canceling requests as needed.

v2: (John Harrison)

• Simply loop in find active request
• Add comments to find ative request / reset loop

v3: (John Harrison)

• s/its'/its/g
• Fix comment when searching for active request
• Reorder if state in __guc_reset_context

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

Reviewed-by: John Harrison John.C.Harrison@Intel.com

---

drivers/gpu/drm/i915/gt/intel_context.c | 15 +++- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 69 ++++++++++++++----- 2 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 79f321c6c008..6aab60584ee5 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -529,20 +529,29 @@ struct i915_request *intel_context_create_request(struct intel_context *ce)

struct i915_request *intel_context_find_active_request(struct intel_context *ce) {

• struct intel_context *parent = intel_context_to_parent(ce); struct i915_request *rq, *active = NULL; unsigned long flags;

  GEM_BUG_ON(!intel_engine_uses_guc(ce->engine));

• spin_lock_irqsave(&ce->guc_state.lock, flags);
• list_for_each_entry_reverse(rq, &ce->guc_state.requests,

• /*

• * We search the parent list to find an active request on the submitted
  

• * context. The parent list contains the requests for all the contexts
  

• * in the relationship so we have to do a compare of each request's
  

• * context.
  

• */
  

• spin_lock_irqsave(&parent->guc_state.lock, flags);

• list_for_each_entry_reverse(rq, &parent->guc_state.requests, sched.link) {

• if (rq->context != ce)
  

• 	continue;
  

  if (i915_request_completed(rq)) break;

  active = rq; }

• spin_unlock_irqrestore(&ce->guc_state.lock, flags);

• spin_unlock_irqrestore(&parent->guc_state.lock, flags);

  return active; }

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 f690b7c2b295..bc052d206861 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -683,6 +683,11 @@ static inline int rq_prio(const struct i915_request *rq) return rq->sched.attr.priority; }

+static inline bool is_multi_lrc(struct intel_context *ce) +{

• return intel_context_is_parallel(ce);

+}

• static bool is_multi_lrc_rq(struct i915_request *rq) { return intel_context_is_parallel(rq->context);

@@ -1218,10 +1223,15 @@ __unwind_incomplete_requests(struct intel_context *ce)

static void __guc_reset_context(struct intel_context *ce, bool stalled) {

• bool local_stalled; struct i915_request *rq; unsigned long flags; u32 head;

• int i, number_children = ce->parallel.number_children; bool skip = false;

• struct intel_context *parent = ce;

• GEM_BUG_ON(intel_context_is_child(ce));

  intel_context_get(ce);

@@ -1247,25 +1257,38 @@ static void __guc_reset_context(struct intel_context *ce, bool stalled) if (unlikely(skip)) goto out_put;

• rq = intel_context_find_active_request(ce);
• if (!rq) {

• head = ce->ring->tail;
  

• stalled = false;
  

• goto out_replay;
  

• }

• /*

• * For each context in the relationship find the hanging request
  

• * resetting each context / request as needed
  

• */
  

• for (i = 0; i < number_children + 1; ++i) {

• if (!intel_context_is_pinned(ce))
  

• 	goto next_context;
  

• local_stalled = false;
  

• rq = intel_context_find_active_request(ce);
  

• if (!rq) {
  

• 	head = ce->ring->tail;
  

• 	goto out_replay;
  

• }
  
  

• if (!i915_request_started(rq))

• stalled = false;
  

• if (i915_request_started(rq))
  

• 	local_stalled = true;
  
  

• GEM_BUG_ON(i915_active_is_idle(&ce->active));
• head = intel_ring_wrap(ce->ring, rq->head);
• __i915_request_reset(rq, stalled);

• GEM_BUG_ON(i915_active_is_idle(&ce->active));
  

• head = intel_ring_wrap(ce->ring, rq->head);
  
  

• __i915_request_reset(rq, local_stalled && stalled);
  

  out_replay:

• guc_reset_state(ce, head, stalled);
• __unwind_incomplete_requests(ce);

• guc_reset_state(ce, head, local_stalled && stalled);
  

+next_context:

• if (i != number_children)
  

• 	ce = list_next_entry(ce, parallel.child_link);
  

• }
• __unwind_incomplete_requests(parent); out_put:

• intel_context_put(ce);

• intel_context_put(parent); }

  void intel_guc_submission_reset(struct intel_guc *guc, bool stalled)

@@ -1286,7 +1309,8 @@ void intel_guc_submission_reset(struct intel_guc *guc, bool stalled)

xa_unlock(&guc->context_lookup);

• if (intel_context_is_pinned(ce))
  

• if (intel_context_is_pinned(ce) &&
  

•     !intel_context_is_child(ce))
  __guc_reset_context(ce, stalled);
  
  

  intel_context_put(ce);

@@ -1378,7 +1402,8 @@ void intel_guc_submission_cancel_requests(struct intel_guc *guc)

xa_unlock(&guc->context_lookup);

• if (intel_context_is_pinned(ce))
  

• if (intel_context_is_pinned(ce) &&
  

•     !intel_context_is_child(ce))
  guc_cancel_context_requests(ce);
  
  

  intel_context_put(ce);

@@ -2071,6 +2096,8 @@ static struct i915_sw_fence *guc_context_block(struct intel_context *ce) u16 guc_id; bool enabled;

• GEM_BUG_ON(intel_context_is_child(ce));

• spin_lock_irqsave(&ce->guc_state.lock, flags);

  incr_context_blocked(ce);

@@ -2125,6 +2152,7 @@ static void guc_context_unblock(struct intel_context *ce) bool enable;

GEM_BUG_ON(context_enabled(ce));

• GEM_BUG_ON(intel_context_is_child(ce));

  spin_lock_irqsave(&ce->guc_state.lock, flags);

@@ -2151,11 +2179,14 @@ static void guc_context_unblock(struct intel_context *ce) static void guc_context_cancel_request(struct intel_context *ce, struct i915_request *rq) {

• struct intel_context *block_context =

• request_to_scheduling_context(rq);
  

• if (i915_sw_fence_signaled(&rq->submit)) { struct i915_sw_fence *fence;

  intel_context_get(ce);

• fence = guc_context_block(ce);
  

• fence = guc_context_block(block_context);
  

  i915_sw_fence_wait(fence); if (!i915_request_completed(rq)) { __i915_request_skip(rq);

@@ -2169,7 +2200,7 @@ static void guc_context_cancel_request(struct intel_context *ce, */ flush_work(&ce_to_guc(ce)->ct.requests.worker);

• guc_context_unblock(ce);
  

• guc_context_unblock(block_context);
  

  intel_context_put(ce); } }

@@ -2195,6 +2226,8 @@ static void guc_context_ban(struct intel_context *ce, struct i915_request *rq) intel_wakeref_t wakeref; unsigned long flags;

• GEM_BUG_ON(intel_context_is_child(ce));

• guc_flush_submissions(guc);

  spin_lock_irqsave(&ce->guc_state.lock, flags);

On 10/13/2021 13:42, Matthew Brost wrote:

Introduce 'set parallel submit' extension to connect UAPI to GuC multi-lrc interface. Kernel doc in new uAPI should explain it all.

IGT: https://patchwork.freedesktop.org/patch/447008/?series=93071&rev=1 media UMD: https://github.com/intel/media-driver/pull/1252

v2: (Daniel Vetter)

• Add IGT link and placeholder for media UMD link

v3: (Kernel test robot)

• Fix warning in unpin engines call

(John Harrison)

• Reword a bunch of the kernel doc

v4: (John Harrison)

• Add comment why perma-pin is done after setting gem context
• Update some comments / docs for proto contexts

Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_context.c | 228 +++++++++++++++++- .../gpu/drm/i915/gem/i915_gem_context_types.h | 16 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 9 +- drivers/gpu/drm/i915/gt/intel_engine.h | 12 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 +- .../drm/i915/gt/intel_execlists_submission.c | 6 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 12 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 114 ++++++++- include/uapi/drm/i915_drm.h | 131 ++++++++++ 9 files changed, 503 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index d225d3dd0b40..6f23aff6e642 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -556,9 +556,150 @@ set_proto_ctx_engines_bond(struct i915_user_extension __user *base, void *data) return 0; }

+static int +set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base,

• 		      void *data)
  

+{

• struct i915_context_engines_parallel_submit __user *ext =

• container_of_user(base, typeof(*ext), base);
  

• const struct set_proto_ctx_engines *set = data;
• struct drm_i915_private *i915 = set->i915;
• u64 flags;
• int err = 0, n, i, j;
• u16 slot, width, num_siblings;
• struct intel_engine_cs **siblings = NULL;
• intel_engine_mask_t prev_mask;
• /* Disabling for now */
• return -ENODEV;
• /* FIXME: This is NIY for execlists */
• if (!(intel_uc_uses_guc_submission(&i915->gt.uc)))

• return -ENODEV;
  

• if (get_user(slot, &ext->engine_index))

• return -EFAULT;
  

• if (get_user(width, &ext->width))

• return -EFAULT;
  

• if (get_user(num_siblings, &ext->num_siblings))

• return -EFAULT;
  

• if (slot >= set->num_engines) {

• drm_dbg(&i915->drm, "Invalid placement value, %d >= %d\n",
  

• 	slot, set->num_engines);
  

• return -EINVAL;
  

• }
• if (set->engines[slot].type != I915_GEM_ENGINE_TYPE_INVALID) {

• drm_dbg(&i915->drm,
  

• 	"Invalid placement[%d], already occupied\n", slot);
  

• return -EINVAL;
  

• }
• if (get_user(flags, &ext->flags))

• return -EFAULT;
  

• if (flags) {

• drm_dbg(&i915->drm, "Unknown flags 0x%02llx", flags);
  

• return -EINVAL;
  

• }
• for (n = 0; n < ARRAY_SIZE(ext->mbz64); n++) {

• err = check_user_mbz(&ext->mbz64[n]);
  

• if (err)
  

• 	return err;
  

• }
• if (width < 2) {

• drm_dbg(&i915->drm, "Width (%d) < 2\n", width);
  

• return -EINVAL;
  

• }
• if (num_siblings < 1) {

• drm_dbg(&i915->drm, "Number siblings (%d) < 1\n",
  

• 	num_siblings);
  

• return -EINVAL;
  

• }
• siblings = kmalloc_array(num_siblings * width,

• 		 sizeof(*siblings),
  

• 		 GFP_KERNEL);
  

• if (!siblings)

• return -ENOMEM;
  

• /* Create contexts / engines */
• for (i = 0; i < width; ++i) {

• intel_engine_mask_t current_mask = 0;
  

• struct i915_engine_class_instance prev_engine;
  

• for (j = 0; j < num_siblings; ++j) {
  

• 	struct i915_engine_class_instance ci;
  

• 	n = i * num_siblings + j;
  

• 	if (copy_from_user(&ci, &ext->engines[n], sizeof(ci))) {
  

• 		err = -EFAULT;
  

• 		goto out_err;
  

• 	}
  

• 	siblings[n] =
  

• 		intel_engine_lookup_user(i915, ci.engine_class,
  

• 					 ci.engine_instance);
  

• 	if (!siblings[n]) {
  

• 		drm_dbg(&i915->drm,
  

• 			"Invalid sibling[%d]: { class:%d, inst:%d }\n",
  

• 			n, ci.engine_class, ci.engine_instance);
  

• 		err = -EINVAL;
  

• 		goto out_err;
  

• 	}
  

• 	if (n) {
  

• 		if (prev_engine.engine_class !=
  

• 		    ci.engine_class) {
  

• 			drm_dbg(&i915->drm,
  

• 				"Mismatched class %d, %d\n",
  

• 				prev_engine.engine_class,
  

• 				ci.engine_class);
  

• 			err = -EINVAL;
  

• 			goto out_err;
  

• 		}
  

• 	}
  

• 	prev_engine = ci;
  

• 	current_mask |= siblings[n]->logical_mask;
  

• }
  

• if (i > 0) {
  

• 	if (current_mask != prev_mask << 1) {
  

• 		drm_dbg(&i915->drm,
  

• 			"Non contiguous logical mask 0x%x, 0x%x\n",
  

• 			prev_mask, current_mask);
  

• 		err = -EINVAL;
  

• 		goto out_err;
  

• 	}
  

• }
  

• prev_mask = current_mask;
  

• }
• set->engines[slot].type = I915_GEM_ENGINE_TYPE_PARALLEL;
• set->engines[slot].num_siblings = num_siblings;
• set->engines[slot].width = width;
• set->engines[slot].siblings = siblings;
• return 0;

+out_err:

• kfree(siblings);
• return err;

+}

• static const i915_user_extension_fn set_proto_ctx_engines_extensions[] = { [I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE] = set_proto_ctx_engines_balance, [I915_CONTEXT_ENGINES_EXT_BOND] = set_proto_ctx_engines_bond,

• [I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT] =

• set_proto_ctx_engines_parallel_submit,
  

  };

  static int set_proto_ctx_engines(struct drm_i915_file_private *fpriv,

@@ -818,6 +959,25 @@ static int intel_context_set_gem(struct intel_context *ce, return ret; }

+static void __unpin_engines(struct i915_gem_engines *e, unsigned int count) +{

• while (count--) {

• struct intel_context *ce = e->engines[count], *child;
  

• if (!ce || !test_bit(CONTEXT_PERMA_PIN, &ce->flags))
  

• 	continue;
  

• for_each_child(ce, child)
  

• 	intel_context_unpin(child);
  

• intel_context_unpin(ce);
  

• }

+}

+static void unpin_engines(struct i915_gem_engines *e) +{

• __unpin_engines(e, e->num_engines);

+}

• static void __free_engines(struct i915_gem_engines *e, unsigned int count) { while (count--) {

@@ -933,6 +1093,40 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx, return err; }

+static int perma_pin_contexts(struct intel_context *ce) +{

• struct intel_context *child;
• int i = 0, j = 0, ret;
• GEM_BUG_ON(!intel_context_is_parent(ce));
• ret = intel_context_pin(ce);
• if (unlikely(ret))

• return ret;
  

• for_each_child(ce, child) {

• ret = intel_context_pin(child);
  

• if (unlikely(ret))
  

• 	goto unwind;
  

• ++i;
  

• }
• set_bit(CONTEXT_PERMA_PIN, &ce->flags);
• return 0;

+unwind:

• intel_context_unpin(ce);
• for_each_child(ce, child) {

• if (j++ < i)
  

• 	intel_context_unpin(child);
  

• else
  

• 	break;
  

• }
• return ret;

+}

• static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, unsigned int num_engines, struct i915_gem_proto_engine *pe)

@@ -946,7 +1140,7 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, e->num_engines = num_engines;

for (n = 0; n < num_engines; n++) {

• struct intel_context *ce;
  

• struct intel_context *ce, *child;
  

  int ret;

  switch (pe[n].type) {

@@ -956,7 +1150,13 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,

case I915_GEM_ENGINE_TYPE_BALANCED:
	ce = intel_engine_create_virtual(pe[n].siblings,

• 					 pe[n].num_siblings);
  

• 					 pe[n].num_siblings, 0);
  

• 	break;
  

• case I915_GEM_ENGINE_TYPE_PARALLEL:
  

• 	ce = intel_engine_create_parallel(pe[n].siblings,
  

• 					  pe[n].num_siblings,
  

• 					  pe[n].width);
  break;
  
  

  case I915_GEM_ENGINE_TYPE_INVALID:

@@ -977,6 +1177,29 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, err = ERR_PTR(ret); goto free_engines; }

• for_each_child(ce, child) {
  

• 	ret = intel_context_set_gem(child, ctx, pe->sseu);
  

• 	if (ret) {
  

• 		err = ERR_PTR(ret);
  

• 		goto free_engines;
  

• 	}
  

• }
  

• /*
  

•  * XXX: Must be done after calling intel_context_set_gem as that
  

•  * function changes the ring size while the ring is allocated
  

•  * when the context is pinned. Mismatch the ring size will cause
  

while... when... is ambiguous - changes the ring size at the point when the context is pinned even though the ring is already allocated? Or changes the ring size even though the ring is already allocated and the context is already pinned? Something else?

Mismatch the ring -> A mismatch of the ring

John.

•  * the context to hang. Presumably with a bit of reordering we
  

•  * could move the perma-pin step to the backend function
  

•  * intel_engine_create_parallel.
  

•  */
  

• if (pe[n].type == I915_GEM_ENGINE_TYPE_PARALLEL) {
  

• 	ret = perma_pin_contexts(ce);
  

• 	if (ret) {
  

• 		err = ERR_PTR(ret);
  

• 		goto free_engines;
  

• 	}
  

• }
  

  }

  return e;

@@ -1219,6 +1442,7 @@ static void context_close(struct i915_gem_context *ctx)

/* Flush any concurrent set_engines() */ mutex_lock(&ctx->engines_mutex);

• unpin_engines(__context_engines_static(ctx)); engines_idle_release(ctx, rcu_replace_pointer(ctx->engines, NULL, 1)); i915_gem_context_set_closed(ctx); mutex_unlock(&ctx->engines_mutex);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index a627b09c4680..282cdb8a5c5a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -78,13 +78,16 @@ enum i915_gem_engine_type {

/** @I915_GEM_ENGINE_TYPE_BALANCED: A load-balanced engine set */ I915_GEM_ENGINE_TYPE_BALANCED,

• /** @I915_GEM_ENGINE_TYPE_PARALLEL: A parallel engine set */

• I915_GEM_ENGINE_TYPE_PARALLEL, };

  /**

  • struct i915_gem_proto_engine - prototype engine
  • This struct describes an engine that a context may contain. Engines

• • have three types:

• • have four types:
  • • I915_GEM_ENGINE_TYPE_INVALID: Invalid engines can be created but they
  • show up as a NULL in i915_gem_engines::engines[i] and any attempt to

@@ -97,6 +100,10 @@ enum i915_gem_engine_type {

• • I915_GEM_ENGINE_TYPE_BALANCED: A load-balanced engine set, described
• i915_gem_proto_engine::num_siblings and i915_gem_proto_engine::siblings.

• • • I915_GEM_ENGINE_TYPE_PARALLEL: A parallel submission engine set, described
• • i915_gem_proto_engine::width, i915_gem_proto_engine::num_siblings, and
• • i915_gem_proto_engine::siblings.
  */ struct i915_gem_proto_engine { /** @type: Type of this engine */

@@ -105,10 +112,13 @@ struct i915_gem_proto_engine { /** @engine: Engine, for physical */ struct intel_engine_cs *engine;

• /** @num_siblings: Number of balanced siblings */

• /** @num_siblings: Number of balanced or parallel siblings */ unsigned int num_siblings;

• /** @siblings: Balanced siblings */

• /** @width: Width of each sibling */

• unsigned int width;

• /** @siblings: Balanced siblings or num_siblings * width for parallel */ struct intel_engine_cs **siblings;

  /** @sseu: Client-set SSEU parameters */

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 8309d1141d0a..1d880303a7e4 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -55,9 +55,13 @@ struct intel_context_ops { void (*reset)(struct intel_context *ce); void (*destroy)(struct kref *kref);

• /* virtual engine/context interface */

• /* virtual/parallel engine/context interface */ struct intel_context *(*create_virtual)(struct intel_engine_cs **engine,

• 				unsigned int count);
  

• 				unsigned int count,
  

• 				unsigned long flags);
  

• struct intel_context *(*create_parallel)(struct intel_engine_cs **engines,

• 				 unsigned int num_siblings,
  

• 				 unsigned int width);
  

  struct intel_engine_cs *(*get_sibling)(struct intel_engine_cs *engine, unsigned int sibling); };

@@ -113,6 +117,7 @@ struct intel_context { #define CONTEXT_NOPREEMPT 8 #define CONTEXT_LRCA_DIRTY 9 #define CONTEXT_GUC_INIT 10 +#define CONTEXT_PERMA_PIN 11

struct { u64 timeout_us; diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index d5ac49c0691e..08559ace0ada 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -282,9 +282,19 @@ intel_engine_has_preempt_reset(const struct intel_engine_cs *engine) return intel_engine_has_preemption(engine); }

+#define FORCE_VIRTUAL BIT(0) struct intel_context * intel_engine_create_virtual(struct intel_engine_cs **siblings,

• 	    unsigned int count);
  

• 	    unsigned int count, unsigned long flags);
  

+static inline struct intel_context * +intel_engine_create_parallel(struct intel_engine_cs **engines,

• 	     unsigned int num_engines,
  

• 	     unsigned int width)
  

+{

• GEM_BUG_ON(!engines[0]->cops->create_parallel);
• return engines[0]->cops->create_parallel(engines, num_engines, width);

+}

static inline bool intel_virtual_engine_has_heartbeat(const struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2eb798ad068b..ff6753ccb129 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1953,16 +1953,16 @@ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine, ktime_t *now)

struct intel_context * intel_engine_create_virtual(struct intel_engine_cs **siblings,

• 	    unsigned int count)
  

• 	    unsigned int count, unsigned long flags)
  

  { if (count == 0) return ERR_PTR(-EINVAL);

• if (count == 1)

• if (count == 1 && !(flags & FORCE_VIRTUAL)) return intel_context_create(siblings[0]);

  GEM_BUG_ON(!siblings[0]->cops->create_virtual);

• return siblings[0]->cops->create_virtual(siblings, count);

• return siblings[0]->cops->create_virtual(siblings, count, flags); }

  struct i915_request *

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 43a74b216efb..bedb80057046 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -201,7 +201,8 @@ static struct virtual_engine *to_virtual_engine(struct intel_engine_cs *engine) }

static struct intel_context * -execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count); +execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

• 	 unsigned long flags);
  
  

  static struct i915_request * __active_request(const struct intel_timeline * const tl,

@@ -3784,7 +3785,8 @@ static void virtual_submit_request(struct i915_request *rq) }

static struct intel_context * -execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count) +execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

• 	 unsigned long flags)
  

  { struct virtual_engine *ve; unsigned int n;

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index 25a8c4f62b0d..b367ecfa42de 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -3733,7 +3733,7 @@ static int nop_virtual_engine(struct intel_gt *gt, GEM_BUG_ON(!nctx || nctx > ARRAY_SIZE(ve));

for (n = 0; n < nctx; n++) {

• ve[n] = intel_engine_create_virtual(siblings, nsibling);
  

• ve[n] = intel_engine_create_virtual(siblings, nsibling, 0);
  

  if (IS_ERR(ve[n])) { err = PTR_ERR(ve[n]); nctx = n;

@@ -3929,7 +3929,7 @@ static int mask_virtual_engine(struct intel_gt *gt, * restrict it to our desired engine within the virtual engine. */

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_close;

@@ -4060,7 +4060,7 @@ static int slicein_virtual_engine(struct intel_gt *gt, i915_request_add(rq); }

• ce = intel_engine_create_virtual(siblings, nsibling);

• ce = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ce)) { err = PTR_ERR(ce); goto out;

@@ -4112,7 +4112,7 @@ static int sliceout_virtual_engine(struct intel_gt *gt,

/* XXX We do not handle oversubscription and fairness with normal rq */ for (n = 0; n < nsibling; n++) {

• ce = intel_engine_create_virtual(siblings, nsibling);
  

• ce = intel_engine_create_virtual(siblings, nsibling, 0);
  

  if (IS_ERR(ce)) { err = PTR_ERR(ce); goto out;

@@ -4214,7 +4214,7 @@ static int preserved_virtual_engine(struct intel_gt *gt, if (err) goto out_scratch;

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_scratch;

@@ -4354,7 +4354,7 @@ static int reset_virtual_engine(struct intel_gt *gt, if (igt_spinner_init(&spin, gt)) return -ENOMEM;

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_spin;

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 013f36ef98cc..ecb938bb99fb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -124,7 +124,13 @@ struct guc_virtual_engine { };

static struct intel_context * -guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count); +guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

•    unsigned long flags);
  

+static struct intel_context * +guc_create_parallel(struct intel_engine_cs **engines,

•     unsigned int num_siblings,
  

•     unsigned int width);
  
  

  #define GUC_REQUEST_SIZE 64 /* bytes */

@@ -2615,6 +2621,7 @@ static const struct intel_context_ops guc_context_ops = { .destroy = guc_context_destroy,

.create_virtual = guc_create_virtual,

• .create_parallel = guc_create_parallel, };

  static void submit_work_cb(struct irq_work *wrk)

@@ -2864,8 +2871,6 @@ static const struct intel_context_ops virtual_guc_context_ops = { .get_sibling = guc_virtual_get_sibling, };

-/* Future patches will use this function */ -__maybe_unused static int guc_parent_context_pin(struct intel_context *ce, void *vaddr) { struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); @@ -2882,8 +2887,6 @@ static int guc_parent_context_pin(struct intel_context *ce, void *vaddr) return __guc_context_pin(ce, engine, vaddr); }

-/* Future patches will use this function */ -__maybe_unused static int guc_child_context_pin(struct intel_context *ce, void *vaddr) { struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); @@ -2895,8 +2898,6 @@ static int guc_child_context_pin(struct intel_context *ce, void *vaddr) return __guc_context_pin(ce, engine, vaddr); }

-/* Future patches will use this function */ -__maybe_unused static void guc_parent_context_unpin(struct intel_context *ce) { struct intel_guc *guc = ce_to_guc(ce); @@ -2912,8 +2913,6 @@ static void guc_parent_context_unpin(struct intel_context *ce) lrc_unpin(ce); }

-/* Future patches will use this function */ -__maybe_unused static void guc_child_context_unpin(struct intel_context *ce) { GEM_BUG_ON(context_enabled(ce)); @@ -2924,8 +2923,6 @@ static void guc_child_context_unpin(struct intel_context *ce) lrc_unpin(ce); }

-/* Future patches will use this function */ -__maybe_unused static void guc_child_context_post_unpin(struct intel_context *ce) { GEM_BUG_ON(!intel_context_is_child(ce)); @@ -2936,6 +2933,98 @@ static void guc_child_context_post_unpin(struct intel_context *ce) intel_context_unpin(ce->parallel.parent); }

+static void guc_child_context_destroy(struct kref *kref) +{

• struct intel_context *ce = container_of(kref, typeof(*ce), ref);
• __guc_context_destroy(ce);

+}

+static const struct intel_context_ops virtual_parent_context_ops = {

• .alloc = guc_virtual_context_alloc,
• .pre_pin = guc_context_pre_pin,
• .pin = guc_parent_context_pin,
• .unpin = guc_parent_context_unpin,
• .post_unpin = guc_context_post_unpin,
• .ban = guc_context_ban,
• .cancel_request = guc_context_cancel_request,
• .enter = guc_virtual_context_enter,
• .exit = guc_virtual_context_exit,
• .sched_disable = guc_context_sched_disable,
• .destroy = guc_context_destroy,
• .get_sibling = guc_virtual_get_sibling,

+};

+static const struct intel_context_ops virtual_child_context_ops = {

• .alloc = guc_virtual_context_alloc,
• .pre_pin = guc_context_pre_pin,
• .pin = guc_child_context_pin,
• .unpin = guc_child_context_unpin,
• .post_unpin = guc_child_context_post_unpin,
• .cancel_request = guc_context_cancel_request,
• .enter = guc_virtual_context_enter,
• .exit = guc_virtual_context_exit,
• .destroy = guc_child_context_destroy,
• .get_sibling = guc_virtual_get_sibling,

+};

+static struct intel_context * +guc_create_parallel(struct intel_engine_cs **engines,

•     unsigned int num_siblings,
  

•     unsigned int width)
  

+{

• struct intel_engine_cs **siblings = NULL;
• struct intel_context *parent = NULL, *ce, *err;
• int i, j;
• siblings = kmalloc_array(num_siblings,

• 		 sizeof(*siblings),
  

• 		 GFP_KERNEL);
  

• if (!siblings)

• return ERR_PTR(-ENOMEM);
  

• for (i = 0; i < width; ++i) {

• for (j = 0; j < num_siblings; ++j)
  

• 	siblings[j] = engines[i * num_siblings + j];
  

• ce = intel_engine_create_virtual(siblings, num_siblings,
  

• 				 FORCE_VIRTUAL);
  

• if (!ce) {
  

• 	err = ERR_PTR(-ENOMEM);
  

• 	goto unwind;
  

• }
  

• if (i == 0) {
  

• 	parent = ce;
  

• 	parent->ops = &virtual_parent_context_ops;
  

• } else {
  

• 	ce->ops = &virtual_child_context_ops;
  

• 	intel_context_bind_parent_child(parent, ce);
  

• }
  

• }
• kfree(siblings);
• return parent;

+unwind:

• if (parent)

• intel_context_put(parent);
  

• kfree(siblings);
• return err;

+}

• static bool guc_irq_enable_breadcrumbs(struct intel_breadcrumbs *b) {

@@ -3762,7 +3851,8 @@ void intel_guc_submission_print_context_info(struct intel_guc *guc, }

static struct intel_context * -guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count) +guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

•    unsigned long flags)
  

  { struct guc_virtual_engine *ve; struct intel_guc *guc;

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 0179f92e0916..c2a63e1584cb 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1824,6 +1824,7 @@ struct drm_i915_gem_context_param {

• Extensions:
• i915_context_engines_load_balance (I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE)
• i915_context_engines_bond (I915_CONTEXT_ENGINES_EXT_BOND)

• • i915_context_engines_parallel_submit (I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT)
  */ #define I915_CONTEXT_PARAM_ENGINES 0xa

@@ -2098,6 +2099,135 @@ struct i915_context_engines_bond { struct i915_engine_class_instance engines[N__]; \ } __attribute__((packed)) name__

+/**

• • struct i915_context_engines_parallel_submit - Configure engine for
• • parallel submission.
• • Setup a slot in the context engine map to allow multiple BBs to be submitted
• • in a single execbuf IOCTL. Those BBs will then be scheduled to run on the GPU
• • in parallel. Multiple hardware contexts are created internally in the i915 to
• • run these BBs. Once a slot is configured for N BBs only N BBs can be
• • submitted in each execbuf IOCTL and this is implicit behavior e.g. The user
• • doesn't tell the execbuf IOCTL there are N BBs, the execbuf IOCTL knows how
• • many BBs there are based on the slot's configuration. The N BBs are the last
• • N buffer objects or first N if I915_EXEC_BATCH_FIRST is set.
• • The default placement behavior is to create implicit bonds between each
• • context if each context maps to more than 1 physical engine (e.g. context is
• • a virtual engine). Also we only allow contexts of same engine class and these
• • contexts must be in logically contiguous order. Examples of the placement
• • behavior are described below. Lastly, the default is to not allow BBs to be
• • preempted mid-batch. Rather insert coordinated preemption points on all
• • hardware contexts between each set of BBs. Flags could be added in the future
• • to change both of these default behaviors.
• • Returns -EINVAL if hardware context placement configuration is invalid or if
• • the placement configuration isn't supported on the platform / submission
• • interface.
• • Returns -ENODEV if extension isn't supported on the platform / submission
• • interface.
• • .. code-block:: none
• • Examples syntax:
• • CS[X] = generic engine of same class, logical instance X
• • INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
• • Example 1 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=1,

• •      engines=CS[0],CS[1])
    

• • Results in the following valid placement:
• • CS[0], CS[1]
• • Example 2 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=2,

• •      engines=CS[0],CS[2],CS[1],CS[3])
    

• • Results in the following valid placements:
• • CS[0], CS[1]
• • CS[2], CS[3]
• • This can be thought of as two virtual engines, each containing two
• • engines thereby making a 2D array. However, there are bonds tying the
• • entries together and placing restrictions on how they can be scheduled.
• • Specifically, the scheduler can choose only vertical columns from the 2D
• • array. That is, CS[0] is bonded to CS[1] and CS[2] to CS[3]. So if the
• • scheduler wants to submit to CS[0], it must also choose CS[1] and vice
• • versa. Same for CS[2] requires also using CS[3].
• • VE[0] = CS[0], CS[2]
• • VE[1] = CS[1], CS[3]
• • Example 3 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=2,

• •      engines=CS[0],CS[1],CS[1],CS[3])
    

• • Results in the following valid and invalid placements:
• • CS[0], CS[1]
• • CS[1], CS[3] - Not logically contiguous, return -EINVAL
• */

+struct i915_context_engines_parallel_submit {

• /**

• * @base: base user extension.
  

• */
  

• struct i915_user_extension base;
• /**

• * @engine_index: slot for parallel engine
  

• */
  

• __u16 engine_index;
• /**

• * @width: number of contexts per parallel engine or in other words the
  

• * number of batches in each submission
  

• */
  

• __u16 width;
• /**

• * @num_siblings: number of siblings per context or in other words the
  

• * number of possible placements for each submission
  

• */
  

• __u16 num_siblings;
• /**

• * @mbz16: reserved for future use; must be zero
  

• */
  

• __u16 mbz16;
• /**

• * @flags: all undefined flags must be zero, currently not defined flags
  

• */
  

• __u64 flags;
• /**

• * @mbz64: reserved for future use; must be zero
  

• */
  

• __u64 mbz64[3];
• /**

• * @engines: 2-d array of engine instances to configure parallel engine
  

• *
  

• * length = width (i) * num_siblings (j)
  

• * index = j + i * num_siblings
  

• */
  

• struct i915_engine_class_instance engines[0];

+} __packed;

+#define I915_DEFINE_CONTEXT_ENGINES_PARALLEL_SUBMIT(name__, N__) struct { \

• struct i915_user_extension base; \
• __u16 engine_index; \
• __u16 width; \
• __u16 num_siblings; \
• __u16 mbz16; \
• __u64 flags; \
• __u64 mbz64[3]; \
• struct i915_engine_class_instance engines[N__]; \

+} __attribute__((packed)) name__

• /**
  • DOC: Context Engine Map uAPI

@@ -2157,6 +2287,7 @@ struct i915_context_param_engines { __u64 extensions; /* linked chain of extension blocks, 0 terminates */ #define I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE 0 /* see i915_context_engines_load_balance */ #define I915_CONTEXT_ENGINES_EXT_BOND 1 /* see i915_context_engines_bond */ +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see i915_context_engines_parallel_submit */ struct i915_engine_class_instance engines[0]; } __attribute__((packed));

On 10/14/2021 08:32, Matthew Brost wrote:

On Wed, Oct 13, 2021 at 06:02:42PM -0700, John Harrison wrote:

...

On 10/13/2021 13:42, Matthew Brost wrote:

...

Introduce 'set parallel submit' extension to connect UAPI to GuC multi-lrc interface. Kernel doc in new uAPI should explain it all.

IGT: https://patchwork.freedesktop.org/patch/447008/?series=93071&rev=1 media UMD: https://github.com/intel/media-driver/pull/1252

v2: (Daniel Vetter) - Add IGT link and placeholder for media UMD link v3: (Kernel test robot) - Fix warning in unpin engines call (John Harrison) - Reword a bunch of the kernel doc v4: (John Harrison) - Add comment why perma-pin is done after setting gem context - Update some comments / docs for proto contexts

Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_context.c | 228 +++++++++++++++++- .../gpu/drm/i915/gem/i915_gem_context_types.h | 16 +- drivers/gpu/drm/i915/gt/intel_context_types.h | 9 +- drivers/gpu/drm/i915/gt/intel_engine.h | 12 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 6 +- .../drm/i915/gt/intel_execlists_submission.c | 6 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 12 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 114 ++++++++- include/uapi/drm/i915_drm.h | 131 ++++++++++ 9 files changed, 503 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index d225d3dd0b40..6f23aff6e642 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -556,9 +556,150 @@ set_proto_ctx_engines_bond(struct i915_user_extension __user *base, void *data) return 0; } +static int +set_proto_ctx_engines_parallel_submit(struct i915_user_extension __user *base,

• 		      void *data)
  

+{

• struct i915_context_engines_parallel_submit __user *ext =

• container_of_user(base, typeof(*ext), base);
  

• const struct set_proto_ctx_engines *set = data;
• struct drm_i915_private *i915 = set->i915;
• u64 flags;
• int err = 0, n, i, j;
• u16 slot, width, num_siblings;
• struct intel_engine_cs **siblings = NULL;
• intel_engine_mask_t prev_mask;
• /* Disabling for now */
• return -ENODEV;
• /* FIXME: This is NIY for execlists */
• if (!(intel_uc_uses_guc_submission(&i915->gt.uc)))

• return -ENODEV;
  

• if (get_user(slot, &ext->engine_index))

• return -EFAULT;
  

• if (get_user(width, &ext->width))

• return -EFAULT;
  

• if (get_user(num_siblings, &ext->num_siblings))

• return -EFAULT;
  

• if (slot >= set->num_engines) {

• drm_dbg(&i915->drm, "Invalid placement value, %d >= %d\n",
  

• 	slot, set->num_engines);
  

• return -EINVAL;
  

• }
• if (set->engines[slot].type != I915_GEM_ENGINE_TYPE_INVALID) {

• drm_dbg(&i915->drm,
  

• 	"Invalid placement[%d], already occupied\n", slot);
  

• return -EINVAL;
  

• }
• if (get_user(flags, &ext->flags))

• return -EFAULT;
  

• if (flags) {

• drm_dbg(&i915->drm, "Unknown flags 0x%02llx", flags);
  

• return -EINVAL;
  

• }
• for (n = 0; n < ARRAY_SIZE(ext->mbz64); n++) {

• err = check_user_mbz(&ext->mbz64[n]);
  

• if (err)
  

• 	return err;
  

• }
• if (width < 2) {

• drm_dbg(&i915->drm, "Width (%d) < 2\n", width);
  

• return -EINVAL;
  

• }
• if (num_siblings < 1) {

• drm_dbg(&i915->drm, "Number siblings (%d) < 1\n",
  

• 	num_siblings);
  

• return -EINVAL;
  

• }
• siblings = kmalloc_array(num_siblings * width,

• 		 sizeof(*siblings),
  

• 		 GFP_KERNEL);
  

• if (!siblings)

• return -ENOMEM;
  

• /* Create contexts / engines */
• for (i = 0; i < width; ++i) {

• intel_engine_mask_t current_mask = 0;
  

• struct i915_engine_class_instance prev_engine;
  

• for (j = 0; j < num_siblings; ++j) {
  

• 	struct i915_engine_class_instance ci;
  

• 	n = i * num_siblings + j;
  

• 	if (copy_from_user(&ci, &ext->engines[n], sizeof(ci))) {
  

• 		err = -EFAULT;
  

• 		goto out_err;
  

• 	}
  

• 	siblings[n] =
  

• 		intel_engine_lookup_user(i915, ci.engine_class,
  

• 					 ci.engine_instance);
  

• 	if (!siblings[n]) {
  

• 		drm_dbg(&i915->drm,
  

• 			"Invalid sibling[%d]: { class:%d, inst:%d }\n",
  

• 			n, ci.engine_class, ci.engine_instance);
  

• 		err = -EINVAL;
  

• 		goto out_err;
  

• 	}
  

• 	if (n) {
  

• 		if (prev_engine.engine_class !=
  

• 		    ci.engine_class) {
  

• 			drm_dbg(&i915->drm,
  

• 				"Mismatched class %d, %d\n",
  

• 				prev_engine.engine_class,
  

• 				ci.engine_class);
  

• 			err = -EINVAL;
  

• 			goto out_err;
  

• 		}
  

• 	}
  

• 	prev_engine = ci;
  

• 	current_mask |= siblings[n]->logical_mask;
  

• }
  

• if (i > 0) {
  

• 	if (current_mask != prev_mask << 1) {
  

• 		drm_dbg(&i915->drm,
  

• 			"Non contiguous logical mask 0x%x, 0x%x\n",
  

• 			prev_mask, current_mask);
  

• 		err = -EINVAL;
  

• 		goto out_err;
  

• 	}
  

• }
  

• prev_mask = current_mask;
  

• }
• set->engines[slot].type = I915_GEM_ENGINE_TYPE_PARALLEL;
• set->engines[slot].num_siblings = num_siblings;
• set->engines[slot].width = width;
• set->engines[slot].siblings = siblings;
• return 0;

+out_err:

• kfree(siblings);
• return err;

+}

• static const i915_user_extension_fn set_proto_ctx_engines_extensions[] = { [I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE] = set_proto_ctx_engines_balance, [I915_CONTEXT_ENGINES_EXT_BOND] = set_proto_ctx_engines_bond,
• [I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT] =

• set_proto_ctx_engines_parallel_submit,
  

  }; static int set_proto_ctx_engines(struct drm_i915_file_private *fpriv,

@@ -818,6 +959,25 @@ static int intel_context_set_gem(struct intel_context *ce, return ret; } +static void __unpin_engines(struct i915_gem_engines *e, unsigned int count) +{

• while (count--) {

• struct intel_context *ce = e->engines[count], *child;
  

• if (!ce || !test_bit(CONTEXT_PERMA_PIN, &ce->flags))
  

• 	continue;
  

• for_each_child(ce, child)
  

• 	intel_context_unpin(child);
  

• intel_context_unpin(ce);
  

• }

+}

+static void unpin_engines(struct i915_gem_engines *e) +{

• __unpin_engines(e, e->num_engines);

+}

• static void __free_engines(struct i915_gem_engines *e, unsigned int count) { while (count--) {

@@ -933,6 +1093,40 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx, return err; } +static int perma_pin_contexts(struct intel_context *ce) +{

• struct intel_context *child;
• int i = 0, j = 0, ret;
• GEM_BUG_ON(!intel_context_is_parent(ce));
• ret = intel_context_pin(ce);
• if (unlikely(ret))

• return ret;
  

• for_each_child(ce, child) {

• ret = intel_context_pin(child);
  

• if (unlikely(ret))
  

• 	goto unwind;
  

• ++i;
  

• }
• set_bit(CONTEXT_PERMA_PIN, &ce->flags);
• return 0;

+unwind:

• intel_context_unpin(ce);
• for_each_child(ce, child) {

• if (j++ < i)
  

• 	intel_context_unpin(child);
  

• else
  

• 	break;
  

• }
• return ret;

+}

• static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, unsigned int num_engines, struct i915_gem_proto_engine *pe)

@@ -946,7 +1140,7 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, e->num_engines = num_engines; for (n = 0; n < num_engines; n++) {

• struct intel_context *ce;
  

• struct intel_context *ce, *child;
  int ret;
  switch (pe[n].type) {
  

@@ -956,7 +1150,13 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, case I915_GEM_ENGINE_TYPE_BALANCED: ce = intel_engine_create_virtual(pe[n].siblings,

• 					 pe[n].num_siblings);
  

• 					 pe[n].num_siblings, 0);
  

• 	break;
  

• case I915_GEM_ENGINE_TYPE_PARALLEL:
  

• 	ce = intel_engine_create_parallel(pe[n].siblings,
  

• 					  pe[n].num_siblings,
  

• 					  pe[n].width);
  	break;
  case I915_GEM_ENGINE_TYPE_INVALID:
  

@@ -977,6 +1177,29 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, err = ERR_PTR(ret); goto free_engines; }

• for_each_child(ce, child) {
  

• 	ret = intel_context_set_gem(child, ctx, pe->sseu);
  

• 	if (ret) {
  

• 		err = ERR_PTR(ret);
  

• 		goto free_engines;
  

• 	}
  

• }
  

• /*
  

•  * XXX: Must be done after calling intel_context_set_gem as that
  

•  * function changes the ring size while the ring is allocated
  

•  * when the context is pinned. Mismatch the ring size will cause
  

while... when... is ambiguous - changes the ring size at the point when the context is pinned even though the ring is already allocated? Or changes the ring size even though the ring is already allocated and the context is already pinned? Something else?

The default size is 4k, intel_context_set_gem changes the size the 16k. If the pin is done before intel_context_set_gem the ring is allocated at 4k, then intel_context_set_gem changes the size to 16k == mismatch and hangs.

Right. But, what does 'changes the ring size while the ring is allocated when the context is pinned' mean? That sentence needs to be split up / re-worded because it is not possible to parse unambiguously.

John.

...

Mismatch the ring -> A mismatch of the ring

Yep.

Matt

...

John.

...

•  * the context to hang. Presumably with a bit of reordering we
  

•  * could move the perma-pin step to the backend function
  

•  * intel_engine_create_parallel.
  

•  */
  

• if (pe[n].type == I915_GEM_ENGINE_TYPE_PARALLEL) {
  

• 	ret = perma_pin_contexts(ce);
  

• 	if (ret) {
  

• 		err = ERR_PTR(ret);
  

• 		goto free_engines;
  

• 	}
  

• }
  

  } return e;

@@ -1219,6 +1442,7 @@ static void context_close(struct i915_gem_context *ctx) /* Flush any concurrent set_engines() */ mutex_lock(&ctx->engines_mutex);

• unpin_engines(__context_engines_static(ctx)); engines_idle_release(ctx, rcu_replace_pointer(ctx->engines, NULL, 1)); i915_gem_context_set_closed(ctx); mutex_unlock(&ctx->engines_mutex);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index a627b09c4680..282cdb8a5c5a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -78,13 +78,16 @@ enum i915_gem_engine_type { /** @I915_GEM_ENGINE_TYPE_BALANCED: A load-balanced engine set */ I915_GEM_ENGINE_TYPE_BALANCED,

• /** @I915_GEM_ENGINE_TYPE_PARALLEL: A parallel engine set */
• I915_GEM_ENGINE_TYPE_PARALLEL, }; /**
  • struct i915_gem_proto_engine - prototype engine
  • This struct describes an engine that a context may contain. Engines

• • have three types:

• • have four types:
    • • I915_GEM_ENGINE_TYPE_INVALID: Invalid engines can be created but they
    • show up as a NULL in i915_gem_engines::engines[i] and any attempt to

@@ -97,6 +100,10 @@ enum i915_gem_engine_type { * * - I915_GEM_ENGINE_TYPE_BALANCED: A load-balanced engine set, described * i915_gem_proto_engine::num_siblings and i915_gem_proto_engine::siblings.

• • • I915_GEM_ENGINE_TYPE_PARALLEL: A parallel submission engine set, described
• • i915_gem_proto_engine::width, i915_gem_proto_engine::num_siblings, and
• • i915_gem_proto_engine::siblings. */
  struct i915_gem_proto_engine { /** @type: Type of this engine */

@@ -105,10 +112,13 @@ struct i915_gem_proto_engine { /** @engine: Engine, for physical */ struct intel_engine_cs *engine;

• /** @num_siblings: Number of balanced siblings */

• /** @num_siblings: Number of balanced or parallel siblings */ unsigned int num_siblings;

• /** @siblings: Balanced siblings */

• /** @width: Width of each sibling */
• unsigned int width;
• /** @siblings: Balanced siblings or num_siblings * width for parallel */ struct intel_engine_cs **siblings; /** @sseu: Client-set SSEU parameters */

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 8309d1141d0a..1d880303a7e4 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -55,9 +55,13 @@ struct intel_context_ops { void (*reset)(struct intel_context *ce); void (*destroy)(struct kref *kref);

• /* virtual engine/context interface */

• /* virtual/parallel engine/context interface */ struct intel_context *(*create_virtual)(struct intel_engine_cs **engine,

• 				unsigned int count);
  

• 				unsigned int count,
  

• 				unsigned long flags);
  

• struct intel_context *(*create_parallel)(struct intel_engine_cs **engines,

• 				 unsigned int num_siblings,
  

• 				 unsigned int width);
  

  struct intel_engine_cs *(*get_sibling)(struct intel_engine_cs *engine, unsigned int sibling); };

@@ -113,6 +117,7 @@ struct intel_context { #define CONTEXT_NOPREEMPT 8 #define CONTEXT_LRCA_DIRTY 9 #define CONTEXT_GUC_INIT 10 +#define CONTEXT_PERMA_PIN 11 struct { u64 timeout_us; diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index d5ac49c0691e..08559ace0ada 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -282,9 +282,19 @@ intel_engine_has_preempt_reset(const struct intel_engine_cs *engine) return intel_engine_has_preemption(engine); } +#define FORCE_VIRTUAL BIT(0) struct intel_context * intel_engine_create_virtual(struct intel_engine_cs **siblings,

• 	    unsigned int count);
  

• 	    unsigned int count, unsigned long flags);
  

+static inline struct intel_context * +intel_engine_create_parallel(struct intel_engine_cs **engines,

• 	     unsigned int num_engines,
  

• 	     unsigned int width)
  

+{

• GEM_BUG_ON(!engines[0]->cops->create_parallel);
• return engines[0]->cops->create_parallel(engines, num_engines, width);

+} static inline bool intel_virtual_engine_has_heartbeat(const struct intel_engine_cs *engine) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2eb798ad068b..ff6753ccb129 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1953,16 +1953,16 @@ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine, ktime_t *now) struct intel_context * intel_engine_create_virtual(struct intel_engine_cs **siblings,

• 	    unsigned int count)
  

• 	    unsigned int count, unsigned long flags)
  

  { if (count == 0) return ERR_PTR(-EINVAL);

• if (count == 1)

• if (count == 1 && !(flags & FORCE_VIRTUAL)) return intel_context_create(siblings[0]); GEM_BUG_ON(!siblings[0]->cops->create_virtual);

• return siblings[0]->cops->create_virtual(siblings, count);

• return siblings[0]->cops->create_virtual(siblings, count, flags); } struct i915_request *

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index 43a74b216efb..bedb80057046 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -201,7 +201,8 @@ static struct virtual_engine *to_virtual_engine(struct intel_engine_cs *engine) } static struct intel_context * -execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count); +execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

• 	 unsigned long flags);
  

  static struct i915_request * __active_request(const struct intel_timeline * const tl,

@@ -3784,7 +3785,8 @@ static void virtual_submit_request(struct i915_request *rq) } static struct intel_context * -execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count) +execlists_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

• 	 unsigned long flags)
  

  { struct virtual_engine *ve; unsigned int n;

diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index 25a8c4f62b0d..b367ecfa42de 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -3733,7 +3733,7 @@ static int nop_virtual_engine(struct intel_gt *gt, GEM_BUG_ON(!nctx || nctx > ARRAY_SIZE(ve)); for (n = 0; n < nctx; n++) {

• ve[n] = intel_engine_create_virtual(siblings, nsibling);
  

• ve[n] = intel_engine_create_virtual(siblings, nsibling, 0);
  if (IS_ERR(ve[n])) {
  	err = PTR_ERR(ve[n]);
  	nctx = n;
  

@@ -3929,7 +3929,7 @@ static int mask_virtual_engine(struct intel_gt *gt, * restrict it to our desired engine within the virtual engine. */

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_close;

@@ -4060,7 +4060,7 @@ static int slicein_virtual_engine(struct intel_gt *gt, i915_request_add(rq); }

• ce = intel_engine_create_virtual(siblings, nsibling);

• ce = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ce)) { err = PTR_ERR(ce); goto out;

@@ -4112,7 +4112,7 @@ static int sliceout_virtual_engine(struct intel_gt *gt, /* XXX We do not handle oversubscription and fairness with normal rq */ for (n = 0; n < nsibling; n++) {

• ce = intel_engine_create_virtual(siblings, nsibling);
  

• ce = intel_engine_create_virtual(siblings, nsibling, 0);
  if (IS_ERR(ce)) {
  	err = PTR_ERR(ce);
  	goto out;
  

@@ -4214,7 +4214,7 @@ static int preserved_virtual_engine(struct intel_gt *gt, if (err) goto out_scratch;

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_scratch;

@@ -4354,7 +4354,7 @@ static int reset_virtual_engine(struct intel_gt *gt, if (igt_spinner_init(&spin, gt)) return -ENOMEM;

• ve = intel_engine_create_virtual(siblings, nsibling);

• ve = intel_engine_create_virtual(siblings, nsibling, 0); if (IS_ERR(ve)) { err = PTR_ERR(ve); goto out_spin;

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 013f36ef98cc..ecb938bb99fb 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -124,7 +124,13 @@ struct guc_virtual_engine { }; static struct intel_context * -guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count); +guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

•    unsigned long flags);
  

+static struct intel_context * +guc_create_parallel(struct intel_engine_cs **engines,

•     unsigned int num_siblings,
  

•     unsigned int width);
  

  #define GUC_REQUEST_SIZE 64 /* bytes */

@@ -2615,6 +2621,7 @@ static const struct intel_context_ops guc_context_ops = { .destroy = guc_context_destroy, .create_virtual = guc_create_virtual,

• .create_parallel = guc_create_parallel, }; static void submit_work_cb(struct irq_work *wrk)

@@ -2864,8 +2871,6 @@ static const struct intel_context_ops virtual_guc_context_ops = { .get_sibling = guc_virtual_get_sibling, }; -/* Future patches will use this function */ -__maybe_unused static int guc_parent_context_pin(struct intel_context *ce, void *vaddr) { struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); @@ -2882,8 +2887,6 @@ static int guc_parent_context_pin(struct intel_context *ce, void *vaddr) return __guc_context_pin(ce, engine, vaddr); } -/* Future patches will use this function */ -__maybe_unused static int guc_child_context_pin(struct intel_context *ce, void *vaddr) { struct intel_engine_cs *engine = guc_virtual_get_sibling(ce->engine, 0); @@ -2895,8 +2898,6 @@ static int guc_child_context_pin(struct intel_context *ce, void *vaddr) return __guc_context_pin(ce, engine, vaddr); } -/* Future patches will use this function */ -__maybe_unused static void guc_parent_context_unpin(struct intel_context *ce) { struct intel_guc *guc = ce_to_guc(ce); @@ -2912,8 +2913,6 @@ static void guc_parent_context_unpin(struct intel_context *ce) lrc_unpin(ce); } -/* Future patches will use this function */ -__maybe_unused static void guc_child_context_unpin(struct intel_context *ce) { GEM_BUG_ON(context_enabled(ce)); @@ -2924,8 +2923,6 @@ static void guc_child_context_unpin(struct intel_context *ce) lrc_unpin(ce); } -/* Future patches will use this function */ -__maybe_unused static void guc_child_context_post_unpin(struct intel_context *ce) { GEM_BUG_ON(!intel_context_is_child(ce)); @@ -2936,6 +2933,98 @@ static void guc_child_context_post_unpin(struct intel_context *ce) intel_context_unpin(ce->parallel.parent); } +static void guc_child_context_destroy(struct kref *kref) +{

• struct intel_context *ce = container_of(kref, typeof(*ce), ref);
• __guc_context_destroy(ce);

+}

+static const struct intel_context_ops virtual_parent_context_ops = {

• .alloc = guc_virtual_context_alloc,
• .pre_pin = guc_context_pre_pin,
• .pin = guc_parent_context_pin,
• .unpin = guc_parent_context_unpin,
• .post_unpin = guc_context_post_unpin,
• .ban = guc_context_ban,
• .cancel_request = guc_context_cancel_request,
• .enter = guc_virtual_context_enter,
• .exit = guc_virtual_context_exit,
• .sched_disable = guc_context_sched_disable,
• .destroy = guc_context_destroy,
• .get_sibling = guc_virtual_get_sibling,

+};

+static const struct intel_context_ops virtual_child_context_ops = {

• .alloc = guc_virtual_context_alloc,
• .pre_pin = guc_context_pre_pin,
• .pin = guc_child_context_pin,
• .unpin = guc_child_context_unpin,
• .post_unpin = guc_child_context_post_unpin,
• .cancel_request = guc_context_cancel_request,
• .enter = guc_virtual_context_enter,
• .exit = guc_virtual_context_exit,
• .destroy = guc_child_context_destroy,
• .get_sibling = guc_virtual_get_sibling,

+};

+static struct intel_context * +guc_create_parallel(struct intel_engine_cs **engines,

•     unsigned int num_siblings,
  

•     unsigned int width)
  

+{

• struct intel_engine_cs **siblings = NULL;
• struct intel_context *parent = NULL, *ce, *err;
• int i, j;
• siblings = kmalloc_array(num_siblings,

• 		 sizeof(*siblings),
  

• 		 GFP_KERNEL);
  

• if (!siblings)

• return ERR_PTR(-ENOMEM);
  

• for (i = 0; i < width; ++i) {

• for (j = 0; j < num_siblings; ++j)
  

• 	siblings[j] = engines[i * num_siblings + j];
  

• ce = intel_engine_create_virtual(siblings, num_siblings,
  

• 				 FORCE_VIRTUAL);
  

• if (!ce) {
  

• 	err = ERR_PTR(-ENOMEM);
  

• 	goto unwind;
  

• }
  

• if (i == 0) {
  

• 	parent = ce;
  

• 	parent->ops = &virtual_parent_context_ops;
  

• } else {
  

• 	ce->ops = &virtual_child_context_ops;
  

• 	intel_context_bind_parent_child(parent, ce);
  

• }
  

• }
• kfree(siblings);
• return parent;

+unwind:

• if (parent)

• intel_context_put(parent);
  

• kfree(siblings);
• return err;

+}

• static bool guc_irq_enable_breadcrumbs(struct intel_breadcrumbs *b) {

@@ -3762,7 +3851,8 @@ void intel_guc_submission_print_context_info(struct intel_guc *guc, } static struct intel_context * -guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count) +guc_create_virtual(struct intel_engine_cs **siblings, unsigned int count,

•    unsigned long flags)
  

  { struct guc_virtual_engine *ve; struct intel_guc *guc;

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 0179f92e0916..c2a63e1584cb 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1824,6 +1824,7 @@ struct drm_i915_gem_context_param { * Extensions: * i915_context_engines_load_balance (I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE) * i915_context_engines_bond (I915_CONTEXT_ENGINES_EXT_BOND)

• • i915_context_engines_parallel_submit (I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT) */
  #define I915_CONTEXT_PARAM_ENGINES 0xa

@@ -2098,6 +2099,135 @@ struct i915_context_engines_bond { struct i915_engine_class_instance engines[N__]; \ } __attribute__((packed)) name__ +/**

• • struct i915_context_engines_parallel_submit - Configure engine for
• • parallel submission.
• • Setup a slot in the context engine map to allow multiple BBs to be submitted
• • in a single execbuf IOCTL. Those BBs will then be scheduled to run on the GPU
• • in parallel. Multiple hardware contexts are created internally in the i915 to
• • run these BBs. Once a slot is configured for N BBs only N BBs can be
• • submitted in each execbuf IOCTL and this is implicit behavior e.g. The user
• • doesn't tell the execbuf IOCTL there are N BBs, the execbuf IOCTL knows how
• • many BBs there are based on the slot's configuration. The N BBs are the last
• • N buffer objects or first N if I915_EXEC_BATCH_FIRST is set.
• • The default placement behavior is to create implicit bonds between each
• • context if each context maps to more than 1 physical engine (e.g. context is
• • a virtual engine). Also we only allow contexts of same engine class and these
• • contexts must be in logically contiguous order. Examples of the placement
• • behavior are described below. Lastly, the default is to not allow BBs to be
• • preempted mid-batch. Rather insert coordinated preemption points on all
• • hardware contexts between each set of BBs. Flags could be added in the future
• • to change both of these default behaviors.
• • Returns -EINVAL if hardware context placement configuration is invalid or if
• • the placement configuration isn't supported on the platform / submission
• • interface.
• • Returns -ENODEV if extension isn't supported on the platform / submission
• • interface.
• • .. code-block:: none
• • Examples syntax:
• • CS[X] = generic engine of same class, logical instance X
• • INVALID = I915_ENGINE_CLASS_INVALID, I915_ENGINE_CLASS_INVALID_NONE
• • Example 1 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=1,

• •      engines=CS[0],CS[1])
    

• • Results in the following valid placement:
• • CS[0], CS[1]
• • Example 2 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=2,

• •      engines=CS[0],CS[2],CS[1],CS[3])
    

• • Results in the following valid placements:
• • CS[0], CS[1]
• • CS[2], CS[3]
• • This can be thought of as two virtual engines, each containing two
• • engines thereby making a 2D array. However, there are bonds tying the
• • entries together and placing restrictions on how they can be scheduled.
• • Specifically, the scheduler can choose only vertical columns from the 2D
• • array. That is, CS[0] is bonded to CS[1] and CS[2] to CS[3]. So if the
• • scheduler wants to submit to CS[0], it must also choose CS[1] and vice
• • versa. Same for CS[2] requires also using CS[3].
• • VE[0] = CS[0], CS[2]
• • VE[1] = CS[1], CS[3]
• • Example 3 pseudo code:
• • set_engines(INVALID)
• • set_parallel(engine_index=0, width=2, num_siblings=2,

• •      engines=CS[0],CS[1],CS[1],CS[3])
    

• • Results in the following valid and invalid placements:
• • CS[0], CS[1]
• • CS[1], CS[3] - Not logically contiguous, return -EINVAL
• */

+struct i915_context_engines_parallel_submit {

• /**

• * @base: base user extension.
  

• */
  

• struct i915_user_extension base;
• /**

• * @engine_index: slot for parallel engine
  

• */
  

• __u16 engine_index;
• /**

• * @width: number of contexts per parallel engine or in other words the
  

• * number of batches in each submission
  

• */
  

• __u16 width;
• /**

• * @num_siblings: number of siblings per context or in other words the
  

• * number of possible placements for each submission
  

• */
  

• __u16 num_siblings;
• /**

• * @mbz16: reserved for future use; must be zero
  

• */
  

• __u16 mbz16;
• /**

• * @flags: all undefined flags must be zero, currently not defined flags
  

• */
  

• __u64 flags;
• /**

• * @mbz64: reserved for future use; must be zero
  

• */
  

• __u64 mbz64[3];
• /**

• * @engines: 2-d array of engine instances to configure parallel engine
  

• *
  

• * length = width (i) * num_siblings (j)
  

• * index = j + i * num_siblings
  

• */
  

• struct i915_engine_class_instance engines[0];

+} __packed;

+#define I915_DEFINE_CONTEXT_ENGINES_PARALLEL_SUBMIT(name__, N__) struct { \

• struct i915_user_extension base; \
• __u16 engine_index; \
• __u16 width; \
• __u16 num_siblings; \
• __u16 mbz16; \
• __u64 flags; \
• __u64 mbz64[3]; \
• struct i915_engine_class_instance engines[N__]; \

+} __attribute__((packed)) name__

• /**
  • DOC: Context Engine Map uAPI

@@ -2157,6 +2287,7 @@ struct i915_context_param_engines { __u64 extensions; /* linked chain of extension blocks, 0 terminates */ #define I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE 0 /* see i915_context_engines_load_balance */ #define I915_CONTEXT_ENGINES_EXT_BOND 1 /* see i915_context_engines_bond */ +#define I915_CONTEXT_ENGINES_EXT_PARALLEL_SUBMIT 2 /* see i915_context_engines_parallel_submit */ struct i915_engine_class_instance engines[0]; } __attribute__((packed));