[CI 0/5] Refactor to expand subslice mask

[CI 0/5] Refactor to expand subslice mask

[Stuart Summers]

This patch series contains a few code clean-up patches, followed
by a patch which changes the storage of the subslice mask to better
match the userspace access through the I915_QUERY_TOPOLOGY_INFO
ioctl. The index into the subslice_mask array is then calculated:
  slice * subslice stride + subslice index / 8

v2: fix i915_pm_sseu test failure
v3: no changes to patches in the series, just resending to pick up
    in CI correctly
v4: rebase
v5: fix header test
v6: address review comments from Jari
    address minor checkpatch warning in existing code
    use eu_stride for EU div-by-8
v7: another rebase
v8: address review comments from Tvrtko and Daniele
v9: address review comments from Daniele
v10: add reviewed-by on last patch with minor suggested change,
     rebase, and repost for CI

Stuart Summers (5):
  drm/i915: Use local variable for SSEU info in GETPARAM ioctl
  drm/i915: Add macro for SSEU stride calculation
  drm/i915: Move calculation of subslices per slice to new function
  drm/i915: Refactor sseu helper functions
  drm/i915: Expand subslice mask

 drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
 drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
 drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
 drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
 drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
 drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
 drivers/gpu/drm/i915/i915_drv.c              |  15 +-
 drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
 drivers/gpu/drm/i915/i915_query.c            |  15 +-
 drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--------
 drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
 12 files changed, 280 insertions(+), 180 deletions(-)

-- 
2.21.0.5.gaeb582a983

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

[CI 1/5] drm/i915: Use local variable for SSEU info in GETPARAM ioctl

[Stuart Summers]

In the GETPARAM ioctl handler, use a local variable to consolidate
usage of SSEU runtime info.

v2: add const to sseu_dev_info variable

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 83d2eb9e74cb..712d48936e8a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -329,6 +329,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
+	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	drm_i915_getparam_t *param = data;
 	int value;
 
@@ -382,12 +383,12 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = i915_cmd_parser_get_version(dev_priv);
 		break;
 	case I915_PARAM_SUBSLICE_TOTAL:
-		value = sseu_subslice_total(&RUNTIME_INFO(dev_priv)->sseu);
+		value = sseu_subslice_total(sseu);
 		if (!value)
 			return -ENODEV;
 		break;
 	case I915_PARAM_EU_TOTAL:
-		value = RUNTIME_INFO(dev_priv)->sseu.eu_total;
+		value = sseu->eu_total;
 		if (!value)
 			return -ENODEV;
 		break;
@@ -404,7 +405,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = HAS_POOLED_EU(dev_priv);
 		break;
 	case I915_PARAM_MIN_EU_IN_POOL:
-		value = RUNTIME_INFO(dev_priv)->sseu.min_eu_in_pool;
+		value = sseu->min_eu_in_pool;
 		break;
 	case I915_PARAM_HUC_STATUS:
 		value = intel_huc_check_status(&dev_priv->huc);
@@ -455,12 +456,12 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = intel_engines_has_context_isolation(dev_priv);
 		break;
 	case I915_PARAM_SLICE_MASK:
-		value = RUNTIME_INFO(dev_priv)->sseu.slice_mask;
+		value = sseu->slice_mask;
 		if (!value)
 			return -ENODEV;
 		break;
 	case I915_PARAM_SUBSLICE_MASK:
-		value = RUNTIME_INFO(dev_priv)->sseu.subslice_mask[0];
+		value = sseu->subslice_mask[0];
 		if (!value)
 			return -ENODEV;
 		break;
-- 
2.21.0.5.gaeb582a983

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

[CI 2/5] drm/i915: Add macro for SSEU stride calculation

[Stuart Summers]

Subslice stride and EU stride are calculated multiple times in
i915_query. Move this calculation to a macro to reduce code duplication.

v2: update headers in intel_sseu.h
v3: use GEN_SSEU_STRIDE for stride calculations in intel_sseu.h
    apply s/bits/max_entries/ to GEN_SSEU_STRIDE parameter

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_sseu.h     |  2 ++
 drivers/gpu/drm/i915/i915_query.c        | 17 ++++++++---------
 drivers/gpu/drm/i915/intel_device_info.h |  9 +++------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
index 73bc824094e8..d20b7f96907d 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.h
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
@@ -8,11 +8,13 @@
 #define __INTEL_SSEU_H__
 
 #include <linux/types.h>
+#include <linux/kernel.h>
 
 struct drm_i915_private;
 
 #define GEN_MAX_SLICES		(6) /* CNL upper bound */
 #define GEN_MAX_SUBSLICES	(8) /* ICL upper bound */
+#define GEN_SSEU_STRIDE(max_entries) DIV_ROUND_UP(max_entries, BITS_PER_BYTE)
 
 struct sseu_dev_info {
 	u8 slice_mask;
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 414d0a6d1f70..7b7016171057 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -37,6 +37,8 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	struct drm_i915_query_topology_info topo;
 	u32 slice_length, subslice_length, eu_length, total_length;
+	u8 subslice_stride = GEN_SSEU_STRIDE(sseu->max_subslices);
+	u8 eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
 	int ret;
 
 	if (query_item->flags != 0)
@@ -48,12 +50,10 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
 
 	slice_length = sizeof(sseu->slice_mask);
-	subslice_length = sseu->max_slices *
-		DIV_ROUND_UP(sseu->max_subslices, BITS_PER_BYTE);
-	eu_length = sseu->max_slices * sseu->max_subslices *
-		DIV_ROUND_UP(sseu->max_eus_per_subslice, BITS_PER_BYTE);
-
-	total_length = sizeof(topo) + slice_length + subslice_length + eu_length;
+	subslice_length = sseu->max_slices * subslice_stride;
+	eu_length = sseu->max_slices * sseu->max_subslices * eu_stride;
+	total_length = sizeof(topo) + slice_length + subslice_length +
+		       eu_length;
 
 	ret = copy_query_item(&topo, sizeof(topo), total_length,
 			      query_item);
@@ -69,10 +69,9 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	topo.max_eus_per_subslice = sseu->max_eus_per_subslice;
 
 	topo.subslice_offset = slice_length;
-	topo.subslice_stride = DIV_ROUND_UP(sseu->max_subslices, BITS_PER_BYTE);
+	topo.subslice_stride = subslice_stride;
 	topo.eu_offset = slice_length + subslice_length;
-	topo.eu_stride =
-		DIV_ROUND_UP(sseu->max_eus_per_subslice, BITS_PER_BYTE);
+	topo.eu_stride = eu_stride;
 
 	if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr),
 			   &topo, sizeof(topo)))
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 5a2e17d6146b..9d43f7edfd63 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -231,8 +231,7 @@ static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
 static inline int sseu_eu_idx(const struct sseu_dev_info *sseu,
 			      int slice, int subslice)
 {
-	int subslice_stride = DIV_ROUND_UP(sseu->max_eus_per_subslice,
-					   BITS_PER_BYTE);
+	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
 	int slice_stride = sseu->max_subslices * subslice_stride;
 
 	return slice * slice_stride + subslice * subslice_stride;
@@ -244,8 +243,7 @@ static inline u16 sseu_get_eus(const struct sseu_dev_info *sseu,
 	int i, offset = sseu_eu_idx(sseu, slice, subslice);
 	u16 eu_mask = 0;
 
-	for (i = 0;
-	     i < DIV_ROUND_UP(sseu->max_eus_per_subslice, BITS_PER_BYTE); i++) {
+	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
 		eu_mask |= ((u16) sseu->eu_mask[offset + i]) <<
 			(i * BITS_PER_BYTE);
 	}
@@ -258,8 +256,7 @@ static inline void sseu_set_eus(struct sseu_dev_info *sseu,
 {
 	int i, offset = sseu_eu_idx(sseu, slice, subslice);
 
-	for (i = 0;
-	     i < DIV_ROUND_UP(sseu->max_eus_per_subslice, BITS_PER_BYTE); i++) {
+	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
 		sseu->eu_mask[offset + i] =
 			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
 	}
-- 
2.21.0.5.gaeb582a983

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

[CI 3/5] drm/i915: Move calculation of subslices per slice to new function

[Stuart Summers]

Add a new function to return the number of subslices per slice to
consolidate code usage.

v2: rebase on changes to move sseu struct to intel_sseu.h
v3: add intel_* prefix to sseu_subslices_per_slice

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_sseu.h     | 6 ++++++
 drivers/gpu/drm/i915/i915_debugfs.c      | 2 +-
 drivers/gpu/drm/i915/intel_device_info.c | 4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
index d20b7f96907d..9618dff46d83 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.h
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
@@ -63,6 +63,12 @@ intel_sseu_from_device_info(const struct sseu_dev_info *sseu)
 	return value;
 }
 
+static inline unsigned int
+intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice)
+{
+	return hweight8(sseu->subslice_mask[slice]);
+}
+
 u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
 			 const struct intel_sseu *req_sseu);
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 633a08c0f907..73cc6d9f9157 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4203,7 +4203,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 		   sseu_subslice_total(sseu));
 	for (s = 0; s < fls(sseu->slice_mask); s++) {
 		seq_printf(m, "  %s Slice%i subslices: %u\n", type,
-			   s, hweight8(sseu->subslice_mask[s]));
+			   s, intel_sseu_subslices_per_slice(sseu, s));
 	}
 	seq_printf(m, "  %s EU Total: %u\n", type,
 		   sseu->eu_total);
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 6af480b95bc6..9d6b9c45bc5e 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -93,7 +93,7 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
 	drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
 	for (s = 0; s < sseu->max_slices; s++) {
 		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
-			   s, hweight8(sseu->subslice_mask[s]),
+			   s, intel_sseu_subslices_per_slice(sseu, s),
 			   sseu->subslice_mask[s]);
 	}
 	drm_printf(p, "EU total: %u\n", sseu->eu_total);
@@ -126,7 +126,7 @@ void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
 
 	for (s = 0; s < sseu->max_slices; s++) {
 		drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n",
-			   s, hweight8(sseu->subslice_mask[s]),
+			   s, intel_sseu_subslices_per_slice(sseu, s),
 			   sseu->subslice_mask[s]);
 
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
-- 
2.21.0.5.gaeb582a983

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

[CI 4/5] drm/i915: Refactor sseu helper functions

[Stuart Summers]

Move functions to intel_sseu.h and remove inline qualifier.
Additionally, ensure these are all prefixed with intel_sseu_*
to match the convention of other functions in i915.

v2: fix spacing from checkpatch warning
v3: squash helper function changes into a single patch
    break 80 character line to fix checkpatch warning
    move get/set_eus helpers to intel_device_info.c
v4: Remove intel_ prefix from static functions in
    intel_device_info.c and correctly copy changes
    to stride calculation in those functions.

Acked-by: Jani Nikula <jani.nikula@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_sseu.c     | 17 +++++++
 drivers/gpu/drm/i915/gt/intel_sseu.h     | 10 ++--
 drivers/gpu/drm/i915/i915_debugfs.c      |  4 +-
 drivers/gpu/drm/i915/i915_drv.c          |  2 +-
 drivers/gpu/drm/i915/intel_device_info.c | 60 +++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_device_info.h | 44 -----------------
 6 files changed, 74 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
index 7f448f3bea0b..a0756f006f5f 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -8,6 +8,23 @@
 #include "intel_lrc_reg.h"
 #include "intel_sseu.h"
 
+unsigned int
+intel_sseu_subslice_total(const struct sseu_dev_info *sseu)
+{
+	unsigned int i, total = 0;
+
+	for (i = 0; i < ARRAY_SIZE(sseu->subslice_mask); i++)
+		total += hweight8(sseu->subslice_mask[i]);
+
+	return total;
+}
+
+unsigned int
+intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice)
+{
+	return hweight8(sseu->subslice_mask[slice]);
+}
+
 u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
 			 const struct intel_sseu *req_sseu)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
index 9618dff46d83..b50d0401a4e2 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.h
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
@@ -63,11 +63,11 @@ intel_sseu_from_device_info(const struct sseu_dev_info *sseu)
 	return value;
 }
 
-static inline unsigned int
-intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice)
-{
-	return hweight8(sseu->subslice_mask[slice]);
-}
+unsigned int
+intel_sseu_subslice_total(const struct sseu_dev_info *sseu);
+
+unsigned int
+intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice);
 
 u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
 			 const struct intel_sseu *req_sseu);
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 73cc6d9f9157..7556a205f677 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4176,7 +4176,7 @@ static void broadwell_sseu_device_status(struct drm_i915_private *dev_priv,
 				RUNTIME_INFO(dev_priv)->sseu.subslice_mask[s];
 		}
 		sseu->eu_total = sseu->eu_per_subslice *
-				 sseu_subslice_total(sseu);
+				 intel_sseu_subslice_total(sseu);
 
 		/* subtract fused off EU(s) from enabled slice(s) */
 		for (s = 0; s < fls(sseu->slice_mask); s++) {
@@ -4200,7 +4200,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 	seq_printf(m, "  %s Slice Total: %u\n", type,
 		   hweight8(sseu->slice_mask));
 	seq_printf(m, "  %s Subslice Total: %u\n", type,
-		   sseu_subslice_total(sseu));
+		   intel_sseu_subslice_total(sseu));
 	for (s = 0; s < fls(sseu->slice_mask); s++) {
 		seq_printf(m, "  %s Slice%i subslices: %u\n", type,
 			   s, intel_sseu_subslices_per_slice(sseu, s));
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 712d48936e8a..f3eea5676123 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -383,7 +383,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		value = i915_cmd_parser_get_version(dev_priv);
 		break;
 	case I915_PARAM_SUBSLICE_TOTAL:
-		value = sseu_subslice_total(sseu);
+		value = intel_sseu_subslice_total(sseu);
 		if (!value)
 			return -ENODEV;
 		break;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 9d6b9c45bc5e..97f742530fa1 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -90,7 +90,7 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
 
 	drm_printf(p, "slice total: %u, mask=%04x\n",
 		   hweight8(sseu->slice_mask), sseu->slice_mask);
-	drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
+	drm_printf(p, "subslice total: %u\n", intel_sseu_subslice_total(sseu));
 	for (s = 0; s < sseu->max_slices; s++) {
 		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
 			   s, intel_sseu_subslices_per_slice(sseu, s),
@@ -114,6 +114,40 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info,
 		   info->cs_timestamp_frequency_khz);
 }
 
+static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice,
+		       int subslice)
+{
+	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
+	int slice_stride = sseu->max_subslices * subslice_stride;
+
+	return slice * slice_stride + subslice * subslice_stride;
+}
+
+static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
+			int subslice)
+{
+	int i, offset = sseu_eu_idx(sseu, slice, subslice);
+	u16 eu_mask = 0;
+
+	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
+		eu_mask |= ((u16)sseu->eu_mask[offset + i]) <<
+			(i * BITS_PER_BYTE);
+	}
+
+	return eu_mask;
+}
+
+static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice,
+			 u16 eu_mask)
+{
+	int i, offset = sseu_eu_idx(sseu, slice, subslice);
+
+	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
+		sseu->eu_mask[offset + i] =
+			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
+	}
+}
+
 void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
 				     struct drm_printer *p)
 {
@@ -260,9 +294,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
 	 * EU in any one subslice may be fused off for die
 	 * recovery.
 	 */
-	sseu->eu_per_subslice = sseu_subslice_total(sseu) ?
+	sseu->eu_per_subslice = intel_sseu_subslice_total(sseu) ?
 				DIV_ROUND_UP(sseu->eu_total,
-					     sseu_subslice_total(sseu)) : 0;
+					     intel_sseu_subslice_total(sseu)) :
+				0;
 
 	/* No restrictions on Power Gating */
 	sseu->has_slice_pg = 1;
