Re: [PATCH] drm/xe: Add and use TLBINVAL component

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

On Wed, 2026-09-23 at 18:39 +0200, Michal Wajdeczko wrote:
> 
> 
> On 9/22/2026 9:22 PM, Summers, Stuart wrote:
> > On Tue, 2026-09-22 at 19:19 +0200, Michal Wajdeczko wrote:
> > > Replace the existing TLB invalidation error message with a new
> > > one
> > > based on the xe_log_err() macro and use the dedicated TLBINVAL
> > > log
> > > component.
> > > 
> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > ---
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > Cc: Tales A. Mendonça <talesam@gmail.com>
> > > ---
> > >  drivers/gpu/drm/xe/abi/xe_log_abi.h | 1 +
> > >  drivers/gpu/drm/xe/xe_tlb_inval.c   | 6 +++---
> > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/abi/xe_log_abi.h
> > > b/drivers/gpu/drm/xe/abi/xe_log_abi.h
> > > index 526aadf85fe9..adb3c2128251 100644
> > > --- a/drivers/gpu/drm/xe/abi/xe_log_abi.h
> > > +++ b/drivers/gpu/drm/xe/abi/xe_log_abi.h
> > > @@ -145,6 +145,7 @@ enum xe_log_location_bits {
> > >         define(DRIVER, 5, WA, SW,
> > > "Workarounds")                                \
> > >         define(DRIVER, 6, PAGEFAULT, MEM_FAULT, "Page
> > > Fault")                   \
> > >         define(DRIVER, 7, GUCSUBMIT, GT_TDR, "GuC
> > > Submission")                  \
> > > +       define(DRIVER, 8, TLBINVAL, GT_TDR, "TLB
> > > Invalidation")                 \
> > >         /*
> > > */                                                               
> > >     
> > > \
> > >         define(DRIVER_HARDWARE, 1, REGS, IO_BUS,
> > > "Registers")                   \
> > >         define(DRIVER_HARDWARE, 2, GGTT, IO_BUS, "Global
> > > GTT")                  \
> > > diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > index dd1030de9ab6..da629d410829 100644
> > > --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c
> > > @@ -11,6 +11,7 @@
> > >  #include "xe_gt_types.h"
> > >  #include "xe_guc_ct.h"
> > >  #include "xe_guc_tlb_inval.h"
> > > +#include "xe_log.h"
> > >  #include "xe_mmio.h"
> > >  #include "xe_pm.h"
> > >  #include "xe_tlb_inval.h"
> > > @@ -86,9 +87,8 @@ static void xe_tlb_inval_fence_timeout(struct
> > > work_struct *work)
> > >                         break;
> > >  
> > >                 trace_xe_tlb_inval_fence_timeout(xe, fence);
> > > -               drm_err(&xe->drm,
> > > -                       "TLB invalidation fence timeout, seqno=%d
> > > recv=%d",
> > > -                       fence->seqno, tlb_inval->seqno_recv);
> > > +               xe_log_err(xe, TLBINVAL, -ETIME, "fence timeout,
> > > seqno=%d recv=%d",
> > 
> > Can we keep the "TLB invalidation" piece here since that's a little
> > easier for grepping when manually debugging?
> 
> this would be little redundant, as xe_log_err will emit dmesg like
> this:
> 
>   [drm] *ERROR* SIGID=106 (-ETIME) TLBINVAL: fence timeout...
> 
> vs
> 
>   [drm] *ERROR* SIGID=106 (-ETIME) TLBINVAL: TLB invalidation fence
> timeout...

Ah makes sense. Yeah I'll drop my request there.

Would be great if we can get a variable for the ETIME, but it's minor.
My R-B below still stands.

Thanks,
Stuart

> 
> > 
> > And slight nitpick.. Can we use a variable for -ETIME here since
> > we're
> > now using it in two places in case we want to change in the future?
> > 
> > Otherwise the patch looks good:
> > Reviewed-by: Stuart Summers <stuart.summers@intel.com>
> > 
> > Thanks,
> > Stuart
> > 
> > > +                          fence->seqno, tlb_inval->seqno_recv);
> > >  
> > >                 fence->base.error = -ETIME;
> > >                 xe_tlb_inval_fence_signal(fence);
> > 
>