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

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

Hi Matt,

> > +	/*
> > +	 * 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(gt->i915)) {
> > +		intel_engine_mask_t first_ccs = BIT((CCS0 + __ffs(CCS_MASK(gt))));
> > +		intel_engine_mask_t all_ccs = CCS_MASK(gt) << CCS0;
> > +
> > +		info->engine_mask &= ~(all_ccs &= ~first_ccs);
> 
> Shouldn't the second "&=" just be an "&" since there's no need to modify
> the all_ccs variable that never gets used again?

yes, that's a leftover from me trying different ways of removing
all the non first CCS engines.

> In fact since this is DG2-specific, it seems like it might be more
> intuitive to just write the whole thing more directly as
> 
>         if (IS_DG2(gt->i915)) {
>                 int first_ccs = __ffs(CCS_MASK(gt));
> 
>                 info->engine_mask &= ~GENMASK(CCS3, CCS0);
>                 info->engine_mask |= BIT(_CCS(first_ccs));
>         }

yes, looks a bit simpler. Will use this way.

> But up to you; if you just want to remove the unnecessary "=" that's
> fine too.  Either way,
> 
>         Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks!

Andi