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
