Re: [PATCH i-g-t 08/10] lib/intel_pat: Encapsulate management of xe_device's pat_cache

On Tue, Jul 21, 2026 at 04:00:01PM -0300, Gustavo Sousa wrote:
> Currently xe_device_get() manages the PAT cached information
> information by itself, by doing the necessary memory
> allocation/deallocation and calling xe_get_pat_sw_config() to
> initialize the cached information.
> 
> Such management is arguably better suited to be implemented as part of
> the intel_pat module and encapsulated such that xe_query doesn't need
> to know the details.
> 
> We will introduce changes that will require an extra logic for
> initializing the cache and having such encapsulation makes it much
> easier to implement.  Since we will touch this area, take this
> opportunity to move the existing management to intel_pat.
> 
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
>  lib/intel_pat.c   | 27 ++++++++++++++++++++++++++-
>  lib/intel_pat.h   |  5 +++++
>  lib/xe/xe_query.c |  9 ++-------
>  lib/xe/xe_query.h |  4 +---
>  4 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/intel_pat.c b/lib/intel_pat.c
> index 2f35bb77249f..14935e3c4692 100644
> --- a/lib/intel_pat.c
> +++ b/lib/intel_pat.c
> @@ -144,6 +144,31 @@ int32_t xe_get_pat_hw_config(int drm_fd, struct intel_pat_cache *xe_pat_cache, i
>  	return xe_get_pat_config(drm_fd, xe_pat_cache, gt, PAT_HW_CONFIG);
>  }
>  
> +void intel_pat_build_xe_cache(struct xe_device *xe_dev)
> +{
> +	struct intel_pat_cache *pat_cache;
> +
> +	pat_cache = calloc(1, sizeof(*pat_cache));
> +	igt_assert(pat_cache);
> +
> +	if (igt_debug_on(xe_get_pat_sw_config(xe_dev->fd, pat_cache, 0) <= 0)) {
> +		free(pat_cache);
> +		return;
> +	}
> +
> +	xe_dev->pat_cache = pat_cache;
> +}
> +
> +void intel_pat_free_xe_cache(struct xe_device *xe_dev)
> +{
> +	if (!xe_dev->pat_cache)
> +		return;
> +
> +	free(xe_dev->pat_cache);
> +
> +	xe_dev->pat_cache = NULL;
> +}
> +
>  /*
>   * Hardcoded PAT indices for Xe platforms, used as a fallback when the
>   * kernel doesn't expose gt0/pat_sw_config in debugfs.
> @@ -217,7 +242,7 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>  		struct xe_device *xe_dev = xe_device_get(fd);
>  
>  		if (xe_dev->pat_cache) {
> -			*pat = *xe_dev->pat_cache;
> +			*pat = *((struct intel_pat_cache *)xe_dev->pat_cache);
>  		} else if (xe_pat_fallback(fd, pat)) {
>  			igt_info("PAT sw_config debugfs not available, "
>  				 "using hardcoded fallback\n");
> diff --git a/lib/intel_pat.h b/lib/intel_pat.h
> index a31b60e86e2a..fc9ef9b650bf 100644
> --- a/lib/intel_pat.h
> +++ b/lib/intel_pat.h
> @@ -13,6 +13,8 @@
>  #define XE_PAT_IDX_INVALID ((uint8_t)-2) /* no such PAT index on this platform */
>  #define XE_PAT_MAX_ENTRIES 32
>  
> +struct xe_device;
> +
>  struct xe_pat_entry {
>  	uint32_t pat;
>  	bool rsvd;
> @@ -40,4 +42,7 @@ uint8_t intel_get_pat_idx_uc_comp(int fd);
>  int32_t xe_get_pat_sw_config(int drm_fd, struct intel_pat_cache *xe_pat_cache, int gt);
>  int32_t xe_get_pat_hw_config(int drm_fd, struct intel_pat_cache *xe_pat_cache, int gt);
>  
> +void intel_pat_build_xe_cache(struct xe_device *xe_dev);
> +void intel_pat_free_xe_cache(struct xe_device *xe_dev);
> +
>  #endif /* INTEL_PAT_H */
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index ea095b207534..aa81db45a41a 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -377,8 +377,8 @@ static void xe_device_free(struct xe_device *xe_dev)
>  	free(xe_dev->mem_regions);
>  	free(xe_dev->vram_size);
>  	free(xe_dev->eu_stall);
> -	free(xe_dev->pat_cache);
>  
> +	intel_pat_free_xe_cache(xe_dev);
>  	xe_wa_free_cache(xe_dev);
>  
>  	free(xe_dev);
> @@ -458,12 +458,7 @@ struct xe_device *xe_device_get(int fd)
>  	 * should be extended to cache PAT entries by platform version/
>  	 * revision instead.
>  	 */
> -	xe_dev->pat_cache = calloc(1, sizeof(*xe_dev->pat_cache));
> -	igt_assert(xe_dev->pat_cache);
> -	if (xe_get_pat_sw_config(xe_dev->fd, xe_dev->pat_cache, 0) <= 0) {
> -		free(xe_dev->pat_cache);
> -		xe_dev->pat_cache = NULL;
> -	}
> +	intel_pat_build_xe_cache(xe_dev);
>  
>  	/* We may get here from multiple threads, use first cached xe_dev */
>  	pthread_mutex_lock(&cache.cache_mutex);
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index f70476945dee..82f72314f072 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -20,8 +20,6 @@
>  #define XE_DEFAULT_ALIGNMENT           SZ_4K
>  #define XE_DEFAULT_ALIGNMENT_64K       SZ_64K
>  
> -struct intel_pat_cache;
> -
>  struct xe_device {
>  	/** @fd: xe fd */
>  	int fd;
> @@ -78,7 +76,7 @@ struct xe_device {
>  	uint16_t dev_id;
>  
>  	/** @pat_cache: cached PAT index configuration, NULL if not yet populated */
> -	struct intel_pat_cache *pat_cache;
> +	void *pat_cache;

Is there a need to change this to a void pointer?  It seems like we can
just leave this as-is and avoid the cast earlier.


Matt

>  
>  	/** @wa_cache: cached data for xe_wa() and related functions. */
>  	void *wa_cache;
> 
> -- 
> 2.55.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation