From 2edda80db3d065b875731de2a846f9e700ccb5ce Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 7 Aug 2019 12:04:31 +0100 Subject: [PATCH] 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 Reviewed-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20190807110431.8130-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 42 +++++--------------- drivers/gpu/drm/i915/gt/intel_engine_user.c | 23 +++++++++++ drivers/gpu/drm/i915/gt/intel_engine_user.h | 2 + drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 26 +++++++----- 4 files changed, 51 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..86247eeb6f2b 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,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 68fda1ac3c60..c41ae05988a5 100644 --- 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 9e5f113e5027..f845ea1cbfaa 100644 --- 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 cfaa6b296835..3880f07c29b8 100644 --- 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; -- 2.30.2