Re: [Intel-gfx] [PATCH] drm/i915/pvc: Add register steering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 02, 2022 at 05:53:35PM -0700, Matt Roper wrote:
> Ponte Vecchio no longer has MSLICE or LNCF steering, but the bspec does
> document several new types of multicast register ranges.  Fortunately,
> most of the different MCR types all provide valid values at instance
> (0,0) so there's no need to read fuse registers and calculate a valid
> instance.  We'll lump all of those range types (BSLICE, HALFBSLICE,
> TILEPSMI, CC, and L3BANK) into a single category called "INSTANCE0" to
> keep things simple.  We'll also perform explicit steering for each of
> these multicast register types, even if the implicit steering setup for
> COMPUTE/DSS ranges would have worked too; this is based on guidance from
> our hardware architects who suggested that we move away from implicit
> steering and start explicitly steer all MCR register accesses on modern
> platforms (we'll work on transitioning COMPUTE/DSS to explicit steering
> in the future).
> 
> Note that there's one additional MCR range type defined in the bspec
> (SQIDI) that we don't handle here.  Those ranges use a different
> steering control register that we never touch; since instance 0 is also
> always a valid setting there, we can just ignore those ranges.
> 
> Finally, we'll rename the HAS_MSLICES() macro to HAS_MSLICE_STEERING().
> PVC hardware still has units referred to as mslices, but there's no
> register steering based on mslice for this platform.
> 
> Bspec: 67609
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c          | 50 ++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  7 +++
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++++++++-
>  drivers/gpu/drm/i915/i915_drv.h             |  3 +-
>  drivers/gpu/drm/i915/i915_pci.c             |  3 +-
>  drivers/gpu/drm/i915/intel_device_info.h    |  2 +-
>  6 files changed, 76 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index ddfb98f70489..b335eacd7a6c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -106,6 +106,7 @@ static const char * const intel_steering_types[] = {
>  	"L3BANK",
>  	"MSLICE",
>  	"LNCF",
> +	"INSTANCE 0",
>  };
>  
>  static const struct intel_mmio_range icl_l3bank_steering_table[] = {
> @@ -133,6 +134,27 @@ static const struct intel_mmio_range dg2_lncf_steering_table[] = {
>  	{},
>  };
>  
> +/*
> + * We have several types of MCR registers on PVC where steering to (0,0)
> + * will always provide us with a non-terminated value.  We'll stick them
> + * all in the same table for simplicity.
> + */
> +static const struct intel_mmio_range pvc_instance0_steering_table[] = {
> +	{ 0x004000, 0x004AFF },		/* HALF-BSLICE */
> +	{ 0x008A80, 0x008AFF },		/* TILEPSMI */
> +	{ 0x008800, 0x00887F },		/* CC */
minor nit: The above two lines need to be swapped to keep this table
sorted in the increasing order of addresses.
> +	{ 0x00B000, 0x00B0FF },		/* HALF-BSLICE */
> +	{ 0x00B100, 0x00B3FF },		/* L3BANK */
> +	{ 0x00C800, 0x00CFFF },		/* HALF-BSLICE */
> +	{ 0x00D800, 0x00D8FF },		/* HALF-BSLICE */
> +	{ 0x00DD00, 0x00DDFF },		/* BSLICE */
> +	{ 0x00E900, 0x00E9FF },		/* HALF-BSLICE */
> +	{ 0x00EC00, 0x00EEFF },		/* HALF-BSLICE */
> +	{ 0x00F000, 0x00FFFF },		/* HALF-BSLICE */
> +	{ 0x024180, 0x0241FF },		/* HALF-BSLICE */
> +	{},
> +};
> +
>  int intel_gt_init_mmio(struct intel_gt *gt)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
> @@ -146,7 +168,7 @@ int intel_gt_init_mmio(struct intel_gt *gt)
>  	 * An mslice is unavailable only if both the meml3 for the slice is
>  	 * disabled *and* all of the DSS in the slice (quadrant) are disabled.
>  	 */
> -	if (HAS_MSLICES(i915)) {
> +	if (HAS_MSLICE_STEERING(i915)) {
>  		gt->info.mslice_mask =
>  			intel_slicemask_from_xehp_dssmask(gt->info.sseu.subslice_mask,
>  							  GEN_DSS_PER_MSLICE);
> @@ -158,7 +180,9 @@ int intel_gt_init_mmio(struct intel_gt *gt)
>  			drm_warn(&i915->drm, "mslice mask all zero!\n");
>  	}
>  
> -	if (IS_DG2(i915)) {
> +	if (IS_PONTEVECCHIO(i915)) {
> +		gt->steering_table[INSTANCE0] = pvc_instance0_steering_table;
> +	} else if (IS_DG2(i915)) {
>  		gt->steering_table[MSLICE] = xehpsdv_mslice_steering_table;
>  		gt->steering_table[LNCF] = dg2_lncf_steering_table;
>  	} else if (IS_XEHPSDV(i915)) {
> @@ -172,7 +196,11 @@ int intel_gt_init_mmio(struct intel_gt *gt)
>  			GEN10_L3BANK_MASK;
>  		if (!gt->info.l3bank_mask) /* should be impossible! */
>  			drm_warn(&i915->drm, "L3 bank mask is all zero!\n");
> -	} else if (HAS_MSLICES(i915)) {
> +	} else if (GRAPHICS_VER(i915) >= 11) {
> +		/*
> +		 * We expect all modern platforms to have at least some
> +		 * type of steering that needs to be initialized.
> +		 */
>  		MISSING_CASE(INTEL_INFO(i915)->platform);
>  	}
>  
> @@ -888,7 +916,7 @@ static void intel_gt_get_valid_steering(struct intel_gt *gt,
>  		*subsliceid = __ffs(gt->info.l3bank_mask);
>  		break;
>  	case MSLICE:
> -		GEM_WARN_ON(!HAS_MSLICES(gt->i915));
> +		GEM_WARN_ON(!HAS_MSLICE_STEERING(gt->i915));
>  		*sliceid = __ffs(gt->info.mslice_mask);
>  		*subsliceid = 0;	/* unused */
>  		break;
> @@ -897,10 +925,18 @@ static void intel_gt_get_valid_steering(struct intel_gt *gt,
>  		 * An LNCF is always present if its mslice is present, so we
>  		 * can safely just steer to LNCF 0 in all cases.
>  		 */
> -		GEM_WARN_ON(!HAS_MSLICES(gt->i915));
> +		GEM_WARN_ON(!HAS_MSLICE_STEERING(gt->i915));
>  		*sliceid = __ffs(gt->info.mslice_mask) << 1;
>  		*subsliceid = 0;	/* unused */
>  		break;
> +	case INSTANCE0:
> +		/*
> +		 * There are a lot of MCR types for which instance (0, 0)
> +		 * will always provide a non-terminated value.
> +		 */
> +		*sliceid = 0;
> +		*subsliceid = 0;
> +		break;
>  	default:
>  		MISSING_CASE(type);
>  		*sliceid = 0;
> @@ -1020,7 +1056,9 @@ void intel_gt_report_steering(struct drm_printer *p, struct intel_gt *gt,
>  		   gt->default_steering.groupid,
>  		   gt->default_steering.instanceid);
>  
> -	if (HAS_MSLICES(gt->i915)) {
> +	if (IS_PONTEVECCHIO(gt->i915)) {
> +		report_steering_type(p, gt, INSTANCE0, dump_table);
> +	} else if (HAS_MSLICE_STEERING(gt->i915)) {
>  		report_steering_type(p, gt, MSLICE, dump_table);
>  		report_steering_type(p, gt, LNCF, dump_table);
>  	}
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index 993f003dad1d..df708802889d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -59,6 +59,13 @@ enum intel_steering_type {
>  	MSLICE,
>  	LNCF,
>  
> +	/*
> +	 * On some platforms there are multiple types of MCR registers that
> +	 * will always return a non-terminated value at instance (0, 0).  We'll
> +	 * lump those all into a single category to keep things simple.
> +	 */
> +	INSTANCE0,
> +
>  	NUM_STEERING_TYPES
>  };
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index eb0598593724..1b191b234160 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1195,6 +1195,20 @@ xehp_init_mcr(struct intel_gt *gt, struct i915_wa_list *wal)
>  	__set_mcr_steering(wal, SF_MCR_SELECTOR, 0, 2);
>  }
>  
> +static void
> +pvc_init_mcr(struct intel_gt *gt, struct i915_wa_list *wal)
> +{
> +	unsigned int dss;
> +
> +	/*
> +	 * Setup implicit steering for COMPUTE and DSS ranges to the first
> +	 * non-fused-off DSS.  All other types of MCR registers will be
> +	 * explicitly steered.
> +	 */
> +	dss = intel_sseu_find_first_xehp_dss(&gt->info.sseu, 0, 0);
> +	__add_mcr_wa(gt, wal, dss / GEN_DSS_PER_CSLICE, dss % GEN_DSS_PER_CSLICE);
> +}
> +
>  static void
>  icl_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  {
> @@ -1488,13 +1502,19 @@ dg2_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
>  	wa_write_or(wal, GEN12_SQCM, EN_32B_ACCESS);
>  }
>  
> +static void
> +pvc_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> +{
> +	pvc_init_mcr(gt, wal);
> +}
> +
>  static void
>  gt_init_workarounds(struct intel_gt *gt, struct i915_wa_list *wal)
>  {
>  	struct drm_i915_private *i915 = gt->i915;
>  
>  	if (IS_PONTEVECCHIO(i915))
> -		; /* none yet */
> +		pvc_gt_workarounds_init(gt, wal);
>  	else if (IS_DG2(i915))
>  		dg2_gt_workarounds_init(gt, wal);
>  	else if (IS_XEHPSDV(i915))
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c3854b8a014f..5870cf9eb0b4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1283,8 +1283,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define HAS_RUNTIME_PM(dev_priv) (INTEL_INFO(dev_priv)->has_runtime_pm)
>  #define HAS_64BIT_RELOC(dev_priv) (INTEL_INFO(dev_priv)->has_64bit_reloc)
>  
> -#define HAS_MSLICES(dev_priv) \
> -	(INTEL_INFO(dev_priv)->has_mslices)
> +#define HAS_MSLICE_STEERING(dev_priv)	(INTEL_INFO(dev_priv)->has_mslice_steering)
>  
>  /*
>   * Set this flag, when platform requires 64K GTT page sizes or larger for
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index a5a1a7647320..5e51fc29bb8b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1021,7 +1021,7 @@ static const struct intel_device_info adl_p_info = {
>  	.has_llc = 1, \
>  	.has_logical_ring_contexts = 1, \
>  	.has_logical_ring_elsq = 1, \
> -	.has_mslices = 1, \
> +	.has_mslice_steering = 1, \
>  	.has_rc6 = 1, \
>  	.has_reset_engine = 1, \
>  	.has_rps = 1, \
> @@ -1091,6 +1091,7 @@ static const struct intel_device_info ats_m_info = {
>  	.has_3d_pipeline = 0, \
>  	.has_guc_deprivilege = 1, \
>  	.has_l3_ccs_read = 1, \
> +	.has_mslice_steering = 0, \
>  	.has_one_eu_per_fuse_bit = 1
>  
>  __maybe_unused
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 346f17f2dce8..08341174ee0a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -157,7 +157,7 @@ enum intel_ppgtt_type {
>  	func(has_logical_ring_contexts); \
>  	func(has_logical_ring_elsq); \
>  	func(has_media_ratio_mode); \
> -	func(has_mslices); \
> +	func(has_mslice_steering); \
>  	func(has_one_eu_per_fuse_bit); \
>  	func(has_pooled_eu); \
>  	func(has_pxp); \
Looks good to me.
Reviewed-by: Harish Chegondi <harish.chegondi@xxxxxxxxx>
> -- 
> 2.35.3
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux