Re: [PATCH 5/7] drm/xe/nvlp: Implement Wa_14026539277

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

Gustavo Sousa <gustavo.sousa@intel.com> writes:

> Implement the KMD part of Wa_14026539277, which applies to NVL-P A0.
> The KMD implementation is just one component of the workaround, which
> also depends on Pcode to implement its part in order to be complete.
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  4 ++++
>  drivers/gpu/drm/xe/xe_gt.c           | 27 +++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_wa_oob.rules   |  2 ++
>  3 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index 66ddad767ad4..a83cafbe03fd 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -452,6 +452,10 @@
>  
>  #define XEHPC_L3CLOS_MASK(i)			XE_REG_MCR(0xb194 + (i) * 8)
>  
> +#define L2COMPUTESIDECTRL			XE_REG_MCR(0xb1c0)
> +#define   CECTRL				REG_GENMASK(2, 1)
> +#define   CECTRL_CENODATA_ALWAYS		REG_FIELD_PREP(CECTRL, 0x0)
> +
>  #define XE2_GLOBAL_INVAL			XE_REG(0xb404)
>  
>  #define XE2LPM_L3SQCREG2			XE_REG_MCR(0xb604)
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index b455af1e6072..3c8692f9b8cf 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -450,6 +450,25 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
>  	return err;
>  }
>  
> +static void xe_gt_wa_14026539277(struct xe_gt *gt)
> +{
> +	u32 val;
> +
> +	if (!XE_GT_WA(gt, 14026539277))
> +		return;

It appears SRIOV VF wouldn't have access to the register below, right?

Adding Piotr here, since we talked about it offline recently. Does
implementing this only on PF side suffice?

If so, should we return early here if on a VF or should we rather add a
FUNC(xe_rtp_match_not_sriov_vf) in the rules for Wa_14026539277?

One thing that I realize with the latter approach is that 14026539277
would not be listed in the workarounds debugfs file.  Do we really want
that? I think this question applies to all cases where we use
FUNC(xe_rtp_match_not_sriov_vf), not only this one.

--
Gustavo Sousa

> +
> +	/*
> +	 * L2COMPUTESIDECTRL has a specific offset for media and the GSI offset
> +	 * does not apply.
> +	 */
> +	xe_gt_assert(gt, xe_gt_is_main_type(gt));
> +
> +	val = xe_gt_mcr_unicast_read_any(gt, L2COMPUTESIDECTRL);
> +	val &= ~CECTRL;
> +	val |= CECTRL_CENODATA_ALWAYS;
> +	xe_gt_mcr_multicast_write(gt, L2COMPUTESIDECTRL, val);
> +}
> +
>  int xe_gt_init_early(struct xe_gt *gt)
>  {
>  	int err;
> @@ -575,6 +594,14 @@ static int gt_init_with_gt_forcewake(struct xe_gt *gt)
>  	 */
>  	gt->info.gmdid = xe_mmio_read32(&gt->mmio, GMD_ID);
>  
> +	/*
> +	 * Wa_14026539277 can't be implemented as a regular GT workaround (i.e.
> +	 * as an entry in gt_was[]) because we would get the hardware already in
> +	 * a bad state by the time it would be applied.  Hence, we implement it
> +	 * as an OOB workaround and apply it early to prevent that.
> +	 */
> +	xe_gt_wa_14026539277(gt);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
> index 80b54b195f20..7f42436df6ab 100644
> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
> @@ -58,3 +58,5 @@
>  
>  14025883347	MEDIA_VERSION_RANGE(1301, 3503)
>  		GRAPHICS_VERSION_RANGE(2004, 3005)
> +
> +14026539277	PLATFORM(NOVALAKE_P), PLATFORM_STEP(A0, B0), GRAPHICS_VERSION(3510)
>
> -- 
> 2.52.0