@@ -310,8 +345,9 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 	 * CHV expected to always have a uniform distribution of EU
 	 * across subslices.
 	*/
-	sseu->eu_per_subslice = sseu_subslice_total(sseu) ?
-				sseu->eu_total / sseu_subslice_total(sseu) :
+	sseu->eu_per_subslice = intel_sseu_subslice_total(sseu) ?
+				sseu->eu_total /
+					intel_sseu_subslice_total(sseu) :
 				0;
 	/*
 	 * CHV supports subslice power gating on devices with more than
@@ -319,7 +355,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 	 * more than one EU pair per subslice.
 	*/
 	sseu->has_slice_pg = 0;
-	sseu->has_subslice_pg = sseu_subslice_total(sseu) > 1;
+	sseu->has_subslice_pg = intel_sseu_subslice_total(sseu) > 1;
 	sseu->has_eu_pg = (sseu->eu_per_subslice > 2);
 }
 
@@ -393,9 +429,10 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 	 * recovery. BXT is expected to be perfectly uniform in EU
 	 * distribution.
 	*/
-	sseu->eu_per_subslice = sseu_subslice_total(sseu) ?
+	sseu->eu_per_subslice = intel_sseu_subslice_total(sseu) ?
 				DIV_ROUND_UP(sseu->eu_total,
-					     sseu_subslice_total(sseu)) : 0;
+					     intel_sseu_subslice_total(sseu)) :
+				0;
 	/*
 	 * SKL+ supports slice power gating on devices with more than
 	 * one slice, and supports EU power gating on devices with
@@ -407,7 +444,7 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 	sseu->has_slice_pg =
 		!IS_GEN9_LP(dev_priv) && hweight8(sseu->slice_mask) > 1;
 	sseu->has_subslice_pg =
-		IS_GEN9_LP(dev_priv) && sseu_subslice_total(sseu) > 1;
+		IS_GEN9_LP(dev_priv) && intel_sseu_subslice_total(sseu) > 1;
 	sseu->has_eu_pg = sseu->eu_per_subslice > 2;
 
 	if (IS_GEN9_LP(dev_priv)) {
@@ -496,9 +533,10 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
 	 * subslices with the exception that any one EU in any one subslice may
 	 * be fused off for die recovery.
 	 */
-	sseu->eu_per_subslice = sseu_subslice_total(sseu) ?
+	sseu->eu_per_subslice = intel_sseu_subslice_total(sseu) ?
 				DIV_ROUND_UP(sseu->eu_total,
-					     sseu_subslice_total(sseu)) : 0;
+					     intel_sseu_subslice_total(sseu)) :
+				0;
 
 	/*
 	 * BDW supports slice power gating on devices with more than
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 9d43f7edfd63..6412a9c72898 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -218,50 +218,6 @@ struct intel_driver_caps {
 	bool has_logical_contexts:1;
 };
 
-static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
-{
-	unsigned int i, total = 0;
-
-	for (i = 0; i < ARRAY_SIZE(sseu->subslice_mask); i++)
-		total += hweight8(sseu->subslice_mask[i]);
-
-	return total;
-}
-
-static inline int sseu_eu_idx(const struct sseu_dev_info *sseu,
-			      int slice, int subslice)
-{
-	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
-	int slice_stride = sseu->max_subslices * subslice_stride;
-
-	return slice * slice_stride + subslice * subslice_stride;
-}
-
-static inline u16 sseu_get_eus(const struct sseu_dev_info *sseu,
-			       int slice, int subslice)
-{
-	int i, offset = sseu_eu_idx(sseu, slice, subslice);
-	u16 eu_mask = 0;
-
-	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
-		eu_mask |= ((u16) sseu->eu_mask[offset + i]) <<
-			(i * BITS_PER_BYTE);
-	}
-
-	return eu_mask;
-}
-
-static inline void sseu_set_eus(struct sseu_dev_info *sseu,
-				int slice, int subslice, u16 eu_mask)
-{
-	int i, offset = sseu_eu_idx(sseu, slice, subslice);
-
-	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
-		sseu->eu_mask[offset + i] =
-			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
-	}
-}
-
 const char *intel_platform_name(enum intel_platform platform);
 
 void intel_device_info_subplatform_init(struct drm_i915_private *dev_priv);
-- 
2.21.0.5.gaeb582a983

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

[CI 5/5] drm/i915: Expand subslice mask

[Stuart Summers]

Currently, the subslice_mask runtime parameter is stored as an
array of subslices per slice. Expand the subslice mask array to
better match what is presented to userspace through the
I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
then calculated:
  slice * subslice stride + subslice index / 8

v2: fix spacing in set_sseu_info args
    use set_sseu_info to initialize sseu data when building
    device status in debugfs
    rename variables in intel_engine_types.h to avoid checkpatch
    warnings
v3: update headers in intel_sseu.h
v4: add const to some sseu_dev_info variables
    use sseu->eu_stride for EU stride calculations
v5: address review comments from Tvrtko and Daniele
v6: remove extra space in intel_sseu_get_subslices
    return the correct subslice enable in for_each_instdone
    add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size
    use printk formatted string for subslice mask
v7: remove string.h header and rebase

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 +++-
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++---
 drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
 drivers/gpu/drm/i915/gt/intel_sseu.c         |  47 ++++++-
 drivers/gpu/drm/i915/gt/intel_sseu.h         |  27 +++-
 drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
 drivers/gpu/drm/i915/i915_debugfs.c          |  40 +++---
 drivers/gpu/drm/i915/i915_drv.c              |   6 +-
 drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
 drivers/gpu/drm/i915/i915_query.c            |  10 +-
 drivers/gpu/drm/i915/intel_device_info.c     | 122 +++++++++----------
 11 files changed, 200 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 2590f5904b67..509f02a65408 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -951,12 +951,30 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
 	}
 }
 
+static inline u32
+intel_sseu_fls_subslice(const struct sseu_dev_info *sseu, u32 slice)
+{
+	u32 subslice;
+	int i;
+
+	for (i = sseu->ss_stride - 1; i >= 0; i--) {
+		subslice = fls(sseu->subslice_mask[slice * sseu->ss_stride +
+						   i]);
+		if (subslice) {
+			subslice += i * BITS_PER_BYTE;
+			break;
+		}
+	}
+
+	return subslice;
+}
+
 u32 intel_calculate_mcr_s_ss_select(struct drm_i915_private *dev_priv)
 {
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	u32 mcr_s_ss_select;
 	u32 slice = fls(sseu->slice_mask);
-	u32 subslice = fls(sseu->subslice_mask[slice]);
+	u32 subslice = intel_sseu_fls_subslice(sseu, slice);
 
 	if (IS_GEN(dev_priv, 10))
 		mcr_s_ss_select = GEN8_MCR_SLICE(slice) |
@@ -1032,6 +1050,7 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine,
 			       struct intel_instdone *instdone)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	struct intel_uncore *uncore = engine->uncore;
 	u32 mmio_base = engine->mmio_base;
 	int slice;
@@ -1049,7 +1068,8 @@ void intel_engine_get_instdone(struct intel_engine_cs *engine,
 
 		instdone->slice_common =
 			intel_uncore_read(uncore, GEN7_SC_INSTDONE);
-		for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
+		for_each_instdone_slice_subslice(dev_priv, sseu, slice,
+						 subslice) {
 			instdone->sampler[slice][subslice] =
 				read_subslice_reg(dev_priv, slice, subslice,
 						  GEN7_SAMPLER_INSTDONE);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 40e774acc2cd..41c257f6a560 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -551,20 +551,20 @@ intel_engine_is_virtual(const struct intel_engine_cs *engine)
 	return engine->flags & I915_ENGINE_IS_VIRTUAL;
 }
 
-#define instdone_slice_mask(dev_priv__) \
-	(IS_GEN(dev_priv__, 7) ? \
-	 1 : RUNTIME_INFO(dev_priv__)->sseu.slice_mask)
-
-#define instdone_subslice_mask(dev_priv__) \
-	(IS_GEN(dev_priv__, 7) ? \
-	 1 : RUNTIME_INFO(dev_priv__)->sseu.subslice_mask[0])
-
-#define for_each_instdone_slice_subslice(dev_priv__, slice__, subslice__) \
-	for ((slice__) = 0, (subslice__) = 0; \
-	     (slice__) < I915_MAX_SLICES; \
-	     (subslice__) = ((subslice__) + 1) < I915_MAX_SUBSLICES ? (subslice__) + 1 : 0, \
-	       (slice__) += ((subslice__) == 0)) \
-		for_each_if((BIT(slice__) & instdone_slice_mask(dev_priv__)) && \
-			    (BIT(subslice__) & instdone_subslice_mask(dev_priv__)))
+#define instdone_has_slice(dev_priv___, sseu___, slice___) \
+	((IS_GEN(dev_priv___, 7) ? 1 : ((sseu___)->slice_mask)) & \
+	BIT(slice___))
+
+#define instdone_has_subslice(dev_priv__, sseu__, slice__, subslice__) \
+	(IS_GEN(dev_priv__, 7) ? (1 & BIT(subslice__)) : \
+	 intel_sseu_has_subslice(sseu__, slice__, subslice__))
+
+#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \
+	for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \
+	     (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \
+	     (slice_) += ((subslice_) == 0)) \
+		for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \
+			    (instdone_has_subslice(dev_priv_, sseu_, slice_, \
+						    subslice_)))
 
 #endif /* __INTEL_ENGINE_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/gt/intel_hangcheck.c b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
index 3a4d09b80fa0..b19cd4cdcb5c 100644
--- a/drivers/gpu/drm/i915/gt/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/intel_hangcheck.c
@@ -51,6 +51,7 @@ static bool instdone_unchanged(u32 current_instdone, u32 *old_instdone)
 static bool subunits_stuck(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
+	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	struct intel_instdone instdone;
 	struct intel_instdone *accu_instdone = &engine->hangcheck.instdone;
 	bool stuck;
@@ -72,7 +73,7 @@ static bool subunits_stuck(struct intel_engine_cs *engine)
 	stuck &= instdone_unchanged(instdone.slice_common,
 				    &accu_instdone->slice_common);
 
-	for_each_instdone_slice_subslice(dev_priv, slice, subslice) {
+	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice) {
 		stuck &= instdone_unchanged(instdone.sampler[slice][subslice],
 					    &accu_instdone->sampler[slice][subslice]);
 		stuck &= instdone_unchanged(instdone.row[slice][subslice],
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c
index a0756f006f5f..763b811f2c9d 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.c
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.c
@@ -8,6 +8,17 @@
 #include "intel_lrc_reg.h"
 #include "intel_sseu.h"
 
+void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
+			 u8 max_subslices, u8 max_eus_per_subslice)
+{
+	sseu->max_slices = max_slices;
+	sseu->max_subslices = max_subslices;
+	sseu->max_eus_per_subslice = max_eus_per_subslice;
+
+	sseu->ss_stride = GEN_SSEU_STRIDE(sseu->max_subslices);
+	sseu->eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
+}
+
 unsigned int
 intel_sseu_subslice_total(const struct sseu_dev_info *sseu)
 {
@@ -19,10 +30,44 @@ intel_sseu_subslice_total(const struct sseu_dev_info *sseu)
 	return total;
 }
 
+void intel_sseu_copy_subslices(const struct sseu_dev_info *sseu, int slice,
+			       u8 *to_mask)
+{
+	int offset = slice * sseu->ss_stride;
+
+	memcpy(&to_mask[offset], &sseu->subslice_mask[offset], sseu->ss_stride);
+}
+
+u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice)
+{
+	int i, offset = slice * sseu->ss_stride;
+	u32 mask;
+
+	GEM_BUG_ON(slice >= sseu->max_slices);
+
+	GEM_BUG_ON(sseu->ss_stride > sizeof(mask));
+
+	for (i = 0; i < sseu->ss_stride; i++)
+		mask |= (u32)sseu->subslice_mask[offset + i] <<
+			i * BITS_PER_BYTE;
+
+	return mask;
+}
+
+void intel_sseu_set_subslices(struct sseu_dev_info *sseu, int slice,
+			      u32 ss_mask)
+{
+	int i, offset = slice * sseu->ss_stride;
+
+	for (i = 0; i < sseu->ss_stride; i++)
+		sseu->subslice_mask[offset + i] =
+			(ss_mask >> (BITS_PER_BYTE * i)) & 0xff;
+}
+
 unsigned int
 intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice)
 {
-	return hweight8(sseu->subslice_mask[slice]);
+	return hweight32(intel_sseu_get_subslices(sseu, slice));
 }
 
 u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h
index b50d0401a4e2..eee21d9f320e 100644
--- a/drivers/gpu/drm/i915/gt/intel_sseu.h
+++ b/drivers/gpu/drm/i915/gt/intel_sseu.h
@@ -15,10 +15,11 @@ struct drm_i915_private;
 #define GEN_MAX_SLICES		(6) /* CNL upper bound */
 #define GEN_MAX_SUBSLICES	(8) /* ICL upper bound */
 #define GEN_SSEU_STRIDE(max_entries) DIV_ROUND_UP(max_entries, BITS_PER_BYTE)
+#define GEN_MAX_SUBSLICE_STRIDE GEN_SSEU_STRIDE(GEN_MAX_SUBSLICES)
 
 struct sseu_dev_info {
 	u8 slice_mask;
-	u8 subslice_mask[GEN_MAX_SLICES];
+	u8 subslice_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICE_STRIDE];
 	u16 eu_total;
 	u8 eu_per_subslice;
 	u8 min_eu_in_pool;
