From: Tobias Heider Subject: Re: scmi: hook up to cpu_* to get apm working To: Mark Kettenis Cc: tech@openbsd.org, patrick@openbsd.org Date: Sat, 24 May 2025 05:41:25 +0200 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? 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