Re: [PATCH v7] cpufreq: intel_pstate: Implement passive mode with HWP enabled

From: Rafael J. Wysocki
Date: Tue Aug 11 2020 - 11:33:55 EST

On Tuesday, August 11, 2020 2:51:41 AM CEST Francisco Jerez wrote:
> --==-=-=
> Content-Type: multipart/mixed; boundary="=-=-="
> --=-=-=
> Content-Type: text/plain; charset=utf-8
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx> writes:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Allow intel_pstate to work in the passive mode with HWP enabled and
> > make it set the HWP minimum performance limit (HWP floor) to the
> > P-state value given by the target frequency supplied by the cpufreq
> > governor, so as to prevent the HWP algorithm and the CPU scheduler
> > from working against each other, at least when the schedutil governor
> > is in use, and update the intel_pstate documentation accordingly.
> >
> > Among other things, this allows utilization clamps to be taken
> > into account, at least to a certain extent, when intel_pstate is
> > in use and makes it more likely that sufficient capacity for
> > deadline tasks will be provided.
> >
> > After this change, the resulting behavior of an HWP system with
> > intel_pstate in the passive mode should be close to the behavior
> > of the analogous non-HWP system with intel_pstate in the passive
> > mode, except that in the frequency range below the base frequency
> > (ie. the frequency retured by the base_frequency cpufreq attribute
> > in sysfs on HWP systems) the HWP algorithm is allowed to make the
> > CPU run at a frequency above the floor P-state set by intel_pstate,
> > with or without hardware coordination of P-states among CPUs in the
> > same package.
> >
> The "frequency range below the base frequency" part of the paragraph
> above seems somewhat misleading, since AFAICT the same thing will happen
> in the P-state range above the base frequency.

Fair enough. I rephrased the changelog when applying the patch.

> Another minor comment below, other than that LGTM:

And this one has been fixed too.

> Reviewed-by: Francisco Jerez <currojerez@xxxxxxxxxx>