@@ -33,6 +34,9 @@ struct sseu_dev_info {
 	u8 max_subslices;
 	u8 max_eus_per_subslice;
 
+	u8 ss_stride;
+	u8 eu_stride;
+
 	/* We don't have more than 8 eus per subslice at the moment and as we
 	 * store eus enabled using bits, no need to multiply by eus per
 	 * subslice.
@@ -63,12 +67,33 @@ intel_sseu_from_device_info(const struct sseu_dev_info *sseu)
 	return value;
 }
 
+static inline bool
+intel_sseu_has_subslice(const struct sseu_dev_info *sseu, int slice,
+			int subslice)
+{
+	u8 mask = sseu->subslice_mask[slice * sseu->ss_stride +
+				      subslice / BITS_PER_BYTE];
+
+	return mask & BIT(subslice % BITS_PER_BYTE);
+}
+
+void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices,
+			 u8 max_subslices, u8 max_eus_per_subslice);
+
 unsigned int
 intel_sseu_subslice_total(const struct sseu_dev_info *sseu);
 
 unsigned int
 intel_sseu_subslices_per_slice(const struct sseu_dev_info *sseu, u8 slice);
 
+void intel_sseu_copy_subslices(const struct sseu_dev_info *sseu, int slice,
+			       u8 *to_mask);
+
+u32  intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice);
+
+void intel_sseu_set_subslices(struct sseu_dev_info *sseu, int slice,
+			      u32 ss_mask);
+
 u32 intel_sseu_make_rpcs(struct drm_i915_private *i915,
 			 const struct intel_sseu *req_sseu);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index ce4bcca3f83c..0ff58d6bb01f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -783,7 +783,7 @@ wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
 		u32 slice = fls(sseu->slice_mask);
 		u32 fuse3 =
 			intel_uncore_read(&i915->uncore, GEN10_MIRROR_FUSE3);
-		u8 ss_mask = sseu->subslice_mask[slice];
+		u32 ss_mask = intel_sseu_get_subslices(sseu, slice);
 
 		u8 enabled_mask = (ss_mask | ss_mask >>
 				   GEN10_L3BANK_PAIR_COUNT) & GEN10_L3BANK_MASK;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7556a205f677..3f9f833007dd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1271,6 +1271,7 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
 			       struct seq_file *m,
 			       struct intel_instdone *instdone)
 {
+	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	int slice;
 	int subslice;
 
@@ -1286,11 +1287,11 @@ static void i915_instdone_info(struct drm_i915_private *dev_priv,
 	if (INTEL_GEN(dev_priv) <= 6)
 		return;
 
-	for_each_instdone_slice_subslice(dev_priv, slice, subslice)
+	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
 		seq_printf(m, "\t\tSAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
 			   slice, subslice, instdone->sampler[slice][subslice]);
 
-	for_each_instdone_slice_subslice(dev_priv, slice, subslice)
+	for_each_instdone_slice_subslice(dev_priv, sseu, slice, subslice)
 		seq_printf(m, "\t\tROW_INSTDONE[%d][%d]: 0x%08x\n",
 			   slice, subslice, instdone->row[slice][subslice]);
 }
@@ -4084,7 +4085,7 @@ static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
 			continue;
 
 		sseu->slice_mask |= BIT(s);
-		sseu->subslice_mask[s] = info->sseu.subslice_mask[s];
+		intel_sseu_copy_subslices(&info->sseu, s, sseu->subslice_mask);
 
 		for (ss = 0; ss < info->sseu.max_subslices; ss++) {
 			unsigned int eu_cnt;
@@ -4135,18 +4136,21 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 		sseu->slice_mask |= BIT(s);
 
 		if (IS_GEN9_BC(dev_priv))
-			sseu->subslice_mask[s] =
-				RUNTIME_INFO(dev_priv)->sseu.subslice_mask[s];
+			intel_sseu_copy_subslices(&info->sseu, s,
+						  sseu->subslice_mask);
 
 		for (ss = 0; ss < info->sseu.max_subslices; ss++) {
 			unsigned int eu_cnt;
+			u8 ss_idx = s * info->sseu.ss_stride +
+				    ss / BITS_PER_BYTE;
 
 			if (IS_GEN9_LP(dev_priv)) {
 				if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
 					/* skip disabled subslice */
 					continue;
 
-				sseu->subslice_mask[s] |= BIT(ss);
+				sseu->subslice_mask[ss_idx] |=
+					BIT(ss % BITS_PER_BYTE);
 			}
 
 			eu_cnt = 2 * hweight32(eu_reg[2*s + ss/2] &
@@ -4163,25 +4167,23 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
 static void broadwell_sseu_device_status(struct drm_i915_private *dev_priv,
 					 struct sseu_dev_info *sseu)
 {
+	struct intel_runtime_info *info = RUNTIME_INFO(dev_priv);
 	u32 slice_info = I915_READ(GEN8_GT_SLICE_INFO);
 	int s;
 
 	sseu->slice_mask = slice_info & GEN8_LSLICESTAT_MASK;
 
 	if (sseu->slice_mask) {
-		sseu->eu_per_subslice =
-			RUNTIME_INFO(dev_priv)->sseu.eu_per_subslice;
-		for (s = 0; s < fls(sseu->slice_mask); s++) {
-			sseu->subslice_mask[s] =
-				RUNTIME_INFO(dev_priv)->sseu.subslice_mask[s];
-		}
+		sseu->eu_per_subslice = info->sseu.eu_per_subslice;
+		for (s = 0; s < fls(sseu->slice_mask); s++)
+			intel_sseu_copy_subslices(&info->sseu, s,
+						  sseu->subslice_mask);
 		sseu->eu_total = sseu->eu_per_subslice *
 				 intel_sseu_subslice_total(sseu);
 
 		/* subtract fused off EU(s) from enabled slice(s) */
 		for (s = 0; s < fls(sseu->slice_mask); s++) {
-			u8 subslice_7eu =
-				RUNTIME_INFO(dev_priv)->sseu.subslice_7eu[s];
+			u8 subslice_7eu = info->sseu.subslice_7eu[s];
 
 			sseu->eu_total -= hweight8(subslice_7eu);
 		}
@@ -4228,6 +4230,7 @@ static void i915_print_sseu_info(struct seq_file *m, bool is_available_info,
 static int i915_sseu_status(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	const struct intel_runtime_info *info = RUNTIME_INFO(dev_priv);
 	struct sseu_dev_info sseu;
 	intel_wakeref_t wakeref;
 
@@ -4235,14 +4238,13 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 		return -ENODEV;
 
 	seq_puts(m, "SSEU Device Info\n");
-	i915_print_sseu_info(m, true, &RUNTIME_INFO(dev_priv)->sseu);
+	i915_print_sseu_info(m, true, &info->sseu);
 
 	seq_puts(m, "SSEU Device Status\n");
 	memset(&sseu, 0, sizeof(sseu));
-	sseu.max_slices = RUNTIME_INFO(dev_priv)->sseu.max_slices;
-	sseu.max_subslices = RUNTIME_INFO(dev_priv)->sseu.max_subslices;
-	sseu.max_eus_per_subslice =
-		RUNTIME_INFO(dev_priv)->sseu.max_eus_per_subslice;
+	intel_sseu_set_info(&sseu, info->sseu.max_slices,
+			    info->sseu.max_subslices,
+			    info->sseu.max_eus_per_subslice);
 
 	with_intel_runtime_pm(dev_priv, wakeref) {
 		if (IS_CHERRYVIEW(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f3eea5676123..0bac9bdde1e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -331,7 +331,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	drm_i915_getparam_t *param = data;
-	int value;
+	int value = 0;
 
 	switch (param->param) {
 	case I915_PARAM_IRQ_ACTIVE:
@@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
 			return -ENODEV;
 		break;
 	case I915_PARAM_SUBSLICE_MASK:
-		value = sseu->subslice_mask[0];
+		/* Only copy bits from the first slice */
+		memcpy(&value, sseu->subslice_mask,
+		       min(sseu->ss_stride, (u8)sizeof(value)));
 		if (!value)
 			return -ENODEV;
 		break;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 4f85cbdddb0d..c760cc5b3388 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -408,6 +408,7 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m,
 static void error_print_instdone(struct drm_i915_error_state_buf *m,
 				 const struct drm_i915_error_engine *ee)
 {
+	struct sseu_dev_info *sseu = &RUNTIME_INFO(m->i915)->sseu;
 	int slice;
 	int subslice;
 
@@ -423,12 +424,12 @@ static void error_print_instdone(struct drm_i915_error_state_buf *m,
 	if (INTEL_GEN(m->i915) <= 6)
 		return;
 
-	for_each_instdone_slice_subslice(m->i915, slice, subslice)
+	for_each_instdone_slice_subslice(m->i915, sseu, slice, subslice)
 		err_printf(m, "  SAMPLER_INSTDONE[%d][%d]: 0x%08x\n",
 			   slice, subslice,
 			   ee->instdone.sampler[slice][subslice]);
 
-	for_each_instdone_slice_subslice(m->i915, slice, subslice)
+	for_each_instdone_slice_subslice(m->i915, sseu, slice, subslice)
 		err_printf(m, "  ROW_INSTDONE[%d][%d]: 0x%08x\n",
 			   slice, subslice,
 			   ee->instdone.row[slice][subslice]);
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 7b7016171057..ac8ac59c4860 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -37,8 +37,6 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	struct drm_i915_query_topology_info topo;
 	u32 slice_length, subslice_length, eu_length, total_length;
-	u8 subslice_stride = GEN_SSEU_STRIDE(sseu->max_subslices);
-	u8 eu_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
 	int ret;
 
 	if (query_item->flags != 0)
@@ -50,8 +48,8 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
 
 	slice_length = sizeof(sseu->slice_mask);
-	subslice_length = sseu->max_slices * subslice_stride;
-	eu_length = sseu->max_slices * sseu->max_subslices * eu_stride;
+	subslice_length = sseu->max_slices * sseu->ss_stride;
+	eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride;
 	total_length = sizeof(topo) + slice_length + subslice_length +
 		       eu_length;
 
@@ -69,9 +67,9 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
 	topo.max_eus_per_subslice = sseu->max_eus_per_subslice;
 
 	topo.subslice_offset = slice_length;
-	topo.subslice_stride = subslice_stride;
+	topo.subslice_stride = sseu->ss_stride;
 	topo.eu_offset = slice_length + subslice_length;
-	topo.eu_stride = eu_stride;
+	topo.eu_stride = sseu->eu_stride;
 
 	if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr),
 			   &topo, sizeof(topo)))
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 97f742530fa1..3625f777f3a3 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -92,9 +92,9 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
 		   hweight8(sseu->slice_mask), sseu->slice_mask);
 	drm_printf(p, "subslice total: %u\n", intel_sseu_subslice_total(sseu));
 	for (s = 0; s < sseu->max_slices; s++) {
-		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
+		drm_printf(p, "slice%d: %u subslices, mask=%08x\n",
 			   s, intel_sseu_subslices_per_slice(sseu, s),
-			   sseu->subslice_mask[s]);
+			   intel_sseu_get_subslices(sseu, s));
 	}
 	drm_printf(p, "EU total: %u\n", sseu->eu_total);
 	drm_printf(p, "EU per subslice: %u\n", sseu->eu_per_subslice);
@@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info,
 static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice,
 		       int subslice)
 {
-	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
-	int slice_stride = sseu->max_subslices * subslice_stride;
+	int slice_stride = sseu->max_subslices * sseu->eu_stride;
 
-	return slice * slice_stride + subslice * subslice_stride;
+	return slice * slice_stride + subslice * sseu->eu_stride;
 }
 
 static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
@@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
 	int i, offset = sseu_eu_idx(sseu, slice, subslice);
 	u16 eu_mask = 0;
 
-	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
+	for (i = 0; i < sseu->eu_stride; i++) {
 		eu_mask |= ((u16)sseu->eu_mask[offset + i]) <<
 			(i * BITS_PER_BYTE);
 	}
@@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice,
 {
 	int i, offset = sseu_eu_idx(sseu, slice, subslice);
 
-	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
+	for (i = 0; i < sseu->eu_stride; i++) {
 		sseu->eu_mask[offset + i] =
 			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
 	}
@@ -159,9 +158,9 @@ void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
 	}
 
 	for (s = 0; s < sseu->max_slices; s++) {
-		drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n",
+		drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n",
 			   s, intel_sseu_subslices_per_slice(sseu, s),
-			   sseu->subslice_mask[s]);
+			   intel_sseu_get_subslices(sseu, s));
 
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
 			u16 enabled_eus = sseu_get_eus(sseu, s, ss);
@@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
 	u8 eu_en;
 	int s;
 
-	if (IS_ELKHARTLAKE(dev_priv)) {
-		sseu->max_slices = 1;
-		sseu->max_subslices = 4;
-		sseu->max_eus_per_subslice = 8;
-	} else {
-		sseu->max_slices = 1;
-		sseu->max_subslices = 8;
-		sseu->max_eus_per_subslice = 8;
-	}
+	if (IS_ELKHARTLAKE(dev_priv))
+		intel_sseu_set_info(sseu, 1, 4, 8);
+	else
+		intel_sseu_set_info(sseu, 1, 8, 8);
 
 	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
 	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
@@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
 
 	for (s = 0; s < sseu->max_slices; s++) {
 		if (s_en & BIT(s)) {
-			int ss_idx = sseu->max_subslices * s;
 			int ss;
 
 			sseu->slice_mask |= BIT(s);
-			sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask;
-			for (ss = 0; ss < sseu->max_subslices; ss++) {
-				if (sseu->subslice_mask[s] & BIT(ss))
+
+			intel_sseu_set_subslices(sseu, s, ss_en_mask);
+
+			for (ss = 0; ss < sseu->max_subslices; ss++)
+				if (intel_sseu_has_subslice(sseu, s, ss))
 					sseu_set_eus(sseu, s, ss, eu_en);
-			}
 		}
 	}
 	sseu->eu_per_subslice = hweight8(eu_en);
@@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
 	const int eu_mask = 0xff;
 	u32 subslice_mask, eu_en;
 
+	intel_sseu_set_info(sseu, 6, 4, 8);
+
 	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
 			    GEN10_F2_S_ENA_SHIFT;
-	sseu->max_slices = 6;
-	sseu->max_subslices = 4;
-	sseu->max_eus_per_subslice = 8;
-
-	subslice_mask = (1 << 4) - 1;
-	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
-			   GEN10_F2_SS_DIS_SHIFT);
-
-	/*
-	 * Slice0 can have up to 3 subslices, but there are only 2 in
-	 * slice1/2.
-	 */
-	sseu->subslice_mask[0] = subslice_mask;
-	for (s = 1; s < sseu->max_slices; s++)
-		sseu->subslice_mask[s] = subslice_mask & 0x3;
 
 	/* Slice0 */
 	eu_en = ~I915_READ(GEN8_EU_DISABLE0);
@@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
 	eu_en = ~I915_READ(GEN10_EU_DISABLE3);
 	sseu_set_eus(sseu, 5, 1, eu_en & eu_mask);
 
-	/* Do a second pass where we mark the subslices disabled if all their
-	 * eus are off.
-	 */
+	subslice_mask = (1 << 4) - 1;
+	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
+			   GEN10_F2_SS_DIS_SHIFT);
+
 	for (s = 0; s < sseu->max_slices; s++) {
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
 			if (sseu_get_eus(sseu, s, ss) == 0)
-				sseu->subslice_mask[s] &= ~BIT(ss);
+				subslice_mask &= ~BIT(ss);
 		}
+
+		/*
+		 * Slice0 can have up to 3 subslices, but there are only 2 in
+		 * slice1/2.
+		 */
+		intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask :
+							   subslice_mask & 0x3);
 	}
 
 	sseu->eu_total = compute_eu_total(sseu);
@@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 {
 	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	u32 fuse;
+	u8 subslice_mask;
 
 	fuse = I915_READ(CHV_FUSE_GT);
 
 	sseu->slice_mask = BIT(0);
-	sseu->max_slices = 1;
-	sseu->max_subslices = 2;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, 1, 2, 8);
 
 	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
 		u8 disabled_mask =
@@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 			(((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >>
 			  CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4);
 
-		sseu->subslice_mask[0] |= BIT(0);
+		subslice_mask |= BIT(0);
 		sseu_set_eus(sseu, 0, 0, ~disabled_mask);
 	}
 
@@ -335,10 +323,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
 			(((fuse & CHV_FGT_EU_DIS_SS1_R1_MASK) >>
 			  CHV_FGT_EU_DIS_SS1_R1_SHIFT) << 4);
 
-		sseu->subslice_mask[0] |= BIT(1);
+		subslice_mask |= BIT(1);
 		sseu_set_eus(sseu, 0, 1, ~disabled_mask);
 	}
 
+	intel_sseu_set_subslices(sseu, 0, subslice_mask);
+
 	sseu->eu_total = compute_eu_total(sseu);
 
 	/*
@@ -371,9 +361,8 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
 
 	/* BXT has a single slice and at most 3 subslices. */
-	sseu->max_slices = IS_GEN9_LP(dev_priv) ? 1 : 3;
-	sseu->max_subslices = IS_GEN9_LP(dev_priv) ? 3 : 4;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, IS_GEN9_LP(dev_priv) ? 1 : 3,
+			    IS_GEN9_LP(dev_priv) ? 3 : 4, 8);
 
 	/*
 	 * The subslice disable field is global, i.e. it applies
@@ -392,14 +381,14 @@ static void gen9_sseu_info_init(struct drm_i915_private *dev_priv)
 			/* skip disabled slice */
 			continue;
 
-		sseu->subslice_mask[s] = subslice_mask;
+		intel_sseu_set_subslices(sseu, s, subslice_mask);
 
 		eu_disable = I915_READ(GEN9_EU_DISABLE(s));
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
 			int eu_per_ss;
 			u8 eu_disabled_mask;
 
