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

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

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>