2023-09-30 14:01:59

by Robert Marko

[permalink] [raw]
Subject: [PATCH v5 4/4] ARM: dts: qcom: ipq8064: Add CPU OPP table

From: Christian Marangi <[email protected]>

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 <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
---
Changes v4:
* Add OPP DTS patch for IPQ8064

arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
3 files changed, 162 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
index 5d3ebd3e2e51..72d9782c3d6f 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
@@ -6,3 +6,33 @@ / {
model = "Qualcomm Technologies, Inc. IPQ8062";
compatible = "qcom,ipq8062", "qcom,ipq8064";
};
+
+&opp_table_cpu {
+ opp-384000000 {
+ opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+ opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+ opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+ };
+
+ opp-600000000 {
+ opp-microvolt-speed0-pvs0 = <1050000 997500 1102500>;
+ opp-microvolt-speed0-pvs1 = <975000 926250 1023750>;
+ opp-microvolt-speed0-pvs2 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs3 = <850000 807500 892500>;
+ };
+
+ opp-800000000 {
+ opp-microvolt-speed0-pvs0 = <1100000 1045000 1155000>;
+ opp-microvolt-speed0-pvs1 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs2 = <995000 945250 1044750>;
+ opp-microvolt-speed0-pvs3 = <900000 855000 945000>;
+ };
+
+ opp-1000000000 {
+ opp-microvolt-speed0-pvs0 = <1150000 1092500 1207500>;
+ opp-microvolt-speed0-pvs1 = <1075000 1021250 1128750>;
+ opp-microvolt-speed0-pvs2 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs3 = <950000 902500 997500>;
+ };
+};
diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
index 6198f42f6a9c..cbbd28b43dc4 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi
@@ -30,6 +30,7 @@ cpu0: cpu@0 {
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ operating-points-v2 = <&opp_table_cpu>;
};

cpu1: cpu@1 {
@@ -40,6 +41,7 @@ cpu1: cpu@1 {
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ operating-points-v2 = <&opp_table_cpu>;
};

L2: l2-cache {
@@ -49,6 +51,71 @@ L2: l2-cache {
};
};

+ opp_table_cpu: opp-table-cpu {
+ compatible = "operating-points-v2-kryo-cpu";
+ nvmem-cells = <&speedbin_efuse>;
+
+ opp-384000000 {
+ opp-hz = /bits/ 64 <384000000>;
+ opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+ opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
+ opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <100000>;
+ };
+
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ opp-microvolt-speed0-pvs0 = <1050000 997500 1102500>;
+ opp-microvolt-speed0-pvs1 = <975000 926250 1023750>;
+ opp-microvolt-speed0-pvs2 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs3 = <850000 807500 892500>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <100000>;
+ };
+
+ opp-800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt-speed0-pvs0 = <1100000 1045000 1155000>;
+ opp-microvolt-speed0-pvs1 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs2 = <995000 945250 1044750>;
+ opp-microvolt-speed0-pvs3 = <900000 855000 945000>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <100000>;
+ };
+
+ opp-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt-speed0-pvs0 = <1150000 1092500 1207500>;
+ opp-microvolt-speed0-pvs1 = <1075000 1021250 1128750>;
+ opp-microvolt-speed0-pvs2 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs3 = <950000 902500 997500>;
+ opp-supported-hw = <0x7>;
+ clock-latency-ns = <100000>;
+ };
+
+ opp-1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt-speed0-pvs0 = <1200000 1140000 1260000>;
+ opp-microvolt-speed0-pvs1 = <1125000 1068750 1181250>;
+ opp-microvolt-speed0-pvs2 = <1075000 1021250 1128750>;
+ opp-microvolt-speed0-pvs3 = <1000000 950000 1050000>;
+ opp-supported-hw = <0x2>;
+ clock-latency-ns = <100000>;
+ };
+
+ opp-1400000000 {
+ opp-hz = /bits/ 64 <1400000000>;
+ opp-microvolt-speed0-pvs0 = <1250000 1187500 1312500>;
+ opp-microvolt-speed0-pvs1 = <1175000 1116250 1233750>;
+ opp-microvolt-speed0-pvs2 = <1125000 1068750 1181250>;
+ opp-microvolt-speed0-pvs3 = <1050000 997500 1102500>;
+ opp-supported-hw = <0x6>;
+ clock-latency-ns = <100000>;
+ };
+ };
+
thermal-zones {
sensor0-thermal {
polling-delay-passive = <0>;
diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi
index ea49f6cc416d..d9ead31b897b 100644
--- a/arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi
@@ -6,3 +6,68 @@ / {
model = "Qualcomm Technologies, Inc. IPQ8065";
compatible = "qcom,ipq8065", "qcom,ipq8064";
};
+
+&opp_table_cpu {
+ opp-384000000 {
+ opp-microvolt-speed0-pvs0 = <975000 926250 1023750>;
+ opp-microvolt-speed0-pvs1 = <950000 902500 997500>;
+ opp-microvolt-speed0-pvs2 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs3 = <900000 855000 945000>;
+ opp-microvolt-speed0-pvs4 = <875000 831250 918750>;
+ opp-microvolt-speed0-pvs5 = <825000 783750 866250>;
+ opp-microvolt-speed0-pvs6 = <775000 736250 813750>;
+ };
+
+ opp-600000000 {
+ opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
+ opp-microvolt-speed0-pvs1 = <975000 926250 1023750>;
+ opp-microvolt-speed0-pvs2 = <950000 902500 997500>;
+ opp-microvolt-speed0-pvs3 = <925000 878750 971250>;
+ opp-microvolt-speed0-pvs4 = <900000 855000 945000>;
+ opp-microvolt-speed0-pvs5 = <850000 807500 892500>;
+ opp-microvolt-speed0-pvs6 = <800000 760000 840000>;
+ };
+
+ opp-800000000 {
+ opp-microvolt-speed0-pvs0 = <1050000 997500 1102500>;
+ opp-microvolt-speed0-pvs1 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs2 = <1000000 950000 1050000>;
+ opp-microvolt-speed0-pvs3 = <975000 926250 1023750>;
+ opp-microvolt-speed0-pvs4 = <950000 902500 997500>;
+ opp-microvolt-speed0-pvs5 = <900000 855000 945000>;
+ opp-microvolt-speed0-pvs6 = <850000 807500 892500>;
+ };
+
+ opp-1000000000 {
+ opp-microvolt-speed0-pvs0 = <1100000 1045000 1155000>;
+ opp-microvolt-speed0-pvs1 = <1075000 1021250 1128750>;
+ opp-microvolt-speed0-pvs2 = <1050000 997500 1102500>;
+ opp-microvolt-speed0-pvs3 = <1025000 973750 1076250>;
+ opp-microvolt-speed0-pvs4 = <1000000 950000 1050000>;
+ opp-microvolt-speed0-pvs5 = <950000 902500 997500>;
+ opp-microvolt-speed0-pvs6 = <900000 855000 945000>;
+ };
+
+ opp-1400000000 {
+ opp-microvolt-speed4-pvs0 = <1175000 1116250 1233750>;
+ opp-microvolt-speed4-pvs1 = <1150000 1092500 1207500>;
+ opp-microvolt-speed4-pvs2 = <1125000 1068750 1181250>;
+ opp-microvolt-speed4-pvs3 = <1100000 1045000 1155000>;
+ opp-microvolt-speed4-pvs4 = <1075000 1021250 1128750>;
+ opp-microvolt-speed4-pvs5 = <1025000 973750 1076250>;
+ opp-microvolt-speed4-pvs6 = <975000 926250 1023750>;
+ };
+
+ opp-1725000000 {
+ opp-hz = /bits/ 64 <1725000000>;
+ opp-microvolt-speed0-pvs0 = <1262500 1199375 1325625>;
+ opp-microvolt-speed0-pvs1 = <1225000 1163750 1286250>;
+ opp-microvolt-speed0-pvs2 = <1200000 1140000 1260000>;
+ opp-microvolt-speed0-pvs3 = <1175000 1116250 1233750>;
+ opp-microvolt-speed0-pvs4 = <1150000 1092500 1207500>;
+ opp-microvolt-speed0-pvs5 = <1100000 1045000 1155000>;
+ opp-microvolt-speed0-pvs6 = <1050000 997500 1102500>;
+ opp-supported-hw = <0x4>;
+ clock-latency-ns = <100000>;
+ };
+};
--
2.41.0


2023-10-10 13:44:54

by Konrad Dybcio

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



On 9/30/23 12:21, Robert Marko wrote:
> From: Christian Marangi <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Robert Marko <[email protected]>
> ---
Christian/Robert, can you provide a downstream source for this?

Konrad

2023-10-10 14:16:22

by Christian Marangi

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

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 <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Robert Marko <[email protected]>
> > ---
> Christian/Robert, can you provide a downstream source for this?
>

Sure, consider that everything is with +/-5%.

Qsdk ships with these values but I will link a more secure source that
is from a very old qsdk source where march-msm was still used instead of
dt.

Here the source [1]. Confirmed by internal verification and also other
qsdk. At first view you might be scared by confusion but...

- _lite = ipq8062
- nothing = ipq8064
- tn_3 = ipq8065

and the psv are both in number and slow, nominal, fast and faster and
they all reflect efuse values.

There is on the left frequency and on the right voltage.

[1] https://github.com/Getnear/R7800/blob/master/git_home/linux.git/sourcecode/arch/arm/mach-msm/acpuclock-ipq806x.c

--
Ansuel

2023-10-10 19:53:10

by Konrad Dybcio

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



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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Robert Marko <[email protected]>
>>> ---
>> 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.

Konrad

2023-10-10 19:55:47

by Konrad Dybcio

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



On 9/30/23 12:21, Robert Marko wrote:
> From: Christian Marangi <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Robert Marko <[email protected]>
> ---
> Changes v4:
> * Add OPP DTS patch for IPQ8064
>
> arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
> arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
> arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
> 3 files changed, 162 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> index 5d3ebd3e2e51..72d9782c3d6f 100644
> --- a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> @@ -6,3 +6,33 @@ / {
> model = "Qualcomm Technologies, Inc. IPQ8062";
> compatible = "qcom,ipq8062", "qcom,ipq8064";
> };
> +
> +&opp_table_cpu {
> + opp-384000000 {
> + opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
> + opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
> + opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
> + opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
We can just make use of opp-supported-hw and define opp-384...-0,
opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw

otherwise it's somewhat confusing to follow, I think..

Konrad

2023-10-10 20:01:00

by Christian Marangi

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

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 <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > Signed-off-by: Robert Marko <[email protected]>
> > > > ---
> > > 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.

--
Ansuel

2023-10-10 20:08:11

by Christian Marangi

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

On Tue, Oct 10, 2023 at 09:55:26PM +0200, Konrad Dybcio wrote:
>
>
> On 9/30/23 12:21, Robert Marko wrote:
> > From: Christian Marangi <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Robert Marko <[email protected]>
> > ---
> > Changes v4:
> > * Add OPP DTS patch for IPQ8064
> >
> > arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
> > arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
> > arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
> > 3 files changed, 162 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> > index 5d3ebd3e2e51..72d9782c3d6f 100644
> > --- a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> > +++ b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
> > @@ -6,3 +6,33 @@ / {
> > model = "Qualcomm Technologies, Inc. IPQ8062";
> > compatible = "qcom,ipq8062", "qcom,ipq8064";
> > };
> > +
> > +&opp_table_cpu {
> > + opp-384000000 {
> > + opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
> > + opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
> > + opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
> > + opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
> We can just make use of opp-supported-hw and define opp-384...-0,
> opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw
>
> otherwise it's somewhat confusing to follow, I think..
>

Ehh should we really double the nodes for ipq8062 and ipq8065?
The idea here was to overwrite the one since the pvs always match and at
worst(ipq8065) have 7 pvs instead of 4. From the system it would be
easier to read since only one table is present in the final dts and not
2 and referring to the opp-supported-hw.

The original idea was to declare one opp table and reuse pvs version (by
faking it with hardcoded values) to put additional pvs for ipq8062 and
ipq8065, but Dmitry didn't like it and asked to move the opp in
different dtsi.

--
Ansuel

2023-10-10 21:18:13

by Konrad Dybcio

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



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 <[email protected]>
>>>>>
>>>>> 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 <[email protected]>
>>>>> Signed-off-by: Robert Marko <[email protected]>
>>>>> ---
>>>> 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

Konrad

2023-10-10 21:50:11

by Christian Marangi

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

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 <[email protected]>
> > > > > >
> > > > > > 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 <[email protected]>
> > > > > > Signed-off-by: Robert Marko <[email protected]>
> > > > > > ---
> > > > > 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

2023-10-12 16:47:08

by Konrad Dybcio

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



On 10/10/23 22:05, Christian Marangi wrote:
> On Tue, Oct 10, 2023 at 09:55:26PM +0200, Konrad Dybcio wrote:
>>
>>
>> On 9/30/23 12:21, Robert Marko wrote:
>>> From: Christian Marangi <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Robert Marko <[email protected]>
>>> ---
>>> Changes v4:
>>> * Add OPP DTS patch for IPQ8064
>>>
>>> arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi | 30 +++++++++++
>>> arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 67 ++++++++++++++++++++++++
>>> arch/arm/boot/dts/qcom/qcom-ipq8065.dtsi | 65 +++++++++++++++++++++++
>>> 3 files changed, 162 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
>>> index 5d3ebd3e2e51..72d9782c3d6f 100644
>>> --- a/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
>>> +++ b/arch/arm/boot/dts/qcom/qcom-ipq8062.dtsi
>>> @@ -6,3 +6,33 @@ / {
>>> model = "Qualcomm Technologies, Inc. IPQ8062";
>>> compatible = "qcom,ipq8062", "qcom,ipq8064";
>>> };
>>> +
>>> +&opp_table_cpu {
>>> + opp-384000000 {
>>> + opp-microvolt-speed0-pvs0 = <1000000 950000 1050000>;
>>> + opp-microvolt-speed0-pvs1 = <925000 878750 971250>;
>>> + opp-microvolt-speed0-pvs2 = <875000 831250 918750>;
>>> + opp-microvolt-speed0-pvs3 = <800000 760000 840000>;
>> We can just make use of opp-supported-hw and define opp-384...-0,
>> opp-384..-1 etc. with a valid corresponding bitmask in opp-supported-hw
>>
>> otherwise it's somewhat confusing to follow, I think..
>>
>
> Ehh should we really double the nodes for ipq8062 and ipq8065?
Hm.. I'm not 100% sure, both solutions are kinda meh, but perhaps
overwriting it will be less of a hassle for looking at the decompiled dt
indeed..

> The idea here was to overwrite the one since the pvs always match and at
> worst(ipq8065) have 7 pvs instead of 4. From the system it would be
> easier to read since only one table is present in the final dts and not
> 2 and referring to the opp-supported-hw.
>
> The original idea was to declare one opp table and reuse pvs version (by
> faking it with hardcoded values) to put additional pvs for ipq8062 and
> ipq8065, but Dmitry didn't like it and asked to move the opp in
> different dtsi.
Yeah this fusing thing is complex enough already

KOnrad