Re: [PATCH v4 06/12] drm/xe: Engine class and instance into a u8

[Maciej Patelczyk <maciej.patelczyk@intel.com>]

On 06/05/2026 18:04, Maciej Patelczyk wrote:
> On 26/02/2026 05:28, Matthew Brost wrote:
>
>> Pack the engine class and instance fields into a single u8 to save space
>> in struct xe_pagefault. This also makes future extensions easier.
>>
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_guc_pagefault.c   |  7 +++++--
>>   drivers/gpu/drm/xe/xe_pagefault.c       | 12 ++++++++----
>>   drivers/gpu/drm/xe/xe_pagefault_types.h | 10 ++++++----
>>   3 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c 
>> b/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> index 607e32392f46..2470faf3d5d8 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
>> @@ -89,8 +89,11 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, 
>> u32 *msg, u32 len)
>>                      FIELD_GET(PFD_FAULT_LEVEL, msg[0])) |
>>               FIELD_PREP(XE_PAGEFAULT_TYPE_MASK,
>>                      FIELD_GET(PFD_FAULT_TYPE, msg[2]));
>> -    pf.consumer.engine_class = FIELD_GET(PFD_ENG_CLASS, msg[0]);
>> -    pf.consumer.engine_instance = FIELD_GET(PFD_ENG_INSTANCE, msg[0]);
>> +    pf.consumer.engine_class_instance =
>> +        FIELD_PREP(XE_PAGEFAULT_ENGINE_CLASS_MASK,
>> +               FIELD_GET(PFD_ENG_CLASS, msg[0])) |
>> +        FIELD_PREP(XE_PAGEFAULT_ENGINE_INSTANCE_MASK,
>> +               FIELD_GET(PFD_ENG_INSTANCE, msg[0]));
>>         pf.producer.private = guc;
>>       pf.producer.ops = &guc_pagefault_ops;
>> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c 
>> b/drivers/gpu/drm/xe/xe_pagefault.c
>> index 64b1dc574ab7..a6fa790774c5 100644
>> --- a/drivers/gpu/drm/xe/xe_pagefault.c
>> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
>> @@ -245,13 +245,16 @@ static bool xe_pagefault_queue_pop(struct 
>> xe_pagefault_queue *pf_queue,
>>     static void xe_pagefault_print(struct xe_pagefault *pf)
>>   {
>> +    u8 engine_class = FIELD_GET(XE_PAGEFAULT_ENGINE_CLASS_MASK,
>> +                    pf->consumer.engine_class_instance);
>> +
>>       xe_gt_info(pf->gt, "\n\tASID: %d\n"
>>              "\tFaulted Address: 0x%08x%08x\n"
>>              "\tFaultType: %lu\n"
>>              "\tAccessType: %lu\n"
>>              "\tFaultLevel: %lu\n"
>>              "\tEngineClass: %d %s\n"
>> -           "\tEngineInstance: %d\n",
>> +           "\tEngineInstance: %lu\n",
>>              pf->consumer.asid,
>>              upper_32_bits(pf->consumer.page_addr),
>>              lower_32_bits(pf->consumer.page_addr),
>> @@ -261,9 +264,10 @@ static void xe_pagefault_print(struct 
>> xe_pagefault *pf)
>>                    pf->consumer.access_type),
>>              FIELD_GET(XE_PAGEFAULT_LEVEL_MASK,
>>                    pf->consumer.fault_type_level),
>> -           pf->consumer.engine_class,
>> - xe_hw_engine_class_to_str(pf->consumer.engine_class),
>> -           pf->consumer.engine_instance);
>> +           engine_class,
>> +           xe_hw_engine_class_to_str(engine_class),
>> +           FIELD_GET(XE_PAGEFAULT_ENGINE_INSTANCE_MASK,
>> +                 pf->consumer.engine_class_instance));
>>   }
>>     static void xe_pagefault_queue_work(struct work_struct *w)
>> diff --git a/drivers/gpu/drm/xe/xe_pagefault_types.h 
>> b/drivers/gpu/drm/xe/xe_pagefault_types.h
>> index 45065c25c25f..75bc53205601 100644
>> --- a/drivers/gpu/drm/xe/xe_pagefault_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pagefault_types.h
>> @@ -82,10 +82,12 @@ struct xe_pagefault {
>>   #define XE_PAGEFAULT_TYPE_LEVEL_NACK        0xff    /* Producer 
>> indicates nack fault */
>>   #define XE_PAGEFAULT_LEVEL_MASK            GENMASK(3, 0)
>>   #define XE_PAGEFAULT_TYPE_MASK            GENMASK(7, 4)
>> -        /** @consumer.engine_class: engine class */
>> -        u8 engine_class;
>> -        /** @consumer.engine_instance: engine instance */
>> -        u8 engine_instance;
>> +        /** @consumer.engine_class_instance: engine class and 
>> instance */
>> +        u8 engine_class_instance;
>> +#define XE_PAGEFAULT_ENGINE_CLASS_MASK        GENMASK(3, 0)
>> +#define XE_PAGEFAULT_ENGINE_INSTANCE_MASK    GENMASK(7, 4)
>> +        /** @pad: alignment padding */
>> +        u8 pad;
>>           /** consumer.reserved: reserved bits for future expansion */
>>           u64 reserved;
>>       } consumer;
>
> Looks good,
>
> Reviewed-by: Maciej Patelczyk <maciej.patelczyk@intel.com>
>
Sorry,

../drivers/gpu/drm/xe/xe_vm.c: In function ‘xe_vm_add_fault_entry_pf’:

../drivers/gpu/drm/xe/xe_vm.c:605:51: error: ‘struct <anonymous>’ has no 
member named ‘engine_class’

   605 |         hwe = xe_gt_hw_engine(pf->gt, pf->consumer.engine_class,

       |                                                   ^

../drivers/gpu/drm/xe/xe_vm.c:606:44: error: ‘struct <anonymous>’ has no 
member named ‘engine_instance’; did you mean ‘engine_class_instance’?

  606 |  pf->consumer.engine_instance, false);

      |                                            ^~~~~~~~~~~~~~~

       | engine_class_instance


Missing:

--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -602,8 +602,12 @@ void xe_vm_add_fault_entry_pf(struct xe_vm *vm, 
struct xe_pagefault *pf)
         struct xe_hw_engine *hwe;

         /* Do not report faults on reserved engines */
-       hwe = xe_gt_hw_engine(pf->gt, pf->consumer.engine_class,
-                             pf->consumer.engine_instance, false);
+       hwe = xe_gt_hw_engine(pf->gt,
+  FIELD_GET(XE_PAGEFAULT_ENGINE_CLASS_MASK,
+  pf->consumer.engine_class_instance),
+  FIELD_GET(XE_PAGEFAULT_ENGINE_INSTANCE_MASK,
+  pf->consumer.engine_class_instance),
+                             false);
         if (!hwe || xe_hw_engine_is_reserved(hwe))


Maciej