All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] drm/i915: read out slice/subslice masks
@ 2015-10-21 15:40 Imre Deak
  2015-10-21 15:40 ` [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h Imre Deak
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

The per-slice/subslice INSTDONE patchset from Ben [1] will need the
subslice/slice masks in addition to the corresponding counts that we
maintain atm. So I added support to store the masks instead of the
counts and calculate the counts whenever we need them based on the
masks. While at it I also noticed that the SSEU readout code could be
simplified by reusing the data structure storing the SSEU properties.

Tested on BXT/SKL.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2015-September/077050.html

Imre Deak (7):
  drm/i915: sseu: move sseu_dev_status to i915_drv.h
  drm/i915: sseu: use sseu_dev_info in device info
  drm/i915: sseu: simplify debugfs status/info printing
  drm/i915: sseu: convert slice count field to mask
  drm/i915: sseu: convert subslice count fields to subslice mask
  drm/i915: sseu: add debug printf for slice/subslice masks
  drm/i915/bdw: sseu: fix sseu status parsing

 drivers/gpu/drm/i915/i915_debugfs.c     | 134 +++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_dma.c         | 115 +++++++++++++--------------
 drivers/gpu/drm/i915/i915_drv.h         |  28 ++++---
 drivers/gpu/drm/i915/intel_lrc.c        |  14 ++--
 drivers/gpu/drm/i915/intel_pm.c         |   2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   4 +-
 6 files changed, 150 insertions(+), 147 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-19 23:10   ` Ben Widawsky
  2015-10-21 15:40 ` [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info Imre Deak
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

The data in this struct is provided both by getting the
slice/subslice/eu features available on a given platform and the actual
runtime state of these same features which depends on the HW's current
power saving state.

Atm members of this struct are duplicated in sseu_dev_status and
intel_device_info. For clarity and code reuse we can share one struct
for both of the above purposes. This patch only moves the struct to the
header file, the next patch will convert users of intel_device_info to
use this struct too.

Instead of unsigned int u8 is used now, which is big enough and is used
anyway in intel_device_info.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 29 ++++++++++++-----------------
 drivers/gpu/drm/i915/i915_drv.h     |  8 ++++++++
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a3b22bd..3dd7076 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4945,16 +4945,8 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
 			i915_cache_sharing_get, i915_cache_sharing_set,
 			"%llu\n");
 
-struct sseu_dev_status {
-	unsigned int slice_total;
-	unsigned int subslice_total;
-	unsigned int subslice_per_slice;
-	unsigned int eu_total;
-	unsigned int eu_per_subslice;
-};
-
 static void cherryview_sseu_device_status(struct drm_device *dev,
-					  struct sseu_dev_status *stat)
+					  struct sseu_dev_info *stat)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ss_max = 2;
@@ -4980,13 +4972,14 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
 			 ((sig1[ss] & CHV_EU210_PG_ENABLE) ? 0 : 2) +
 			 ((sig2[ss] & CHV_EU311_PG_ENABLE) ? 0 : 2);
 		stat->eu_total += eu_cnt;
-		stat->eu_per_subslice = max(stat->eu_per_subslice, eu_cnt);
+		stat->eu_per_subslice = max_t(unsigned int,
+					      stat->eu_per_subslice, eu_cnt);
 	}
 	stat->subslice_total = stat->subslice_per_slice;
 }
 
 static void gen9_sseu_device_status(struct drm_device *dev,
-				    struct sseu_dev_status *stat)
+				    struct sseu_dev_info *stat)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int s_max = 3, ss_max = 4;
@@ -5040,18 +5033,20 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
 					       eu_mask[ss%2]);
 			stat->eu_total += eu_cnt;
-			stat->eu_per_subslice = max(stat->eu_per_subslice,
-						    eu_cnt);
+			stat->eu_per_subslice = max_t(unsigned int,
+						      stat->eu_per_subslice,
+						      eu_cnt);
 		}
 
 		stat->subslice_total += ss_cnt;
-		stat->subslice_per_slice = max(stat->subslice_per_slice,
-					       ss_cnt);
+		stat->subslice_per_slice = max_t(unsigned int,
+						 stat->subslice_per_slice,
+						 ss_cnt);
 	}
 }
 
 static void broadwell_sseu_device_status(struct drm_device *dev,
-					 struct sseu_dev_status *stat)
+					 struct sseu_dev_info *stat)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int s;
@@ -5079,7 +5074,7 @@ 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;
+	struct sseu_dev_info stat;
 
 	if (INTEL_INFO(dev)->gen < 8)
 		return -ENODEV;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8afda45..73ff01f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -782,6 +782,14 @@ struct intel_csr {
 #define DEFINE_FLAG(name) u8 name:1
 #define SEP_SEMICOLON ;
 
+struct sseu_dev_info {
+	u8 slice_total;
+	u8 subslice_total;
+	u8 subslice_per_slice;
+	u8 eu_total;
+	u8 eu_per_subslice;
+};
+
 struct intel_device_info {
 	u32 display_mmio_offset;
 	u16 device_id;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
  2015-10-21 15:40 ` [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-19 23:18   ` Ben Widawsky
  2015-10-21 15:40 ` [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing Imre Deak
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Move all slice/subslice/eu related properties to the sseu_dev_info
struct.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 25 ++++++++++++------------
 drivers/gpu/drm/i915/i915_dma.c         | 34 +++++++++++++++++----------------
 drivers/gpu/drm/i915/i915_drv.h         | 16 ++++++----------
 drivers/gpu/drm/i915/intel_lrc.c        | 14 +++++++-------
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
 6 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3dd7076..de188d0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5017,7 +5017,7 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 		stat->slice_total++;
 
 		if (IS_SKYLAKE(dev))
-			ss_cnt = INTEL_INFO(dev)->subslice_per_slice;
+			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
 
 		for (ss = 0; ss < ss_max; ss++) {
 			unsigned int eu_cnt;
@@ -5055,15 +5055,16 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
 	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_per_slice =
+				INTEL_INFO(dev)->sseu.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_per_subslice = INTEL_INFO(dev)->sseu.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];
+			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
 
 			stat->eu_total -= hweight8(subslice_7eu);
 		}
@@ -5081,21 +5082,21 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 
 	seq_puts(m, "SSEU Device Info\n");
 	seq_printf(m, "  Available Slice Total: %u\n",
-		   INTEL_INFO(dev)->slice_total);
+		   INTEL_INFO(dev)->sseu.slice_total);
 	seq_printf(m, "  Available Subslice Total: %u\n",
-		   INTEL_INFO(dev)->subslice_total);
+		   INTEL_INFO(dev)->sseu.subslice_total);
 	seq_printf(m, "  Available Subslice Per Slice: %u\n",
-		   INTEL_INFO(dev)->subslice_per_slice);
+		   INTEL_INFO(dev)->sseu.subslice_per_slice);
 	seq_printf(m, "  Available EU Total: %u\n",
-		   INTEL_INFO(dev)->eu_total);
+		   INTEL_INFO(dev)->sseu.eu_total);
 	seq_printf(m, "  Available EU Per Subslice: %u\n",
-		   INTEL_INFO(dev)->eu_per_subslice);
+		   INTEL_INFO(dev)->sseu.eu_per_subslice);
 	seq_printf(m, "  Has Slice Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->has_slice_pg));
+		   yesno(INTEL_INFO(dev)->sseu.has_slice_pg));
 	seq_printf(m, "  Has Subslice Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->has_subslice_pg));
+		   yesno(INTEL_INFO(dev)->sseu.has_subslice_pg));
 	seq_printf(m, "  Has EU Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->has_eu_pg));
+		   yesno(INTEL_INFO(dev)->sseu.has_eu_pg));
 
 	seq_puts(m, "SSEU Device Status\n");
 	memset(&stat, 0, sizeof(stat));
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..be8e141 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -154,12 +154,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		value = 1;
 		break;
 	case I915_PARAM_SUBSLICE_TOTAL:
-		value = INTEL_INFO(dev)->subslice_total;
+		value = INTEL_INFO(dev)->sseu.subslice_total;
 		if (!value)
 			return -ENODEV;
 		break;
 	case I915_PARAM_EU_TOTAL:
-		value = INTEL_INFO(dev)->eu_total;
+		value = INTEL_INFO(dev)->sseu.eu_total;
 		if (!value)
 			return -ENODEV;
 		break;
@@ -553,10 +553,10 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 static void cherryview_sseu_info_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_device_info *info;
+	struct sseu_dev_info *info;
 	u32 fuse, eu_dis;
 
-	info = (struct intel_device_info *)&dev_priv->info;
+	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
 	fuse = I915_READ(CHV_FUSE_GT);
 
 	info->slice_total = 1;
@@ -596,13 +596,13 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
 static void gen9_sseu_info_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_device_info *info;
+	struct sseu_dev_info *info;
 	int s_max = 3, ss_max = 4, eu_max = 8;
 	int s, ss;
 	u32 fuse2, s_enable, ss_disable, eu_disable;
 	u8 eu_mask = 0xff;
 
-	info = (struct intel_device_info *)&dev_priv->info;
+	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
 	fuse2 = I915_READ(GEN8_FUSE2);
 	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
 		   GEN8_F2_S_ENA_SHIFT;
@@ -676,7 +676,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 static void broadwell_sseu_info_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_device_info *info;
+	struct sseu_dev_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;
@@ -694,7 +694,8 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 			 (32 - GEN8_EU_DIS1_S2_SHIFT));
 
 
-	info = (struct intel_device_info *)&dev_priv->info;
+	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
+
 	info->slice_total = hweight32(s_enable);
 
 	/*
@@ -824,17 +825,18 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 	else if (INTEL_INFO(dev)->gen >= 9)
 		gen9_sseu_info_init(dev);
 
-	DRM_DEBUG_DRIVER("slice total: %u\n", info->slice_total);
-	DRM_DEBUG_DRIVER("subslice total: %u\n", info->subslice_total);
-	DRM_DEBUG_DRIVER("subslice per slice: %u\n", info->subslice_per_slice);
-	DRM_DEBUG_DRIVER("EU total: %u\n", info->eu_total);
-	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->eu_per_subslice);
+	DRM_DEBUG_DRIVER("slice total: %u\n", info->sseu.slice_total);
+	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
+	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
+						info->sseu.subslice_per_slice);
+	DRM_DEBUG_DRIVER("EU total: %u\n", info->sseu.eu_total);
+	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->sseu.eu_per_subslice);
 	DRM_DEBUG_DRIVER("has slice power gating: %s\n",
-			 info->has_slice_pg ? "y" : "n");
+			 info->sseu.has_slice_pg ? "y" : "n");
 	DRM_DEBUG_DRIVER("has subslice power gating: %s\n",
-			 info->has_subslice_pg ? "y" : "n");
+			 info->sseu.has_subslice_pg ? "y" : "n");
 	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
-			 info->has_eu_pg ? "y" : "n");
+			 info->sseu.has_eu_pg ? "y" : "n");
 }
 
 static void intel_init_dpio(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 73ff01f..cb0427d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -788,6 +788,11 @@ struct sseu_dev_info {
 	u8 subslice_per_slice;
 	u8 eu_total;
 	u8 eu_per_subslice;
+	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
+	u8 subslice_7eu[3];
+	u8 has_slice_pg:1;
+	u8 has_subslice_pg:1;
+	u8 has_eu_pg:1;
 };
 
 struct intel_device_info {
@@ -805,16 +810,7 @@ struct intel_device_info {
 	int cursor_offsets[I915_MAX_PIPES];
 
 	/* Slice/subslice/EU info */
