drm/i915: Rename engines to match their user interface

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 from their class:instance tuple. Replace our
internal name with the uabi name for consistency with, for example, error
states.

v2: Keep the pretty printing of class name in the selftest

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111311
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190807110431.8130-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index d0befd6..86247ee 100644 (file)
--- 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,16 @@ 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);
+       /*
+        * Before we know what the uABI name for this engine will be,
+        * we still would like to keep track of this engine in the debug logs.
+        * We throw in a ' here as a reminder that this isn't its final name.
+        */
+       GEM_WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s'%u",
+                            intel_engine_class_repr(engine->class),
+                            engine->instance) >= sizeof(engine->name));
 }
 
 void intel_engine_set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask)
@@ -292,8 +273,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 +296,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 68fda1a..c41ae05 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -127,6 +127,21 @@ static void set_scheduler_caps(struct drm_i915_private *i915)
                i915->caps.scheduler = 0;
 }
 
+const char *intel_engine_class_repr(u8 class)
+{
+       static const char * const uabi_names[] = {
+               [RENDER_CLASS] = "rcs",
+               [COPY_ENGINE_CLASS] = "bcs",
+               [VIDEO_DECODE_CLASS] = "vcs",
+               [VIDEO_ENHANCEMENT_CLASS] = "vecs",
+       };
+
+       if (class >= ARRAY_SIZE(uabi_names) || !uabi_names[class])
+               return "xxx";
+
+       return uabi_names[class];
+}
+
 void intel_engines_driver_register(struct drm_i915_private *i915)
 {
        u8 uabi_instances[4] = {};
@@ -142,6 +157,7 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
                struct intel_engine_cs *engine =
                        container_of((struct rb_node *)it, typeof(*engine),
                                     uabi_node);
+               char old[sizeof(engine->name)];
 
                GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes));
                engine->uabi_class = uabi_classes[engine->class];
@@ -149,6 +165,13 @@ 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 */
+               memcpy(old, engine->name, sizeof(engine->name));
+               scnprintf(engine->name, sizeof(engine->name), "%s%u",
+                         intel_engine_class_repr(engine->class),
+                         engine->uabi_instance);
+               DRM_DEBUG_DRIVER("renamed %s to %s\n", old, engine->name);
+
                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 9e5f113..f845ea1 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.h
@@ -20,4 +20,6 @@ unsigned int intel_engines_has_context_isolation(struct drm_i915_private *i915);
 void intel_engine_add_user(struct intel_engine_cs *engine);
 void intel_engines_driver_register(struct drm_i915_private *i915);
 
+const char *intel_engine_class_repr(u8 class);
+
 #endif /* INTEL_ENGINE_USER_H */

diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
index cfaa6b2..3880f07 100644 (file)
--- a/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_cs.c
@@ -12,19 +12,18 @@ 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(%s, class:%d, instance:%d): mmio base for gen %x is before the one for gen %x\n",
+                                      __func__,
+                                      intel_engine_class_repr(info->class),
+                                      info->class, info->instance,
+                                      prev, gen);
                                return -EINVAL;
                        }
 
@@ -32,17 +31,22 @@ 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(%s, class:%d, instance:%d): invalid mmio base (%x) for gen %x at entry %u\n",
+                                      __func__,
+                                      intel_engine_class_repr(info->class),
+                                      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 %s%d is %d\n",
+                        __func__,
+                        intel_engine_class_repr(info->class),
+                        info->instance,
+                        prev);
        }
 
        return 0;