| Message ID | 20231116084456.291533-1-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/gsc: Mark internal GSC engine with reserved uabi class | expand |
On 11/16/2023 12:44 AM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > The GSC CS is not exposed to the user, so we skipped assigning a uabi > class number for it. However, the trace logs use the uabi class and > instance to identify the engine, so leaving uabi class unset makes the > GSC CS show up as the RCS in those logs. > > Given that the engine is not exposed to the user, we can't add a new > case in the uabi enum, so we insted internally define a kernel > internal class as -1. > > At the same time remove special handling for the name and complete > the uabi_classes array so internal class is automatically correctly > assigned. > > Engine will show as 65535:0 other0 in the logs/traces which should > be unique enough. > > v2: > * Fix uabi class u8 vs u16 type confusion. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Fixes: 194babe26bdc ("drm/i915/mtl: don't expose GSC command streamer to the user") > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Alan Previn <alan.previn.teres.alexis@intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> # v1 My r-b stands. Thanks, Daniele > --- > drivers/gpu/drm/i915/gt/intel_engine_user.c | 39 ++++++++++++--------- > 1 file changed, 22 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c > index 118164ddbb2e..833987015b8b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c > @@ -41,12 +41,15 @@ void intel_engine_add_user(struct intel_engine_cs *engine) > llist_add(&engine->uabi_llist, &engine->i915->uabi_engines_llist); > } > > -static const u8 uabi_classes[] = { > +#define I915_NO_UABI_CLASS ((u16)(-1)) > + > +static const u16 uabi_classes[] = { > [RENDER_CLASS] = I915_ENGINE_CLASS_RENDER, > [COPY_ENGINE_CLASS] = I915_ENGINE_CLASS_COPY, > [VIDEO_DECODE_CLASS] = I915_ENGINE_CLASS_VIDEO, > [VIDEO_ENHANCEMENT_CLASS] = I915_ENGINE_CLASS_VIDEO_ENHANCE, > [COMPUTE_CLASS] = I915_ENGINE_CLASS_COMPUTE, > + [OTHER_CLASS] = I915_NO_UABI_CLASS, /* Not exposed to users, no uabi class. */ > }; > > static int engine_cmp(void *priv, const struct list_head *A, > @@ -200,6 +203,7 @@ static void engine_rename(struct intel_engine_cs *engine, const char *name, u16 > > void intel_engines_driver_register(struct drm_i915_private *i915) > { > + u16 name_instance, other_instance = 0; > struct legacy_ring ring = {}; > struct list_head *it, *next; > struct rb_node **p, *prev; > @@ -216,27 +220,28 @@ void intel_engines_driver_register(struct drm_i915_private *i915) > if (intel_gt_has_unrecoverable_error(engine->gt)) > continue; /* ignore incomplete engines */ > > - /* > - * We don't want to expose the GSC engine to the users, but we > - * still rename it so it is easier to identify in the debug logs > - */ > - if (engine->id == GSC0) { > - engine_rename(engine, "gsc", 0); > - continue; > - } > - > GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes)); > engine->uabi_class = uabi_classes[engine->class]; > + if (engine->uabi_class == I915_NO_UABI_CLASS) { > + name_instance = other_instance++; > + } else { > + GEM_BUG_ON(engine->uabi_class >= > + ARRAY_SIZE(i915->engine_uabi_class_count)); > + name_instance = > + i915->engine_uabi_class_count[engine->uabi_class]++; > + } > + engine->uabi_instance = name_instance; > > - GEM_BUG_ON(engine->uabi_class >= > - ARRAY_SIZE(i915->engine_uabi_class_count)); > - engine->uabi_instance = > - i915->engine_uabi_class_count[engine->uabi_class]++; > - > - /* Replace the internal name with the final user facing name */ > + /* > + * Replace the internal name with the final user and log facing > + * name. > + */ > engine_rename(engine, > intel_engine_class_repr(engine->class), > - engine->uabi_instance); > + name_instance); > + > + if (engine->uabi_class == I915_NO_UABI_CLASS) > + continue; > > 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.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c index 118164ddbb2e..833987015b8b 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c @@ -41,12 +41,15 @@ void intel_engine_add_user(struct intel_engine_cs *engine) llist_add(&engine->uabi_llist, &engine->i915->uabi_engines_llist); } -static const u8 uabi_classes[] = { +#define I915_NO_UABI_CLASS ((u16)(-1)) + +static const u16 uabi_classes[] = { [RENDER_CLASS] = I915_ENGINE_CLASS_RENDER, [COPY_ENGINE_CLASS] = I915_ENGINE_CLASS_COPY, [VIDEO_DECODE_CLASS] = I915_ENGINE_CLASS_VIDEO, [VIDEO_ENHANCEMENT_CLASS] = I915_ENGINE_CLASS_VIDEO_ENHANCE, [COMPUTE_CLASS] = I915_ENGINE_CLASS_COMPUTE, + [OTHER_CLASS] = I915_NO_UABI_CLASS, /* Not exposed to users, no uabi class. */ }; static int engine_cmp(void *priv, const struct list_head *A, @@ -200,6 +203,7 @@ static void engine_rename(struct intel_engine_cs *engine, const char *name, u16 void intel_engines_driver_register(struct drm_i915_private *i915) { + u16 name_instance, other_instance = 0; struct legacy_ring ring = {}; struct list_head *it, *next; struct rb_node **p, *prev; @@ -216,27 +220,28 @@ void intel_engines_driver_register(struct drm_i915_private *i915) if (intel_gt_has_unrecoverable_error(engine->gt)) continue; /* ignore incomplete engines */ - /* - * We don't want to expose the GSC engine to the users, but we - * still rename it so it is easier to identify in the debug logs - */ - if (engine->id == GSC0) { - engine_rename(engine, "gsc", 0); - continue; - } - GEM_BUG_ON(engine->class >= ARRAY_SIZE(uabi_classes)); engine->uabi_class = uabi_classes[engine->class]; + if (engine->uabi_class == I915_NO_UABI_CLASS) { + name_instance = other_instance++; + } else { + GEM_BUG_ON(engine->uabi_class >= + ARRAY_SIZE(i915->engine_uabi_class_count)); + name_instance = + i915->engine_uabi_class_count[engine->uabi_class]++; + } + engine->uabi_instance = name_instance; - GEM_BUG_ON(engine->uabi_class >= - ARRAY_SIZE(i915->engine_uabi_class_count)); - engine->uabi_instance = - i915->engine_uabi_class_count[engine->uabi_class]++; - - /* Replace the internal name with the final user facing name */ + /* + * Replace the internal name with the final user and log facing + * name. + */ engine_rename(engine, intel_engine_class_repr(engine->class), - engine->uabi_instance); + name_instance); + + if (engine->uabi_class == I915_NO_UABI_CLASS) + continue; rb_link_node(&engine->uabi_node, prev, p); rb_insert_color(&engine->uabi_node, &i915->uabi_engines);