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

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

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...

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