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

[Andi Shyti]

Hi Matt,

On Tue, Mar 26, 2024 at 02:30:33PM -0700, Matt Roper wrote:
> On Tue, Mar 26, 2024 at 07:42:34PM +0100, Andi Shyti wrote:
> > 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.

Yes, can do like this, as well. After all this is done I'm going
to do some cleanup here, as well.

Thanks,
Andi