> Date: Sat, 24 May 2025 05:41:25 +0200
> From: Tobias Heider <tobias.heider@stusta.de>
> 
> 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?

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
>