2022-05-03 00:43:05

by Chukun Pan

[permalink] [raw]
Subject: [PATCH] arm64: dts: allwinner: h6: Enable CPU opp tables for OrangePi One Plus

Enable CPU opp tables for OrangePi One Plus.

This needs to change the CPU regulator max voltage to fit
the OPP table.

Also add the ramp-delay information to avoid any out of spec
running as the regulator is slower at reaching the voltage
requested compare to the PLL reaching the frequency.

There is no such information for AXP805 but similar PMIC (AXP813)
has a DVM (Dynamic Voltage scaling Management) ramp rate equal
to 2500uV/us.

Signed-off-by: Chukun Pan <[email protected]>
---
arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
index 92745128fcfe..d7b82ef6be55 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
@@ -5,6 +5,7 @@
/dts-v1/;

#include "sun50i-h6.dtsi"
+#include "sun50i-h6-cpu-opp.dtsi"

#include <dt-bindings/gpio/gpio.h>

@@ -64,6 +65,10 @@ reg_vcc5v: vcc5v {
};
};

+&cpu0 {
+ cpu-supply = <&reg_dcdca>;
+};
+
&de {
status = "okay";
};
@@ -208,7 +213,8 @@ reg_cldo3: cldo3 {
reg_dcdca: dcdca {
regulator-always-on;
regulator-min-microvolt = <810000>;
- regulator-max-microvolt = <1080000>;
+ regulator-max-microvolt = <1160000>;
+ regulator-ramp-delay = <2500>;
regulator-name = "vdd-cpu";
};

@@ -216,6 +222,7 @@ reg_dcdcc: dcdcc {
regulator-enable-ramp-delay = <32000>;
regulator-min-microvolt = <810000>;
regulator-max-microvolt = <1080000>;
+ regulator-ramp-delay = <2500>;
regulator-name = "vdd-gpu";
};

--
2.25.1


2022-05-03 02:24:29

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: allwinner: h6: Enable CPU opp tables for OrangePi One Plus

On 5/2/22 10:01 AM, Chukun Pan wrote:
> Enable CPU opp tables for OrangePi One Plus.
>
> This needs to change the CPU regulator max voltage to fit
> the OPP table.
>
> Also add the ramp-delay information to avoid any out of spec
> running as the regulator is slower at reaching the voltage
> requested compare to the PLL reaching the frequency.
>
> There is no such information for AXP805 but similar PMIC (AXP813)
> has a DVM (Dynamic Voltage scaling Management) ramp rate equal
> to 2500uV/us.

The AXP805 datasheet has this information in the description for REG 1A. DVM is
disabled by default, and when it is enabled, the default ramp rate is 10
mV/15.625 us == 640 uV/us.

Did you notice any instability without this delay?

> Signed-off-by: Chukun Pan <[email protected]>
> ---
> arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> index 92745128fcfe..d7b82ef6be55 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> @@ -5,6 +5,7 @@
> /dts-v1/;
>
> #include "sun50i-h6.dtsi"
> +#include "sun50i-h6-cpu-opp.dtsi"
>
> #include <dt-bindings/gpio/gpio.h>
>
> @@ -64,6 +65,10 @@ reg_vcc5v: vcc5v {
> };
> };
>
> +&cpu0 {
> + cpu-supply = <&reg_dcdca>;
> +};
> +
> &de {
> status = "okay";
> };
> @@ -208,7 +213,8 @@ reg_cldo3: cldo3 {
> reg_dcdca: dcdca {
> regulator-always-on;
> regulator-min-microvolt = <810000>;
> - regulator-max-microvolt = <1080000>;
> + regulator-max-microvolt = <1160000>;
> + regulator-ramp-delay = <2500>;
> regulator-name = "vdd-cpu";
> };
>
> @@ -216,6 +222,7 @@ reg_dcdcc: dcdcc {
> regulator-enable-ramp-delay = <32000>;
> regulator-min-microvolt = <810000>;
> regulator-max-microvolt = <1080000>;
> + regulator-ramp-delay = <2500>;

This change is not related to CPU frequency scaling, so it belongs in a separate
patch (if it is needed).

Regards,
Samuel

> regulator-name = "vdd-gpu";
> };
>
>

2022-05-09 02:22:03

by Chukun Pan

[permalink] [raw]
Subject: Re:Re: [PATCH] arm64: dts: allwinner: h6: Enable CPU opp tables for OrangePi One Plus

>The AXP805 datasheet has this information in the description for REG 1A.
>DVM is disabled by default, and when it is enabled, the default ramp rate
>is 10mV/15.625 us == 640 uV/us.
>
>Did you notice any instability without this delay?

Actually I write this based on the commit https://github.com/torvalds/linux/commit/ebae33c
("arm64: dts: allwinner: h6: Enable CPU opp tables for Orange Pi 3") and https://github.com/
torvalds/linux/commit/fe79ea5 ("arm64: dts: allwinner: h6: Enable CPU opp tables for Pine
H64"), so I think it's necessary to add this delay.

>> @@ -216,6 +222,7 @@ reg_dcdcc: dcdcc {
>> regulator-enable-ramp-delay = <32000>;
>> regulator-min-microvolt = <810000>;
>> regulator-max-microvolt = <1080000>;
>> + regulator-ramp-delay = <2500>;
>
>This change is not related to CPU frequency scaling, so it belongs in a separate
>patch (if it is needed).

The two commits mentioned above also add this delay to dcdcc regulator.
If there is a need for a separate patch, I will send these separately.

Thanks,
Chukun


2022-05-09 11:05:07

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: allwinner: h6: Enable CPU opp tables for OrangePi One Plus

Hi Chukun,

On 5/8/22 8:29 AM, Chukun Pan wrote:
>> The AXP805 datasheet has this information in the description for REG 1A.
>> DVM is disabled by default, and when it is enabled, the default ramp rate
>> is 10mV/15.625 us == 640 uV/us.
>>
>> Did you notice any instability without this delay?
>
> Actually I write this based on the commit https://github.com/torvalds/linux/commit/ebae33c
> ("arm64: dts: allwinner: h6: Enable CPU opp tables for Orange Pi 3") and https://github.com/
> torvalds/linux/commit/fe79ea5 ("arm64: dts: allwinner: h6: Enable CPU opp tables for Pine
> H64"), so I think it's necessary to add this delay.

Thanks for the context! I think the suggestion comes originally from here:
https://lore.kernel.org/lkml/[email protected]/

From my reading of that thread, there appear to have been no reliability issues
before adding this change. It was just based on the available information at the
time.

On the other hand, adding this property will cause the CPU to spin for up to
112us in _regulator_do_set_voltage() during each CPU frequency change. So this
adds a lot of latency, which I would like to avoid if possible.

Regards,
Samuel

>>> @@ -216,6 +222,7 @@ reg_dcdcc: dcdcc {
>>> regulator-enable-ramp-delay = <32000>;
>>> regulator-min-microvolt = <810000>;
>>> regulator-max-microvolt = <1080000>;
>>> + regulator-ramp-delay = <2500>;
>>
>> This change is not related to CPU frequency scaling, so it belongs in a separate
>> patch (if it is needed).
>
> The two commits mentioned above also add this delay to dcdcc regulator.
> If there is a need for a separate patch, I will send these separately.
>
> Thanks,
> Chukun
>
>