-	u8 slice_total;
-	u8 subslice_total;
-	u8 subslice_per_slice;
-	u8 eu_total;
-	u8 eu_per_subslice;
-	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
-	u8 subslice_7eu[3];
-	u8 has_slice_pg:1;
-	u8 has_subslice_pg:1;
-	u8 has_eu_pg:1;
+	struct sseu_dev_info sseu;
 };
 
 #undef DEFINE_FLAG
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 88e12bd..8a55f8a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2200,24 +2200,24 @@ make_rpcs(struct drm_device *dev)
 	 * must make an explicit request through RPCS for full
 	 * enablement.
 	*/
-	if (INTEL_INFO(dev)->has_slice_pg) {
+	if (INTEL_INFO(dev)->sseu.has_slice_pg) {
 		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
-		rpcs |= INTEL_INFO(dev)->slice_total <<
+		rpcs |= INTEL_INFO(dev)->sseu.slice_total <<
 			GEN8_RPCS_S_CNT_SHIFT;
 		rpcs |= GEN8_RPCS_ENABLE;
 	}
 
-	if (INTEL_INFO(dev)->has_subslice_pg) {
+	if (INTEL_INFO(dev)->sseu.has_subslice_pg) {
 		rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
-		rpcs |= INTEL_INFO(dev)->subslice_per_slice <<
+		rpcs |= INTEL_INFO(dev)->sseu.subslice_per_slice <<
 			GEN8_RPCS_SS_CNT_SHIFT;
 		rpcs |= GEN8_RPCS_ENABLE;
 	}
 
-	if (INTEL_INFO(dev)->has_eu_pg) {
-		rpcs |= INTEL_INFO(dev)->eu_per_subslice <<
+	if (INTEL_INFO(dev)->sseu.has_eu_pg) {
+		rpcs |= INTEL_INFO(dev)->sseu.eu_per_subslice <<
 			GEN8_RPCS_EU_MIN_SHIFT;
-		rpcs |= INTEL_INFO(dev)->eu_per_subslice <<
+		rpcs |= INTEL_INFO(dev)->sseu.eu_per_subslice <<
 			GEN8_RPCS_EU_MAX_SHIFT;
 		rpcs |= GEN8_RPCS_ENABLE;
 	}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index df22b9c..dad0a67 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5101,7 +5101,7 @@ static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv)
 
 	val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE);
 
-	switch (INTEL_INFO(dev)->eu_total) {
+	switch (INTEL_INFO(dev)->sseu.eu_total) {
 	case 8:
 		/* (2 * 4) config */
 		rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a492705..57a756d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1000,7 +1000,7 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring)
 		 * Only consider slices where one, and only one, subslice has 7
 		 * EUs
 		 */
-		if (hweight8(dev_priv->info.subslice_7eu[i]) != 1)
+		if (hweight8(INTEL_INFO(dev_priv)->sseu.subslice_7eu[i]) != 1)
 			continue;
 
 		/*
@@ -1009,7 +1009,7 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring)
 		 *
 		 * ->    0 <= ss <= 3;
 		 */
-		ss = ffs(dev_priv->info.subslice_7eu[i]) - 1;
+		ss = ffs(INTEL_INFO(dev_priv)->sseu.subslice_7eu[i]) - 1;
 		vals[i] = 3 - ss;
 	}
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
  2015-10-21 15:40 ` [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h Imre Deak
  2015-10-21 15:40 ` [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-19 23:24   ` Ben Widawsky
  2015-10-21 15:40 ` [PATCH 4/7] drm/i915: sseu: convert slice count field to mask Imre Deak
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 55 +++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index de188d0..183c1f2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5071,6 +5071,33 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
 	}
 }
 
+static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
+				 const struct sseu_dev_info *sseu)
+{
+	const char *type = is_available_info ? "Available" : "Enabled";
+
+	seq_printf(m, "  %s Slice Total: %u\n", type,
+		   sseu->slice_total);
+	seq_printf(m, "  %s Subslice Total: %u\n", type,
+		   sseu->subslice_total);
+	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
+		   sseu->subslice_per_slice);
+	seq_printf(m, "  %s EU Total: %u\n", type,
+		   sseu->eu_total);
+	seq_printf(m, "  %s EU Per Subslice: %u\n", type,
+		   sseu->eu_per_subslice);
+
+	if (!is_available_info)
+		return;
+
+	seq_printf(m, "  Has Slice Power Gating: %s\n",
+		   yesno(sseu->has_slice_pg));
+	seq_printf(m, "  Has Subslice Power Gating: %s\n",
+		   yesno(sseu->has_subslice_pg));
+	seq_printf(m, "  Has EU Power Gating: %s\n",
+		   yesno(sseu->has_eu_pg));
+}
+
 static int i915_sseu_status(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -5081,22 +5108,7 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 		return -ENODEV;
 
 	seq_puts(m, "SSEU Device Info\n");
-	seq_printf(m, "  Available Slice Total: %u\n",
-		   INTEL_INFO(dev)->sseu.slice_total);
-	seq_printf(m, "  Available Subslice Total: %u\n",
-		   INTEL_INFO(dev)->sseu.subslice_total);
-	seq_printf(m, "  Available Subslice Per Slice: %u\n",
-		   INTEL_INFO(dev)->sseu.subslice_per_slice);
-	seq_printf(m, "  Available EU Total: %u\n",
-		   INTEL_INFO(dev)->sseu.eu_total);
-	seq_printf(m, "  Available EU Per Subslice: %u\n",
-		   INTEL_INFO(dev)->sseu.eu_per_subslice);
-	seq_printf(m, "  Has Slice Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->sseu.has_slice_pg));
-	seq_printf(m, "  Has Subslice Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->sseu.has_subslice_pg));
-	seq_printf(m, "  Has EU Power Gating: %s\n",
-		   yesno(INTEL_INFO(dev)->sseu.has_eu_pg));
+	i915_print_sseu_info(m, true, &INTEL_INFO(dev)->sseu);
 
 	seq_puts(m, "SSEU Device Status\n");
 	memset(&stat, 0, sizeof(stat));
@@ -5107,16 +5119,7 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 	} else if (INTEL_INFO(dev)->gen >= 9) {
 		gen9_sseu_device_status(dev, &stat);
 	}
-	seq_printf(m, "  Enabled Slice Total: %u\n",
-		   stat.slice_total);
-	seq_printf(m, "  Enabled Subslice Total: %u\n",
-		   stat.subslice_total);
-	seq_printf(m, "  Enabled Subslice Per Slice: %u\n",
-		   stat.subslice_per_slice);
-	seq_printf(m, "  Enabled EU Total: %u\n",
-		   stat.eu_total);
-	seq_printf(m, "  Enabled EU Per Subslice: %u\n",
-		   stat.eu_per_subslice);
+	i915_print_sseu_info(m, false, &stat);
 
 	return 0;
 }
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] drm/i915: sseu: convert slice count field to mask
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
                   ` (2 preceding siblings ...)
  2015-10-21 15:40 ` [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-19 23:40   ` Ben Widawsky
  2015-10-21 15:40 ` [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask Imre Deak
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

In an upcoming patch we'll need the actual mask of slices in addition to
their count, so replace the count field with a mask.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++-------
 drivers/gpu/drm/i915/i915_dma.c     | 37 +++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
 4 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 183c1f2..390dc59 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4965,7 +4965,7 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
 			/* skip disabled subslice */
 			continue;
 
-		stat->slice_total = 1;
+		stat->slice_mask = BIT(0);
 		stat->subslice_per_slice++;
 		eu_cnt = ((sig1[ss] & CHV_EU08_PG_ENABLE) ? 0 : 2) +
 			 ((sig1[ss] & CHV_EU19_PG_ENABLE) ? 0 : 2) +
@@ -5014,7 +5014,7 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 			/* skip disabled slice */
 			continue;
 
-		stat->slice_total++;
+		stat->slice_mask |= BIT(s);
 
 		if (IS_SKYLAKE(dev))
 			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
@@ -5052,18 +5052,18 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
 	int s;
 	u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
 
-	stat->slice_total = hweight32(slice_info & GEN8_LSLICESTAT_MASK);
+	stat->slice_mask = slice_info & GEN8_LSLICESTAT_MASK;
 
-	if (stat->slice_total) {
+	if (stat->slice_mask) {
 		stat->subslice_per_slice =
 				INTEL_INFO(dev)->sseu.subslice_per_slice;
-		stat->subslice_total = stat->slice_total *
+		stat->subslice_total = hweight32(stat->slice_mask) *
 				       stat->subslice_per_slice;
 		stat->eu_per_subslice = INTEL_INFO(dev)->sseu.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++) {
+		for (s = 0; s < hweight32(stat->slice_mask); s++) {
 			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
 
 			stat->eu_total -= hweight8(subslice_7eu);
@@ -5077,7 +5077,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 	const char *type = is_available_info ? "Available" : "Enabled";
 
 	seq_printf(m, "  %s Slice Total: %u\n", type,
-		   sseu->slice_total);
+		   hweight32(sseu->slice_mask));
 	seq_printf(m, "  %s Subslice Total: %u\n", type,
 		   sseu->subslice_total);
 	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index be8e141..1f5f3a7d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -559,7 +559,7 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
 	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
 	fuse = I915_READ(CHV_FUSE_GT);
 
-	info->slice_total = 1;
+	info->slice_mask = BIT(0);
 
 	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
 		info->subslice_per_slice++;
@@ -599,23 +599,21 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	struct sseu_dev_info *info;
 	int s_max = 3, ss_max = 4, eu_max = 8;
 	int s, ss;
-	u32 fuse2, s_enable, ss_disable, eu_disable;
+	u32 fuse2, ss_disable, eu_disable;
 	u8 eu_mask = 0xff;
 
 	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
 	fuse2 = I915_READ(GEN8_FUSE2);
-	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
-		   GEN8_F2_S_ENA_SHIFT;
+	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
 	ss_disable = (fuse2 & GEN9_F2_SS_DIS_MASK) >>
 		     GEN9_F2_SS_DIS_SHIFT;
 
-	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_total = hweight32(info->slice_mask) *
 			       info->subslice_per_slice;
 
 	/*
@@ -623,7 +621,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	 * count the total enabled EU.
 	*/
 	for (s = 0; s < s_max; s++) {
-		if (!(s_enable & (0x1 << s)))
+		if (!(info->slice_mask & BIT(s)))
 			/* skip disabled slice */
 			continue;
 
@@ -668,7 +666,8 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	 * supports EU power gating on devices with more than one EU
 	 * pair per subslice.
 	*/
-	info->has_slice_pg = (IS_SKYLAKE(dev) && (info->slice_total > 1));
+	info->has_slice_pg = IS_SKYLAKE(dev) &&
+			     hweight32(info->slice_mask) > 1;
 	info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total > 1));
 	info->has_eu_pg = (info->eu_per_subslice > 2);
 }
@@ -679,10 +678,14 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 	struct sseu_dev_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;
+	u32 fuse2, eu_disable[s_max], ss_disable;
+
+	info = (struct sseu_dev_info *)&dev_priv->info.sseu;
 
 	fuse2 = I915_READ(GEN8_FUSE2);
-	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
+
+	info->slice_mask = (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;
@@ -694,23 +697,20 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 			 (32 - GEN8_EU_DIS1_S2_SHIFT));
 
 
-	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
-
-	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;
+	info->subslice_total = hweight32(info->slice_mask) *
+			       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)))
+		if (!(info->slice_mask & BIT(s)))
 			/* skip disabled slice */
 			continue;
 
@@ -745,7 +745,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 	 * BDW supports slice power gating on devices with more than
 	 * one slice.
 	 */
-	info->has_slice_pg = (info->slice_total > 1);
+	info->has_slice_pg = hweight32(info->slice_mask) > 1;
 	info->has_subslice_pg = 0;
 	info->has_eu_pg = 0;
 }
@@ -825,7 +825,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 	else if (INTEL_INFO(dev)->gen >= 9)
 		gen9_sseu_info_init(dev);
 
