Re: [PATCH] drm/xe: Wait for HW clearance before issuing the next TLB inval.

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Mar 17, 2026 at 05:28:14PM -0600, Summers, Stuart wrote:
> On Tue, 2026-03-17 at 16:21 -0700, fei.yang@intel.com wrote:
> > From: Fei Yang <fei.yang@intel.com>
> > 
> > Hardware requires the software to poll the valid bit and make sure
> > it's
> > cleared before issuing a new TLB invalidation request.
> > 
> > Signed-off-by: Fei Yang <fei.yang@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_tlb_inval.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > index ced58f46f846..4c2f87db3167 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > @@ -63,6 +63,7 @@ static int send_tlb_inval_ggtt(struct xe_tlb_inval
> > *tlb_inval, u32 seqno)
> >         struct xe_guc *guc = tlb_inval->private;
> >         struct xe_gt *gt = guc_to_gt(guc);
> >         struct xe_device *xe = guc_to_xe(guc);
> > +       int ret;
> >  
> >         /*
> >          * Returning -ECANCELED in this function is squashed at the
> > caller and
> > @@ -85,11 +86,25 @@ static int send_tlb_inval_ggtt(struct
> > xe_tlb_inval *tlb_inval, u32 seqno)
> >  
> >                 CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GT);
> >                 if (xe->info.platform == XE_PVC || GRAPHICS_VER(xe)
> > >= 20) {
> > +                       /* Wait 1-second for the valid bit to be
> > cleared */
> > +                       ret = xe_mmio_wait32(mmio,
> > PVC_GUC_TLB_INV_DESC0, PVC_GUC_TLB_INV_DESC0_VALID,
> > +                                            0, 1000 * USEC_PER_MSEC,
> > NULL, false);
> > +                       if (ret) {
> > +                               pr_info("TLB INVAL cancelled due to
> > uncleared valid bit\n");
> > +                               return -ECANCELED;
> > +                       }
> 
> Is there a reason we aren't waiting after the write to make sure the
> invalidation completed? It seems like we should be serializing these
> and at least making sure hardware completes the request rather than
> just sending and hoping for the best.

Yes, this is correct—we should after wait issue *if* this code is
actually needed.

This is early Xe code from me, and it’s questionable whether it’s even
required. Typically, if the CTs are not live, the GuC isn’t doing
anything meaningful in terms of referencing memory that the KMD is
moving around (which would require an invalidation). So this entire flow
of issuing a GAM port TLB invalidation is, again, questionable.

So I'd suggest move the wait after issue, plus throw in:

“XXX: Why do we need to invalidate GGTT memory when the CTs are not
live? This suggests the GuC is still in the load phase. Investigate and
remove this code once confirmed.'

Matt

> 
> Thanks,
> Stuart
> 
> >                         xe_mmio_write32(mmio, PVC_GUC_TLB_INV_DESC1,
> >                                         PVC_GUC_TLB_INV_DESC1_INVALID
> > ATE);
> >                         xe_mmio_write32(mmio, PVC_GUC_TLB_INV_DESC0,
> >                                         PVC_GUC_TLB_INV_DESC0_VALID);
> >                 } else {
> > +                       /* Wait 1-second for the valid bit to be
> > cleared */
> > +                       ret = xe_mmio_wait32(mmio, GUC_TLB_INV_CR,
> > GUC_TLB_INV_CR_INVALIDATE,
> > +                                            0, 1000 * USEC_PER_MSEC,
> > NULL, false);
> > +                       if (ret) {
> > +                               pr_info("TLB INVAL cancelled due to
> > uncleared valid bit\n");
> > +                               return -ECANCELED;
> > +                       }
> >                         xe_mmio_write32(mmio, GUC_TLB_INV_CR,
> >                                         GUC_TLB_INV_CR_INVALIDATE);
> >                 }
>