Re: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

[Christian Marangi]

On Tue, Oct 10, 2023 at 11:17:34PM +0200, Konrad Dybcio wrote:
> 
> 
> On 10/10/23 22:00, Christian Marangi wrote:
> > On Tue, Oct 10, 2023 at 09:52:50PM +0200, Konrad Dybcio wrote:
> > > 
> > > 
> > > On 10/10/23 16:15, Christian Marangi wrote:
> > > > On Tue, Oct 10, 2023 at 03:40:32PM +0200, Konrad Dybcio wrote:
> > > > > 
> > > > > 
> > > > > On 9/30/23 12:21, Robert Marko wrote:
> > > > > > From: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > > > > > 
> > > > > > Add CPU OPP table for IPQ8062, IPQ8064 and IPQ8065 SoC.
> > > > > > Use opp-supported-hw binding to correctly enable and disable the
> > > > > > frequency as IPQ8062 supports up to 1.0Ghz, IPQ8064 supports up to
> > > > > > 1.4GHz with 1.2GHz as an additional frequency and IPQ8065 supports
> > > > > > 1.7GHZ but doesn't have 1.2GHZ frequency and has to be disabled.
> > > > > > 
> > > > > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > > > > > Signed-off-by: Robert Marko <robimarko@xxxxxxxxx>
> > > > > > ---
> > > > > Christian/Robert, can you provide a downstream source for this?
> > > > > 
> > > > 
> > > > Sure, consider that everything is with +/-5%.
> > > Hm, so you're e.g. putting ipq8062 384MHz voltage for PVS3 equal to
> > > 0.95*800000 = 760000, but I'm not sure if it's a good idea?
> > > 
> > > The comment in downstream:
> > > 
> > > "These are based on +/-5% Margin on the VDD_APCx that is advertised in our
> > > Datasheet across Temperature"
> > > 
> > > suggests this is already not very accurate, and betting that the lower
> > > threshold works on all chips is probably not the best idea.
> > > 
> > 
> > Consider that everything is driven by the rpm. The original qsdk used the
> > same approach of taking the value, apply +-5% and pass it as a voltage
> > triplet to the rpm regulator. Also the driver have ranges so it
> > autodecide the best voltage in the range of the voltage triplet based on
> > the one supported by the regulator. Normally the normal voltage is
> > always used.
> Eeh? So you pass any half-random value to it and RPM edits it in flight?
> 
> Please be more specific, I'm not very familiar with this platform
> 

Sorry, probably I was a bit confusing.

ipq806x mount on 99% of the devices (this is the suggested design by
qcom) smb208 regulator. These have selector and step since not every
voltage is supported. So the closest one is selected in the range of the
provided min and max.

Most of the time the normal voltage is correctly used by sometimes an
higher one is used. The ranges are described here [1].

Consider that in later version of the qsdk where the moved to DT
definition, they started using OPP v1 where voltage-tollerance binding
is used. The voltage-tollerance was set to 5.

You can find how this value was used for OPPv1 here [2].

As you can see they internally calculate the min and max value and set
them. OPP v2 dropped this and make the dev directly provide min and max.

Effectively we pass the same voltage values.

For the voltage and how it's set, we use cpufreq-dt where if an attached
regulator is found, the set_voltage_triplet is used by providing normal
min and max value and then internally the best value is selected. This
same implementation was used in the qsdk source when they moved to dt
implementation.

Hope it's more clear now how the voltages are set on this platform.
For the RPM part, the voltage is requested but there isn't a direct
control of the system on the regulator since everything is handled by
RPM so there is also that extra step.

[1] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/qcom_rpm-regulator.c#L178
[2] https://elixir.bootlin.com/linux/latest/source/drivers/opp/core.c#L1956
-- 
	Ansuel