-	DRM_DEBUG_DRIVER("slice total: %u\n", info->sseu.slice_total);
+	DRM_DEBUG_DRIVER("slice total: %u\n",
+			 hweight32(info->sseu.slice_mask));
 	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
 	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
 						info->sseu.subslice_per_slice);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cb0427d..7cc9ec8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -783,7 +783,7 @@ struct intel_csr {
 #define SEP_SEMICOLON ;
 
 struct sseu_dev_info {
-	u8 slice_total;
+	u8 slice_mask;
 	u8 subslice_total;
 	u8 subslice_per_slice;
 	u8 eu_total;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8a55f8a..4130ff1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2202,7 +2202,7 @@ make_rpcs(struct drm_device *dev)
 	*/
 	if (INTEL_INFO(dev)->sseu.has_slice_pg) {
 		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
-		rpcs |= INTEL_INFO(dev)->sseu.slice_total <<
+		rpcs |= hweight32(INTEL_INFO(dev)->sseu.slice_mask) <<
 			GEN8_RPCS_S_CNT_SHIFT;
 		rpcs |= GEN8_RPCS_ENABLE;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
                   ` (3 preceding siblings ...)
  2015-10-21 15:40 ` [PATCH 4/7] drm/i915: sseu: convert slice count field to mask Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-20  0:07   ` Ben Widawsky
  2015-10-21 15:40 ` [PATCH 6/7] drm/i915: sseu: add debug printf for slice/subslice masks Imre Deak
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

In an upcoming patch we'll need the actual mask of subslices in addition
to their count, so convert the subslice_per_slice field to a mask.
Also we can easily calculate subslice_total from the other fields, so
instead of storing a cached version of this, add a helper to calculate
it.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 37 ++++++++-------------
 drivers/gpu/drm/i915/i915_dma.c     | 64 +++++++++++++++++--------------------
 drivers/gpu/drm/i915/i915_drv.h     |  8 +++--
 drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
 4 files changed, 51 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 390dc59..3fb83ea 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4966,7 +4966,7 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
 			continue;
 
 		stat->slice_mask = BIT(0);
-		stat->subslice_per_slice++;
+		stat->subslice_mask |= BIT(ss);
 		eu_cnt = ((sig1[ss] & CHV_EU08_PG_ENABLE) ? 0 : 2) +
 			 ((sig1[ss] & CHV_EU19_PG_ENABLE) ? 0 : 2) +
 			 ((sig1[ss] & CHV_EU210_PG_ENABLE) ? 0 : 2) +
@@ -4975,7 +4975,6 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
 		stat->eu_per_subslice = max_t(unsigned int,
 					      stat->eu_per_subslice, eu_cnt);
 	}
-	stat->subslice_total = stat->subslice_per_slice;
 }
 
 static void gen9_sseu_device_status(struct drm_device *dev,
@@ -5008,8 +5007,6 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 		     GEN9_PGCTL_SSB_EU311_ACK;
 
 	for (s = 0; s < s_max; s++) {
-		unsigned int ss_cnt = 0;
-
 		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
 			/* skip disabled slice */
 			continue;
@@ -5017,18 +5014,19 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 		stat->slice_mask |= BIT(s);
 
 		if (IS_SKYLAKE(dev))
-			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
+			stat->subslice_mask =
+				INTEL_INFO(dev_priv)->sseu.subslice_mask;
 
 		for (ss = 0; ss < ss_max; ss++) {
 			unsigned int eu_cnt;
 
-			if (IS_BROXTON(dev) &&
-			    !(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
-				/* skip disabled subslice */
-				continue;
+			if (IS_BROXTON(dev)) {
+				if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
+					/* skip disabled subslice */
+					continue;
 
-			if (IS_BROXTON(dev))
-				ss_cnt++;
+				stat->subslice_mask |= BIT(ss);
+			}
 
 			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
 					       eu_mask[ss%2]);
@@ -5037,11 +5035,6 @@ static void gen9_sseu_device_status(struct drm_device *dev,
 						      stat->eu_per_subslice,
 						      eu_cnt);
 		}
-
-		stat->subslice_total += ss_cnt;
-		stat->subslice_per_slice = max_t(unsigned int,
-						 stat->subslice_per_slice,
-						 ss_cnt);
 	}
 }
 
@@ -5055,12 +5048,10 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
 	stat->slice_mask = slice_info & GEN8_LSLICESTAT_MASK;
 
 	if (stat->slice_mask) {
-		stat->subslice_per_slice =
-				INTEL_INFO(dev)->sseu.subslice_per_slice;
-		stat->subslice_total = hweight32(stat->slice_mask) *
-				       stat->subslice_per_slice;
+		stat->subslice_mask = INTEL_INFO(dev)->sseu.subslice_mask;
 		stat->eu_per_subslice = INTEL_INFO(dev)->sseu.eu_per_subslice;
-		stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
+		stat->eu_total = stat->eu_per_subslice *
+				 sseu_subslice_total(stat);
 
 		/* subtract fused off EU(s) from enabled slice(s) */
 		for (s = 0; s < hweight32(stat->slice_mask); s++) {
@@ -5079,9 +5070,9 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 	seq_printf(m, "  %s Slice Total: %u\n", type,
 		   hweight32(sseu->slice_mask));
 	seq_printf(m, "  %s Subslice Total: %u\n", type,
-		   sseu->subslice_total);
+		   sseu_subslice_total(sseu));
 	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
-		   sseu->subslice_per_slice);
+		   hweight32(sseu->subslice_mask));
 	seq_printf(m, "  %s EU Total: %u\n", type,
 		   sseu->eu_total);
 	seq_printf(m, "  %s EU Per Subslice: %u\n", type,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1f5f3a7d..69a81ae9 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -154,7 +154,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		value = 1;
 		break;
 	case I915_PARAM_SUBSLICE_TOTAL:
-		value = INTEL_INFO(dev)->sseu.subslice_total;
+		value = sseu_subslice_total(&INTEL_INFO(dev)->sseu);
 		if (!value)
 			return -ENODEV;
 		break;
@@ -562,26 +562,25 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
 	info->slice_mask = BIT(0);
 
 	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
-		info->subslice_per_slice++;
+		info->subslice_mask |= BIT(0);
 		eu_dis = fuse & (CHV_FGT_EU_DIS_SS0_R0_MASK |
 				 CHV_FGT_EU_DIS_SS0_R1_MASK);
 		info->eu_total += 8 - hweight32(eu_dis);
 	}
 
 	if (!(fuse & CHV_FGT_DISABLE_SS1)) {
-		info->subslice_per_slice++;
+		info->subslice_mask |= BIT(1);
 		eu_dis = fuse & (CHV_FGT_EU_DIS_SS1_R0_MASK |
 				 CHV_FGT_EU_DIS_SS1_R1_MASK);
 		info->eu_total += 8 - hweight32(eu_dis);
 	}
 
-	info->subslice_total = info->subslice_per_slice;
 	/*
 	 * CHV expected to always have a uniform distribution of EU
 	 * across subslices.
 	*/
-	info->eu_per_subslice = info->subslice_total ?
-				info->eu_total / info->subslice_total :
+	info->eu_per_subslice = sseu_subslice_total(info) ?
+				info->eu_total / sseu_subslice_total(info) :
 				0;
 	/*
 	 * CHV supports subslice power gating on devices with more than
@@ -589,7 +588,7 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
 	 * more than one EU pair per subslice.
 	*/
 	info->has_slice_pg = 0;
-	info->has_subslice_pg = (info->subslice_total > 1);
+	info->has_subslice_pg = sseu_subslice_total(info) > 1;
 	info->has_eu_pg = (info->eu_per_subslice > 2);
 }
 
@@ -599,22 +598,19 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	struct sseu_dev_info *info;
 	int s_max = 3, ss_max = 4, eu_max = 8;
 	int s, ss;
-	u32 fuse2, ss_disable, eu_disable;
+	u32 fuse2, eu_disable;
 	u8 eu_mask = 0xff;
 
 	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
 	fuse2 = I915_READ(GEN8_FUSE2);
 	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
-	ss_disable = (fuse2 & GEN9_F2_SS_DIS_MASK) >>
-		     GEN9_F2_SS_DIS_SHIFT;
-
 	/*
 	 * 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 = hweight32(info->slice_mask) *
-			       info->subslice_per_slice;
+	info->subslice_mask = (1 << ss_max) - 1;
+	info->subslice_mask &= ~((fuse2 & GEN9_F2_SS_DIS_MASK) >>
+				GEN9_F2_SS_DIS_SHIFT);
 
 	/*
 	 * Iterate through enabled slices and subslices to
@@ -629,7 +625,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 		for (ss = 0; ss < ss_max; ss++) {
 			int eu_per_ss;
 
-			if (ss_disable & (0x1 << ss))
+			if (!(info->subslice_mask & BIT(ss)))
 				/* skip disabled subslice */
 				continue;
 
@@ -655,9 +651,9 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	 * recovery. BXT is expected to be perfectly uniform in EU
 	 * distribution.
 	*/
-	info->eu_per_subslice = info->subslice_total ?
+	info->eu_per_subslice = sseu_subslice_total(info) ?
 				DIV_ROUND_UP(info->eu_total,
-					     info->subslice_total) : 0;
+					     sseu_subslice_total(info)) : 0;
 	/*
 	 * SKL supports slice power gating on devices with more than
 	 * one slice, and supports EU power gating on devices with
@@ -668,7 +664,8 @@ static void gen9_sseu_info_init(struct drm_device *dev)
 	*/
 	info->has_slice_pg = IS_SKYLAKE(dev) &&
 			     hweight32(info->slice_mask) > 1;
-	info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total > 1));
+	info->has_subslice_pg = IS_BROXTON(dev) &&
+				sseu_subslice_total(info) > 1;
 	info->has_eu_pg = (info->eu_per_subslice > 2);
 }
 
@@ -678,7 +675,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 	struct sseu_dev_info *info;
 	const int s_max = 3, ss_max = 3, eu_max = 8;
 	int s, ss;
-	u32 fuse2, eu_disable[s_max], ss_disable;
+	u32 fuse2, eu_disable[s_max];
 
 	info = (struct sseu_dev_info *)&dev_priv->info.sseu;
 
@@ -686,7 +683,13 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 
 	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >>
 				GEN8_F2_S_ENA_SHIFT;
