Re: [PATCH 02/11] drm/xe: Reset tlb fence timeout on invalid seqno received

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

On Tue, Nov 18, 2025 at 05:05:43PM +0800, Brian Nguyen wrote:
> TLB_INVALIDATION_SEQNO_INVALID are now used to indicate in progress
> multi-step TLB invalidations, so reset tdr to ensure that action
> won't prematurely trigger when G2H actions are still ongoing.
> 

I think thid patch makes sense but comments below.

> Signed-off-by: Brian Nguyen <brian3.nguyen@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_tlb_inval.c |  2 ++
>  drivers/gpu/drm/xe/xe_tlb_inval.c     | 16 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_tlb_inval.h     |  1 +
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> index f1fd2dd90742..cd126c53faab 100644
> --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> @@ -238,6 +238,8 @@ int xe_guc_tlb_inval_done_handler(struct xe_guc *guc, u32 *msg, u32 len)
>  
>  	if (msg[0] != TLB_INVALIDATION_SEQNO_INVALID)
>  		xe_tlb_inval_done_handler(&gt->tlb_inval, msg[0]);
> +	else
> +		xe_tlb_inval_reset_timeout(&gt->tlb_inval);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tlb_inval.c
> index 918a59e686ea..50f05d6b5672 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> @@ -199,6 +199,22 @@ void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval)
>  	mutex_unlock(&tlb_inval->seqno_lock);
>  }
>  
> +/**
> + * xe_tlb_inval_reset_timeout() - Reset TLB inval fence timeout
> + * @tlb_inval: TLB invalidation client
> + *
> + * Reset the TLB invalidation timeout timer.
> + */
> +void xe_tlb_inval_reset_timeout(struct xe_tlb_inval *tlb_inval)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&tlb_inval->pending_lock, flags);
> +	mod_delayed_work(system_wq, &tlb_inval->fence_tdr,
> +			 tlb_inval->ops->timeout_delay(tlb_inval));

You don't need a lock for this. It is done in xe_tlb_inval_done_handler
under this lock as the pending list of TLB invalidations, which
pending_lock protects, is tied to whether we want to reschedule the
timeout or cancel it. So please drop the lock here and then also update
xe_tlb_inval_done_handler to call this new helper.

Matt

> +	spin_unlock_irqrestore(&tlb_inval->pending_lock, flags);
> +}
> +
>  static bool xe_tlb_inval_seqno_past(struct xe_tlb_inval *tlb_inval, int seqno)
>  {
>  	int seqno_recv = READ_ONCE(tlb_inval->seqno_recv);
> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.h b/drivers/gpu/drm/xe/xe_tlb_inval.h
> index 05614915463a..9dbddc310eb9 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval.h
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval.h
> @@ -17,6 +17,7 @@ struct xe_vm;
>  int xe_gt_tlb_inval_init_early(struct xe_gt *gt);
>  
>  void xe_tlb_inval_reset(struct xe_tlb_inval *tlb_inval);
> +void xe_tlb_inval_reset_timeout(struct xe_tlb_inval *tlb_inval);
>  int xe_tlb_inval_all(struct xe_tlb_inval *tlb_inval,
>  		     struct xe_tlb_inval_fence *fence);
>  int xe_tlb_inval_ggtt(struct xe_tlb_inval *tlb_inval);
> -- 
> 2.51.2
>