Re: [PATCH v3] drm/i915/bdw: Check for slice, subslice and EU count for BDW

[Jeff McGee <jeff.mcgee@intel.com>]

On Thu, Sep 17, 2015 at 01:51:58PM +0200, Łukasz Daniluk wrote:
> Added checks for available slices, subslices and EUs for Broadwell. This
> information is filled in intel_device_info and is available to user with
> GET_PARAM.
> Added checks for enabled slices, subslices and EU for Broadwell. This
> information is based on available counts but takes power gated slices
> into account. It can be read in debugfs.
> Introduce new register defines that contain information on slices on
> Broadwell.
> 
> v2:
> - Introduce GT_SLICE_INFO register
> - Change Broadwell sseu_device_status function to use GT_SLICE_INFO
>   register instead of RPCS register
> - Undo removal of dev_priv variables in Cherryview and Gen9
>   sseu_device_satus functions
> 
> v3:
> - Fix style issues
> 
> Cc: Jeff Mcgee <jeff.mcgee@intel.com>
> Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
> Signed-off-by: Łukasz Daniluk <lukasz.daniluk@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++-
>  drivers/gpu/drm/i915/i915_dma.c     | 80 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h     | 22 +++++++++-
>  3 files changed, 128 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 72ae347..d0f784d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5037,13 +5037,38 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  	}
>  }
>  
> +static void broadwell_sseu_device_status(struct drm_device *dev,
> +					 struct sseu_dev_status *stat)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int s;
> +	u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
> +
> +	stat->slice_total = hweight32(slice_info & GEN8_LSLICESTAT_MASK);
> +
> +	if (stat->slice_total) {
> +		stat->subslice_per_slice = INTEL_INFO(dev)->subslice_per_slice;
> +		stat->subslice_total = stat->slice_total *
> +				       stat->subslice_per_slice;
> +		stat->eu_per_subslice = INTEL_INFO(dev)->eu_per_subslice;
> +		stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
> +
> +		/* subtract fused off EU(s) from enabled slice(s) */
> +		for (s = 0; s < stat->slice_total; s++) {
> +			u8 subslice_7eu = INTEL_INFO(dev)->subslice_7eu[s];
> +
> +			stat->eu_total -= hweight8(subslice_7eu);
> +		}
> +	}
> +}
> +
>  static int i915_sseu_status(struct seq_file *m, void *unused)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
>  	struct drm_device *dev = node->minor->dev;
>  	struct sseu_dev_status stat;
>  
> -	if ((INTEL_INFO(dev)->gen < 8) || IS_BROADWELL(dev))
> +	if (INTEL_INFO(dev)->gen < 8)
>  		return -ENODEV;
>  
>  	seq_puts(m, "SSEU Device Info\n");
> @@ -5068,6 +5093,8 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>  	memset(&stat, 0, sizeof(stat));
>  	if (IS_CHERRYVIEW(dev)) {
>  		cherryview_sseu_device_status(dev, &stat);
> +	} else if (IS_BROADWELL(dev)) {
> +		broadwell_sseu_device_status(dev, &stat);
>  	} else if (INTEL_INFO(dev)->gen >= 9) {
>  		gen9_sseu_device_status(dev, &stat);
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 066a0ef..6ec3ed6 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -713,6 +713,84 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	info->has_eu_pg = (info->eu_per_subslice > 2);
>  }
>  
> +static void broadwell_sseu_info_init(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_device_info *info;
> +	const int s_max = 3, ss_max = 3, eu_max = 8;
> +	int s, ss;
> +	u32 fuse2, eu_disable[s_max], s_enable, ss_disable;
> +
> +	fuse2 = I915_READ(GEN8_FUSE2);
> +	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
> +	ss_disable = (fuse2 & GEN8_F2_SS_DIS_MASK) >> GEN8_F2_SS_DIS_SHIFT;
> +
> +	eu_disable[0] = I915_READ(GEN8_EU_DISABLE0) & GEN8_EU_DIS0_S0_MASK;
> +	eu_disable[1] = (I915_READ(GEN8_EU_DISABLE0) >> GEN8_EU_DIS0_S1_SHIFT) |
> +			((I915_READ(GEN8_EU_DISABLE1) & GEN8_EU_DIS1_S1_MASK) <<
> +			 (32 - GEN8_EU_DIS0_S1_SHIFT));
> +	eu_disable[2] = (I915_READ(GEN8_EU_DISABLE1) >> GEN8_EU_DIS1_S2_SHIFT) |
> +			((I915_READ(GEN8_EU_DISABLE2) & GEN8_EU_DIS2_S2_MASK) <<
> +			 (32 - GEN8_EU_DIS1_S2_SHIFT));
> +
> +
> +	info = (struct intel_device_info *)&dev_priv->info;
> +	info->slice_total = hweight32(s_enable);
> +
> +	/*
> +	 * The subslice disable field is global, i.e. it applies
> +	 * to each of the enabled slices.
> +	 */
> +	info->subslice_per_slice = ss_max - hweight32(ss_disable);
> +	info->subslice_total = info->slice_total * info->subslice_per_slice;
> +
> +	/*
> +	 * Iterate through enabled slices and subslices to
> +	 * count the total enabled EU.
> +	 */
> +	for (s = 0; s < s_max; s++) {
> +		if (!(s_enable & (0x1 << s)))
> +			/* skip disabled slice */
> +			continue;
> +
> +		for (ss = 0; ss < ss_max; ss++) {
> +			u32 n_disabled;
> +
> +			if (ss_disable & (0x1 << ss))
> +				/* skip disabled subslice */
> +				continue;
> +
> +			n_disabled = hweight8(eu_disable[s] >> (ss * eu_max));
> +
> +			/*
> +			 * Record which subslice(s) has(have) 7 EUs. we
> +			 * can tune the hash used to spread work among
> +			 * subslices if they are unbalanced.
> +			 */
Should probably remove the part of the above comment which refers to hash
tuning since this doesn't seem to apply for BDW, only SKL.
> +			if (eu_max - n_disabled == 7)
> +				info->subslice_7eu[s] |= 1 << ss;
> +
> +			info->eu_total += eu_max - n_disabled;
> +		}
> +	}
> +
> +	/*
> +	 * BDW is expected to always have a uniform distribution of EU across
> +	 * subslices with the exception that any one EU in any one subslice may
> +	 * be fused off for die recovery.
> +	 */
> +	info->eu_per_subslice = info->subslice_total ?
> +		DIV_ROUND_UP(info->eu_total, info->subslice_total) : 0;
> +
> +	/*
> +	 * BDW supports slice power gating on devices with more than
> +	 * one slice.
> +	 */
> +	info->has_slice_pg = (info->slice_total > 1);
> +	info->has_subslice_pg = 0;
> +	info->has_eu_pg = 0;
> +}
> +
>  /*
>   * Determine various intel_device_info fields at runtime.
>   *
> @@ -783,6 +861,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  	/* Initialize slice/subslice/EU info */
>  	if (IS_CHERRYVIEW(dev))
>  		cherryview_sseu_info_init(dev);
> +	else if (IS_BROADWELL(dev))
> +		broadwell_sseu_info_init(dev);
>  	else if (INTEL_INFO(dev)->gen >= 9)
>  		gen9_sseu_info_init(dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 67bf205..da84488 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1876,11 +1876,26 @@ enum skl_disp_power_wells {
>  #define   CHV_FGT_EU_DIS_SS1_R1_MASK	(0xf << CHV_FGT_EU_DIS_SS1_R1_SHIFT)
>  
>  #define GEN8_FUSE2			0x9120
> +#define   GEN8_F2_SS_DIS_SHIFT		21
> +#define   GEN8_F2_SS_DIS_MASK		(0x7 << GEN8_F2_SS_DIS_SHIFT)
>  #define   GEN8_F2_S_ENA_SHIFT		25
>  #define   GEN8_F2_S_ENA_MASK		(0x7 << GEN8_F2_S_ENA_SHIFT)
>  
> -#define   GEN9_F2_SS_DIS_SHIFT		20
> -#define   GEN9_F2_SS_DIS_MASK		(0xf << GEN9_F2_SS_DIS_SHIFT)
I think it is helpful to keep the above two macros here rather than move
them down below the GEN8_EU_DISABLEX definitions. Having them here makes it
more clear that they are Gen9-specific field definitions for the GEN8_FUSE2
register.

> +#define GEN8_EU_DISABLE0		0x9134
> +#define   GEN8_EU_DIS0_S0_MASK		0xffffff
> +#define   GEN8_EU_DIS0_S1_SHIFT		24
> +#define   GEN8_EU_DIS0_S1_MASK		(0xff << GEN8_EU_DIS0_S1_SHIFT)
> +
> +#define GEN8_EU_DISABLE1		0x9138
> +#define   GEN8_EU_DIS1_S1_MASK		0xffff
> +#define   GEN8_EU_DIS1_S2_SHIFT		16
> +#define   GEN8_EU_DIS1_S2_MASK		(0xffff << GEN8_EU_DIS1_S2_SHIFT)
> +
> +#define GEN8_EU_DISABLE2		0x913c
> +#define   GEN8_EU_DIS2_S2_MASK		0xff
> +
> +#define GEN9_F2_SS_DIS_SHIFT		20
> +#define GEN9_F2_SS_DIS_MASK		(0xf << GEN9_F2_SS_DIS_SHIFT)
>  
>  #define GEN9_EU_DISABLE(slice)		(0x9134 + (slice)*0x4)
>  
> @@ -6867,6 +6882,9 @@ enum skl_disp_power_wells {
>  #define   GEN6_RC6			3
>  #define   GEN6_RC7			4
>  
> +#define GEN8_GT_SLICE_INFO		0x138064
> +#define   GEN8_LSLICESTAT_MASK		0x7
> +
>  #define CHV_POWER_SS0_SIG1		0xa720
>  #define CHV_POWER_SS1_SIG1		0xa728
>  #define   CHV_SS_PG_ENABLE		(1<<1)
> -- 
> Lukasz Daniluk
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx