All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Nuke legacy hw_id
@ 2021-07-20 23:20 ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin

Motivated by my review in
https://patchwork.freedesktop.org/patch/443857/?series=92135&rev=5 I
went to look why we needed the additional hw_id fields. It turns out we
don't, but we kept adding new IDs to keep it consistent. Now that with
the extra media engines we would just leave than zero'ed, let's refactor
the code so we don't keep them around: they aren't used since
GRAPHICS_VER == 8.

I'd say last patch is a stretch due to the use of _PICK() and hardcoding
the map, but to me it seems to avoid making it more complex elsewhere.

Lucas De Marchi (4):
  drm/i915/gt: fix platform prefix
  drm/i915/gt: nuke unused legacy engine hw_id
  drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  drm/i915/gt: nuke gen6_hw_id

 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 10 ----------
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 12 ------------
 drivers/gpu/drm/i915/gt/intel_gt.c           |  4 ++--
 drivers/gpu/drm/i915/i915_reg.h              |  4 +++-
 4 files changed, 5 insertions(+), 25 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 37+ messages in thread
* [Intel-gfx] [PATCH 0/4] Nuke legacy hw_id
@ 2021-07-20 23:20 ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, dri-devel

Motivated by my review in
https://patchwork.freedesktop.org/patch/443857/?series=92135&rev=5 I
went to look why we needed the additional hw_id fields. It turns out we
don't, but we kept adding new IDs to keep it consistent. Now that with
the extra media engines we would just leave than zero'ed, let's refactor
the code so we don't keep them around: they aren't used since
GRAPHICS_VER == 8.

I'd say last patch is a stretch due to the use of _PICK() and hardcoding
the map, but to me it seems to avoid making it more complex elsewhere.

Lucas De Marchi (4):
  drm/i915/gt: fix platform prefix
  drm/i915/gt: nuke unused legacy engine hw_id
  drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  drm/i915/gt: nuke gen6_hw_id

 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 10 ----------
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 12 ------------
 drivers/gpu/drm/i915/gt/intel_gt.c           |  4 ++--
 drivers/gpu/drm/i915/i915_reg.h              |  4 +++-
 4 files changed, 5 insertions(+), 25 deletions(-)

-- 
2.31.1

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* [PATCH 1/4] drm/i915/gt: fix platform prefix
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-20 23:20   ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin

gen8_clear_engine_error_register() is actually not used by
GRAPHICS_VER >= 8, since for those we are using another register that is
not engine-dependent. Fix the platform prefix, to make clear we are not
using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index e714e21c0a4d..a8efdd44e9cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
 	intel_uncore_rmw(uncore, reg, 0, 0);
 }
 
-static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
+static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
 {
 	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
 	GEN6_RING_FAULT_REG_POSTING_READ(engine);
@@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		enum intel_engine_id id;
 
 		for_each_engine_masked(engine, gt, engine_mask, id)
-			gen8_clear_engine_error_register(engine);
+			gen6_clear_engine_error_register(engine);
 	}
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [Intel-gfx] [PATCH 1/4] drm/i915/gt: fix platform prefix
@ 2021-07-20 23:20   ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, dri-devel

gen8_clear_engine_error_register() is actually not used by
GRAPHICS_VER >= 8, since for those we are using another register that is
not engine-dependent. Fix the platform prefix, to make clear we are not
using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index e714e21c0a4d..a8efdd44e9cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
 	intel_uncore_rmw(uncore, reg, 0, 0);
 }
 
-static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
+static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
 {
 	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
 	GEN6_RING_FAULT_REG_POSTING_READ(engine);
@@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
 		enum intel_engine_id id;
 
 		for_each_engine_masked(engine, gt, engine_mask, id)
-			gen8_clear_engine_error_register(engine);
+			gen6_clear_engine_error_register(engine);
 	}
 }
 
-- 
2.31.1

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

^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-20 23:20   ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin

The engine hw_id is only used by RING_FAULT_REG(), which is not used
since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
consistent, but let's try to remove them and let them defined to 0 when
not used.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d561573ed98c..a11f69f2e46e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS1] = {
-		.hw_id = VCS1_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 1,
 		.mmio_bases = {
@@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS2] = {
-		.hw_id = VCS2_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 2,
 		.mmio_bases = {
@@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS3] = {
-		.hw_id = VCS3_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 3,
 		.mmio_bases = {
@@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS1] = {
-		.hw_id = VECS1_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 1,
 		.mmio_bases = {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 1cb9c3b70b29..a107eb58ffa2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -34,10 +34,6 @@
 #define VCS0_HW		1
 #define BCS0_HW		2
 #define VECS0_HW	3
-#define VCS1_HW		4
-#define VCS2_HW		6
-#define VCS3_HW		7
-#define VECS1_HW	12
 
 /* Gen11+ HW Engine class + instance */
 #define RENDER_CLASS		0
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
@ 2021-07-20 23:20   ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, dri-devel

The engine hw_id is only used by RING_FAULT_REG(), which is not used
since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
consistent, but let's try to remove them and let them defined to 0 when
not used.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d561573ed98c..a11f69f2e46e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS1] = {
-		.hw_id = VCS1_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 1,
 		.mmio_bases = {
@@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS2] = {
-		.hw_id = VCS2_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 2,
 		.mmio_bases = {
@@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS3] = {
-		.hw_id = VCS3_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 3,
 		.mmio_bases = {
@@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS1] = {
-		.hw_id = VECS1_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 1,
 		.mmio_bases = {
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 1cb9c3b70b29..a107eb58ffa2 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -34,10 +34,6 @@
 #define VCS0_HW		1
 #define BCS0_HW		2
 #define VECS0_HW	3
-#define VCS1_HW		4
-#define VCS2_HW		6
-#define VCS3_HW		7
-#define VECS1_HW	12
 
 /* Gen11+ HW Engine class + instance */
 #define RENDER_CLASS		0
-- 
2.31.1

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

^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-20 23:20   ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin

We kept adding new engines and for that increasing hw_id unnecessarily:
it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
try to pack it in the structs to give a hint this field is actually not
used in recent platforms.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
 drivers/gpu/drm/i915/i915_reg.h              |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index a11f69f2e46e..508221de411c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,7 @@
 
 #define MAX_MMIO_BASES 3
 struct engine_info {
-	unsigned int hw_id;
+	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 	/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +54,7 @@ struct engine_info {
 
 static const struct engine_info intel_engines[] = {
 	[RCS0] = {
-		.hw_id = RCS0_HW,
+		.gen6_hw_id = RCS0_HW,
 		.class = RENDER_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[BCS0] = {
-		.hw_id = BCS0_HW,
+		.gen6_hw_id = BCS0_HW,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS0] = {
-		.hw_id = VCS0_HW,
+		.gen6_hw_id = VCS0_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS0] = {
-		.hw_id = VECS0_HW,
+		.gen6_hw_id = VECS0_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	engine->i915 = i915;
 	engine->gt = gt;
 	engine->uncore = gt->uncore;
-	engine->hw_id = info->hw_id;
+	engine->gen6_hw_id = info->gen6_hw_id;
 	guc_class = engine_class_to_guc_class(info->class);
 	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
 	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a107eb58ffa2..266422d8d1b1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -264,11 +264,11 @@ struct intel_engine_cs {
 	enum intel_engine_id id;
 	enum intel_engine_id legacy_idx;
 
-	unsigned int hw_id;
 	unsigned int guc_id;
 
 	intel_engine_mask_t mask;
 
+	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 943fe485c662..8750ffce9d61 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
 #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
 #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
 #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
 #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
@ 2021-07-20 23:20   ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, dri-devel

We kept adding new engines and for that increasing hw_id unnecessarily:
it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
try to pack it in the structs to give a hint this field is actually not
used in recent platforms.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
 drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
 drivers/gpu/drm/i915/i915_reg.h              |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index a11f69f2e46e..508221de411c 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,7 @@
 
 #define MAX_MMIO_BASES 3
 struct engine_info {
-	unsigned int hw_id;
+	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 	/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +54,7 @@ struct engine_info {
 
 static const struct engine_info intel_engines[] = {
 	[RCS0] = {
-		.hw_id = RCS0_HW,
+		.gen6_hw_id = RCS0_HW,
 		.class = RENDER_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[BCS0] = {
-		.hw_id = BCS0_HW,
+		.gen6_hw_id = BCS0_HW,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS0] = {
-		.hw_id = VCS0_HW,
+		.gen6_hw_id = VCS0_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS0] = {
-		.hw_id = VECS0_HW,
+		.gen6_hw_id = VECS0_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	engine->i915 = i915;
 	engine->gt = gt;
 	engine->uncore = gt->uncore;
-	engine->hw_id = info->hw_id;
+	engine->gen6_hw_id = info->gen6_hw_id;
 	guc_class = engine_class_to_guc_class(info->class);
 	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
 	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index a107eb58ffa2..266422d8d1b1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -264,11 +264,11 @@ struct intel_engine_cs {
 	enum intel_engine_id id;
 	enum intel_engine_id legacy_idx;
 
-	unsigned int hw_id;
 	unsigned int guc_id;
 
 	intel_engine_mask_t mask;
 
+	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 943fe485c662..8750ffce9d61 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
 #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
 #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
 #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
 #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
-- 
2.31.1

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

^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-20 23:20   ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, John Harrison, dri-devel, Tvrtko Ursulin

This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
recent platforms do not depend on this field, so it doesn't make much
sense to keep it generic like that. Instead, just do a mapping from
engine class to HW ID in the single place that is needed.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
 drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
 3 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 508221de411c..0a04e8d90e9e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,6 @@
 
 #define MAX_MMIO_BASES 3
 struct engine_info {
-	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 	/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +53,6 @@ struct engine_info {
 
 static const struct engine_info intel_engines[] = {
 	[RCS0] = {
-		.gen6_hw_id = RCS0_HW,
 		.class = RENDER_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[BCS0] = {
-		.gen6_hw_id = BCS0_HW,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS0] = {
-		.gen6_hw_id = VCS0_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS0] = {
-		.gen6_hw_id = VECS0_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	engine->i915 = i915;
 	engine->gt = gt;
 	engine->uncore = gt->uncore;
-	engine->gen6_hw_id = info->gen6_hw_id;
 	guc_class = engine_class_to_guc_class(info->class);
 	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
 	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 266422d8d1b1..64330bfb7641 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -28,13 +28,6 @@
 #include "intel_wakeref.h"
 #include "intel_workarounds_types.h"
 
-/* Legacy HW Engine ID */
-
-#define RCS0_HW		0
-#define VCS0_HW		1
-#define BCS0_HW		2
-#define VECS0_HW	3
-
 /* Gen11+ HW Engine class + instance */
 #define RENDER_CLASS		0
 #define VIDEO_DECODE_CLASS	1
@@ -268,7 +261,6 @@ struct intel_engine_cs {
 
 	intel_engine_mask_t mask;
 
-	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8750ffce9d61..d91386f4828e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
 #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
+
+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
 #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
 #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
 #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
@ 2021-07-20 23:20   ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-20 23:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tomas Winkler, dri-devel

This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
recent platforms do not depend on this field, so it doesn't make much
sense to keep it generic like that. Instead, just do a mapping from
engine class to HW ID in the single place that is needed.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
 drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
 3 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 508221de411c..0a04e8d90e9e 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -42,7 +42,6 @@
 
 #define MAX_MMIO_BASES 3
 struct engine_info {
-	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 	/* mmio bases table *must* be sorted in reverse graphics_ver order */
@@ -54,7 +53,6 @@ struct engine_info {
 
 static const struct engine_info intel_engines[] = {
 	[RCS0] = {
-		.gen6_hw_id = RCS0_HW,
 		.class = RENDER_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[BCS0] = {
-		.gen6_hw_id = BCS0_HW,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VCS0] = {
-		.gen6_hw_id = VCS0_HW,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
 		},
 	},
 	[VECS0] = {
-		.gen6_hw_id = VECS0_HW,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
 		.mmio_bases = {
@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	engine->i915 = i915;
 	engine->gt = gt;
 	engine->uncore = gt->uncore;
-	engine->gen6_hw_id = info->gen6_hw_id;
 	guc_class = engine_class_to_guc_class(info->class);
 	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
 	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 266422d8d1b1..64330bfb7641 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -28,13 +28,6 @@
 #include "intel_wakeref.h"
 #include "intel_workarounds_types.h"
 
-/* Legacy HW Engine ID */
-
-#define RCS0_HW		0
-#define VCS0_HW		1
-#define BCS0_HW		2
-#define VECS0_HW	3
-
 /* Gen11+ HW Engine class + instance */
 #define RENDER_CLASS		0
 #define VIDEO_DECODE_CLASS	1
@@ -268,7 +261,6 @@ struct intel_engine_cs {
 
 	intel_engine_mask_t mask;
 
-	u8 gen6_hw_id;
 	u8 class;
 	u8 instance;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8750ffce9d61..d91386f4828e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
 #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
 #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
+
+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
 #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
 #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
 #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
-- 
2.31.1

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

^ permalink raw reply related	[flat|nested] 37+ messages in thread
* [Intel-gfx] ✗ Fi.CI.DOCS: warning for Nuke legacy hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
                   ` (4 preceding siblings ...)
  (?)
@ 2021-07-21  0:35 ` Patchwork
  -1 siblings, 0 replies; 37+ messages in thread
From: Patchwork @ 2021-07-21  0:35 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: Nuke legacy hw_id
URL   : https://patchwork.freedesktop.org/series/92797/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'jump_whitelist' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'shadow_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'batch_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Function parameter or member 'trampoline' not described in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'jump_whitelist' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'shadow_map' description in 'intel_engine_cmd_parser'
./drivers/gpu/drm/i915/i915_cmd_parser.c:1436: warning: Excess function parameter 'batch_map' description in 'intel_engine_cmd_parser'


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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Nuke legacy hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
                   ` (5 preceding siblings ...)
  (?)
@ 2021-07-21  0:59 ` Patchwork
  -1 siblings, 0 replies; 37+ messages in thread
From: Patchwork @ 2021-07-21  0:59 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx


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

== Series Details ==

Series: Nuke legacy hw_id
URL   : https://patchwork.freedesktop.org/series/92797/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10359 -> Patchwork_20660
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][1] ([fdo#109271]) +23 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-soraka/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-cfl-8700k:       [PASS][3] -> [DMESG-FAIL][4] ([i915#2291] / [i915#541])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/fi-cfl-8700k/igt@i915_selftest@live@gt_heartbeat.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-cfl-8700k/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][5] ([i915#1886] / [i915#2291])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-soraka/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [PASS][7] -> [FAIL][8] ([i915#1372])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#533])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-kbl-soraka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][10] ([i915#1602] / [i915#2029])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/fi-bdw-5557u/igt@runner@aborted.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#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1372]: https://gitlab.freedesktop.org/drm/intel/issues/1372
  [i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Participating hosts (38 -> 34)
------------------------------

  Missing    (4): fi-ilk-m540 fi-hsw-4200u fi-bdw-samus fi-bsw-n3050 


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

  * Linux: CI_DRM_10359 -> Patchwork_20660

  CI-20190529: 20190529
  CI_DRM_10359: f7edad825d9dd6a1387bfa9fafd28c112cfc45a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6146: 6caef22e4aafed275771f564d4ea4cab09896ebc @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20660: 2ffc2fcfc778d21cd84dfd15cd967647db5ae574 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2ffc2fcfc778 drm/i915/gt: nuke gen6_hw_id
127ba2e87242 drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
8686676b120a drm/i915/gt: nuke unused legacy engine hw_id
21e1464390fa drm/i915/gt: fix platform prefix

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 5456 bytes --]

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

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for Nuke legacy hw_id
  2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
                   ` (6 preceding siblings ...)
  (?)
@ 2021-07-21  3:46 ` Patchwork
  -1 siblings, 0 replies; 37+ messages in thread
From: Patchwork @ 2021-07-21  3:46 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx


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

== Series Details ==

Series: Nuke legacy hw_id
URL   : https://patchwork.freedesktop.org/series/92797/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10359_full -> Patchwork_20660_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Suppressed ####

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

  * igt@kms_ccs@pipe-a-bad-rotation-90-yf_tiled_ccs:
    - {shard-rkl}:        [FAIL][1] ([i915#3678]) -> [SKIP][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_ccs@pipe-a-bad-rotation-90-yf_tiled_ccs.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_ccs@pipe-a-bad-rotation-90-yf_tiled_ccs.html

  * igt@sysfs_heartbeat_interval@precise@bcs0:
    - {shard-rkl}:        [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-1/igt@sysfs_heartbeat_interval@precise@bcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-1/igt@sysfs_heartbeat_interval@precise@bcs0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@display-4x:
    - shard-tglb:         NOTRUN -> [SKIP][5] ([i915#1839])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@feature_discovery@display-4x.html

  * igt@gem_ctx_persistence@legacy-engines-queued:
    - shard-snb:          NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#1099]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-snb5/igt@gem_ctx_persistence@legacy-engines-queued.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-tglb:         [PASS][7] -> [FAIL][8] ([i915#2842]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb6/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#2842]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-glk5/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-glk8/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][11] -> [FAIL][12] ([i915#2842]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl2/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([i915#2842])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb7/igt@gem_exec_fair@basic-pace@vcs0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_exec_whisper@basic-fds-all:
    - shard-glk:          [PASS][15] -> [DMESG-WARN][16] ([i915#118] / [i915#95])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-glk9/igt@gem_exec_whisper@basic-fds-all.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-glk5/igt@gem_exec_whisper@basic-fds-all.html

  * igt@gem_mmap_gtt@cpuset-medium-copy-odd:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([i915#307])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb4/igt@gem_mmap_gtt@cpuset-medium-copy-odd.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb1/igt@gem_mmap_gtt@cpuset-medium-copy-odd.html

  * igt@gem_pread@exhaustion:
    - shard-iclb:         NOTRUN -> [WARN][19] ([i915#2658])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@gem_pread@exhaustion.html

  * igt@gem_render_copy@y-tiled-to-vebox-linear:
    - shard-iclb:         NOTRUN -> [SKIP][20] ([i915#768])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb1/igt@gem_render_copy@y-tiled-to-vebox-linear.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-apl:          NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3323])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl6/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@input-checking:
    - shard-apl:          NOTRUN -> [DMESG-WARN][22] ([i915#3002])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl8/igt@gem_userptr_blits@input-checking.html

  * igt@gem_userptr_blits@vma-merge:
    - shard-tglb:         NOTRUN -> [FAIL][23] ([i915#3318])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@gem_userptr_blits@vma-merge.html
    - shard-skl:          NOTRUN -> [FAIL][24] ([i915#3318])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl7/igt@gem_userptr_blits@vma-merge.html

  * igt@i915_selftest@mock@requests:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][25] ([i915#3746]) +17 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl4/igt@i915_selftest@mock@requests.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-apl:          NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#3777]) +2 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl6/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-skl:          NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#3777])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl1/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-async-flip:
    - shard-tglb:         NOTRUN -> [SKIP][28] ([fdo#111615])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html

  * igt@kms_chamelium@hdmi-hpd:
    - shard-skl:          NOTRUN -> [SKIP][29] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl1/igt@kms_chamelium@hdmi-hpd.html

  * igt@kms_chamelium@hdmi-hpd-enable-disable-mode:
    - shard-snb:          NOTRUN -> [SKIP][30] ([fdo#109271] / [fdo#111827]) +13 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-snb5/igt@kms_chamelium@hdmi-hpd-enable-disable-mode.html

  * igt@kms_chamelium@vga-hpd-after-suspend:
    - shard-iclb:         NOTRUN -> [SKIP][31] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_chamelium@vga-hpd-after-suspend.html

  * igt@kms_chamelium@vga-hpd-for-each-pipe:
    - shard-kbl:          NOTRUN -> [SKIP][32] ([fdo#109271] / [fdo#111827])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl4/igt@kms_chamelium@vga-hpd-for-each-pipe.html

  * igt@kms_color_chamelium@pipe-a-ctm-limited-range:
    - shard-apl:          NOTRUN -> [SKIP][33] ([fdo#109271] / [fdo#111827]) +15 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl1/igt@kms_color_chamelium@pipe-a-ctm-limited-range.html

  * igt@kms_color_chamelium@pipe-d-ctm-0-25:
    - shard-tglb:         NOTRUN -> [SKIP][34] ([fdo#109284] / [fdo#111827])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@kms_color_chamelium@pipe-d-ctm-0-25.html

  * igt@kms_content_protection@lic:
    - shard-apl:          NOTRUN -> [TIMEOUT][35] ([i915#1319])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl6/igt@kms_content_protection@lic.html

  * igt@kms_content_protection@srm:
    - shard-kbl:          NOTRUN -> [TIMEOUT][36] ([i915#1319])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl4/igt@kms_content_protection@srm.html

  * igt@kms_cursor_crc@pipe-a-cursor-32x10-sliding:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([i915#3359])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@kms_cursor_crc@pipe-a-cursor-32x10-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [PASS][38] -> [DMESG-WARN][39] ([i915#180]) +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-d-cursor-64x64-random:
    - shard-iclb:         NOTRUN -> [SKIP][40] ([fdo#109278]) +13 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_cursor_crc@pipe-d-cursor-64x64-random.html

  * igt@kms_cursor_legacy@cursora-vs-flipb-atomic-transitions-varying-size:
    - shard-iclb:         NOTRUN -> [SKIP][41] ([fdo#109274] / [fdo#109278])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_cursor_legacy@cursora-vs-flipb-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][42] -> [FAIL][43] ([i915#2346] / [i915#533])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][44] -> [DMESG-WARN][45] ([i915#1982])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl8/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl2/igt@kms_cursor_legacy@short-flip-after-cursor-atomic-transitions-varying-size.html

  * igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled:
    - shard-skl:          [PASS][46] -> [FAIL][47] ([i915#3451])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl1/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl9/igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [PASS][48] -> [INCOMPLETE][49] ([i915#155] / [i915#180] / [i915#636])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl1/igt@kms_fbcon_fbt@fbc-suspend.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl3/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@2x-flip-vs-modeset-vs-hang:
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109274])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb1/igt@kms_flip@2x-flip-vs-modeset-vs-hang.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-apl:          NOTRUN -> [DMESG-WARN][51] ([i915#180]) +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-apl:          [PASS][52] -> [DMESG-WARN][53] ([i915#180]) +1 similar issue
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-apl3/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl1/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs:
    - shard-apl:          NOTRUN -> [SKIP][54] ([fdo#109271] / [i915#2672])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl6/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-iclb:         NOTRUN -> [SKIP][55] ([fdo#109280]) +4 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-msflip-blt:
    - shard-tglb:         NOTRUN -> [SKIP][56] ([fdo#111825]) +6 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-apl:          NOTRUN -> [SKIP][57] ([fdo#109271]) +193 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-cur-indfb-draw-pwrite:
    - shard-kbl:          NOTRUN -> [SKIP][58] ([fdo#109271]) +16 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl4/igt@kms_frontbuffer_tracking@psr-2p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_hdr@static-toggle-suspend:
    - shard-iclb:         NOTRUN -> [SKIP][59] ([i915#1187])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_hdr@static-toggle-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][60] ([fdo#108145] / [i915#265])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl7/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][61] ([i915#265]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl8/igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          NOTRUN -> [FAIL][62] ([fdo#108145] / [i915#265])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-b-tiling-none:
    - shard-iclb:         NOTRUN -> [SKIP][63] ([i915#3536])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_plane_lowres@pipe-b-tiling-none.html

  * igt@kms_psr2_sf@cursor-plane-update-sf:
    - shard-skl:          NOTRUN -> [SKIP][64] ([fdo#109271] / [i915#658])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl8/igt@kms_psr2_sf@cursor-plane-update-sf.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][65] ([fdo#109271] / [i915#658]) +3 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl6/igt@kms_psr2_sf@plane-move-sf-dmg-area-2.html

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         NOTRUN -> [SKIP][66] ([fdo#109441])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@kms_psr@psr2_primary_mmap_gtt.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-tglb:         NOTRUN -> [FAIL][67] ([i915#132] / [i915#3467])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [PASS][68] -> [SKIP][69] ([fdo#109441])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb2/igt@kms_psr@psr2_suspend.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb6/igt@kms_psr@psr2_suspend.html

  * igt@kms_vblank@pipe-d-query-forked-hang:
    - shard-snb:          NOTRUN -> [SKIP][70] ([fdo#109271]) +249 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-snb2/igt@kms_vblank@pipe-d-query-forked-hang.html

  * igt@kms_writeback@writeback-check-output:
    - shard-apl:          NOTRUN -> [SKIP][71] ([fdo#109271] / [i915#2437])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl8/igt@kms_writeback@writeback-check-output.html

  * igt@nouveau_crc@pipe-d-ctx-flip-detection:
    - shard-skl:          NOTRUN -> [SKIP][72] ([fdo#109271]) +33 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl8/igt@nouveau_crc@pipe-d-ctx-flip-detection.html

  * igt@perf@polling-parameterized:
    - shard-iclb:         [PASS][73] -> [FAIL][74] ([i915#1542])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb2/igt@perf@polling-parameterized.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb6/igt@perf@polling-parameterized.html

  * igt@prime_vgem@coherency-gtt:
    - shard-tglb:         NOTRUN -> [SKIP][75] ([fdo#111656])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@prime_vgem@coherency-gtt.html

  * igt@sysfs_clients@fair-1:
    - shard-apl:          NOTRUN -> [SKIP][76] ([fdo#109271] / [i915#2994]) +2 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl8/igt@sysfs_clients@fair-1.html

  * igt@sysfs_clients@sema-10:
    - shard-iclb:         NOTRUN -> [SKIP][77] ([i915#2994])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb8/igt@sysfs_clients@sema-10.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-deadline:
    - {shard-rkl}:        [FAIL][78] ([i915#2846]) -> [PASS][79]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-1/igt@gem_exec_fair@basic-deadline.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-1/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-tglb:         [FAIL][80] ([i915#2842]) -> [PASS][81] +1 similar issue
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-tglb3/igt@gem_exec_fair@basic-none-share@rcs0.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-tglb3/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-kbl:          [FAIL][82] ([i915#2842]) -> [PASS][83] +1 similar issue
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl2/igt@gem_exec_fair@basic-pace@rcs0.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl3/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-glk:          [FAIL][84] ([i915#2842]) -> [PASS][85] +1 similar issue
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-glk4/igt@gem_exec_fair@basic-throttle@rcs0.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-glk5/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_schedule@u-independent@vecs0:
    - shard-kbl:          [FAIL][86] ([i915#3795]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl1/igt@gem_exec_schedule@u-independent@vecs0.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl2/igt@gem_exec_schedule@u-independent@vecs0.html

  * igt@gem_exec_whisper@basic-queues-all:
    - shard-glk:          [DMESG-WARN][88] ([i915#118] / [i915#95]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-glk3/igt@gem_exec_whisper@basic-queues-all.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-glk3/igt@gem_exec_whisper@basic-queues-all.html

  * igt@gem_mmap_offset@clear:
    - shard-skl:          [FAIL][90] ([i915#1888] / [i915#3160]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl2/igt@gem_mmap_offset@clear.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl8/igt@gem_mmap_offset@clear.html

  * igt@gem_mmap_wc@set-cache-level:
    - {shard-rkl}:        [SKIP][92] ([i915#1850]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@gem_mmap_wc@set-cache-level.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@gem_mmap_wc@set-cache-level.html

  * igt@gem_workarounds@suspend-resume:
    - shard-kbl:          [DMESG-WARN][94] ([i915#180]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl3/igt@gem_workarounds@suspend-resume.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl4/igt@gem_workarounds@suspend-resume.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][96] ([i915#454]) -> [PASS][97]
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@basic-rte:
    - {shard-rkl}:        [SKIP][98] ([fdo#109308]) -> [PASS][99]
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@i915_pm_rpm@basic-rte.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_selftest@live@gt_pm:
    - {shard-rkl}:        [DMESG-FAIL][100] ([i915#1021]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-5/igt@i915_selftest@live@gt_pm.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-2/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
    - shard-snb:          [INCOMPLETE][102] ([i915#2782]) -> [PASS][103]
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-snb6/igt@i915_selftest@live@hangcheck.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-snb5/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@mock@vma:
    - shard-skl:          [DMESG-WARN][104] ([i915#3746]) -> [PASS][105] +7 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl10/igt@i915_selftest@mock@vma.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl1/igt@i915_selftest@mock@vma.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-apl:          [DMESG-WARN][106] ([i915#180]) -> [PASS][107] +1 similar issue
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-apl2/igt@i915_suspend@fence-restore-untiled.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-apl7/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_async_flips@alternate-sync-async-flip:
    - shard-skl:          [FAIL][108] ([i915#2521]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl7/igt@kms_async_flips@alternate-sync-async-flip.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl4/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_atomic@test-only:
    - {shard-rkl}:        [SKIP][110] ([i915#1845]) -> [PASS][111] +5 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_atomic@test-only.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_atomic@test-only.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0:
    - {shard-rkl}:        [SKIP][112] ([i915#3721]) -> [PASS][113] +1 similar issue
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_big_fb@y-tiled-16bpp-rotate-0:
    - {shard-rkl}:        [SKIP][114] ([i915#3638]) -> [PASS][115] +1 similar issue
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_big_fb@y-tiled-16bpp-rotate-0.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_big_fb@y-tiled-16bpp-rotate-0.html

  * igt@kms_color@pipe-c-ctm-0-5:
    - {shard-rkl}:        [SKIP][116] ([i915#1149] / [i915#1849]) -> [PASS][117] +1 similar issue
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_color@pipe-c-ctm-0-5.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_color@pipe-c-ctm-0-5.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x128-offscreen:
    - {shard-rkl}:        [SKIP][118] ([fdo#112022]) -> [PASS][119] +2 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_cursor_crc@pipe-b-cursor-128x128-offscreen.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_cursor_crc@pipe-b-cursor-128x128-offscreen.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x128-sliding:
    - shard-skl:          [FAIL][120] ([i915#3444]) -> [PASS][121]
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl8/igt@kms_cursor_crc@pipe-b-cursor-128x128-sliding.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl2/igt@kms_cursor_crc@pipe-b-cursor-128x128-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [INCOMPLETE][122] ([i915#300]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic:
    - {shard-rkl}:        [SKIP][124] ([fdo#111825]) -> [PASS][125]
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-atomic.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled:
    - {shard-rkl}:        [SKIP][126] ([fdo#111314]) -> [PASS][127] +1 similar issue
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-skl:          [DMESG-WARN][128] ([i915#1982]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-skl7/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - {shard-rkl}:        [SKIP][130] ([i915#1849]) -> [PASS][131] +10 similar issues
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-vs-premult-vs-constant:
    - shard-iclb:         [SKIP][132] ([fdo#109278]) -> [PASS][133]
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb2/igt@kms_plane_alpha_blend@pipe-c-coverage-vs-premult-vs-constant.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb6/igt@kms_plane_alpha_blend@pipe-c-coverage-vs-premult-vs-constant.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [SKIP][134] ([fdo#109441]) -> [PASS][135]
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb4/igt@kms_psr@psr2_sprite_render.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb2/igt@kms_psr@psr2_sprite_render.html

  * igt@kms_psr@sprite_mmap_cpu:
    - {shard-rkl}:        [SKIP][136] ([i915#1072]) -> [PASS][137]
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-rkl-2/igt@kms_psr@sprite_mmap_cpu.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-rkl-6/igt@kms_psr@sprite_mmap_cpu.html

  * igt@perf@polling-parameterized:
    - shard-glk:          [FAIL][138] ([i915#1542]) -> [PASS][139]
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-glk4/igt@perf@polling-parameterized.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-glk3/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-kbl:          [FAIL][140] ([i915#2842]) -> [SKIP][141] ([fdo#109271])
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-kbl2/igt@gem_exec_fair@basic-pace@vcs0.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-kbl3/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@i915_pm_rc6_residency@rc6-fence:
    - shard-iclb:         [WARN][142] ([i915#2684]) -> [WARN][143] ([i915#1804] / [i915#2684]) +1 similar issue
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-iclb5/igt@i915_pm_rc6_residency@rc6-fence.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20660/shard-iclb4/igt@i915_pm_rc6_residency@rc6-fence.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          [FAIL][144] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][145] ([fdo#108145] / [i915#1982] / [i915#265])
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10359/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [145]: https://intel-gfx-ci.01.

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 33426 bytes --]

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

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/gt: fix platform prefix
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21  9:11     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:11 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
>   	intel_uncore_rmw(uncore, reg, 0, 0);
>   }
>   
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
>   {
>   	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
>   	GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
>   		enum intel_engine_id id;
>   
>   		for_each_engine_masked(engine, gt, engine_mask, id)
> -			gen8_clear_engine_error_register(engine);
> +			gen6_clear_engine_error_register(engine);
>   	}
>   }
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/gt: fix platform prefix
@ 2021-07-21  9:11     ` Tvrtko Ursulin
  0 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:11 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
>   	intel_uncore_rmw(uncore, reg, 0, 0);
>   }
>   
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
>   {
>   	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
>   	GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
>   		enum intel_engine_id id;
>   
>   		for_each_engine_masked(engine, gt, engine_mask, id)
> -			gen8_clear_engine_error_register(engine);
> +			gen6_clear_engine_error_register(engine);
>   	}
>   }
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21  9:18     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:18 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
>   drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
>   2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS1] = {
> -		.hw_id = VCS1_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 1,
>   		.mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS2] = {
> -		.hw_id = VCS2_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 2,
>   		.mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS3] = {
> -		.hw_id = VCS3_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 3,
>   		.mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS1] = {
> -		.hw_id = VECS1_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 1,
>   		.mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
>   #define VCS0_HW		1
>   #define BCS0_HW		2
>   #define VECS0_HW	3
> -#define VCS1_HW		4
> -#define VCS2_HW		6
> -#define VCS3_HW		7
> -#define VECS1_HW	12
>   
>   /* Gen11+ HW Engine class + instance */
>   #define RENDER_CLASS		0
> 

Story checks out AFAICS.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
@ 2021-07-21  9:18     ` Tvrtko Ursulin
  0 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:18 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
>   drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
>   2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS1] = {
> -		.hw_id = VCS1_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 1,
>   		.mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS2] = {
> -		.hw_id = VCS2_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 2,
>   		.mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS3] = {
> -		.hw_id = VCS3_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 3,
>   		.mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS1] = {
> -		.hw_id = VECS1_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 1,
>   		.mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
>   #define VCS0_HW		1
>   #define BCS0_HW		2
>   #define VECS0_HW	3
> -#define VCS1_HW		4
> -#define VCS2_HW		6
> -#define VCS3_HW		7
> -#define VECS1_HW	12
>   
>   /* Gen11+ HW Engine class + instance */
>   #define RENDER_CLASS		0
> 

Story checks out AFAICS.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21  9:20     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:20 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
>   drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
>   drivers/gpu/drm/i915/i915_reg.h              |  2 +-
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>   
>   #define MAX_MMIO_BASES 3
>   struct engine_info {
> -	unsigned int hw_id;
> +	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>   
>   static const struct engine_info intel_engines[] = {
>   	[RCS0] = {
> -		.hw_id = RCS0_HW,
> +		.gen6_hw_id = RCS0_HW,
>   		.class = RENDER_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[BCS0] = {
> -		.hw_id = BCS0_HW,
> +		.gen6_hw_id = BCS0_HW,
>   		.class = COPY_ENGINE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS0] = {
> -		.hw_id = VCS0_HW,
> +		.gen6_hw_id = VCS0_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS0] = {
> -		.hw_id = VECS0_HW,
> +		.gen6_hw_id = VECS0_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	engine->i915 = i915;
>   	engine->gt = gt;
>   	engine->uncore = gt->uncore;
> -	engine->hw_id = info->hw_id;
> +	engine->gen6_hw_id = info->gen6_hw_id;
>   	guc_class = engine_class_to_guc_class(info->class);
>   	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>   	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
>   	enum intel_engine_id id;
>   	enum intel_engine_id legacy_idx;
>   
> -	unsigned int hw_id;
>   	unsigned int guc_id;
>   
>   	intel_engine_mask_t mask;
>   
> +	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>   #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>   #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>   #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>   #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>   #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
@ 2021-07-21  9:20     ` Tvrtko Ursulin
  0 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:20 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
>   drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
>   drivers/gpu/drm/i915/i915_reg.h              |  2 +-
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>   
>   #define MAX_MMIO_BASES 3
>   struct engine_info {
> -	unsigned int hw_id;
> +	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>   
>   static const struct engine_info intel_engines[] = {
>   	[RCS0] = {
> -		.hw_id = RCS0_HW,
> +		.gen6_hw_id = RCS0_HW,
>   		.class = RENDER_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[BCS0] = {
> -		.hw_id = BCS0_HW,
> +		.gen6_hw_id = BCS0_HW,
>   		.class = COPY_ENGINE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS0] = {
> -		.hw_id = VCS0_HW,
> +		.gen6_hw_id = VCS0_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS0] = {
> -		.hw_id = VECS0_HW,
> +		.gen6_hw_id = VECS0_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	engine->i915 = i915;
>   	engine->gt = gt;
>   	engine->uncore = gt->uncore;
> -	engine->hw_id = info->hw_id;
> +	engine->gen6_hw_id = info->gen6_hw_id;
>   	guc_class = engine_class_to_guc_class(info->class);
>   	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>   	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
>   	enum intel_engine_id id;
>   	enum intel_engine_id legacy_idx;
>   
> -	unsigned int hw_id;
>   	unsigned int guc_id;
>   
>   	intel_engine_mask_t mask;
>   
> +	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>   #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>   #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>   #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>   #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>   #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21  9:25     ` Tvrtko Ursulin
  -1 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:25 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>   drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>   drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>   3 files changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>   
>   #define MAX_MMIO_BASES 3
>   struct engine_info {
> -	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>   
>   static const struct engine_info intel_engines[] = {
>   	[RCS0] = {
> -		.gen6_hw_id = RCS0_HW,
>   		.class = RENDER_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[BCS0] = {
> -		.gen6_hw_id = BCS0_HW,
>   		.class = COPY_ENGINE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS0] = {
> -		.gen6_hw_id = VCS0_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS0] = {
> -		.gen6_hw_id = VECS0_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	engine->i915 = i915;
>   	engine->gt = gt;
>   	engine->uncore = gt->uncore;
> -	engine->gen6_hw_id = info->gen6_hw_id;
>   	guc_class = engine_class_to_guc_class(info->class);
>   	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>   	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
>   #include "intel_wakeref.h"
>   #include "intel_workarounds_types.h"
>   
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW		0
> -#define VCS0_HW		1
> -#define BCS0_HW		2
> -#define VECS0_HW	3
> -
>   /* Gen11+ HW Engine class + instance */
>   #define RENDER_CLASS		0
>   #define VIDEO_DECODE_CLASS	1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>   
>   	intel_engine_mask_t mask;
>   
> -	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>   #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>   #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))

Makes sense to me.

Maybe HW_ID and HW_CLASS in the macro name? Not sure.

Only open I have is why the "Gen11+ HW Engine class + instance" comment 
and now we would tie that, allegedly Gen11 concept, with Gen6-7. Care to 
do some digging?

Regards,

Tvrtko

>   #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>   #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>   #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
@ 2021-07-21  9:25     ` Tvrtko Ursulin
  0 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-21  9:25 UTC (permalink / raw)
  To: Lucas De Marchi, intel-gfx; +Cc: Tomas Winkler, dri-devel


On 21/07/2021 00:20, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>   drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>   drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>   3 files changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>   
>   #define MAX_MMIO_BASES 3
>   struct engine_info {
> -	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>   
>   static const struct engine_info intel_engines[] = {
>   	[RCS0] = {
> -		.gen6_hw_id = RCS0_HW,
>   		.class = RENDER_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[BCS0] = {
> -		.gen6_hw_id = BCS0_HW,
>   		.class = COPY_ENGINE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VCS0] = {
> -		.gen6_hw_id = VCS0_HW,
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>   		},
>   	},
>   	[VECS0] = {
> -		.gen6_hw_id = VECS0_HW,
>   		.class = VIDEO_ENHANCEMENT_CLASS,
>   		.instance = 0,
>   		.mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>   	engine->i915 = i915;
>   	engine->gt = gt;
>   	engine->uncore = gt->uncore;
> -	engine->gen6_hw_id = info->gen6_hw_id;
>   	guc_class = engine_class_to_guc_class(info->class);
>   	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>   	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
>   #include "intel_wakeref.h"
>   #include "intel_workarounds_types.h"
>   
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW		0
> -#define VCS0_HW		1
> -#define BCS0_HW		2
> -#define VECS0_HW	3
> -
>   /* Gen11+ HW Engine class + instance */
>   #define RENDER_CLASS		0
>   #define VIDEO_DECODE_CLASS	1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>   
>   	intel_engine_mask_t mask;
>   
> -	u8 gen6_hw_id;
>   	u8 class;
>   	u8 instance;
>   
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>   #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>   #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))

Makes sense to me.

Maybe HW_ID and HW_CLASS in the macro name? Not sure.

Only open I have is why the "Gen11+ HW Engine class + instance" comment 
and now we would tie that, allegedly Gen11 concept, with Gen6-7. Care to 
do some digging?

Regards,

Tvrtko

>   #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>   #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>   #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
  2021-07-21  9:25     ` Tvrtko Ursulin
@ 2021-07-21 18:44       ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-21 18:44 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: intel-gfx, Daniele Ceraolo Spurio, Tomas Winkler, dri-devel

On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>
>On 21/07/2021 00:20, Lucas De Marchi wrote:
>>This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>recent platforms do not depend on this field, so it doesn't make much
>>sense to keep it generic like that. Instead, just do a mapping from
>>engine class to HW ID in the single place that is needed.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>---
>>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>>  3 files changed, 3 insertions(+), 15 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>index 508221de411c..0a04e8d90e9e 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>@@ -42,7 +42,6 @@
>>  #define MAX_MMIO_BASES 3
>>  struct engine_info {
>>-	u8 gen6_hw_id;
>>  	u8 class;
>>  	u8 instance;
>>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
>>@@ -54,7 +53,6 @@ struct engine_info {
>>  static const struct engine_info intel_engines[] = {
>>  	[RCS0] = {
>>-		.gen6_hw_id = RCS0_HW,
>>  		.class = RENDER_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[BCS0] = {
>>-		.gen6_hw_id = BCS0_HW,
>>  		.class = COPY_ENGINE_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[VCS0] = {
>>-		.gen6_hw_id = VCS0_HW,
>>  		.class = VIDEO_DECODE_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[VECS0] = {
>>-		.gen6_hw_id = VECS0_HW,
>>  		.class = VIDEO_ENHANCEMENT_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>>  	engine->i915 = i915;
>>  	engine->gt = gt;
>>  	engine->uncore = gt->uncore;
>>-	engine->gen6_hw_id = info->gen6_hw_id;
>>  	guc_class = engine_class_to_guc_class(info->class);
>>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>index 266422d8d1b1..64330bfb7641 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>@@ -28,13 +28,6 @@
>>  #include "intel_wakeref.h"
>>  #include "intel_workarounds_types.h"
>>-/* Legacy HW Engine ID */
>>-
>>-#define RCS0_HW		0
>>-#define VCS0_HW		1
>>-#define BCS0_HW		2
>>-#define VECS0_HW	3
>>-
>>  /* Gen11+ HW Engine class + instance */
>>  #define RENDER_CLASS		0
>>  #define VIDEO_DECODE_CLASS	1
>>@@ -268,7 +261,6 @@ struct intel_engine_cs {
>>  	intel_engine_mask_t mask;
>>-	u8 gen6_hw_id;
>>  	u8 class;
>>  	u8 instance;
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index 8750ffce9d61..d91386f4828e 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
>>-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>>+
>>+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>
>Makes sense to me.
>
>Maybe HW_ID and HW_CLASS in the macro name? Not sure.

I can do that... I think I avoided it because it makes the macro
very big. Anyway, this should be called in just one place, so it doesn't
matter much... I can add it.

>
>Only open I have is why the "Gen11+ HW Engine class + instance" 
>comment and now we would tie that, allegedly Gen11 concept, with 
>Gen6-7. Care to do some digging?

Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out of i915_reg.h")
that I reviewed :-o

Cc'ing  Daniele. I don't see "class" as a Gen11+ thing. Is it just that
those numbers started to make sense for gen11?  Since

a) we are using the class even for GRAPHICS_VER < 11
b) the legacy HW IDs shouldn't be used anywhere else anymore


we could

1) move the legacy defines back to i915_reg.h
2) use them in the macro above (IMO would slightly improve the
readability of that _PICK() call)
3) Remove the "Gen11+" comment in the _CLASS macros to avoid
misunderstandings


Thoughts?

thanks
Lucas De Marchi



>
>Regards,
>
>Tvrtko
>
>>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
>>

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
@ 2021-07-21 18:44       ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-21 18:44 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>
>On 21/07/2021 00:20, Lucas De Marchi wrote:
>>This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>recent platforms do not depend on this field, so it doesn't make much
>>sense to keep it generic like that. Instead, just do a mapping from
>>engine class to HW ID in the single place that is needed.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>---
>>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>>  3 files changed, 3 insertions(+), 15 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>index 508221de411c..0a04e8d90e9e 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>@@ -42,7 +42,6 @@
>>  #define MAX_MMIO_BASES 3
>>  struct engine_info {
>>-	u8 gen6_hw_id;
>>  	u8 class;
>>  	u8 instance;
>>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
>>@@ -54,7 +53,6 @@ struct engine_info {
>>  static const struct engine_info intel_engines[] = {
>>  	[RCS0] = {
>>-		.gen6_hw_id = RCS0_HW,
>>  		.class = RENDER_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[BCS0] = {
>>-		.gen6_hw_id = BCS0_HW,
>>  		.class = COPY_ENGINE_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[VCS0] = {
>>-		.gen6_hw_id = VCS0_HW,
>>  		.class = VIDEO_DECODE_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>>  		},
>>  	},
>>  	[VECS0] = {
>>-		.gen6_hw_id = VECS0_HW,
>>  		.class = VIDEO_ENHANCEMENT_CLASS,
>>  		.instance = 0,
>>  		.mmio_bases = {
>>@@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>>  	engine->i915 = i915;
>>  	engine->gt = gt;
>>  	engine->uncore = gt->uncore;
>>-	engine->gen6_hw_id = info->gen6_hw_id;
>>  	guc_class = engine_class_to_guc_class(info->class);
>>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>index 266422d8d1b1..64330bfb7641 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>@@ -28,13 +28,6 @@
>>  #include "intel_wakeref.h"
>>  #include "intel_workarounds_types.h"
>>-/* Legacy HW Engine ID */
>>-
>>-#define RCS0_HW		0
>>-#define VCS0_HW		1
>>-#define BCS0_HW		2
>>-#define VECS0_HW	3
>>-
>>  /* Gen11+ HW Engine class + instance */
>>  #define RENDER_CLASS		0
>>  #define VIDEO_DECODE_CLASS	1
>>@@ -268,7 +261,6 @@ struct intel_engine_cs {
>>  	intel_engine_mask_t mask;
>>-	u8 gen6_hw_id;
>>  	u8 class;
>>  	u8 instance;
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index 8750ffce9d61..d91386f4828e 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
>>-#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>>+
>>+#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>+#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>
>Makes sense to me.
>
>Maybe HW_ID and HW_CLASS in the macro name? Not sure.

I can do that... I think I avoided it because it makes the macro
very big. Anyway, this should be called in just one place, so it doesn't
matter much... I can add it.

>
>Only open I have is why the "Gen11+ HW Engine class + instance" 
>comment and now we would tie that, allegedly Gen11 concept, with 
>Gen6-7. Care to do some digging?

Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out of i915_reg.h")
that I reviewed :-o

Cc'ing  Daniele. I don't see "class" as a Gen11+ thing. Is it just that
those numbers started to make sense for gen11?  Since

a) we are using the class even for GRAPHICS_VER < 11
b) the legacy HW IDs shouldn't be used anywhere else anymore


we could

1) move the legacy defines back to i915_reg.h
2) use them in the macro above (IMO would slightly improve the
readability of that _PICK() call)
3) Remove the "Gen11+" comment in the _CLASS macros to avoid
misunderstandings


Thoughts?

thanks
Lucas De Marchi



>
>Regards,
>
>Tvrtko
>
>>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] drm/i915/gt: fix platform prefix
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21 22:34     ` Matt Roper
  -1 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:34 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison

On Tue, Jul 20, 2021 at 04:20:11PM -0700, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
>  	intel_uncore_rmw(uncore, reg, 0, 0);
>  }
>  
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
>  {
>  	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
>  	GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
>  		enum intel_engine_id id;
>  
>  		for_each_engine_masked(engine, gt, engine_mask, id)
> -			gen8_clear_engine_error_register(engine);
> +			gen6_clear_engine_error_register(engine);
>  	}
>  }
>  
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915/gt: fix platform prefix
@ 2021-07-21 22:34     ` Matt Roper
  0 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:34 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Tue, Jul 20, 2021 at 04:20:11PM -0700, Lucas De Marchi wrote:
> gen8_clear_engine_error_register() is actually not used by
> GRAPHICS_VER >= 8, since for those we are using another register that is
> not engine-dependent. Fix the platform prefix, to make clear we are not
> using any GEN6_RING_FAULT_REG_* one GRAPHICS_VER >= 8.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index e714e21c0a4d..a8efdd44e9cf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -205,7 +205,7 @@ static void clear_register(struct intel_uncore *uncore, i915_reg_t reg)
>  	intel_uncore_rmw(uncore, reg, 0, 0);
>  }
>  
> -static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> +static void gen6_clear_engine_error_register(struct intel_engine_cs *engine)
>  {
>  	GEN6_RING_FAULT_REG_RMW(engine, RING_FAULT_VALID, 0);
>  	GEN6_RING_FAULT_REG_POSTING_READ(engine);
> @@ -251,7 +251,7 @@ intel_gt_clear_error_registers(struct intel_gt *gt,
>  		enum intel_engine_id id;
>  
>  		for_each_engine_masked(engine, gt, engine_mask, id)
> -			gen8_clear_engine_error_register(engine);
> +			gen6_clear_engine_error_register(engine);
>  	}
>  }
>  
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21 22:47     ` Matt Roper
  -1 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:47 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison

On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.

s/when not used/for engines that only exist on gen8+ platforms/

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

For historical reference, we did use hw_id on gen8+ platforms too until
relatively recently --- it was used to set the engine's guc_id as well
up until:

        commit c784e5249e773689e38d2bc1749f08b986621a26
        Author: John Harrison <John.C.Harrison@Intel.com>
        Date:   Wed Oct 28 07:58:24 2020 -0700

            drm/i915/guc: Update to use firmware v49.0.1


Matt

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
>  2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS1] = {
> -		.hw_id = VCS1_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 1,
>  		.mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS2] = {
> -		.hw_id = VCS2_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 2,
>  		.mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS3] = {
> -		.hw_id = VCS3_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 3,
>  		.mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS1] = {
> -		.hw_id = VECS1_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 1,
>  		.mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
>  #define VCS0_HW		1
>  #define BCS0_HW		2
>  #define VECS0_HW	3
> -#define VCS1_HW		4
> -#define VCS2_HW		6
> -#define VCS3_HW		7
> -#define VECS1_HW	12
>  
>  /* Gen11+ HW Engine class + instance */
>  #define RENDER_CLASS		0
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
@ 2021-07-21 22:47     ` Matt Roper
  0 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:47 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
> The engine hw_id is only used by RING_FAULT_REG(), which is not used
> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
> consistent, but let's try to remove them and let them defined to 0 when
> not used.

s/when not used/for engines that only exist on gen8+ platforms/

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

For historical reference, we did use hw_id on gen8+ platforms too until
relatively recently --- it was used to set the engine's guc_id as well
up until:

        commit c784e5249e773689e38d2bc1749f08b986621a26
        Author: John Harrison <John.C.Harrison@Intel.com>
        Date:   Wed Oct 28 07:58:24 2020 -0700

            drm/i915/guc: Update to use firmware v49.0.1


Matt

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 4 ----
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ----
>  2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index d561573ed98c..a11f69f2e46e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -80,7 +80,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS1] = {
> -		.hw_id = VCS1_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 1,
>  		.mmio_bases = {
> @@ -89,7 +88,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS2] = {
> -		.hw_id = VCS2_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 2,
>  		.mmio_bases = {
> @@ -97,7 +95,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS3] = {
> -		.hw_id = VCS3_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 3,
>  		.mmio_bases = {
> @@ -114,7 +111,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS1] = {
> -		.hw_id = VECS1_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 1,
>  		.mmio_bases = {
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 1cb9c3b70b29..a107eb58ffa2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -34,10 +34,6 @@
>  #define VCS0_HW		1
>  #define BCS0_HW		2
>  #define VECS0_HW	3
> -#define VCS1_HW		4
> -#define VCS2_HW		6
> -#define VCS3_HW		7
> -#define VECS1_HW	12
>  
>  /* Gen11+ HW Engine class + instance */
>  #define RENDER_CLASS		0
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21 22:51     ` Matt Roper
  -1 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:51 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison

On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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

although if we apply patch #4 we could probably drop this intermediate
step.


Matt

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
>  drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h              |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>  
>  #define MAX_MMIO_BASES 3
>  struct engine_info {
> -	unsigned int hw_id;
> +	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>  
>  static const struct engine_info intel_engines[] = {
>  	[RCS0] = {
> -		.hw_id = RCS0_HW,
> +		.gen6_hw_id = RCS0_HW,
>  		.class = RENDER_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[BCS0] = {
> -		.hw_id = BCS0_HW,
> +		.gen6_hw_id = BCS0_HW,
>  		.class = COPY_ENGINE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS0] = {
> -		.hw_id = VCS0_HW,
> +		.gen6_hw_id = VCS0_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS0] = {
> -		.hw_id = VECS0_HW,
> +		.gen6_hw_id = VECS0_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>  	engine->i915 = i915;
>  	engine->gt = gt;
>  	engine->uncore = gt->uncore;
> -	engine->hw_id = info->hw_id;
> +	engine->gen6_hw_id = info->gen6_hw_id;
>  	guc_class = engine_class_to_guc_class(info->class);
>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
>  	enum intel_engine_id id;
>  	enum intel_engine_id legacy_idx;
>  
> -	unsigned int hw_id;
>  	unsigned int guc_id;
>  
>  	intel_engine_mask_t mask;
>  
> +	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
@ 2021-07-21 22:51     ` Matt Roper
  0 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 22:51 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> We kept adding new engines and for that increasing hw_id unnecessarily:
> it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> try to pack it in the structs to give a hint this field is actually not
> used in recent platforms.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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

although if we apply patch #4 we could probably drop this intermediate
step.


Matt

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
>  drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h              |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index a11f69f2e46e..508221de411c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,7 @@
>  
>  #define MAX_MMIO_BASES 3
>  struct engine_info {
> -	unsigned int hw_id;
> +	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +54,7 @@ struct engine_info {
>  
>  static const struct engine_info intel_engines[] = {
>  	[RCS0] = {
> -		.hw_id = RCS0_HW,
> +		.gen6_hw_id = RCS0_HW,
>  		.class = RENDER_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[BCS0] = {
> -		.hw_id = BCS0_HW,
> +		.gen6_hw_id = BCS0_HW,
>  		.class = COPY_ENGINE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS0] = {
> -		.hw_id = VCS0_HW,
> +		.gen6_hw_id = VCS0_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS0] = {
> -		.hw_id = VECS0_HW,
> +		.gen6_hw_id = VECS0_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>  	engine->i915 = i915;
>  	engine->gt = gt;
>  	engine->uncore = gt->uncore;
> -	engine->hw_id = info->hw_id;
> +	engine->gen6_hw_id = info->gen6_hw_id;
>  	guc_class = engine_class_to_guc_class(info->class);
>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index a107eb58ffa2..266422d8d1b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -264,11 +264,11 @@ struct intel_engine_cs {
>  	enum intel_engine_id id;
>  	enum intel_engine_id legacy_idx;
>  
> -	unsigned int hw_id;
>  	unsigned int guc_id;
>  
>  	intel_engine_mask_t mask;
>  
> +	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 943fe485c662..8750ffce9d61 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->hw_id)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
  2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
@ 2021-07-21 23:02     ` Matt Roper
  -1 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 23:02 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison

On Tue, Jul 20, 2021 at 04:20:14PM -0700, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>  3 files changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>  
>  #define MAX_MMIO_BASES 3
>  struct engine_info {
> -	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>  
>  static const struct engine_info intel_engines[] = {
>  	[RCS0] = {
> -		.gen6_hw_id = RCS0_HW,
>  		.class = RENDER_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[BCS0] = {
> -		.gen6_hw_id = BCS0_HW,
>  		.class = COPY_ENGINE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS0] = {
> -		.gen6_hw_id = VCS0_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS0] = {
> -		.gen6_hw_id = VECS0_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>  	engine->i915 = i915;
>  	engine->gt = gt;
>  	engine->uncore = gt->uncore;
> -	engine->gen6_hw_id = info->gen6_hw_id;
>  	guc_class = engine_class_to_guc_class(info->class);
>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
>  #include "intel_wakeref.h"
>  #include "intel_workarounds_types.h"
>  
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW		0
> -#define VCS0_HW		1
> -#define BCS0_HW		2
> -#define VECS0_HW	3
> -
>  /* Gen11+ HW Engine class + instance */
>  #define RENDER_CLASS		0
>  #define VIDEO_DECODE_CLASS	1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>  
>  	intel_engine_mask_t mask;
>  
> -	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))

If you want to make this more clear to someone reading it down the road,
you could always do something explicit like:

  #define _RING_FAULT_REG_RCS        0x4094
  #define _RING_FAULT_REG_VCS        0x4194
  #define _RING_FAULT_REG_BCS        0x4294
  #define _RING_FAULT_REG_VECS       0x4394
  #define RING_FAULT_REG(engine)     _MMIO(_PICK((engine)->class, \
                                                 _RING_FAULT_REG_RCS, \
                                                 _RING_FAULT_REG_VCS, \
                                                 _RING_FAULT_REG_VECS, \
                                                 _RING_FAULT_REG_BCS))

But in general,

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


>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
@ 2021-07-21 23:02     ` Matt Roper
  0 siblings, 0 replies; 37+ messages in thread
From: Matt Roper @ 2021-07-21 23:02 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Tue, Jul 20, 2021 at 04:20:14PM -0700, Lucas De Marchi wrote:
> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
> recent platforms do not depend on this field, so it doesn't make much
> sense to keep it generic like that. Instead, just do a mapping from
> engine class to HW ID in the single place that is needed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>  3 files changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 508221de411c..0a04e8d90e9e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -42,7 +42,6 @@
>  
>  #define MAX_MMIO_BASES 3
>  struct engine_info {
> -	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  	/* mmio bases table *must* be sorted in reverse graphics_ver order */
> @@ -54,7 +53,6 @@ struct engine_info {
>  
>  static const struct engine_info intel_engines[] = {
>  	[RCS0] = {
> -		.gen6_hw_id = RCS0_HW,
>  		.class = RENDER_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[BCS0] = {
> -		.gen6_hw_id = BCS0_HW,
>  		.class = COPY_ENGINE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VCS0] = {
> -		.gen6_hw_id = VCS0_HW,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>  		},
>  	},
>  	[VECS0] = {
> -		.gen6_hw_id = VECS0_HW,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 0,
>  		.mmio_bases = {
> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
>  	engine->i915 = i915;
>  	engine->gt = gt;
>  	engine->uncore = gt->uncore;
> -	engine->gen6_hw_id = info->gen6_hw_id;
>  	guc_class = engine_class_to_guc_class(info->class);
>  	engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>  	engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 266422d8d1b1..64330bfb7641 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -28,13 +28,6 @@
>  #include "intel_wakeref.h"
>  #include "intel_workarounds_types.h"
>  
> -/* Legacy HW Engine ID */
> -
> -#define RCS0_HW		0
> -#define VCS0_HW		1
> -#define BCS0_HW		2
> -#define VECS0_HW	3
> -
>  /* Gen11+ HW Engine class + instance */
>  #define RENDER_CLASS		0
>  #define VIDEO_DECODE_CLASS	1
> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>  
>  	intel_engine_mask_t mask;
>  
> -	u8 gen6_hw_id;
>  	u8 class;
>  	u8 instance;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8750ffce9d61..d91386f4828e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2572,7 +2572,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>  #define   ARB_MODE_SWIZZLE_BDW	(1 << 1)
>  #define RENDER_HWS_PGA_GEN7	_MMIO(0x04080)
> -#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> +
> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
> +#define RING_FAULT_REG(engine)	_MMIO(0x4094 + 0x100 * _GEN6_ENGINE_CLASS_TO_ID((engine)->class))

If you want to make this more clear to someone reading it down the road,
you could always do something explicit like:

  #define _RING_FAULT_REG_RCS        0x4094
  #define _RING_FAULT_REG_VCS        0x4194
  #define _RING_FAULT_REG_BCS        0x4294
  #define _RING_FAULT_REG_VECS       0x4394
  #define RING_FAULT_REG(engine)     _MMIO(_PICK((engine)->class, \
                                                 _RING_FAULT_REG_RCS, \
                                                 _RING_FAULT_REG_VCS, \
                                                 _RING_FAULT_REG_VECS, \
                                                 _RING_FAULT_REG_BCS))

But in general,

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


>  #define GEN8_RING_FAULT_REG	_MMIO(0x4094)
>  #define GEN12_RING_FAULT_REG	_MMIO(0xcec4)
>  #define   GEN8_RING_FAULT_ENGINE_ID(x)	(((x) >> 12) & 0x7)
> -- 
> 2.31.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
  2021-07-21 22:47     ` [Intel-gfx] " Matt Roper
@ 2021-07-21 23:17       ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-21 23:17 UTC (permalink / raw)
  To: Matt Roper
  Cc: intel-gfx, Tvrtko Ursulin, Tomas Winkler, dri-devel, John Harrison

On Wed, Jul 21, 2021 at 03:47:22PM -0700, Matt Roper wrote:
>On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
>> The engine hw_id is only used by RING_FAULT_REG(), which is not used
>> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
>> consistent, but let's try to remove them and let them defined to 0 when
>> not used.
>
>s/when not used/for engines that only exist on gen8+ platforms/
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>For historical reference, we did use hw_id on gen8+ platforms too until
>relatively recently --- it was used to set the engine's guc_id as well
>up until:
>
>        commit c784e5249e773689e38d2bc1749f08b986621a26
>        Author: John Harrison <John.C.Harrison@Intel.com>
>        Date:   Wed Oct 28 07:58:24 2020 -0700
>
>            drm/i915/guc: Update to use firmware v49.0.1

thanks for digging this, I will add that to the commit message as well.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id
@ 2021-07-21 23:17       ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-21 23:17 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx, Tomas Winkler, dri-devel

On Wed, Jul 21, 2021 at 03:47:22PM -0700, Matt Roper wrote:
>On Tue, Jul 20, 2021 at 04:20:12PM -0700, Lucas De Marchi wrote:
>> The engine hw_id is only used by RING_FAULT_REG(), which is not used
>> since GRAPHICS_VER == 8. We tend to keep adding new defines just to be
>> consistent, but let's try to remove them and let them defined to 0 when
>> not used.
>
>s/when not used/for engines that only exist on gen8+ platforms/
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>For historical reference, we did use hw_id on gen8+ platforms too until
>relatively recently --- it was used to set the engine's guc_id as well
>up until:
>
>        commit c784e5249e773689e38d2bc1749f08b986621a26
>        Author: John Harrison <John.C.Harrison@Intel.com>
>        Date:   Wed Oct 28 07:58:24 2020 -0700
>
>            drm/i915/guc: Update to use firmware v49.0.1

thanks for digging this, I will add that to the commit message as well.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
  2021-07-21 22:51     ` [Intel-gfx] " Matt Roper
@ 2021-07-22  5:11       ` Lucas De Marchi
  -1 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-22  5:11 UTC (permalink / raw)
  To: Matt Roper; +Cc: Intel Graphics, Lucas De Marchi, Tomas Winkler, DRI

On Wed, Jul 21, 2021 at 3:51 PM Matt Roper <matthew.d.roper@intel.com> wrote:
>
> On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> > We kept adding new engines and for that increasing hw_id unnecessarily:
> > it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> > try to pack it in the structs to give a hint this field is actually not
> > used in recent platforms.
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
> although if we apply patch #4 we could probably drop this intermediate

I was not so confident people would agree with that patch. Adding the macros to
the header as suggested helps it being more palatable though.

thanks
Lucas De Marchi

> step.
>
>
> Matt
>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
> >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
> >  drivers/gpu/drm/i915/i915_reg.h              |  2 +-
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index a11f69f2e46e..508221de411c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -42,7 +42,7 @@
> >
> >  #define MAX_MMIO_BASES 3
> >  struct engine_info {
> > -     unsigned int hw_id;
> > +     u8 gen6_hw_id;
> >       u8 class;
> >       u8 instance;
> >       /* mmio bases table *must* be sorted in reverse graphics_ver order */
> > @@ -54,7 +54,7 @@ struct engine_info {
> >
> >  static const struct engine_info intel_engines[] = {
> >       [RCS0] = {
> > -             .hw_id = RCS0_HW,
> > +             .gen6_hw_id = RCS0_HW,
> >               .class = RENDER_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [BCS0] = {
> > -             .hw_id = BCS0_HW,
> > +             .gen6_hw_id = BCS0_HW,
> >               .class = COPY_ENGINE_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [VCS0] = {
> > -             .hw_id = VCS0_HW,
> > +             .gen6_hw_id = VCS0_HW,
> >               .class = VIDEO_DECODE_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [VECS0] = {
> > -             .hw_id = VECS0_HW,
> > +             .gen6_hw_id = VECS0_HW,
> >               .class = VIDEO_ENHANCEMENT_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> >       engine->i915 = i915;
> >       engine->gt = gt;
> >       engine->uncore = gt->uncore;
> > -     engine->hw_id = info->hw_id;
> > +     engine->gen6_hw_id = info->gen6_hw_id;
> >       guc_class = engine_class_to_guc_class(info->class);
> >       engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> >       engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index a107eb58ffa2..266422d8d1b1 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -264,11 +264,11 @@ struct intel_engine_cs {
> >       enum intel_engine_id id;
> >       enum intel_engine_id legacy_idx;
> >
> > -     unsigned int hw_id;
> >       unsigned int guc_id;
> >
> >       intel_engine_mask_t mask;
> >
> > +     u8 gen6_hw_id;
> >       u8 class;
> >       u8 instance;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 943fe485c662..8750ffce9d61 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> >  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
> >  #define   ARB_MODE_SWIZZLE_BDW       (1 << 1)
> >  #define RENDER_HWS_PGA_GEN7  _MMIO(0x04080)
> > -#define RING_FAULT_REG(engine)       _MMIO(0x4094 + 0x100 * (engine)->hw_id)
> > +#define RING_FAULT_REG(engine)       _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> >  #define GEN8_RING_FAULT_REG  _MMIO(0x4094)
> >  #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> >  #define   GEN8_RING_FAULT_ENGINE_ID(x)       (((x) >> 12) & 0x7)
> > --
> > 2.31.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id
@ 2021-07-22  5:11       ` Lucas De Marchi
  0 siblings, 0 replies; 37+ messages in thread
From: Lucas De Marchi @ 2021-07-22  5:11 UTC (permalink / raw)
  To: Matt Roper; +Cc: Intel Graphics, Lucas De Marchi, Tomas Winkler, DRI

On Wed, Jul 21, 2021 at 3:51 PM Matt Roper <matthew.d.roper@intel.com> wrote:
>
> On Tue, Jul 20, 2021 at 04:20:13PM -0700, Lucas De Marchi wrote:
> > We kept adding new engines and for that increasing hw_id unnecessarily:
> > it's not used since GRAPHICS_VER == 8. Prepend "gen6" to the field and
> > try to pack it in the structs to give a hint this field is actually not
> > used in recent platforms.
> >
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
> although if we apply patch #4 we could probably drop this intermediate

I was not so confident people would agree with that patch. Adding the macros to
the header as suggested helps it being more palatable though.

thanks
Lucas De Marchi

> step.
>
>
> Matt
>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 12 ++++++------
> >  drivers/gpu/drm/i915/gt/intel_engine_types.h |  2 +-
> >  drivers/gpu/drm/i915/i915_reg.h              |  2 +-
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index a11f69f2e46e..508221de411c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -42,7 +42,7 @@
> >
> >  #define MAX_MMIO_BASES 3
> >  struct engine_info {
> > -     unsigned int hw_id;
> > +     u8 gen6_hw_id;
> >       u8 class;
> >       u8 instance;
> >       /* mmio bases table *must* be sorted in reverse graphics_ver order */
> > @@ -54,7 +54,7 @@ struct engine_info {
> >
> >  static const struct engine_info intel_engines[] = {
> >       [RCS0] = {
> > -             .hw_id = RCS0_HW,
> > +             .gen6_hw_id = RCS0_HW,
> >               .class = RENDER_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -62,7 +62,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [BCS0] = {
> > -             .hw_id = BCS0_HW,
> > +             .gen6_hw_id = BCS0_HW,
> >               .class = COPY_ENGINE_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -70,7 +70,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [VCS0] = {
> > -             .hw_id = VCS0_HW,
> > +             .gen6_hw_id = VCS0_HW,
> >               .class = VIDEO_DECODE_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -102,7 +102,7 @@ static const struct engine_info intel_engines[] = {
> >               },
> >       },
> >       [VECS0] = {
> > -             .hw_id = VECS0_HW,
> > +             .gen6_hw_id = VECS0_HW,
> >               .class = VIDEO_ENHANCEMENT_CLASS,
> >               .instance = 0,
> >               .mmio_bases = {
> > @@ -290,7 +290,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> >       engine->i915 = i915;
> >       engine->gt = gt;
> >       engine->uncore = gt->uncore;
> > -     engine->hw_id = info->hw_id;
> > +     engine->gen6_hw_id = info->gen6_hw_id;
> >       guc_class = engine_class_to_guc_class(info->class);
> >       engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
> >       engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index a107eb58ffa2..266422d8d1b1 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -264,11 +264,11 @@ struct intel_engine_cs {
> >       enum intel_engine_id id;
> >       enum intel_engine_id legacy_idx;
> >
> > -     unsigned int hw_id;
> >       unsigned int guc_id;
> >
> >       intel_engine_mask_t mask;
> >
> > +     u8 gen6_hw_id;
> >       u8 class;
> >       u8 instance;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 943fe485c662..8750ffce9d61 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2572,7 +2572,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> >  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
> >  #define   ARB_MODE_SWIZZLE_BDW       (1 << 1)
> >  #define RENDER_HWS_PGA_GEN7  _MMIO(0x04080)
> > -#define RING_FAULT_REG(engine)       _MMIO(0x4094 + 0x100 * (engine)->hw_id)
> > +#define RING_FAULT_REG(engine)       _MMIO(0x4094 + 0x100 * (engine)->gen6_hw_id)
> >  #define GEN8_RING_FAULT_REG  _MMIO(0x4094)
> >  #define GEN12_RING_FAULT_REG _MMIO(0xcec4)
> >  #define   GEN8_RING_FAULT_ENGINE_ID(x)       (((x) >> 12) & 0x7)
> > --
> > 2.31.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
  2021-07-21 18:44       ` Lucas De Marchi
@ 2021-07-22  8:27         ` Tvrtko Ursulin
  -1 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-22  8:27 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: intel-gfx, Daniele Ceraolo Spurio, Tomas Winkler, dri-devel


On 21/07/2021 19:44, Lucas De Marchi wrote:
> On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>>
>> On 21/07/2021 00:20, Lucas De Marchi wrote:
>>> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>> recent platforms do not depend on this field, so it doesn't make much
>>> sense to keep it generic like that. Instead, just do a mapping from
>>> engine class to HW ID in the single place that is needed.
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>>>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>>>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>>>  3 files changed, 3 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
>>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 508221de411c..0a04e8d90e9e 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -42,7 +42,6 @@
>>>  #define MAX_MMIO_BASES 3
>>>  struct engine_info {
>>> -    u8 gen6_hw_id;
>>>      u8 class;
>>>      u8 instance;
>>>      /* mmio bases table *must* be sorted in reverse graphics_ver 
>>> order */
>>> @@ -54,7 +53,6 @@ struct engine_info {
>>>  static const struct engine_info intel_engines[] = {
>>>      [RCS0] = {
>>> -        .gen6_hw_id = RCS0_HW,
>>>          .class = RENDER_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [BCS0] = {
>>> -        .gen6_hw_id = BCS0_HW,
>>>          .class = COPY_ENGINE_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [VCS0] = {
>>> -        .gen6_hw_id = VCS0_HW,
>>>          .class = VIDEO_DECODE_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [VECS0] = {
>>> -        .gen6_hw_id = VECS0_HW,
>>>          .class = VIDEO_ENHANCEMENT_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt 
>>> *gt, enum intel_engine_id id)
>>>      engine->i915 = i915;
>>>      engine->gt = gt;
>>>      engine->uncore = gt->uncore;
>>> -    engine->gen6_hw_id = info->gen6_hw_id;
>>>      guc_class = engine_class_to_guc_class(info->class);
>>>      engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>>>      engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 266422d8d1b1..64330bfb7641 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -28,13 +28,6 @@
>>>  #include "intel_wakeref.h"
>>>  #include "intel_workarounds_types.h"
>>> -/* Legacy HW Engine ID */
>>> -
>>> -#define RCS0_HW        0
>>> -#define VCS0_HW        1
>>> -#define BCS0_HW        2
>>> -#define VECS0_HW    3
>>> -
>>>  /* Gen11+ HW Engine class + instance */
>>>  #define RENDER_CLASS        0
>>>  #define VIDEO_DECODE_CLASS    1
>>> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>>>      intel_engine_mask_t mask;
>>> -    u8 gen6_hw_id;
>>>      u8 class;
>>>      u8 instance;
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 8750ffce9d61..d91386f4828e 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -2572,7 +2572,9 @@ static inline bool 
>>> i915_mmio_reg_valid(i915_reg_t reg)
>>>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>>>  #define   ARB_MODE_SWIZZLE_BDW    (1 << 1)
>>>  #define RENDER_HWS_PGA_GEN7    _MMIO(0x04080)
>>> -#define RING_FAULT_REG(engine)    _MMIO(0x4094 + 0x100 * 
>>> (engine)->gen6_hw_id)
>>> +
>>> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>> +#define RING_FAULT_REG(engine)    _MMIO(0x4094 + 0x100 * 
>>> _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>>
>> Makes sense to me.
>>
>> Maybe HW_ID and HW_CLASS in the macro name? Not sure.
> 
> I can do that... I think I avoided it because it makes the macro
> very big. Anyway, this should be called in just one place, so it doesn't
> matter much... I can add it.
> 
>>
>> Only open I have is why the "Gen11+ HW Engine class + instance" 
>> comment and now we would tie that, allegedly Gen11 concept, with 
>> Gen6-7. Care to do some digging?
> 
> Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out 
> of i915_reg.h")
> that I reviewed :-o
> 
> Cc'ing  Daniele. I don't see "class" as a Gen11+ thing. Is it just that
> those numbers started to make sense for gen11?  Since
> 
> a) we are using the class even for GRAPHICS_VER < 11
> b) the legacy HW IDs shouldn't be used anywhere else anymore
> 
> 
> we could
> 
> 1) move the legacy defines back to i915_reg.h
> 2) use them in the macro above (IMO would slightly improve the
> readability of that _PICK() call)
> 3) Remove the "Gen11+" comment in the _CLASS macros to avoid
> misunderstandings
> 
> 
> Thoughts?

What Matt suggested sounds fine to me (using _PICK with 
_RING_FAULT_REG_RCS etc). It retires the concept of hw id from the code 
base completely and hopefully the need for it does not re-surface.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 37+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id
@ 2021-07-22  8:27         ` Tvrtko Ursulin
  0 siblings, 0 replies; 37+ messages in thread
From: Tvrtko Ursulin @ 2021-07-22  8:27 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Tomas Winkler, dri-devel


On 21/07/2021 19:44, Lucas De Marchi wrote:
> On Wed, Jul 21, 2021 at 10:25:59AM +0100, Tvrtko Ursulin wrote:
>>
>> On 21/07/2021 00:20, Lucas De Marchi wrote:
>>> This is only used by GRAPHICS_VER == 6 and GRAPHICS_VER == 7. All other
>>> recent platforms do not depend on this field, so it doesn't make much
>>> sense to keep it generic like that. Instead, just do a mapping from
>>> engine class to HW ID in the single place that is needed.
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 6 ------
>>>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 8 --------
>>>  drivers/gpu/drm/i915/i915_reg.h              | 4 +++-
>>>  3 files changed, 3 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
>>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 508221de411c..0a04e8d90e9e 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -42,7 +42,6 @@
>>>  #define MAX_MMIO_BASES 3
>>>  struct engine_info {
>>> -    u8 gen6_hw_id;
>>>      u8 class;
>>>      u8 instance;
>>>      /* mmio bases table *must* be sorted in reverse graphics_ver 
>>> order */
>>> @@ -54,7 +53,6 @@ struct engine_info {
>>>  static const struct engine_info intel_engines[] = {
>>>      [RCS0] = {
>>> -        .gen6_hw_id = RCS0_HW,
>>>          .class = RENDER_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -62,7 +60,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [BCS0] = {
>>> -        .gen6_hw_id = BCS0_HW,
>>>          .class = COPY_ENGINE_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -70,7 +67,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [VCS0] = {
>>> -        .gen6_hw_id = VCS0_HW,
>>>          .class = VIDEO_DECODE_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -102,7 +98,6 @@ static const struct engine_info intel_engines[] = {
>>>          },
>>>      },
>>>      [VECS0] = {
>>> -        .gen6_hw_id = VECS0_HW,
>>>          .class = VIDEO_ENHANCEMENT_CLASS,
>>>          .instance = 0,
>>>          .mmio_bases = {
>>> @@ -290,7 +285,6 @@ static int intel_engine_setup(struct intel_gt 
>>> *gt, enum intel_engine_id id)
>>>      engine->i915 = i915;
>>>      engine->gt = gt;
>>>      engine->uncore = gt->uncore;
>>> -    engine->gen6_hw_id = info->gen6_hw_id;
>>>      guc_class = engine_class_to_guc_class(info->class);
>>>      engine->guc_id = MAKE_GUC_ID(guc_class, info->instance);
>>>      engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
>>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 266422d8d1b1..64330bfb7641 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -28,13 +28,6 @@
>>>  #include "intel_wakeref.h"
>>>  #include "intel_workarounds_types.h"
>>> -/* Legacy HW Engine ID */
>>> -
>>> -#define RCS0_HW        0
>>> -#define VCS0_HW        1
>>> -#define BCS0_HW        2
>>> -#define VECS0_HW    3
>>> -
>>>  /* Gen11+ HW Engine class + instance */
>>>  #define RENDER_CLASS        0
>>>  #define VIDEO_DECODE_CLASS    1
>>> @@ -268,7 +261,6 @@ struct intel_engine_cs {
>>>      intel_engine_mask_t mask;
>>> -    u8 gen6_hw_id;
>>>      u8 class;
>>>      u8 instance;
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 8750ffce9d61..d91386f4828e 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -2572,7 +2572,9 @@ static inline bool 
>>> i915_mmio_reg_valid(i915_reg_t reg)
>>>  #define   ARB_MODE_BWGTLB_DISABLE (1 << 9)
>>>  #define   ARB_MODE_SWIZZLE_BDW    (1 << 1)
>>>  #define RENDER_HWS_PGA_GEN7    _MMIO(0x04080)
>>> -#define RING_FAULT_REG(engine)    _MMIO(0x4094 + 0x100 * 
>>> (engine)->gen6_hw_id)
>>> +
>>> +#define _GEN6_ENGINE_CLASS_TO_ID(class) _PICK((class), 0, 1, 3, 2)
>>> +#define RING_FAULT_REG(engine)    _MMIO(0x4094 + 0x100 * 
>>> _GEN6_ENGINE_CLASS_TO_ID((engine)->class))
>>
>> Makes sense to me.
>>
>> Maybe HW_ID and HW_CLASS in the macro name? Not sure.
> 
> I can do that... I think I avoided it because it makes the macro
> very big. Anyway, this should be called in just one place, so it doesn't
> matter much... I can add it.
> 
>>
>> Only open I have is why the "Gen11+ HW Engine class + instance" 
>> comment and now we would tie that, allegedly Gen11 concept, with 
>> Gen6-7. Care to do some digging?
> 
> Not sure. This comes from 3d7b3039741d ("drm/i915: Move engine IDs out 
> of i915_reg.h")
> that I reviewed :-o
> 
> Cc'ing  Daniele. I don't see "class" as a Gen11+ thing. Is it just that
> those numbers started to make sense for gen11?  Since
> 
> a) we are using the class even for GRAPHICS_VER < 11
> b) the legacy HW IDs shouldn't be used anywhere else anymore
> 
> 
> we could
> 
> 1) move the legacy defines back to i915_reg.h
> 2) use them in the macro above (IMO would slightly improve the
> readability of that _PICK() call)
> 3) Remove the "Gen11+" comment in the _CLASS macros to avoid
> misunderstandings
> 
> 
> Thoughts?

What Matt suggested sounds fine to me (using _PICK with 
_RING_FAULT_REG_RCS etc). It retires the concept of hw id from the code 
base completely and hopefully the need for it does not re-surface.

Regards,

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

^ permalink raw reply	[flat|nested] 37+ messages in thread
end of thread, other threads:[~2021-07-22  8:28 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 23:20 [PATCH 0/4] Nuke legacy hw_id Lucas De Marchi
2021-07-20 23:20 ` [Intel-gfx] " Lucas De Marchi
2021-07-20 23:20 ` [PATCH 1/4] drm/i915/gt: fix platform prefix Lucas De Marchi
2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
2021-07-21  9:11   ` Tvrtko Ursulin
2021-07-21  9:11     ` Tvrtko Ursulin
2021-07-21 22:34   ` Matt Roper
2021-07-21 22:34     ` [Intel-gfx] " Matt Roper
2021-07-20 23:20 ` [PATCH 2/4] drm/i915/gt: nuke unused legacy engine hw_id Lucas De Marchi
2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
2021-07-21  9:18   ` Tvrtko Ursulin
2021-07-21  9:18     ` Tvrtko Ursulin
2021-07-21 22:47   ` Matt Roper
2021-07-21 22:47     ` [Intel-gfx] " Matt Roper
2021-07-21 23:17     ` Lucas De Marchi
2021-07-21 23:17       ` [Intel-gfx] " Lucas De Marchi
2021-07-20 23:20 ` [PATCH 3/4] drm/i915/gt: rename legacy engine->hw_id to engine->gen6_hw_id Lucas De Marchi
2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
2021-07-21  9:20   ` Tvrtko Ursulin
2021-07-21  9:20     ` Tvrtko Ursulin
2021-07-21 22:51   ` Matt Roper
2021-07-21 22:51     ` [Intel-gfx] " Matt Roper
2021-07-22  5:11     ` Lucas De Marchi
2021-07-22  5:11       ` Lucas De Marchi
2021-07-20 23:20 ` [PATCH 4/4] drm/i915/gt: nuke gen6_hw_id Lucas De Marchi
2021-07-20 23:20   ` [Intel-gfx] " Lucas De Marchi
2021-07-21  9:25   ` Tvrtko Ursulin
2021-07-21  9:25     ` Tvrtko Ursulin
2021-07-21 18:44     ` Lucas De Marchi
2021-07-21 18:44       ` Lucas De Marchi
2021-07-22  8:27       ` Tvrtko Ursulin
2021-07-22  8:27         ` Tvrtko Ursulin
2021-07-21 23:02   ` Matt Roper
2021-07-21 23:02     ` [Intel-gfx] " Matt Roper
2021-07-21  0:35 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for Nuke legacy hw_id Patchwork
2021-07-21  0:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-07-21  3:46 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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