[PATCH v2 1/3] drm/i915: store all mmio bases in intel_engines

[PATCH v2 1/3] drm/i915: store all mmio bases in intel_engines

[Daniele Ceraolo Spurio]

The mmio bases we're currently storing in the intel_engines array are
only valid for a subset of gens, so we need to ignore them and use
different values in some cases. Instead of doing that, we can have a
table of [starting gen, mmio base] pairs for each engine in
intel_engines and select the correct one based on the gen we're running
on in a consistent way.

v2: document that the list goes in reverse order, update starting gen
    for render (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 78 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_ringbuffer.c |  1 -
 2 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 4ba139c27fba..08711665061c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -81,12 +81,17 @@ static const struct engine_class_info intel_engine_classes[] = {
 	},
 };
 
+#define MAX_MMIO_BASES 3
 struct engine_info {
 	unsigned int hw_id;
 	unsigned int uabi_id;
 	u8 class;
 	u8 instance;
-	u32 mmio_base;
+	/* mmio bases table *must* be sorted in reverse gen order */
+	struct engine_mmio_base {
+		u32 gen : 8;
+		u32 base : 24;
+	} mmio_bases[MAX_MMIO_BASES];
 	unsigned irq_shift;
 };
 
@@ -96,7 +101,9 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_RENDER,
 		.class = RENDER_CLASS,
 		.instance = 0,
-		.mmio_base = RENDER_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 0, .base = RENDER_RING_BASE }
+		},
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 	},
 	[BCS] = {
@@ -104,7 +111,9 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_BLT,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
-		.mmio_base = BLT_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 6, .base = BLT_RING_BASE }
+		},
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 	},
 	[VCS] = {
@@ -112,7 +121,11 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
-		.mmio_base = GEN6_BSD_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_BSD_RING_BASE },
+			{ .gen = 6, .base = GEN6_BSD_RING_BASE },
+			{ .gen = 4, .base = BSD_RING_BASE }
+		},
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 	},
 	[VCS2] = {
@@ -120,7 +133,10 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 1,
-		.mmio_base = GEN8_BSD2_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_BSD2_RING_BASE },
+			{ .gen = 8, .base = GEN8_BSD2_RING_BASE }
+		},
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 	},
 	[VCS3] = {
@@ -128,7 +144,9 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 2,
-		.mmio_base = GEN11_BSD3_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_BSD3_RING_BASE }
+		},
 		.irq_shift = 0, /* not used */
 	},
 	[VCS4] = {
@@ -136,7 +154,9 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 3,
-		.mmio_base = GEN11_BSD4_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_BSD4_RING_BASE }
+		},
 		.irq_shift = 0, /* not used */
 	},
 	[VECS] = {
@@ -144,7 +164,10 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_VEBOX,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
-		.mmio_base = VEBOX_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_VEBOX_RING_BASE },
+			{ .gen = 7, .base = VEBOX_RING_BASE }
+		},
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 	},
 	[VECS2] = {
@@ -152,7 +175,9 @@ static const struct engine_info intel_engines[] = {
 		.uabi_id = I915_EXEC_VEBOX,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 1,
-		.mmio_base = GEN11_VEBOX2_RING_BASE,
+		.mmio_bases = {
+			{ .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
+		},
 		.irq_shift = 0, /* not used */
 	},
 };
@@ -223,6 +248,21 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
 	}
 }
 
+static u32 __engine_mmio_base(struct drm_i915_private *i915,
+			      const struct engine_mmio_base* bases)
+{
+	int i;
+
+	for (i = 0; i < MAX_MMIO_BASES; i++)
+		if (INTEL_GEN(i915) >= bases[i].gen)
+			break;
+
+	GEM_BUG_ON(i == MAX_MMIO_BASES);
+	GEM_BUG_ON(!bases[i].base);
+
+	return bases[i].base;
+}
+
 static int
 intel_engine_setup(struct drm_i915_private *dev_priv,
 		   enum intel_engine_id id)
@@ -257,25 +297,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 			 class_info->name, info->instance) >=
 		sizeof(engine->name));
 	engine->hw_id = engine->guc_id = info->hw_id;
