On Mon, Jun 27, 2022 at 11:47:58PM +0200, Stefan Hagen wrote:
> Bryan Steele wrote (2022-06-27 23:12 CEST):
> > On Mon, Jun 27, 2022 at 11:01:31PM +0200, Stefan Hagen wrote:
> > > acpitz(4) implements passive cooling, which starts throttling the CPU to
> > > keep it under the temperature reported by the _PSV trip point.
> > >
> > > https://uefi.org/specs/ACPI/6.4/11_Thermal_Management/thermal-control.html
> > >
> > > The specs (1.1.5.1) leave the decision to activate passive cooling to
> > > the OS. It is a way to limit noise and heat rather than to protect the
> > > CPU (for which _HOT and _CRT are the better trip points).
> > >
> > > I would like to restrict passive cooling to the AUTO perfpolicy.
> > >
> > > For low (apm -L) and high (apm -H) it doesn't make much sense, because
> > > - low is setting a low pstate anyway
> > > - high is probably never set with the intention to have a cool and
> > > quiet machine.
> >
> > Shouldn't this also take into consideration hw.power as well? If it
> > doesn't make sense for perfpolicy=high then it probably doesn't for
> > perfpolicy=auto when on AC power?
>
> I would say yes.
no particular opinion for now regarding the logic, but please avoid magic
number when constant/define exists.
AUTO perfpolicy is PERFPOL_AUTO (which is 1).
Thanks.
> Index: sys/dev/acpi/acpitz.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/acpi/acpitz.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 acpitz.c
> --- sys/dev/acpi/acpitz.c 6 Apr 2022 18:59:27 -0000 1.58
> +++ sys/dev/acpi/acpitz.c 27 Jun 2022 21:23:06 -0000
> @@ -89,7 +89,9 @@ void acpitz_init(struct acpitz_softc *,
> void (*acpitz_cpu_setperf)(int);
> int acpitz_perflevel = -1;
> extern void (*cpu_setperf)(int);
> +extern int hw_power;
> extern int perflevel;
> +extern int perfpolicy;
> #define PERFSTEP 10
>
> #define ACPITZ_TRIPS (1L << 0)
> @@ -381,7 +383,7 @@ acpitz_refresh(void *arg)
> sc->sc_tc1, sc->sc_tc2, sc->sc_psv);
>
> nperf = acpitz_perflevel;
> - if (sc->sc_psv <= sc->sc_tmp) {
> + if (sc->sc_psv <= sc->sc_tmp && perfpolicy == 1 && hw_power ==
> 0) {
> /* Passive cooling enabled */
> dnprintf(1, "%s: enabling passive %d %d\n",
> DEVNAME(sc), sc->sc_tmp, sc->sc_psv);
>
--
Sebastien Marie
