Re: [PATCH v5 2/4] drm/i915/gt: Refactor uabi engine class/instance list creation

[Andi Shyti <andi.shyti@linux.intel.com>]

On Tue, Mar 12, 2024 at 10:08:33AM -0700, Matt Roper wrote:
> 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@intel.com>
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # v6.2+
> 
> I don't really see why we need patches 2 & 3 in this series. 

For patch number '2' We had a round of review with Tvrtko and we
wanted to avoid the change I pasted at the bottom[*], which would
decrease something that was increased earlier.

> 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.

It's true, it would even be easier to mask out all the CCS
engines after the first. I thought of this.

On one hand hand, adding a for_each_available_engine() throught
the stable path its a bit of abusing, but it's functional to the
single CCS mode.

I was aiming for a longer term solution. If I add a patch to mask
off CCS engines, then I will need to revert it quite soon for
the stable release.

I'm not sure which one is better, though.

Thanks,
Andi

[*]
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
index 833987015b8b..7041acc77810 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
@@ -243,6 +243,15 @@  void intel_engines_driver_register(struct drm_i915_private *i915)
 		if (engine->uabi_class == I915_NO_UABI_CLASS)
 			continue;

+		/*
+		 * Do not list and do not count CCS engines other than the first
+		 */
+		if (engine->uabi_class == I915_ENGINE_CLASS_COMPUTE &&
+		    engine->uabi_instance > 0) {
+			i915->engine_uabi_class_count[engine->uabi_class]--;
+			continue;
+		}
+
 		rb_link_node(&engine->uabi_node, prev, p);
 		rb_insert_color(&engine->uabi_node, &i915->uabi_engines);