-			if (!(sseu->subslice_mask[s] & BIT(ss)))
+			if (!intel_sseu_has_subslice(sseu, s, ss))
 				/* skip disabled subslice */
 				continue;
 
@@ -472,9 +461,7 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
 
 	fuse2 = I915_READ(GEN8_FUSE2);
 	sseu->slice_mask = (fuse2 & GEN8_F2_S_ENA_MASK) >> GEN8_F2_S_ENA_SHIFT;
-	sseu->max_slices = 3;
-	sseu->max_subslices = 3;
-	sseu->max_eus_per_subslice = 8;
+	intel_sseu_set_info(sseu, 3, 3, 8);
 
 	/*
 	 * The subslice disable field is global, i.e. it applies
@@ -501,18 +488,19 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
 			/* skip disabled slice */
 			continue;
 
-		sseu->subslice_mask[s] = subslice_mask;
+		intel_sseu_set_subslices(sseu, s, subslice_mask);
 
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
 			u8 eu_disabled_mask;
 			u32 n_disabled;
 
-			if (!(sseu->subslice_mask[s] & BIT(ss)))
+			if (!intel_sseu_has_subslice(sseu, s, ss))
 				/* skip disabled subslice */
 				continue;
 
 			eu_disabled_mask =
-				eu_disable[s] >> (ss * sseu->max_eus_per_subslice);
+				eu_disable[s] >>
+					(ss * sseu->max_eus_per_subslice);
 
 			sseu_set_eus(sseu, s, ss, ~eu_disabled_mask);
 
@@ -552,6 +540,7 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
 	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
 	u32 fuse1;
 	int s, ss;
+	u32 subslice_mask;
 
 	/*
 	 * There isn't a register to tell us how many slices/subslices. We
@@ -563,22 +552,18 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
 		/* fall through */
 	case 1:
 		sseu->slice_mask = BIT(0);
-		sseu->subslice_mask[0] = BIT(0);
+		subslice_mask = BIT(0);
 		break;
 	case 2:
 		sseu->slice_mask = BIT(0);
-		sseu->subslice_mask[0] = BIT(0) | BIT(1);
+		subslice_mask = BIT(0) | BIT(1);
 		break;
 	case 3:
 		sseu->slice_mask = BIT(0) | BIT(1);
-		sseu->subslice_mask[0] = BIT(0) | BIT(1);
-		sseu->subslice_mask[1] = BIT(0) | BIT(1);
+		subslice_mask = BIT(0) | BIT(1);
 		break;
 	}
 
-	sseu->max_slices = hweight8(sseu->slice_mask);
-	sseu->max_subslices = hweight8(sseu->subslice_mask[0]);
-
 	fuse1 = I915_READ(HSW_PAVP_FUSE1);
 	switch ((fuse1 & HSW_F1_EU_DIS_MASK) >> HSW_F1_EU_DIS_SHIFT) {
 	default:
@@ -595,9 +580,14 @@ static void haswell_sseu_info_init(struct drm_i915_private *dev_priv)
 		sseu->eu_per_subslice = 6;
 		break;
 	}
-	sseu->max_eus_per_subslice = sseu->eu_per_subslice;
+
+	intel_sseu_set_info(sseu, hweight8(sseu->slice_mask),
+			    hweight8(subslice_mask),
+			    sseu->eu_per_subslice);
 
 	for (s = 0; s < sseu->max_slices; s++) {
+		intel_sseu_set_subslices(sseu, s, subslice_mask);
+
 		for (ss = 0; ss < sseu->max_subslices; ss++) {
 			sseu_set_eus(sseu, s, ss,
 				     (1UL << sseu->eu_per_subslice) - 1);
-- 
2.21.0.5.gaeb582a983

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

✗ Fi.CI.CHECKPATCH: warning for Refactor to expand subslice mask (rev10)

[Patchwork]

== Series Details ==

Series: Refactor to expand subslice mask (rev10)
URL   : https://patchwork.freedesktop.org/series/59742/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1d605edfee68 drm/i915: Use local variable for SSEU info in GETPARAM ioctl
1b9931fa807c drm/i915: Add macro for SSEU stride calculation
3ff1c4ffe466 drm/i915: Move calculation of subslices per slice to new function
9380b92a1933 drm/i915: Refactor sseu helper functions
1b9826e9f3ff drm/i915: Expand subslice mask
-:115: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'subslice__' - possible side-effects?
#115: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:558:
+#define instdone_has_subslice(dev_priv__, sseu__, slice__, subslice__) \
+	(IS_GEN(dev_priv__, 7) ? (1 & BIT(subslice__)) : \
+	 intel_sseu_has_subslice(sseu__, slice__, subslice__))

-:119: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dev_priv_' - possible side-effects?
#119: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:562:
+#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \
+	for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \
+	     (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \
+	     (slice_) += ((subslice_) == 0)) \
+		for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \
+			    (instdone_has_subslice(dev_priv_, sseu_, slice_, \
+						    subslice_)))

-:119: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'sseu_' - possible side-effects?
#119: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:562:
+#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \
+	for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \
+	     (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \
+	     (slice_) += ((subslice_) == 0)) \
+		for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \
+			    (instdone_has_subslice(dev_priv_, sseu_, slice_, \
+						    subslice_)))

-:119: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'slice_' - possible side-effects?
#119: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:562:
+#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \
+	for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \
+	     (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \
+	     (slice_) += ((subslice_) == 0)) \
+		for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \
+			    (instdone_has_subslice(dev_priv_, sseu_, slice_, \
+						    subslice_)))

-:119: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'subslice_' - possible side-effects?
#119: FILE: drivers/gpu/drm/i915/gt/intel_engine_types.h:562:
+#define for_each_instdone_slice_subslice(dev_priv_, sseu_, slice_, subslice_) \
+	for ((slice_) = 0, (subslice_) = 0; (slice_) < I915_MAX_SLICES; \
+	     (subslice_) = ((subslice_) + 1) % I915_MAX_SUBSLICES, \
+	     (slice_) += ((subslice_) == 0)) \
+		for_each_if((instdone_has_slice(dev_priv_, sseu_, slice_)) && \
+			    (instdone_has_subslice(dev_priv_, sseu_, slice_, \
+						    subslice_)))

total: 0 errors, 0 warnings, 5 checks, 679 lines checked

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

✗ Fi.CI.SPARSE: warning for Refactor to expand subslice mask (rev10)

[Patchwork]

== Series Details ==

Series: Refactor to expand subslice mask (rev10)
URL   : https://patchwork.freedesktop.org/series/59742/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Use local variable for SSEU info in GETPARAM ioctl
Okay!

Commit: drm/i915: Add macro for SSEU stride calculation
Okay!

Commit: drm/i915: Move calculation of subslices per slice to new function
Okay!

Commit: drm/i915: Refactor sseu helper functions
Okay!

Commit: drm/i915: Expand subslice mask
+drivers/gpu/drm/i915/i915_drv.c:466:24: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for Refactor to expand subslice mask (rev10)

[Patchwork]

== Series Details ==

Series: Refactor to expand subslice mask (rev10)
URL   : https://patchwork.freedesktop.org/series/59742/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6141 -> Patchwork_13092
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/

