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

["Summers, Stuart" <stuart.summers@intel.com>]

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.

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