[PATCH 1/3] drm/i915: Rename engines with to match their user interface

* [PATCH 1/3] drm/i915: Rename engines with to match their user interface
@ 2019-08-07  8:37 Chris Wilson
  2019-08-07  8:37 ` [PATCH 2/3] drm/i915: Use intel_engine_lookup_user for probing HAS_BSD etc Chris Wilson
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Chris Wilson @ 2019-08-07  8:37 UTC (permalink / raw)
  To: intel-gfx

During engine setup, we may find that some engines are fused off causing
a misalignment between internal names and the instances seen by users,
e.g. (I915_ENGINE_CLASS_VIDEO_DECODE, 1) may be vcs2 in hardware.
Normally this is invisible to the user, but a few debug interfaces (and
our own internal tracing) use the original HW name not the name the user
would expect as formed their class:instance tuple. Replace our internal
name with the uabi name for consistency, for example in error states.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111311
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 37 ++++----------------
 drivers/gpu/drm/i915/gt/intel_engine_user.c  | 21 +++++++++++
 drivers/gpu/drm/i915/gt/intel_engine_user.h  |  2 ++
 drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 19 +++++-----
 4 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d0befd6c023a..d38c114b0964 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -55,30 +55,6 @@
 
 #define GEN8_LR_CONTEXT_OTHER_SIZE	( 2 * PAGE_SIZE)
 
-struct engine_class_info {
-	const char *name;
-	u8 uabi_class;
-};
-
-static const struct engine_class_info intel_engine_classes[] = {
-	[RENDER_CLASS] = {
-		.name = "rcs",
-		.uabi_class = I915_ENGINE_CLASS_RENDER,
-	},
-	[COPY_ENGINE_CLASS] = {
-		.name = "bcs",
-		.uabi_class = I915_ENGINE_CLASS_COPY,
-	},
-	[VIDEO_DECODE_CLASS] = {
-		.name = "vcs",
-		.uabi_class = I915_ENGINE_CLASS_VIDEO,
-	},
-	[VIDEO_ENHANCEMENT_CLASS] = {
-		.name = "vecs",
-		.uabi_class = I915_ENGINE_CLASS_VIDEO_ENHANCE,
-	},
-};
-
 #define MAX_MMIO_BASES 3
 struct engine_info {
 	unsigned int hw_id;
@@ -259,11 +235,11 @@ static u32 __engine_mmio_base(struct drm_i915_private *i915,
 	return bases[i].base;
 }
 
-static void __sprint_engine_name(char *name, const struct engine_info *info)
+static void __sprint_engine_name(struct intel_engine_cs *engine)
 {
-	WARN_ON(snprintf(name, INTEL_ENGINE_CS_MAX_NAME, "%s%u",
-			 intel_engine_classes[info->class].name,
-			 info->instance) >= INTEL_ENGINE_CS_MAX_NAME);
+	GEM_WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s'%u",
+			     intel_engine_class_repr(engine),
+			     engine->instance) >= sizeof(engine->name));
 }
 
 void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask)
@@ -292,8 +268,6 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	const struct engine_info *info = &intel_engines[id];
 	struct intel_engine_cs *engine;
 
-	GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes));
-
 	BUILD_BUG_ON(MAX_ENGINE_CLASS >= BIT(GEN11_ENGINE_CLASS_WIDTH));
 	BUILD_BUG_ON(MAX_ENGINE_INSTANCE >= BIT(GEN11_ENGINE_INSTANCE_WIDTH));
 
@@ -317,11 +291,12 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
 	engine->i915 = gt->i915;
 	engine->gt = gt;
 	engine->uncore = gt->uncore;
-	__sprint_engine_name(engine->name, info);
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = __engine_mmio_base(gt->i915, info->mmio_bases);
+
 	engine->class = info->class;
 	engine->instance = info->instance;
+	__sprint_engine_name(engine);
 
 	/*
 	 * To be overridden by the backend on setup. However to facilitate
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 68fda1ac3c60..03b4ace578d7 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -127,6 +127,22 @@ static void set_scheduler_caps(struct drm_i915_private *i915)
 		i915->caps.scheduler = 0;
 }
 
+const char *intel_engine_class_repr(struct intel_engine_cs *engine)
+{
+	static const char *uabi_names[] = {
+		[RENDER_CLASS] = "rcs",
+		[COPY_ENGINE_CLASS] = "bcs",
+		[VIDEO_DECODE_CLASS] = "vcs",
+		[VIDEO_ENHANCEMENT_CLASS] = "vecs",
+	};
+
+	if (engine->class >= ARRAY_SIZE(uabi_names) ||
+	    !uabi_names[engine->class])
+		return "xxx";
+
+	return uabi_names[engine->class];
+}
+
 void intel_engines_driver_register(struct drm_i915_private *i915)
 {
 	u8 uabi_instances[4] = {};
@@ -149,6 +165,11 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
 		GEM_BUG_ON(engine->uabi_class >= ARRAY_SIZE(uabi_instances));
 		engine->uabi_instance = uabi_instances[engine->uabi_class]++;
 
+		/* Replace the internal name with the final user facing name */
+		scnprintf(engine->name, sizeof(engine->name), "%s%u",
+			  intel_engine_class_repr(engine),
+			  engine->uabi_instance);
+
 		rb_link_node(&engine->uabi_node, prev, p);
 		rb_insert_color(&engine->uabi_node, &i915->uabi_engines);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.h b/drivers/gpu/drm/i915/gt/intel_engine_user.h
index 9e5f113e5027..d1a08f336e70 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.h
@@ -17,6 +17,8 @@ intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
 
 unsigned int intel_engines_has_context_isolation(struct drm_i915_private *i915);
 
+const char *intel_engine_class_repr(struct intel_engine_cs *engine);
+
 void intel_engine_add_user(struct intel_engine_cs *engine);
 void intel_engines_driver_register(struct drm_i915_private *i915);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index cfaa6b296835..d0f4217b148a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -12,19 +12,16 @@ static int intel_mmio_bases_check(void *arg)
 
 	for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
 		const struct engine_info *info = &intel_engines[i];
-		char name[INTEL_ENGINE_CS_MAX_NAME];
 		u8 prev = U8_MAX;
 
-		__sprint_engine_name(name, info);
-
 		for (j = 0; j < MAX_MMIO_BASES; j++) {
 			u8 gen = info->mmio_bases[j].gen;
 			u32 base = info->mmio_bases[j].base;
 
 			if (gen >= prev) {
-				pr_err("%s: %s: mmio base for gen %x "
-					"is before the one for gen %x\n",
-				       __func__, name, prev, gen);
+				pr_err("%s(class:%d, instance:%d): mmio base for gen %x is before the one for gen %x\n",
+				       __func__, info->class, info->instance,
+				       prev, gen);
 				return -EINVAL;
 			}
 
@@ -32,17 +29,17 @@ static int intel_mmio_bases_check(void *arg)
 				break;
 
 			if (!base) {
-				pr_err("%s: %s: invalid mmio base (%x) "
-					"for gen %x at entry %u\n",
-				       __func__, name, base, gen, j);
+				pr_err("%s(class:%d, instance:%d): invalid mmio base (%x) for gen %x at entry %u\n",
+				       __func__, info->class, info->instance,
+				       base, gen, j);
 				return -EINVAL;
 			}
 
 			prev = gen;
 		}
 
-		pr_info("%s: min gen supported for %s = %d\n",
-			__func__, name, prev);
+		pr_debug("%s: min gen supported for {class:%d, instance:%d} is %d\n",
+			 __func__, info->class, info->instance, prev);
 	}
 
 	return 0;
-- 
2.23.0.rc1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread