Re: [PATCH] drm/xe/xe3p_lpg: Program TR_PTA_MODE

[Gustavo Sousa <gustavo.sousa@intel.com>]

Matt Roper <matthew.d.roper@intel.com> writes:

> Up until Xe3p_LPG, the PTA_MODE register controlled cacheability of
> accesses to the page tables for both PPGTT and TRTT.  Starting with
> Xe3p_LPG, PTA_MODE is now only responsible for the PPGTT accesses, and a
> separate register, TR_PTA_MODE is used to control the TRTT accesses. The
> currently recommeded value for TR_PTA_MODE differs from PTA_MODE on
> Xe3p_LPG.  Track and program this value separately in the driver.
>
> Note that even though the Xe3p_LP[G/M] IPs didn't add support for this
> new TR_PTA_MODE register until b-stepping, it's safe us to ignore that
> detail code-wise.  Writes of the unrecognized registers on a-step
> hardware will be silently ignored, and the reads on a-step will come
> back as 0x0 which happens to be the value we'd be trying to program on
> these IP versions anyway (for both graphics and media).
>
> The new TRTT-specific register also does not exist on Xe3p_XPC
> platforms.
>
> Bspec: 79814, 71582
> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_types.h |  4 +++
>  drivers/gpu/drm/xe/xe_pat.c          | 47 ++++++++++++++++++++++++++++++------
>  2 files changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 022e08205897..de5acc994ce6 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -404,6 +404,10 @@ struct xe_device {
>  		const struct xe_pat_table_entry *pat_primary_pta;
>  		/** @pat.pat_media_pta: media GT PAT entry for page table accesses */
>  		const struct xe_pat_table_entry *pat_media_pta;
> +		/** @pat.pat_primary_tr_pta: primary GT PAT entry for TRTT page table accesses */
> +		const struct xe_pat_table_entry *pat_primary_tr_pta;
> +		/** @pat.pat_media_tr_pta: media GT PAT entry for TRTT page table accesses */
> +		const struct xe_pat_table_entry *pat_media_tr_pta;
>  		u16 idx[__XE_CACHE_LEVEL_COUNT];
>  	} pat;
>  
> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
> index fad5b5a5ed4a..cde59270acc4 100644
> --- a/drivers/gpu/drm/xe/xe_pat.c
> +++ b/drivers/gpu/drm/xe/xe_pat.c
> @@ -25,6 +25,7 @@
>  								   0x4800, 0x4804, \
>  								   0x4848, 0x484c)
>  #define _PAT_PTA				0x4820
> +#define _PAT_TR_PTA				0x48cc
>  
>  #define XE2_NO_PROMOTE				REG_BIT(10)
>  #define XE2_COMP_EN				REG_BIT(9)
> @@ -256,6 +257,7 @@ static const struct xe_pat_table_entry xe3p_xpc_pat_table[] = {
>  
>  static const struct xe_pat_table_entry xe3p_primary_pat_pta = XE2_PAT(0, 0, 0, 0, 0, 3);
>  static const struct xe_pat_table_entry xe3p_media_pat_pta = XE2_PAT(0, 0, 0, 0, 0, 2);
> +static const struct xe_pat_table_entry xe3p_pat_tr_pta = XE2_PAT(0, 0, 0, 0, 0, 0);
>  
>  static const struct xe_pat_table_entry xe3p_lpg_pat_table[] = {
>  	[ 0] = XE2_PAT( 0, 0, 0, 0, 3, 0 ),
> @@ -311,15 +313,18 @@ u16 xe_pat_index_get_l3_policy(struct xe_device *xe, u16 pat_index)
>  	return REG_FIELD_GET(XE2_L3_POLICY, xe->pat.table[pat_index].value);
>  }
>  
> -static const struct xe_pat_table_entry *gt_pta_entry(struct xe_gt *gt)
> +static const struct xe_pat_table_entry *gt_pta_entry(struct xe_gt *gt, bool trtt)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
>  
> -	if (xe_gt_is_main_type(gt))
> +	if (xe_gt_is_main_type(gt) && !trtt)
>  		return xe->pat.pat_primary_pta;
> -
> -	if (xe_gt_is_media_type(gt))
> +	if (xe_gt_is_main_type(gt) && trtt)
> +		return xe->pat.pat_primary_tr_pta;
> +	if (xe_gt_is_media_type(gt) && !trtt)
>  		return xe->pat.pat_media_pta;
> +	if (xe_gt_is_media_type(gt) && trtt)
> +		return xe->pat.pat_media_tr_pta;
>  
>  	xe_assert(xe, false);
>  	return NULL;

I think I would slightly prefer a separate function called
gt_tr_pta_entry(), because it is clearer that it is about the TRTT PTA
mode when compared with a call like gt_pta_entry(gt, true).

The patch looks good to me, so take that suggestion as optional.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

> @@ -329,7 +334,8 @@ static void program_pat(struct xe_gt *gt, const struct xe_pat_table_entry table[
>  			int n_entries)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> -	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt);
> +	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt, false);
> +	const struct xe_pat_table_entry *tr_pta_entry = gt_pta_entry(gt, true);
>  
>  	for (int i = 0; i < n_entries; i++) {
>  		struct xe_reg reg = XE_REG(_PAT_INDEX(i));
> @@ -342,13 +348,17 @@ static void program_pat(struct xe_gt *gt, const struct xe_pat_table_entry table[
>  
>  	if (pta_entry)
>  		xe_mmio_write32(&gt->mmio, XE_REG(_PAT_PTA), pta_entry->value);
> +
> +	if (tr_pta_entry)
> +		xe_mmio_write32(&gt->mmio, XE_REG(_PAT_TR_PTA), tr_pta_entry->value);
>  }
>  
>  static void program_pat_mcr(struct xe_gt *gt, const struct xe_pat_table_entry table[],
>  			    int n_entries)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> -	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt);
> +	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt, false);
> +	const struct xe_pat_table_entry *tr_pta_entry = gt_pta_entry(gt, true);
>  
>  	for (int i = 0; i < n_entries; i++) {
>  		struct xe_reg_mcr reg_mcr = XE_REG_MCR(_PAT_INDEX(i));
> @@ -361,6 +371,9 @@ static void program_pat_mcr(struct xe_gt *gt, const struct xe_pat_table_entry ta
>  
>  	if (pta_entry)
>  		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_PTA), pta_entry->value);
> +
> +	if (tr_pta_entry)
> +		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_TR_PTA), pta_entry->value);
>  }
>  
>  static int xelp_dump(struct xe_gt *gt, struct drm_printer *p)
> @@ -531,6 +544,16 @@ static int xe2_dump(struct xe_gt *gt, struct drm_printer *p)
>  	drm_printf(p, "Page Table Access:\n");
>  	xe->pat.ops->entry_dump(p, "PTA_MODE", pat, false);
>  
> +	if (gt_pta_entry(gt, true)) {
> +		if (xe_gt_is_media_type(gt))
> +			pat = xe_mmio_read32(&gt->mmio, XE_REG(_PAT_TR_PTA));
> +		else
> +			pat = xe_gt_mcr_unicast_read_any(gt, XE_REG_MCR(_PAT_TR_PTA));
> +
> +		drm_printf(p, "TRTT Page Table Access:\n");
> +		xe->pat.ops->entry_dump(p, "TR_PTA_MODE", pat, false);
> +	}
> +
>  	if (xe_gt_is_media_type(gt))
>  		pat = xe_mmio_read32(&gt->mmio, XE_REG(_PAT_ATS));
>  	else
> @@ -577,6 +600,8 @@ void xe_pat_init_early(struct xe_device *xe)
>  		if (!IS_DGFX(xe)) {
>  			xe->pat.pat_primary_pta = &xe3p_primary_pat_pta;
>  			xe->pat.pat_media_pta = &xe3p_media_pat_pta;
> +			xe->pat.pat_primary_tr_pta = &xe3p_pat_tr_pta;
> +			xe->pat.pat_media_tr_pta = &xe3p_pat_tr_pta;
>  		}
>  		xe->pat.n_entries = ARRAY_SIZE(xe3p_lpg_pat_table);
>  		xe->pat.idx[XE_CACHE_NONE] = 3;
> @@ -700,7 +725,8 @@ int xe_pat_dump(struct xe_gt *gt, struct drm_printer *p)
>  int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> -	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt);
> +	const struct xe_pat_table_entry *pta_entry = gt_pta_entry(gt, false);
> +	const struct xe_pat_table_entry *tr_pta_entry = gt_pta_entry(gt, true);
>  	char label[PAT_LABEL_LEN];
>  
>  	if (!xe->pat.table || !xe->pat.n_entries)
> @@ -731,6 +757,13 @@ int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>  		xe->pat.ops->entry_dump(p, "PTA_MODE", pat, false);
>  	}
>  
> +	if (tr_pta_entry) {
> +		u32 pat = tr_pta_entry->value;
> +
> +		drm_printf(p, "TRTT Page Table Access:\n");
> +		xe->pat.ops->entry_dump(p, "TR_PTA_MODE", pat, false);
> +	}
> +
>  	if (xe->pat.pat_ats) {
>  		u32 pat = xe->pat.pat_ats->value;
>  
>
> ---
> base-commit: 9cf91140eff27765d402aedeea89505d52db121e
> change-id: 20260629-tr_pta_mode-b45fbb593987
>
> Best regards,
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation