Hi Tvrtko,

[1]

> > +   /*
> > +    * Loop over all available slices and assign each a user engine.
> > +    *
> > +    * With 1 engine (ccs0):
> > +    *   slice 0, 1, 2, 3: ccs0
> > +    *
> > +    * With 2 engines (ccs0, ccs1):
> > +    *   slice 0, 2: ccs0
> > +    *   slice 1, 3: ccs1
> > +    *
> > +    * With 4 engines (ccs0, ccs1, ccs2, ccs3):
> > +    *   slice 0: ccs0
> > +    *   slice 1: ccs1
> > +    *   slice 2: ccs2
> > +    *   slice 3: ccs3
> > +    *
> > +    * Since the number of slices and the number of engines is
> > +    * known, and we ensure that there is an exact multiple of
> > +    * engines for slices, the double loop becomes a loop over each
> > +    * slice.
> > +    */
> > +   for (i = num_slices / num_engines; i < num_slices; i++) {
> > +           struct intel_engine_cs *engine;
> > +           intel_engine_mask_t tmp;
> > +
> > +           for_each_engine_masked(engine, gt, ALL_CCS(gt), tmp) {
> > +                   /* If a slice is fused off, leave disabled */
> > +                   while (!(CCS_MASK(gt) & BIT(slice)))
> > +                           slice++;
> > +
> > +                   mode &= ~XEHP_CCS_MODE_CSLICE(slice, 
> > XEHP_CCS_MODE_CSLICE_MASK);
> > +                   mode |= XEHP_CCS_MODE_CSLICE(slice, engine->instance);
> > +
> > +                   /* assign the next slice */
> > +                   slice++;
> > +           }
> > +   }
> > +
> > +   intel_uncore_write(gt->uncore, XEHP_CCS_MODE, mode);
> > +}
> > +
> > +void intel_gt_apply_ccs_mode(struct intel_gt *gt)
> > +{
> > +   mutex_lock(&gt->ccs.mutex);
> > +   __intel_gt_apply_ccs_mode(gt);
> > +   mutex_unlock(&gt->ccs.mutex);
> > +}
> > +
> > +void intel_gt_init_ccs_mode(struct intel_gt *gt)
> > +{
> > +   mutex_init(&gt->ccs.mutex);
> > +   gt->ccs.mode = 1;
> 
> What is '1'? And this question carries over to the sysfs interface in the
> following patch - who will use it and where it is documented how to use it?

The value '1' is explained in the comment above[1] and in the
comment below[2]. Maybe we should give it an enum meaning? But
that would be something like CCS_MODE_1/2/4, I thinks
ccs.mode = 1/2/4 is more understandable.

> Also, should this setting somehow be gated by an applicable platform? Or if
> not on setting then when acting on it in __intel_gt_apply_ccs_mode?
> 
> Creation of sysfs files as well should be gated by platform too in the
> following patch?

The idea of this series is to disable the CCS load balancing
(which automatically chooses between mode 1/2/4) and used the
a fixed scheme chosen by the user.

(I'm preparing v3 as Chris was so kind to recommend some changes
offline)

Thanks,
Andi

[2]

> > +   /*
> > +    * Track fixed mapping between CCS engines and compute slices.
> > +    *
> > +    * In order to w/a HW that has the inability to dynamically load
> > +    * balance between CCS engines and EU in the compute slices, we have to
> > +    * reconfigure a static mapping on the fly. We track the current CCS
> > +    * configuration (set by thr user through a sysfs interface) and compare
> > +    * it against the current CCS_MODE (which maps CCS engines to compute
> > +    * slices). If there is only a single engine selected, we can map it to
> > +    * all available compute slices for maximal single task performance
> > +    * (fast/narrow). If there are more then one engine selected, we have to
> > +    * reduce the number of slices allocated to each engine (wide/slow),
> > +    * fairly distributing the EU between the equivalent engines.
> > +    */
> > +   struct {
> > +           struct mutex mutex;
> > +           u32 mode;
> > +   } ccs;

Reply via email to