Re: [PATCH v34 2/6] drm/xe/xe_pagefault: Pack engine class instance into u8

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Feb 13, 2026 at 10:34:13PM +0000, Jonathan Cavitt wrote:
> Pack the engine class and instance fields into a single u8 to save space
> in struct xe_pagefault.  The saved space will be used later to store
> something else.
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc_pagefault.c   |  7 +++++--
>  drivers/gpu/drm/xe/xe_pagefault.c       | 11 +++++++----
>  drivers/gpu/drm/xe/xe_pagefault_types.h | 16 ++++++++++++----
>  3 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> index d48f6ed103bb..8eaa1dfc1e66 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> @@ -85,8 +85,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 0281b5b6d4ab..47b6a7628dc1 100644
> --- a/drivers/gpu/drm/xe/xe_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> @@ -229,13 +229,15 @@ static bool xe_pagefault_queue_pop(struct xe_pagefault_queue *pf_queue,
>  
>  static void xe_pagefault_print(struct xe_pagefault *pf)
>  {
> +	enum xe_engine_class 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: %d\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),
> @@ -244,9 +246,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);
> +		   class,
> +		   xe_hw_engine_class_to_str(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 0e378f41ede6..a14725a02f39 100644
> --- a/drivers/gpu/drm/xe/xe_pagefault_types.h
> +++ b/drivers/gpu/drm/xe/xe_pagefault_types.h
> @@ -80,10 +80,18 @@ 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, packed
> +		 * into a single u8 to keep compact
> +		 */
> +		u8 engine_class_instance;
> +#define XE_PAGEFAULT_ENGINE_CLASS_MASK		GENMASK(3, 0)
> +#define XE_PAGEFAULT_ENGINE_INSTANCE_MASK	GENMASK(7, 4)
> +		/**
> +		 * consumer.align: buffer u8 to keep struct aligned to u64.
> +		 * Will be used later to store data.
> +		 */
> +		u8 align;
>  		/** consumer.reserved: reserved bits for future expansion */
>  		u64 reserved;
>  	} consumer;
> -- 
> 2.43.0
>