-	if (INTEL_GEN(dev_priv) >= 11) {
-		switch (engine->id) {
-		case VCS:
-			engine->mmio_base = GEN11_BSD_RING_BASE;
-			break;
-		case VCS2:
-			engine->mmio_base = GEN11_BSD2_RING_BASE;
-			break;
-		case VECS:
-			engine->mmio_base = GEN11_VEBOX_RING_BASE;
-			break;
-		default:
-			/* take the original value for all other engines  */
-			engine->mmio_base = info->mmio_base;
-			break;
-		}
-	} else {
-		engine->mmio_base = info->mmio_base;
-	}
+	engine->mmio_base = __engine_mmio_base(dev_priv, info->mmio_bases);
 	engine->irq_shift = info->irq_shift;
 	engine->class = info->class;
 	engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d599524a759..2e4408477ab5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2079,7 +2079,6 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 		engine->emit_flush = gen6_bsd_ring_flush;
 		engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
 	} else {
-		engine->mmio_base = BSD_RING_BASE;
 		engine->emit_flush = bsd_ring_flush;
 		if (IS_GEN5(dev_priv))
 			engine->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
-- 
2.16.2

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

[PATCH v2 2/3] drm/i915: add a selftest for the mmio_bases table

[Daniele Ceraolo Spurio]

Check that the entries are in reverse gen order and that the first entry
and all the following entries with gen > 0 have an mmio base set.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c             |  1 +
 .../gpu/drm/i915/selftests/i915_mock_selftests.h   |  1 +
 drivers/gpu/drm/i915/selftests/intel_engine_cs.c   | 48 ++++++++++++++++++++++
 3 files changed, 50 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/selftests/intel_engine_cs.c

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 08711665061c..a33171d82aee 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -2131,4 +2131,5 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_engine.c"
+#include "selftests/intel_engine_cs.c"
 #endif
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index 9a48aa441743..2842f93ca29e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -23,3 +23,4 @@ selftest(vma, i915_vma_mock_selftests)
 selftest(evict, i915_gem_evict_mock_selftests)
 selftest(gtt, i915_gem_gtt_mock_selftests)
 selftest(hugepages, i915_gem_huge_page_mock_selftests)
+selftest(engine, intel_engine_cs_mock_selftests)
diff --git a/drivers/gpu/drm/i915/selftests/intel_engine_cs.c b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
new file mode 100644
index 000000000000..8ef453905520
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
@@ -0,0 +1,48 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include "../i915_selftest.h"
+
+static int intel_mmio_bases_check(void)
+{
+	const struct engine_info *info;
+	int i, j;
+	u32 gen;
+	s32 prev;
+
+	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
+		info = &intel_engines[i];
+
+		for (prev = -1, j = MAX_MMIO_BASES -1; j >= 0; j--) {
+			gen = info->mmio_bases[j].gen;
+
+			if (prev >= (s32)gen) {
+				pr_err("%s: engine[%d]: mmio base for gen %x "
+					"is before the one for gen %x\n",
+				       __func__, i, gen, prev);
+				return -EINVAL;
+			}
+
+			if ((j == 0 || gen > 0) && !info->mmio_bases[j].base) {
+				pr_err("%s: engine[%d]: invalid mmio base (%x) "
+					"for gen %x at entry %u\n",
+				       __func__, i, info->mmio_bases[j].base, gen, j);
+				return -EINVAL;
+			}
+
+			/* we can have multiple empty entries in a row */
+			if (gen > 0)
+				prev = gen;
+		}
+	}
+
+	return 0;
+}
+
+int intel_engine_cs_mock_selftests(void)
+{
+	return intel_mmio_bases_check();
+}
-- 
2.16.2

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

[PATCH v2 3/3] drm/i915: move gen8 irq shifts to intel_lrc.c

[Daniele Ceraolo Spurio]

The only usage outside the intel_lrc.c file is in the ringbuffer
init, but the irq mask calculated there is then overwritten for
all engines that have a non-zero shift, so we can drop it.

This change is not aimed at code saving but at removing from
intel_engines information that does not apply to all gens that have
the engine. When checking without the temporary WARN_ON, code size
is basically unchanged:

add/remove: 1/0 grow/shrink: 3/4 up/down: 70/-67 (3)
Function                                     old     new   delta
logical_ring_setup                           315     343     +28
irq_shifts                                     -      28     +28
intel_init_render_ring_buffer                258     268     +10
reset_common_ring                            704     708      +4
intel_engine_init_cmd_parser                1064    1058      -6
intel_engines_init_mmio                     1264    1256      -8
intel_ring_default_vfuncs                    584     563     -21
intel_engines                                224     192     -32
Total: Before=1479719, After=1479722, chg +0.00%

