From: Mark Kettenis Subject: Re: scmi: hook up to cpu_* to get apm working To: Tobias Heider Cc: tech@openbsd.org, patrick@openbsd.org Date: Fri, 30 May 2025 17:00:28 +0200 > Date: Sat, 24 May 2025 05:41:25 +0200 > From: Tobias Heider > > On Fri, May 23, 2025 at 02:58:45PM +0200, Mark Kettenis wrote: > > > Date: Fri, 23 May 2025 03:40:43 +0200 > > > From: Tobias Heider > > > > > > On Fri, May 23, 2025 at 02:20:31AM +0200, Tobias Heider wrote: > > > > On Fri, May 23, 2025 at 12:20:50AM +0200, Mark Kettenis wrote: > > > > > > Date: Thu, 22 May 2025 07:16:17 +0200 > > > > > > From: Tobias Heider > > > > > > > > > > > > On Snapdragon X Elite chips we have to use ARM SCMI to control cpu > > > > > > performance. Our driver now supports reading the current > > > > > > performance level, translating it to HZ and exporting it as a sensor > > > > > > as well as setting initial levels. > > > > > > > > > > > > I think it is time to hook everything up to cpu_setperf and > > > > > > cpu_clockspeed to make those controls available to apm and > > > > > > hw.setperf. Loosely inspired by aplcpu which deals with a similar > > > > > > problem on Apple machines. > > > > > > > > > > > > ok? test feedback? > > > > > > > > > > I think this should take pl_perf into account in mapping the perflevel > > > > > into a level. > > > > > > > > Makes sense yes. I took a bit of a shortcut there based on the behaviour > > > > observed on my machine. > > > > > > > > We are currently making a few assumptions about the levels: > > > > the first is is that higher index means higher frequency, another > > > > is that frequency grows linearly with the index. > > > > The latter would be fixed by the change you proposed. > > > > > > > > > > How do you like this one instead? No functional difference to the > > > previous one but it does respect ifreq. > > > > > > I went with pl_ifreq instead of pl_perf because that seems to be the > > > more reliable unit. According to the standard, perf is an abstract > > > performance level value while ifreq is defined as the clock frequency > > > in kHz. In practice they are the same on X Elite chips. > > > > Well, pl_ifreq is optional. Firmware can set it to 0. So you have to > > use pl_perf. > > > > I'd argue that pl_perf is better anyway, since out hw.setperf also is > > a "abstract performance level". It just has a different scaling. To > > determine the scaling you simply need to figure out the min and max > > pl_perf for the domain. See the arm64 acpicpu(4) diff I mailed out > > recently. > > > > (The ACPI mechanism was clearly designed to expose this SCMI > > performance control mechanism to an ACPI aware OS). > > > > Right, makes sense. > > I can just collect the min and max while I am building the list of > per-domain perf levels. How do you like this one? ok kettenis@ > Index: scmi.c > =================================================================== > RCS file: /cvs/src/sys/dev/fdt/scmi.c,v > retrieving revision 1.3 > diff -u -p -r1.3 scmi.c > --- scmi.c 22 May 2025 03:04:01 -0000 1.3 > +++ scmi.c 24 May 2025 03:33:07 -0000 > @@ -104,6 +104,8 @@ struct scmi_perf_level { > > struct scmi_perf_domain { > size_t pd_nlevels; > + uint32_t pd_perf_max; > + uint32_t pd_perf_min; > struct scmi_perf_level *pd_levels; > int pd_curlevel; > }; > @@ -160,6 +162,7 @@ struct cfdriver scmi_cd = { > void scmi_attach_proto(struct scmi_softc *, int); > void scmi_attach_clock(struct scmi_softc *, int); > void scmi_attach_perf(struct scmi_softc *, int); > +void scmi_attach_sensors(struct scmi_softc *, int); > > int32_t scmi_smc_command(struct scmi_softc *); > int32_t scmi_mbox_command(struct scmi_softc *); > @@ -484,6 +487,9 @@ int scmi_perf_level_get(struct scmi_soft > int scmi_perf_level_set(struct scmi_softc *, int, int); > void scmi_perf_refresh_sensor(void *); > > +int scmi_perf_cpuspeed(int *); > +void scmi_perf_cpusetperf(int); > + > void > scmi_attach_perf(struct scmi_softc *sc, int node) > { > @@ -558,6 +564,10 @@ scmi_attach_perf(struct scmi_softc *sc, > } > sensordev_install(&sc->sc_perf_sensordev); > sensor_task_register(sc, scmi_perf_refresh_sensor, 1); > + > + cpu_setperf = scmi_perf_cpusetperf; > + cpu_cpuspeed = scmi_perf_cpuspeed; > + > return; > err: > free(sc->sc_perf_fsensors, M_DEVBUF, > @@ -596,9 +606,17 @@ scmi_perf_descr_levels(struct scmi_softc > pd->pd_levels = malloc(pd->pd_nlevels * > sizeof(struct scmi_perf_level), > M_DEVBUF, M_ZERO | M_WAITOK); > + > + pd->pd_perf_min = UINT32_MAX; > + pd->pd_perf_max = 0; > } > > for (i = 0; i < pl->pl_nret; i++) { > + if (pl->pl_entry[i].pe_perf < pd->pd_perf_min) > + pd->pd_perf_min = pl->pl_entry[i].pe_perf; > + if (pl->pl_entry[i].pe_perf > pd->pd_perf_max) > + pd->pd_perf_max = pl->pl_entry[i].pe_perf; > + > pd->pd_levels[idx + i].pl_cost = > pl->pl_entry[i].pe_cost; > pd->pd_levels[idx + i].pl_perf = > @@ -647,6 +665,77 @@ scmi_perf_level_set(struct scmi_softc *s > return -1; > } > return 0; > +} > + > +int > +scmi_perf_cpuspeed(int *freq) > +{ > + struct scmi_softc *sc; > + int i, level = -1; > + uint64_t opp_hz = 0; > + > + for (i = 0; i < scmi_cd.cd_ndevs; i++) { > + sc = scmi_cd.cd_devs[i]; > + if (sc == NULL) > + continue; > + > + if (sc->sc_perf_domains == NULL) > + continue; > + > + for (i = 0; i < sc->sc_perf_ndomains; i++) { > + if (sc->sc_perf_domains[i].pd_levels == NULL) > + return EINVAL; > + > + level = scmi_perf_level_get(sc, i); > + opp_hz = MAX(opp_hz, (uint64_t)sc->sc_perf_domains[i]. > + pd_levels[level].pl_ifreq * 1000); > + } > + } > + > + if (opp_hz == 0) > + return EINVAL; > + > + *freq = opp_hz / 1000000; > + return 0; > +} > + > +void > +scmi_perf_cpusetperf(int level) > +{ > + struct scmi_softc *sc; > + struct scmi_perf_domain *d; > + uint64_t min, max, perf; > + int i, j; > + > + for (i = 0; i < scmi_cd.cd_ndevs; i++) { > + sc = scmi_cd.cd_devs[i]; > + if (sc == NULL) > + return; > + if (sc->sc_perf_domains == NULL) > + continue; > + > + for (i = 0; i < sc->sc_perf_ndomains; i++) { > + d = &sc->sc_perf_domains[i]; > + if (d->pd_nlevels == 0) > + continue; > + > + /* > + * Map the performance level onto SCMI perf scale > + */ > + min = d->pd_perf_min; > + max = d->pd_perf_max; > + perf = min + (level * (max - min)) / 100; > + > + /* > + * Find best matching level > + */ > + for (j = 0; j < d->pd_nlevels - 1; j++) > + if (d->pd_levels[j + 1].pl_perf > perf) > + break; > + > + scmi_perf_level_set(sc, i, j); > + } > + } > } > > void >