On Fri, Mar 08, 2024 at 09:22:17PM +0100, Andi Shyti wrote:
> For the upcoming changes we need a cleaner way to build the list
> of uabi engines.
>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v6.2+
I don't really see why we need patches 2 & 3 in this series. If we want
to restrict the platform to a single CCS engine for now (and give that
single engine access to all of the cslices), it would be much simpler to
only create a single intel_engine_cs which which would then cause both
i915 and userspace to only consider a single engine, even if more than
one is physically present. That could be done with a simple adjustment
to engine_mask_apply_compute_fuses() to mask off extra bits from the
engine mask such that only a single CCS can get returned rather than the
mask of all CCSs that are present.
Managing all of the engines in the KMD but only exposing one (some) of
them to userspace might be something we need if you want to add extra
functionality down to road to "hotplug" extra engines, or to allow
userspace to explicitly request multi-CCS mode. But none of that seems
necessary for this series, especially for something you're backporting
to stable kernels.
Matt
> ---
> drivers/gpu/drm/i915/gt/intel_engine_user.c | 29 ++++++++++++---------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> index 833987015b8b..11cc06c0c785 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> @@ -203,7 +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;
> + u16 class_instance[I915_LAST_UABI_ENGINE_CLASS + 2] = { };
> struct legacy_ring ring = {};
> struct list_head *it, *next;
> struct rb_node **p, *prev;
> @@ -214,6 +214,8 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
> prev = NULL;
> p = &i915->uabi_engines.rb_node;
> list_for_each_safe(it, next, &engines) {
> + u16 uabi_class;
> +
> struct intel_engine_cs *engine =
> container_of(it, typeof(*engine), uabi_list);
>
> @@ -222,15 +224,14 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
>
> 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;
> +
> + if (engine->uabi_class == I915_NO_UABI_CLASS)
> + uabi_class = I915_LAST_UABI_ENGINE_CLASS + 1;
> + else
> + uabi_class = engine->uabi_class;
> +
> + GEM_BUG_ON(uabi_class >= ARRAY_SIZE(class_instance));
> + engine->uabi_instance = class_instance[uabi_class]++;
>
> /*
> * Replace the internal name with the final user and log facing
> @@ -238,11 +239,15 @@ void intel_engines_driver_register(struct drm_i915_private *i915)
> */
> engine_rename(engine,
> intel_engine_class_repr(engine->class),
> - name_instance);
> + engine->uabi_instance);
>
> - if (engine->uabi_class == I915_NO_UABI_CLASS)
> + if (uabi_class > I915_LAST_UABI_ENGINE_CLASS)
> continue;
>
> + GEM_BUG_ON(uabi_class >=
> + ARRAY_SIZE(i915->engine_uabi_class_count));
> + i915->engine_uabi_class_count[uabi_class]++;
> +
> rb_link_node(&engine->uabi_node, prev, p);
> rb_insert_color(&engine->uabi_node, &i915->uabi_engines);
>
> --
> 2.43.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
