diff mbox series

[2/9] drm/i915: Add function to set SSEU info per platform

Message ID 20190723154934.26967-3-stuart.summers@intel.com (mailing list archive)
State New, archived
Headers show
Series Refactor to expand subslice mask (rev 2) | expand

Commit Message

Summers, Stuart July 23, 2019, 3:49 p.m. UTC 
Add a new function to allow each platform to set maximum
slice, subslice, and EU information to reduce code duplication.

Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_sseu.c     |  8 +++++
 drivers/gpu/drm/i915/gt/intel_sseu.h     |  3 ++
 drivers/gpu/drm/i915/i915_debugfs.c      |  6 ++--
 drivers/gpu/drm/i915/intel_device_info.c | 39 +++++++++---------------
 4 files changed, 28 insertions(+), 28 deletions(-)

Comments

Tvrtko Ursulin July 24, 2019, 12:50 p.m. UTC | #1 
On 23/07/2019 16:49, Stuart Summers wrote:
> Add a new function to allow each platform to set maximum
> slice, subslice, and EU information to reduce code duplication.
> 
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_sseu.c     |  8 +++++
>   drivers/gpu/drm/i915/gt/intel_sseu.h     |  3 ++
>   drivers/gpu/drm/i915/i915_debugfs.c      |  6 ++--
>   drivers/gpu/drm/i915/intel_device_info.c | 39 +++++++++---------------
>   4 files changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
> index a0756f006f5f..08b74ae40739 100644
> --- a/drivers/gpu/drm/i915/gt/intel_sseu.c
> +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
> @@ -8,6 +8,14 @@
>   #include "intel_lrc_reg.h"
>   #include "intel_sseu.h"
>   
> +void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
> +			 u8 max_subslices, u8 max_eus_per_subslice)
> +{
> +	sseu->max_slices = max_slices;
> +	sseu->max_subslices = max_subslices;
> +	sseu->max_eus_per_subslice = max_eus_per_subslice;
> +}

Static inline in header since it is so trivial?

Is the term "info" established in this space? I am thinking if just 
intel_sseu_set would be enough? Or set_topology? Although it is not a 
full topology.. don't know.

> +
>   unsigned int
>   intel_sseu_subslice_total(const struct sseu_dev_info *sseu)
>   {
> diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
> index b50d0401a4e2..64e47dad07be 100644
> --- a/drivers/gpu/drm/i915/gt/intel_sseu.h
> +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
> @@ -63,6 +63,9 @@ intel_sseu_from_device_info(const struct sseu_dev_info *sseu)
>   	return value;
>   }
>   
> +void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
> +			 u8 max_subslices, u8 max_eus_per_subslice);
> +
>   unsigned int
>   intel_sseu_subslice_total(const struct sseu_dev_info *sseu);
>   
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 14eadab4209b..011ed2fd391d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4034,9 +4034,9 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>   
>   	seq_puts(m, "SSEU Device Status\n");
>   	memset(&sseu, 0, sizeof(sseu));
> -	sseu.max_slices = info->sseu.max_slices;
> -	sseu.max_subslices = info->sseu.max_subslices;
> -	sseu.max_eus_per_subslice = info->sseu.max_eus_per_subslice;
> +	intel_sseu_set_info(&sseu, info->sseu.max_slices,
> +			    info->sseu.max_subslices,
> +			    info->sseu.max_eus_per_subslice);
>   
>   	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
>   		if (IS_CHERRYVIEW(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index f99c9fd497b2..9a79d9d547c5 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -191,15 +191,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
>   	u8 eu_en;
>   	int s;
>   
> -	if (IS_ELKHARTLAKE(dev_priv)) {
> -		sseu->max_slices = 1;
> -		sseu->max_subslices = 4;
> -		sseu->max_eus_per_subslice = 8;
> -	} else {
> -		sseu->max_slices = 1;
> -		sseu->max_subslices = 8;
> -		sseu->max_eus_per_subslice = 8;
> -	}
> +	if (IS_ELKHARTLAKE(dev_priv))
> +		intel_sseu_set_info(sseu, 1, 4, 8);
> +	else
> +		intel_sseu_set_info(sseu, 1, 8, 8);
>   
>   	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
>   	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
> @@ -236,11 +231,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
>   	const int eu_mask = 0xff;
>   	u32 subslice_mask, eu_en;
>   
> +	intel_sseu_set_info(sseu, 6, 4, 8);
> +
>   	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
>   			    GEN10_F2_S_ENA_SHIFT;

Right, so intel_sseu_set_info sets some bits but not all. I guess 
topology is then definitely the wrong name. intel_sseu_set_sseu is what 
it does :), so the final suffix might be redundant be it info or sseu. 
As you prefer.

> -	sseu->max_slices = 6;
> -	sseu->max_subslices = 4;
> -	sseu->max_eus_per_subslice = 8;
>   
>   	subslice_mask = (1 << 4) - 1;
>   	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
> @@ -314,9 +308,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
>   	fuse = I915_READ(CHV_FUSE_GT);
>   
>   	sseu->slice_mask = BIT(0);
> -	sseu->max_slices = 1;
> -	sseu->max_subslices = 2;
> -	sseu->max_eus_per_subslice = 8;
> +	intel_sseu_set_info(sseu, 1, 2, 8);
>   
>   	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
>   		u8 disabled_mask =
> @@ -372,9 +364,8 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
>   	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
>   
>   	/* BXT has a single slice and at most 3 subslices. */
> -	sseu->max_slices = IS_GEN9_LP(dev_priv) ? 1 : 3;
> -	sseu->max_subslices = IS_GEN9_LP(dev_priv) ? 3 : 4;
> -	sseu->max_eus_per_subslice = 8;
> +	intel_sseu_set_info(sseu, IS_GEN9_LP(dev_priv) ? 1 : 3,
> +			    IS_GEN9_LP(dev_priv) ? 3 : 4, 8);

So far it all improved readability but not here. I think here

  if (IS_GEN9_LP(dev_priv))
	intel_sseu_set(...);
  else
	intel_sseu_set(...);

Would be cleaner.

>   
>   	/*
>   	 * The subslice disable field is global, i.e. it applies
> @@ -473,9 +464,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
>   
>   	fuse2 = I915_READ(GEN8_FUSE2);
>   	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
> -	sseu->max_slices = 3;
> -	sseu->max_subslices = 3;
> -	sseu->max_eus_per_subslice = 8;
> +	intel_sseu_set_info(sseu, 3, 3, 8);
>   
>   	/*
>   	 * The subslice disable field is global, i.e. it applies
> @@ -577,9 +566,6 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
>   		break;
>   	}
>   
> -	sseu->max_slices = hweight8(sseu->slice_mask);
> -	sseu->max_subslices = hweight8(sseu->subslice_mask[0]);
> -
>   	fuse1 = I915_READ(HSW_PAVP_FUSE1);
>   	switch ((fuse1 & HSW_F1_EU_DIS_MASK) >> HSW_F1_EU_DIS_SHIFT) {
>   	default:
> @@ -596,7 +582,10 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
>   		sseu->eu_per_subslice = 6;
>   		break;
>   	}
> -	sseu->max_eus_per_subslice = sseu->eu_per_subslice;
> +
> +	intel_sseu_set_info(sseu, hweight8(sseu->slice_mask),
> +			    hweight8(sseu->subslice_mask[0]),
> +			    sseu->eu_per_subslice);
>   
>   	for (s = 0; s < sseu->max_slices; s++) {
>   		for (ss = 0; ss < sseu->max_subslices; ss++) {
> 

Overall looks like a readability improvement. Some tweaks or not as 
mentioned above can be discussed.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
index a0756f006f5f..08b74ae40739 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -8,6 +8,14 @@ 
 #include "intel_lrc_reg.h"
 #include "intel_sseu.h"
 
+void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
+			 u8 max_subslices, u8 max_eus_per_subslice)
+{
+	sseu->max_slices = max_slices;
+	sseu->max_subslices = max_subslices;
+	sseu->max_eus_per_subslice = max_eus_per_subslice;
+}
+
 unsigned int
 intel_sseu_subslice_total(const struct sseu_dev_info *sseu)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
index b50d0401a4e2..64e47dad07be 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.h
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
@@ -63,6 +63,9 @@  intel_sseu_from_device_info(const struct sseu_dev_info *sseu)
 	return value;
 }
 
+void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
+			 u8 max_subslices, u8 max_eus_per_subslice);
+
 unsigned int
 intel_sseu_subslice_total(const struct sseu_dev_info *sseu);
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 14eadab4209b..011ed2fd391d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4034,9 +4034,9 @@  static int i915_sseu_status(struct seq_file *m, void *unused)
 
 	seq_puts(m, "SSEU Device Status\n");
 	memset(&sseu, 0, sizeof(sseu));
-	sseu.max_slices = info->sseu.max_slices;
-	sseu.max_subslices = info->sseu.max_subslices;
-	sseu.max_eus_per_subslice = info->sseu.max_eus_per_subslice;
+	intel_sseu_set_info(&sseu, info->sseu.max_slices,
+			    info->sseu.max_subslices,
+			    info->sseu.max_eus_per_subslice);
 
 	with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) {
 		if (IS_CHERRYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index f99c9fd497b2..9a79d9d547c5 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -191,15 +191,10 @@  static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
 	u8 eu_en;
 	int s;
 
-	if (IS_ELKHARTLAKE(dev_priv)) {
-		sseu->max_slices = 1;
-		sseu->max_subslices = 4;
-		sseu->max_eus_per_subslice = 8;
-	} else {
-		sseu->max_slices = 1;
-		sseu->max_subslices = 8;
-		sseu->max_eus_per_subslice = 8;
-	}
+	if (IS_ELKHARTLAKE(dev_priv))
+		intel_sseu_set_info(sseu, 1, 4, 8);
+	else
+		intel_sseu_set_info(sseu, 1, 8, 8);
 
 	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
 	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
@@ -236,11 +231,10 @@  static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
 	const int eu_mask = 0xff;
 	u32 subslice_mask, eu_en;
 
+	intel_sseu_set_info(sseu, 6, 4, 8);
+
 	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
 			    GEN10_F2_S_ENA_SHIFT;
-	sseu->max_slices = 6;
-	sseu->max_subslices = 4;
-	sseu->max_eus_per_subslice = 8;
 
 	subslice_mask = (1 << 4) - 1;
 	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
@@ -314,9 +308,7 @@  static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 	fuse = I915_READ(CHV_FUSE_GT);
 
 	sseu->slice_mask = BIT(0);
-	sseu->max_slices = 1;
-	sseu->max_subslices = 2;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, 1, 2, 8);
 
 	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
 		u8 disabled_mask =
@@ -372,9 +364,8 @@  static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
 
 	/* BXT has a single slice and at most 3 subslices. */
-	sseu->max_slices = IS_GEN9_LP(dev_priv) ? 1 : 3;
-	sseu->max_subslices = IS_GEN9_LP(dev_priv) ? 3 : 4;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, IS_GEN9_LP(dev_priv) ? 1 : 3,
+			    IS_GEN9_LP(dev_priv) ? 3 : 4, 8);
 
 	/*
 	 * The subslice disable field is global, i.e. it applies
@@ -473,9 +464,7 @@  static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
 
 	fuse2 = I915_READ(GEN8_FUSE2);
 	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
-	sseu->max_slices = 3;
-	sseu->max_subslices = 3;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, 3, 3, 8);
 
 	/*
 	 * The subslice disable field is global, i.e. it applies
@@ -577,9 +566,6 @@  static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	sseu->max_slices = hweight8(sseu->slice_mask);
-	sseu->max_subslices = hweight8(sseu->subslice_mask[0]);
-
 	fuse1 = I915_READ(HSW_PAVP_FUSE1);
 	switch ((fuse1 & HSW_F1_EU_DIS_MASK) >> HSW_F1_EU_DIS_SHIFT) {
 	default:
@@ -596,7 +582,10 @@  static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
 		sseu->eu_per_subslice = 6;
 		break;
 	}
-	sseu->max_eus_per_subslice = sseu->eu_per_subslice;
+
+	intel_sseu_set_info(sseu, hweight8(sseu->slice_mask),
+			    hweight8(sseu->subslice_mask[0]),
+			    sseu->eu_per_subslice);
 
 	for (s = 0; s < sseu->max_slices; s++) {
 		for (ss = 0; ss < sseu->max_subslices; ss++) {