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