Suggested-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 10 ----------
 drivers/gpu/drm/i915/intel_lrc.c        | 22 +++++++++++++++++++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a33171d82aee..dbfeff6c46a8 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -92,7 +92,6 @@ struct engine_info {
 		u32 gen : 8;
 		u32 base : 24;
 	} mmio_bases[MAX_MMIO_BASES];
-	unsigned irq_shift;
 };
 
 static const struct engine_info intel_engines[] = {
@@ -104,7 +103,6 @@ static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 0, .base = RENDER_RING_BASE }
 		},
-		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 	},
 	[BCS] = {
 		.hw_id = BCS_HW,
@@ -114,7 +112,6 @@ static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 6, .base = BLT_RING_BASE }
 		},
-		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 	},
 	[VCS] = {
 		.hw_id = VCS_HW,
@@ -126,7 +123,6 @@ static const struct engine_info intel_engines[] = {
 			{ .gen = 6, .base = GEN6_BSD_RING_BASE },
 			{ .gen = 4, .base = BSD_RING_BASE }
 		},
-		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 	},
 	[VCS2] = {
 		.hw_id = VCS2_HW,
@@ -137,7 +133,6 @@ static const struct engine_info intel_engines[] = {
 			{ .gen = 11, .base = GEN11_BSD2_RING_BASE },
 			{ .gen = 8, .base = GEN8_BSD2_RING_BASE }
 		},
-		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 	},
 	[VCS3] = {
 		.hw_id = VCS3_HW,
@@ -147,7 +142,6 @@ static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_BSD3_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 	[VCS4] = {
 		.hw_id = VCS4_HW,
@@ -157,7 +151,6 @@ static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_BSD4_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 	[VECS] = {
 		.hw_id = VECS_HW,
@@ -168,7 +161,6 @@ static const struct engine_info intel_engines[] = {
 			{ .gen = 11, .base = GEN11_VEBOX_RING_BASE },
 			{ .gen = 7, .base = VEBOX_RING_BASE }
 		},
-		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 	},
 	[VECS2] = {
 		.hw_id = VECS2_HW,
@@ -178,7 +170,6 @@ static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 };
 
@@ -298,7 +289,6 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 		sizeof(engine->name));
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = __engine_mmio_base(dev_priv, info->mmio_bases);
-	engine->irq_shift = info->irq_shift;
 	engine->class = info->class;
 	engine->instance = info->instance;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 999d5f2539d4..1bf8e16d7fa3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1573,6 +1573,14 @@ static u8 gtiir[] = {
 	[VECS] = 3,
 };
 
+unsigned int irq_shifts[] = {
+	[RCS] = GEN8_RCS_IRQ_SHIFT,
+	[BCS] = GEN8_BCS_IRQ_SHIFT,
+	[VCS] = GEN8_VCS1_IRQ_SHIFT,
+	[VCS2] = GEN8_VCS2_IRQ_SHIFT,
+	[VECS] = GEN8_VECS_IRQ_SHIFT,
+};
+
 static void enable_execlists(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -1661,6 +1669,10 @@ static void reset_irq(struct intel_engine_cs *engine)
 	struct drm_i915_private *dev_priv = engine->i915;
 	int i;
 
+	/* TODO: correctly reset irqs for gen11 */
+	if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
+		return;
+
 	GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
 
 	/*
@@ -1672,11 +1684,11 @@ static void reset_irq(struct intel_engine_cs *engine)
 	 */
 	for (i = 0; i < 2; i++) {
 		I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
-			   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
+			   GT_CONTEXT_SWITCH_INTERRUPT << irq_shifts[engine->id]);
 		POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
 	}
 	GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
-		   (GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift));
+		   (GT_CONTEXT_SWITCH_INTERRUPT << irq_shifts[engine->id]));
 
 	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 }
@@ -2109,7 +2121,11 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 static inline void
 logical_ring_default_irqs(struct intel_engine_cs *engine)
 {
-	unsigned shift = engine->irq_shift;
+	unsigned shift = 0;
+
+	if (INTEL_GEN(engine->i915) < 11)
+		shift = irq_shifts[engine->id];
+
 	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << shift;
 	engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2e4408477ab5..3a7024fdad6f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1943,8 +1943,6 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
 				struct intel_engine_cs *engine)
 {
-	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << engine->irq_shift;
-
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->irq_enable = gen6_irq_enable;
 		engine->irq_disable = gen6_irq_disable;
@@ -2029,6 +2027,8 @@ int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 	if (HAS_L3_DPF(dev_priv))
 		engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
+	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
+
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
 		engine->emit_flush = gen7_render_ring_flush;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d8ddea0174ca..08265e312dc2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -330,7 +330,6 @@ struct intel_engine_cs {
 	u8 instance;
 	u32 context_size;
 	u32 mmio_base;
-	unsigned int irq_shift;
 
 	struct intel_ring *buffer;
 	struct intel_timeline *timeline;
-- 
2.16.2

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

✗ Fi.CI.SPARSE: warning for series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines
URL   : https://patchwork.freedesktop.org/series/39644/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: store all mmio bases in intel_engines
Okay!

Commit: drm/i915: add a selftest for the mmio_bases table
Okay!

Commit: drm/i915: move gen8 irq shifts to intel_lrc.c
+drivers/gpu/drm/i915/intel_lrc.c:1581:14: warning: symbol 'irq_shifts' was not declared. Should it be static?

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

✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines
URL   : https://patchwork.freedesktop.org/series/39644/
State : success

== Summary ==

Series 39644v1 series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines
https://patchwork.freedesktop.org/api/1.0/series/39644/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:421s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:422s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:499s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:492s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:481s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:466s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:408s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:417s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:291s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:514s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:394s
fi-ilk-650       total:288  pass:227  dwarn:0   dfail:0   fail:1   skip:60  time:402s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:454s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:421s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:467s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:507s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:582s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:433s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:524s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:534s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:482s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:421s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:524s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:391s
Blacklisted hosts:
fi-cnl-drrs      total:288  pass:257  dwarn:3   dfail:0   fail:0   skip:19  time:527s

469c28df8d66d3cc0a4a2e4e12433a5c92102022 drm-tip: 2018y-03m-08d-22h-40m-12s UTC integration manifest
b70c67929415 drm/i915: move gen8 irq shifts to intel_lrc.c
879b9440fc97 drm/i915: add a selftest for the mmio_bases table
ef4f215557ee drm/i915: store all mmio bases in intel_engines

== Logs ==

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

Re: [PATCH v2 3/3] drm/i915: move gen8 irq shifts to intel_lrc.c

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-08 23:46:29)
> The only usage outside the intel_lrc.c file is in the ringbuffer
> init, but the irq mask calculated there is then overwritten for
> all engines that have a non-zero shift, so we can drop it.
> 
> This change is not aimed at code saving but at removing from
> intel_engines information that does not apply to all gens that have
> the engine. When checking without the temporary WARN_ON, code size
> is basically unchanged:
> 
> add/remove: 1/0 grow/shrink: 3/4 up/down: 70/-67 (3)
> Function                                     old     new   delta
> logical_ring_setup                           315     343     +28
> irq_shifts                                     -      28     +28
> intel_init_render_ring_buffer                258     268     +10
> reset_common_ring                            704     708      +4
> intel_engine_init_cmd_parser                1064    1058      -6
> intel_engines_init_mmio                     1264    1256      -8
> intel_ring_default_vfuncs                    584     563     -21
> intel_engines                                224     192     -32
> Total: Before=1479719, After=1479722, chg +0.00%
> 
> Suggested-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

With the missing static const,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I had a look at restoring the use of engine->irq_shift in the irq
handlers, and while that does shrink the code abit, rearranging the code
brought even more savings. So I don't see a need to keep
engine->irq_shift around, and we can always bring it back but unlikely
if future gen do not need it.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/3] drm/i915: store all mmio bases in intel_engines

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-08 23:46:27)
> The mmio bases we're currently storing in the intel_engines array are
> only valid for a subset of gens, so we need to ignore them and use
> different values in some cases. Instead of doing that, we can have a
> table of [starting gen, mmio base] pairs for each engine in
> intel_engines and select the correct one based on the gen we're running
> on in a consistent way.
> 
> v2: document that the list goes in reverse order, update starting gen
>     for render (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> +static u32 __engine_mmio_base(struct drm_i915_private *i915,
> +                             const struct engine_mmio_base* bases)
> +{
> +       int i;
> +
> +       for (i = 0; i < MAX_MMIO_BASES; i++)
> +               if (INTEL_GEN(i915) >= bases[i].gen)
> +                       break;
> +
> +       GEM_BUG_ON(i == MAX_MMIO_BASES);
> +       GEM_BUG_ON(!bases[i].base);

Idly contemplating

do {
	if (INTEL_GEN(i915) >= bases->gen)
		return bases->base;

	bases++;
} while(1);

given the selftest for validating adding new gen.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/3] drm/i915: add a selftest for the mmio_bases table

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-08 23:46:28)
> Check that the entries are in reverse gen order and that the first entry
> and all the following entries with gen > 0 have an mmio base set.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c             |  1 +
>  .../gpu/drm/i915/selftests/i915_mock_selftests.h   |  1 +
>  drivers/gpu/drm/i915/selftests/intel_engine_cs.c   | 48 ++++++++++++++++++++++
>  3 files changed, 50 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/selftests/intel_engine_cs.c
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 08711665061c..a33171d82aee 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -2131,4 +2131,5 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
>  
>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>  #include "selftests/mock_engine.c"
> +#include "selftests/intel_engine_cs.c"
>  #endif
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 9a48aa441743..2842f93ca29e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -23,3 +23,4 @@ selftest(vma, i915_vma_mock_selftests)
>  selftest(evict, i915_gem_evict_mock_selftests)
>  selftest(gtt, i915_gem_gtt_mock_selftests)
>  selftest(hugepages, i915_gem_huge_page_mock_selftests)
> +selftest(engine, intel_engine_cs_mock_selftests)

Plonk this after uncore. It's a lowlevel sanity check that should come
before we start looking at features.

> diff --git a/drivers/gpu/drm/i915/selftests/intel_engine_cs.c b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
> new file mode 100644
> index 000000000000..8ef453905520
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
> @@ -0,0 +1,48 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright © 2018 Intel Corporation
> + */
> +
> +#include "../i915_selftest.h"
> +
> +static int intel_mmio_bases_check(void)
> +{
> +       const struct engine_info *info;
> +       int i, j;
> +       u32 gen;
> +       s32 prev;
> +
> +       for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
> +               info = &intel_engines[i];
> +
> +               for (prev = -1, j = MAX_MMIO_BASES -1; j >= 0; j--) {
> +                       gen = info->mmio_bases[j].gen;
> +
> +                       if (prev >= (s32)gen) {
> +                               pr_err("%s: engine[%d]: mmio base for gen %x "
> +                                       "is before the one for gen %x\n",
> +                                      __func__, i, gen, prev);
> +                               return -EINVAL;
> +                       }
> +
> +                       if ((j == 0 || gen > 0) && !info->mmio_bases[j].base) {

Ok, setting gen=0 upset us. Make that gen=1 in the previous patch.

Looping backwards here definitely seems to make it harder than it needs
to be. We only need to validate the array as seen by the algorithm so,

u8 prev = U8_MAX;
for (j = 0; j < MAX_MMIO_BASES; j++) {
	u8 gen = info->mmio_bases[j].gen;

	if (gen >= prev) {
		...
		return -EINVAL;

	}

	if (gen == 0)
		break;
	
	if (!info->mmio.bases[j].base) {
		...
		return -EINVAL;
	}

	prev = gen;
}
pr_info("%s: min gen supported for %s = %d\n", __func__, magic_engine_name(i), prev);

I'm not sure how we could automate that check (not without hardcoding
the same information twice), so just print it.

> +
> +int intel_engine_cs_mock_selftests(void)
> +{
> +       return intel_mmio_bases_check();

I'd stick this in a
	static const struct i915_subtest tests[] = {
		SUBTEST(mmio_bases_check),
	};

	return i915_subtests(tests, NULL);
just for the convenience of adding more.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: store all mmio bases in intel_engines
URL   : https://patchwork.freedesktop.org/series/39644/
State : success

== Summary ==

---- Known issues:

Test gem_eio:
        Subgroup in-flight-contexts:
                pass       -> INCOMPLETE (shard-apl) fdo#105341
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047
Test pm_lpsp:
        Subgroup screens-disabled:
                pass       -> FAIL       (shard-hsw) fdo#104941

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#104941 https://bugs.freedesktop.org/show_bug.cgi?id=104941

shard-apl        total:3382 pass:1780 dwarn:1   dfail:0   fail:8   skip:1591 time:11735s
shard-hsw        total:3468 pass:1772 dwarn:1   dfail:0   fail:3   skip:1691 time:11645s
shard-snb        total:3468 pass:1364 dwarn:1   dfail:0   fail:2   skip:2101 time:6894s
Blacklisted hosts:
shard-kbl        total:3382 pass:1900 dwarn:1   dfail:0   fail:9   skip:1471 time:8998s

== Logs ==

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