[PATCH v6 2/3] drm/i915/gt: Do not generate the command streamer for all the CCS

[Matt Roper]

On Tue, Mar 26, 2024 at 07:42:34PM +0100, Andi Shyti wrote:
> Hi Matt,
> 
> On Tue, Mar 26, 2024 at 09:03:10AM -0700, Matt Roper wrote:
> > On Wed, Mar 13, 2024 at 09:19:50PM +0100, Andi Shyti wrote:
> > > +			/*
> > > +			 * Do not create the command streamer for CCS slices
> > > +			 * beyond the first. All the workload submitted to the
> > > +			 * first engine will be shared among all the slices.
> > > +			 *
> > > +			 * Once the user will be allowed to customize the CCS
> > > +			 * mode, then this check needs to be removed.
> > > +			 */
> > > +			if (IS_DG2(i915) &&
> > > +			    class == COMPUTE_CLASS &&
> > > +			    ccs_instance++)
> > > +				continue;
> > 
> > Wouldn't it be more intuitive to drop the non-lowest CCS engines in
> > init_engine_mask() since that's the function that's dedicated to
> > building the list of engines we'll use?  Then we don't need to kill the
> > assertion farther down either.
> 
> Because we don't check the result of init_engine_mask() while
> creating the engine's structure. We check it only after and
> indeed I removed the drm_WARN_ON() check.
> 
> I think the whole process of creating the engine's structure in
> the intel_engines_init_mmio() can be simplified, but this goes
> beyong the scope of the series.
> 
> Or am I missing something?

The important part of init_engine_mask isn't the return value, but
rather that it's what sets up gt->info.engine_mask.  The HAS_ENGINE()
check that intel_engines_init_mmio() uses is based on the value stored
there, so updating that function will also ensure that we skip the
engines we don't want in the loop.


Matt

> 
> Thanks,
> Andi

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation