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

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.

> 
> > Index: scmi.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/fdt/scmi.c,v
> > diff -u -p -r1.3 scmi.c
> > --- scmi.c	22 May 2025 03:04:01 -0000	1.3
> > +++ scmi.c	22 May 2025 03:37:38 -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,63 @@ 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;
> > +	size_t nlevels;
> > +	int i;
> > +
> > +        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;
> > +
> > +		/* Find number of levels per domain */
> > +		for (i = 0; i < sc->sc_perf_ndomains; i++) {
> > +			nlevels = sc->sc_perf_domains[i].pd_nlevels;
> > +			if (nlevels == 0)
> > +				continue;
> > +			scmi_perf_level_set(sc, i,
> > +			    (level * (nlevels - 1) / 100));
> > +		}
> > +	}
> >  }
> >  
> >  void
> > 
> >