-	ss_disable = (fuse2 & GEN8_F2_SS_DIS_MASK) >> GEN8_F2_SS_DIS_SHIFT;
+	/*
+	 * The subslice disable field is global, i.e. it applies
+	 * to each of the enabled slices.
+	 */
+	info->subslice_mask = (1 << ss_max) - 1;
+	info->subslice_mask &= ~((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) |
@@ -696,15 +699,6 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 			((I915_READ(GEN8_EU_DISABLE2) & GEN8_EU_DIS2_S2_MASK) <<
 			 (32 - GEN8_EU_DIS1_S2_SHIFT));
 
-
-	/*
-	 * 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 = hweight32(info->slice_mask) *
-			       info->subslice_per_slice;
-
 	/*
 	 * Iterate through enabled slices and subslices to
 	 * count the total enabled EU.
@@ -717,7 +711,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 		for (ss = 0; ss < ss_max; ss++) {
 			u32 n_disabled;
 
-			if (ss_disable & (0x1 << ss))
+			if (!(info->subslice_mask & BIT(ss)))
 				/* skip disabled subslice */
 				continue;
 
@@ -738,8 +732,9 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
 	 * 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;
+	info->eu_per_subslice = sseu_subslice_total(info) ?
+				DIV_ROUND_UP(info->eu_total,
+					     sseu_subslice_total(info)) : 0;
 
 	/*
 	 * BDW supports slice power gating on devices with more than
@@ -827,9 +822,10 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 
 	DRM_DEBUG_DRIVER("slice total: %u\n",
 			 hweight32(info->sseu.slice_mask));
-	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
+	DRM_DEBUG_DRIVER("subslice total: %u\n",
+			 sseu_subslice_total(&info->sseu));
 	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
-						info->sseu.subslice_per_slice);
+			 hweight32(info->sseu.subslice_mask));
 	DRM_DEBUG_DRIVER("EU total: %u\n", info->sseu.eu_total);
 	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->sseu.eu_per_subslice);
 	DRM_DEBUG_DRIVER("has slice power gating: %s\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7cc9ec8..d47e544 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -784,8 +784,7 @@ struct intel_csr {
 
 struct sseu_dev_info {
 	u8 slice_mask;
-	u8 subslice_total;
-	u8 subslice_per_slice;
+	u8 subslice_mask;
 	u8 eu_total;
 	u8 eu_per_subslice;
 	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
@@ -795,6 +794,11 @@ struct sseu_dev_info {
 	u8 has_eu_pg:1;
 };
 
+static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
+{
+	return hweight32((sseu)->slice_mask) * hweight32((sseu)->subslice_mask);
+}
+
 struct intel_device_info {
 	u32 display_mmio_offset;
 	u16 device_id;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4130ff1..158f008 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2209,7 +2209,7 @@ make_rpcs(struct drm_device *dev)
 
 	if (INTEL_INFO(dev)->sseu.has_subslice_pg) {
 		rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
-		rpcs |= INTEL_INFO(dev)->sseu.subslice_per_slice <<
+		rpcs |= hweight32(INTEL_INFO(dev)->sseu.subslice_mask) <<
 			GEN8_RPCS_SS_CNT_SHIFT;
 		rpcs |= GEN8_RPCS_ENABLE;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] drm/i915: sseu: add debug printf for slice/subslice masks
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
                   ` (4 preceding siblings ...)
  2015-10-21 15:40 ` [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-10-21 15:40 ` [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing Imre Deak
  2015-10-27 11:45 ` [PATCH 0/7] drm/i915: read out slice/subslice masks Robert Bragg
  7 siblings, 0 replies; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++
 drivers/gpu/drm/i915/i915_dma.c     | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3fb83ea..24504a3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5067,10 +5067,14 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 {
 	const char *type = is_available_info ? "Available" : "Enabled";
 
+	seq_printf(m, "  %s Slice Mask: %04x\n", type,
+		   sseu->slice_mask);
 	seq_printf(m, "  %s Slice Total: %u\n", type,
 		   hweight32(sseu->slice_mask));
 	seq_printf(m, "  %s Subslice Total: %u\n", type,
 		   sseu_subslice_total(sseu));
+	seq_printf(m, "  %s Subslice Mask: %04x\n", type,
+		   sseu->subslice_mask);
 	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
 		   hweight32(sseu->subslice_mask));
 	seq_printf(m, "  %s EU Total: %u\n", type,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 69a81ae9..2c9a5d5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -820,10 +820,12 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 	else if (INTEL_INFO(dev)->gen >= 9)
 		gen9_sseu_info_init(dev);
 
+	DRM_DEBUG_DRIVER("slice mask: %04x\n", info->sseu.slice_mask);
 	DRM_DEBUG_DRIVER("slice total: %u\n",
 			 hweight32(info->sseu.slice_mask));
 	DRM_DEBUG_DRIVER("subslice total: %u\n",
 			 sseu_subslice_total(&info->sseu));
+	DRM_DEBUG_DRIVER("subslice mask %04x\n", info->sseu.subslice_mask);
 	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
 			 hweight32(info->sseu.subslice_mask));
 	DRM_DEBUG_DRIVER("EU total: %u\n", info->sseu.eu_total);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
                   ` (5 preceding siblings ...)
  2015-10-21 15:40 ` [PATCH 6/7] drm/i915: sseu: add debug printf for slice/subslice masks Imre Deak
@ 2015-10-21 15:40 ` Imre Deak
  2015-11-20  0:08   ` Ben Widawsky
  2015-10-27 11:45 ` [PATCH 0/7] drm/i915: read out slice/subslice masks Robert Bragg
  7 siblings, 1 reply; 17+ messages in thread
From: Imre Deak @ 2015-10-21 15:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Currently when checking for fused off EUs we may ignore the EU count in
an enabled slice if there is any disabled slice preceding the enabled
one (with a lower slice ID). Perhaps this can't happen in reality, but
there is no reason to have this assumption built-in, the code is clearer
without it.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 24504a3..1d43624 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5054,7 +5054,7 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
 				 sseu_subslice_total(stat);
 
 		/* subtract fused off EU(s) from enabled slice(s) */
-		for (s = 0; s < hweight32(stat->slice_mask); s++) {
+		for (s = 0; s < fls(stat->slice_mask); s++) {
 			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
 
 			stat->eu_total -= hweight8(subslice_7eu);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] drm/i915: read out slice/subslice masks
  2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
                   ` (6 preceding siblings ...)
  2015-10-21 15:40 ` [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing Imre Deak
@ 2015-10-27 11:45 ` Robert Bragg
  7 siblings, 0 replies; 17+ messages in thread
From: Robert Bragg @ 2015-10-27 11:45 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, Ben Widawsky

This series looks good to me.

Just to let people know; I have some related changes on my
wip/rib/oa-next branch where I'm defining the subslice mask
differently to this (to reflect the slice mask too)...

In my case for exposing OA unit counters we have a number of metric
sets which have multiple associated MUX configurations and the
appropriate config sometimes depends on which specific subslices are
enabled.

The way this is managed a.t.m is that the XML files maintained by VPG,
that describe the configs, have an RPN 'Availability' expression
associated with each MUX config which is parsed to auto generate
corresponding if() statements to test when trying to select a
particular metric set requested by userspace. These expressions can
refer to a $SubsliceMask variable and in this case the mask is
expected to reflect the slice and subslice mask in one mask, e.g. a
3bit mask per slice packed together on BDW.

Given this, the simplest way for me to work with these availability
checks has been to make the subslice_mask follow the same definition,
something like:

> ss_mask = ss_disable ^ ((1 << ss_max) - 1);
> for (s = 0; s < s_max; s++) {
>         if (s_enable & (0x1 << s))
>                 info->subslice_mask |= ss_mask << (ss_max * s);
> }

Ref: https://github.com/rib/linux/blob/wip/rib/oa-next/drivers/gpu/drm/i915/i915_dma.c#L622

(sorry the patches on my  wip/rib/oa-next branch related to this
aren't currently well contained)

For reference my wip/rib/oa-next branch is also exposing the subslice
mask to userspace via GETPARM, as described above, mainly because the
same kinds of RPN expressions are also tested in userspace to
determine how to interpret some metric sets.

Not that the above should affect landing this series; it just seems
relevant to mention.


Reviewed-by: Robert Bragg <robert@sixbynine.org>

Regards,
- Robert

On Wed, Oct 21, 2015 at 4:40 PM, Imre Deak <imre.deak@intel.com> wrote:
> The per-slice/subslice INSTDONE patchset from Ben [1] will need the
> subslice/slice masks in addition to the corresponding counts that we
> maintain atm. So I added support to store the masks instead of the
> counts and calculate the counts whenever we need them based on the
> masks. While at it I also noticed that the SSEU readout code could be
> simplified by reusing the data structure storing the SSEU properties.
>
> Tested on BXT/SKL.
>
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2015-September/077050.html
>
> Imre Deak (7):
>   drm/i915: sseu: move sseu_dev_status to i915_drv.h
>   drm/i915: sseu: use sseu_dev_info in device info
>   drm/i915: sseu: simplify debugfs status/info printing
>   drm/i915: sseu: convert slice count field to mask
>   drm/i915: sseu: convert subslice count fields to subslice mask
>   drm/i915: sseu: add debug printf for slice/subslice masks
>   drm/i915/bdw: sseu: fix sseu status parsing
>
>  drivers/gpu/drm/i915/i915_debugfs.c     | 134 +++++++++++++++-----------------
>  drivers/gpu/drm/i915/i915_dma.c         | 115 +++++++++++++--------------
>  drivers/gpu/drm/i915/i915_drv.h         |  28 ++++---
>  drivers/gpu/drm/i915/intel_lrc.c        |  14 ++--
>  drivers/gpu/drm/i915/intel_pm.c         |   2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   4 +-
>  6 files changed, 150 insertions(+), 147 deletions(-)
>
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h
  2015-10-21 15:40 ` [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h Imre Deak
@ 2015-11-19 23:10   ` Ben Widawsky
  2015-11-20  0:29     ` Jeff McGee
  0 siblings, 1 reply; 17+ messages in thread
From: Ben Widawsky @ 2015-11-19 23:10 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:31PM +0300, Imre Deak wrote:
> The data in this struct is provided both by getting the
> slice/subslice/eu features available on a given platform and the actual
> runtime state of these same features which depends on the HW's current
> power saving state.
> 
> Atm members of this struct are duplicated in sseu_dev_status and
I> intel_device_info. For clarity and code reuse we can share one struct
> for both of the above purposes. This patch only moves the struct to the
> header file, the next patch will convert users of intel_device_info to
> use this struct too.
> 
> Instead of unsigned int u8 is used now, which is big enough and is used
> anyway in intel_device_info.
> 
> No functional change.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 29 ++++++++++++-----------------
>  drivers/gpu/drm/i915/i915_drv.h     |  8 ++++++++
>  2 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a3b22bd..3dd7076 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4945,16 +4945,8 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
>  			i915_cache_sharing_get, i915_cache_sharing_set,
>  			"%llu\n");
>  
> -struct sseu_dev_status {
> -	unsigned int slice_total;
> -	unsigned int subslice_total;
> -	unsigned int subslice_per_slice;
> -	unsigned int eu_total;
> -	unsigned int eu_per_subslice;
> -};
> -
>  static void cherryview_sseu_device_status(struct drm_device *dev,
> -					  struct sseu_dev_status *stat)
> +					  struct sseu_dev_info *stat)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ss_max = 2;
> @@ -4980,13 +4972,14 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
>  			 ((sig1[ss] & CHV_EU210_PG_ENABLE) ? 0 : 2) +
>  			 ((sig2[ss] & CHV_EU311_PG_ENABLE) ? 0 : 2);
>  		stat->eu_total += eu_cnt;
> -		stat->eu_per_subslice = max(stat->eu_per_subslice, eu_cnt);
> +		stat->eu_per_subslice = max_t(unsigned int,
> +					      stat->eu_per_subslice, eu_cnt);
>  	}
>  	stat->subslice_total = stat->subslice_per_slice;
>  }
>  
>  static void gen9_sseu_device_status(struct drm_device *dev,
> -				    struct sseu_dev_status *stat)
> +				    struct sseu_dev_info *stat)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int s_max = 3, ss_max = 4;
> @@ -5040,18 +5033,20 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
>  					       eu_mask[ss%2]);
>  			stat->eu_total += eu_cnt;
> -			stat->eu_per_subslice = max(stat->eu_per_subslice,
> -						    eu_cnt);
> +			stat->eu_per_subslice = max_t(unsigned int,
> +						      stat->eu_per_subslice,
> +						      eu_cnt);
>  		}
>  
>  		stat->subslice_total += ss_cnt;
> -		stat->subslice_per_slice = max(stat->subslice_per_slice,
> -					       ss_cnt);
> +		stat->subslice_per_slice = max_t(unsigned int,
> +						 stat->subslice_per_slice,
> +						 ss_cnt);
>  	}
>  }
>  
>  static void broadwell_sseu_device_status(struct drm_device *dev,
> -					 struct sseu_dev_status *stat)
> +					 struct sseu_dev_info *stat)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int s;
> @@ -5079,7 +5074,7 @@ 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;
> +	struct sseu_dev_info stat;

If you're going through this rename pain with the type anyway you may as well
s/stat/info.

