[Intel-gfx] [PATCH 0/5] Improvements to GuC error capture list processing

[Intel-gfx] [PATCH 0/5] Improvements to GuC error capture list processing

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

The GuC error capture list creation was including Gen8 registers on Xe
platforms. While fixing that, it was noticed that there were other
issues. The platform naming was wrong, the naming of lists was
misleading, the steered register code was duplicated and steered
registers were not included on all supported platforms.

NB: The changes are being sent as multiple patches to make code review
simpler. However, before merging it may be better to squash into a
single patch, especially if it going to be sent with a 'fixes' tag.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>


John Harrison (5):
  drm/i915/guc: Don't capture Gen8 regs on Xe devices
  drm/i915/guc: Capture list clean up - 1
  drm/i915/guc: Capture list clean up - 2
  drm/i915/guc: Capture list clean up - 3
  drm/i915/guc: Capture list clean up - 4

 .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 199 +++++++-----------
 1 file changed, 73 insertions(+), 126 deletions(-)

-- 
2.39.1

[Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

A pair of pre-Xe registers were being included in the Xe capture list.
GuC was rejecting those as being invalid and logging errors about
them. So, stop doing it.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Fixes: dce2bd542337 ("drm/i915/guc: Add Gen9 registers for GuC error state capture.")
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index cf49188db6a6e..e0e793167d61b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -31,12 +31,14 @@
 	{ FORCEWAKE_MT,             0,      0, "FORCEWAKE" }
 
 #define COMMON_GEN9BASE_GLOBAL \
-	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
-	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
 	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
 	{ DONE_REG,                 0,      0, "DONE_REG" }, \
 	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
 
+#define GEN9_GLOBAL \
+	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
+	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
+
 #define COMMON_GEN12BASE_GLOBAL \
 	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
 	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
@@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
 static const struct __guc_mmio_reg_descr default_global_regs[] = {
 	COMMON_BASE_GLOBAL,
 	COMMON_GEN9BASE_GLOBAL,
+	GEN9_GLOBAL,
 };
 
 static const struct __guc_mmio_reg_descr default_rc_class_regs[] = {
-- 
2.39.1

[Intel-gfx] [PATCH 2/5] drm/i915/guc: Capture list clean up - 1

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Remove 99% duplicated steered register list code. Also, include the
pre-Xe steered registers in the pre-Xe list generation.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 112 +++++-------------
 1 file changed, 29 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index e0e793167d61b..9184d2595e4ce 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -260,11 +260,15 @@ struct __ext_steer_reg {
 	i915_mcr_reg_t reg;
 };
 
-static const struct __ext_steer_reg xe_extregs[] = {
+static const struct __ext_steer_reg gen8_extregs[] = {
 	{"GEN8_SAMPLER_INSTDONE", GEN8_SAMPLER_INSTDONE},
 	{"GEN8_ROW_INSTDONE", GEN8_ROW_INSTDONE}
 };
 
+static const struct __ext_steer_reg xehpg_extregs[] = {
+	{"XEHPG_INSTDONE_GEOM_SVG", XEHPG_INSTDONE_GEOM_SVG}
+};
+
 static void __fill_ext_reg(struct __guc_mmio_reg_descr *ext,
 			   const struct __ext_steer_reg *extlist,
 			   int slice_id, int subslice_id)
@@ -295,8 +299,8 @@ __alloc_ext_regs(struct __guc_mmio_reg_descr_group *newlist,
 }
 
 static void
-guc_capture_alloc_steered_lists_xe_lpd(struct intel_guc *guc,
-				       const struct __guc_mmio_reg_descr_group *lists)
+guc_capture_alloc_steered_lists(struct intel_guc *guc,
+				const struct __guc_mmio_reg_descr_group *lists)
 {
 	struct intel_gt *gt = guc_to_gt(guc);
 	int slice, subslice, iter, i, num_steer_regs, num_tot_regs = 0;
@@ -304,74 +308,19 @@ guc_capture_alloc_steered_lists_xe_lpd(struct intel_guc *guc,
 	struct __guc_mmio_reg_descr_group *extlists;
 	struct __guc_mmio_reg_descr *extarray;
 	struct sseu_dev_info *sseu;
+	bool has_xehpg_extregs;
 
-	/* In XE_LPD we only have steered registers for the render-class */
+	/* steered registers currently only exist for the render-class */
 	list = guc_capture_get_one_list(lists, GUC_CAPTURE_LIST_INDEX_PF,
 					GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS, GUC_RENDER_CLASS);
 	/* skip if extlists was previously allocated */
 	if (!list || guc->capture->extlists)
 		return;
 
-	num_steer_regs = ARRAY_SIZE(xe_extregs);
-
-	sseu = &gt->info.sseu;
-	for_each_ss_steering(iter, gt, slice, subslice)
-		num_tot_regs += num_steer_regs;
-
-	if (!num_tot_regs)
-		return;
-
-	/* allocate an extra for an end marker */
-	extlists = kcalloc(2, sizeof(struct __guc_mmio_reg_descr_group), GFP_KERNEL);
-	if (!extlists)
-		return;
-
-	if (__alloc_ext_regs(&extlists[0], list, num_tot_regs)) {
-		kfree(extlists);
-		return;
-	}
-
-	extarray = extlists[0].extlist;
-	for_each_ss_steering(iter, gt, slice, subslice) {
-		for (i = 0; i < num_steer_regs; ++i) {
-			__fill_ext_reg(extarray, &xe_extregs[i], slice, subslice);
-			++extarray;
-		}
-	}
-
-	guc->capture->extlists = extlists;
-}
-
-static const struct __ext_steer_reg xehpg_extregs[] = {
-	{"XEHPG_INSTDONE_GEOM_SVG", XEHPG_INSTDONE_GEOM_SVG}
-};
-
-static bool __has_xehpg_extregs(u32 ipver)
-{
-	return (ipver >= IP_VER(12, 55));
-}
-
-static void
-guc_capture_alloc_steered_lists_xe_hpg(struct intel_guc *guc,
-				       const struct __guc_mmio_reg_descr_group *lists,
-				       u32 ipver)
-{
-	struct intel_gt *gt = guc_to_gt(guc);
-	struct sseu_dev_info *sseu;
-	int slice, subslice, i, iter, num_steer_regs, num_tot_regs = 0;
-	const struct __guc_mmio_reg_descr_group *list;
-	struct __guc_mmio_reg_descr_group *extlists;
-	struct __guc_mmio_reg_descr *extarray;
-
-	/* In XE_LP / HPG we only have render-class steering registers during error-capture */
-	list = guc_capture_get_one_list(lists, GUC_CAPTURE_LIST_INDEX_PF,
-					GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS, GUC_RENDER_CLASS);
-	/* skip if extlists was previously allocated */
-	if (!list || guc->capture->extlists)
-		return;
+	has_xehpg_extregs = GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 55);
 
-	num_steer_regs = ARRAY_SIZE(xe_extregs);
-	if (__has_xehpg_extregs(ipver))
+	num_steer_regs = ARRAY_SIZE(gen8_extregs);
+	if (has_xehpg_extregs)
 		num_steer_regs += ARRAY_SIZE(xehpg_extregs);
 
 	sseu = &gt->info.sseu;
@@ -393,11 +342,12 @@ guc_capture_alloc_steered_lists_xe_hpg(struct intel_guc *guc,
 
 	extarray = extlists[0].extlist;
 	for_each_ss_steering(iter, gt, slice, subslice) {
-		for (i = 0; i < ARRAY_SIZE(xe_extregs); ++i) {
-			__fill_ext_reg(extarray, &xe_extregs[i], slice, subslice);
+		for (i = 0; i < ARRAY_SIZE(gen8_extregs); ++i) {
+			__fill_ext_reg(extarray, &gen8_extregs[i], slice, subslice);
 			++extarray;
 		}
-		if (__has_xehpg_extregs(ipver)) {
+
+		if (has_xehpg_extregs) {
 			for (i = 0; i < ARRAY_SIZE(xehpg_extregs); ++i) {
 				__fill_ext_reg(extarray, &xehpg_extregs[i], slice, subslice);
 				++extarray;
@@ -413,26 +363,22 @@ static const struct __guc_mmio_reg_descr_group *
 guc_capture_get_device_reglist(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+	const struct __guc_mmio_reg_descr_group *lists;
 
-	if (GRAPHICS_VER(i915) > 11) {
-		/*
-		 * For certain engine classes, there are slice and subslice
-		 * level registers requiring steering. We allocate and populate
-		 * these at init time based on hw config add it as an extension
-		 * list at the end of the pre-populated render list.
-		 */
-		if (IS_DG2(i915))
-			guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 55));
-		else if (IS_XEHPSDV(i915))
-			guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 50));
-		else
-			guc_capture_alloc_steered_lists_xe_lpd(guc, xe_lpd_lists);
+	if (GRAPHICS_VER(i915) >= 12)
+		lists = xe_lpd_lists;
+	else
+		lists = default_lists;
 
-		return xe_lpd_lists;
-	}
+	/*
+	 * For certain engine classes, there are slice and subslice
+	 * level registers requiring steering. We allocate and populate
+	 * these at init time based on hw config add it as an extension
+	 * list at the end of the pre-populated render list.
+	 */
+	guc_capture_alloc_steered_lists(guc, lists);
 
-	/* if GuC submission is enabled on a non-POR platform, just use a common baseline */
-	return default_lists;
+	return lists;
 }
 
 static const char *
-- 
2.39.1

[Intel-gfx] [PATCH 3/5] drm/i915/guc: Capture list clean up - 2

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Don't use 'xe_lp*' prefixes for register lists that are common with
Gen8.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 9184d2595e4ce..7968a495fcfa8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -111,12 +111,12 @@ static const struct __guc_mmio_reg_descr xe_lpd_rc_class_regs[] = {
 };
 
 /* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */
-static const struct __guc_mmio_reg_descr xe_lpd_rc_inst_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_rc_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
 /* GEN9/XE_LPD - Media Decode/Encode Per-Engine-Instance */
-static const struct __guc_mmio_reg_descr xe_lpd_vd_inst_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_vd_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
@@ -126,12 +126,12 @@ static const struct __guc_mmio_reg_descr xe_lpd_vec_class_regs[] = {
 };
 
 /* GEN9/XE_LPD - Video Enhancement Per-Engine-Instance */
-static const struct __guc_mmio_reg_descr xe_lpd_vec_inst_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_vec_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
 /* GEN9/XE_LPD - Blitter Per-Engine-Instance */
-static const struct __guc_mmio_reg_descr xe_lpd_blt_inst_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_blt_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
@@ -177,32 +177,32 @@ static const struct __guc_mmio_reg_descr empty_regs_list[] = {
 static const struct __guc_mmio_reg_descr_group default_lists[] = {
 	MAKE_REGLIST(default_global_regs, PF, GLOBAL, 0),
 	MAKE_REGLIST(default_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
-	MAKE_REGLIST(xe_lpd_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
+	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
 	MAKE_REGLIST(default_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
-	MAKE_REGLIST(xe_lpd_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
+	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEO_CLASS),
-	MAKE_REGLIST(xe_lpd_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
+	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
-	MAKE_REGLIST(xe_lpd_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
+	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS),
-	MAKE_REGLIST(xe_lpd_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
+	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_GSC_OTHER_CLASS),
-	MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
+	MAKE_REGLIST(empty_regs_list, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
 	{}
 };
 
 static const struct __guc_mmio_reg_descr_group xe_lpd_lists[] = {
 	MAKE_REGLIST(xe_lpd_global_regs, PF, GLOBAL, 0),
 	MAKE_REGLIST(xe_lpd_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
-	MAKE_REGLIST(xe_lpd_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
+	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
 	MAKE_REGLIST(xe_lpd_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
-	MAKE_REGLIST(xe_lpd_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
+	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEO_CLASS),
-	MAKE_REGLIST(xe_lpd_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
+	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
 	MAKE_REGLIST(xe_lpd_vec_class_regs, PF, ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
-	MAKE_REGLIST(xe_lpd_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
+	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS),
-	MAKE_REGLIST(xe_lpd_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
+	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_GSC_OTHER_CLASS),
 	MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
 	{}
-- 
2.39.1

[Intel-gfx] [PATCH 4/5] drm/i915/guc: Capture list clean up - 3

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Fix Xe_LP name.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 44 +++++++++----------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 7968a495fcfa8..fbd0be4afc6d5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -96,47 +96,47 @@
 	{ GEN12_SFC_DONE(2),        0,      0, "SFC_DONE[2]" }, \
 	{ GEN12_SFC_DONE(3),        0,      0, "SFC_DONE[3]" }
 
-/* XE_LPD - Global */
-static const struct __guc_mmio_reg_descr xe_lpd_global_regs[] = {
+/* XE_LP Global */
+static const struct __guc_mmio_reg_descr xe_lp_global_regs[] = {
 	COMMON_BASE_GLOBAL,
 	COMMON_GEN9BASE_GLOBAL,
 	COMMON_GEN12BASE_GLOBAL,
 };
 
-/* XE_LPD - Render / Compute Per-Class */
-static const struct __guc_mmio_reg_descr xe_lpd_rc_class_regs[] = {
+/* XE_LP Render / Compute Per-Class */
+static const struct __guc_mmio_reg_descr xe_lp_rc_class_regs[] = {
 	COMMON_BASE_HAS_EU,
 	COMMON_BASE_RENDER,
 	COMMON_GEN12BASE_RENDER,
 };
 
-/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */
+/* GEN8+ Render / Compute Per-Engine-Instance */
 static const struct __guc_mmio_reg_descr gen8_rc_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* GEN9/XE_LPD - Media Decode/Encode Per-Engine-Instance */
+/* GEN8+ Media Decode/Encode Per-Engine-Instance */
 static const struct __guc_mmio_reg_descr gen8_vd_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* XE_LPD - Video Enhancement Per-Class */
-static const struct __guc_mmio_reg_descr xe_lpd_vec_class_regs[] = {
+/* XE_LP Video Enhancement Per-Class */
+static const struct __guc_mmio_reg_descr xe_lp_vec_class_regs[] = {
 	COMMON_GEN12BASE_VEC,
 };
 
-/* GEN9/XE_LPD - Video Enhancement Per-Engine-Instance */
+/* GEN8+ Video Enhancement Per-Engine-Instance */
 static const struct __guc_mmio_reg_descr gen8_vec_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* GEN9/XE_LPD - Blitter Per-Engine-Instance */
+/* GEN8+ Blitter Per-Engine-Instance */
 static const struct __guc_mmio_reg_descr gen8_blt_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* XE_LPD - GSC Per-Engine-Instance */
-static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
+/* XE_LP - GSC Per-Engine-Instance */
+static const struct __guc_mmio_reg_descr xe_lp_gsc_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
@@ -153,10 +153,8 @@ static const struct __guc_mmio_reg_descr default_rc_class_regs[] = {
 };
 
 /*
- * Empty lists:
- * GEN9/XE_LPD - Blitter Per-Class
- * GEN9/XE_LPD - Media Decode/Encode Per-Class
- * GEN9 - VEC Class
+ * Empty list to prevent warnings about unknown class/instance types
+ * as not all class/instanace types have entries on all platforms.
  */
 static const struct __guc_mmio_reg_descr empty_regs_list[] = {
 };
@@ -191,20 +189,20 @@ static const struct __guc_mmio_reg_descr_group default_lists[] = {
 	{}
 };
 
-static const struct __guc_mmio_reg_descr_group xe_lpd_lists[] = {
-	MAKE_REGLIST(xe_lpd_global_regs, PF, GLOBAL, 0),
-	MAKE_REGLIST(xe_lpd_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
+static const struct __guc_mmio_reg_descr_group xe_lp_lists[] = {
+	MAKE_REGLIST(xe_lp_global_regs, PF, GLOBAL, 0),
+	MAKE_REGLIST(xe_lp_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
 	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
-	MAKE_REGLIST(xe_lpd_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
+	MAKE_REGLIST(xe_lp_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEO_CLASS),
 	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
-	MAKE_REGLIST(xe_lpd_vec_class_regs, PF, ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
+	MAKE_REGLIST(xe_lp_vec_class_regs, PF, ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
 	MAKE_REGLIST(gen8_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS),
 	MAKE_REGLIST(gen8_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_GSC_OTHER_CLASS),
-	MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
+	MAKE_REGLIST(xe_lp_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
 	{}
 };
 
@@ -366,7 +364,7 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
 	const struct __guc_mmio_reg_descr_group *lists;
 
 	if (GRAPHICS_VER(i915) >= 12)
-		lists = xe_lpd_lists;
+		lists = xe_lp_lists;
 	else
 		lists = default_lists;
 
-- 
2.39.1

[Intel-gfx] [PATCH 5/5] drm/i915/guc: Capture list clean up - 4

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Don't use GEN9 as a prefix for register lists that contain all GEN8
registers.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index fbd0be4afc6d5..c1a75a2d17f1e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -30,12 +30,12 @@
 #define COMMON_BASE_GLOBAL \
 	{ FORCEWAKE_MT,             0,      0, "FORCEWAKE" }
 
-#define COMMON_GEN9BASE_GLOBAL \
+#define COMMON_GEN8BASE_GLOBAL \
 	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
 	{ DONE_REG,                 0,      0, "DONE_REG" }, \
 	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
 
-#define GEN9_GLOBAL \
+#define GEN8_GLOBAL \
 	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
 	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
 
@@ -99,7 +99,7 @@
 /* XE_LP Global */
 static const struct __guc_mmio_reg_descr xe_lp_global_regs[] = {
 	COMMON_BASE_GLOBAL,
-	COMMON_GEN9BASE_GLOBAL,
+	COMMON_GEN8BASE_GLOBAL,
 	COMMON_GEN12BASE_GLOBAL,
 };
 
@@ -140,11 +140,11 @@ static const struct __guc_mmio_reg_descr xe_lp_gsc_inst_regs[] = {
 	COMMON_BASE_ENGINE_INSTANCE,
 };
 
-/* GEN9 - Global */
+/* GEN8 - Global */
 static const struct __guc_mmio_reg_descr default_global_regs[] = {
 	COMMON_BASE_GLOBAL,
-	COMMON_GEN9BASE_GLOBAL,
-	GEN9_GLOBAL,
+	COMMON_GEN8BASE_GLOBAL,
+	GEN8_GLOBAL,
 };
 
 static const struct __guc_mmio_reg_descr default_rc_class_regs[] = {
-- 
2.39.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to GuC error capture list processing

[Patchwork]

== Series Details ==

Series: Improvements to GuC error capture list processing
URL   : https://patchwork.freedesktop.org/series/116219/
State : warning

== Summary ==

Error: dim checkpatch failed
71d5e265121d drm/i915/guc: Don't capture Gen8 regs on Xe devices
-:35: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#35: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c:38:
+#define GEN9_GLOBAL \
+	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
+	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }

total: 1 errors, 0 warnings, 0 checks, 23 lines checked
63e97480c431 drm/i915/guc: Capture list clean up - 1
0ff5accf4a26 drm/i915/guc: Capture list clean up - 2
2baa2c852173 drm/i915/guc: Capture list clean up - 3
096c1408d373 drm/i915/guc: Capture list clean up - 4
-:20: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#20: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c:33:
+#define COMMON_GEN8BASE_GLOBAL \
 	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
 	{ DONE_REG,                 0,      0, "DONE_REG" }, \
 	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }

-:26: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#26: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c:38:
+#define GEN8_GLOBAL \
 	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
 	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }

total: 2 errors, 0 warnings, 0 checks, 36 lines checked

[Intel-gfx] ✓ Fi.CI.BAT: success for Improvements to GuC error capture list processing

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3546 bytes --]

== Series Details ==

Series: Improvements to GuC error capture list processing
URL   : https://patchwork.freedesktop.org/series/116219/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12981 -> Patchwork_116219v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/index.html

Participating hosts (36 -> 35)
------------------------------

  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-11:         [PASS][1] -> [ABORT][2] ([i915#7913])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/bat-dg2-11/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/bat-dg2-11/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [PASS][3] -> [ABORT][4] ([i915#4983])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/bat-rpls-1/igt@i915_selftest@live@reset.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/bat-rpls-1/igt@i915_selftest@live@reset.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
    - bat-dg2-8:          [PASS][5] -> [FAIL][6] ([i915#7932])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html

  
#### Possible fixes ####

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-11:         [FAIL][7] ([i915#8308]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/bat-dg2-11/igt@i915_pm_rps@basic-api.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/bat-dg2-11/igt@i915_pm_rps@basic-api.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
    - bat-dg2-8:          [FAIL][9] ([i915#7932]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html

  
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


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

  * Linux: CI_DRM_12981 -> Patchwork_116219v1

  CI-20190529: 20190529
  CI_DRM_12981: fbadfcf137737f02425a35bf3ae17a1492301f21 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7243: 402a13477510ab05591839a2bf4586de1158e60c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_116219v1: fbadfcf137737f02425a35bf3ae17a1492301f21 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

caa2f6eff144 drm/i915/guc: Capture list clean up - 4
96c173471d22 drm/i915/guc: Capture list clean up - 3
3853aa3edf5c drm/i915/guc: Capture list clean up - 2
0c25ea8925de drm/i915/guc: Capture list clean up - 1
decdff745f34 drm/i915/guc: Don't capture Gen8 regs on Xe devices

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/index.html

[-- Attachment #2: Type: text/html, Size: 4362 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for Improvements to GuC error capture list processing

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 10899 bytes --]

== Series Details ==

Series: Improvements to GuC error capture list processing
URL   : https://patchwork.freedesktop.org/series/116219/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12981_full -> Patchwork_116219v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_116219v1_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_plane_lowres@tiling-x@pipe-b-hdmi-a-1:
    - {shard-dg1}:        NOTRUN -> [FAIL][1] +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-dg1-14/igt@kms_plane_lowres@tiling-x@pipe-b-hdmi-a-1.html

  * igt@kms_rotation_crc@primary-y-tiled-reflect-x-180:
    - {shard-rkl}:        [PASS][2] -> [ABORT][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-rkl-3/igt@kms_rotation_crc@primary-y-tiled-reflect-x-180.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-rkl-7/igt@kms_rotation_crc@primary-y-tiled-reflect-x-180.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-glk:          [PASS][4] -> [DMESG-FAIL][5] ([i915#5334])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-glk7/igt@i915_selftest@live@gt_heartbeat.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-glk8/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#3886])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-apl4/igt@kms_ccs@pipe-c-bad-pixel-format-y_tiled_gen12_mc_ccs.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [PASS][7] -> [FAIL][8] ([i915#2346])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#79])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@2x-nonexisting-fb:
    - shard-apl:          NOTRUN -> [SKIP][11] ([fdo#109271]) +32 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-apl4/igt@kms_flip@2x-nonexisting-fb.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [FAIL][12] ([i915#2842]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-apl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-apl6/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-dg1}:        [SKIP][14] ([i915#1937]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-dg1-17/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-dg1-14/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@kms_cursor_crc@cursor-suspend@pipe-c-dp-1:
    - shard-apl:          [ABORT][16] ([i915#180]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-apl1/igt@kms_cursor_crc@cursor-suspend@pipe-c-dp-1.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-apl4/igt@kms_cursor_crc@cursor-suspend@pipe-c-dp-1.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][18] ([i915#2346]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@forked-move@pipe-b:
    - {shard-rkl}:        [INCOMPLETE][20] ([i915#8011]) -> [PASS][21] +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12981/shard-rkl-7/igt@kms_cursor_legacy@forked-move@pipe-b.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116219v1/shard-rkl-3/igt@kms_cursor_legacy@forked-move@pipe-b.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4879]: https://gitlab.freedesktop.org/drm/intel/issues/4879
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


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

  * Linux: CI_DRM_12981 -> Patchwork_116219v1

  CI-20190529: 20190529
  CI_DRM_12981: fbadfcf137737f02425a35bf3ae17a1492301f21 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7243: 402a13477510ab05591839a2bf4586de1158e60c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_116219v1: fbadfcf137737f02425a35bf3ae17a1492301f21 @ 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_116219v1/index.html

[-- Attachment #2: Type: text/html, Size: 7600 bytes --]

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[Teres Alexis, Alan Previn]

On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> A pair of pre-Xe registers were being included in the Xe capture list.
> GuC was rejecting those as being invalid and logging errors about
> them. So, stop doing it.
> 
alan:snip
>  #define COMMON_GEN9BASE_GLOBAL \
> -	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> -	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
>  	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
>  	{ DONE_REG,                 0,      0, "DONE_REG" }, \
>  	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
>  
> +#define GEN9_GLOBAL \
> +	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> +	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
> +
>  #define COMMON_GEN12BASE_GLOBAL \
>  	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
>  	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
> @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
>  static const struct __guc_mmio_reg_descr default_global_regs[] = {
>  	COMMON_BASE_GLOBAL,
>  	COMMON_GEN9BASE_GLOBAL,
> +	GEN9_GLOBAL,
>  };
>  
alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into GEN9_GLOBAL
doesn't seem to communicate the intent of fix for this patch. This is more of a naming,
thing and i am not sure what counter-proposal will work well in terms of readibility.
One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to "COMMON_GEN9PLUS_BASE_GLOBAL"
and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two gen9-global
with a clear distinction in naming where one is "GEN9PLUS" and the other is "GEN9LEGACY".

But since this is a list-naming thing, i am okay either above change... OR...
keeping the same but with the condition of adding a comment under
COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where one
is gen9-legacy and the other is gen9-and-future that carries over to beyond Gen9.
(side note: coding style wise, is it possible to add the comment right under the #define
line as opposed to under the entire list?)

(conditional) Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc: Capture list clean up - 1

[Teres Alexis, Alan Previn]

On Thu, 2023-04-06 at 15:26 -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Remove 99% duplicated steered register list code. Also, include the
> pre-Xe steered registers in the pre-Xe list generation.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

alan: Nice work - good cleanup. Thanks so much.
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Capture list clean up - 2

[Teres Alexis, Alan Previn]

On Thu, 2023-04-06 at 15:26 -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Don't use 'xe_lp*' prefixes for register lists that are common with
> Gen8.

alan:snip


> @@ -177,32 +177,32 @@ static const struct __guc_mmio_reg_descr empty_regs_list[] = {
>  static const struct __guc_mmio_reg_descr_group default_lists[] = {
alan:snip
> -	MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
alan: i missed from the review of Daniele's GSC enabling patch - thanks for catching this.
>  	{}
>  };
> 
> 
simple patch - all looks good:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Capture list clean up - 3

[Teres Alexis, Alan Previn]

On Thu, 2023-04-06 at 15:26 -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Fix Xe_LP name.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
alan:snip


> -/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */
> +/* GEN8+ Render / Compute Per-Engine-Instance */
alan: two comments on this:
1. shouldnt this go with the earlier patch?
2. i agree with renaming the names of the register to reflect the when
   all of those registers got first introduced... however, considering
   we only support GuC on Gen12 and beyond (we do have select CI-tests
   that enable GuC on Gen9 but not on Gen8 and before), should we also
   change the comments? I think the comment should reflect the usage
   not just follow the same name of the registe #define - else why even
   add the comments. (please apply this same comment for gen8_vd_inst_regs,
   gen8_vec_inst_regs and gen8_blt_inst_regs).
alternatively, we could keep those GEN8+ comments above the list but maybe
add just one comment in the default list - see below.

alan: snip

> @@ -366,7 +364,7 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
>  	const struct __guc_mmio_reg_descr_group *lists;
>  
>  	if (GRAPHICS_VER(i915) >= 12)
> -		lists = xe_lpd_lists;
> +		lists = xe_lp_lists;
>  	else
>  		lists = default_lists;
alan: perhaps add a comment that we really don't support any of this
on anything below Gen9?

>  

Re: [Intel-gfx] [PATCH 5/5] drm/i915/guc: Capture list clean up - 4

[Teres Alexis, Alan Previn]

On Thu, 2023-04-06 at 15:26 -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Don't use GEN9 as a prefix for register lists that contain all GEN8
> registers.
alan:snip

alan: This patch as a stand-along looks good, so I'll provide the RB but take note of the comment below
that should be reflected by decision on the review comments of patch #1 so this patch might
change from GEN9_foo-from-patch-1 to GEN8_foo-from-patch-1.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

> -/* GEN9 - Global */
> +/* GEN8 - Global */
>  static const struct __guc_mmio_reg_descr default_global_regs[] = {
>  	COMMON_BASE_GLOBAL,
> -	COMMON_GEN9BASE_GLOBAL,
> -	GEN9_GLOBAL,
> +	COMMON_GEN8BASE_GLOBAL,
> +	GEN8_GLOBAL,
alan: see patch comment about "COMMON_GLOBAL" vs "GLOBAL" confusion.

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[John Harrison]

On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> A pair of pre-Xe registers were being included in the Xe capture list.
>> GuC was rejecting those as being invalid and logging errors about
>> them. So, stop doing it.
>>
> alan:snip
>>   #define COMMON_GEN9BASE_GLOBAL \
>> -	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>> -	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
>>   	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
>>   	{ DONE_REG,                 0,      0, "DONE_REG" }, \
>>   	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
>>   
>> +#define GEN9_GLOBAL \
>> +	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>> +	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
>> +
>>   #define COMMON_GEN12BASE_GLOBAL \
>>   	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
>>   	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
>> @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
>>   static const struct __guc_mmio_reg_descr default_global_regs[] = {
>>   	COMMON_BASE_GLOBAL,
>>   	COMMON_GEN9BASE_GLOBAL,
>> +	GEN9_GLOBAL,
>>   };
>>   
> alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into GEN9_GLOBAL
> doesn't seem to communicate the intent of fix for this patch. This is more of a naming,
> thing and i am not sure what counter-proposal will work well in terms of readibility.
> One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to "COMMON_GEN9PLUS_BASE_GLOBAL"
> and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two gen9-global
> with a clear distinction in naming where one is "GEN9PLUS" and the other is "GEN9LEGACY".
>
> But since this is a list-naming thing, i am okay either above change... OR...
> keeping the same but with the condition of adding a comment under
> COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where one
> is gen9-legacy and the other is gen9-and-future that carries over to beyond Gen9.
> (side note: coding style wise, is it possible to add the comment right under the #define
> line as opposed to under the entire list?)
>
> (conditional) Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>
I'm not entirely sure what you are arguing here.

My reading of the original code is that COMMON_GENX_ means the registers 
were introduced on the named device but a are common to later devices. 
Whereas GENX_ means the registers are specific to that device alone. 
That seems a pretty straight forward and simple naming scheme to me.

John.

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[John Harrison]

On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> A pair of pre-Xe registers were being included in the Xe capture list.
>> GuC was rejecting those as being invalid and logging errors about
>> them. So, stop doing it.
>>
> alan:snip
>>   #define COMMON_GEN9BASE_GLOBAL \
>> -	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>> -	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
>>   	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
>>   	{ DONE_REG,                 0,      0, "DONE_REG" }, \
>>   	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
>>   
>> +#define GEN9_GLOBAL \
>> +	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>> +	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
>> +
>>   #define COMMON_GEN12BASE_GLOBAL \
>>   	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
>>   	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
>> @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
>>   static const struct __guc_mmio_reg_descr default_global_regs[] = {
>>   	COMMON_BASE_GLOBAL,
>>   	COMMON_GEN9BASE_GLOBAL,
>> +	GEN9_GLOBAL,
>>   };
>>   
> alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into GEN9_GLOBAL
> doesn't seem to communicate the intent of fix for this patch. This is more of a naming,
> thing and i am not sure what counter-proposal will work well in terms of readibility.
> One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to "COMMON_GEN9PLUS_BASE_GLOBAL"
> and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two gen9-global
> with a clear distinction in naming where one is "GEN9PLUS" and the other is "GEN9LEGACY".
>
> But since this is a list-naming thing, i am okay either above change... OR...
> keeping the same but with the condition of adding a comment under
> COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where one
> is gen9-legacy and the other is gen9-and-future that carries over to beyond Gen9.
> (side note: coding style wise, is it possible to add the comment right under the #define
> line as opposed to under the entire list?)
>
> (conditional) Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>
I'm not entirely sure what you are arguing here.

My reading of the original code is that COMMON_GENX_ means the registers 
were introduced on the named device but a are common to later devices. 
Whereas GENX_ means the registers are specific to that device alone. 
That seems a pretty straight forward and simple naming scheme to me.

John.

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Capture list clean up - 3

[John Harrison]

[-- Attachment #1: Type: text/plain, Size: 2182 bytes --]

On 4/25/2023 12:05, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700,John.C.Harrison@Intel.com  wrote:
>> From: John Harrison<John.C.Harrison@Intel.com>
>>
>> Fix Xe_LP name.
>>
>> Signed-off-by: John Harrison<John.C.Harrison@Intel.com>
> alan:snip
>
>
>> -/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */
>> +/* GEN8+ Render / Compute Per-Engine-Instance */
> alan: two comments on this:
> 1. shouldnt this go with the earlier patch?
See comment in cover letter:

    NB: The changes are being sent as multiple patches to make code review
    simpler. However, before merging it may be better to squash into a
    single patch, especially if it going to be sent with a 'fixes' tag.



> 2. i agree with renaming the names of the register to reflect the when
>     all of those registers got first introduced... however, considering
>     we only support GuC on Gen12 and beyond (we do have select CI-tests
>     that enable GuC on Gen9 but not on Gen8 and before), should we also
>     change the comments? I think the comment should reflect the usage
>     not just follow the same name of the registe #define - else why even
>     add the comments. (please apply this same comment for gen8_vd_inst_regs,
>     gen8_vec_inst_regs and gen8_blt_inst_regs).
> alternatively, we could keep those GEN8+ comments above the list but maybe
> add just one comment in the default list - see below.
Because at some point we might want to start supporting other platforms. 
My view is that the comment should be accurate. These registers exist on 
Gen8+. So if you are building a register list for a Gen8 or later 
device, they can/should be included.

> alan: snip
>
>> @@ -366,7 +364,7 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
>>   	const struct __guc_mmio_reg_descr_group *lists;
>>   
>>   	if (GRAPHICS_VER(i915) >= 12)
>> -		lists = xe_lpd_lists;
>> +		lists = xe_lp_lists;
>>   	else
>>   		lists = default_lists;
> alan: perhaps add a comment that we really don't support any of this
> on anything below Gen9?
It wasn't me that named it 'default_' rather than gen9_. I could add yet 
another rename of s/default_/gen9_/g...

John.

>
>>   

[-- Attachment #2: Type: text/html, Size: 3873 bytes --]

Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: Capture list clean up - 3

[John Harrison]

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

On 4/26/2023 10:29, John Harrison wrote:
> On 4/25/2023 12:05, Teres Alexis, Alan Previn wrote:
>> On Thu, 2023-04-06 at 15:26 -0700,John.C.Harrison@Intel.com  wrote:
>>> From: John Harrison<John.C.Harrison@Intel.com>
>>>
>>> Fix Xe_LP name.
>>>
>>> Signed-off-by: John Harrison<John.C.Harrison@Intel.com>
>> alan:snip
>>
>>
>>> -/* GEN9/XE_LPD - Render / Compute Per-Engine-Instance */
>>> +/* GEN8+ Render / Compute Per-Engine-Instance */
>> alan: two comments on this:
>> 1. shouldnt this go with the earlier patch?
> See comment in cover letter:
>
>     NB: The changes are being sent as multiple patches to make code review
>     simpler. However, before merging it may be better to squash into a
>     single patch, especially if it going to be sent with a 'fixes' tag.
>
>
>
>> 2. i agree with renaming the names of the register to reflect the when
>>     all of those registers got first introduced... however, considering
>>     we only support GuC on Gen12 and beyond (we do have select CI-tests
>>     that enable GuC on Gen9 but not on Gen8 and before), should we also
>>     change the comments? I think the comment should reflect the usage
>>     not just follow the same name of the registe #define - else why even
>>     add the comments. (please apply this same comment for gen8_vd_inst_regs,
>>     gen8_vec_inst_regs and gen8_blt_inst_regs).
>> alternatively, we could keep those GEN8+ comments above the list but maybe
>> add just one comment in the default list - see below.
> Because at some point we might want to start supporting other 
> platforms. My view is that the comment should be accurate. These 
> registers exist on Gen8+. So if you are building a register list for a 
> Gen8 or later device, they can/should be included.
>
>> alan: snip
>>
>>> @@ -366,7 +364,7 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
>>>   	const struct __guc_mmio_reg_descr_group *lists;
>>>   
>>>   	if (GRAPHICS_VER(i915) >= 12)
>>> -		lists = xe_lpd_lists;
>>> +		lists = xe_lp_lists;
>>>   	else
>>>   		lists = default_lists;
>> alan: perhaps add a comment that we really don't support any of this
>> on anything below Gen9?
> It wasn't me that named it 'default_' rather than gen9_. I could add 
> yet another rename of s/default_/gen9_/g...
>
> John.
Although looking at the lists, there is nothing gen9 specific anywhere. 
So gen8_ would be the more accurate name.

John.

>
>>>   
>

[-- Attachment #2: Type: text/html, Size: 4378 bytes --]

Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: Capture list clean up - 2

[John Harrison]

On 4/25/2023 11:54, Teres Alexis, Alan Previn wrote:
> On Thu, 2023-04-06 at 15:26 -0700, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Don't use 'xe_lp*' prefixes for register lists that are common with
>> Gen8.
> alan:snip
>
>
>> @@ -177,32 +177,32 @@ static const struct __guc_mmio_reg_descr empty_regs_list[] = {
>>   static const struct __guc_mmio_reg_descr_group default_lists[] = {
> alan:snip
>> -	MAKE_REGLIST(xe_lpd_gsc_inst_regs, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
>> +	MAKE_REGLIST(empty_regs_list, PF, ENGINE_INSTANCE, GUC_GSC_OTHER_CLASS),
> alan: i missed from the review of Daniele's GSC enabling patch - thanks for catching this.
Hmm. That should have been a separate patch. Or at least called out 
separately in the commit message. Will add a comment about it.

John.

>>   	{}
>>   };
>>
>>
> simple patch - all looks good:
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

[Intel-gfx] [PATCH 6/6] drm/i915/guc: Capture list clean up - 5

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Rename the 'default_' register list prefix to 'gen8_' as that is the
more accurate name.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index c1a75a2d17f1e..729a8fcf20dda 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -141,13 +141,13 @@ static const struct __guc_mmio_reg_descr xe_lp_gsc_inst_regs[] = {
 };
 
 /* GEN8 - Global */
-static const struct __guc_mmio_reg_descr default_global_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_global_regs[] = {
 	COMMON_BASE_GLOBAL,
 	COMMON_GEN8BASE_GLOBAL,
 	GEN8_GLOBAL,
 };
 
-static const struct __guc_mmio_reg_descr default_rc_class_regs[] = {
+static const struct __guc_mmio_reg_descr gen8_rc_class_regs[] = {
 	COMMON_BASE_HAS_EU,
 	COMMON_BASE_RENDER,
 };
@@ -172,11 +172,11 @@ static const struct __guc_mmio_reg_descr empty_regs_list[] = {
 	}
 
 /* List of lists */
-static const struct __guc_mmio_reg_descr_group default_lists[] = {
-	MAKE_REGLIST(default_global_regs, PF, GLOBAL, 0),
-	MAKE_REGLIST(default_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
+static const struct __guc_mmio_reg_descr_group gen8_lists[] = {
+	MAKE_REGLIST(gen8_global_regs, PF, GLOBAL, 0),
+	MAKE_REGLIST(gen8_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
 	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
-	MAKE_REGLIST(default_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
+	MAKE_REGLIST(gen8_rc_class_regs, PF, ENGINE_CLASS, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(gen8_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_COMPUTE_CLASS),
 	MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEO_CLASS),
 	MAKE_REGLIST(gen8_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
@@ -366,7 +366,7 @@ guc_capture_get_device_reglist(struct intel_guc *guc)
 	if (GRAPHICS_VER(i915) >= 12)
 		lists = xe_lp_lists;
 	else
-		lists = default_lists;
+		lists = gen8_lists;
 
 	/*
 	 * For certain engine classes, there are slice and subslice
-- 
2.39.1

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[Teres Alexis, Alan Previn]

On Wed, 2023-04-26 at 10:23 -0700, Harrison, John C wrote:
> On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
> > On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
> > > From: John Harrison <John.C.Harrison@Intel.com>
> > > 
> > > A pair of pre-Xe registers were being included in the Xe capture list.
> > > GuC was rejecting those as being invalid and logging errors about
> > > them. So, stop doing it.
> > > 
> > alan:snip
> > >   #define COMMON_GEN9BASE_GLOBAL \
> > > -	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> > > -	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
> > >   	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
> > >   	{ DONE_REG,                 0,      0, "DONE_REG" }, \
> > >   	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
> > >   
> > > +#define GEN9_GLOBAL \
> > > +	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
> > > +	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
> > > +
> > >   #define COMMON_GEN12BASE_GLOBAL \
> > >   	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
> > >   	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
> > > @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
> > >   static const struct __guc_mmio_reg_descr default_global_regs[] = {
> > >   	COMMON_BASE_GLOBAL,
> > >   	COMMON_GEN9BASE_GLOBAL,
> > > +	GEN9_GLOBAL,
> > >   };
> > >   
> > alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into GEN9_GLOBAL
> > doesn't seem to communicate the intent of fix for this patch. This is more of a naming,
> > thing and i am not sure what counter-proposal will work well in terms of readibility.
> > One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to "COMMON_GEN9PLUS_BASE_GLOBAL"
> > and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two gen9-global
> > with a clear distinction in naming where one is "GEN9PLUS" and the other is "GEN9LEGACY".
> > 
> > But since this is a list-naming thing, i am okay either above change... OR...
> > keeping the same but with the condition of adding a comment under
> > COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where one
> > is gen9-legacy and the other is gen9-and-future that carries over to beyond Gen9.
> > (side note: coding style wise, is it possible to add the comment right under the #define
> > line as opposed to under the entire list?)
> > 
> > (conditional) Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> > 
> I'm not entirely sure what you are arguing here.
> 
> My reading of the original code is that COMMON_GENX_ means the registers 
> were introduced on the named device but a are common to later devices. 
> Whereas GENX_ means the registers are specific to that device alone. 
> That seems a pretty straight forward and simple naming scheme to me.
> 
> John.
> 
alan: you said "reading of the original code is that COMMON_GENX_
means the registers were introduced on the named device but a are
common to later devices".
That wasnt the original intent when authored. The original intent
was list names and its comments to corresponded to what platforms we
supported (thats why the first patch was all of Gen12 registers in a
single list that included Gen8/9 register definitions and Gen9 sublists
got abstracted out later).

I'm okay with changing the intent of the list naming - but your code still
looks a bit off considering you have at least one list with two sublists
that carry the term "GEN9":
  static const struct __guc_mmio_reg_descr default_global_regs[] = {
	COMMON_BASE_GLOBAL,
	COMMON_GEN9BASE_GLOBAL,
	GEN9_GLOBAL,

Again, i feel the name of these two sublists ("COMMON_GEN9BASE_GLOBAL" vs
"GEN9_GLOBAL") dont easily portray differences and why they were separated
out. If they both reflect "when the named register got introduced", then
why not just have them in the same list? Since this patch is not
about "when a register got introduced" but "pre-Xe registers are not
recognized by GuC on Xe", then perhaps we can change the names to:
	COMMON_GEN9BASE_GLOBAL -> [same]
	GEN9_GLOBAL -> PREXE_GEN9_GLOBAL

Either way, i rather not argue about variable names - so here is the Rb
(since patch comment describes the issue and functionality seems correct):
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Capture list clean up - 5

[Teres Alexis, Alan Previn]

On Wed, 2023-04-26 at 11:24 -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Rename the 'default_' register list prefix to 'gen8_' as that is the
> more accurate name.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Don't capture Gen8 regs on Xe devices

[John Harrison]

On 4/26/2023 14:14, Teres Alexis, Alan Previn wrote:
> On Wed, 2023-04-26 at 10:23 -0700, Harrison, John C wrote:
>> On 4/25/2023 10:55, Teres Alexis, Alan Previn wrote:
>>> On Thu, 2023-04-06 at 15:26 -0700, Harrison, John C wrote:
>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>
>>>> A pair of pre-Xe registers were being included in the Xe capture list.
>>>> GuC was rejecting those as being invalid and logging errors about
>>>> them. So, stop doing it.
>>>>
>>> alan:snip
>>>>    #define COMMON_GEN9BASE_GLOBAL \
>>>> -	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>>>> -	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }, \
>>>>    	{ ERROR_GEN6,               0,      0, "ERROR_GEN6" }, \
>>>>    	{ DONE_REG,                 0,      0, "DONE_REG" }, \
>>>>    	{ HSW_GTT_CACHE_EN,         0,      0, "HSW_GTT_CACHE_EN" }
>>>>    
>>>> +#define GEN9_GLOBAL \
>>>> +	{ GEN8_FAULT_TLB_DATA0,     0,      0, "GEN8_FAULT_TLB_DATA0" }, \
>>>> +	{ GEN8_FAULT_TLB_DATA1,     0,      0, "GEN8_FAULT_TLB_DATA1" }
>>>> +
>>>>    #define COMMON_GEN12BASE_GLOBAL \
>>>>    	{ GEN12_FAULT_TLB_DATA0,    0,      0, "GEN12_FAULT_TLB_DATA0" }, \
>>>>    	{ GEN12_FAULT_TLB_DATA1,    0,      0, "GEN12_FAULT_TLB_DATA1" }, \
>>>> @@ -142,6 +144,7 @@ static const struct __guc_mmio_reg_descr xe_lpd_gsc_inst_regs[] = {
>>>>    static const struct __guc_mmio_reg_descr default_global_regs[] = {
>>>>    	COMMON_BASE_GLOBAL,
>>>>    	COMMON_GEN9BASE_GLOBAL,
>>>> +	GEN9_GLOBAL,
>>>>    };
>>>>    
>>> alan: splitting out a couple registers from COMMON_GEN9BASE_GLOBAL into GEN9_GLOBAL
>>> doesn't seem to communicate the intent of fix for this patch. This is more of a naming,
>>> thing and i am not sure what counter-proposal will work well in terms of readibility.
>>> One idea: perhaps we rename "COMMON_GEN9BASE_GLOBAL" to "COMMON_GEN9PLUS_BASE_GLOBAL"
>>> and rename GEN9_GLOBAL to COMMON_GEN9LEGACY_GLOBAL. so we would have two gen9-global
>>> with a clear distinction in naming where one is "GEN9PLUS" and the other is "GEN9LEGACY".
>>>
>>> But since this is a list-naming thing, i am okay either above change... OR...
>>> keeping the same but with the condition of adding a comment under
>>> COMMON_GEN9BASE_GLOBAL and GEN9_GLOBAL names that explain the differences where one
>>> is gen9-legacy and the other is gen9-and-future that carries over to beyond Gen9.
>>> (side note: coding style wise, is it possible to add the comment right under the #define
>>> line as opposed to under the entire list?)
>>>
>>> (conditional) Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>>>
>> I'm not entirely sure what you are arguing here.
>>
>> My reading of the original code is that COMMON_GENX_ means the registers
>> were introduced on the named device but a are common to later devices.
>> Whereas GENX_ means the registers are specific to that device alone.
>> That seems a pretty straight forward and simple naming scheme to me.
>>
>> John.
>>
> alan: you said "reading of the original code is that COMMON_GENX_
> means the registers were introduced on the named device but a are
> common to later devices".
> That wasnt the original intent when authored. The original intent
> was list names and its comments to corresponded to what platforms we
> supported (thats why the first patch was all of Gen12 registers in a
> single list that included Gen8/9 register definitions and Gen9 sublists
> got abstracted out later).
>
> I'm okay with changing the intent of the list naming - but your code still
> looks a bit off considering you have at least one list with two sublists
> that carry the term "GEN9":
>    static const struct __guc_mmio_reg_descr default_global_regs[] = {
> 	COMMON_BASE_GLOBAL,
> 	COMMON_GEN9BASE_GLOBAL,
> 	GEN9_GLOBAL,
>
> Again, i feel the name of these two sublists ("COMMON_GEN9BASE_GLOBAL" vs
> "GEN9_GLOBAL") dont easily portray differences and why they were separated
> out. If they both reflect "when the named register got introduced", then
> why not just have them in the same list? Since this patch is not
> about "when a register got introduced" but "pre-Xe registers are not
> recognized by GuC on Xe", then perhaps we can change the names to:
> 	COMMON_GEN9BASE_GLOBAL -> [same]
> 	GEN9_GLOBAL -> PREXE_GEN9_GLOBAL
PREXE_GEN9_... just sounds confused to me. What is Gen9 if it is not PreXe?

The point is to ensure that platform specific register lists are 
constructed from the sublists that apply to that platform. Therefore the 
sublists are named for the platform they apply to.  Thus the gen8 list 
should only contain gen8 sub-lists. The Xe list can contain Xe sublists 
together with gen8 sublists that are still applicable. Those sublists 
have a COMMON_ prefix if they are expected to be multi-platform and 
don't if they are specific to their one named platform.

Note that you can't get hung on 'the end result looks odd' when only 
looking at the first step of a whole series of cleanups. This patch is 
specifically about moving the pre-Xe registers out of the COMMON_ define 
so that they are not used on Xe. It is not trying to fix up all the 
naming and other issues.

John.

>
> Either way, i rather not argue about variable names - so here is the Rb
> (since patch comment describes the issue and functionality seems correct):
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>