-----Original Message-----
From: Harrison, John C <[email protected]>
Sent: Tuesday, October 10, 2023 2:57 PM
To: Cavitt, Jonathan <[email protected]>;
[email protected]
Cc: Gupta, saurabhg <[email protected]>; [email protected];
Iddamsetty, Aravind <[email protected]>; Yang, Fei
<[email protected]>; Shyti, Andi <[email protected]>; Das, Nirmoy
<[email protected]>; Krzysztofik, Janusz <[email protected]>;
Roper, Matthew D <[email protected]>; [email protected];
[email protected]
Subject: Re: [PATCH v10 4/7] drm/i915: No TLB invalidation on suspended GT
>
> On 10/10/2023 08:02, Jonathan Cavitt wrote:
> > In case of GT is suspended, don't allow submission of new TLB invalidation
> > request and cancel all pending requests. The TLB entries will be
> > invalidated either during GuC reload or on system resume.
> >
> > Signed-off-by: Fei Yang <[email protected]>
> > Signed-off-by: Jonathan Cavitt <[email protected]>
> > CC: John Harrison <[email protected]>
> > ---
> > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 1 +
> > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 21 +++++++++++++------
> > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 7 +++++++
> > 3 files changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > index 06c44f5c28776..ff7e7b90fd49b 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > @@ -536,4 +536,5 @@ void intel_guc_dump_time_info(struct intel_guc *guc,
> > struct drm_printer *p);
> >
> > int intel_guc_sched_disable_gucid_threshold_max(struct intel_guc *guc);
> >
> > +void wake_up_all_tlb_invalidate(struct intel_guc *guc);
> > #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 e9854652c2b52..b9c168ea57270 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1796,13 +1796,25 @@ static void __guc_reset_context(struct
> > intel_context *ce, intel_engine_mask_t st
> > intel_context_put(parent);
> > }
> >
> > -void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t
> > stalled)
> > +void wake_up_all_tlb_invalidate(struct intel_guc *guc)
> > {
> > struct intel_guc_tlb_wait *wait;
> > + unsigned long i;
> > +
> > + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915))
> > + return;
> > +
> > + xa_lock_irq(&guc->tlb_lookup);
> > + xa_for_each(&guc->tlb_lookup, i, wait)
> > + wake_up(&wait->wq);
> > + xa_unlock_irq(&guc->tlb_lookup);
> > +}
> > +
> > +void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t
> > stalled)
> What is changed on this line? Or is it just diff being confused and
> seeing the move of the 'wait' declaration as being the anchor point
> rather than the function declaration?
It's the latter, yes. Diff is confused.
-Jonathan Cavitt
>
> John.
>
>
> > +{
> > struct intel_context *ce;
> > unsigned long index;
> > unsigned long flags;
> > - unsigned long i;
> >
> > if (unlikely(!guc_submission_initialized(guc))) {
> > /* Reset called during driver load? GuC not yet initialised! */
> > @@ -1833,10 +1845,7 @@ void intel_guc_submission_reset(struct intel_guc
> > *guc, intel_engine_mask_t stall
> > * The full GT reset will have cleared the TLB caches and flushed the
> > * G2H message queue; we can release all the blocked waiters.
> > */
> > - xa_lock_irq(&guc->tlb_lookup);
> > - xa_for_each(&guc->tlb_lookup, i, wait)
> > - wake_up(&wait->wq);
> > - xa_unlock_irq(&guc->tlb_lookup);
> > + wake_up_all_tlb_invalidate(guc);
> > }
> >
> > static void guc_cancel_context_requests(struct intel_context *ce)
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index 98b103375b7ab..750cb63503dd7 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -688,6 +688,8 @@ void intel_uc_suspend(struct intel_uc *uc)
> > /* flush the GSC worker */
> > intel_gsc_uc_flush_work(&uc->gsc);
> >
> > + wake_up_all_tlb_invalidate(guc);
> > +
> > if (!intel_guc_is_ready(guc)) {
> > guc->interrupts.enabled = false;
> > return;
> > @@ -736,6 +738,11 @@ static int __uc_resume(struct intel_uc *uc, bool
> > enable_communication)
> >
> > intel_gsc_uc_resume(&uc->gsc);
> >
> > + if (HAS_GUC_TLB_INVALIDATION(gt->i915)) {
> > + intel_guc_invalidate_tlb_engines(guc);
> > + intel_guc_invalidate_tlb_guc(guc);
> > + }
> > +
> > return 0;
> > }
> >
>
>