Also, I never understood what "sseu" was supposed to be short for. The spec
calls these "global attributes" which is admittedly a way too generic name. As a
suggestion based on hindsight, I believe the following would be a bit nicer.
struct slice_attributes {
	u8 slice_count;
	u8 eu_total; /* This is sort of useless since if eu_total isn't trivially
		      * eu_per_subslice * subslice_count * slice_count, then we
		      * need to know exactly which subslice is missing EUs. */
	struct {
		u8 subslices_per_slice;
		u8 eu_count; /* XXX: see above comment */
	} subslice;

	#define subslice.total (subslices_per_slice * slice_count)
}


Just a thought. What you have is fine too though:
Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

>  
>  	if (INTEL_INFO(dev)->gen < 8)
>  		return -ENODEV;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8afda45..73ff01f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -782,6 +782,14 @@ struct intel_csr {
>  #define DEFINE_FLAG(name) u8 name:1
>  #define SEP_SEMICOLON ;
>  
> +struct sseu_dev_info {
> +	u8 slice_total;
> +	u8 subslice_total;
> +	u8 subslice_per_slice;
> +	u8 eu_total;
> +	u8 eu_per_subslice;
> +};
> +
>  struct intel_device_info {
>  	u32 display_mmio_offset;
>  	u16 device_id;


-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info
  2015-10-21 15:40 ` [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info Imre Deak
@ 2015-11-19 23:18   ` Ben Widawsky
  0 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2015-11-19 23:18 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:32PM +0300, Imre Deak wrote:
> Move all slice/subslice/eu related properties to the sseu_dev_info
> struct.
> 
> No functional change.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 25 ++++++++++++------------
>  drivers/gpu/drm/i915/i915_dma.c         | 34 +++++++++++++++++----------------
>  drivers/gpu/drm/i915/i915_drv.h         | 16 ++++++----------
>  drivers/gpu/drm/i915/intel_lrc.c        | 14 +++++++-------
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
>  6 files changed, 47 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3dd7076..de188d0 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5017,7 +5017,7 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  		stat->slice_total++;
>  
>  		if (IS_SKYLAKE(dev))
> -			ss_cnt = INTEL_INFO(dev)->subslice_per_slice;
> +			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
>  
>  		for (ss = 0; ss < ss_max; ss++) {
>  			unsigned int eu_cnt;
> @@ -5055,15 +5055,16 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
>  	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_per_slice =
> +				INTEL_INFO(dev)->sseu.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_per_subslice = INTEL_INFO(dev)->sseu.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];
> +			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
>  
>  			stat->eu_total -= hweight8(subslice_7eu);
>  		}
> @@ -5081,21 +5082,21 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>  
>  	seq_puts(m, "SSEU Device Info\n");
>  	seq_printf(m, "  Available Slice Total: %u\n",
> -		   INTEL_INFO(dev)->slice_total);
> +		   INTEL_INFO(dev)->sseu.slice_total);
>  	seq_printf(m, "  Available Subslice Total: %u\n",
> -		   INTEL_INFO(dev)->subslice_total);
> +		   INTEL_INFO(dev)->sseu.subslice_total);
>  	seq_printf(m, "  Available Subslice Per Slice: %u\n",
> -		   INTEL_INFO(dev)->subslice_per_slice);
> +		   INTEL_INFO(dev)->sseu.subslice_per_slice);
>  	seq_printf(m, "  Available EU Total: %u\n",
> -		   INTEL_INFO(dev)->eu_total);
> +		   INTEL_INFO(dev)->sseu.eu_total);
>  	seq_printf(m, "  Available EU Per Subslice: %u\n",
> -		   INTEL_INFO(dev)->eu_per_subslice);
> +		   INTEL_INFO(dev)->sseu.eu_per_subslice);
>  	seq_printf(m, "  Has Slice Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->has_slice_pg));
> +		   yesno(INTEL_INFO(dev)->sseu.has_slice_pg));
>  	seq_printf(m, "  Has Subslice Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->has_subslice_pg));
> +		   yesno(INTEL_INFO(dev)->sseu.has_subslice_pg));
>  	seq_printf(m, "  Has EU Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->has_eu_pg));
> +		   yesno(INTEL_INFO(dev)->sseu.has_eu_pg));
>  
>  	seq_puts(m, "SSEU Device Status\n");
>  	memset(&stat, 0, sizeof(stat));
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 2336af9..be8e141 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -154,12 +154,12 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  		value = 1;
>  		break;
>  	case I915_PARAM_SUBSLICE_TOTAL:
> -		value = INTEL_INFO(dev)->subslice_total;
> +		value = INTEL_INFO(dev)->sseu.subslice_total;
>  		if (!value)
>  			return -ENODEV;
>  		break;
>  	case I915_PARAM_EU_TOTAL:
> -		value = INTEL_INFO(dev)->eu_total;
> +		value = INTEL_INFO(dev)->sseu.eu_total;
>  		if (!value)
>  			return -ENODEV;
>  		break;
> @@ -553,10 +553,10 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>  static void cherryview_sseu_info_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_device_info *info;
> +	struct sseu_dev_info *info;
>  	u32 fuse, eu_dis;
>  
> -	info = (struct intel_device_info *)&dev_priv->info;
> +	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
>  	fuse = I915_READ(CHV_FUSE_GT);
>  
>  	info->slice_total = 1;
> @@ -596,13 +596,13 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
>  static void gen9_sseu_info_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_device_info *info;
> +	struct sseu_dev_info *info;
>  	int s_max = 3, ss_max = 4, eu_max = 8;
>  	int s, ss;
>  	u32 fuse2, s_enable, ss_disable, eu_disable;
>  	u8 eu_mask = 0xff;
>  
> -	info = (struct intel_device_info *)&dev_priv->info;
> +	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
>  	fuse2 = I915_READ(GEN8_FUSE2);
>  	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
>  		   GEN8_F2_S_ENA_SHIFT;
> @@ -676,7 +676,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  static void broadwell_sseu_info_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_device_info *info;
> +	struct sseu_dev_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;
> @@ -694,7 +694,8 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  			 (32 - GEN8_EU_DIS1_S2_SHIFT));
>  
>  
> -	info = (struct intel_device_info *)&dev_priv->info;
> +	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
> +
>  	info->slice_total = hweight32(s_enable);
>  
>  	/*
> @@ -824,17 +825,18 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  	else if (INTEL_INFO(dev)->gen >= 9)
>  		gen9_sseu_info_init(dev);
>  
> -	DRM_DEBUG_DRIVER("slice total: %u\n", info->slice_total);
> -	DRM_DEBUG_DRIVER("subslice total: %u\n", info->subslice_total);
> -	DRM_DEBUG_DRIVER("subslice per slice: %u\n", info->subslice_per_slice);
> -	DRM_DEBUG_DRIVER("EU total: %u\n", info->eu_total);
> -	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->eu_per_subslice);
> +	DRM_DEBUG_DRIVER("slice total: %u\n", info->sseu.slice_total);
> +	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
> +	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
> +						info->sseu.subslice_per_slice);
> +	DRM_DEBUG_DRIVER("EU total: %u\n", info->sseu.eu_total);
> +	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->sseu.eu_per_subslice);
>  	DRM_DEBUG_DRIVER("has slice power gating: %s\n",
> -			 info->has_slice_pg ? "y" : "n");
> +			 info->sseu.has_slice_pg ? "y" : "n");
>  	DRM_DEBUG_DRIVER("has subslice power gating: %s\n",
> -			 info->has_subslice_pg ? "y" : "n");
> +			 info->sseu.has_subslice_pg ? "y" : "n");
>  	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
> -			 info->has_eu_pg ? "y" : "n");
> +			 info->sseu.has_eu_pg ? "y" : "n");
>  }
>  
>  static void intel_init_dpio(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 73ff01f..cb0427d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -788,6 +788,11 @@ struct sseu_dev_info {
>  	u8 subslice_per_slice;
>  	u8 eu_total;
>  	u8 eu_per_subslice;
> +	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
> +	u8 subslice_7eu[3];
> +	u8 has_slice_pg:1;
> +	u8 has_subslice_pg:1;
> +	u8 has_eu_pg:1;
>  };
>  
>  struct intel_device_info {
> @@ -805,16 +810,7 @@ struct intel_device_info {
>  	int cursor_offsets[I915_MAX_PIPES];
>  
>  	/* Slice/subslice/EU info */
> -	u8 slice_total;
> -	u8 subslice_total;
> -	u8 subslice_per_slice;
> -	u8 eu_total;
> -	u8 eu_per_subslice;
> -	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
> -	u8 subslice_7eu[3];
> -	u8 has_slice_pg:1;
> -	u8 has_subslice_pg:1;
> -	u8 has_eu_pg:1;
> +	struct sseu_dev_info sseu;
>  };
>  
>  #undef DEFINE_FLAG
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 88e12bd..8a55f8a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2200,24 +2200,24 @@ make_rpcs(struct drm_device *dev)
>  	 * must make an explicit request through RPCS for full
>  	 * enablement.
>  	*/
> -	if (INTEL_INFO(dev)->has_slice_pg) {
> +	if (INTEL_INFO(dev)->sseu.has_slice_pg) {
>  		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
> -		rpcs |= INTEL_INFO(dev)->slice_total <<
> +		rpcs |= INTEL_INFO(dev)->sseu.slice_total <<
>  			GEN8_RPCS_S_CNT_SHIFT;
>  		rpcs |= GEN8_RPCS_ENABLE;
>  	}
>  
> -	if (INTEL_INFO(dev)->has_subslice_pg) {
> +	if (INTEL_INFO(dev)->sseu.has_subslice_pg) {
>  		rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
> -		rpcs |= INTEL_INFO(dev)->subslice_per_slice <<
> +		rpcs |= INTEL_INFO(dev)->sseu.subslice_per_slice <<
>  			GEN8_RPCS_SS_CNT_SHIFT;
>  		rpcs |= GEN8_RPCS_ENABLE;
>  	}
>  
> -	if (INTEL_INFO(dev)->has_eu_pg) {
> -		rpcs |= INTEL_INFO(dev)->eu_per_subslice <<
> +	if (INTEL_INFO(dev)->sseu.has_eu_pg) {
> +		rpcs |= INTEL_INFO(dev)->sseu.eu_per_subslice <<
>  			GEN8_RPCS_EU_MIN_SHIFT;
> -		rpcs |= INTEL_INFO(dev)->eu_per_subslice <<
> +		rpcs |= INTEL_INFO(dev)->sseu.eu_per_subslice <<
>  			GEN8_RPCS_EU_MAX_SHIFT;
>  		rpcs |= GEN8_RPCS_ENABLE;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index df22b9c..dad0a67 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5101,7 +5101,7 @@ static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv)
>  
>  	val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE);
>  
> -	switch (INTEL_INFO(dev)->eu_total) {
> +	switch (INTEL_INFO(dev)->sseu.eu_total) {
>  	case 8:
>  		/* (2 * 4) config */
>  		rp0 = (val >> FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index a492705..57a756d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1000,7 +1000,7 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring)
>  		 * Only consider slices where one, and only one, subslice has 7
>  		 * EUs
>  		 */
> -		if (hweight8(dev_priv->info.subslice_7eu[i]) != 1)
> +		if (hweight8(INTEL_INFO(dev_priv)->sseu.subslice_7eu[i]) != 1)
>  			continue;
>  
>  		/*
> @@ -1009,7 +1009,7 @@ static int skl_tune_iz_hashing(struct intel_engine_cs *ring)
>  		 *
>  		 * ->    0 <= ss <= 3;
>  		 */
> -		ss = ffs(dev_priv->info.subslice_7eu[i]) - 1;
> +		ss = ffs(INTEL_INFO(dev_priv)->sseu.subslice_7eu[i]) - 1;
>  		vals[i] = 3 - ss;
>  	}
>  
> -- 
> 2.1.4
> 

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing
  2015-10-21 15:40 ` [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing Imre Deak
@ 2015-11-19 23:24   ` Ben Widawsky
  0 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2015-11-19 23:24 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:33PM +0300, Imre Deak wrote:
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 55 +++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index de188d0..183c1f2 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5071,6 +5071,33 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
>  	}
>  }
>  
> +static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
> +				 const struct sseu_dev_info *sseu)
> +{
> +	const char *type = is_available_info ? "Available" : "Enabled";
> +
> +	seq_printf(m, "  %s Slice Total: %u\n", type,
> +		   sseu->slice_total);
> +	seq_printf(m, "  %s Subslice Total: %u\n", type,
> +		   sseu->subslice_total);
> +	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
> +		   sseu->subslice_per_slice);
> +	seq_printf(m, "  %s EU Total: %u\n", type,
> +		   sseu->eu_total);
> +	seq_printf(m, "  %s EU Per Subslice: %u\n", type,
> +		   sseu->eu_per_subslice);
> +
> +	if (!is_available_info)
> +		return;
> +
> +	seq_printf(m, "  Has Slice Power Gating: %s\n",
> +		   yesno(sseu->has_slice_pg));
> +	seq_printf(m, "  Has Subslice Power Gating: %s\n",
> +		   yesno(sseu->has_subslice_pg));
> +	seq_printf(m, "  Has EU Power Gating: %s\n",
> +		   yesno(sseu->has_eu_pg));
> +}
> +
>  static int i915_sseu_status(struct seq_file *m, void *unused)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
> @@ -5081,22 +5108,7 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>  		return -ENODEV;
>  
>  	seq_puts(m, "SSEU Device Info\n");
> -	seq_printf(m, "  Available Slice Total: %u\n",
> -		   INTEL_INFO(dev)->sseu.slice_total);
> -	seq_printf(m, "  Available Subslice Total: %u\n",
> -		   INTEL_INFO(dev)->sseu.subslice_total);
> -	seq_printf(m, "  Available Subslice Per Slice: %u\n",
> -		   INTEL_INFO(dev)->sseu.subslice_per_slice);
> -	seq_printf(m, "  Available EU Total: %u\n",
> -		   INTEL_INFO(dev)->sseu.eu_total);
> -	seq_printf(m, "  Available EU Per Subslice: %u\n",
> -		   INTEL_INFO(dev)->sseu.eu_per_subslice);
> -	seq_printf(m, "  Has Slice Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->sseu.has_slice_pg));
> -	seq_printf(m, "  Has Subslice Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->sseu.has_subslice_pg));
> -	seq_printf(m, "  Has EU Power Gating: %s\n",
> -		   yesno(INTEL_INFO(dev)->sseu.has_eu_pg));
> +	i915_print_sseu_info(m, true, &INTEL_INFO(dev)->sseu);
>  
>  	seq_puts(m, "SSEU Device Status\n");
>  	memset(&stat, 0, sizeof(stat));
> @@ -5107,16 +5119,7 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>  	} else if (INTEL_INFO(dev)->gen >= 9) {
>  		gen9_sseu_device_status(dev, &stat);
>  	}
> -	seq_printf(m, "  Enabled Slice Total: %u\n",
> -		   stat.slice_total);
> -	seq_printf(m, "  Enabled Subslice Total: %u\n",
> -		   stat.subslice_total);
> -	seq_printf(m, "  Enabled Subslice Per Slice: %u\n",
> -		   stat.subslice_per_slice);
> -	seq_printf(m, "  Enabled EU Total: %u\n",
> -		   stat.eu_total);
> -	seq_printf(m, "  Enabled EU Per Subslice: %u\n",
> -		   stat.eu_per_subslice);
> +	i915_print_sseu_info(m, false, &stat);
>  
>  	return 0;
>  }
> -- 
> 2.1.4
> 

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/7] drm/i915: sseu: convert slice count field to mask
  2015-10-21 15:40 ` [PATCH 4/7] drm/i915: sseu: convert slice count field to mask Imre Deak
@ 2015-11-19 23:40   ` Ben Widawsky
  2015-11-20 13:00     ` Ville Syrjälä
  0 siblings, 1 reply; 17+ messages in thread
From: Ben Widawsky @ 2015-11-19 23:40 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:34PM +0300, Imre Deak wrote:
> In an upcoming patch we'll need the actual mask of slices in addition to
> their count, so replace the count field with a mask.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++-------
>  drivers/gpu/drm/i915/i915_dma.c     | 37 +++++++++++++++++++------------------
>  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
>  4 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 183c1f2..390dc59 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4965,7 +4965,7 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
>  			/* skip disabled subslice */
>  			continue;
>  
> -		stat->slice_total = 1;
> +		stat->slice_mask = BIT(0);
>  		stat->subslice_per_slice++;
>  		eu_cnt = ((sig1[ss] & CHV_EU08_PG_ENABLE) ? 0 : 2) +
>  			 ((sig1[ss] & CHV_EU19_PG_ENABLE) ? 0 : 2) +
> @@ -5014,7 +5014,7 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  			/* skip disabled slice */
>  			continue;
>  
> -		stat->slice_total++;
> +		stat->slice_mask |= BIT(s);
>  
>  		if (IS_SKYLAKE(dev))
>  			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
> @@ -5052,18 +5052,18 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
>  	int s;
>  	u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
>  
> -	stat->slice_total = hweight32(slice_info & GEN8_LSLICESTAT_MASK);
> +	stat->slice_mask = slice_info & GEN8_LSLICESTAT_MASK;
>  
> -	if (stat->slice_total) {
> +	if (stat->slice_mask) {
>  		stat->subslice_per_slice =
>  				INTEL_INFO(dev)->sseu.subslice_per_slice;
> -		stat->subslice_total = stat->slice_total *
> +		stat->subslice_total = hweight32(stat->slice_mask) *
					^ hweight8?

>  				       stat->subslice_per_slice;
>  		stat->eu_per_subslice = INTEL_INFO(dev)->sseu.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++) {
> +		for (s = 0; s < hweight32(stat->slice_mask); s++) {
				^ hweight8?

how about for_each_set_bit(s, stat->slice_mask, 8) {} ?

>  			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
>  
>  			stat->eu_total -= hweight8(subslice_7eu);
> @@ -5077,7 +5077,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
>  	const char *type = is_available_info ? "Available" : "Enabled";
>  
>  	seq_printf(m, "  %s Slice Total: %u\n", type,
> -		   sseu->slice_total);
> +		   hweight32(sseu->slice_mask));
			^ hweight8?

>  	seq_printf(m, "  %s Subslice Total: %u\n", type,
>  		   sseu->subslice_total);
>  	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index be8e141..1f5f3a7d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -559,7 +559,7 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
>  	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
>  	fuse = I915_READ(CHV_FUSE_GT);
>  
> -	info->slice_total = 1;
> +	info->slice_mask = BIT(0);
>  
>  	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
>  		info->subslice_per_slice++;
> @@ -599,23 +599,21 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	struct sseu_dev_info *info;
>  	int s_max = 3, ss_max = 4, eu_max = 8;
>  	int s, ss;
> -	u32 fuse2, s_enable, ss_disable, eu_disable;
> +	u32 fuse2, ss_disable, eu_disable;
>  	u8 eu_mask = 0xff;
>  
>  	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
>  	fuse2 = I915_READ(GEN8_FUSE2);
> -	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >>
> -		   GEN8_F2_S_ENA_SHIFT;
> +	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
>  	ss_disable = (fuse2 & GEN9_F2_SS_DIS_MASK) >>
>  		     GEN9_F2_SS_DIS_SHIFT;
>  
> -	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_total = hweight32(info->slice_mask) *
				^ hweight8?
>  			       info->subslice_per_slice;
>  
>  	/*
> @@ -623,7 +621,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	 * count the total enabled EU.
>  	*/
>  	for (s = 0; s < s_max; s++) {
> -		if (!(s_enable & (0x1 << s)))
> +		if (!(info->slice_mask & BIT(s)))

You could do for_each_set_bit again here too if you like.

>  			/* skip disabled slice */
>  			continue;
>  
> @@ -668,7 +666,8 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	 * supports EU power gating on devices with more than one EU
>  	 * pair per subslice.
>  	*/
> -	info->has_slice_pg = (IS_SKYLAKE(dev) && (info->slice_total > 1));
> +	info->has_slice_pg = IS_SKYLAKE(dev) &&
> +			     hweight32(info->slice_mask) > 1;
				^ hweight8?

>  	info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total > 1));
>  	info->has_eu_pg = (info->eu_per_subslice > 2);
>  }
> @@ -679,10 +678,14 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  	struct sseu_dev_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;
> +	u32 fuse2, eu_disable[s_max], ss_disable;
> +
> +	info = (struct sseu_dev_info *)&dev_priv->info.sseu;
>  
>  	fuse2 = I915_READ(GEN8_FUSE2);
> -	s_enable = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
> +
> +	info->slice_mask = (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;
> @@ -694,23 +697,20 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  			 (32 - GEN8_EU_DIS1_S2_SHIFT));
>  
>  
> -	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
> -
> -	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;
> +	info->subslice_total = hweight32(info->slice_mask) *
				^ hweight8?
> +			       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)))
> +		if (!(info->slice_mask & BIT(s)))

(same about for_each set_bit)

>  			/* skip disabled slice */
>  			continue;
>  
> @@ -745,7 +745,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  	 * BDW supports slice power gating on devices with more than
>  	 * one slice.
>  	 */
> -	info->has_slice_pg = (info->slice_total > 1);
> +	info->has_slice_pg = hweight32(info->slice_mask) > 1;
				^ hweight8?
>  	info->has_subslice_pg = 0;
>  	info->has_eu_pg = 0;
>  }
> @@ -825,7 +825,8 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  	else if (INTEL_INFO(dev)->gen >= 9)
>  		gen9_sseu_info_init(dev);
>  
> -	DRM_DEBUG_DRIVER("slice total: %u\n", info->sseu.slice_total);
> +	DRM_DEBUG_DRIVER("slice total: %u\n",
> +			 hweight32(info->sseu.slice_mask));
				^ hweight8?
>  	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
>  	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
>  						info->sseu.subslice_per_slice);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cb0427d..7cc9ec8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -783,7 +783,7 @@ struct intel_csr {
>  #define SEP_SEMICOLON ;
>  
>  struct sseu_dev_info {
> -	u8 slice_total;
> +	u8 slice_mask;
>  	u8 subslice_total;
>  	u8 subslice_per_slice;
>  	u8 eu_total;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 8a55f8a..4130ff1 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2202,7 +2202,7 @@ make_rpcs(struct drm_device *dev)
>  	*/
>  	if (INTEL_INFO(dev)->sseu.has_slice_pg) {
>  		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
> -		rpcs |= INTEL_INFO(dev)->sseu.slice_total <<
> +		rpcs |= hweight32(INTEL_INFO(dev)->sseu.slice_mask) <<
			^ hweight8?

>  			GEN8_RPCS_S_CNT_SHIFT;
>  		rpcs |= GEN8_RPCS_ENABLE;
>  	}

I'm not positive if hweight32 is actually okay on an 8bit type. I remember Ville
correcting me once on this, but I can't remember it's correct. Assuming
hweight32 is fine to use, with or without my recommendations, this is:
Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask
  2015-10-21 15:40 ` [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask Imre Deak
@ 2015-11-20  0:07   ` Ben Widawsky
  0 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2015-11-20  0:07 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:35PM +0300, Imre Deak wrote:
> In an upcoming patch we'll need the actual mask of subslices in addition
> to their count, so convert the subslice_per_slice field to a mask.
> Also we can easily calculate subslice_total from the other fields, so
> instead of storing a cached version of this, add a helper to calculate
> it.

Oh good, I think I asked for this in patch 1.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 37 ++++++++-------------
>  drivers/gpu/drm/i915/i915_dma.c     | 64 +++++++++++++++++--------------------
>  drivers/gpu/drm/i915/i915_drv.h     |  8 +++--
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
>  4 files changed, 51 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 390dc59..3fb83ea 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4966,7 +4966,7 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
>  			continue;
>  
>  		stat->slice_mask = BIT(0);
> -		stat->subslice_per_slice++;
> +		stat->subslice_mask |= BIT(ss);
>  		eu_cnt = ((sig1[ss] & CHV_EU08_PG_ENABLE) ? 0 : 2) +
>  			 ((sig1[ss] & CHV_EU19_PG_ENABLE) ? 0 : 2) +
>  			 ((sig1[ss] & CHV_EU210_PG_ENABLE) ? 0 : 2) +
> @@ -4975,7 +4975,6 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
>  		stat->eu_per_subslice = max_t(unsigned int,
>  					      stat->eu_per_subslice, eu_cnt);
>  	}
> -	stat->subslice_total = stat->subslice_per_slice;
>  }
>  
>  static void gen9_sseu_device_status(struct drm_device *dev,
> @@ -5008,8 +5007,6 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  		     GEN9_PGCTL_SSB_EU311_ACK;
>  
>  	for (s = 0; s < s_max; s++) {
> -		unsigned int ss_cnt = 0;
> -
>  		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>  			/* skip disabled slice */
>  			continue;
> @@ -5017,18 +5014,19 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  		stat->slice_mask |= BIT(s);
>  
>  		if (IS_SKYLAKE(dev))
> -			ss_cnt = INTEL_INFO(dev)->sseu.subslice_per_slice;
> +			stat->subslice_mask =
> +				INTEL_INFO(dev_priv)->sseu.subslice_mask;
>  
>  		for (ss = 0; ss < ss_max; ss++) {
>  			unsigned int eu_cnt;
>  
> -			if (IS_BROXTON(dev) &&
> -			    !(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
> -				/* skip disabled subslice */
> -				continue;
> +			if (IS_BROXTON(dev)) {
> +				if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
> +					/* skip disabled subslice */
> +					continue;
>  
> -			if (IS_BROXTON(dev))
> -				ss_cnt++;
> +				stat->subslice_mask |= BIT(ss);
> +			}
>  
>  			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
>  					       eu_mask[ss%2]);
> @@ -5037,11 +5035,6 @@ static void gen9_sseu_device_status(struct drm_device *dev,
>  						      stat->eu_per_subslice,
>  						      eu_cnt);
>  		}
> -
> -		stat->subslice_total += ss_cnt;
> -		stat->subslice_per_slice = max_t(unsigned int,
> -						 stat->subslice_per_slice,
> -						 ss_cnt);
>  	}
>  }
>  
> @@ -5055,12 +5048,10 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
>  	stat->slice_mask = slice_info & GEN8_LSLICESTAT_MASK;
>  
>  	if (stat->slice_mask) {
> -		stat->subslice_per_slice =
> -				INTEL_INFO(dev)->sseu.subslice_per_slice;
> -		stat->subslice_total = hweight32(stat->slice_mask) *
> -				       stat->subslice_per_slice;
> +		stat->subslice_mask = INTEL_INFO(dev)->sseu.subslice_mask;
>  		stat->eu_per_subslice = INTEL_INFO(dev)->sseu.eu_per_subslice;
> -		stat->eu_total = stat->eu_per_subslice * stat->subslice_total;
> +		stat->eu_total = stat->eu_per_subslice *
> +				 sseu_subslice_total(stat);
>  
>  		/* subtract fused off EU(s) from enabled slice(s) */
>  		for (s = 0; s < hweight32(stat->slice_mask); s++) {
> @@ -5079,9 +5070,9 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
>  	seq_printf(m, "  %s Slice Total: %u\n", type,
>  		   hweight32(sseu->slice_mask));
>  	seq_printf(m, "  %s Subslice Total: %u\n", type,
> -		   sseu->subslice_total);
> +		   sseu_subslice_total(sseu));
>  	seq_printf(m, "  %s Subslice Per Slice: %u\n", type,
> -		   sseu->subslice_per_slice);
> +		   hweight32(sseu->subslice_mask));
>  	seq_printf(m, "  %s EU Total: %u\n", type,
>  		   sseu->eu_total);
>  	seq_printf(m, "  %s EU Per Subslice: %u\n", type,
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 1f5f3a7d..69a81ae9 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -154,7 +154,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  		value = 1;
>  		break;
>  	case I915_PARAM_SUBSLICE_TOTAL:
> -		value = INTEL_INFO(dev)->sseu.subslice_total;
> +		value = sseu_subslice_total(&INTEL_INFO(dev)->sseu);
>  		if (!value)
>  			return -ENODEV;
>  		break;
> @@ -562,26 +562,25 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
>  	info->slice_mask = BIT(0);
>  
>  	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
> -		info->subslice_per_slice++;
> +		info->subslice_mask |= BIT(0);
>  		eu_dis = fuse & (CHV_FGT_EU_DIS_SS0_R0_MASK |
>  				 CHV_FGT_EU_DIS_SS0_R1_MASK);
>  		info->eu_total += 8 - hweight32(eu_dis);
>  	}
>  
>  	if (!(fuse & CHV_FGT_DISABLE_SS1)) {
> -		info->subslice_per_slice++;
> +		info->subslice_mask |= BIT(1);
>  		eu_dis = fuse & (CHV_FGT_EU_DIS_SS1_R0_MASK |
>  				 CHV_FGT_EU_DIS_SS1_R1_MASK);
>  		info->eu_total += 8 - hweight32(eu_dis);
>  	}
>  
> -	info->subslice_total = info->subslice_per_slice;
>  	/*
>  	 * CHV expected to always have a uniform distribution of EU
>  	 * across subslices.
>  	*/
> -	info->eu_per_subslice = info->subslice_total ?
> -				info->eu_total / info->subslice_total :
> +	info->eu_per_subslice = sseu_subslice_total(info) ?
> +				info->eu_total / sseu_subslice_total(info) :
>  				0;
>  	/*
>  	 * CHV supports subslice power gating on devices with more than
> @@ -589,7 +588,7 @@ static void cherryview_sseu_info_init(struct drm_device *dev)
>  	 * more than one EU pair per subslice.
>  	*/
>  	info->has_slice_pg = 0;
> -	info->has_subslice_pg = (info->subslice_total > 1);
> +	info->has_subslice_pg = sseu_subslice_total(info) > 1;
>  	info->has_eu_pg = (info->eu_per_subslice > 2);
>  }
>  
> @@ -599,22 +598,19 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	struct sseu_dev_info *info;
>  	int s_max = 3, ss_max = 4, eu_max = 8;
>  	int s, ss;
> -	u32 fuse2, ss_disable, eu_disable;
> +	u32 fuse2, eu_disable;
>  	u8 eu_mask = 0xff;
>  
>  	info = (struct sseu_dev_info *)&INTEL_INFO(dev_priv)->sseu;
>  	fuse2 = I915_READ(GEN8_FUSE2);
>  	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
> -	ss_disable = (fuse2 & GEN9_F2_SS_DIS_MASK) >>
> -		     GEN9_F2_SS_DIS_SHIFT;
> -
>  	/*
>  	 * 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 = hweight32(info->slice_mask) *
> -			       info->subslice_per_slice;
> +	info->subslice_mask = (1 << ss_max) - 1;
> +	info->subslice_mask &= ~((fuse2 & GEN9_F2_SS_DIS_MASK) >>
> +				GEN9_F2_SS_DIS_SHIFT);
>  
>  	/*
>  	 * Iterate through enabled slices and subslices to
> @@ -629,7 +625,7 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  		for (ss = 0; ss < ss_max; ss++) {
>  			int eu_per_ss;
>  
> -			if (ss_disable & (0x1 << ss))
> +			if (!(info->subslice_mask & BIT(ss)))
>  				/* skip disabled subslice */
>  				continue;
>  
> @@ -655,9 +651,9 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	 * recovery. BXT is expected to be perfectly uniform in EU
>  	 * distribution.
>  	*/
> -	info->eu_per_subslice = info->subslice_total ?
> +	info->eu_per_subslice = sseu_subslice_total(info) ?
>  				DIV_ROUND_UP(info->eu_total,
> -					     info->subslice_total) : 0;
> +					     sseu_subslice_total(info)) : 0;
>  	/*
>  	 * SKL supports slice power gating on devices with more than
>  	 * one slice, and supports EU power gating on devices with
> @@ -668,7 +664,8 @@ static void gen9_sseu_info_init(struct drm_device *dev)
>  	*/
>  	info->has_slice_pg = IS_SKYLAKE(dev) &&
>  			     hweight32(info->slice_mask) > 1;
> -	info->has_subslice_pg = (IS_BROXTON(dev) && (info->subslice_total > 1));
> +	info->has_subslice_pg = IS_BROXTON(dev) &&
> +				sseu_subslice_total(info) > 1;
>  	info->has_eu_pg = (info->eu_per_subslice > 2);
>  }
>  
> @@ -678,7 +675,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  	struct sseu_dev_info *info;
>  	const int s_max = 3, ss_max = 3, eu_max = 8;
>  	int s, ss;
> -	u32 fuse2, eu_disable[s_max], ss_disable;
> +	u32 fuse2, eu_disable[s_max];
>  
>  	info = (struct sseu_dev_info *)&dev_priv->info.sseu;
>  
> @@ -686,7 +683,13 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  
>  	info->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >>
>  				GEN8_F2_S_ENA_SHIFT;
> -	ss_disable = (fuse2 & GEN8_F2_SS_DIS_MASK) >> GEN8_F2_SS_DIS_SHIFT;
> +	/*
> +	 * The subslice disable field is global, i.e. it applies
> +	 * to each of the enabled slices.
> +	 */
> +	info->subslice_mask = (1 << ss_max) - 1;
> +	info->subslice_mask &= ~((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) |
> @@ -696,15 +699,6 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  			((I915_READ(GEN8_EU_DISABLE2) & GEN8_EU_DIS2_S2_MASK) <<
>  			 (32 - GEN8_EU_DIS1_S2_SHIFT));
>  
> -
> -	/*
> -	 * 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 = hweight32(info->slice_mask) *
> -			       info->subslice_per_slice;
> -
>  	/*
>  	 * Iterate through enabled slices and subslices to
>  	 * count the total enabled EU.
> @@ -717,7 +711,7 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  		for (ss = 0; ss < ss_max; ss++) {
>  			u32 n_disabled;
>  
> -			if (ss_disable & (0x1 << ss))
> +			if (!(info->subslice_mask & BIT(ss)))
>  				/* skip disabled subslice */
>  				continue;
>  
> @@ -738,8 +732,9 @@ static void broadwell_sseu_info_init(struct drm_device *dev)
>  	 * 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;
> +	info->eu_per_subslice = sseu_subslice_total(info) ?
> +				DIV_ROUND_UP(info->eu_total,
> +					     sseu_subslice_total(info)) : 0;
>  
>  	/*
>  	 * BDW supports slice power gating on devices with more than
> @@ -827,9 +822,10 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  
>  	DRM_DEBUG_DRIVER("slice total: %u\n",
>  			 hweight32(info->sseu.slice_mask));
> -	DRM_DEBUG_DRIVER("subslice total: %u\n", info->sseu.subslice_total);
> +	DRM_DEBUG_DRIVER("subslice total: %u\n",
> +			 sseu_subslice_total(&info->sseu));
>  	DRM_DEBUG_DRIVER("subslice per slice: %u\n",
> -						info->sseu.subslice_per_slice);
> +			 hweight32(info->sseu.subslice_mask));
>  	DRM_DEBUG_DRIVER("EU total: %u\n", info->sseu.eu_total);
>  	DRM_DEBUG_DRIVER("EU per subslice: %u\n", info->sseu.eu_per_subslice);
>  	DRM_DEBUG_DRIVER("has slice power gating: %s\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7cc9ec8..d47e544 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -784,8 +784,7 @@ struct intel_csr {
>  
>  struct sseu_dev_info {
>  	u8 slice_mask;
> -	u8 subslice_total;
> -	u8 subslice_per_slice;
> +	u8 subslice_mask;

I know we have situations for GT1 parts where the number of subslices per slice
is less than that of the same GEN of a different SKU. AFAIK, this never carries
over into higher GT (ie. GT2 would always have 3 subslices per slice, but GT1
may have 2 subslices per slice). However. I am not certain this is the case - I
hope you've double checked that.

>  	u8 eu_total;
>  	u8 eu_per_subslice;
>  	/* For each slice, which subslice(s) has(have) 7 EUs (bitfield)? */
> @@ -795,6 +794,11 @@ struct sseu_dev_info {
>  	u8 has_eu_pg:1;
>  };
>  
> +static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
> +{
> +	return hweight32((sseu)->slice_mask) * hweight32((sseu)->subslice_mask);

hweight8

basically s/hweight32/hweight8 on the whole file

> +}
> +
>  struct intel_device_info {
>  	u32 display_mmio_offset;
>  	u16 device_id;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4130ff1..158f008 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2209,7 +2209,7 @@ make_rpcs(struct drm_device *dev)
>  
>  	if (INTEL_INFO(dev)->sseu.has_subslice_pg) {
>  		rpcs |= GEN8_RPCS_SS_CNT_ENABLE;
> -		rpcs |= INTEL_INFO(dev)->sseu.subslice_per_slice <<
> +		rpcs |= hweight32(INTEL_INFO(dev)->sseu.subslice_mask) <<


>  			GEN8_RPCS_SS_CNT_SHIFT;
>  		rpcs |= GEN8_RPCS_ENABLE;
>  	}

Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing
  2015-10-21 15:40 ` [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing Imre Deak
@ 2015-11-20  0:08   ` Ben Widawsky
  0 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2015-11-20  0:08 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Wed, Oct 21, 2015 at 06:40:37PM +0300, Imre Deak wrote:
> Currently when checking for fused off EUs we may ignore the EU count in
> an enabled slice if there is any disabled slice preceding the enabled
> one (with a lower slice ID). Perhaps this can't happen in reality, but
> there is no reason to have this assumption built-in, the code is clearer
> without it.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 24504a3..1d43624 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5054,7 +5054,7 @@ static void broadwell_sseu_device_status(struct drm_device *dev,
>  				 sseu_subslice_total(stat);
>  
>  		/* subtract fused off EU(s) from enabled slice(s) */
> -		for (s = 0; s < hweight32(stat->slice_mask); s++) {
> +		for (s = 0; s < fls(stat->slice_mask); s++) {

Could use for_each_set_bit here too.
>  			u8 subslice_7eu = INTEL_INFO(dev)->sseu.subslice_7eu[s];
>  
>  			stat->eu_total -= hweight8(subslice_7eu);

6 & 7 are:
Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

1-7 are also:
Tested-by: Ben Widawsky <benjamin.widawsky@intel.com>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h
  2015-11-19 23:10   ` Ben Widawsky
@ 2015-11-20  0:29     ` Jeff McGee
  0 siblings, 0 replies; 17+ messages in thread
From: Jeff McGee @ 2015-11-20  0:29 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Thu, Nov 19, 2015 at 03:10:14PM -0800, Ben Widawsky wrote:
> On Wed, Oct 21, 2015 at 06:40:31PM +0300, Imre Deak wrote:
> > The data in this struct is provided both by getting the
> > slice/subslice/eu features available on a given platform and the actual
> > runtime state of these same features which depends on the HW's current
> > power saving state.
> > 
> > Atm members of this struct are duplicated in sseu_dev_status and
> I> intel_device_info. For clarity and code reuse we can share one struct
> > for both of the above purposes. This patch only moves the struct to the
> > header file, the next patch will convert users of intel_device_info to
> > use this struct too.
> > 
> > Instead of unsigned int u8 is used now, which is big enough and is used
> > anyway in intel_device_info.
> > 
> > No functional change.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 29 ++++++++++++-----------------
> >  drivers/gpu/drm/i915/i915_drv.h     |  8 ++++++++
> >  2 files changed, 20 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index a3b22bd..3dd7076 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4945,16 +4945,8 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
> >  			i915_cache_sharing_get, i915_cache_sharing_set,
> >  			"%llu\n");
> >  
> > -struct sseu_dev_status {
> > -	unsigned int slice_total;
> > -	unsigned int subslice_total;
> > -	unsigned int subslice_per_slice;
> > -	unsigned int eu_total;
> > -	unsigned int eu_per_subslice;
> > -};
> > -
> >  static void cherryview_sseu_device_status(struct drm_device *dev,
> > -					  struct sseu_dev_status *stat)
> > +					  struct sseu_dev_info *stat)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int ss_max = 2;
> > @@ -4980,13 +4972,14 @@ static void cherryview_sseu_device_status(struct drm_device *dev,
> >  			 ((sig1[ss] & CHV_EU210_PG_ENABLE) ? 0 : 2) +
> >  			 ((sig2[ss] & CHV_EU311_PG_ENABLE) ? 0 : 2);
> >  		stat->eu_total += eu_cnt;
> > -		stat->eu_per_subslice = max(stat->eu_per_subslice, eu_cnt);
> > +		stat->eu_per_subslice = max_t(unsigned int,
> > +					      stat->eu_per_subslice, eu_cnt);
> >  	}
> >  	stat->subslice_total = stat->subslice_per_slice;
> >  }
> >  
> >  static void gen9_sseu_device_status(struct drm_device *dev,
> > -				    struct sseu_dev_status *stat)
> > +				    struct sseu_dev_info *stat)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int s_max = 3, ss_max = 4;
> > @@ -5040,18 +5033,20 @@ static void gen9_sseu_device_status(struct drm_device *dev,
> >  			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
> >  					       eu_mask[ss%2]);
> >  			stat->eu_total += eu_cnt;
> > -			stat->eu_per_subslice = max(stat->eu_per_subslice,
> > -						    eu_cnt);
> > +			stat->eu_per_subslice = max_t(unsigned int,
> > +						      stat->eu_per_subslice,
> > +						      eu_cnt);
> >  		}
> >  
> >  		stat->subslice_total += ss_cnt;
> > -		stat->subslice_per_slice = max(stat->subslice_per_slice,
> > -					       ss_cnt);
> > +		stat->subslice_per_slice = max_t(unsigned int,
> > +						 stat->subslice_per_slice,
> > +						 ss_cnt);
> >  	}
> >  }
> >  
> >  static void broadwell_sseu_device_status(struct drm_device *dev,
> > -					 struct sseu_dev_status *stat)
> > +					 struct sseu_dev_info *stat)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int s;
> > @@ -5079,7 +5074,7 @@ 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;
> > +	struct sseu_dev_info stat;
> 
> If you're going through this rename pain with the type anyway you may as well
> s/stat/info.
> 
> Also, I never understood what "sseu" was supposed to be short for. The spec

I started the use of sseu here as abbreviation of slice/subslice/EU, as in
encapsulation of attributes of this hierarchy. Originally we just needed a
few summarized count values but there was intent to add lists of per-component
details as needed.
-Jeff

> calls these "global attributes" which is admittedly a way too generic name. As a
> suggestion based on hindsight, I believe the following would be a bit nicer.
> struct slice_attributes {
> 	u8 slice_count;
> 	u8 eu_total; /* This is sort of useless since if eu_total isn't trivially
> 		      * eu_per_subslice * subslice_count * slice_count, then we
> 		      * need to know exactly which subslice is missing EUs. */
> 	struct {
> 		u8 subslices_per_slice;
> 		u8 eu_count; /* XXX: see above comment */
> 	} subslice;
> 
> 	#define subslice.total (subslices_per_slice * slice_count)
> }
> 
> 
> Just a thought. What you have is fine too though:
> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> >  
> >  	if (INTEL_INFO(dev)->gen < 8)
> >  		return -ENODEV;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8afda45..73ff01f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -782,6 +782,14 @@ struct intel_csr {
> >  #define DEFINE_FLAG(name) u8 name:1
> >  #define SEP_SEMICOLON ;
> >  
> > +struct sseu_dev_info {
> > +	u8 slice_total;
> > +	u8 subslice_total;
> > +	u8 subslice_per_slice;
> > +	u8 eu_total;
> > +	u8 eu_per_subslice;
> > +};
> > +
> >  struct intel_device_info {
> >  	u32 display_mmio_offset;
> >  	u16 device_id;
> 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/7] drm/i915: sseu: convert slice count field to mask
  2015-11-19 23:40   ` Ben Widawsky
@ 2015-11-20 13:00     ` Ville Syrjälä
  0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2015-11-20 13:00 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Thu, Nov 19, 2015 at 03:40:45PM -0800, Ben Widawsky wrote:
> On Wed, Oct 21, 2015 at 06:40:34PM +0300, Imre Deak wrote:
> > In an upcoming patch we'll need the actual mask of slices in addition to
> > their count, so replace the count field with a mask.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++-------
> >  drivers/gpu/drm/i915/i915_dma.c     | 37 +++++++++++++++++++------------------
> >  drivers/gpu/drm/i915/i915_drv.h     |  2 +-
> >  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
> >  4 files changed, 28 insertions(+), 27 deletions(-)
>>  
<snip>
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 8a55f8a..4130ff1 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2202,7 +2202,7 @@ make_rpcs(struct drm_device *dev)
> >  	*/
> >  	if (INTEL_INFO(dev)->sseu.has_slice_pg) {
> >  		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
> > -		rpcs |= INTEL_INFO(dev)->sseu.slice_total <<
> > +		rpcs |= hweight32(INTEL_INFO(dev)->sseu.slice_mask) <<
> 			^ hweight8?
> 
> >  			GEN8_RPCS_S_CNT_SHIFT;
> >  		rpcs |= GEN8_RPCS_ENABLE;
> >  	}
> 
> I'm not positive if hweight32 is actually okay on an 8bit type. I remember Ville
> correcting me once on this, but I can't remember it's correct. Assuming
> hweight32 is fine to use, with or without my recommendations, this is:
> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>

I think my hweight8() comment (w.r.t. some patches IIRC) was just an
idea for a micro-optimizition. C promotes small stuff to ints anyway
so I assume hweight32() is just fine functionally.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-11-20 13:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-21 15:40 [PATCH 0/7] drm/i915: read out slice/subslice masks Imre Deak
2015-10-21 15:40 ` [PATCH 1/7] drm/i915: sseu: move sseu_dev_status to i915_drv.h Imre Deak
2015-11-19 23:10   ` Ben Widawsky
2015-11-20  0:29     ` Jeff McGee
2015-10-21 15:40 ` [PATCH 2/7] drm/i915: sseu: use sseu_dev_info in device info Imre Deak
2015-11-19 23:18   ` Ben Widawsky
2015-10-21 15:40 ` [PATCH 3/7] drm/i915: sseu: simplify debugfs status/info printing Imre Deak
2015-11-19 23:24   ` Ben Widawsky
2015-10-21 15:40 ` [PATCH 4/7] drm/i915: sseu: convert slice count field to mask Imre Deak
2015-11-19 23:40   ` Ben Widawsky
2015-11-20 13:00     ` Ville Syrjälä
2015-10-21 15:40 ` [PATCH 5/7] drm/i915: sseu: convert subslice count fields to subslice mask Imre Deak
2015-11-20  0:07   ` Ben Widawsky
2015-10-21 15:40 ` [PATCH 6/7] drm/i915: sseu: add debug printf for slice/subslice masks Imre Deak
2015-10-21 15:40 ` [PATCH 7/7] drm/i915/bdw: sseu: fix sseu status parsing Imre Deak
2015-11-20  0:08   ` Ben Widawsky
2015-10-27 11:45 ` [PATCH 0/7] drm/i915: read out slice/subslice masks Robert Bragg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.