Message ID | 20210716201724.54804-27-matthew.brost@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | GuC submission support | expand |
On 7/16/2021 13:16, Matthew Brost wrote: > Reset implementation for new GuC interface. This is the legacy reset > implementation which is called when the i915 owns the engine hang check. > Future patches will offload the engine hang check to GuC but we will > continue to maintain this legacy path as a fallback and this code path > is also required if the GuC dies. > > With the new GuC interface it is not possible to reset individual > engines - it is only possible to reset the GPU entirely. This patch > forces an entire chip reset if any engine hangs. > > v2: > (Michal) > - Check for -EPIPE rather than -EIO (CT deadlock/corrupt check) > v3: > (John H) > - Split into a series of smaller patches While the split happened, it doesn't look like any of the other comments were address. Repeated below for clarity. Also, Tvrtko has a bunch of outstanding comments too. > > Cc: John Harrison <john.c.harrison@intel.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_gt_pm.c | 6 +- > drivers/gpu/drm/i915/gt/intel_reset.c | 18 +- > drivers/gpu/drm/i915/gt/uc/intel_guc.c | 13 - > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +- > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 562 ++++++++++++++---- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 39 +- > drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 + > 7 files changed, 515 insertions(+), 134 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > index aef3084e8b16..463a6ae605a0 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > @@ -174,8 +174,6 @@ static void gt_sanitize(struct intel_gt *gt, bool force) > if (intel_gt_is_wedged(gt)) > intel_gt_unset_wedged(gt); > > - intel_uc_sanitize(>->uc); > - > for_each_engine(engine, gt, id) > if (engine->reset.prepare) > engine->reset.prepare(engine); > @@ -191,6 +189,8 @@ static void gt_sanitize(struct intel_gt *gt, bool force) > __intel_engine_reset(engine, false); > } > > + intel_uc_reset(>->uc, false); > + > for_each_engine(engine, gt, id) > if (engine->reset.finish) > engine->reset.finish(engine); > @@ -243,6 +243,8 @@ int intel_gt_resume(struct intel_gt *gt) > goto err_wedged; > } > > + intel_uc_reset_finish(>->uc); > + > intel_rps_enable(>->rps); > intel_llc_enable(>->llc); > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > index 72251638d4ea..2987282dff6d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > @@ -826,6 +826,8 @@ static int gt_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask) > __intel_engine_reset(engine, stalled_mask & engine->mask); > local_bh_enable(); > > + intel_uc_reset(>->uc, true); > + > intel_ggtt_restore_fences(gt->ggtt); > > return err; > @@ -850,6 +852,8 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake) > if (awake & engine->mask) > intel_engine_pm_put(engine); > } > + > + intel_uc_reset_finish(>->uc); > } > > static void nop_submit_request(struct i915_request *request) > @@ -903,6 +907,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt) > for_each_engine(engine, gt, id) > if (engine->reset.cancel) > engine->reset.cancel(engine); > + intel_uc_cancel_requests(>->uc); > local_bh_enable(); > > reset_finish(gt, awake); > @@ -1191,6 +1196,9 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) > ENGINE_TRACE(engine, "flags=%lx\n", gt->reset.flags); > GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, >->reset.flags)); > > + if (intel_engine_uses_guc(engine)) > + return -ENODEV; > + > if (!intel_engine_pm_get_if_awake(engine)) > return 0; > > @@ -1201,13 +1209,10 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) > "Resetting %s for %s\n", engine->name, msg); > atomic_inc(&engine->i915->gpu_error.reset_engine_count[engine->uabi_class]); > > - if (intel_engine_uses_guc(engine)) > - ret = intel_guc_reset_engine(&engine->gt->uc.guc, engine); > - else > - ret = intel_gt_reset_engine(engine); > + ret = intel_gt_reset_engine(engine); > if (ret) { > /* If we fail here, we expect to fallback to a global reset */ > - ENGINE_TRACE(engine, "Failed to reset, err: %d\n", ret); > + ENGINE_TRACE(engine, "Failed to reset %s, err: %d\n", engine->name, ret); > goto out; > } > > @@ -1341,7 +1346,8 @@ void intel_gt_handle_error(struct intel_gt *gt, > * Try engine reset when available. We fall back to full reset if > * single reset fails. > */ > - if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { > + if (!intel_uc_uses_guc_submission(>->uc) && > + intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { > local_bh_disable(); > for_each_engine_masked(engine, gt, engine_mask, tmp) { > BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > index 6661dcb02239..9b09395b998f 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > @@ -572,19 +572,6 @@ int intel_guc_suspend(struct intel_guc *guc) > return 0; > } > > -/** > - * intel_guc_reset_engine() - ask GuC to reset an engine > - * @guc: intel_guc structure > - * @engine: engine to be reset > - */ > -int intel_guc_reset_engine(struct intel_guc *guc, > - struct intel_engine_cs *engine) > -{ > - /* XXX: to be implemented with submission interface rework */ > - > - return -ENODEV; > -} > - > /** > * intel_guc_resume() - notify GuC resuming from suspend state > * @guc: the guc > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > index 3cc566565224..d75a76882a44 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > @@ -242,14 +242,16 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask) > > int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout); > > -int intel_guc_reset_engine(struct intel_guc *guc, > - struct intel_engine_cs *engine); > - > int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > const u32 *msg, u32 len); > int intel_guc_sched_done_process_msg(struct intel_guc *guc, > const u32 *msg, u32 len); > > +void intel_guc_submission_reset_prepare(struct intel_guc *guc); > +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled); > +void intel_guc_submission_reset_finish(struct intel_guc *guc); > +void intel_guc_submission_cancel_requests(struct intel_guc *guc); > + > void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p); > > #endif > 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 f8a6077fa3f9..0f5823922369 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -141,7 +141,7 @@ context_wait_for_deregister_to_register(struct intel_context *ce) > static inline void > set_context_wait_for_deregister_to_register(struct intel_context *ce) > { > - /* Only should be called from guc_lrc_desc_pin() */ > + /* Only should be called from guc_lrc_desc_pin() without lock */ > ce->guc_state.sched_state |= > SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER; > } > @@ -241,15 +241,31 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc) > > static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) > { > + guc->lrc_desc_pool_vaddr = NULL; > i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); > } > > +static inline bool guc_submission_initialized(struct intel_guc *guc) > +{ > + return guc->lrc_desc_pool_vaddr != NULL; > +} > + > static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) > { > - struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > + if (likely(guc_submission_initialized(guc))) { > + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > + unsigned long flags; > > - memset(desc, 0, sizeof(*desc)); > - xa_erase_irq(&guc->context_lookup, id); > + memset(desc, 0, sizeof(*desc)); > + > + /* > + * xarray API doesn't have xa_erase_irqsave wrapper, so calling > + * the lower level functions directly. > + */ > + xa_lock_irqsave(&guc->context_lookup, flags); > + __xa_erase(&guc->context_lookup, id); > + xa_unlock_irqrestore(&guc->context_lookup, flags); > + } > } > > static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > @@ -260,7 +276,15 @@ static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, > struct intel_context *ce) > { > - xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); > + unsigned long flags; > + > + /* > + * xarray API doesn't have xa_save_irqsave wrapper, so calling the > + * lower level functions directly. > + */ > + xa_lock_irqsave(&guc->context_lookup, flags); > + __xa_store(&guc->context_lookup, id, ce, GFP_ATOMIC); > + xa_unlock_irqrestore(&guc->context_lookup, flags); > } > > static int guc_submission_send_busy_loop(struct intel_guc* guc, > @@ -326,6 +350,8 @@ int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout) > true, timeout); > } > > +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop); > + > static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > { > int err; > @@ -333,11 +359,22 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > u32 action[3]; > int len = 0; > u32 g2h_len_dw = 0; > - bool enabled = context_enabled(ce); > + bool enabled; > > GEM_BUG_ON(!atomic_read(&ce->guc_id_ref)); > GEM_BUG_ON(context_guc_id_invalid(ce)); > > + /* > + * Corner case where the GuC firmware was blown away and reloaded while > + * this context was pinned. > + */ > + if (unlikely(!lrc_desc_registered(guc, ce->guc_id))) { > + err = guc_lrc_desc_pin(ce, false); > + if (unlikely(err)) > + goto out; > + } > + enabled = context_enabled(ce); > + > if (!enabled) { > action[len++] = INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET; > action[len++] = ce->guc_id; > @@ -360,6 +397,7 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > intel_context_put(ce); > } > > +out: > return err; > } > > @@ -414,15 +452,10 @@ static int guc_dequeue_one_context(struct intel_guc *guc) > if (submit) { > guc_set_lrc_tail(last); > resubmit: > - /* > - * We only check for -EBUSY here even though it is possible for > - * -EDEADLK to be returned. If -EDEADLK is returned, the GuC has > - * died and a full GT reset needs to be done. The hangcheck will > - * eventually detect that the GuC has died and trigger this > - * reset so no need to handle -EDEADLK here. > - */ > ret = guc_add_request(guc, last); > - if (ret == -EBUSY) { > + if (unlikely(ret == -EPIPE)) > + goto deadlk; > + else if (ret == -EBUSY) { > tasklet_schedule(&sched_engine->tasklet); > guc->stalled_request = last; > return false; > @@ -432,6 +465,11 @@ static int guc_dequeue_one_context(struct intel_guc *guc) > > guc->stalled_request = NULL; > return submit; > + > +deadlk: > + sched_engine->tasklet.callback = NULL; > + tasklet_disable_nosync(&sched_engine->tasklet); > + return false; > } > > static void guc_submission_tasklet(struct tasklet_struct *t) > @@ -458,27 +496,166 @@ static void cs_irq_handler(struct intel_engine_cs *engine, u16 iir) > intel_engine_signal_breadcrumbs(engine); > } > > -static void guc_reset_prepare(struct intel_engine_cs *engine) > +static void __guc_context_destroy(struct intel_context *ce); > +static void release_guc_id(struct intel_guc *guc, struct intel_context *ce); > +static void guc_signal_context_fence(struct intel_context *ce); > + > +static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc) > +{ > + struct intel_context *ce; > + unsigned long index, flags; > + bool pending_disable, pending_enable, deregister, destroyed; > + > + xa_for_each(&guc->context_lookup, index, ce) { > + /* Flush context */ > + spin_lock_irqsave(&ce->guc_state.lock, flags); > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > + > + /* > + * Once we are at this point submission_disabled() is guaranteed > + * to visible to all callers who set the below flags (see above Still needs to be 'to be visible' > + * flush and flushes in reset_prepare). If submission_disabled() > + * is set, the caller shouldn't set these flags. > + */ > + > + destroyed = context_destroyed(ce); > + pending_enable = context_pending_enable(ce); > + pending_disable = context_pending_disable(ce); > + deregister = context_wait_for_deregister_to_register(ce); > + init_sched_state(ce); > + > + if (pending_enable || destroyed || deregister) { > + atomic_dec(&guc->outstanding_submission_g2h); > + if (deregister) > + guc_signal_context_fence(ce); > + if (destroyed) { > + release_guc_id(guc, ce); > + __guc_context_destroy(ce); > + } > + if (pending_enable|| deregister) > + intel_context_put(ce); > + } > + > + /* Not mutualy exclusive with above if statement. */ > + if (pending_disable) { > + guc_signal_context_fence(ce); > + intel_context_sched_disable_unpin(ce); > + atomic_dec(&guc->outstanding_submission_g2h); > + intel_context_put(ce); > + } > + } > +} > + > +static inline bool > +submission_disabled(struct intel_guc *guc) > +{ > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > + > + return unlikely(!sched_engine || > + !__tasklet_is_enabled(&sched_engine->tasklet)); > +} > + > +static void disable_submission(struct intel_guc *guc) > +{ > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > + > + if (__tasklet_is_enabled(&sched_engine->tasklet)) { > + GEM_BUG_ON(!guc->ct.enabled); > + __tasklet_disable_sync_once(&sched_engine->tasklet); > + sched_engine->tasklet.callback = NULL; > + } > +} > + > +static void enable_submission(struct intel_guc *guc) > +{ > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > + unsigned long flags; > + > + spin_lock_irqsave(&guc->sched_engine->lock, flags); > + sched_engine->tasklet.callback = guc_submission_tasklet; > + wmb(); > + if (!__tasklet_is_enabled(&sched_engine->tasklet) && > + __tasklet_enable(&sched_engine->tasklet)) { > + GEM_BUG_ON(!guc->ct.enabled); > + > + /* And kick in case we missed a new request submission. */ > + tasklet_hi_schedule(&sched_engine->tasklet); > + } > + spin_unlock_irqrestore(&guc->sched_engine->lock, flags); > +} > + > +static void guc_flush_submissions(struct intel_guc *guc) > +{ > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > + unsigned long flags; > + > + spin_lock_irqsave(&sched_engine->lock, flags); > + spin_unlock_irqrestore(&sched_engine->lock, flags); > +} > + > +void intel_guc_submission_reset_prepare(struct intel_guc *guc) > { > - ENGINE_TRACE(engine, "\n"); > + int i; > + > + if (unlikely(!guc_submission_initialized(guc))) > + /* Reset called during driver load? GuC not yet initialised! */ > + return; Still think this should have braces. Maybe not syntactically required but just looks wrong. > + > + disable_submission(guc); > + guc->interrupts.disable(guc); > + > + /* Flush IRQ handler */ > + spin_lock_irq(&guc_to_gt(guc)->irq_lock); > + spin_unlock_irq(&guc_to_gt(guc)->irq_lock); > + > + guc_flush_submissions(guc); > > /* > - * Prevent request submission to the hardware until we have > - * completed the reset in i915_gem_reset_finish(). If a request > - * is completed by one engine, it may then queue a request > - * to a second via its execlists->tasklet *just* as we are > - * calling engine->init_hw() and also writing the ELSP. > - * Turning off the execlists->tasklet until the reset is over > - * prevents the race. > - */ > - __tasklet_disable_sync_once(&engine->sched_engine->tasklet); > + * Handle any outstanding G2Hs before reset. Call IRQ handler directly > + * each pass as interrupt have been disabled. We always scrub for > + * outstanding G2H as it is possible for outstanding_submission_g2h to > + * be incremented after the context state update. > + */ > + for (i = 0; i < 4 && atomic_read(&guc->outstanding_submission_g2h); ++i) { > + intel_guc_to_host_event_handler(guc); > +#define wait_for_reset(guc, wait_var) \ > + guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20)) > + do { > + wait_for_reset(guc, &guc->outstanding_submission_g2h); > + } while (!list_empty(&guc->ct.requests.incoming)); > + } > + scrub_guc_desc_for_outstanding_g2h(guc); > +} > + > +static struct intel_engine_cs * > +guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) > +{ > + struct intel_engine_cs *engine; > + intel_engine_mask_t tmp, mask = ve->mask; > + unsigned int num_siblings = 0; > + > + for_each_engine_masked(engine, ve->gt, mask, tmp) > + if (num_siblings++ == sibling) > + return engine; > + > + return NULL; > +} > + > +static inline struct intel_engine_cs * > +__context_to_physical_engine(struct intel_context *ce) > +{ > + struct intel_engine_cs *engine = ce->engine; > + > + if (intel_engine_is_virtual(engine)) > + engine = guc_virtual_get_sibling(engine, 0); > + > + return engine; > } > > -static void guc_reset_state(struct intel_context *ce, > - struct intel_engine_cs *engine, > - u32 head, > - bool scrub) > +static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub) > { > + struct intel_engine_cs *engine = __context_to_physical_engine(ce); > + > GEM_BUG_ON(!intel_context_is_pinned(ce)); > > /* > @@ -496,42 +673,147 @@ static void guc_reset_state(struct intel_context *ce, > lrc_update_regs(ce, engine, head); > } > > -static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled) > +static void guc_reset_nop(struct intel_engine_cs *engine) > { > - struct intel_engine_execlists * const execlists = &engine->execlists; > - struct i915_request *rq; > +} > + > +static void guc_rewind_nop(struct intel_engine_cs *engine, bool stalled) > +{ > +} > + > +static void > +__unwind_incomplete_requests(struct intel_context *ce) > +{ > + struct i915_request *rq, *rn; > + struct list_head *pl; > + int prio = I915_PRIORITY_INVALID; > + struct i915_sched_engine * const sched_engine = > + ce->engine->sched_engine; > unsigned long flags; > > - spin_lock_irqsave(&engine->sched_engine->lock, flags); > + spin_lock_irqsave(&sched_engine->lock, flags); > + spin_lock(&ce->guc_active.lock); > + list_for_each_entry_safe(rq, rn, > + &ce->guc_active.requests, > + sched.link) { > + if (i915_request_completed(rq)) > + continue; > + > + list_del_init(&rq->sched.link); > + spin_unlock(&ce->guc_active.lock); > + > + __i915_request_unsubmit(rq); > + > + /* Push the request back into the queue for later resubmission. */ > + GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID); > + if (rq_prio(rq) != prio) { > + prio = rq_prio(rq); > + pl = i915_sched_lookup_priolist(sched_engine, prio); > + } > + GEM_BUG_ON(i915_sched_engine_is_empty(sched_engine)); > > - /* Push back any incomplete requests for replay after the reset. */ > - rq = execlists_unwind_incomplete_requests(execlists); > - if (!rq) > - goto out_unlock; > + list_add_tail(&rq->sched.link, pl); > + set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); > + > + spin_lock(&ce->guc_active.lock); > + } > + spin_unlock(&ce->guc_active.lock); > + spin_unlock_irqrestore(&sched_engine->lock, flags); > +} > + > +static struct i915_request *context_find_active_request(struct intel_context *ce) > +{ > + struct i915_request *rq, *active = NULL; > + unsigned long flags; > + > + spin_lock_irqsave(&ce->guc_active.lock, flags); > + list_for_each_entry_reverse(rq, &ce->guc_active.requests, > + sched.link) { > + if (i915_request_completed(rq)) > + break; > + > + active = rq; > + } > + spin_unlock_irqrestore(&ce->guc_active.lock, flags); > + > + return active; > +} > + > +static void __guc_reset_context(struct intel_context *ce, bool stalled) > +{ > + struct i915_request *rq; > + u32 head; > + > + /* > + * GuC will implicitly mark the context as non-schedulable > + * when it sends the reset notification. Make sure our state > + * reflects this change. The context will be marked enabled > + * on resubmission. > + */ > + clr_context_enabled(ce); > + > + rq = context_find_active_request(ce); > + if (!rq) { > + head = ce->ring->tail; > + stalled = false; > + goto out_replay; > + } > > if (!i915_request_started(rq)) > stalled = false; > > + GEM_BUG_ON(i915_active_is_idle(&ce->active)); > + head = intel_ring_wrap(ce->ring, rq->head); > __i915_request_reset(rq, stalled); > - guc_reset_state(rq->context, engine, rq->head, stalled); > > -out_unlock: > - spin_unlock_irqrestore(&engine->sched_engine->lock, flags); > +out_replay: > + guc_reset_state(ce, head, stalled); > + __unwind_incomplete_requests(ce); > +} > + > +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled) > +{ > + struct intel_context *ce; > + unsigned long index; > + > + if (unlikely(!guc_submission_initialized(guc))) > + /* Reset called during driver load? GuC not yet initialised! */ > + return; And again. > + > + xa_for_each(&guc->context_lookup, index, ce) > + if (intel_context_is_pinned(ce)) > + __guc_reset_context(ce, stalled); > + > + /* GuC is blown away, drop all references to contexts */ > + xa_destroy(&guc->context_lookup); > +} > + > +static void guc_cancel_context_requests(struct intel_context *ce) > +{ > + struct i915_sched_engine *sched_engine = ce_to_guc(ce)->sched_engine; > + struct i915_request *rq; > + unsigned long flags; > + > + /* Mark all executing requests as skipped. */ > + spin_lock_irqsave(&sched_engine->lock, flags); > + spin_lock(&ce->guc_active.lock); > + list_for_each_entry(rq, &ce->guc_active.requests, sched.link) > + i915_request_put(i915_request_mark_eio(rq)); > + spin_unlock(&ce->guc_active.lock); > + spin_unlock_irqrestore(&sched_engine->lock, flags); > } > > -static void guc_reset_cancel(struct intel_engine_cs *engine) > +static void > +guc_cancel_sched_engine_requests(struct i915_sched_engine *sched_engine) > { > - struct i915_sched_engine * const sched_engine = engine->sched_engine; > struct i915_request *rq, *rn; > struct rb_node *rb; > unsigned long flags; > > /* Can be called during boot if GuC fails to load */ > - if (!engine->gt) > + if (!sched_engine) > return; > > - ENGINE_TRACE(engine, "\n"); > - > /* > * Before we call engine->cancel_requests(), we should have exclusive > * access to the submission state. This is arranged for us by the > @@ -548,21 +830,16 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) > */ > spin_lock_irqsave(&sched_engine->lock, flags); > > - /* Mark all executing requests as skipped. */ > - list_for_each_entry(rq, &sched_engine->requests, sched.link) { > - i915_request_set_error_once(rq, -EIO); > - i915_request_mark_complete(rq); > - } > - > /* Flush the queued requests to the timeline list (for retiring). */ > while ((rb = rb_first_cached(&sched_engine->queue))) { > struct i915_priolist *p = to_priolist(rb); > > priolist_for_each_request_consume(rq, rn, p) { > list_del_init(&rq->sched.link); > + > __i915_request_submit(rq); > - dma_fence_set_error(&rq->fence, -EIO); > - i915_request_mark_complete(rq); > + > + i915_request_put(i915_request_mark_eio(rq)); > } > > rb_erase_cached(&p->node, &sched_engine->queue); > @@ -577,14 +854,38 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) > spin_unlock_irqrestore(&sched_engine->lock, flags); > } > > -static void guc_reset_finish(struct intel_engine_cs *engine) > +void intel_guc_submission_cancel_requests(struct intel_guc *guc) > { > - if (__tasklet_enable(&engine->sched_engine->tasklet)) > - /* And kick in case we missed a new request submission. */ > - tasklet_hi_schedule(&engine->sched_engine->tasklet); > + struct intel_context *ce; > + unsigned long index; > > - ENGINE_TRACE(engine, "depth->%d\n", > - atomic_read(&engine->sched_engine->tasklet.count)); > + xa_for_each(&guc->context_lookup, index, ce) > + if (intel_context_is_pinned(ce)) > + guc_cancel_context_requests(ce); > + > + guc_cancel_sched_engine_requests(guc->sched_engine); > + > + /* GuC is blown away, drop all references to contexts */ > + xa_destroy(&guc->context_lookup); And again, most of this code is common to 'intel_guc_submission_reset' so could be moved to a shared helper function. John. > +} > + > +void intel_guc_submission_reset_finish(struct intel_guc *guc) > +{ > + /* Reset called during driver load or during wedge? */ > + if (unlikely(!guc_submission_initialized(guc) || > + test_bit(I915_WEDGED, &guc_to_gt(guc)->reset.flags))) > + return; > + > + /* > + * Technically possible for either of these values to be non-zero here, > + * but very unlikely + harmless. Regardless let's add a warn so we can > + * see in CI if this happens frequently / a precursor to taking down the > + * machine. > + */ > + GEM_WARN_ON(atomic_read(&guc->outstanding_submission_g2h)); > + atomic_set(&guc->outstanding_submission_g2h, 0); > + > + enable_submission(guc); > } > > /* > @@ -651,6 +952,9 @@ static int guc_bypass_tasklet_submit(struct intel_guc *guc, > else > trace_i915_request_guc_submit(rq); > > + if (unlikely(ret == -EPIPE)) > + disable_submission(guc); > + > return ret; > } > > @@ -663,7 +967,8 @@ static void guc_submit_request(struct i915_request *rq) > /* Will be called from irq-context when using foreign fences. */ > spin_lock_irqsave(&sched_engine->lock, flags); > > - if (guc->stalled_request || !i915_sched_engine_is_empty(sched_engine)) > + if (submission_disabled(guc) || guc->stalled_request || > + !i915_sched_engine_is_empty(sched_engine)) > queue_request(sched_engine, rq, rq_prio(rq)); > else if (guc_bypass_tasklet_submit(guc, rq) == -EBUSY) > tasklet_hi_schedule(&sched_engine->tasklet); > @@ -800,7 +1105,8 @@ static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce) > > static int __guc_action_register_context(struct intel_guc *guc, > u32 guc_id, > - u32 offset) > + u32 offset, > + bool loop) > { > u32 action[] = { > INTEL_GUC_ACTION_REGISTER_CONTEXT, > @@ -809,10 +1115,10 @@ static int __guc_action_register_context(struct intel_guc *guc, > }; > > return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > - 0, true); > + 0, loop); > } > > -static int register_context(struct intel_context *ce) > +static int register_context(struct intel_context *ce, bool loop) > { > struct intel_guc *guc = ce_to_guc(ce); > u32 offset = intel_guc_ggtt_offset(guc, guc->lrc_desc_pool) + > @@ -820,11 +1126,12 @@ static int register_context(struct intel_context *ce) > > trace_intel_context_register(ce); > > - return __guc_action_register_context(guc, ce->guc_id, offset); > + return __guc_action_register_context(guc, ce->guc_id, offset, loop); > } > > static int __guc_action_deregister_context(struct intel_guc *guc, > - u32 guc_id) > + u32 guc_id, > + bool loop) > { > u32 action[] = { > INTEL_GUC_ACTION_DEREGISTER_CONTEXT, > @@ -833,16 +1140,16 @@ static int __guc_action_deregister_context(struct intel_guc *guc, > > return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > G2H_LEN_DW_DEREGISTER_CONTEXT, > - true); > + loop); > } > > -static int deregister_context(struct intel_context *ce, u32 guc_id) > +static int deregister_context(struct intel_context *ce, u32 guc_id, bool loop) > { > struct intel_guc *guc = ce_to_guc(ce); > > trace_intel_context_deregister(ce); > > - return __guc_action_deregister_context(guc, guc_id); > + return __guc_action_deregister_context(guc, guc_id, loop); > } > > static intel_engine_mask_t adjust_engine_mask(u8 class, intel_engine_mask_t mask) > @@ -871,7 +1178,7 @@ static void guc_context_policy_init(struct intel_engine_cs *engine, > desc->preemption_timeout = CONTEXT_POLICY_DEFAULT_PREEMPTION_TIME_US; > } > > -static int guc_lrc_desc_pin(struct intel_context *ce) > +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) > { > struct intel_runtime_pm *runtime_pm = > &ce->engine->gt->i915->runtime_pm; > @@ -917,18 +1224,45 @@ static int guc_lrc_desc_pin(struct intel_context *ce) > */ > if (context_registered) { > trace_intel_context_steal_guc_id(ce); > - set_context_wait_for_deregister_to_register(ce); > - intel_context_get(ce); > + if (!loop) { > + set_context_wait_for_deregister_to_register(ce); > + intel_context_get(ce); > + } else { > + bool disabled; > + unsigned long flags; > + > + /* Seal race with Reset */ > + spin_lock_irqsave(&ce->guc_state.lock, flags); > + disabled = submission_disabled(guc); > + if (likely(!disabled)) { > + set_context_wait_for_deregister_to_register(ce); > + intel_context_get(ce); > + } > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > + if (unlikely(disabled)) { > + reset_lrc_desc(guc, desc_idx); > + return 0; /* Will get registered later */ > + } > + } > > /* > * If stealing the guc_id, this ce has the same guc_id as the > * context whos guc_id was stole. > */ > with_intel_runtime_pm(runtime_pm, wakeref) > - ret = deregister_context(ce, ce->guc_id); > + ret = deregister_context(ce, ce->guc_id, loop); > + if (unlikely(ret == -EBUSY)) { > + clr_context_wait_for_deregister_to_register(ce); > + intel_context_put(ce); > + } else if (unlikely(ret == -ENODEV)) > + ret = 0; /* Will get registered later */ > } else { > with_intel_runtime_pm(runtime_pm, wakeref) > - ret = register_context(ce); > + ret = register_context(ce, loop); > + if (unlikely(ret == -EBUSY)) > + reset_lrc_desc(guc, desc_idx); > + else if (unlikely(ret == -ENODEV)) > + ret = 0; /* Will get registered later */ > } > > return ret; > @@ -991,7 +1325,6 @@ static void __guc_context_sched_disable(struct intel_guc *guc, > GEM_BUG_ON(guc_id == GUC_INVALID_LRC_ID); > > trace_intel_context_sched_disable(ce); > - intel_context_get(ce); > > guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true); > @@ -1003,6 +1336,7 @@ static u16 prep_context_pending_disable(struct intel_context *ce) > > set_context_pending_disable(ce); > clr_context_enabled(ce); > + intel_context_get(ce); > > return ce->guc_id; > } > @@ -1015,7 +1349,7 @@ static void guc_context_sched_disable(struct intel_context *ce) > u16 guc_id; > intel_wakeref_t wakeref; > > - if (context_guc_id_invalid(ce) || > + if (submission_disabled(guc) || context_guc_id_invalid(ce) || > !lrc_desc_registered(guc, ce->guc_id)) { > clr_context_enabled(ce); > goto unpin; > @@ -1036,6 +1370,7 @@ static void guc_context_sched_disable(struct intel_context *ce) > * a request before we set the 'context_pending_disable' flag here. > */ > if (unlikely(atomic_add_unless(&ce->pin_count, -2, 2))) { > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > return; > } > guc_id = prep_context_pending_disable(ce); > @@ -1052,19 +1387,13 @@ static void guc_context_sched_disable(struct intel_context *ce) > > static inline void guc_lrc_desc_unpin(struct intel_context *ce) > { > - struct intel_engine_cs *engine = ce->engine; > - struct intel_guc *guc = &engine->gt->uc.guc; > - unsigned long flags; > + struct intel_guc *guc = ce_to_guc(ce); > > GEM_BUG_ON(!lrc_desc_registered(guc, ce->guc_id)); > GEM_BUG_ON(ce != __get_context(guc, ce->guc_id)); > GEM_BUG_ON(context_enabled(ce)); > > - spin_lock_irqsave(&ce->guc_state.lock, flags); > - set_context_destroyed(ce); > - spin_unlock_irqrestore(&ce->guc_state.lock, flags); > - > - deregister_context(ce, ce->guc_id); > + deregister_context(ce, ce->guc_id, true); > } > > static void __guc_context_destroy(struct intel_context *ce) > @@ -1092,13 +1421,15 @@ static void guc_context_destroy(struct kref *kref) > struct intel_guc *guc = &ce->engine->gt->uc.guc; > intel_wakeref_t wakeref; > unsigned long flags; > + bool disabled; > > /* > * 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. > */ > - if (context_guc_id_invalid(ce) || > + if (submission_disabled(guc) || > + context_guc_id_invalid(ce) || > !lrc_desc_registered(guc, ce->guc_id)) { > release_guc_id(guc, ce); > __guc_context_destroy(ce); > @@ -1125,6 +1456,18 @@ static void guc_context_destroy(struct kref *kref) > list_del_init(&ce->guc_id_link); > spin_unlock_irqrestore(&guc->contexts_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); > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > + if (unlikely(disabled)) { > + release_guc_id(guc, ce); > + __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 > @@ -1212,8 +1555,6 @@ static void guc_signal_context_fence(struct intel_context *ce) > { > unsigned long flags; > > - GEM_BUG_ON(!context_wait_for_deregister_to_register(ce)); > - > spin_lock_irqsave(&ce->guc_state.lock, flags); > clr_context_wait_for_deregister_to_register(ce); > __guc_signal_context_fence(ce); > @@ -1222,8 +1563,9 @@ static void guc_signal_context_fence(struct intel_context *ce) > > static bool context_needs_register(struct intel_context *ce, bool new_guc_id) > { > - return new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || > - !lrc_desc_registered(ce_to_guc(ce), ce->guc_id); > + return (new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || > + !lrc_desc_registered(ce_to_guc(ce), ce->guc_id)) && > + !submission_disabled(ce_to_guc(ce)); > } > > static int guc_request_alloc(struct i915_request *rq) > @@ -1281,8 +1623,12 @@ static int guc_request_alloc(struct i915_request *rq) > if (unlikely(ret < 0)) > return ret;; > if (context_needs_register(ce, !!ret)) { > - ret = guc_lrc_desc_pin(ce); > + ret = guc_lrc_desc_pin(ce, true); > if (unlikely(ret)) { /* unwind */ > + if (ret == -EPIPE) { > + disable_submission(guc); > + goto out; /* GPU will be reset */ > + } > atomic_dec(&ce->guc_id_ref); > unpin_guc_id(guc, ce); > return ret; > @@ -1319,20 +1665,6 @@ static int guc_request_alloc(struct i915_request *rq) > return 0; > } > > -static struct intel_engine_cs * > -guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) > -{ > - struct intel_engine_cs *engine; > - intel_engine_mask_t tmp, mask = ve->mask; > - unsigned int num_siblings = 0; > - > - for_each_engine_masked(engine, ve->gt, mask, tmp) > - if (num_siblings++ == sibling) > - return engine; > - > - return NULL; > -} > - > static int guc_virtual_context_pre_pin(struct intel_context *ce, > struct i915_gem_ww_ctx *ww, > void **vaddr) > @@ -1528,7 +1860,7 @@ static inline void guc_kernel_context_pin(struct intel_guc *guc, > { > if (context_guc_id_invalid(ce)) > pin_guc_id(guc, ce); > - guc_lrc_desc_pin(ce); > + guc_lrc_desc_pin(ce, true); > } > > static inline void guc_init_lrc_mapping(struct intel_guc *guc) > @@ -1599,10 +1931,10 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) > > engine->sched_engine->schedule = i915_schedule; > > - engine->reset.prepare = guc_reset_prepare; > - engine->reset.rewind = guc_reset_rewind; > - engine->reset.cancel = guc_reset_cancel; > - engine->reset.finish = guc_reset_finish; > + engine->reset.prepare = guc_reset_nop; > + engine->reset.rewind = guc_rewind_nop; > + engine->reset.cancel = guc_reset_nop; > + engine->reset.finish = guc_reset_nop; > > engine->emit_flush = gen8_emit_flush_xcs; > engine->emit_init_breadcrumb = gen8_emit_init_breadcrumb; > @@ -1651,6 +1983,17 @@ static inline void guc_default_irqs(struct intel_engine_cs *engine) > intel_engine_set_irq_handler(engine, cs_irq_handler); > } > > +static void guc_sched_engine_destroy(struct kref *kref) > +{ > + struct i915_sched_engine *sched_engine = > + container_of(kref, typeof(*sched_engine), ref); > + struct intel_guc *guc = sched_engine->private_data; > + > + guc->sched_engine = NULL; > + tasklet_kill(&sched_engine->tasklet); /* flush the callback */ > + kfree(sched_engine); > +} > + > int intel_guc_submission_setup(struct intel_engine_cs *engine) > { > struct drm_i915_private *i915 = engine->i915; > @@ -1669,6 +2012,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine) > > guc->sched_engine->schedule = i915_schedule; > guc->sched_engine->private_data = guc; > + guc->sched_engine->destroy = guc_sched_engine_destroy; > tasklet_setup(&guc->sched_engine->tasklet, > guc_submission_tasklet); > } > @@ -1775,7 +2119,7 @@ int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > * register this context. > */ > with_intel_runtime_pm(runtime_pm, wakeref) > - register_context(ce); > + register_context(ce, true); > guc_signal_context_fence(ce); > intel_context_put(ce); > } else if (context_destroyed(ce)) { > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index 6d8b9233214e..f0b02200aa01 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -565,12 +565,49 @@ void intel_uc_reset_prepare(struct intel_uc *uc) > { > struct intel_guc *guc = &uc->guc; > > - if (!intel_guc_is_ready(guc)) > + > + /* Nothing to do if GuC isn't supported */ > + if (!intel_uc_supports_guc(uc)) > return; > > + /* Firmware expected to be running when this function is called */ > + if (!intel_guc_is_ready(guc)) > + goto sanitize; > + > + if (intel_uc_uses_guc_submission(uc)) > + intel_guc_submission_reset_prepare(guc); > + > +sanitize: > __uc_sanitize(uc); > } > > +void intel_uc_reset(struct intel_uc *uc, bool stalled) > +{ > + struct intel_guc *guc = &uc->guc; > + > + /* Firmware can not be running when this function is called */ > + if (intel_uc_uses_guc_submission(uc)) > + intel_guc_submission_reset(guc, stalled); > +} > + > +void intel_uc_reset_finish(struct intel_uc *uc) > +{ > + struct intel_guc *guc = &uc->guc; > + > + /* Firmware expected to be running when this function is called */ > + if (intel_guc_is_fw_running(guc) && intel_uc_uses_guc_submission(uc)) > + intel_guc_submission_reset_finish(guc); > +} > + > +void intel_uc_cancel_requests(struct intel_uc *uc) > +{ > + struct intel_guc *guc = &uc->guc; > + > + /* Firmware can not be running when this function is called */ > + if (intel_uc_uses_guc_submission(uc)) > + intel_guc_submission_cancel_requests(guc); > +} > + > void intel_uc_runtime_suspend(struct intel_uc *uc) > { > struct intel_guc *guc = &uc->guc; > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > index c4cef885e984..eaa3202192ac 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > @@ -37,6 +37,9 @@ void intel_uc_driver_late_release(struct intel_uc *uc); > void intel_uc_driver_remove(struct intel_uc *uc); > void intel_uc_init_mmio(struct intel_uc *uc); > void intel_uc_reset_prepare(struct intel_uc *uc); > +void intel_uc_reset(struct intel_uc *uc, bool stalled); > +void intel_uc_reset_finish(struct intel_uc *uc); > +void intel_uc_cancel_requests(struct intel_uc *uc); > void intel_uc_suspend(struct intel_uc *uc); > void intel_uc_runtime_suspend(struct intel_uc *uc); > int intel_uc_resume(struct intel_uc *uc);
On Tue, Jul 20, 2021 at 01:19:48PM -0700, John Harrison wrote: > On 7/16/2021 13:16, Matthew Brost wrote: > > Reset implementation for new GuC interface. This is the legacy reset > > implementation which is called when the i915 owns the engine hang check. > > Future patches will offload the engine hang check to GuC but we will > > continue to maintain this legacy path as a fallback and this code path > > is also required if the GuC dies. > > > > With the new GuC interface it is not possible to reset individual > > engines - it is only possible to reset the GPU entirely. This patch > > forces an entire chip reset if any engine hangs. > > > > v2: > > (Michal) > > - Check for -EPIPE rather than -EIO (CT deadlock/corrupt check) > > v3: > > (John H) > > - Split into a series of smaller patches > While the split happened, it doesn't look like any of the other comments > were address. Repeated below for clarity. Also, Tvrtko has a bunch of Opps, I thought you asked for the split and didn't realize you had comments. I thought I addressed Tvrtko's comments but a I'll circle back to them. > outstanding comments too. > > > > > Cc: John Harrison <john.c.harrison@intel.com> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_gt_pm.c | 6 +- > > drivers/gpu/drm/i915/gt/intel_reset.c | 18 +- > > drivers/gpu/drm/i915/gt/uc/intel_guc.c | 13 - > > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +- > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 562 ++++++++++++++---- > > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 39 +- > > drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 + > > 7 files changed, 515 insertions(+), 134 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > > index aef3084e8b16..463a6ae605a0 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c > > @@ -174,8 +174,6 @@ static void gt_sanitize(struct intel_gt *gt, bool force) > > if (intel_gt_is_wedged(gt)) > > intel_gt_unset_wedged(gt); > > - intel_uc_sanitize(>->uc); > > - > > for_each_engine(engine, gt, id) > > if (engine->reset.prepare) > > engine->reset.prepare(engine); > > @@ -191,6 +189,8 @@ static void gt_sanitize(struct intel_gt *gt, bool force) > > __intel_engine_reset(engine, false); > > } > > + intel_uc_reset(>->uc, false); > > + > > for_each_engine(engine, gt, id) > > if (engine->reset.finish) > > engine->reset.finish(engine); > > @@ -243,6 +243,8 @@ int intel_gt_resume(struct intel_gt *gt) > > goto err_wedged; > > } > > + intel_uc_reset_finish(>->uc); > > + > > intel_rps_enable(>->rps); > > intel_llc_enable(>->llc); > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c > > index 72251638d4ea..2987282dff6d 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_reset.c > > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c > > @@ -826,6 +826,8 @@ static int gt_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask) > > __intel_engine_reset(engine, stalled_mask & engine->mask); > > local_bh_enable(); > > + intel_uc_reset(>->uc, true); > > + > > intel_ggtt_restore_fences(gt->ggtt); > > return err; > > @@ -850,6 +852,8 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake) > > if (awake & engine->mask) > > intel_engine_pm_put(engine); > > } > > + > > + intel_uc_reset_finish(>->uc); > > } > > static void nop_submit_request(struct i915_request *request) > > @@ -903,6 +907,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt) > > for_each_engine(engine, gt, id) > > if (engine->reset.cancel) > > engine->reset.cancel(engine); > > + intel_uc_cancel_requests(>->uc); > > local_bh_enable(); > > reset_finish(gt, awake); > > @@ -1191,6 +1196,9 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) > > ENGINE_TRACE(engine, "flags=%lx\n", gt->reset.flags); > > GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, >->reset.flags)); > > + if (intel_engine_uses_guc(engine)) > > + return -ENODEV; > > + > > if (!intel_engine_pm_get_if_awake(engine)) > > return 0; > > @@ -1201,13 +1209,10 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) > > "Resetting %s for %s\n", engine->name, msg); > > atomic_inc(&engine->i915->gpu_error.reset_engine_count[engine->uabi_class]); > > - if (intel_engine_uses_guc(engine)) > > - ret = intel_guc_reset_engine(&engine->gt->uc.guc, engine); > > - else > > - ret = intel_gt_reset_engine(engine); > > + ret = intel_gt_reset_engine(engine); > > if (ret) { > > /* If we fail here, we expect to fallback to a global reset */ > > - ENGINE_TRACE(engine, "Failed to reset, err: %d\n", ret); > > + ENGINE_TRACE(engine, "Failed to reset %s, err: %d\n", engine->name, ret); > > goto out; > > } > > @@ -1341,7 +1346,8 @@ void intel_gt_handle_error(struct intel_gt *gt, > > * Try engine reset when available. We fall back to full reset if > > * single reset fails. > > */ > > - if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { > > + if (!intel_uc_uses_guc_submission(>->uc) && > > + intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { > > local_bh_disable(); > > for_each_engine_masked(engine, gt, engine_mask, tmp) { > > BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > index 6661dcb02239..9b09395b998f 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > > @@ -572,19 +572,6 @@ int intel_guc_suspend(struct intel_guc *guc) > > return 0; > > } > > -/** > > - * intel_guc_reset_engine() - ask GuC to reset an engine > > - * @guc: intel_guc structure > > - * @engine: engine to be reset > > - */ > > -int intel_guc_reset_engine(struct intel_guc *guc, > > - struct intel_engine_cs *engine) > > -{ > > - /* XXX: to be implemented with submission interface rework */ > > - > > - return -ENODEV; > > -} > > - > > /** > > * intel_guc_resume() - notify GuC resuming from suspend state > > * @guc: the guc > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index 3cc566565224..d75a76882a44 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -242,14 +242,16 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask) > > int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout); > > -int intel_guc_reset_engine(struct intel_guc *guc, > > - struct intel_engine_cs *engine); > > - > > int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > > const u32 *msg, u32 len); > > int intel_guc_sched_done_process_msg(struct intel_guc *guc, > > const u32 *msg, u32 len); > > +void intel_guc_submission_reset_prepare(struct intel_guc *guc); > > +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled); > > +void intel_guc_submission_reset_finish(struct intel_guc *guc); > > +void intel_guc_submission_cancel_requests(struct intel_guc *guc); > > + > > void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p); > > #endif > > 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 f8a6077fa3f9..0f5823922369 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -141,7 +141,7 @@ context_wait_for_deregister_to_register(struct intel_context *ce) > > static inline void > > set_context_wait_for_deregister_to_register(struct intel_context *ce) > > { > > - /* Only should be called from guc_lrc_desc_pin() */ > > + /* Only should be called from guc_lrc_desc_pin() without lock */ > > ce->guc_state.sched_state |= > > SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER; > > } > > @@ -241,15 +241,31 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc) > > static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) > > { > > + guc->lrc_desc_pool_vaddr = NULL; > > i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); > > } > > +static inline bool guc_submission_initialized(struct intel_guc *guc) > > +{ > > + return guc->lrc_desc_pool_vaddr != NULL; > > +} > > + > > static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) > > { > > - struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > > + if (likely(guc_submission_initialized(guc))) { > > + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); > > + unsigned long flags; > > - memset(desc, 0, sizeof(*desc)); > > - xa_erase_irq(&guc->context_lookup, id); > > + memset(desc, 0, sizeof(*desc)); > > + > > + /* > > + * xarray API doesn't have xa_erase_irqsave wrapper, so calling > > + * the lower level functions directly. > > + */ > > + xa_lock_irqsave(&guc->context_lookup, flags); > > + __xa_erase(&guc->context_lookup, id); > > + xa_unlock_irqrestore(&guc->context_lookup, flags); > > + } > > } > > static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > > @@ -260,7 +276,15 @@ static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) > > static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, > > struct intel_context *ce) > > { > > - xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); > > + unsigned long flags; > > + > > + /* > > + * xarray API doesn't have xa_save_irqsave wrapper, so calling the > > + * lower level functions directly. > > + */ > > + xa_lock_irqsave(&guc->context_lookup, flags); > > + __xa_store(&guc->context_lookup, id, ce, GFP_ATOMIC); > > + xa_unlock_irqrestore(&guc->context_lookup, flags); > > } > > static int guc_submission_send_busy_loop(struct intel_guc* guc, > > @@ -326,6 +350,8 @@ int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout) > > true, timeout); > > } > > +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop); > > + > > static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > > { > > int err; > > @@ -333,11 +359,22 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > > u32 action[3]; > > int len = 0; > > u32 g2h_len_dw = 0; > > - bool enabled = context_enabled(ce); > > + bool enabled; > > GEM_BUG_ON(!atomic_read(&ce->guc_id_ref)); > > GEM_BUG_ON(context_guc_id_invalid(ce)); > > + /* > > + * Corner case where the GuC firmware was blown away and reloaded while > > + * this context was pinned. > > + */ > > + if (unlikely(!lrc_desc_registered(guc, ce->guc_id))) { > > + err = guc_lrc_desc_pin(ce, false); > > + if (unlikely(err)) > > + goto out; > > + } > > + enabled = context_enabled(ce); > > + > > if (!enabled) { > > action[len++] = INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET; > > action[len++] = ce->guc_id; > > @@ -360,6 +397,7 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) > > intel_context_put(ce); > > } > > +out: > > return err; > > } > > @@ -414,15 +452,10 @@ static int guc_dequeue_one_context(struct intel_guc *guc) > > if (submit) { > > guc_set_lrc_tail(last); > > resubmit: > > - /* > > - * We only check for -EBUSY here even though it is possible for > > - * -EDEADLK to be returned. If -EDEADLK is returned, the GuC has > > - * died and a full GT reset needs to be done. The hangcheck will > > - * eventually detect that the GuC has died and trigger this > > - * reset so no need to handle -EDEADLK here. > > - */ > > ret = guc_add_request(guc, last); > > - if (ret == -EBUSY) { > > + if (unlikely(ret == -EPIPE)) > > + goto deadlk; > > + else if (ret == -EBUSY) { > > tasklet_schedule(&sched_engine->tasklet); > > guc->stalled_request = last; > > return false; > > @@ -432,6 +465,11 @@ static int guc_dequeue_one_context(struct intel_guc *guc) > > guc->stalled_request = NULL; > > return submit; > > + > > +deadlk: > > + sched_engine->tasklet.callback = NULL; > > + tasklet_disable_nosync(&sched_engine->tasklet); > > + return false; > > } > > static void guc_submission_tasklet(struct tasklet_struct *t) > > @@ -458,27 +496,166 @@ static void cs_irq_handler(struct intel_engine_cs *engine, u16 iir) > > intel_engine_signal_breadcrumbs(engine); > > } > > -static void guc_reset_prepare(struct intel_engine_cs *engine) > > +static void __guc_context_destroy(struct intel_context *ce); > > +static void release_guc_id(struct intel_guc *guc, struct intel_context *ce); > > +static void guc_signal_context_fence(struct intel_context *ce); > > + > > +static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc) > > +{ > > + struct intel_context *ce; > > + unsigned long index, flags; > > + bool pending_disable, pending_enable, deregister, destroyed; > > + > > + xa_for_each(&guc->context_lookup, index, ce) { > > + /* Flush context */ > > + spin_lock_irqsave(&ce->guc_state.lock, flags); > > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > + > > + /* > > + * Once we are at this point submission_disabled() is guaranteed > > + * to visible to all callers who set the below flags (see above > Still needs to be 'to be visible' > Fixed. > > + * flush and flushes in reset_prepare). If submission_disabled() > > + * is set, the caller shouldn't set these flags. > > + */ > > + > > + destroyed = context_destroyed(ce); > > + pending_enable = context_pending_enable(ce); > > + pending_disable = context_pending_disable(ce); > > + deregister = context_wait_for_deregister_to_register(ce); > > + init_sched_state(ce); > > + > > + if (pending_enable || destroyed || deregister) { > > + atomic_dec(&guc->outstanding_submission_g2h); > > + if (deregister) > > + guc_signal_context_fence(ce); > > + if (destroyed) { > > + release_guc_id(guc, ce); > > + __guc_context_destroy(ce); > > + } > > + if (pending_enable|| deregister) > > + intel_context_put(ce); > > + } > > + > > + /* Not mutualy exclusive with above if statement. */ > > + if (pending_disable) { > > + guc_signal_context_fence(ce); > > + intel_context_sched_disable_unpin(ce); > > + atomic_dec(&guc->outstanding_submission_g2h); > > + intel_context_put(ce); > > + } > > + } > > +} > > + > > +static inline bool > > +submission_disabled(struct intel_guc *guc) > > +{ > > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > > + > > + return unlikely(!sched_engine || > > + !__tasklet_is_enabled(&sched_engine->tasklet)); > > +} > > + > > +static void disable_submission(struct intel_guc *guc) > > +{ > > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > > + > > + if (__tasklet_is_enabled(&sched_engine->tasklet)) { > > + GEM_BUG_ON(!guc->ct.enabled); > > + __tasklet_disable_sync_once(&sched_engine->tasklet); > > + sched_engine->tasklet.callback = NULL; > > + } > > +} > > + > > +static void enable_submission(struct intel_guc *guc) > > +{ > > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&guc->sched_engine->lock, flags); > > + sched_engine->tasklet.callback = guc_submission_tasklet; > > + wmb(); > > + if (!__tasklet_is_enabled(&sched_engine->tasklet) && > > + __tasklet_enable(&sched_engine->tasklet)) { > > + GEM_BUG_ON(!guc->ct.enabled); > > + > > + /* And kick in case we missed a new request submission. */ > > + tasklet_hi_schedule(&sched_engine->tasklet); > > + } > > + spin_unlock_irqrestore(&guc->sched_engine->lock, flags); > > +} > > + > > +static void guc_flush_submissions(struct intel_guc *guc) > > +{ > > + struct i915_sched_engine * const sched_engine = guc->sched_engine; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&sched_engine->lock, flags); > > + spin_unlock_irqrestore(&sched_engine->lock, flags); > > +} > > + > > +void intel_guc_submission_reset_prepare(struct intel_guc *guc) > > { > > - ENGINE_TRACE(engine, "\n"); > > + int i; > > + > > + if (unlikely(!guc_submission_initialized(guc))) > > + /* Reset called during driver load? GuC not yet initialised! */ > > + return; > Still think this should have braces. Maybe not syntactically required but > just looks wrong. > Sure. > > + > > + disable_submission(guc); > > + guc->interrupts.disable(guc); > > + > > + /* Flush IRQ handler */ > > + spin_lock_irq(&guc_to_gt(guc)->irq_lock); > > + spin_unlock_irq(&guc_to_gt(guc)->irq_lock); > > + > > + guc_flush_submissions(guc); > > /* > > - * Prevent request submission to the hardware until we have > > - * completed the reset in i915_gem_reset_finish(). If a request > > - * is completed by one engine, it may then queue a request > > - * to a second via its execlists->tasklet *just* as we are > > - * calling engine->init_hw() and also writing the ELSP. > > - * Turning off the execlists->tasklet until the reset is over > > - * prevents the race. > > - */ > > - __tasklet_disable_sync_once(&engine->sched_engine->tasklet); > > + * Handle any outstanding G2Hs before reset. Call IRQ handler directly > > + * each pass as interrupt have been disabled. We always scrub for > > + * outstanding G2H as it is possible for outstanding_submission_g2h to > > + * be incremented after the context state update. > > + */ > > + for (i = 0; i < 4 && atomic_read(&guc->outstanding_submission_g2h); ++i) { > > + intel_guc_to_host_event_handler(guc); > > +#define wait_for_reset(guc, wait_var) \ > > + guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20)) > > + do { > > + wait_for_reset(guc, &guc->outstanding_submission_g2h); > > + } while (!list_empty(&guc->ct.requests.incoming)); > > + } > > + scrub_guc_desc_for_outstanding_g2h(guc); > > +} > > + > > +static struct intel_engine_cs * > > +guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) > > +{ > > + struct intel_engine_cs *engine; > > + intel_engine_mask_t tmp, mask = ve->mask; > > + unsigned int num_siblings = 0; > > + > > + for_each_engine_masked(engine, ve->gt, mask, tmp) > > + if (num_siblings++ == sibling) > > + return engine; > > + > > + return NULL; > > +} > > + > > +static inline struct intel_engine_cs * > > +__context_to_physical_engine(struct intel_context *ce) > > +{ > > + struct intel_engine_cs *engine = ce->engine; > > + > > + if (intel_engine_is_virtual(engine)) > > + engine = guc_virtual_get_sibling(engine, 0); > > + > > + return engine; > > } > > -static void guc_reset_state(struct intel_context *ce, > > - struct intel_engine_cs *engine, > > - u32 head, > > - bool scrub) > > +static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub) > > { > > + struct intel_engine_cs *engine = __context_to_physical_engine(ce); > > + > > GEM_BUG_ON(!intel_context_is_pinned(ce)); > > /* > > @@ -496,42 +673,147 @@ static void guc_reset_state(struct intel_context *ce, > > lrc_update_regs(ce, engine, head); > > } > > -static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled) > > +static void guc_reset_nop(struct intel_engine_cs *engine) > > { > > - struct intel_engine_execlists * const execlists = &engine->execlists; > > - struct i915_request *rq; > > +} > > + > > +static void guc_rewind_nop(struct intel_engine_cs *engine, bool stalled) > > +{ > > +} > > + > > +static void > > +__unwind_incomplete_requests(struct intel_context *ce) > > +{ > > + struct i915_request *rq, *rn; > > + struct list_head *pl; > > + int prio = I915_PRIORITY_INVALID; > > + struct i915_sched_engine * const sched_engine = > > + ce->engine->sched_engine; > > unsigned long flags; > > - spin_lock_irqsave(&engine->sched_engine->lock, flags); > > + spin_lock_irqsave(&sched_engine->lock, flags); > > + spin_lock(&ce->guc_active.lock); > > + list_for_each_entry_safe(rq, rn, > > + &ce->guc_active.requests, > > + sched.link) { > > + if (i915_request_completed(rq)) > > + continue; > > + > > + list_del_init(&rq->sched.link); > > + spin_unlock(&ce->guc_active.lock); > > + > > + __i915_request_unsubmit(rq); > > + > > + /* Push the request back into the queue for later resubmission. */ > > + GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID); > > + if (rq_prio(rq) != prio) { > > + prio = rq_prio(rq); > > + pl = i915_sched_lookup_priolist(sched_engine, prio); > > + } > > + GEM_BUG_ON(i915_sched_engine_is_empty(sched_engine)); > > - /* Push back any incomplete requests for replay after the reset. */ > > - rq = execlists_unwind_incomplete_requests(execlists); > > - if (!rq) > > - goto out_unlock; > > + list_add_tail(&rq->sched.link, pl); > > + set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); > > + > > + spin_lock(&ce->guc_active.lock); > > + } > > + spin_unlock(&ce->guc_active.lock); > > + spin_unlock_irqrestore(&sched_engine->lock, flags); > > +} > > + > > +static struct i915_request *context_find_active_request(struct intel_context *ce) > > +{ > > + struct i915_request *rq, *active = NULL; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&ce->guc_active.lock, flags); > > + list_for_each_entry_reverse(rq, &ce->guc_active.requests, > > + sched.link) { > > + if (i915_request_completed(rq)) > > + break; > > + > > + active = rq; > > + } > > + spin_unlock_irqrestore(&ce->guc_active.lock, flags); > > + > > + return active; > > +} > > + > > +static void __guc_reset_context(struct intel_context *ce, bool stalled) > > +{ > > + struct i915_request *rq; > > + u32 head; > > + > > + /* > > + * GuC will implicitly mark the context as non-schedulable > > + * when it sends the reset notification. Make sure our state > > + * reflects this change. The context will be marked enabled > > + * on resubmission. > > + */ > > + clr_context_enabled(ce); > > + > > + rq = context_find_active_request(ce); > > + if (!rq) { > > + head = ce->ring->tail; > > + stalled = false; > > + goto out_replay; > > + } > > if (!i915_request_started(rq)) > > stalled = false; > > + GEM_BUG_ON(i915_active_is_idle(&ce->active)); > > + head = intel_ring_wrap(ce->ring, rq->head); > > __i915_request_reset(rq, stalled); > > - guc_reset_state(rq->context, engine, rq->head, stalled); > > -out_unlock: > > - spin_unlock_irqrestore(&engine->sched_engine->lock, flags); > > +out_replay: > > + guc_reset_state(ce, head, stalled); > > + __unwind_incomplete_requests(ce); > > +} > > + > > +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled) > > +{ > > + struct intel_context *ce; > > + unsigned long index; > > + > > + if (unlikely(!guc_submission_initialized(guc))) > > + /* Reset called during driver load? GuC not yet initialised! */ > > + return; > And again. > Yep. > > + > > + xa_for_each(&guc->context_lookup, index, ce) > > + if (intel_context_is_pinned(ce)) > > + __guc_reset_context(ce, stalled); > > + > > + /* GuC is blown away, drop all references to contexts */ > > + xa_destroy(&guc->context_lookup); > > +} > > + > > +static void guc_cancel_context_requests(struct intel_context *ce) > > +{ > > + struct i915_sched_engine *sched_engine = ce_to_guc(ce)->sched_engine; > > + struct i915_request *rq; > > + unsigned long flags; > > + > > + /* Mark all executing requests as skipped. */ > > + spin_lock_irqsave(&sched_engine->lock, flags); > > + spin_lock(&ce->guc_active.lock); > > + list_for_each_entry(rq, &ce->guc_active.requests, sched.link) > > + i915_request_put(i915_request_mark_eio(rq)); > > + spin_unlock(&ce->guc_active.lock); > > + spin_unlock_irqrestore(&sched_engine->lock, flags); > > } > > -static void guc_reset_cancel(struct intel_engine_cs *engine) > > +static void > > +guc_cancel_sched_engine_requests(struct i915_sched_engine *sched_engine) > > { > > - struct i915_sched_engine * const sched_engine = engine->sched_engine; > > struct i915_request *rq, *rn; > > struct rb_node *rb; > > unsigned long flags; > > /* Can be called during boot if GuC fails to load */ > > - if (!engine->gt) > > + if (!sched_engine) > > return; > > - ENGINE_TRACE(engine, "\n"); > > - > > /* > > * Before we call engine->cancel_requests(), we should have exclusive > > * access to the submission state. This is arranged for us by the > > @@ -548,21 +830,16 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) > > */ > > spin_lock_irqsave(&sched_engine->lock, flags); > > - /* Mark all executing requests as skipped. */ > > - list_for_each_entry(rq, &sched_engine->requests, sched.link) { > > - i915_request_set_error_once(rq, -EIO); > > - i915_request_mark_complete(rq); > > - } > > - > > /* Flush the queued requests to the timeline list (for retiring). */ > > while ((rb = rb_first_cached(&sched_engine->queue))) { > > struct i915_priolist *p = to_priolist(rb); > > priolist_for_each_request_consume(rq, rn, p) { > > list_del_init(&rq->sched.link); > > + > > __i915_request_submit(rq); > > - dma_fence_set_error(&rq->fence, -EIO); > > - i915_request_mark_complete(rq); > > + > > + i915_request_put(i915_request_mark_eio(rq)); > > } > > rb_erase_cached(&p->node, &sched_engine->queue); > > @@ -577,14 +854,38 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) > > spin_unlock_irqrestore(&sched_engine->lock, flags); > > } > > -static void guc_reset_finish(struct intel_engine_cs *engine) > > +void intel_guc_submission_cancel_requests(struct intel_guc *guc) > > { > > - if (__tasklet_enable(&engine->sched_engine->tasklet)) > > - /* And kick in case we missed a new request submission. */ > > - tasklet_hi_schedule(&engine->sched_engine->tasklet); > > + struct intel_context *ce; > > + unsigned long index; > > - ENGINE_TRACE(engine, "depth->%d\n", > > - atomic_read(&engine->sched_engine->tasklet.count)); > > + xa_for_each(&guc->context_lookup, index, ce) > > + if (intel_context_is_pinned(ce)) > > + guc_cancel_context_requests(ce); > > + > > + guc_cancel_sched_engine_requests(guc->sched_engine); > > + > > + /* GuC is blown away, drop all references to contexts */ > > + xa_destroy(&guc->context_lookup); > And again, most of this code is common to 'intel_guc_submission_reset' so > could be moved to a shared helper function. > Similar code structure but different. We use the same xa_for_each loop + pinned check but call __guc_reset_context vs guc_cancel_context_requests. Also here I don't think we can clear &guc->context_lookup until guc_cancel_sched_engine_requests completes. Not sure how to structure a helper that makes sense. Matt > John. > > > +} > > + > > +void intel_guc_submission_reset_finish(struct intel_guc *guc) > > +{ > > + /* Reset called during driver load or during wedge? */ > > + if (unlikely(!guc_submission_initialized(guc) || > > + test_bit(I915_WEDGED, &guc_to_gt(guc)->reset.flags))) > > + return; > > + > > + /* > > + * Technically possible for either of these values to be non-zero here, > > + * but very unlikely + harmless. Regardless let's add a warn so we can > > + * see in CI if this happens frequently / a precursor to taking down the > > + * machine. > > + */ > > + GEM_WARN_ON(atomic_read(&guc->outstanding_submission_g2h)); > > + atomic_set(&guc->outstanding_submission_g2h, 0); > > + > > + enable_submission(guc); > > } > > /* > > @@ -651,6 +952,9 @@ static int guc_bypass_tasklet_submit(struct intel_guc *guc, > > else > > trace_i915_request_guc_submit(rq); > > + if (unlikely(ret == -EPIPE)) > > + disable_submission(guc); > > + > > return ret; > > } > > @@ -663,7 +967,8 @@ static void guc_submit_request(struct i915_request *rq) > > /* Will be called from irq-context when using foreign fences. */ > > spin_lock_irqsave(&sched_engine->lock, flags); > > - if (guc->stalled_request || !i915_sched_engine_is_empty(sched_engine)) > > + if (submission_disabled(guc) || guc->stalled_request || > > + !i915_sched_engine_is_empty(sched_engine)) > > queue_request(sched_engine, rq, rq_prio(rq)); > > else if (guc_bypass_tasklet_submit(guc, rq) == -EBUSY) > > tasklet_hi_schedule(&sched_engine->tasklet); > > @@ -800,7 +1105,8 @@ static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce) > > static int __guc_action_register_context(struct intel_guc *guc, > > u32 guc_id, > > - u32 offset) > > + u32 offset, > > + bool loop) > > { > > u32 action[] = { > > INTEL_GUC_ACTION_REGISTER_CONTEXT, > > @@ -809,10 +1115,10 @@ static int __guc_action_register_context(struct intel_guc *guc, > > }; > > return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > > - 0, true); > > + 0, loop); > > } > > -static int register_context(struct intel_context *ce) > > +static int register_context(struct intel_context *ce, bool loop) > > { > > struct intel_guc *guc = ce_to_guc(ce); > > u32 offset = intel_guc_ggtt_offset(guc, guc->lrc_desc_pool) + > > @@ -820,11 +1126,12 @@ static int register_context(struct intel_context *ce) > > trace_intel_context_register(ce); > > - return __guc_action_register_context(guc, ce->guc_id, offset); > > + return __guc_action_register_context(guc, ce->guc_id, offset, loop); > > } > > static int __guc_action_deregister_context(struct intel_guc *guc, > > - u32 guc_id) > > + u32 guc_id, > > + bool loop) > > { > > u32 action[] = { > > INTEL_GUC_ACTION_DEREGISTER_CONTEXT, > > @@ -833,16 +1140,16 @@ static int __guc_action_deregister_context(struct intel_guc *guc, > > return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > > G2H_LEN_DW_DEREGISTER_CONTEXT, > > - true); > > + loop); > > } > > -static int deregister_context(struct intel_context *ce, u32 guc_id) > > +static int deregister_context(struct intel_context *ce, u32 guc_id, bool loop) > > { > > struct intel_guc *guc = ce_to_guc(ce); > > trace_intel_context_deregister(ce); > > - return __guc_action_deregister_context(guc, guc_id); > > + return __guc_action_deregister_context(guc, guc_id, loop); > > } > > static intel_engine_mask_t adjust_engine_mask(u8 class, intel_engine_mask_t mask) > > @@ -871,7 +1178,7 @@ static void guc_context_policy_init(struct intel_engine_cs *engine, > > desc->preemption_timeout = CONTEXT_POLICY_DEFAULT_PREEMPTION_TIME_US; > > } > > -static int guc_lrc_desc_pin(struct intel_context *ce) > > +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) > > { > > struct intel_runtime_pm *runtime_pm = > > &ce->engine->gt->i915->runtime_pm; > > @@ -917,18 +1224,45 @@ static int guc_lrc_desc_pin(struct intel_context *ce) > > */ > > if (context_registered) { > > trace_intel_context_steal_guc_id(ce); > > - set_context_wait_for_deregister_to_register(ce); > > - intel_context_get(ce); > > + if (!loop) { > > + set_context_wait_for_deregister_to_register(ce); > > + intel_context_get(ce); > > + } else { > > + bool disabled; > > + unsigned long flags; > > + > > + /* Seal race with Reset */ > > + spin_lock_irqsave(&ce->guc_state.lock, flags); > > + disabled = submission_disabled(guc); > > + if (likely(!disabled)) { > > + set_context_wait_for_deregister_to_register(ce); > > + intel_context_get(ce); > > + } > > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > + if (unlikely(disabled)) { > > + reset_lrc_desc(guc, desc_idx); > > + return 0; /* Will get registered later */ > > + } > > + } > > /* > > * If stealing the guc_id, this ce has the same guc_id as the > > * context whos guc_id was stole. > > */ > > with_intel_runtime_pm(runtime_pm, wakeref) > > - ret = deregister_context(ce, ce->guc_id); > > + ret = deregister_context(ce, ce->guc_id, loop); > > + if (unlikely(ret == -EBUSY)) { > > + clr_context_wait_for_deregister_to_register(ce); > > + intel_context_put(ce); > > + } else if (unlikely(ret == -ENODEV)) > > + ret = 0; /* Will get registered later */ > > } else { > > with_intel_runtime_pm(runtime_pm, wakeref) > > - ret = register_context(ce); > > + ret = register_context(ce, loop); > > + if (unlikely(ret == -EBUSY)) > > + reset_lrc_desc(guc, desc_idx); > > + else if (unlikely(ret == -ENODEV)) > > + ret = 0; /* Will get registered later */ > > } > > return ret; > > @@ -991,7 +1325,6 @@ static void __guc_context_sched_disable(struct intel_guc *guc, > > GEM_BUG_ON(guc_id == GUC_INVALID_LRC_ID); > > trace_intel_context_sched_disable(ce); > > - intel_context_get(ce); > > guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), > > G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true); > > @@ -1003,6 +1336,7 @@ static u16 prep_context_pending_disable(struct intel_context *ce) > > set_context_pending_disable(ce); > > clr_context_enabled(ce); > > + intel_context_get(ce); > > return ce->guc_id; > > } > > @@ -1015,7 +1349,7 @@ static void guc_context_sched_disable(struct intel_context *ce) > > u16 guc_id; > > intel_wakeref_t wakeref; > > - if (context_guc_id_invalid(ce) || > > + if (submission_disabled(guc) || context_guc_id_invalid(ce) || > > !lrc_desc_registered(guc, ce->guc_id)) { > > clr_context_enabled(ce); > > goto unpin; > > @@ -1036,6 +1370,7 @@ static void guc_context_sched_disable(struct intel_context *ce) > > * a request before we set the 'context_pending_disable' flag here. > > */ > > if (unlikely(atomic_add_unless(&ce->pin_count, -2, 2))) { > > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > return; > > } > > guc_id = prep_context_pending_disable(ce); > > @@ -1052,19 +1387,13 @@ static void guc_context_sched_disable(struct intel_context *ce) > > static inline void guc_lrc_desc_unpin(struct intel_context *ce) > > { > > - struct intel_engine_cs *engine = ce->engine; > > - struct intel_guc *guc = &engine->gt->uc.guc; > > - unsigned long flags; > > + struct intel_guc *guc = ce_to_guc(ce); > > GEM_BUG_ON(!lrc_desc_registered(guc, ce->guc_id)); > > GEM_BUG_ON(ce != __get_context(guc, ce->guc_id)); > > GEM_BUG_ON(context_enabled(ce)); > > - spin_lock_irqsave(&ce->guc_state.lock, flags); > > - set_context_destroyed(ce); > > - spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > - > > - deregister_context(ce, ce->guc_id); > > + deregister_context(ce, ce->guc_id, true); > > } > > static void __guc_context_destroy(struct intel_context *ce) > > @@ -1092,13 +1421,15 @@ static void guc_context_destroy(struct kref *kref) > > struct intel_guc *guc = &ce->engine->gt->uc.guc; > > intel_wakeref_t wakeref; > > unsigned long flags; > > + bool disabled; > > /* > > * 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. > > */ > > - if (context_guc_id_invalid(ce) || > > + if (submission_disabled(guc) || > > + context_guc_id_invalid(ce) || > > !lrc_desc_registered(guc, ce->guc_id)) { > > release_guc_id(guc, ce); > > __guc_context_destroy(ce); > > @@ -1125,6 +1456,18 @@ static void guc_context_destroy(struct kref *kref) > > list_del_init(&ce->guc_id_link); > > spin_unlock_irqrestore(&guc->contexts_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); > > + spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > + if (unlikely(disabled)) { > > + release_guc_id(guc, ce); > > + __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 > > @@ -1212,8 +1555,6 @@ static void guc_signal_context_fence(struct intel_context *ce) > > { > > unsigned long flags; > > - GEM_BUG_ON(!context_wait_for_deregister_to_register(ce)); > > - > > spin_lock_irqsave(&ce->guc_state.lock, flags); > > clr_context_wait_for_deregister_to_register(ce); > > __guc_signal_context_fence(ce); > > @@ -1222,8 +1563,9 @@ static void guc_signal_context_fence(struct intel_context *ce) > > static bool context_needs_register(struct intel_context *ce, bool new_guc_id) > > { > > - return new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || > > - !lrc_desc_registered(ce_to_guc(ce), ce->guc_id); > > + return (new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || > > + !lrc_desc_registered(ce_to_guc(ce), ce->guc_id)) && > > + !submission_disabled(ce_to_guc(ce)); > > } > > static int guc_request_alloc(struct i915_request *rq) > > @@ -1281,8 +1623,12 @@ static int guc_request_alloc(struct i915_request *rq) > > if (unlikely(ret < 0)) > > return ret;; > > if (context_needs_register(ce, !!ret)) { > > - ret = guc_lrc_desc_pin(ce); > > + ret = guc_lrc_desc_pin(ce, true); > > if (unlikely(ret)) { /* unwind */ > > + if (ret == -EPIPE) { > > + disable_submission(guc); > > + goto out; /* GPU will be reset */ > > + } > > atomic_dec(&ce->guc_id_ref); > > unpin_guc_id(guc, ce); > > return ret; > > @@ -1319,20 +1665,6 @@ static int guc_request_alloc(struct i915_request *rq) > > return 0; > > } > > -static struct intel_engine_cs * > > -guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) > > -{ > > - struct intel_engine_cs *engine; > > - intel_engine_mask_t tmp, mask = ve->mask; > > - unsigned int num_siblings = 0; > > - > > - for_each_engine_masked(engine, ve->gt, mask, tmp) > > - if (num_siblings++ == sibling) > > - return engine; > > - > > - return NULL; > > -} > > - > > static int guc_virtual_context_pre_pin(struct intel_context *ce, > > struct i915_gem_ww_ctx *ww, > > void **vaddr) > > @@ -1528,7 +1860,7 @@ static inline void guc_kernel_context_pin(struct intel_guc *guc, > > { > > if (context_guc_id_invalid(ce)) > > pin_guc_id(guc, ce); > > - guc_lrc_desc_pin(ce); > > + guc_lrc_desc_pin(ce, true); > > } > > static inline void guc_init_lrc_mapping(struct intel_guc *guc) > > @@ -1599,10 +1931,10 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) > > engine->sched_engine->schedule = i915_schedule; > > - engine->reset.prepare = guc_reset_prepare; > > - engine->reset.rewind = guc_reset_rewind; > > - engine->reset.cancel = guc_reset_cancel; > > - engine->reset.finish = guc_reset_finish; > > + engine->reset.prepare = guc_reset_nop; > > + engine->reset.rewind = guc_rewind_nop; > > + engine->reset.cancel = guc_reset_nop; > > + engine->reset.finish = guc_reset_nop; > > engine->emit_flush = gen8_emit_flush_xcs; > > engine->emit_init_breadcrumb = gen8_emit_init_breadcrumb; > > @@ -1651,6 +1983,17 @@ static inline void guc_default_irqs(struct intel_engine_cs *engine) > > intel_engine_set_irq_handler(engine, cs_irq_handler); > > } > > +static void guc_sched_engine_destroy(struct kref *kref) > > +{ > > + struct i915_sched_engine *sched_engine = > > + container_of(kref, typeof(*sched_engine), ref); > > + struct intel_guc *guc = sched_engine->private_data; > > + > > + guc->sched_engine = NULL; > > + tasklet_kill(&sched_engine->tasklet); /* flush the callback */ > > + kfree(sched_engine); > > +} > > + > > int intel_guc_submission_setup(struct intel_engine_cs *engine) > > { > > struct drm_i915_private *i915 = engine->i915; > > @@ -1669,6 +2012,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine) > > guc->sched_engine->schedule = i915_schedule; > > guc->sched_engine->private_data = guc; > > + guc->sched_engine->destroy = guc_sched_engine_destroy; > > tasklet_setup(&guc->sched_engine->tasklet, > > guc_submission_tasklet); > > } > > @@ -1775,7 +2119,7 @@ int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > > * register this context. > > */ > > with_intel_runtime_pm(runtime_pm, wakeref) > > - register_context(ce); > > + register_context(ce, true); > > guc_signal_context_fence(ce); > > intel_context_put(ce); > > } else if (context_destroyed(ce)) { > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > > index 6d8b9233214e..f0b02200aa01 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > > @@ -565,12 +565,49 @@ void intel_uc_reset_prepare(struct intel_uc *uc) > > { > > struct intel_guc *guc = &uc->guc; > > - if (!intel_guc_is_ready(guc)) > > + > > + /* Nothing to do if GuC isn't supported */ > > + if (!intel_uc_supports_guc(uc)) > > return; > > + /* Firmware expected to be running when this function is called */ > > + if (!intel_guc_is_ready(guc)) > > + goto sanitize; > > + > > + if (intel_uc_uses_guc_submission(uc)) > > + intel_guc_submission_reset_prepare(guc); > > + > > +sanitize: > > __uc_sanitize(uc); > > } > > +void intel_uc_reset(struct intel_uc *uc, bool stalled) > > +{ > > + struct intel_guc *guc = &uc->guc; > > + > > + /* Firmware can not be running when this function is called */ > > + if (intel_uc_uses_guc_submission(uc)) > > + intel_guc_submission_reset(guc, stalled); > > +} > > + > > +void intel_uc_reset_finish(struct intel_uc *uc) > > +{ > > + struct intel_guc *guc = &uc->guc; > > + > > + /* Firmware expected to be running when this function is called */ > > + if (intel_guc_is_fw_running(guc) && intel_uc_uses_guc_submission(uc)) > > + intel_guc_submission_reset_finish(guc); > > +} > > + > > +void intel_uc_cancel_requests(struct intel_uc *uc) > > +{ > > + struct intel_guc *guc = &uc->guc; > > + > > + /* Firmware can not be running when this function is called */ > > + if (intel_uc_uses_guc_submission(uc)) > > + intel_guc_submission_cancel_requests(guc); > > +} > > + > > void intel_uc_runtime_suspend(struct intel_uc *uc) > > { > > struct intel_guc *guc = &uc->guc; > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > > index c4cef885e984..eaa3202192ac 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > > @@ -37,6 +37,9 @@ void intel_uc_driver_late_release(struct intel_uc *uc); > > void intel_uc_driver_remove(struct intel_uc *uc); > > void intel_uc_init_mmio(struct intel_uc *uc); > > void intel_uc_reset_prepare(struct intel_uc *uc); > > +void intel_uc_reset(struct intel_uc *uc, bool stalled); > > +void intel_uc_reset_finish(struct intel_uc *uc); > > +void intel_uc_cancel_requests(struct intel_uc *uc); > > void intel_uc_suspend(struct intel_uc *uc); > > void intel_uc_runtime_suspend(struct intel_uc *uc); > > int intel_uc_resume(struct intel_uc *uc); >
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index aef3084e8b16..463a6ae605a0 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -174,8 +174,6 @@ static void gt_sanitize(struct intel_gt *gt, bool force) if (intel_gt_is_wedged(gt)) intel_gt_unset_wedged(gt); - intel_uc_sanitize(>->uc); - for_each_engine(engine, gt, id) if (engine->reset.prepare) engine->reset.prepare(engine); @@ -191,6 +189,8 @@ static void gt_sanitize(struct intel_gt *gt, bool force) __intel_engine_reset(engine, false); } + intel_uc_reset(>->uc, false); + for_each_engine(engine, gt, id) if (engine->reset.finish) engine->reset.finish(engine); @@ -243,6 +243,8 @@ int intel_gt_resume(struct intel_gt *gt) goto err_wedged; } + intel_uc_reset_finish(>->uc); + intel_rps_enable(>->rps); intel_llc_enable(>->llc); diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 72251638d4ea..2987282dff6d 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -826,6 +826,8 @@ static int gt_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask) __intel_engine_reset(engine, stalled_mask & engine->mask); local_bh_enable(); + intel_uc_reset(>->uc, true); + intel_ggtt_restore_fences(gt->ggtt); return err; @@ -850,6 +852,8 @@ static void reset_finish(struct intel_gt *gt, intel_engine_mask_t awake) if (awake & engine->mask) intel_engine_pm_put(engine); } + + intel_uc_reset_finish(>->uc); } static void nop_submit_request(struct i915_request *request) @@ -903,6 +907,7 @@ static void __intel_gt_set_wedged(struct intel_gt *gt) for_each_engine(engine, gt, id) if (engine->reset.cancel) engine->reset.cancel(engine); + intel_uc_cancel_requests(>->uc); local_bh_enable(); reset_finish(gt, awake); @@ -1191,6 +1196,9 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) ENGINE_TRACE(engine, "flags=%lx\n", gt->reset.flags); GEM_BUG_ON(!test_bit(I915_RESET_ENGINE + engine->id, >->reset.flags)); + if (intel_engine_uses_guc(engine)) + return -ENODEV; + if (!intel_engine_pm_get_if_awake(engine)) return 0; @@ -1201,13 +1209,10 @@ int __intel_engine_reset_bh(struct intel_engine_cs *engine, const char *msg) "Resetting %s for %s\n", engine->name, msg); atomic_inc(&engine->i915->gpu_error.reset_engine_count[engine->uabi_class]); - if (intel_engine_uses_guc(engine)) - ret = intel_guc_reset_engine(&engine->gt->uc.guc, engine); - else - ret = intel_gt_reset_engine(engine); + ret = intel_gt_reset_engine(engine); if (ret) { /* If we fail here, we expect to fallback to a global reset */ - ENGINE_TRACE(engine, "Failed to reset, err: %d\n", ret); + ENGINE_TRACE(engine, "Failed to reset %s, err: %d\n", engine->name, ret); goto out; } @@ -1341,7 +1346,8 @@ void intel_gt_handle_error(struct intel_gt *gt, * Try engine reset when available. We fall back to full reset if * single reset fails. */ - if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { + if (!intel_uc_uses_guc_submission(>->uc) && + intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) { local_bh_disable(); for_each_engine_masked(engine, gt, engine_mask, tmp) { BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index 6661dcb02239..9b09395b998f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -572,19 +572,6 @@ int intel_guc_suspend(struct intel_guc *guc) return 0; } -/** - * intel_guc_reset_engine() - ask GuC to reset an engine - * @guc: intel_guc structure - * @engine: engine to be reset - */ -int intel_guc_reset_engine(struct intel_guc *guc, - struct intel_engine_cs *engine) -{ - /* XXX: to be implemented with submission interface rework */ - - return -ENODEV; -} - /** * intel_guc_resume() - notify GuC resuming from suspend state * @guc: the guc diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 3cc566565224..d75a76882a44 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -242,14 +242,16 @@ static inline void intel_guc_disable_msg(struct intel_guc *guc, u32 mask) int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout); -int intel_guc_reset_engine(struct intel_guc *guc, - struct intel_engine_cs *engine); - int intel_guc_deregister_done_process_msg(struct intel_guc *guc, const u32 *msg, u32 len); int intel_guc_sched_done_process_msg(struct intel_guc *guc, const u32 *msg, u32 len); +void intel_guc_submission_reset_prepare(struct intel_guc *guc); +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled); +void intel_guc_submission_reset_finish(struct intel_guc *guc); +void intel_guc_submission_cancel_requests(struct intel_guc *guc); + void intel_guc_load_status(struct intel_guc *guc, struct drm_printer *p); #endif 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 f8a6077fa3f9..0f5823922369 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -141,7 +141,7 @@ context_wait_for_deregister_to_register(struct intel_context *ce) static inline void set_context_wait_for_deregister_to_register(struct intel_context *ce) { - /* Only should be called from guc_lrc_desc_pin() */ + /* Only should be called from guc_lrc_desc_pin() without lock */ ce->guc_state.sched_state |= SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER; } @@ -241,15 +241,31 @@ static int guc_lrc_desc_pool_create(struct intel_guc *guc) static void guc_lrc_desc_pool_destroy(struct intel_guc *guc) { + guc->lrc_desc_pool_vaddr = NULL; i915_vma_unpin_and_release(&guc->lrc_desc_pool, I915_VMA_RELEASE_MAP); } +static inline bool guc_submission_initialized(struct intel_guc *guc) +{ + return guc->lrc_desc_pool_vaddr != NULL; +} + static inline void reset_lrc_desc(struct intel_guc *guc, u32 id) { - struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); + if (likely(guc_submission_initialized(guc))) { + struct guc_lrc_desc *desc = __get_lrc_desc(guc, id); + unsigned long flags; - memset(desc, 0, sizeof(*desc)); - xa_erase_irq(&guc->context_lookup, id); + memset(desc, 0, sizeof(*desc)); + + /* + * xarray API doesn't have xa_erase_irqsave wrapper, so calling + * the lower level functions directly. + */ + xa_lock_irqsave(&guc->context_lookup, flags); + __xa_erase(&guc->context_lookup, id); + xa_unlock_irqrestore(&guc->context_lookup, flags); + } } static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) @@ -260,7 +276,15 @@ static inline bool lrc_desc_registered(struct intel_guc *guc, u32 id) static inline void set_lrc_desc_registered(struct intel_guc *guc, u32 id, struct intel_context *ce) { - xa_store_irq(&guc->context_lookup, id, ce, GFP_ATOMIC); + unsigned long flags; + + /* + * xarray API doesn't have xa_save_irqsave wrapper, so calling the + * lower level functions directly. + */ + xa_lock_irqsave(&guc->context_lookup, flags); + __xa_store(&guc->context_lookup, id, ce, GFP_ATOMIC); + xa_unlock_irqrestore(&guc->context_lookup, flags); } static int guc_submission_send_busy_loop(struct intel_guc* guc, @@ -326,6 +350,8 @@ int intel_guc_wait_for_idle(struct intel_guc *guc, long timeout) true, timeout); } +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop); + static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) { int err; @@ -333,11 +359,22 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) u32 action[3]; int len = 0; u32 g2h_len_dw = 0; - bool enabled = context_enabled(ce); + bool enabled; GEM_BUG_ON(!atomic_read(&ce->guc_id_ref)); GEM_BUG_ON(context_guc_id_invalid(ce)); + /* + * Corner case where the GuC firmware was blown away and reloaded while + * this context was pinned. + */ + if (unlikely(!lrc_desc_registered(guc, ce->guc_id))) { + err = guc_lrc_desc_pin(ce, false); + if (unlikely(err)) + goto out; + } + enabled = context_enabled(ce); + if (!enabled) { action[len++] = INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_SET; action[len++] = ce->guc_id; @@ -360,6 +397,7 @@ static int guc_add_request(struct intel_guc *guc, struct i915_request *rq) intel_context_put(ce); } +out: return err; } @@ -414,15 +452,10 @@ static int guc_dequeue_one_context(struct intel_guc *guc) if (submit) { guc_set_lrc_tail(last); resubmit: - /* - * We only check for -EBUSY here even though it is possible for - * -EDEADLK to be returned. If -EDEADLK is returned, the GuC has - * died and a full GT reset needs to be done. The hangcheck will - * eventually detect that the GuC has died and trigger this - * reset so no need to handle -EDEADLK here. - */ ret = guc_add_request(guc, last); - if (ret == -EBUSY) { + if (unlikely(ret == -EPIPE)) + goto deadlk; + else if (ret == -EBUSY) { tasklet_schedule(&sched_engine->tasklet); guc->stalled_request = last; return false; @@ -432,6 +465,11 @@ static int guc_dequeue_one_context(struct intel_guc *guc) guc->stalled_request = NULL; return submit; + +deadlk: + sched_engine->tasklet.callback = NULL; + tasklet_disable_nosync(&sched_engine->tasklet); + return false; } static void guc_submission_tasklet(struct tasklet_struct *t) @@ -458,27 +496,166 @@ static void cs_irq_handler(struct intel_engine_cs *engine, u16 iir) intel_engine_signal_breadcrumbs(engine); } -static void guc_reset_prepare(struct intel_engine_cs *engine) +static void __guc_context_destroy(struct intel_context *ce); +static void release_guc_id(struct intel_guc *guc, struct intel_context *ce); +static void guc_signal_context_fence(struct intel_context *ce); + +static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc) +{ + struct intel_context *ce; + unsigned long index, flags; + bool pending_disable, pending_enable, deregister, destroyed; + + xa_for_each(&guc->context_lookup, index, ce) { + /* Flush context */ + spin_lock_irqsave(&ce->guc_state.lock, flags); + spin_unlock_irqrestore(&ce->guc_state.lock, flags); + + /* + * Once we are at this point submission_disabled() is guaranteed + * to visible to all callers who set the below flags (see above + * flush and flushes in reset_prepare). If submission_disabled() + * is set, the caller shouldn't set these flags. + */ + + destroyed = context_destroyed(ce); + pending_enable = context_pending_enable(ce); + pending_disable = context_pending_disable(ce); + deregister = context_wait_for_deregister_to_register(ce); + init_sched_state(ce); + + if (pending_enable || destroyed || deregister) { + atomic_dec(&guc->outstanding_submission_g2h); + if (deregister) + guc_signal_context_fence(ce); + if (destroyed) { + release_guc_id(guc, ce); + __guc_context_destroy(ce); + } + if (pending_enable|| deregister) + intel_context_put(ce); + } + + /* Not mutualy exclusive with above if statement. */ + if (pending_disable) { + guc_signal_context_fence(ce); + intel_context_sched_disable_unpin(ce); + atomic_dec(&guc->outstanding_submission_g2h); + intel_context_put(ce); + } + } +} + +static inline bool +submission_disabled(struct intel_guc *guc) +{ + struct i915_sched_engine * const sched_engine = guc->sched_engine; + + return unlikely(!sched_engine || + !__tasklet_is_enabled(&sched_engine->tasklet)); +} + +static void disable_submission(struct intel_guc *guc) +{ + struct i915_sched_engine * const sched_engine = guc->sched_engine; + + if (__tasklet_is_enabled(&sched_engine->tasklet)) { + GEM_BUG_ON(!guc->ct.enabled); + __tasklet_disable_sync_once(&sched_engine->tasklet); + sched_engine->tasklet.callback = NULL; + } +} + +static void enable_submission(struct intel_guc *guc) +{ + struct i915_sched_engine * const sched_engine = guc->sched_engine; + unsigned long flags; + + spin_lock_irqsave(&guc->sched_engine->lock, flags); + sched_engine->tasklet.callback = guc_submission_tasklet; + wmb(); + if (!__tasklet_is_enabled(&sched_engine->tasklet) && + __tasklet_enable(&sched_engine->tasklet)) { + GEM_BUG_ON(!guc->ct.enabled); + + /* And kick in case we missed a new request submission. */ + tasklet_hi_schedule(&sched_engine->tasklet); + } + spin_unlock_irqrestore(&guc->sched_engine->lock, flags); +} + +static void guc_flush_submissions(struct intel_guc *guc) +{ + struct i915_sched_engine * const sched_engine = guc->sched_engine; + unsigned long flags; + + spin_lock_irqsave(&sched_engine->lock, flags); + spin_unlock_irqrestore(&sched_engine->lock, flags); +} + +void intel_guc_submission_reset_prepare(struct intel_guc *guc) { - ENGINE_TRACE(engine, "\n"); + int i; + + if (unlikely(!guc_submission_initialized(guc))) + /* Reset called during driver load? GuC not yet initialised! */ + return; + + disable_submission(guc); + guc->interrupts.disable(guc); + + /* Flush IRQ handler */ + spin_lock_irq(&guc_to_gt(guc)->irq_lock); + spin_unlock_irq(&guc_to_gt(guc)->irq_lock); + + guc_flush_submissions(guc); /* - * Prevent request submission to the hardware until we have - * completed the reset in i915_gem_reset_finish(). If a request - * is completed by one engine, it may then queue a request - * to a second via its execlists->tasklet *just* as we are - * calling engine->init_hw() and also writing the ELSP. - * Turning off the execlists->tasklet until the reset is over - * prevents the race. - */ - __tasklet_disable_sync_once(&engine->sched_engine->tasklet); + * Handle any outstanding G2Hs before reset. Call IRQ handler directly + * each pass as interrupt have been disabled. We always scrub for + * outstanding G2H as it is possible for outstanding_submission_g2h to + * be incremented after the context state update. + */ + for (i = 0; i < 4 && atomic_read(&guc->outstanding_submission_g2h); ++i) { + intel_guc_to_host_event_handler(guc); +#define wait_for_reset(guc, wait_var) \ + guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20)) + do { + wait_for_reset(guc, &guc->outstanding_submission_g2h); + } while (!list_empty(&guc->ct.requests.incoming)); + } + scrub_guc_desc_for_outstanding_g2h(guc); +} + +static struct intel_engine_cs * +guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) +{ + struct intel_engine_cs *engine; + intel_engine_mask_t tmp, mask = ve->mask; + unsigned int num_siblings = 0; + + for_each_engine_masked(engine, ve->gt, mask, tmp) + if (num_siblings++ == sibling) + return engine; + + return NULL; +} + +static inline struct intel_engine_cs * +__context_to_physical_engine(struct intel_context *ce) +{ + struct intel_engine_cs *engine = ce->engine; + + if (intel_engine_is_virtual(engine)) + engine = guc_virtual_get_sibling(engine, 0); + + return engine; } -static void guc_reset_state(struct intel_context *ce, - struct intel_engine_cs *engine, - u32 head, - bool scrub) +static void guc_reset_state(struct intel_context *ce, u32 head, bool scrub) { + struct intel_engine_cs *engine = __context_to_physical_engine(ce); + GEM_BUG_ON(!intel_context_is_pinned(ce)); /* @@ -496,42 +673,147 @@ static void guc_reset_state(struct intel_context *ce, lrc_update_regs(ce, engine, head); } -static void guc_reset_rewind(struct intel_engine_cs *engine, bool stalled) +static void guc_reset_nop(struct intel_engine_cs *engine) { - struct intel_engine_execlists * const execlists = &engine->execlists; - struct i915_request *rq; +} + +static void guc_rewind_nop(struct intel_engine_cs *engine, bool stalled) +{ +} + +static void +__unwind_incomplete_requests(struct intel_context *ce) +{ + struct i915_request *rq, *rn; + struct list_head *pl; + int prio = I915_PRIORITY_INVALID; + struct i915_sched_engine * const sched_engine = + ce->engine->sched_engine; unsigned long flags; - spin_lock_irqsave(&engine->sched_engine->lock, flags); + spin_lock_irqsave(&sched_engine->lock, flags); + spin_lock(&ce->guc_active.lock); + list_for_each_entry_safe(rq, rn, + &ce->guc_active.requests, + sched.link) { + if (i915_request_completed(rq)) + continue; + + list_del_init(&rq->sched.link); + spin_unlock(&ce->guc_active.lock); + + __i915_request_unsubmit(rq); + + /* Push the request back into the queue for later resubmission. */ + GEM_BUG_ON(rq_prio(rq) == I915_PRIORITY_INVALID); + if (rq_prio(rq) != prio) { + prio = rq_prio(rq); + pl = i915_sched_lookup_priolist(sched_engine, prio); + } + GEM_BUG_ON(i915_sched_engine_is_empty(sched_engine)); - /* Push back any incomplete requests for replay after the reset. */ - rq = execlists_unwind_incomplete_requests(execlists); - if (!rq) - goto out_unlock; + list_add_tail(&rq->sched.link, pl); + set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags); + + spin_lock(&ce->guc_active.lock); + } + spin_unlock(&ce->guc_active.lock); + spin_unlock_irqrestore(&sched_engine->lock, flags); +} + +static struct i915_request *context_find_active_request(struct intel_context *ce) +{ + struct i915_request *rq, *active = NULL; + unsigned long flags; + + spin_lock_irqsave(&ce->guc_active.lock, flags); + list_for_each_entry_reverse(rq, &ce->guc_active.requests, + sched.link) { + if (i915_request_completed(rq)) + break; + + active = rq; + } + spin_unlock_irqrestore(&ce->guc_active.lock, flags); + + return active; +} + +static void __guc_reset_context(struct intel_context *ce, bool stalled) +{ + struct i915_request *rq; + u32 head; + + /* + * GuC will implicitly mark the context as non-schedulable + * when it sends the reset notification. Make sure our state + * reflects this change. The context will be marked enabled + * on resubmission. + */ + clr_context_enabled(ce); + + rq = context_find_active_request(ce); + if (!rq) { + head = ce->ring->tail; + stalled = false; + goto out_replay; + } if (!i915_request_started(rq)) stalled = false; + GEM_BUG_ON(i915_active_is_idle(&ce->active)); + head = intel_ring_wrap(ce->ring, rq->head); __i915_request_reset(rq, stalled); - guc_reset_state(rq->context, engine, rq->head, stalled); -out_unlock: - spin_unlock_irqrestore(&engine->sched_engine->lock, flags); +out_replay: + guc_reset_state(ce, head, stalled); + __unwind_incomplete_requests(ce); +} + +void intel_guc_submission_reset(struct intel_guc *guc, bool stalled) +{ + struct intel_context *ce; + unsigned long index; + + if (unlikely(!guc_submission_initialized(guc))) + /* Reset called during driver load? GuC not yet initialised! */ + return; + + xa_for_each(&guc->context_lookup, index, ce) + if (intel_context_is_pinned(ce)) + __guc_reset_context(ce, stalled); + + /* GuC is blown away, drop all references to contexts */ + xa_destroy(&guc->context_lookup); +} + +static void guc_cancel_context_requests(struct intel_context *ce) +{ + struct i915_sched_engine *sched_engine = ce_to_guc(ce)->sched_engine; + struct i915_request *rq; + unsigned long flags; + + /* Mark all executing requests as skipped. */ + spin_lock_irqsave(&sched_engine->lock, flags); + spin_lock(&ce->guc_active.lock); + list_for_each_entry(rq, &ce->guc_active.requests, sched.link) + i915_request_put(i915_request_mark_eio(rq)); + spin_unlock(&ce->guc_active.lock); + spin_unlock_irqrestore(&sched_engine->lock, flags); } -static void guc_reset_cancel(struct intel_engine_cs *engine) +static void +guc_cancel_sched_engine_requests(struct i915_sched_engine *sched_engine) { - struct i915_sched_engine * const sched_engine = engine->sched_engine; struct i915_request *rq, *rn; struct rb_node *rb; unsigned long flags; /* Can be called during boot if GuC fails to load */ - if (!engine->gt) + if (!sched_engine) return; - ENGINE_TRACE(engine, "\n"); - /* * Before we call engine->cancel_requests(), we should have exclusive * access to the submission state. This is arranged for us by the @@ -548,21 +830,16 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) */ spin_lock_irqsave(&sched_engine->lock, flags); - /* Mark all executing requests as skipped. */ - list_for_each_entry(rq, &sched_engine->requests, sched.link) { - i915_request_set_error_once(rq, -EIO); - i915_request_mark_complete(rq); - } - /* Flush the queued requests to the timeline list (for retiring). */ while ((rb = rb_first_cached(&sched_engine->queue))) { struct i915_priolist *p = to_priolist(rb); priolist_for_each_request_consume(rq, rn, p) { list_del_init(&rq->sched.link); + __i915_request_submit(rq); - dma_fence_set_error(&rq->fence, -EIO); - i915_request_mark_complete(rq); + + i915_request_put(i915_request_mark_eio(rq)); } rb_erase_cached(&p->node, &sched_engine->queue); @@ -577,14 +854,38 @@ static void guc_reset_cancel(struct intel_engine_cs *engine) spin_unlock_irqrestore(&sched_engine->lock, flags); } -static void guc_reset_finish(struct intel_engine_cs *engine) +void intel_guc_submission_cancel_requests(struct intel_guc *guc) { - if (__tasklet_enable(&engine->sched_engine->tasklet)) - /* And kick in case we missed a new request submission. */ - tasklet_hi_schedule(&engine->sched_engine->tasklet); + struct intel_context *ce; + unsigned long index; - ENGINE_TRACE(engine, "depth->%d\n", - atomic_read(&engine->sched_engine->tasklet.count)); + xa_for_each(&guc->context_lookup, index, ce) + if (intel_context_is_pinned(ce)) + guc_cancel_context_requests(ce); + + guc_cancel_sched_engine_requests(guc->sched_engine); + + /* GuC is blown away, drop all references to contexts */ + xa_destroy(&guc->context_lookup); +} + +void intel_guc_submission_reset_finish(struct intel_guc *guc) +{ + /* Reset called during driver load or during wedge? */ + if (unlikely(!guc_submission_initialized(guc) || + test_bit(I915_WEDGED, &guc_to_gt(guc)->reset.flags))) + return; + + /* + * Technically possible for either of these values to be non-zero here, + * but very unlikely + harmless. Regardless let's add a warn so we can + * see in CI if this happens frequently / a precursor to taking down the + * machine. + */ + GEM_WARN_ON(atomic_read(&guc->outstanding_submission_g2h)); + atomic_set(&guc->outstanding_submission_g2h, 0); + + enable_submission(guc); } /* @@ -651,6 +952,9 @@ static int guc_bypass_tasklet_submit(struct intel_guc *guc, else trace_i915_request_guc_submit(rq); + if (unlikely(ret == -EPIPE)) + disable_submission(guc); + return ret; } @@ -663,7 +967,8 @@ static void guc_submit_request(struct i915_request *rq) /* Will be called from irq-context when using foreign fences. */ spin_lock_irqsave(&sched_engine->lock, flags); - if (guc->stalled_request || !i915_sched_engine_is_empty(sched_engine)) + if (submission_disabled(guc) || guc->stalled_request || + !i915_sched_engine_is_empty(sched_engine)) queue_request(sched_engine, rq, rq_prio(rq)); else if (guc_bypass_tasklet_submit(guc, rq) == -EBUSY) tasklet_hi_schedule(&sched_engine->tasklet); @@ -800,7 +1105,8 @@ static void unpin_guc_id(struct intel_guc *guc, struct intel_context *ce) static int __guc_action_register_context(struct intel_guc *guc, u32 guc_id, - u32 offset) + u32 offset, + bool loop) { u32 action[] = { INTEL_GUC_ACTION_REGISTER_CONTEXT, @@ -809,10 +1115,10 @@ static int __guc_action_register_context(struct intel_guc *guc, }; return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), - 0, true); + 0, loop); } -static int register_context(struct intel_context *ce) +static int register_context(struct intel_context *ce, bool loop) { struct intel_guc *guc = ce_to_guc(ce); u32 offset = intel_guc_ggtt_offset(guc, guc->lrc_desc_pool) + @@ -820,11 +1126,12 @@ static int register_context(struct intel_context *ce) trace_intel_context_register(ce); - return __guc_action_register_context(guc, ce->guc_id, offset); + return __guc_action_register_context(guc, ce->guc_id, offset, loop); } static int __guc_action_deregister_context(struct intel_guc *guc, - u32 guc_id) + u32 guc_id, + bool loop) { u32 action[] = { INTEL_GUC_ACTION_DEREGISTER_CONTEXT, @@ -833,16 +1140,16 @@ static int __guc_action_deregister_context(struct intel_guc *guc, return guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), G2H_LEN_DW_DEREGISTER_CONTEXT, - true); + loop); } -static int deregister_context(struct intel_context *ce, u32 guc_id) +static int deregister_context(struct intel_context *ce, u32 guc_id, bool loop) { struct intel_guc *guc = ce_to_guc(ce); trace_intel_context_deregister(ce); - return __guc_action_deregister_context(guc, guc_id); + return __guc_action_deregister_context(guc, guc_id, loop); } static intel_engine_mask_t adjust_engine_mask(u8 class, intel_engine_mask_t mask) @@ -871,7 +1178,7 @@ static void guc_context_policy_init(struct intel_engine_cs *engine, desc->preemption_timeout = CONTEXT_POLICY_DEFAULT_PREEMPTION_TIME_US; } -static int guc_lrc_desc_pin(struct intel_context *ce) +static int guc_lrc_desc_pin(struct intel_context *ce, bool loop) { struct intel_runtime_pm *runtime_pm = &ce->engine->gt->i915->runtime_pm; @@ -917,18 +1224,45 @@ static int guc_lrc_desc_pin(struct intel_context *ce) */ if (context_registered) { trace_intel_context_steal_guc_id(ce); - set_context_wait_for_deregister_to_register(ce); - intel_context_get(ce); + if (!loop) { + set_context_wait_for_deregister_to_register(ce); + intel_context_get(ce); + } else { + bool disabled; + unsigned long flags; + + /* Seal race with Reset */ + spin_lock_irqsave(&ce->guc_state.lock, flags); + disabled = submission_disabled(guc); + if (likely(!disabled)) { + set_context_wait_for_deregister_to_register(ce); + intel_context_get(ce); + } + spin_unlock_irqrestore(&ce->guc_state.lock, flags); + if (unlikely(disabled)) { + reset_lrc_desc(guc, desc_idx); + return 0; /* Will get registered later */ + } + } /* * If stealing the guc_id, this ce has the same guc_id as the * context whos guc_id was stole. */ with_intel_runtime_pm(runtime_pm, wakeref) - ret = deregister_context(ce, ce->guc_id); + ret = deregister_context(ce, ce->guc_id, loop); + if (unlikely(ret == -EBUSY)) { + clr_context_wait_for_deregister_to_register(ce); + intel_context_put(ce); + } else if (unlikely(ret == -ENODEV)) + ret = 0; /* Will get registered later */ } else { with_intel_runtime_pm(runtime_pm, wakeref) - ret = register_context(ce); + ret = register_context(ce, loop); + if (unlikely(ret == -EBUSY)) + reset_lrc_desc(guc, desc_idx); + else if (unlikely(ret == -ENODEV)) + ret = 0; /* Will get registered later */ } return ret; @@ -991,7 +1325,6 @@ static void __guc_context_sched_disable(struct intel_guc *guc, GEM_BUG_ON(guc_id == GUC_INVALID_LRC_ID); trace_intel_context_sched_disable(ce); - intel_context_get(ce); guc_submission_send_busy_loop(guc, action, ARRAY_SIZE(action), G2H_LEN_DW_SCHED_CONTEXT_MODE_SET, true); @@ -1003,6 +1336,7 @@ static u16 prep_context_pending_disable(struct intel_context *ce) set_context_pending_disable(ce); clr_context_enabled(ce); + intel_context_get(ce); return ce->guc_id; } @@ -1015,7 +1349,7 @@ static void guc_context_sched_disable(struct intel_context *ce) u16 guc_id; intel_wakeref_t wakeref; - if (context_guc_id_invalid(ce) || + if (submission_disabled(guc) || context_guc_id_invalid(ce) || !lrc_desc_registered(guc, ce->guc_id)) { clr_context_enabled(ce); goto unpin; @@ -1036,6 +1370,7 @@ static void guc_context_sched_disable(struct intel_context *ce) * a request before we set the 'context_pending_disable' flag here. */ if (unlikely(atomic_add_unless(&ce->pin_count, -2, 2))) { + spin_unlock_irqrestore(&ce->guc_state.lock, flags); return; } guc_id = prep_context_pending_disable(ce); @@ -1052,19 +1387,13 @@ static void guc_context_sched_disable(struct intel_context *ce) static inline void guc_lrc_desc_unpin(struct intel_context *ce) { - struct intel_engine_cs *engine = ce->engine; - struct intel_guc *guc = &engine->gt->uc.guc; - unsigned long flags; + struct intel_guc *guc = ce_to_guc(ce); GEM_BUG_ON(!lrc_desc_registered(guc, ce->guc_id)); GEM_BUG_ON(ce != __get_context(guc, ce->guc_id)); GEM_BUG_ON(context_enabled(ce)); - spin_lock_irqsave(&ce->guc_state.lock, flags); - set_context_destroyed(ce); - spin_unlock_irqrestore(&ce->guc_state.lock, flags); - - deregister_context(ce, ce->guc_id); + deregister_context(ce, ce->guc_id, true); } static void __guc_context_destroy(struct intel_context *ce) @@ -1092,13 +1421,15 @@ static void guc_context_destroy(struct kref *kref) struct intel_guc *guc = &ce->engine->gt->uc.guc; intel_wakeref_t wakeref; unsigned long flags; + bool disabled; /* * 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. */ - if (context_guc_id_invalid(ce) || + if (submission_disabled(guc) || + context_guc_id_invalid(ce) || !lrc_desc_registered(guc, ce->guc_id)) { release_guc_id(guc, ce); __guc_context_destroy(ce); @@ -1125,6 +1456,18 @@ static void guc_context_destroy(struct kref *kref) list_del_init(&ce->guc_id_link); spin_unlock_irqrestore(&guc->contexts_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); + spin_unlock_irqrestore(&ce->guc_state.lock, flags); + if (unlikely(disabled)) { + release_guc_id(guc, ce); + __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 @@ -1212,8 +1555,6 @@ static void guc_signal_context_fence(struct intel_context *ce) { unsigned long flags; - GEM_BUG_ON(!context_wait_for_deregister_to_register(ce)); - spin_lock_irqsave(&ce->guc_state.lock, flags); clr_context_wait_for_deregister_to_register(ce); __guc_signal_context_fence(ce); @@ -1222,8 +1563,9 @@ static void guc_signal_context_fence(struct intel_context *ce) static bool context_needs_register(struct intel_context *ce, bool new_guc_id) { - return new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || - !lrc_desc_registered(ce_to_guc(ce), ce->guc_id); + return (new_guc_id || test_bit(CONTEXT_LRCA_DIRTY, &ce->flags) || + !lrc_desc_registered(ce_to_guc(ce), ce->guc_id)) && + !submission_disabled(ce_to_guc(ce)); } static int guc_request_alloc(struct i915_request *rq) @@ -1281,8 +1623,12 @@ static int guc_request_alloc(struct i915_request *rq) if (unlikely(ret < 0)) return ret;; if (context_needs_register(ce, !!ret)) { - ret = guc_lrc_desc_pin(ce); + ret = guc_lrc_desc_pin(ce, true); if (unlikely(ret)) { /* unwind */ + if (ret == -EPIPE) { + disable_submission(guc); + goto out; /* GPU will be reset */ + } atomic_dec(&ce->guc_id_ref); unpin_guc_id(guc, ce); return ret; @@ -1319,20 +1665,6 @@ static int guc_request_alloc(struct i915_request *rq) return 0; } -static struct intel_engine_cs * -guc_virtual_get_sibling(struct intel_engine_cs *ve, unsigned int sibling) -{ - struct intel_engine_cs *engine; - intel_engine_mask_t tmp, mask = ve->mask; - unsigned int num_siblings = 0; - - for_each_engine_masked(engine, ve->gt, mask, tmp) - if (num_siblings++ == sibling) - return engine; - - return NULL; -} - static int guc_virtual_context_pre_pin(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void **vaddr) @@ -1528,7 +1860,7 @@ static inline void guc_kernel_context_pin(struct intel_guc *guc, { if (context_guc_id_invalid(ce)) pin_guc_id(guc, ce); - guc_lrc_desc_pin(ce); + guc_lrc_desc_pin(ce, true); } static inline void guc_init_lrc_mapping(struct intel_guc *guc) @@ -1599,10 +1931,10 @@ static void guc_default_vfuncs(struct intel_engine_cs *engine) engine->sched_engine->schedule = i915_schedule; - engine->reset.prepare = guc_reset_prepare; - engine->reset.rewind = guc_reset_rewind; - engine->reset.cancel = guc_reset_cancel; - engine->reset.finish = guc_reset_finish; + engine->reset.prepare = guc_reset_nop; + engine->reset.rewind = guc_rewind_nop; + engine->reset.cancel = guc_reset_nop; + engine->reset.finish = guc_reset_nop; engine->emit_flush = gen8_emit_flush_xcs; engine->emit_init_breadcrumb = gen8_emit_init_breadcrumb; @@ -1651,6 +1983,17 @@ static inline void guc_default_irqs(struct intel_engine_cs *engine) intel_engine_set_irq_handler(engine, cs_irq_handler); } +static void guc_sched_engine_destroy(struct kref *kref) +{ + struct i915_sched_engine *sched_engine = + container_of(kref, typeof(*sched_engine), ref); + struct intel_guc *guc = sched_engine->private_data; + + guc->sched_engine = NULL; + tasklet_kill(&sched_engine->tasklet); /* flush the callback */ + kfree(sched_engine); +} + int intel_guc_submission_setup(struct intel_engine_cs *engine) { struct drm_i915_private *i915 = engine->i915; @@ -1669,6 +2012,7 @@ int intel_guc_submission_setup(struct intel_engine_cs *engine) guc->sched_engine->schedule = i915_schedule; guc->sched_engine->private_data = guc; + guc->sched_engine->destroy = guc_sched_engine_destroy; tasklet_setup(&guc->sched_engine->tasklet, guc_submission_tasklet); } @@ -1775,7 +2119,7 @@ int intel_guc_deregister_done_process_msg(struct intel_guc *guc, * register this context. */ with_intel_runtime_pm(runtime_pm, wakeref) - register_context(ce); + register_context(ce, true); guc_signal_context_fence(ce); intel_context_put(ce); } else if (context_destroyed(ce)) { diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index 6d8b9233214e..f0b02200aa01 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -565,12 +565,49 @@ void intel_uc_reset_prepare(struct intel_uc *uc) { struct intel_guc *guc = &uc->guc; - if (!intel_guc_is_ready(guc)) + + /* Nothing to do if GuC isn't supported */ + if (!intel_uc_supports_guc(uc)) return; + /* Firmware expected to be running when this function is called */ + if (!intel_guc_is_ready(guc)) + goto sanitize; + + if (intel_uc_uses_guc_submission(uc)) + intel_guc_submission_reset_prepare(guc); + +sanitize: __uc_sanitize(uc); } +void intel_uc_reset(struct intel_uc *uc, bool stalled) +{ + struct intel_guc *guc = &uc->guc; + + /* Firmware can not be running when this function is called */ + if (intel_uc_uses_guc_submission(uc)) + intel_guc_submission_reset(guc, stalled); +} + +void intel_uc_reset_finish(struct intel_uc *uc) +{ + struct intel_guc *guc = &uc->guc; + + /* Firmware expected to be running when this function is called */ + if (intel_guc_is_fw_running(guc) && intel_uc_uses_guc_submission(uc)) + intel_guc_submission_reset_finish(guc); +} + +void intel_uc_cancel_requests(struct intel_uc *uc) +{ + struct intel_guc *guc = &uc->guc; + + /* Firmware can not be running when this function is called */ + if (intel_uc_uses_guc_submission(uc)) + intel_guc_submission_cancel_requests(guc); +} + void intel_uc_runtime_suspend(struct intel_uc *uc) { struct intel_guc *guc = &uc->guc; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h index c4cef885e984..eaa3202192ac 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h @@ -37,6 +37,9 @@ void intel_uc_driver_late_release(struct intel_uc *uc); void intel_uc_driver_remove(struct intel_uc *uc); void intel_uc_init_mmio(struct intel_uc *uc); void intel_uc_reset_prepare(struct intel_uc *uc); +void intel_uc_reset(struct intel_uc *uc, bool stalled); +void intel_uc_reset_finish(struct intel_uc *uc); +void intel_uc_cancel_requests(struct intel_uc *uc); void intel_uc_suspend(struct intel_uc *uc); void intel_uc_runtime_suspend(struct intel_uc *uc); int intel_uc_resume(struct intel_uc *uc);
Reset implementation for new GuC interface. This is the legacy reset implementation which is called when the i915 owns the engine hang check. Future patches will offload the engine hang check to GuC but we will continue to maintain this legacy path as a fallback and this code path is also required if the GuC dies. With the new GuC interface it is not possible to reset individual engines - it is only possible to reset the GPU entirely. This patch forces an entire chip reset if any engine hangs. v2: (Michal) - Check for -EPIPE rather than -EIO (CT deadlock/corrupt check) v3: (John H) - Split into a series of smaller patches Cc: John Harrison <john.c.harrison@intel.com> Signed-off-by: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 6 +- drivers/gpu/drm/i915/gt/intel_reset.c | 18 +- drivers/gpu/drm/i915/gt/uc/intel_guc.c | 13 - drivers/gpu/drm/i915/gt/uc/intel_guc.h | 8 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 562 ++++++++++++++---- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 39 +- drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 + 7 files changed, 515 insertions(+), 134 deletions(-)