Known issues
------------

  Here are the changes found in Patchwork_13092 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [PASS][3] -> [DMESG-FAIL][4] ([fdo#110235])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       [PASS][5] -> [DMESG-WARN][6] ([fdo#107709])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/fi-bsw-kefka/igt@i915_selftest@live_evict.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/fi-bsw-kefka/igt@i915_selftest@live_evict.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-skl-6600u:       [INCOMPLETE][7] ([fdo#107807]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/fi-skl-6600u/igt@i915_pm_rpm@basic-rte.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/fi-skl-6600u/igt@i915_pm_rpm@basic-rte.html

  
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235


Participating hosts (53 -> 42)
------------------------------

  Missing    (11): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-byt-clapper fi-icl-u3 fi-icl-y fi-icl-dsi fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6141 -> Patchwork_13092

  CI_DRM_6141: e94845147cc0346c3a9114d5359b188008daff9d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5015: cdd6b0a7630762cec14596b9863f418b48c32f46 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13092: 1b9826e9f3ff6cb1d304f43c6a05e4700c02f97c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1b9826e9f3ff drm/i915: Expand subslice mask
9380b92a1933 drm/i915: Refactor sseu helper functions
3ff1c4ffe466 drm/i915: Move calculation of subslices per slice to new function
1b9931fa807c drm/i915: Add macro for SSEU stride calculation
1d605edfee68 drm/i915: Use local variable for SSEU info in GETPARAM ioctl

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Refactor to expand subslice mask (rev10)

[Patchwork]

== Series Details ==

Series: Refactor to expand subslice mask (rev10)
URL   : https://patchwork.freedesktop.org/series/59742/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6141_full -> Patchwork_13092_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_13092_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@pm-caching:
    - shard-skl:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107807])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl5/igt@i915_pm_rpm@pm-caching.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl8/igt@i915_pm_rpm@pm-caching.html

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#104108] / [fdo#107807])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl6/igt@i915_pm_rpm@system-suspend-modeset.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl1/igt@i915_pm_rpm@system-suspend-modeset.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +6 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-apl8/igt@i915_suspend@debugfs-reader.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-apl8/igt@i915_suspend@debugfs-reader.html

  * igt@kms_atomic_transition@2x-modeset-transitions-nonblocking-fencing:
    - shard-glk:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103359] / [k.org#198133])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-glk1/igt@kms_atomic_transition@2x-modeset-transitions-nonblocking-fencing.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-glk6/igt@kms_atomic_transition@2x-modeset-transitions-nonblocking-fencing.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#108145])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_setmode@basic:
    - shard-hsw:          [PASS][11] -> [FAIL][12] ([fdo#99912])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-hsw1/igt@kms_setmode@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-hsw5/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries_display_off:
    - shard-skl:          [INCOMPLETE][13] ([fdo#104108]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl8/igt@debugfs_test@read_all_entries_display_off.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl8/igt@debugfs_test@read_all_entries_display_off.html

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-skl:          [INCOMPLETE][15] ([fdo#107803] / [fdo#107807]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl4/igt@i915_pm_rpm@gem-execbuf-stress.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl9/igt@i915_pm_rpm@gem-execbuf-stress.html

  * igt@i915_pm_rpm@gem-idle:
    - shard-skl:          [INCOMPLETE][17] ([fdo#107807]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl5/igt@i915_pm_rpm@gem-idle.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl10/igt@i915_pm_rpm@gem-idle.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][19] ([fdo#108566]) -> [PASS][20] +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          [FAIL][21] ([fdo#106509] / [fdo#107409]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-glk8/igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-glk8/igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-hsw:          [SKIP][23] ([fdo#109271]) -> [PASS][24] +20 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-hsw1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-hsw6/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@2x-modeset-vs-vblank-race:
    - shard-glk:          [FAIL][25] ([fdo#103060]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-glk1/igt@kms_flip@2x-modeset-vs-vblank-race.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-glk6/igt@kms_flip@2x-modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-skl:          [FAIL][27] ([fdo#103167]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-wc.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][29] ([fdo#108145]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_setmode@basic:
    - shard-skl:          [FAIL][31] ([fdo#99912]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6141/shard-skl7/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/shard-skl7/igt@kms_setmode@basic.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-iclb 


Build changes
-------------

  * Linux: CI_DRM_6141 -> Patchwork_13092

  CI_DRM_6141: e94845147cc0346c3a9114d5359b188008daff9d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5015: cdd6b0a7630762cec14596b9863f418b48c32f46 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13092: 1b9826e9f3ff6cb1d304f43c6a05e4700c02f97c @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13092/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 0/5] Refactor to expand subslice mask

[Manasi Navare]

Pushed to dinq, thanks for the patches and the reviews!

Regards
Manasi

On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
> This patch series contains a few code clean-up patches, followed
> by a patch which changes the storage of the subslice mask to better
> match the userspace access through the I915_QUERY_TOPOLOGY_INFO
> ioctl. The index into the subslice_mask array is then calculated:
>   slice * subslice stride + subslice index / 8
> 
> v2: fix i915_pm_sseu test failure
> v3: no changes to patches in the series, just resending to pick up
>     in CI correctly
> v4: rebase
> v5: fix header test
> v6: address review comments from Jari
>     address minor checkpatch warning in existing code
>     use eu_stride for EU div-by-8
> v7: another rebase
> v8: address review comments from Tvrtko and Daniele
> v9: address review comments from Daniele
> v10: add reviewed-by on last patch with minor suggested change,
>      rebase, and repost for CI
> 
> Stuart Summers (5):
>   drm/i915: Use local variable for SSEU info in GETPARAM ioctl
>   drm/i915: Add macro for SSEU stride calculation
>   drm/i915: Move calculation of subslices per slice to new function
>   drm/i915: Refactor sseu helper functions
>   drm/i915: Expand subslice mask
> 
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
>  drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
>  drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
>  drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
>  drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
>  drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
>  drivers/gpu/drm/i915/i915_drv.c              |  15 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
>  drivers/gpu/drm/i915/i915_query.c            |  15 +-
>  drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--------
>  drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
>  12 files changed, 280 insertions(+), 180 deletions(-)
> 
> -- 
> 2.21.0.5.gaeb582a983
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 0/5] Refactor to expand subslice mask

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 2547 bytes --]

On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
> Pushed to dinq, thanks for the patches and the reviews!

Thanks for the push Manasi and the reviews Daniele and others!

-Stuart

> 
> Regards
> Manasi
> 
> On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
> > This patch series contains a few code clean-up patches, followed
> > by a patch which changes the storage of the subslice mask to better
> > match the userspace access through the I915_QUERY_TOPOLOGY_INFO
> > ioctl. The index into the subslice_mask array is then calculated:
> >   slice * subslice stride + subslice index / 8
> > 
> > v2: fix i915_pm_sseu test failure
> > v3: no changes to patches in the series, just resending to pick up
> >     in CI correctly
> > v4: rebase
> > v5: fix header test
> > v6: address review comments from Jari
> >     address minor checkpatch warning in existing code
> >     use eu_stride for EU div-by-8
> > v7: another rebase
> > v8: address review comments from Tvrtko and Daniele
> > v9: address review comments from Daniele
> > v10: add reviewed-by on last patch with minor suggested change,
> >      rebase, and repost for CI
> > 
> > Stuart Summers (5):
> >   drm/i915: Use local variable for SSEU info in GETPARAM ioctl
> >   drm/i915: Add macro for SSEU stride calculation
> >   drm/i915: Move calculation of subslices per slice to new function
> >   drm/i915: Refactor sseu helper functions
> >   drm/i915: Expand subslice mask
> > 
> >  drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
> >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
> >  drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
> >  drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
> >  drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
> >  drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
> >  drivers/gpu/drm/i915/i915_drv.c              |  15 +-
> >  drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
> >  drivers/gpu/drm/i915/i915_query.c            |  15 +-
> >  drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--
> > ------
> >  drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
> >  12 files changed, 280 insertions(+), 180 deletions(-)
> > 
> > -- 
> > 2.21.0.5.gaeb582a983
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [CI 0/5] Refactor to expand subslice mask

[Saarinen, Jani]

Hi, 

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Summers, Stuart
> Sent: tiistai 28. toukokuuta 2019 21.33
> To: Navare, Manasi D <manasi.d.navare@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
> 
> On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
> > Pushed to dinq, thanks for the patches and the reviews!
> 
> Thanks for the push Manasi and the reviews Daniele and others!
This broke all the ICL systems because CI data was not looked that they did not actually even boot at all.
All ICL's in BAT and whole ICL shards. 


> 
> -Stuart
> 
> >
> > Regards
> > Manasi
> >
> > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
> > > This patch series contains a few code clean-up patches, followed by
> > > a patch which changes the storage of the subslice mask to better
> > > match the userspace access through the I915_QUERY_TOPOLOGY_INFO
> > > ioctl. The index into the subslice_mask array is then calculated:
> > >   slice * subslice stride + subslice index / 8
> > >
> > > v2: fix i915_pm_sseu test failure
> > > v3: no changes to patches in the series, just resending to pick up
> > >     in CI correctly
> > > v4: rebase
> > > v5: fix header test
> > > v6: address review comments from Jari
> > >     address minor checkpatch warning in existing code
> > >     use eu_stride for EU div-by-8
> > > v7: another rebase
> > > v8: address review comments from Tvrtko and Daniele
> > > v9: address review comments from Daniele
> > > v10: add reviewed-by on last patch with minor suggested change,
> > >      rebase, and repost for CI
> > >
> > > Stuart Summers (5):
> > >   drm/i915: Use local variable for SSEU info in GETPARAM ioctl
> > >   drm/i915: Add macro for SSEU stride calculation
> > >   drm/i915: Move calculation of subslices per slice to new function
> > >   drm/i915: Refactor sseu helper functions
> > >   drm/i915: Expand subslice mask
> > >
> > >  drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
> > >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
> > >  drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
> > >  drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
> > >  drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
> > >  drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
> > >  drivers/gpu/drm/i915/i915_drv.c              |  15 +-
> > >  drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
> > >  drivers/gpu/drm/i915/i915_query.c            |  15 +-
> > >  drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--
> > > ------
> > >  drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
> > >  12 files changed, 280 insertions(+), 180 deletions(-)
> > >
> > > --
> > > 2.21.0.5.gaeb582a983
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI,5/5] drm/i915: Expand subslice mask

[Nathan Chancellor]

Hi Stuart,

On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote:
> Currently, the subslice_mask runtime parameter is stored as an
> array of subslices per slice. Expand the subslice mask array to
> better match what is presented to userspace through the
> I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
> then calculated:
>   slice * subslice stride + subslice index / 8
> 
> v2: fix spacing in set_sseu_info args
>     use set_sseu_info to initialize sseu data when building
>     device status in debugfs
>     rename variables in intel_engine_types.h to avoid checkpatch
>     warnings
> v3: update headers in intel_sseu.h
> v4: add const to some sseu_dev_info variables
>     use sseu->eu_stride for EU stride calculations
> v5: address review comments from Tvrtko and Daniele
> v6: remove extra space in intel_sseu_get_subslices
>     return the correct subslice enable in for_each_instdone
>     add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size
>     use printk formatted string for subslice mask
> v7: remove string.h header and rebase
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---

<snip>

> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 97f742530fa1..3625f777f3a3 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -92,9 +92,9 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>  		   hweight8(sseu->slice_mask), sseu->slice_mask);
>  	drm_printf(p, "subslice total: %u\n", intel_sseu_subslice_total(sseu));
>  	for (s = 0; s < sseu->max_slices; s++) {
> -		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
> +		drm_printf(p, "slice%d: %u subslices, mask=%08x\n",
>  			   s, intel_sseu_subslices_per_slice(sseu, s),
> -			   sseu->subslice_mask[s]);
> +			   intel_sseu_get_subslices(sseu, s));
>  	}
>  	drm_printf(p, "EU total: %u\n", sseu->eu_total);
>  	drm_printf(p, "EU per subslice: %u\n", sseu->eu_per_subslice);
> @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info,
>  static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice,
>  		       int subslice)
>  {
> -	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
> -	int slice_stride = sseu->max_subslices * subslice_stride;
> +	int slice_stride = sseu->max_subslices * sseu->eu_stride;
>  
> -	return slice * slice_stride + subslice * subslice_stride;
> +	return slice * slice_stride + subslice * sseu->eu_stride;
>  }
>  
>  static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
> @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
>  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
>  	u16 eu_mask = 0;
>  
> -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
> +	for (i = 0; i < sseu->eu_stride; i++) {
>  		eu_mask |= ((u16)sseu->eu_mask[offset + i]) <<
>  			(i * BITS_PER_BYTE);
>  	}
> @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice,
>  {
>  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
>  
> -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
> +	for (i = 0; i < sseu->eu_stride; i++) {
>  		sseu->eu_mask[offset + i] =
>  			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
>  	}
> @@ -159,9 +158,9 @@ void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
>  	}
>  
>  	for (s = 0; s < sseu->max_slices; s++) {
> -		drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n",
> +		drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n",
>  			   s, intel_sseu_subslices_per_slice(sseu, s),
> -			   sseu->subslice_mask[s]);
> +			   intel_sseu_get_subslices(sseu, s));
>  
>  		for (ss = 0; ss < sseu->max_subslices; ss++) {
>  			u16 enabled_eus = sseu_get_eus(sseu, s, ss);
> @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
>  	u8 eu_en;
>  	int s;
>  
> -	if (IS_ELKHARTLAKE(dev_priv)) {
> -		sseu->max_slices = 1;
> -		sseu->max_subslices = 4;
> -		sseu->max_eus_per_subslice = 8;
> -	} else {
> -		sseu->max_slices = 1;
> -		sseu->max_subslices = 8;
> -		sseu->max_eus_per_subslice = 8;
> -	}
> +	if (IS_ELKHARTLAKE(dev_priv))
> +		intel_sseu_set_info(sseu, 1, 4, 8);
> +	else
> +		intel_sseu_set_info(sseu, 1, 8, 8);
>  
>  	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
>  	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
> @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
>  
>  	for (s = 0; s < sseu->max_slices; s++) {
>  		if (s_en & BIT(s)) {
> -			int ss_idx = sseu->max_subslices * s;
>  			int ss;
>  
>  			sseu->slice_mask |= BIT(s);
> -			sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask;
> -			for (ss = 0; ss < sseu->max_subslices; ss++) {
> -				if (sseu->subslice_mask[s] & BIT(ss))
> +
> +			intel_sseu_set_subslices(sseu, s, ss_en_mask);
> +
> +			for (ss = 0; ss < sseu->max_subslices; ss++)
> +				if (intel_sseu_has_subslice(sseu, s, ss))
>  					sseu_set_eus(sseu, s, ss, eu_en);
> -			}
>  		}
>  	}
>  	sseu->eu_per_subslice = hweight8(eu_en);
> @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
>  	const int eu_mask = 0xff;
>  	u32 subslice_mask, eu_en;
>  
> +	intel_sseu_set_info(sseu, 6, 4, 8);
> +
>  	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
>  			    GEN10_F2_S_ENA_SHIFT;
> -	sseu->max_slices = 6;
> -	sseu->max_subslices = 4;
> -	sseu->max_eus_per_subslice = 8;
> -
> -	subslice_mask = (1 << 4) - 1;
> -	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
> -			   GEN10_F2_SS_DIS_SHIFT);
> -
> -	/*
> -	 * Slice0 can have up to 3 subslices, but there are only 2 in
> -	 * slice1/2.
> -	 */
> -	sseu->subslice_mask[0] = subslice_mask;
> -	for (s = 1; s < sseu->max_slices; s++)
> -		sseu->subslice_mask[s] = subslice_mask & 0x3;
>  
>  	/* Slice0 */
>  	eu_en = ~I915_READ(GEN8_EU_DISABLE0);
> @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
>  	eu_en = ~I915_READ(GEN10_EU_DISABLE3);
>  	sseu_set_eus(sseu, 5, 1, eu_en & eu_mask);
>  
> -	/* Do a second pass where we mark the subslices disabled if all their
> -	 * eus are off.
> -	 */
> +	subslice_mask = (1 << 4) - 1;
> +	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
> +			   GEN10_F2_SS_DIS_SHIFT);
> +
>  	for (s = 0; s < sseu->max_slices; s++) {
>  		for (ss = 0; ss < sseu->max_subslices; ss++) {
>  			if (sseu_get_eus(sseu, s, ss) == 0)
> -				sseu->subslice_mask[s] &= ~BIT(ss);
> +				subslice_mask &= ~BIT(ss);
>  		}
> +
> +		/*
> +		 * Slice0 can have up to 3 subslices, but there are only 2 in
> +		 * slice1/2.
> +		 */
> +		intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask :
> +							   subslice_mask & 0x3);
>  	}
>  
>  	sseu->eu_total = compute_eu_total(sseu);
> @@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
>  {
>  	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
>  	u32 fuse;
> +	u8 subslice_mask;
>  
>  	fuse = I915_READ(CHV_FUSE_GT);
>  
>  	sseu->slice_mask = BIT(0);
> -	sseu->max_slices = 1;
> -	sseu->max_subslices = 2;
> -	sseu->max_eus_per_subslice = 8;
> +	intel_sseu_set_info(sseu, 1, 2, 8);
>  
>  	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
>  		u8 disabled_mask =
> @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
>  			(((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >>
>  			  CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4);
>  
> -		sseu->subslice_mask[0] |= BIT(0);
> +		subslice_mask |= BIT(0);

When building with -Wuninitialized, clang warns:

drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable 'subslice_mask' is uninitialized when used here [-Wuninitialized]
                subslice_mask |= BIT(0);
                ^~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize the variable 'subslice_mask' to silence this warning
        u8 subslice_mask;
                        ^
                         = '\0'

I assume that it should be initialized to zero but maybe you intended
something different?

Thanks,
Nathan
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 0/5] Refactor to expand subslice mask

[Daniele Ceraolo Spurio]

On 5/28/19 11:48 PM, Saarinen, Jani wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Summers, Stuart
>> Sent: tiistai 28. toukokuuta 2019 21.33
>> To: Navare, Manasi D <manasi.d.navare@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
>>
>> On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
>>> Pushed to dinq, thanks for the patches and the reviews!
>>
>> Thanks for the push Manasi and the reviews Daniele and others!
> This broke all the ICL systems because CI data was not looked that they did not actually even boot at all.
> All ICL's in BAT and whole ICL shards.
> 

Can we change the CI reply for the case where there are extra missing 
machines compared to the reference run from SUCCESS to WARNING or 
something like that, so people have a clearer indication that something 
might have gone wrong?

Daniele

> 
>>
>> -Stuart
>>
>>>
>>> Regards
>>> Manasi
>>>
>>> On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
>>>> This patch series contains a few code clean-up patches, followed by
>>>> a patch which changes the storage of the subslice mask to better
>>>> match the userspace access through the I915_QUERY_TOPOLOGY_INFO
>>>> ioctl. The index into the subslice_mask array is then calculated:
>>>>    slice * subslice stride + subslice index / 8
>>>>
>>>> v2: fix i915_pm_sseu test failure
>>>> v3: no changes to patches in the series, just resending to pick up
>>>>      in CI correctly
>>>> v4: rebase
>>>> v5: fix header test
>>>> v6: address review comments from Jari
>>>>      address minor checkpatch warning in existing code
>>>>      use eu_stride for EU div-by-8
>>>> v7: another rebase
>>>> v8: address review comments from Tvrtko and Daniele
>>>> v9: address review comments from Daniele
>>>> v10: add reviewed-by on last patch with minor suggested change,
>>>>       rebase, and repost for CI
>>>>
>>>> Stuart Summers (5):
>>>>    drm/i915: Use local variable for SSEU info in GETPARAM ioctl
>>>>    drm/i915: Add macro for SSEU stride calculation
>>>>    drm/i915: Move calculation of subslices per slice to new function
>>>>    drm/i915: Refactor sseu helper functions
>>>>    drm/i915: Expand subslice mask
>>>>
>>>>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
>>>>   drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
>>>>   drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
>>>>   drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
>>>>   drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
>>>>   drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
>>>>   drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
>>>>   drivers/gpu/drm/i915/i915_drv.c              |  15 +-
>>>>   drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
>>>>   drivers/gpu/drm/i915/i915_query.c            |  15 +-
>>>>   drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--
>>>> ------
>>>>   drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
>>>>   12 files changed, 280 insertions(+), 180 deletions(-)
>>>>
>>>> --
>>>> 2.21.0.5.gaeb582a983
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 0/5] Refactor to expand subslice mask

[Jani Nikula]

On Wed, 29 May 2019, "Saarinen, Jani" <jani.saarinen@intel.com> wrote:
> Hi, 
>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Summers, Stuart
>> Sent: tiistai 28. toukokuuta 2019 21.33
>> To: Navare, Manasi D <manasi.d.navare@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
>> 
>> On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
>> > Pushed to dinq, thanks for the patches and the reviews!
>> 
>> Thanks for the push Manasi and the reviews Daniele and others!
> This broke all the ICL systems because CI data was not looked that they did not actually even boot at all.
> All ICL's in BAT and whole ICL shards. 

Commit 1ac159e23c2c ("drm/i915: Expand subslice mask") has now been
reverted and we can carry on.

As discussed, I don't really expect people to dig into the logs after
seeing "Fi.CI.IGT: success". For one I didn't expect patches to be able
to take down machines with just a fairly silent "Participating hosts (10
-> 9)" message. But that discussion is for another forum, another time.

BR,
Jani.


>
>
>> 
>> -Stuart
>> 
>> >
>> > Regards
>> > Manasi
>> >
>> > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
>> > > This patch series contains a few code clean-up patches, followed by
>> > > a patch which changes the storage of the subslice mask to better
>> > > match the userspace access through the I915_QUERY_TOPOLOGY_INFO
>> > > ioctl. The index into the subslice_mask array is then calculated:
>> > >   slice * subslice stride + subslice index / 8
>> > >
>> > > v2: fix i915_pm_sseu test failure
>> > > v3: no changes to patches in the series, just resending to pick up
>> > >     in CI correctly
>> > > v4: rebase
>> > > v5: fix header test
>> > > v6: address review comments from Jari
>> > >     address minor checkpatch warning in existing code
>> > >     use eu_stride for EU div-by-8
>> > > v7: another rebase
>> > > v8: address review comments from Tvrtko and Daniele
>> > > v9: address review comments from Daniele
>> > > v10: add reviewed-by on last patch with minor suggested change,
>> > >      rebase, and repost for CI
>> > >
>> > > Stuart Summers (5):
>> > >   drm/i915: Use local variable for SSEU info in GETPARAM ioctl
>> > >   drm/i915: Add macro for SSEU stride calculation
>> > >   drm/i915: Move calculation of subslices per slice to new function
>> > >   drm/i915: Refactor sseu helper functions
>> > >   drm/i915: Expand subslice mask
>> > >
>> > >  drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
>> > >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
>> > >  drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
>> > >  drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
>> > >  drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
>> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
>> > >  drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
>> > >  drivers/gpu/drm/i915/i915_drv.c              |  15 +-
>> > >  drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
>> > >  drivers/gpu/drm/i915/i915_query.c            |  15 +-
>> > >  drivers/gpu/drm/i915/intel_device_info.c     | 176 +++++++++++--
>> > > ------
>> > >  drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
>> > >  12 files changed, 280 insertions(+), 180 deletions(-)
>> > >
>> > > --
>> > > 2.21.0.5.gaeb582a983
>> > >
>> > > _______________________________________________
>> > > Intel-gfx mailing list
>> > > Intel-gfx@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI,5/5] drm/i915: Expand subslice mask

[Jani Nikula]

On Wed, 29 May 2019, Nathan Chancellor <natechancellor@gmail.com> wrote:
> Hi Stuart,
>
> On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote:
>> Currently, the subslice_mask runtime parameter is stored as an
>> array of subslices per slice. Expand the subslice mask array to
>> better match what is presented to userspace through the
>> I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
>> then calculated:
>>   slice * subslice stride + subslice index / 8
>> 
>> v2: fix spacing in set_sseu_info args
>>     use set_sseu_info to initialize sseu data when building
>>     device status in debugfs
>>     rename variables in intel_engine_types.h to avoid checkpatch
>>     warnings
>> v3: update headers in intel_sseu.h
>> v4: add const to some sseu_dev_info variables
>>     use sseu->eu_stride for EU stride calculations
>> v5: address review comments from Tvrtko and Daniele
>> v6: remove extra space in intel_sseu_get_subslices
>>     return the correct subslice enable in for_each_instdone
>>     add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size
>>     use printk formatted string for subslice mask
>> v7: remove string.h header and rebase
>> 
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
>> ---
>
> <snip>
>
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index 97f742530fa1..3625f777f3a3 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -92,9 +92,9 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>>  		   hweight8(sseu->slice_mask), sseu->slice_mask);
>>  	drm_printf(p, "subslice total: %u\n", intel_sseu_subslice_total(sseu));
>>  	for (s = 0; s < sseu->max_slices; s++) {
>> -		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
>> +		drm_printf(p, "slice%d: %u subslices, mask=%08x\n",
>>  			   s, intel_sseu_subslices_per_slice(sseu, s),
>> -			   sseu->subslice_mask[s]);
>> +			   intel_sseu_get_subslices(sseu, s));
>>  	}
>>  	drm_printf(p, "EU total: %u\n", sseu->eu_total);
>>  	drm_printf(p, "EU per subslice: %u\n", sseu->eu_per_subslice);
>> @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const struct intel_runtime_info *info,
>>  static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice,
>>  		       int subslice)
>>  {
>> -	int subslice_stride = GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
>> -	int slice_stride = sseu->max_subslices * subslice_stride;
>> +	int slice_stride = sseu->max_subslices * sseu->eu_stride;
>>  
>> -	return slice * slice_stride + subslice * subslice_stride;
>> +	return slice * slice_stride + subslice * sseu->eu_stride;
>>  }
>>  
>>  static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
>> @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int slice,
>>  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
>>  	u16 eu_mask = 0;
>>  
>> -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
>> +	for (i = 0; i < sseu->eu_stride; i++) {
>>  		eu_mask |= ((u16)sseu->eu_mask[offset + i]) <<
>>  			(i * BITS_PER_BYTE);
>>  	}
>> @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info *sseu, int slice, int subslice,
>>  {
>>  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
>>  
>> -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice); i++) {
>> +	for (i = 0; i < sseu->eu_stride; i++) {
>>  		sseu->eu_mask[offset + i] =
>>  			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
>>  	}
>> @@ -159,9 +158,9 @@ void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
>>  	}
>>  
>>  	for (s = 0; s < sseu->max_slices; s++) {
>> -		drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n",
>> +		drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n",
>>  			   s, intel_sseu_subslices_per_slice(sseu, s),
>> -			   sseu->subslice_mask[s]);
>> +			   intel_sseu_get_subslices(sseu, s));
>>  
>>  		for (ss = 0; ss < sseu->max_subslices; ss++) {
>>  			u16 enabled_eus = sseu_get_eus(sseu, s, ss);
>> @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
>>  	u8 eu_en;
>>  	int s;
>>  
>> -	if (IS_ELKHARTLAKE(dev_priv)) {
>> -		sseu->max_slices = 1;
>> -		sseu->max_subslices = 4;
>> -		sseu->max_eus_per_subslice = 8;
>> -	} else {
>> -		sseu->max_slices = 1;
>> -		sseu->max_subslices = 8;
>> -		sseu->max_eus_per_subslice = 8;
>> -	}
>> +	if (IS_ELKHARTLAKE(dev_priv))
>> +		intel_sseu_set_info(sseu, 1, 4, 8);
>> +	else
>> +		intel_sseu_set_info(sseu, 1, 8, 8);
>>  
>>  	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
>>  	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
>> @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct drm_i915_private *dev_priv)
>>  
>>  	for (s = 0; s < sseu->max_slices; s++) {
>>  		if (s_en & BIT(s)) {
>> -			int ss_idx = sseu->max_subslices * s;
>>  			int ss;
>>  
>>  			sseu->slice_mask |= BIT(s);
>> -			sseu->subslice_mask[s] = (ss_en >> ss_idx) & ss_en_mask;
>> -			for (ss = 0; ss < sseu->max_subslices; ss++) {
>> -				if (sseu->subslice_mask[s] & BIT(ss))
>> +
>> +			intel_sseu_set_subslices(sseu, s, ss_en_mask);
>> +
>> +			for (ss = 0; ss < sseu->max_subslices; ss++)
>> +				if (intel_sseu_has_subslice(sseu, s, ss))
>>  					sseu_set_eus(sseu, s, ss, eu_en);
>> -			}
>>  		}
>>  	}
>>  	sseu->eu_per_subslice = hweight8(eu_en);
>> @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
>>  	const int eu_mask = 0xff;
>>  	u32 subslice_mask, eu_en;
>>  
>> +	intel_sseu_set_info(sseu, 6, 4, 8);
>> +
>>  	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
>>  			    GEN10_F2_S_ENA_SHIFT;
>> -	sseu->max_slices = 6;
>> -	sseu->max_subslices = 4;
>> -	sseu->max_eus_per_subslice = 8;
>> -
>> -	subslice_mask = (1 << 4) - 1;
>> -	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
>> -			   GEN10_F2_SS_DIS_SHIFT);
>> -
>> -	/*
>> -	 * Slice0 can have up to 3 subslices, but there are only 2 in
>> -	 * slice1/2.
>> -	 */
>> -	sseu->subslice_mask[0] = subslice_mask;
>> -	for (s = 1; s < sseu->max_slices; s++)
>> -		sseu->subslice_mask[s] = subslice_mask & 0x3;
>>  
>>  	/* Slice0 */
>>  	eu_en = ~I915_READ(GEN8_EU_DISABLE0);
>> @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct drm_i915_private *dev_priv)
>>  	eu_en = ~I915_READ(GEN10_EU_DISABLE3);
>>  	sseu_set_eus(sseu, 5, 1, eu_en & eu_mask);
>>  
>> -	/* Do a second pass where we mark the subslices disabled if all their
>> -	 * eus are off.
>> -	 */
>> +	subslice_mask = (1 << 4) - 1;
>> +	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
>> +			   GEN10_F2_SS_DIS_SHIFT);
>> +
>>  	for (s = 0; s < sseu->max_slices; s++) {
>>  		for (ss = 0; ss < sseu->max_subslices; ss++) {
>>  			if (sseu_get_eus(sseu, s, ss) == 0)
>> -				sseu->subslice_mask[s] &= ~BIT(ss);
>> +				subslice_mask &= ~BIT(ss);
>>  		}
>> +
>> +		/*
>> +		 * Slice0 can have up to 3 subslices, but there are only 2 in
>> +		 * slice1/2.
>> +		 */
>> +		intel_sseu_set_subslices(sseu, s, s == 0 ? subslice_mask :
>> +							   subslice_mask & 0x3);
>>  	}
>>  
>>  	sseu->eu_total = compute_eu_total(sseu);
>> @@ -309,13 +298,12 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
>>  {
>>  	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
>>  	u32 fuse;
>> +	u8 subslice_mask;
>>  
>>  	fuse = I915_READ(CHV_FUSE_GT);
>>  
>>  	sseu->slice_mask = BIT(0);
>> -	sseu->max_slices = 1;
>> -	sseu->max_subslices = 2;
>> -	sseu->max_eus_per_subslice = 8;
>> +	intel_sseu_set_info(sseu, 1, 2, 8);
>>  
>>  	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
>>  		u8 disabled_mask =
>> @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
>>  			(((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >>
>>  			  CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4);
>>  
>> -		sseu->subslice_mask[0] |= BIT(0);
>> +		subslice_mask |= BIT(0);
>
> When building with -Wuninitialized, clang warns:
>
> drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable 'subslice_mask' is uninitialized when used here [-Wuninitialized]
>                 subslice_mask |= BIT(0);
>                 ^~~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize the variable 'subslice_mask' to silence this warning
>         u8 subslice_mask;
>                         ^
>                          = '\0'
>
> I assume that it should be initialized to zero but maybe you intended
> something different?

As it happens, the commit has been reverted for other reasons, and we
(the royal we, I really mean Stuart) can fix this while at it. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 5/5] drm/i915: Expand subslice mask

[Jani Nikula]

On Fri, 24 May 2019, Stuart Summers <stuart.summers@intel.com> wrote:
> Currently, the subslice_mask runtime parameter is stored as an
> array of subslices per slice. Expand the subslice mask array to
> better match what is presented to userspace through the
> I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
> then calculated:
>   slice * subslice stride + subslice index / 8
>
> v2: fix spacing in set_sseu_info args
>     use set_sseu_info to initialize sseu data when building
>     device status in debugfs
>     rename variables in intel_engine_types.h to avoid checkpatch
>     warnings
> v3: update headers in intel_sseu.h
> v4: add const to some sseu_dev_info variables
>     use sseu->eu_stride for EU stride calculations
> v5: address review comments from Tvrtko and Daniele
> v6: remove extra space in intel_sseu_get_subslices
>     return the correct subslice enable in for_each_instdone
>     add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size
>     use printk formatted string for subslice mask
> v7: remove string.h header and rebase
>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>

As this patch uncovered a latent issue in 1e40d4aea57b ("drm/i915/cnl:
Implement WaProgramMgsrForCorrectSliceSpecificMmioReads") and got
reverted, I'll take the opportunity to comment. I acknowledge the revert
is shooting the messenger a bit, and this will smell like maintainer
bikeshedding.

Now, the first reaction looking at the commit was, it does not fare well
on the "if a bisect landed on this commit, how happy would I be" scale.

While it's mostly refactoring, it could be chopped up to several logical
and obvious steps. For example, add intel_sseu_set_info() first with no
other changes. Add ss_stride and eu_stride to struct sseu_dev_info
separately. Add intel_sseu_get_subslices() but don't expand yet, make it
just sseu->subslice_mask[s] first. And so on, you get the idea, a series
of small non-functional changes followed by patches with functional
changes that stand out. Indeed patches 1-4 did this fine.

It's easy on the reviewer, it's easy on whoever git blames years down
the line. Trust me, we will.

And it would be the commit adding intel_sseu_get_subslices(), or the one
adding the GEM_BUG_ON()s into it, that would blow up 1e40d4aea57b
("drm/i915/cnl: Implement
WaProgramMgsrForCorrectSliceSpecificMmioReads").

One more note below.

> @@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
>  			return -ENODEV;
>  		break;
>  	case I915_PARAM_SUBSLICE_MASK:
> -		value = sseu->subslice_mask[0];
> +		/* Only copy bits from the first slice */
> +		memcpy(&value, sseu->subslice_mask,
> +		       min(sseu->ss_stride, (u8)sizeof(value)));

Frankly I'd rather see this written in self-evident code without the
comment.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI,5/5] drm/i915: Expand subslice mask

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 10187 bytes --]

On Wed, 2019-05-29 at 17:33 +0300, Jani Nikula wrote:
> On Wed, 29 May 2019, Nathan Chancellor <natechancellor@gmail.com>
> wrote:
> > Hi Stuart,
> > 
> > On Fri, May 24, 2019 at 08:40:22AM -0700, Stuart Summers wrote:
> > > Currently, the subslice_mask runtime parameter is stored as an
> > > array of subslices per slice. Expand the subslice mask array to
> > > better match what is presented to userspace through the
> > > I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
> > > then calculated:
> > >   slice * subslice stride + subslice index / 8
> > > 
> > > v2: fix spacing in set_sseu_info args
> > >     use set_sseu_info to initialize sseu data when building
> > >     device status in debugfs
> > >     rename variables in intel_engine_types.h to avoid checkpatch
> > >     warnings
> > > v3: update headers in intel_sseu.h
> > > v4: add const to some sseu_dev_info variables
> > >     use sseu->eu_stride for EU stride calculations
> > > v5: address review comments from Tvrtko and Daniele
> > > v6: remove extra space in intel_sseu_get_subslices
> > >     return the correct subslice enable in for_each_instdone
> > >     add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask
> > > size
> > >     use printk formatted string for subslice mask
> > > v7: remove string.h header and rebase
> > > 
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > > Reviewed-by: Daniele Ceraolo Spurio <
> > > daniele.ceraolospurio@intel.com>
> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > > ---
> > 
> > <snip>
> > 
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > > b/drivers/gpu/drm/i915/intel_device_info.c
> > > index 97f742530fa1..3625f777f3a3 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > @@ -92,9 +92,9 @@ static void sseu_dump(const struct
> > > sseu_dev_info *sseu, struct drm_printer *p)
> > >  		   hweight8(sseu->slice_mask), sseu->slice_mask);
> > >  	drm_printf(p, "subslice total: %u\n",
> > > intel_sseu_subslice_total(sseu));
> > >  	for (s = 0; s < sseu->max_slices; s++) {
> > > -		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
> > > +		drm_printf(p, "slice%d: %u subslices, mask=%08x\n",
> > >  			   s, intel_sseu_subslices_per_slice(sseu, s),
> > > -			   sseu->subslice_mask[s]);
> > > +			   intel_sseu_get_subslices(sseu, s));
> > >  	}
> > >  	drm_printf(p, "EU total: %u\n", sseu->eu_total);
> > >  	drm_printf(p, "EU per subslice: %u\n", sseu->eu_per_subslice);
> > > @@ -117,10 +117,9 @@ void intel_device_info_dump_runtime(const
> > > struct intel_runtime_info *info,
> > >  static int sseu_eu_idx(const struct sseu_dev_info *sseu, int
> > > slice,
> > >  		       int subslice)
> > >  {
> > > -	int subslice_stride = GEN_SSEU_STRIDE(sseu-
> > > >max_eus_per_subslice);
> > > -	int slice_stride = sseu->max_subslices * subslice_stride;
> > > +	int slice_stride = sseu->max_subslices * sseu->eu_stride;
> > >  
> > > -	return slice * slice_stride + subslice * subslice_stride;
> > > +	return slice * slice_stride + subslice * sseu->eu_stride;
> > >  }
> > >  
> > >  static u16 sseu_get_eus(const struct sseu_dev_info *sseu, int
> > > slice,
> > > @@ -129,7 +128,7 @@ static u16 sseu_get_eus(const struct
> > > sseu_dev_info *sseu, int slice,
> > >  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
> > >  	u16 eu_mask = 0;
> > >  
> > > -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
> > > i++) {
> > > +	for (i = 0; i < sseu->eu_stride; i++) {
> > >  		eu_mask |= ((u16)sseu->eu_mask[offset + i]) <<
> > >  			(i * BITS_PER_BYTE);
> > >  	}
> > > @@ -142,7 +141,7 @@ static void sseu_set_eus(struct sseu_dev_info
> > > *sseu, int slice, int subslice,
> > >  {
> > >  	int i, offset = sseu_eu_idx(sseu, slice, subslice);
> > >  
> > > -	for (i = 0; i < GEN_SSEU_STRIDE(sseu->max_eus_per_subslice);
> > > i++) {
> > > +	for (i = 0; i < sseu->eu_stride; i++) {
> > >  		sseu->eu_mask[offset + i] =
> > >  			(eu_mask >> (BITS_PER_BYTE * i)) & 0xff;
> > >  	}
> > > @@ -159,9 +158,9 @@ void intel_device_info_dump_topology(const
> > > struct sseu_dev_info *sseu,
> > >  	}
> > >  
> > >  	for (s = 0; s < sseu->max_slices; s++) {
> > > -		drm_printf(p, "slice%d: %u subslice(s) (0x%hhx):\n",
> > > +		drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n",
> > >  			   s, intel_sseu_subslices_per_slice(sseu, s),
> > > -			   sseu->subslice_mask[s]);
> > > +			   intel_sseu_get_subslices(sseu, s));
> > >  
> > >  		for (ss = 0; ss < sseu->max_subslices; ss++) {
> > >  			u16 enabled_eus = sseu_get_eus(sseu, s, ss);
> > > @@ -190,15 +189,10 @@ static void gen11_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  	u8 eu_en;
> > >  	int s;
> > >  
> > > -	if (IS_ELKHARTLAKE(dev_priv)) {
> > > -		sseu->max_slices = 1;
> > > -		sseu->max_subslices = 4;
> > > -		sseu->max_eus_per_subslice = 8;
> > > -	} else {
> > > -		sseu->max_slices = 1;
> > > -		sseu->max_subslices = 8;
> > > -		sseu->max_eus_per_subslice = 8;
> > > -	}
> > > +	if (IS_ELKHARTLAKE(dev_priv))
> > > +		intel_sseu_set_info(sseu, 1, 4, 8);
> > > +	else
> > > +		intel_sseu_set_info(sseu, 1, 8, 8);
> > >  
> > >  	s_en = I915_READ(GEN11_GT_SLICE_ENABLE) & GEN11_GT_S_ENA_MASK;
> > >  	ss_en = ~I915_READ(GEN11_GT_SUBSLICE_DISABLE);
> > > @@ -207,15 +201,15 @@ static void gen11_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  
> > >  	for (s = 0; s < sseu->max_slices; s++) {
> > >  		if (s_en & BIT(s)) {
> > > -			int ss_idx = sseu->max_subslices * s;
> > >  			int ss;
> > >  
> > >  			sseu->slice_mask |= BIT(s);
> > > -			sseu->subslice_mask[s] = (ss_en >> ss_idx) &
> > > ss_en_mask;
> > > -			for (ss = 0; ss < sseu->max_subslices; ss++) {
> > > -				if (sseu->subslice_mask[s] & BIT(ss))
> > > +
> > > +			intel_sseu_set_subslices(sseu, s, ss_en_mask);
> > > +
> > > +			for (ss = 0; ss < sseu->max_subslices; ss++)
> > > +				if (intel_sseu_has_subslice(sseu, s,
> > > ss))
> > >  					sseu_set_eus(sseu, s, ss,
> > > eu_en);
> > > -			}
> > >  		}
> > >  	}
> > >  	sseu->eu_per_subslice = hweight8(eu_en);
> > > @@ -235,23 +229,10 @@ static void gen10_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  	const int eu_mask = 0xff;
> > >  	u32 subslice_mask, eu_en;
> > >  
> > > +	intel_sseu_set_info(sseu, 6, 4, 8);
> > > +
> > >  	sseu->slice_mask = (fuse2 & GEN10_F2_S_ENA_MASK) >>
> > >  			    GEN10_F2_S_ENA_SHIFT;
> > > -	sseu->max_slices = 6;
> > > -	sseu->max_subslices = 4;
> > > -	sseu->max_eus_per_subslice = 8;
> > > -
> > > -	subslice_mask = (1 << 4) - 1;
> > > -	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
> > > -			   GEN10_F2_SS_DIS_SHIFT);
> > > -
> > > -	/*
> > > -	 * Slice0 can have up to 3 subslices, but there are only 2 in
> > > -	 * slice1/2.
> > > -	 */
> > > -	sseu->subslice_mask[0] = subslice_mask;
> > > -	for (s = 1; s < sseu->max_slices; s++)
> > > -		sseu->subslice_mask[s] = subslice_mask & 0x3;
> > >  
> > >  	/* Slice0 */
> > >  	eu_en = ~I915_READ(GEN8_EU_DISABLE0);
> > > @@ -276,14 +257,22 @@ static void gen10_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  	eu_en = ~I915_READ(GEN10_EU_DISABLE3);
> > >  	sseu_set_eus(sseu, 5, 1, eu_en & eu_mask);
> > >  
> > > -	/* Do a second pass where we mark the subslices disabled if all
> > > their
> > > -	 * eus are off.
> > > -	 */
> > > +	subslice_mask = (1 << 4) - 1;
> > > +	subslice_mask &= ~((fuse2 & GEN10_F2_SS_DIS_MASK) >>
> > > +			   GEN10_F2_SS_DIS_SHIFT);
> > > +
> > >  	for (s = 0; s < sseu->max_slices; s++) {
> > >  		for (ss = 0; ss < sseu->max_subslices; ss++) {
> > >  			if (sseu_get_eus(sseu, s, ss) == 0)
> > > -				sseu->subslice_mask[s] &= ~BIT(ss);
> > > +				subslice_mask &= ~BIT(ss);
> > >  		}
> > > +
> > > +		/*
> > > +		 * Slice0 can have up to 3 subslices, but there are
> > > only 2 in
> > > +		 * slice1/2.
> > > +		 */
> > > +		intel_sseu_set_subslices(sseu, s, s == 0 ?
> > > subslice_mask :
> > > +							   subslice_mas
> > > k & 0x3);
> > >  	}
> > >  
> > >  	sseu->eu_total = compute_eu_total(sseu);
> > > @@ -309,13 +298,12 @@ static void
> > > cherryview_sseu_info_init(struct drm_i915_private *dev_priv)
> > >  {
> > >  	struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu;
> > >  	u32 fuse;
> > > +	u8 subslice_mask;
> > >  
> > >  	fuse = I915_READ(CHV_FUSE_GT);
> > >  
> > >  	sseu->slice_mask = BIT(0);
> > > -	sseu->max_slices = 1;
> > > -	sseu->max_subslices = 2;
> > > -	sseu->max_eus_per_subslice = 8;
> > > +	intel_sseu_set_info(sseu, 1, 2, 8);
> > >  
> > >  	if (!(fuse & CHV_FGT_DISABLE_SS0)) {
> > >  		u8 disabled_mask =
> > > @@ -324,7 +312,7 @@ static void cherryview_sseu_info_init(struct
> > > drm_i915_private *dev_priv)
> > >  			(((fuse & CHV_FGT_EU_DIS_SS0_R1_MASK) >>
> > >  			  CHV_FGT_EU_DIS_SS0_R1_SHIFT) << 4);
> > >  
> > > -		sseu->subslice_mask[0] |= BIT(0);
> > > +		subslice_mask |= BIT(0);
> > 
> > When building with -Wuninitialized, clang warns:
> > 
> > drivers/gpu/drm/i915/intel_device_info.c:315:3: warning: variable
> > 'subslice_mask' is uninitialized when used here [-Wuninitialized]
> >                 subslice_mask |= BIT(0);
> >                 ^~~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_device_info.c:301:18: note: initialize
> > the variable 'subslice_mask' to silence this warning
> >         u8 subslice_mask;
> >                         ^
> >                          = '\0'
> > 
> > I assume that it should be initialized to zero but maybe you
> > intended
> > something different?
> 
> As it happens, the commit has been reverted for other reasons, and we
> (the royal we, I really mean Stuart) can fix this while at it. ;)

I'll accept the royal 'we' descriptor in this instance and address the
issue. Thanks for the comment!

-Stuart

> 
> BR,
> Jani.
> 
> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [CI 5/5] drm/i915: Expand subslice mask

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 3657 bytes --]

On Wed, 2019-05-29 at 17:58 +0300, Jani Nikula wrote:
> On Fri, 24 May 2019, Stuart Summers <stuart.summers@intel.com> wrote:
> > Currently, the subslice_mask runtime parameter is stored as an
> > array of subslices per slice. Expand the subslice mask array to
> > better match what is presented to userspace through the
> > I915_QUERY_TOPOLOGY_INFO ioctl. The index into this array is
> > then calculated:
> >   slice * subslice stride + subslice index / 8
> > 
> > v2: fix spacing in set_sseu_info args
> >     use set_sseu_info to initialize sseu data when building
> >     device status in debugfs
> >     rename variables in intel_engine_types.h to avoid checkpatch
> >     warnings
> > v3: update headers in intel_sseu.h
> > v4: add const to some sseu_dev_info variables
> >     use sseu->eu_stride for EU stride calculations
> > v5: address review comments from Tvrtko and Daniele
> > v6: remove extra space in intel_sseu_get_subslices
> >     return the correct subslice enable in for_each_instdone
> >     add GEM_BUG_ON to ensure user doesn't pass invalid ss_mask size
> >     use printk formatted string for subslice mask
> > v7: remove string.h header and rebase
> > 
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <
> > daniele.ceraolospurio@intel.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> 
> As this patch uncovered a latent issue in 1e40d4aea57b
> ("drm/i915/cnl:
> Implement WaProgramMgsrForCorrectSliceSpecificMmioReads") and got
> reverted, I'll take the opportunity to comment. I acknowledge the
> revert
> is shooting the messenger a bit, and this will smell like maintainer
> bikeshedding.

I have no problem reworking regressions my series caused :) I just
wasn't aware there was an issue when merging.

> 
> Now, the first reaction looking at the commit was, it does not fare
> well
> on the "if a bisect landed on this commit, how happy would I be"
> scale.
> 
> While it's mostly refactoring, it could be chopped up to several
> logical
> and obvious steps. For example, add intel_sseu_set_info() first with
> no
> other changes. Add ss_stride and eu_stride to struct sseu_dev_info
> separately. Add intel_sseu_get_subslices() but don't expand yet, make
> it
> just sseu->subslice_mask[s] first. And so on, you get the idea, a
> series
> of small non-functional changes followed by patches with functional
> changes that stand out. Indeed patches 1-4 did this fine.

This is good general feedback, thanks. I'll split this up when
reposting.

> 
> It's easy on the reviewer, it's easy on whoever git blames years down
> the line. Trust me, we will.
> 
> And it would be the commit adding intel_sseu_get_subslices(), or the
> one
> adding the GEM_BUG_ON()s into it, that would blow up 1e40d4aea57b
> ("drm/i915/cnl: Implement
> WaProgramMgsrForCorrectSliceSpecificMmioReads").
> 
> One more note below.
> 
> > @@ -461,7 +461,9 @@ static int i915_getparam_ioctl(struct
> > drm_device *dev, void *data,
> >  			return -ENODEV;
> >  		break;
> >  	case I915_PARAM_SUBSLICE_MASK:
> > -		value = sseu->subslice_mask[0];
> > +		/* Only copy bits from the first slice */
> > +		memcpy(&value, sseu->subslice_mask,
> > +		       min(sseu->ss_stride, (u8)sizeof(value)));
> 
> Frankly I'd rather see this written in self-evident code without the
> comment.

Sure, I'll take a look.

Thanks for the comments!
Stuart

> 
> BR,
> Jani.
> 
> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [CI 0/5] Refactor to expand subslice mask

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 4517 bytes --]

On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote:
> 
> On 5/28/19 11:48 PM, Saarinen, Jani wrote:
> > Hi,
> > 
> > > -----Original Message-----
> > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> > > On Behalf Of
> > > Summers, Stuart
> > > Sent: tiistai 28. toukokuuta 2019 21.33
> > > To: Navare, Manasi D <manasi.d.navare@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice
> > > mask
> > > 
> > > On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
> > > > Pushed to dinq, thanks for the patches and the reviews!
> > > 
> > > Thanks for the push Manasi and the reviews Daniele and others!
> > 
> > This broke all the ICL systems because CI data was not looked that
> > they did not actually even boot at all.
> > All ICL's in BAT and whole ICL shards.
> > 
> 
> Can we change the CI reply for the case where there are extra
> missing 
> machines compared to the reference run from SUCCESS to WARNING or 
> something like that, so people have a clearer indication that
> something 
> might have gone wrong?

I agree here. I'm sure with time and experience these types of things
will get easier to parse, but this was very unobvious to me when
posting. I have no problem reworking, but would really appreciate a
solution to this from the CI side to ensure we don't hit this type of
thing in the future.

Thanks,
Stuart

> 
> Daniele
> 
> > 
> > > 
> > > -Stuart
> > > 
> > > > 
> > > > Regards
> > > > Manasi
> > > > 
> > > > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
> > > > > This patch series contains a few code clean-up patches,
> > > > > followed by
> > > > > a patch which changes the storage of the subslice mask to
> > > > > better
> > > > > match the userspace access through the
> > > > > I915_QUERY_TOPOLOGY_INFO
> > > > > ioctl. The index into the subslice_mask array is then
> > > > > calculated:
> > > > >    slice * subslice stride + subslice index / 8
> > > > > 
> > > > > v2: fix i915_pm_sseu test failure
> > > > > v3: no changes to patches in the series, just resending to
> > > > > pick up
> > > > >      in CI correctly
> > > > > v4: rebase
> > > > > v5: fix header test
> > > > > v6: address review comments from Jari
> > > > >      address minor checkpatch warning in existing code
> > > > >      use eu_stride for EU div-by-8
> > > > > v7: another rebase
> > > > > v8: address review comments from Tvrtko and Daniele
> > > > > v9: address review comments from Daniele
> > > > > v10: add reviewed-by on last patch with minor suggested
> > > > > change,
> > > > >       rebase, and repost for CI
> > > > > 
> > > > > Stuart Summers (5):
> > > > >    drm/i915: Use local variable for SSEU info in GETPARAM
> > > > > ioctl
> > > > >    drm/i915: Add macro for SSEU stride calculation
> > > > >    drm/i915: Move calculation of subslices per slice to new
> > > > > function
> > > > >    drm/i915: Refactor sseu helper functions
> > > > >    drm/i915: Expand subslice mask
> > > > > 
> > > > >   drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
> > > > >   drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
> > > > >   drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
> > > > >   drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
> > > > >   drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
> > > > >   drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
> > > > >   drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
> > > > >   drivers/gpu/drm/i915/i915_drv.c              |  15 +-
> > > > >   drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
> > > > >   drivers/gpu/drm/i915/i915_query.c            |  15 +-
> > > > >   drivers/gpu/drm/i915/intel_device_info.c     | 176
> > > > > +++++++++++--
> > > > > ------
> > > > >   drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
> > > > >   12 files changed, 280 insertions(+), 180 deletions(-)
> > > > > 
> > > > > --
> > > > > 2.21.0.5.gaeb582a983
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [CI 0/5] Refactor to expand subslice mask

[Saarinen, Jani]

Hi, 


> -----Original Message-----
> From: Summers, Stuart
> Sent: keskiviikko 29. toukokuuta 2019 19.02
> To: Saarinen, Jani <jani.saarinen@intel.com>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; Navare, Manasi D
> <manasi.d.navare@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
> 
> On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote:
> >
> > On 5/28/19 11:48 PM, Saarinen, Jani wrote:
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
> > > > On Behalf Of
> > > > Summers, Stuart
> > > > Sent: tiistai 28. toukokuuta 2019 21.33
> > > > To: Navare, Manasi D <manasi.d.navare@intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org
> > > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice
> > > > mask
> > > >
> > > > On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
> > > > > Pushed to dinq, thanks for the patches and the reviews!
> > > >
> > > > Thanks for the push Manasi and the reviews Daniele and others!
> > >
> > > This broke all the ICL systems because CI data was not looked that
> > > they did not actually even boot at all.
> > > All ICL's in BAT and whole ICL shards.
> > >
> >
> > Can we change the CI reply for the case where there are extra
> > missing
> > machines compared to the reference run from SUCCESS to WARNING or
> > something like that, so people have a clearer indication that
> > something
> > might have gone wrong?
> 
> I agree here. I'm sure with time and experience these types of things
> will get easier to parse, but this was very unobvious to me when
> posting. I have no problem reworking, but would really appreciate a
> solution to this from the CI side to ensure we don't hit this type of
> thing in the future.
Sure, CI team already discussed on this. 
But going forward. Can you fix this still today that ICL's systems are green not orange on ci-grid.
So would be good to get to the state that was on CI_DRM_6158. 
Reference eg. : https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly after module reload we fail.
<3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten w/a for reg 7034 (mask: ffffffff, value: 80000280)

Br,
Jani
> 
> Thanks,
> Stuart
> 
> >
> > Daniele
> >
> > >
> > > >
> > > > -Stuart
> > > >
> > > > >
> > > > > Regards
> > > > > Manasi
> > > > >
> > > > > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
> > > > > > This patch series contains a few code clean-up patches,
> > > > > > followed by
> > > > > > a patch which changes the storage of the subslice mask to
> > > > > > better
> > > > > > match the userspace access through the
> > > > > > I915_QUERY_TOPOLOGY_INFO
> > > > > > ioctl. The index into the subslice_mask array is then
> > > > > > calculated:
> > > > > >    slice * subslice stride + subslice index / 8
> > > > > >
> > > > > > v2: fix i915_pm_sseu test failure
> > > > > > v3: no changes to patches in the series, just resending to
> > > > > > pick up
> > > > > >      in CI correctly
> > > > > > v4: rebase
> > > > > > v5: fix header test
> > > > > > v6: address review comments from Jari
> > > > > >      address minor checkpatch warning in existing code
> > > > > >      use eu_stride for EU div-by-8
> > > > > > v7: another rebase
> > > > > > v8: address review comments from Tvrtko and Daniele
> > > > > > v9: address review comments from Daniele
> > > > > > v10: add reviewed-by on last patch with minor suggested
> > > > > > change,
> > > > > >       rebase, and repost for CI
> > > > > >
> > > > > > Stuart Summers (5):
> > > > > >    drm/i915: Use local variable for SSEU info in GETPARAM
> > > > > > ioctl
> > > > > >    drm/i915: Add macro for SSEU stride calculation
> > > > > >    drm/i915: Move calculation of subslices per slice to new
> > > > > > function
> > > > > >    drm/i915: Refactor sseu helper functions
> > > > > >    drm/i915: Expand subslice mask
> > > > > >
> > > > > >   drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
> > > > > >   drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
> > > > > >   drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
> > > > > >   drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
> > > > > >   drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
> > > > > >   drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
> > > > > >   drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
> > > > > >   drivers/gpu/drm/i915/i915_drv.c              |  15 +-
> > > > > >   drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
> > > > > >   drivers/gpu/drm/i915/i915_query.c            |  15 +-
> > > > > >   drivers/gpu/drm/i915/intel_device_info.c     | 176
> > > > > > +++++++++++--
> > > > > > ------
> > > > > >   drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
> > > > > >   12 files changed, 280 insertions(+), 180 deletions(-)
> > > > > >
> > > > > > --
> > > > > > 2.21.0.5.gaeb582a983
> > > > > >
> > > > > > _______________________________________________
> > > > > > Intel-gfx mailing list
> > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 0/5] Refactor to expand subslice mask

[Summers, Stuart]

[-- Attachment #1.1: Type: text/plain, Size: 6309 bytes --]

On Thu, 2019-05-30 at 09:29 +0100, Saarinen, Jani wrote:
> Hi, 
> 
> 
> > -----Original Message-----
> > From: Summers, Stuart
> > Sent: keskiviikko 29. toukokuuta 2019 19.02
> > To: Saarinen, Jani <jani.saarinen@intel.com>; Ceraolo Spurio,
> > Daniele
> > <daniele.ceraolospurio@intel.com>; Navare, Manasi D
> > <manasi.d.navare@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
> > 
> > On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote:
> > > 
> > > On 5/28/19 11:48 PM, Saarinen, Jani wrote:
> > > > Hi,
> > > > 
> > > > > -----Original Message-----
> > > > > From: Intel-gfx [mailto:
> > > > > intel-gfx-bounces@lists.freedesktop.org]
> > > > > On Behalf Of
> > > > > Summers, Stuart
> > > > > Sent: tiistai 28. toukokuuta 2019 21.33
> > > > > To: Navare, Manasi D <manasi.d.navare@intel.com>
> > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice
> > > > > mask
> > > > > 
> > > > > On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
> > > > > > Pushed to dinq, thanks for the patches and the reviews!
> > > > > 
> > > > > Thanks for the push Manasi and the reviews Daniele and
> > > > > others!sk: ffffffff, value: 80000280)
> > > > 
> > > > This broke all the ICL systems because CI data was not looked
> > > > that
> > > > they did not actually even boot at all.
> > > > All ICL's in BAT and whole ICL shards.
> > > > 
> > > 
> > > Can we change the CI reply for the case where there are extra
> > > missing
> > > machines compared to the reference run from SUCCESS to WARNING or
> > > something like that, so people have a clearer indication that
> > > something
> > > might have gone wrong?
> > 
> > I agree here. I'm sure with time and experience these types of
> > things
> > will get easier to parse, but this was very unobvious to me when
> > posting. I have no problem reworking, but would really appreciate a
> > solution to this from the CI side to ensure we don't hit this type
> > of
> > thing in the future.
> 
> Sure, CI team already discussed on this. 
> But going forward. Can you fix this still today that ICL's systems
> are green not orange on ci-grid.
> So would be good to get to the state that was on CI_DRM_6158. 
> Reference eg. : 
> https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly
> after module reload we fail.
> <3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten
> w/a for reg 7034 (mask: ffffffff, value: 80000280)

Hi Jani,

I will likely not have time to get to this today or maybe even tomorrow
unfortunately. I'll try to look at this as part of my rework of the
SSEU revert from yesterday.

Thanks,
Stuart

> 
> Br,
> Jani
> > 
> > Thanks,
> > Stuart
> > 
> > > 
> > > Daniele
> > > 
> > > > 
> > > > > 
> > > > > -Stuart
> > > > > 
> > > > > > 
> > > > > > Regards
> > > > > > Manasi
> > > > > > 
> > > > > > On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers
> > > > > > wrote:
> > > > > > > This patch series contains a few code clean-up patches,
> > > > > > > followed by
> > > > > > > a patch which changes the storage of the subslice mask to
> > > > > > > better
> > > > > > > match the userspace access through the
> > > > > > > I915_QUERY_TOPOLOGY_INFO
> > > > > > > ioctl. The index into the subslice_mask array is then
> > > > > > > calculated:
> > > > > > >    slice * subslice stride + subslice index / 8
> > > > > > > 
> > > > > > > v2: fix i915_pm_sseu test failure
> > > > > > > v3: no changes to patches in the series, just resending
> > > > > > > to
> > > > > > > pick up
> > > > > > >      in CI correctly
> > > > > > > v4: rebase
> > > > > > > v5: fix header test
> > > > > > > v6: address review comments from Jari
> > > > > > >      address minor checkpatch warning in existing code
> > > > > > >      use eu_stride for EU div-by-8
> > > > > > > v7: another rebase
> > > > > > > v8: address review comments from Tvrtko and Daniele
> > > > > > > v9: address review comments from Daniele
> > > > > > > v10: add reviewed-by on last patch with minor suggested
> > > > > > > change,
> > > > > > >       rebase, and repost for CI
> > > > > > > 
> > > > > > > Stuart Summers (5):
> > > > > > >    drm/i915: Use local variable for SSEU info in GETPARAM
> > > > > > > ioctl
> > > > > > >    drm/i915: Add macro for SSEU stride calculation
> > > > > > >    drm/i915: Move calculation of subslices per slice to
> > > > > > > new
> > > > > > > function
> > > > > > >    drm/i915: Refactor sseu helper functions
> > > > > > >    drm/i915: Expand subslice mask
> > > > > > > 
> > > > > > >   drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
> > > > > > >   drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
> > > > > > >   drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
> > > > > > >   drivers/gpu/drm/i915/gt/intel_sseu.c         |  62
> > > > > > > +++++++
> > > > > > >   drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
> > > > > > >   drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
> > > > > > >   drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++
> > > > > > > ---
> > > > > > >   drivers/gpu/drm/i915/i915_drv.c              |  15 +-
> > > > > > >   drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
> > > > > > >   drivers/gpu/drm/i915/i915_query.c            |  15 +-
> > > > > > >   drivers/gpu/drm/i915/intel_device_info.c     | 176
> > > > > > > +++++++++++--
> > > > > > > ------
> > > > > > >   drivers/gpu/drm/i915/intel_device_info.h     |  47 ----
> > > > > > > -
> > > > > > >   12 files changed, 280 insertions(+), 180 deletions(-)
> > > > > > > 
> > > > > > > --
> > > > > > > 2.21.0.5.gaeb582a983
> > > > > > > 
> > > > > > > _______________________________________________
> > > > > > > Intel-gfx mailing list
> > > > > > > Intel-gfx@lists.freedesktop.org
> > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [CI 0/5] Refactor to expand subslice mask

[Daniele Ceraolo Spurio]

On 5/30/19 1:29 AM, Saarinen, Jani wrote:
> Hi,
> 
> 
>> -----Original Message-----
>> From: Summers, Stuart
>> Sent: keskiviikko 29. toukokuuta 2019 19.02
>> To: Saarinen, Jani <jani.saarinen@intel.com>; Ceraolo Spurio, Daniele
>> <daniele.ceraolospurio@intel.com>; Navare, Manasi D
>> <manasi.d.navare@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice mask
>>
>> On Wed, 2019-05-29 at 07:21 -0700, Daniele Ceraolo Spurio wrote:
>>>
>>> On 5/28/19 11:48 PM, Saarinen, Jani wrote:
>>>> Hi,
>>>>
>>>>> -----Original Message-----
>>>>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org]
>>>>> On Behalf Of
>>>>> Summers, Stuart
>>>>> Sent: tiistai 28. toukokuuta 2019 21.33
>>>>> To: Navare, Manasi D <manasi.d.navare@intel.com>
>>>>> Cc: intel-gfx@lists.freedesktop.org
>>>>> Subject: Re: [Intel-gfx] [CI 0/5] Refactor to expand subslice
>>>>> mask
>>>>>
>>>>> On Tue, 2019-05-28 at 11:32 -0700, Manasi Navare wrote:
>>>>>> Pushed to dinq, thanks for the patches and the reviews!
>>>>>
>>>>> Thanks for the push Manasi and the reviews Daniele and others!
>>>>
>>>> This broke all the ICL systems because CI data was not looked that
>>>> they did not actually even boot at all.
>>>> All ICL's in BAT and whole ICL shards.
>>>>
>>>
>>> Can we change the CI reply for the case where there are extra
>>> missing
>>> machines compared to the reference run from SUCCESS to WARNING or
>>> something like that, so people have a clearer indication that
>>> something
>>> might have gone wrong?
>>
>> I agree here. I'm sure with time and experience these types of things
>> will get easier to parse, but this was very unobvious to me when
>> posting. I have no problem reworking, but would really appreciate a
>> solution to this from the CI side to ensure we don't hit this type of
>> thing in the future.
> Sure, CI team already discussed on this.
> But going forward. Can you fix this still today that ICL's systems are green not orange on ci-grid.
> So would be good to get to the state that was on CI_DRM_6158.
> Reference eg. : https://intel-gfx-ci.01.org/tree/drm-tip/fi-icl-u2.html so clearly after module reload we fail.
> <3> [415.887946] [drm:_wa_add [i915]] *ERROR* Discarding overwritten w/a for reg 7034 (mask: ffffffff, value: 80000280)
> 
> Br,
> Jani

This doesn't seem to have anything to do with this series though. Looks 
like we duplicated the WA when merging 'drm/drm-next' into drm-tip:

static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
				     struct i915_wa_list *wal)
{
	struct drm_i915_private *i915 = engine->i915;

	/* WaDisableBankHangMode:icl */
	wa_write(wal,
		 GEN8_L3CNTLREG,
		 intel_uncore_read(engine->uncore, GEN8_L3CNTLREG) |
		 GEN8_ERRDETBCTRL);

	/* WaDisableBankHangMode:icl */
	wa_write(wal,
		 GEN8_L3CNTLREG,
		 intel_uncore_read(engine->uncore, GEN8_L3CNTLREG) |
		 GEN8_ERRDETBCTRL);


AFAICS the duplication is added by:

commit 7126b65091c417e757b638065ae4fdc2a5dc2f5c
Merge: 3aea8d02f801 14ee642c2ab0
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu May 30 12:06:55 2019 +0100

     Merge remote-tracking branch 'drm/drm-next' into drm-tip

Daniele

>>
>> Thanks,
>> Stuart
>>
>>>
>>> Daniele
>>>
>>>>
>>>>>
>>>>> -Stuart
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Manasi
>>>>>>
>>>>>> On Fri, May 24, 2019 at 08:40:17AM -0700, Stuart Summers wrote:
>>>>>>> This patch series contains a few code clean-up patches,
>>>>>>> followed by
>>>>>>> a patch which changes the storage of the subslice mask to
>>>>>>> better
>>>>>>> match the userspace access through the
>>>>>>> I915_QUERY_TOPOLOGY_INFO
>>>>>>> ioctl. The index into the subslice_mask array is then
>>>>>>> calculated:
>>>>>>>     slice * subslice stride + subslice index / 8
>>>>>>>
>>>>>>> v2: fix i915_pm_sseu test failure
>>>>>>> v3: no changes to patches in the series, just resending to
>>>>>>> pick up
>>>>>>>       in CI correctly
>>>>>>> v4: rebase
>>>>>>> v5: fix header test
>>>>>>> v6: address review comments from Jari
>>>>>>>       address minor checkpatch warning in existing code
>>>>>>>       use eu_stride for EU div-by-8
>>>>>>> v7: another rebase
>>>>>>> v8: address review comments from Tvrtko and Daniele
>>>>>>> v9: address review comments from Daniele
>>>>>>> v10: add reviewed-by on last patch with minor suggested
>>>>>>> change,
>>>>>>>        rebase, and repost for CI
>>>>>>>
>>>>>>> Stuart Summers (5):
>>>>>>>     drm/i915: Use local variable for SSEU info in GETPARAM
>>>>>>> ioctl
>>>>>>>     drm/i915: Add macro for SSEU stride calculation
>>>>>>>     drm/i915: Move calculation of subslices per slice to new
>>>>>>> function
>>>>>>>     drm/i915: Refactor sseu helper functions
>>>>>>>     drm/i915: Expand subslice mask
>>>>>>>
>>>>>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c    |  24 ++-
>>>>>>>    drivers/gpu/drm/i915/gt/intel_engine_types.h |  30 ++--
>>>>>>>    drivers/gpu/drm/i915/gt/intel_hangcheck.c    |   3 +-
>>>>>>>    drivers/gpu/drm/i915/gt/intel_sseu.c         |  62 +++++++
>>>>>>>    drivers/gpu/drm/i915/gt/intel_sseu.h         |  35 +++-
>>>>>>>    drivers/gpu/drm/i915/gt/intel_workarounds.c  |   2 +-
>>>>>>>    drivers/gpu/drm/i915/i915_debugfs.c          |  46 ++---
>>>>>>>    drivers/gpu/drm/i915/i915_drv.c              |  15 +-
>>>>>>>    drivers/gpu/drm/i915/i915_gpu_error.c        |   5 +-
>>>>>>>    drivers/gpu/drm/i915/i915_query.c            |  15 +-
>>>>>>>    drivers/gpu/drm/i915/intel_device_info.c     | 176
>>>>>>> +++++++++++--
>>>>>>> ------
>>>>>>>    drivers/gpu/drm/i915/intel_device_info.h     |  47 -----
>>>>>>>    12 files changed, 280 insertions(+), 180 deletions(-)
>>>>>>>
>>>>>>> --
>>>>>>> 2.21.0.5.gaeb582a983
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Intel-gfx mailing list
>>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx