[PATCH 0/5] Classify the engines in class + instance (v5)

[PATCH 0/5] Classify the engines in class + instance (v5)

[Oscar Mateo]

This refactoring helps simplify a few things here and there.

Daniele Ceraolo Spurio (2):
  drm/i915: Classify the engines in class + instance
  drm/i915: Use the engine class to get the context size

Oscar Mateo (3):
  drm/i915: Use the same vfunc for BSD2 ring init
  drm/i915: Generate the engine name based on the instance number
  drm/i915: Split the engine info table in two levels, using class +
    instance

 drivers/gpu/drm/i915/i915_reg.h              |  8 +++
 drivers/gpu/drm/i915/intel_engine_cs.c       | 80 ++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_lrc.c             | 33 +++++++-----
 drivers/gpu/drm/i915/intel_lrc.h             |  6 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.c      | 14 -----
 drivers/gpu/drm/i915/intel_ringbuffer.h      |  9 +++-
 drivers/gpu/drm/i915/selftests/mock_engine.c |  2 +-
 7 files changed, 99 insertions(+), 53 deletions(-)

-- 
1.9.1

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

[PATCH 1/5] drm/i915: Classify the engines in class + instance

[Oscar Mateo]

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

In such a way that vcs and vcs2 are just two different instances (0 and 1)
of the same engine class (VIDEO_DECODE_CLASS).

v2: Align the instance types (Tvrtko)

v3: Don't use enums for bspec-defined stuff (Michal)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  8 ++++++++
 drivers/gpu/drm/i915/intel_engine_cs.c  | 14 ++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++++
 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 11b12f4..4c72ada 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -85,6 +85,14 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define VECS_HW		3
 #define VCS2_HW		4
 
+/* Engine class */
+
+#define RENDER_CLASS		0
+#define VIDEO_DECODE_CLASS	1
+#define VIDEO_ENHANCEMENT_CLASS	2
+#define COPY_ENGINE_CLASS	3
+#define OTHER_CLASS		4
+
 /* PCI config space */
 
 #define MCHBAR_I915 0x44
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 92f871c..bb22927 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -30,6 +30,8 @@
 	const char *name;
 	unsigned int exec_id;
 	unsigned int hw_id;
+	u8 class;
+	u8 instance;
 	u32 mmio_base;
 	unsigned irq_shift;
 	int (*init_legacy)(struct intel_engine_cs *engine);
@@ -39,6 +41,8 @@
 		.name = "rcs",
 		.hw_id = RCS_HW,
 		.exec_id = I915_EXEC_RENDER,
+		.class = RENDER_CLASS,
+		.instance = 0,
 		.mmio_base = RENDER_RING_BASE,
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 		.init_execlists = logical_render_ring_init,
@@ -48,6 +52,8 @@
 		.name = "bcs",
 		.hw_id = BCS_HW,
 		.exec_id = I915_EXEC_BLT,
+		.class = COPY_ENGINE_CLASS,
+		.instance = 0,
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 		.init_execlists = logical_xcs_ring_init,
@@ -57,6 +63,8 @@
 		.name = "vcs",
 		.hw_id = VCS_HW,
 		.exec_id = I915_EXEC_BSD,
+		.class = VIDEO_DECODE_CLASS,
+		.instance = 0,
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 		.init_execlists = logical_xcs_ring_init,
@@ -66,6 +74,8 @@
 		.name = "vcs2",
 		.hw_id = VCS2_HW,
 		.exec_id = I915_EXEC_BSD,
+		.class = VIDEO_DECODE_CLASS,
+		.instance = 1,
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 		.init_execlists = logical_xcs_ring_init,
@@ -75,6 +85,8 @@
 		.name = "vecs",
 		.hw_id = VECS_HW,
 		.exec_id = I915_EXEC_VEBOX,
+		.class = VIDEO_ENHANCEMENT_CLASS,
+		.instance = 0,
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 		.init_execlists = logical_xcs_ring_init,
@@ -101,6 +113,8 @@
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = info->mmio_base;
 	engine->irq_shift = info->irq_shift;
+	engine->class = info->class;
+	engine->instance = info->instance;
 
 	/* Nothing to do here, execute in order of dependencies */
 	engine->schedule = NULL;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index cbe61d3..f54fe7d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -193,6 +193,10 @@ struct intel_engine_cs {
 	enum intel_engine_id id;
 	unsigned int exec_id;
 	unsigned int hw_id;
+
+	u8 class;
+	u8 instance;
+
 	unsigned int guc_id;
 	u32		mmio_base;
 	unsigned int irq_shift;
-- 
1.9.1

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

[PATCH 2/5] drm/i915: Use the same vfunc for BSD2 ring init

[Oscar Mateo]

If we needed to do something different for the init functions, we could
always look at the engine instance to make the distinction. But, in any
case, the two functions are virtually identical already (please notice
that BSD2_RING is only used from gen8 onwards).

With this, the init functions depends excusively on the engine class
(a fact that we will use soon).

v2: Commit message

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 14 --------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
 3 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index bb22927..a7ffa4c 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -79,7 +79,7 @@
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_bsd2_ring_buffer,
+		.init_legacy = intel_init_bsd_ring_buffer,
 	},
 	[VECS] = {
 		.name = "vecs",
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c98acc2..81eee42 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2175,20 +2175,6 @@ int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine)
 	return intel_init_ring_buffer(engine);
 }
 
-/**
- * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3)
- */
-int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine)
-{
-	struct drm_i915_private *dev_priv = engine->i915;
-
-	intel_ring_default_vfuncs(dev_priv, engine);
-
-	engine->emit_flush = gen6_bsd_ring_flush;
-
-	return intel_init_ring_buffer(engine);
-}
-
 int intel_init_blt_ring_buffer(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index f54fe7d..8b53ddb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -555,7 +555,6 @@ int intel_ring_pin(struct intel_ring *ring,
 
 int intel_init_render_ring_buffer(struct intel_engine_cs *engine);
 int intel_init_bsd_ring_buffer(struct intel_engine_cs *engine);
-int intel_init_bsd2_ring_buffer(struct intel_engine_cs *engine);
 int intel_init_blt_ring_buffer(struct intel_engine_cs *engine);
 int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine);
 
-- 
1.9.1

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

[PATCH 3/5] drm/i915: Generate the engine name based on the instance number

[Oscar Mateo]

Not really needed, but makes the next change a little bit more compact.

v2:
  - Use zero-based numbering for engine names: xcs0, xcs1.. xcsN (Tvrtko, Chris)
  - Make sure the mock engine name is null-terminated (Tvrtko, Chris)

v3: Because I'm stupid (Chris)

v4: Verify engine name wasn't truncated (Michal)

v5:
  - Kill the warning in mock engine (Chris)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c       | 5 +++--
 drivers/gpu/drm/i915/intel_ringbuffer.h      | 4 +++-
 drivers/gpu/drm/i915/selftests/mock_engine.c | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a7ffa4c..5e5cda0 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -71,7 +71,7 @@
 		.init_legacy = intel_init_bsd_ring_buffer,
 	},
 	[VCS2] = {
-		.name = "vcs2",
+		.name = "vcs",
 		.hw_id = VCS2_HW,
 		.exec_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
@@ -108,7 +108,8 @@
 
 	engine->id = id;
 	engine->i915 = dev_priv;
-	engine->name = info->name;
+	WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u",
+			 info->name, info->instance) >= sizeof(engine->name));
 	engine->exec_id = info->exec_id;
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = info->mmio_base;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8b53ddb..fd8994b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -187,9 +187,11 @@ enum intel_engine_id {
 	VECS
 };
 
+#define INTEL_ENGINE_CS_MAX_NAME 8
+
 struct intel_engine_cs {
 	struct drm_i915_private *i915;
-	const char	*name;
+	char name[INTEL_ENGINE_CS_MAX_NAME];
 	enum intel_engine_id id;
 	unsigned int exec_id;
 	unsigned int hw_id;
diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index b89050e..b8e53bd 100644
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -140,7 +140,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
 
 	/* minimal engine setup for requests */
 	engine->base.i915 = i915;
-	engine->base.name = name;
+	snprintf(engine->base.name, sizeof(engine->base.name), "%s", name);
 	engine->base.id = id++;
 	engine->base.status_page.page_addr = (void *)(engine + 1);
 
-- 
1.9.1

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

[PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance

[Oscar Mateo]

There are some properties that logically belong to the engine class, and some
that belong to the engine instance. Make it explicit.

v2: Commit message (Tvrtko)

v3:
  - Rebased
  - Exec/uabi id should be per instance (Chris)

v4:
  - Rebased
  - Avoid re-ordering fields for smaller diff (Tvrtko)
  - Bug on oob access to the class array (Michal)

v5: Bug on the right thing (Michal)

v6: Rebased

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>

Conflicts:
	drivers/gpu/drm/i915/intel_engine_cs.c
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 65 ++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 5e5cda0..80cb0ff 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -26,71 +26,84 @@
 #include "intel_ringbuffer.h"
 #include "intel_lrc.h"
 
-static const struct engine_info {
+struct engine_class_info {
 	const char *name;
-	unsigned int exec_id;
+	int (*init_legacy)(struct intel_engine_cs *engine);
+	int (*init_execlists)(struct intel_engine_cs *engine);
+};
+
+static const struct engine_class_info intel_engine_classes[] = {
+	[RENDER_CLASS] = {
+		.name = "rcs",
+		.init_execlists = logical_render_ring_init,
+		.init_legacy = intel_init_render_ring_buffer,
+	},
+	[COPY_ENGINE_CLASS] = {
+		.name = "bcs",
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_blt_ring_buffer,
+	},
+	[VIDEO_DECODE_CLASS] = {
+		.name = "vcs",
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_bsd_ring_buffer,
+	},
+	[VIDEO_ENHANCEMENT_CLASS] = {
+		.name = "vecs",
+		.init_execlists = logical_xcs_ring_init,
+		.init_legacy = intel_init_vebox_ring_buffer,
+	},
+};
+
+struct engine_info {
 	unsigned int hw_id;
+	unsigned int exec_id;
 	u8 class;
 	u8 instance;
 	u32 mmio_base;
 	unsigned irq_shift;
-	int (*init_legacy)(struct intel_engine_cs *engine);
-	int (*init_execlists)(struct intel_engine_cs *engine);
-} intel_engines[] = {
+};
+
+static const struct engine_info intel_engines[] = {
 	[RCS] = {
-		.name = "rcs",
 		.hw_id = RCS_HW,
 		.exec_id = I915_EXEC_RENDER,
 		.class = RENDER_CLASS,
 		.instance = 0,
 		.mmio_base = RENDER_RING_BASE,
 		.irq_shift = GEN8_RCS_IRQ_SHIFT,
-		.init_execlists = logical_render_ring_init,
-		.init_legacy = intel_init_render_ring_buffer,
 	},
 	[BCS] = {
-		.name = "bcs",
 		.hw_id = BCS_HW,
 		.exec_id = I915_EXEC_BLT,
 		.class = COPY_ENGINE_CLASS,
 		.instance = 0,
 		.mmio_base = BLT_RING_BASE,
 		.irq_shift = GEN8_BCS_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_blt_ring_buffer,
 	},
 	[VCS] = {
-		.name = "vcs",
 		.hw_id = VCS_HW,
 		.exec_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
 		.mmio_base = GEN6_BSD_RING_BASE,
 		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_bsd_ring_buffer,
 	},
 	[VCS2] = {
-		.name = "vcs",
 		.hw_id = VCS2_HW,
 		.exec_id = I915_EXEC_BSD,
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 1,
 		.mmio_base = GEN8_BSD2_RING_BASE,
 		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_bsd_ring_buffer,
 	},
 	[VECS] = {
-		.name = "vecs",
 		.hw_id = VECS_HW,
 		.exec_id = I915_EXEC_VEBOX,
 		.class = VIDEO_ENHANCEMENT_CLASS,
 		.instance = 0,
 		.mmio_base = VEBOX_RING_BASE,
 		.irq_shift = GEN8_VECS_IRQ_SHIFT,
-		.init_execlists = logical_xcs_ring_init,
-		.init_legacy = intel_init_vebox_ring_buffer,
 	},
 };
 
@@ -99,8 +112,12 @@
 		   enum intel_engine_id id)
 {
 	const struct engine_info *info = &intel_engines[id];
+	const struct engine_class_info *class_info;
 	struct intel_engine_cs *engine;
 
+	GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes));
+	class_info = &intel_engine_classes[info->class];
+
 	GEM_BUG_ON(dev_priv->engine[id]);
 	engine = kzalloc(sizeof(*engine), GFP_KERNEL);
 	if (!engine)
@@ -109,7 +126,7 @@
 	engine->id = id;
 	engine->i915 = dev_priv;
 	WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u",
-			 info->name, info->instance) >= sizeof(engine->name));
+			 class_info->name, info->instance) >= sizeof(engine->name));
 	engine->exec_id = info->exec_id;
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = info->mmio_base;
@@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv)
 	int err = 0;
 
 	for_each_engine(engine, dev_priv, id) {
+		const struct engine_class_info *class_info =
+					&intel_engine_classes[engine->class];
 		int (*init)(struct intel_engine_cs *engine);
 
 		if (i915.enable_execlists)
-			init = intel_engines[id].init_execlists;
+			init = class_info->init_execlists;
 		else
-			init = intel_engines[id].init_legacy;
+			init = class_info->init_legacy;
 		if (!init) {
 			kfree(engine);
 			dev_priv->engine[id] = NULL;
-- 
1.9.1

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

[PATCH 5/5] drm/i915: Use the engine class to get the context size

[Oscar Mateo]

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Technically speaking, the context size is per engine class, not per
instance.

v2: Add MISSING_CASE (Tvrtko)

v3: Rebased

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_lrc.h |  6 +++++-
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4..1c6672c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1908,8 +1908,10 @@ static void execlists_init_reg_state(u32 *regs,
 }
 
 /**
- * intel_lr_context_size() - return the size of the context for an engine
- * @engine: which engine to find the context size for
+ * intel_lr_class_context_size() - return the size of the context for a given
+ * engine class
+ * @dev_priv: i915 device private
+ * @class: which engine class to find the context size for
  *
  * Each engine may require a different amount of space for a context image,
  * so when allocating (or copying) an image, this function can be used to
@@ -1921,25 +1923,32 @@ static void execlists_init_reg_state(u32 *regs,
  * in LRC mode, but does not include the "shared data page" used with
  * GuC submission. The caller should account for this if using the GuC.
  */
-uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
+uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class)
 {
 	int ret = 0;
 
-	WARN_ON(INTEL_GEN(engine->i915) < 8);
+	WARN_ON(INTEL_GEN(dev_priv) < 8);
 
-	switch (engine->id) {
-	case RCS:
-		if (INTEL_GEN(engine->i915) >= 9)
+	switch (class) {
+	case RENDER_CLASS:
+		switch (INTEL_GEN(dev_priv)) {
+		default:
+			MISSING_CASE(INTEL_GEN(dev_priv));
+		case 9:
 			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
-		else
+			break;
+		case 8:
 			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
+			break;
+		}
 		break;
-	case VCS:
-	case BCS:
-	case VECS:
-	case VCS2:
+	case VIDEO_DECODE_CLASS:
+	case VIDEO_ENHANCEMENT_CLASS:
+	case COPY_ENGINE_CLASS:
 		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
 		break;
+	default:
+		MISSING_CASE(class);
 	}
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e8015e7..bde2b6e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -78,7 +78,11 @@ enum {
 struct drm_i915_private;
 struct i915_gem_context;
 
-uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
+uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class);
+static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
+{
+	return intel_lr_class_context_size(engine->i915, engine->class);
+}
 
 void intel_lr_context_resume(struct drm_i915_private *dev_priv);
 uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for Classify the engines in class + instance (v5)

[Patchwork]

== Series Details ==

Series: Classify the engines in class + instance (v5)
URL   : https://patchwork.freedesktop.org/series/22808/
State : warning

== Summary ==

Series 22808v1 Classify the engines in class + instance (v5)
https://patchwork.freedesktop.org/api/1.0/series/22808/revisions/1/mbox/

Test gem_exec_basic:
        Subgroup readonly-default:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup readonly-render:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                dmesg-warn -> DMESG-FAIL (fi-snb-2600) fdo#100643
Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-cpu-read-active:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-cpu-read-noreloc:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-gtt-active:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-gtt-cpu-active:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-gtt-read:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-gtt-read-active:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-gtt-read-noreloc:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-softpin:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup basic-write-cpu:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-write-cpu-active:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-write-gtt-active:
                pass       -> DMESG-WARN (fi-snb-2520m) fdo#100643
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-write-read-active:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
Test gem_exec_store:
        Subgroup basic-all:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-snb-2520m) fdo#100643
        Subgroup basic-render:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
Test gem_exec_suspend:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_mmap_gtt:
        Subgroup basic-copy:
                pass       -> DMESG-WARN (fi-snb-2520m) fdo#100643
        Subgroup basic-read:
                dmesg-warn -> PASS       (fi-snb-2520m) fdo#100643
        Subgroup basic-write-gtt-no-prefault:
                pass       -> DMESG-WARN (fi-snb-2520m) fdo#100643
Test gem_render_tiled_blits:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_ringfill:
        Subgroup basic-default-fd:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test kms_addfb_basic:
        Subgroup addfb25-x-tiled-mismatch:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup addfb25-yf-tiled:
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup bad-pitch-128:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup bad-pitch-32:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup basic-x-tiled:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup basic-y-tiled:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup invalid-get-prop:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup invalid-get-prop-any:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup small-bo:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup tile-pitch-mismatch:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
        Subgroup unused-offsets:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup unused-pitches:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
Test kms_force_connector_basic:
        Subgroup force-edid:
                dmesg-warn -> PASS       (fi-snb-2600) fdo#100643
        Subgroup prune-stale-modes:
                pass       -> DMESG-WARN (fi-snb-2600) fdo#100643
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100445
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (fi-snb-2600)
Test prime_vgem:
        Subgroup basic-fence-mmap:
WARNING: Long output truncated

e461ecfd413fb930d00f44f3de0019e528b4731f drm-tip: 2017y-04m-10d-14h-25m-24s UTC integration manifest
0e152e1 drm/i915: Use the engine class to get the context size
8556ab7 drm/i915: Split the engine info table in two levels, using class + instance
b47e857 drm/i915: Generate the engine name based on the instance number
fdf34ca drm/i915: Use the same vfunc for BSD2 ring init
df41d85 drm/i915: Classify the engines in class + instance

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4467/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4] drm/i915: Use the engine class to get the context size

[Oscar Mateo]

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Technically speaking, the context size is per engine class, not per
instance.

v2: Add MISSING_CASE (Tvrtko)

v3: Rebased

v4: Restore the interface back to hiding the class lookup (Chris)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4..3921566 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1923,23 +1923,31 @@ static void execlists_init_reg_state(u32 *regs,
  */
 uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
 {
+	struct drm_i915_private *dev_priv = engine->i915;
 	int ret = 0;
 
-	WARN_ON(INTEL_GEN(engine->i915) < 8);
+	WARN_ON(INTEL_GEN(dev_priv) < 8);
 
-	switch (engine->id) {
-	case RCS:
-		if (INTEL_GEN(engine->i915) >= 9)
+	switch (engine->class) {
+	case RENDER_CLASS:
+		switch (INTEL_GEN(dev_priv)) {
+		default:
+			MISSING_CASE(INTEL_GEN(dev_priv));
+		case 9:
 			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
-		else
+			break;
+		case 8:
 			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
+			break;
+		}
 		break;
-	case VCS:
-	case BCS:
-	case VECS:
-	case VCS2:
+	case VIDEO_DECODE_CLASS:
+	case VIDEO_ENHANCEMENT_CLASS:
+	case COPY_ENGINE_CLASS:
 		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
 		break;
+	default:
+		MISSING_CASE(engine->class);
 	}
 
 	return ret;
-- 
1.9.1

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

Re: [PATCH 5/5] drm/i915: Use the engine class to get the context size

[Chris Wilson]

On Mon, Apr 10, 2017 at 07:34:33AM -0700, Oscar Mateo wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Technically speaking, the context size is per engine class, not per
> instance.
> 
> v2: Add MISSING_CASE (Tvrtko)
> 
> v3: Rebased
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_lrc.h |  6 +++++-
>  2 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0dc1cc4..1c6672c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1908,8 +1908,10 @@ static void execlists_init_reg_state(u32 *regs,
>  }
>  
>  /**
> - * intel_lr_context_size() - return the size of the context for an engine
> - * @engine: which engine to find the context size for
> + * intel_lr_class_context_size() - return the size of the context for a given
> + * engine class
> + * @dev_priv: i915 device private
> + * @class: which engine class to find the context size for
>   *
>   * Each engine may require a different amount of space for a context image,
>   * so when allocating (or copying) an image, this function can be used to
> @@ -1921,25 +1923,32 @@ static void execlists_init_reg_state(u32 *regs,
>   * in LRC mode, but does not include the "shared data page" used with
>   * GuC submission. The caller should account for this if using the GuC.
>   */
> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class)
>  {
>  	int ret = 0;
>  
> -	WARN_ON(INTEL_GEN(engine->i915) < 8);
> +	WARN_ON(INTEL_GEN(dev_priv) < 8);
>  
> -	switch (engine->id) {
> -	case RCS:
> -		if (INTEL_GEN(engine->i915) >= 9)
> +	switch (class) {
> +	case RENDER_CLASS:
> +		switch (INTEL_GEN(dev_priv)) {
> +		default:
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +		case 9:
>  			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
> -		else
> +			break;
> +		case 8:
>  			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
> +			break;
> +		}
>  		break;
> -	case VCS:
> -	case BCS:
> -	case VECS:
> -	case VCS2:
> +	case VIDEO_DECODE_CLASS:
> +	case VIDEO_ENHANCEMENT_CLASS:
> +	case COPY_ENGINE_CLASS:
>  		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
>  		break;
> +	default:
> +		MISSING_CASE(class);
>  	}
>  
>  	return ret;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index e8015e7..bde2b6e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -78,7 +78,11 @@ enum {
>  struct drm_i915_private;
>  struct i915_gem_context;
>  
> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class);
> +static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
> +{
> +	return intel_lr_class_context_size(engine->i915, engine->class);
> +}

I'm not understanding why you want to push this to the caller.

Patches 1-4 are r-b me, and I'll apply once we have put out the fire.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Use the engine class to get the context size

[Tvrtko Ursulin]

On 11/04/2017 11:25, Chris Wilson wrote:
> On Mon, Apr 10, 2017 at 07:34:33AM -0700, Oscar Mateo wrote:
>> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>
>> Technically speaking, the context size is per engine class, not per
>> instance.
>>
>> v2: Add MISSING_CASE (Tvrtko)
>>
>> v3: Rebased
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c | 33 +++++++++++++++++++++------------
>>  drivers/gpu/drm/i915/intel_lrc.h |  6 +++++-
>>  2 files changed, 26 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 0dc1cc4..1c6672c 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1908,8 +1908,10 @@ static void execlists_init_reg_state(u32 *regs,
>>  }
>>
>>  /**
>> - * intel_lr_context_size() - return the size of the context for an engine
>> - * @engine: which engine to find the context size for
>> + * intel_lr_class_context_size() - return the size of the context for a given
>> + * engine class
>> + * @dev_priv: i915 device private
>> + * @class: which engine class to find the context size for
>>   *
>>   * Each engine may require a different amount of space for a context image,
>>   * so when allocating (or copying) an image, this function can be used to
>> @@ -1921,25 +1923,32 @@ static void execlists_init_reg_state(u32 *regs,
>>   * in LRC mode, but does not include the "shared data page" used with
>>   * GuC submission. The caller should account for this if using the GuC.
>>   */
>> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class)
>>  {
>>  	int ret = 0;
>>
>> -	WARN_ON(INTEL_GEN(engine->i915) < 8);
>> +	WARN_ON(INTEL_GEN(dev_priv) < 8);
>>
>> -	switch (engine->id) {
>> -	case RCS:
>> -		if (INTEL_GEN(engine->i915) >= 9)
>> +	switch (class) {
>> +	case RENDER_CLASS:
>> +		switch (INTEL_GEN(dev_priv)) {
>> +		default:
>> +			MISSING_CASE(INTEL_GEN(dev_priv));
>> +		case 9:
>>  			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
>> -		else
>> +			break;
>> +		case 8:
>>  			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
>> +			break;
>> +		}
>>  		break;
>> -	case VCS:
>> -	case BCS:
>> -	case VECS:
>> -	case VCS2:
>> +	case VIDEO_DECODE_CLASS:
>> +	case VIDEO_ENHANCEMENT_CLASS:
>> +	case COPY_ENGINE_CLASS:
>>  		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
>>  		break;
>> +	default:
>> +		MISSING_CASE(class);
>>  	}
>>
>>  	return ret;
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
>> index e8015e7..bde2b6e 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.h
>> +++ b/drivers/gpu/drm/i915/intel_lrc.h
>> @@ -78,7 +78,11 @@ enum {
>>  struct drm_i915_private;
>>  struct i915_gem_context;
>>
>> -uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
>> +uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class);
>> +static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>> +{
>> +	return intel_lr_class_context_size(engine->i915, engine->class);
>> +}
>
> I'm not understanding why you want to push this to the caller.
>
> Patches 1-4 are r-b me, and I'll apply once we have put out the fire.

Just to say you can keep my r-b if you change this detail. I wondered 
the same myself but didn't consider it too critical.

Regards,

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

Re: [PATCH 4/5] drm/i915: Split the engine info table in two levels, using class + instance

[Tvrtko Ursulin]

On 10/04/2017 15:34, Oscar Mateo wrote:
> There are some properties that logically belong to the engine class, and some
> that belong to the engine instance. Make it explicit.
>
> v2: Commit message (Tvrtko)
>
> v3:
>   - Rebased
>   - Exec/uabi id should be per instance (Chris)
>
> v4:
>   - Rebased
>   - Avoid re-ordering fields for smaller diff (Tvrtko)
>   - Bug on oob access to the class array (Michal)
>
> v5: Bug on the right thing (Michal)
>
> v6: Rebased
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>
> Conflicts:
> 	drivers/gpu/drm/i915/intel_engine_cs.c

Snip this next time unless it sneaked in by accident.

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 65 ++++++++++++++++++++++------------
>  1 file changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 5e5cda0..80cb0ff 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -26,71 +26,84 @@
>  #include "intel_ringbuffer.h"
>  #include "intel_lrc.h"
>
> -static const struct engine_info {
> +struct engine_class_info {
>  	const char *name;
> -	unsigned int exec_id;
> +	int (*init_legacy)(struct intel_engine_cs *engine);
> +	int (*init_execlists)(struct intel_engine_cs *engine);
> +};
> +
> +static const struct engine_class_info intel_engine_classes[] = {
> +	[RENDER_CLASS] = {
> +		.name = "rcs",
> +		.init_execlists = logical_render_ring_init,
> +		.init_legacy = intel_init_render_ring_buffer,
> +	},
> +	[COPY_ENGINE_CLASS] = {
> +		.name = "bcs",
> +		.init_execlists = logical_xcs_ring_init,
> +		.init_legacy = intel_init_blt_ring_buffer,
> +	},
> +	[VIDEO_DECODE_CLASS] = {
> +		.name = "vcs",
> +		.init_execlists = logical_xcs_ring_init,
> +		.init_legacy = intel_init_bsd_ring_buffer,
> +	},
> +	[VIDEO_ENHANCEMENT_CLASS] = {
> +		.name = "vecs",
> +		.init_execlists = logical_xcs_ring_init,
> +		.init_legacy = intel_init_vebox_ring_buffer,
> +	},
> +};
> +
> +struct engine_info {
>  	unsigned int hw_id;
> +	unsigned int exec_id;
>  	u8 class;
>  	u8 instance;
>  	u32 mmio_base;
>  	unsigned irq_shift;
> -	int (*init_legacy)(struct intel_engine_cs *engine);
> -	int (*init_execlists)(struct intel_engine_cs *engine);
> -} intel_engines[] = {
> +};
> +
> +static const struct engine_info intel_engines[] = {
>  	[RCS] = {
> -		.name = "rcs",
>  		.hw_id = RCS_HW,
>  		.exec_id = I915_EXEC_RENDER,
>  		.class = RENDER_CLASS,
>  		.instance = 0,
>  		.mmio_base = RENDER_RING_BASE,
>  		.irq_shift = GEN8_RCS_IRQ_SHIFT,
> -		.init_execlists = logical_render_ring_init,
> -		.init_legacy = intel_init_render_ring_buffer,
>  	},
>  	[BCS] = {
> -		.name = "bcs",
>  		.hw_id = BCS_HW,
>  		.exec_id = I915_EXEC_BLT,
>  		.class = COPY_ENGINE_CLASS,
>  		.instance = 0,
>  		.mmio_base = BLT_RING_BASE,
>  		.irq_shift = GEN8_BCS_IRQ_SHIFT,
> -		.init_execlists = logical_xcs_ring_init,
> -		.init_legacy = intel_init_blt_ring_buffer,
>  	},
>  	[VCS] = {
> -		.name = "vcs",
>  		.hw_id = VCS_HW,
>  		.exec_id = I915_EXEC_BSD,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
>  		.mmio_base = GEN6_BSD_RING_BASE,
>  		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
> -		.init_execlists = logical_xcs_ring_init,
> -		.init_legacy = intel_init_bsd_ring_buffer,
>  	},
>  	[VCS2] = {
> -		.name = "vcs",
>  		.hw_id = VCS2_HW,
>  		.exec_id = I915_EXEC_BSD,
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 1,
>  		.mmio_base = GEN8_BSD2_RING_BASE,
>  		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
> -		.init_execlists = logical_xcs_ring_init,
> -		.init_legacy = intel_init_bsd_ring_buffer,
>  	},
>  	[VECS] = {
> -		.name = "vecs",
>  		.hw_id = VECS_HW,
>  		.exec_id = I915_EXEC_VEBOX,
>  		.class = VIDEO_ENHANCEMENT_CLASS,
>  		.instance = 0,
>  		.mmio_base = VEBOX_RING_BASE,
>  		.irq_shift = GEN8_VECS_IRQ_SHIFT,
> -		.init_execlists = logical_xcs_ring_init,
> -		.init_legacy = intel_init_vebox_ring_buffer,
>  	},
>  };
>
> @@ -99,8 +112,12 @@
>  		   enum intel_engine_id id)
>  {
>  	const struct engine_info *info = &intel_engines[id];
> +	const struct engine_class_info *class_info;
>  	struct intel_engine_cs *engine;
>
> +	GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes));
> +	class_info = &intel_engine_classes[info->class];
> +
>  	GEM_BUG_ON(dev_priv->engine[id]);
>  	engine = kzalloc(sizeof(*engine), GFP_KERNEL);
>  	if (!engine)
> @@ -109,7 +126,7 @@
>  	engine->id = id;
>  	engine->i915 = dev_priv;
>  	WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u",
> -			 info->name, info->instance) >= sizeof(engine->name));
> +			 class_info->name, info->instance) >= sizeof(engine->name));
>  	engine->exec_id = info->exec_id;
>  	engine->hw_id = engine->guc_id = info->hw_id;
>  	engine->mmio_base = info->mmio_base;
> @@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv)
>  	int err = 0;
>
>  	for_each_engine(engine, dev_priv, id) {
> +		const struct engine_class_info *class_info =
> +					&intel_engine_classes[engine->class];
>  		int (*init)(struct intel_engine_cs *engine);
>
>  		if (i915.enable_execlists)
> -			init = intel_engines[id].init_execlists;
> +			init = class_info->init_execlists;
>  		else
> -			init = intel_engines[id].init_legacy;
> +			init = class_info->init_legacy;
>  		if (!init) {
>  			kfree(engine);
>  			dev_priv->engine[id] = NULL;
>

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

Re: [PATCH 5/5] drm/i915: Use the engine class to get the context size

[Chris Wilson]

On Tue, Apr 11, 2017 at 12:32:14PM +0100, Tvrtko Ursulin wrote:
> 
> On 11/04/2017 11:25, Chris Wilson wrote:
> >On Mon, Apr 10, 2017 at 07:34:33AM -0700, Oscar Mateo wrote:
> >>From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >>
> >>Technically speaking, the context size is per engine class, not per
> >>instance.
> >>
> >>---
> >>diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> >>index e8015e7..bde2b6e 100644
> >>--- a/drivers/gpu/drm/i915/intel_lrc.h
> >>+++ b/drivers/gpu/drm/i915/intel_lrc.h
> >>@@ -78,7 +78,11 @@ enum {
> >> struct drm_i915_private;
> >> struct i915_gem_context;
> >>
> >>-uint32_t intel_lr_context_size(struct intel_engine_cs *engine);
> >>+uint32_t intel_lr_class_context_size(struct drm_i915_private *dev_priv, u8 class);
> >>+static inline uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
> >>+{
> >>+	return intel_lr_class_context_size(engine->i915, engine->class);
> >>+}
> >
> >I'm not understanding why you want to push this to the caller.
> >
> >Patches 1-4 are r-b me, and I'll apply once we have put out the fire.
> 
> Just to say you can keep my r-b if you change this detail. I
> wondered the same myself but didn't consider it too critical.

I've pushed the first 4 patches. Patch 5 is good to go once we have
clarification why you want to expose the per-class lookup to
context_size (or restore the interface back to hiding the class lookup).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915: Use the engine class to get the context size

[Chris Wilson]

On Tue, Apr 11, 2017 at 03:11:12AM -0700, Oscar Mateo wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Technically speaking, the context size is per engine class, not per
> instance.
> 
> v2: Add MISSING_CASE (Tvrtko)
> 
> v3: Rebased
> 
> v4: Restore the interface back to hiding the class lookup (Chris)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0dc1cc4..3921566 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1923,23 +1923,31 @@ static void execlists_init_reg_state(u32 *regs,
>   */
>  uint32_t intel_lr_context_size(struct intel_engine_cs *engine)
>  {
> +	struct drm_i915_private *dev_priv = engine->i915;
>  	int ret = 0;
>  
> -	WARN_ON(INTEL_GEN(engine->i915) < 8);
> +	WARN_ON(INTEL_GEN(dev_priv) < 8);
>  
> -	switch (engine->id) {
> -	case RCS:
> -		if (INTEL_GEN(engine->i915) >= 9)
> +	switch (engine->class) {
> +	case RENDER_CLASS:
> +		switch (INTEL_GEN(dev_priv)) {
> +		default:
> +			MISSING_CASE(INTEL_GEN(dev_priv));
> +		case 9:
>  			ret = GEN9_LR_CONTEXT_RENDER_SIZE;
> -		else
> +			break;
> +		case 8:
>  			ret = GEN8_LR_CONTEXT_RENDER_SIZE;
> +			break;
> +		}
>  		break;
> -	case VCS:
> -	case BCS:
> -	case VECS:
> -	case VCS2:
> +	case VIDEO_DECODE_CLASS:
> +	case VIDEO_ENHANCEMENT_CLASS:
> +	case COPY_ENGINE_CLASS:
>  		ret = GEN8_LR_CONTEXT_OTHER_SIZE;
>  		break;
> +	default:
> +		MISSING_CASE(engine->class);

Sorry, couldn't resist moving this MISSING_CASE so that both switches
were consistent in style.

Thanks for the patch, pushed. Better update the igt names again...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx