| Message ID | 20240327155622.538140-3-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Disable automatic load CCS load balancing | expand |
On Wed, Mar 27, 2024 at 04:56:18PM +0100, Andi Shyti wrote: > We want a fixed load CCS balancing consisting in all slices > sharing one single user engine. For this reason do not create the > intel_engine_cs structure with its dedicated command streamer for > CCS slices beyond the first. > > Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Chris Wilson <chris.p.wilson@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: <stable@vger.kernel.org> # v6.2+ > Acked-by: Michal Mrozek <michal.mrozek@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > index f553cf4e6449..47c4a69e854c 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c > @@ -908,6 +908,21 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) > info->engine_mask &= ~BIT(GSC0); > } > > + /* > + * 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? 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)); } 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> Matt > + } > + > return info->engine_mask; > } > > -- > 2.43.0 >
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
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index f553cf4e6449..47c4a69e854c 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -908,6 +908,21 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) info->engine_mask &= ~BIT(GSC0); } + /* + * 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); + } + return info->engine_mask; }
We want a fixed load CCS balancing consisting in all slices sharing one single user engine. For this reason do not create the intel_engine_cs structure with its dedicated command streamer for CCS slices beyond the first. Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: <stable@vger.kernel.org> # v6.2+ Acked-by: Michal Mrozek <michal.mrozek@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)