From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: scmi: hook up to cpu_* to get apm working
To: Tobias Heider <tobias.heider@stusta.de>
Cc: tech@openbsd.org, patrick@openbsd.org
Date: Fri, 23 May 2025 14:58:45 +0200

> 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).


> 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	23 May 2025 01:32:39 -0000
> @@ -484,6 +484,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 +561,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,
> @@ -647,6 +654,72 @@ 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, hz;
> +	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;
> +
> +			min = d->pd_levels[0].pl_ifreq;
> +			max = d->pd_levels[d->pd_nlevels - 1].pl_ifreq;
> +			hz = min + (level * (max - min)) / 100;
> +
> +			/* Translate target hz to level */
> +			for (j = 0; j < d->pd_nlevels - 1; j++)
> +				if (d->pd_levels[j + 1].pl_ifreq > hz)
> +					break;
> +
> +			scmi_perf_level_set(sc, i, j);
> +		}
> +	}
>  }
>  
>  void
>