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 <tobias.heider@stusta.de>
> > 
> > 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 <tobias.heider@stusta.de>
> > > > > 
> > > > > 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