Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > 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@xxxxxxxxxxxxxxxxxx> > --- > 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", ^ ? unexpected > + 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) I dont mind the repr, but we have been using _str. > +{ > + 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"; or "unknown" shrug. > + > + 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); Hmm, this was the reason for ' ? -Mika > + > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
