Re: [PATCH] drm/xe/memirq: Size report pages from static engine mask

On 6/15/2026 15:26, Summers, Stuart wrote:
> On Fri, 2026-06-12 at 17:16 -0700, Xin Wang wrote:
>> xe_memirq_init() is called from xe_tile_init(), before HW engines are
> Can we move this to xe_gt_init_early() instead? This is dependent on
> the fuses of course which need to come later, so I'm thinking we have
> an xe_xe_hw_engines_init_prefuse() or something that uses the engines
> defined in xe_pci.c to come up with an early count.
Re: moving this to xe_gt_init_early() -- I actually tried that approach 
before
sending this version. It doesn't work as-is: too much of the state this 
depends
on isn't ready yet at that point in the init sequence (the fuses/engine 
info
aren't reliable that early), so xe_hw_engine_max_instance() can't be called
safely from there yet. Getting it to work would require a broader 
refactor of
the init/early_init sequencing first, which felt out of scope for this fix.
Happy to revisit once/if that refactor happens.
> Maybe I'm overthinking that though... It would be nice to have
> everything in one for_each macro, but then the macro would need to
> comprehend whether the fuses were read already or not...
>
> One downside of this approach is we're allocating more space than we
> really need to since we might not be supporting the full engine set
> you're querying. Maybe we re-allocate after the engine initialization
> is done?
>
> Or maybe we move the memirq init later in the load sequence? That might
> have consequences for VF load though - would need Michal's input there.
>
> One minor comment below as well..
>
>> initialized. hwe_max_count() uses for_each_hw_engine() to size the
>> memirq BO, but at this point hwe->name is still NULL so the iterator
>> yields nothing and the BO is allocated with a single page, which is
>> insufficient on platforms where engine instances do not start at 0.
>>
>> Cc: Michal Wajdeczko <[email protected]>
>> Signed-off-by: Xin Wang <[email protected]>
>> ---
>>   drivers/gpu/drm/xe/xe_hw_engine.c | 26 ++++++++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_hw_engine.h |  1 +
>>   drivers/gpu/drm/xe/xe_memirq.c    |  6 ++----
>>   3 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c
>> b/drivers/gpu/drm/xe/xe_hw_engine.c
>> index 98265293f2dc..16ff37756ddb 100644
>> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>> @@ -1018,6 +1018,32 @@ u32 xe_hw_engine_mask_per_class(struct xe_gt
>> *gt,
>>          return mask;
>>   }
>>   
>> +/**
>> + * xe_hw_engine_max_instance - Get the maximum HW engine instance
>> for a GT.
>> + * @gt: the &xe_gt
>> + *
>> + * Compute the highest HW engine instance present on @gt based on
>> the static
>> + * engine mask. Unlike iterating with for_each_hw_engine(), this
>> relies only on
>> + * @gt->info.engine_mask (set during early PCI probe) and the static
>> engine
>> + * descriptor table, so it is valid even before hw engines are
>> initialized
>> + * (e.g. when sizing the `Memory Based Interrupts`_ report pages).
>> + *
>> + * Return: The maximum engine instance value on @gt.
>> + */
>> +unsigned int xe_hw_engine_max_instance(struct xe_gt *gt)
>> +{
>> +       unsigned int max_instance = 0;
>> +       enum xe_hw_engine_id id;
>> +
>> +       for (id = 0; id < XE_NUM_HW_ENGINES; ++id) {
>> +               if (gt->info.engine_mask & BIT(id))
>> +                       max_instance = max(max_instance,
>> +                                          (unsigned
>> int)engine_infos[id].instance);
> We can drop the cast here since the variable is uint also.
I don't think we can drop it. `instance` is declared as an 8-bit bitfield
of type `unsigned int` in `struct engine_info`, but per the C integer
promotion rules, an unsigned bitfield narrower than `int` whose
full value range fits in `int` gets promoted to plain (signed) `int`
in an expression, not to `unsigned int`. So without the cast,
`engine_infos[id].instance` actually evaluates as `int`, while
`max_instance` is `unsigned int`. The kernel's `max()` macro does
a strict typecheck between its two arguments, so dropping the cast
would trip that check (build warning/error, and likely a sparse
complaint too). I'll keep it as-is unless there's a cleaner way to 
express it.
> Thanks,
> Stuart
>
>> +       }
>> +
>> +       return max_instance;
>> +}
>> +
>>   bool xe_hw_engine_is_reserved(struct xe_hw_engine *hwe)
>>   {
>>          struct xe_gt *gt = hwe->gt;
>> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h
>> b/drivers/gpu/drm/xe/xe_hw_engine.h
>> index c3ee37f8cfc0..838abf7092a5 100644
>> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
>> @@ -55,6 +55,7 @@ void xe_hw_engine_handle_irq(struct xe_hw_engine
>> *hwe, u16 intr_vec);
>>   void xe_hw_engine_enable_ring(struct xe_hw_engine *hwe);
>>   u32 xe_hw_engine_mask_per_class(struct xe_gt *gt,
>>                                  enum xe_engine_class engine_class);
>> +unsigned int xe_hw_engine_max_instance(struct xe_gt *gt);
>>   struct xe_hw_engine_snapshot *
>>   xe_hw_engine_snapshot_capture(struct xe_hw_engine *hwe, struct
>> xe_exec_queue *q);
>>   void xe_hw_engine_snapshot_free(struct xe_hw_engine_snapshot
>> *snapshot);
>> diff --git a/drivers/gpu/drm/xe/xe_memirq.c
>> b/drivers/gpu/drm/xe/xe_memirq.c
>> index 9dfe965cb46e..5a87ac42351d 100644
>> --- a/drivers/gpu/drm/xe/xe_memirq.c
>> +++ b/drivers/gpu/drm/xe/xe_memirq.c
>> @@ -175,13 +175,11 @@ static inline bool
>> hw_reports_to_instance_zero(struct xe_memirq *memirq)
>>   static unsigned int hwe_max_count(struct xe_tile *tile)
>>   {
>>          unsigned int max_instance = 0;
>> -       unsigned int gtid, hweid;
>> -       struct xe_hw_engine *hwe;
>> +       unsigned int gtid;
>>          struct xe_gt *gt;
>>   
>>          for_each_gt_on_tile(gt, tile, gtid)
>> -               for_each_hw_engine(hwe, gt, hweid)
>> -                       max_instance = max(max_instance, hwe-
>>> instance);
>> +               max_instance = max(max_instance,
>> xe_hw_engine_max_instance(gt));
>>   
>>          return max_instance + 1;
>>   }