Re: [PATCH] drm/i915/guc: Don't capture Gen8 regs on Gen12 devices

John Harrison <john.c.harrison@xxxxxxxxx> · Wed, 5 Apr 2023 14:13:31 -0700

On 4/3/2023 17:34, Matt Roper wrote:
On Mon, Apr 03, 2023 at 02:33:34PM -0700, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>

A pair of pre-Gen12 registers were being included in the Gen12 capture
list. GuC was rejecting those as being invalid and logging errors
about them. So, stop doing it.
Looks like these registers existed from gen8-gen11.  With this change,
it looks like they also won't be included in the GuC error capture for
gen11 (ICL and EHL/JSL) since those platforms return xe_lpd_lists [1]
rather than default_lists; do we care about that?  I assume not (since
those platforms don't use GuC submission unless you force it with the
enable_guc modparam and taint your kernel), but I figured I should point
it out.
Yeah, I think the code is treating Gen11 as Gen12 rather than Gen9 or 
it's own thing. I hadn't spotted that before. It certainly seems incorrect.


Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>


[1] Why is the main list we use called xe_lpd (i.e., the name of ADL-P's
     display IP)?  It doesn't seem like we're doing anything with display
     registers here so using display IP naming seems really confusing.
I think because no-one has a clue what name refers to what hardware any 
more :(.

What are the official names for IP_VER 9, 11, 12.00, 12.50 and 12.55?

John.



Matt

Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Fixes: dce2bd542337 ("drm/i915/guc: Add Gen9 registers for GuC error state capture.")
Cc: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
  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








