Re: [PATCH] arch: arm64: dts: apq8016-dbc: Add missing cpu opps

[Amit Kucheria <amit.kucheria@verdurent.com>]

Hi Loic,

On Wed, Apr 1, 2020 at 11:17 PM Loic Poulain <loic.poulain@linaro.org> wrote:
>
> The highest cpu frequency opps have been dropped because CPR is not
> supported. However, we can simply specify operating voltage so that
> they match the max corner voltages for each freq. With that, we can
> support up to 1.36Ghz. Ideally, msm8916 CPR should be implemented to
> fine tune operating voltages and optimize power consumption.
>
> This patch:
> - Adds missing opps and corresponding target voltages to msm8916.dtsi.
> - Adds cpu-supply to apq8016-sbc.dtsi (board level info).
> - Adds pm8916 spmi regulator node to pm8916.dtsi.
>
> Tested with a dragonboard-410c.

FWIW, I took this out for a spin with stress-ng loads. The first
output is w/o the patch.

--o amitrootfs o--(/sys/devices/system/cpu/cpufreq/policy0/stats) $ grep "" *
grep: reset: Permission denied
time_in_state:200000 14304
time_in_state:400000 2
time_in_state:800000 40
time_in_state:998400 4316
total_trans:31
trans_table:   From  :    To
trans_table:         :    200000    400000    800000    998400
trans_table:   200000:         0         1        12         1
trans_table:   400000:         2         0         0         0
trans_table:   800000:        12         0         0         1
trans_table:   998400:         0         1         1         0

The following output is with the patch applied. The cpu ran at 1.3GHz
for a short while until thermal kicked in and throttled the device to
800MHz-1GHz OPPs.

--o amitrootfs o--(/sys/devices/system/cpu/cpufreq/policy0/stats) $ grep "" *
grep: reset: Permission denied
time_in_state:200000 201824
time_in_state:400000 158
time_in_state:533330 2501
time_in_state:800000 37804
time_in_state:998400 23326
time_in_state:1094400 2325
time_in_state:1152000 747
time_in_state:1209600 922
time_in_state:1363200 10788
total_trans:1504
trans_table:   From  :    To
trans_table:         :    200000    400000    533330    800000
998400   1094400   1152000   1209600   1363200
trans_table:   200000:         0        18         5         8
99         0         1         0         2
trans_table:   400000:        18         0         0         0
3         0         0         0         0
trans_table:   533330:         7         1         0        77
1         0         0         0         0
trans_table:   800000:        13         1         1         0
547         0         0         0         1
trans_table:   998400:        92         1        80       477
0        12         0         0         2
trans_table:  1094400:         1         0         0         0
14         0         0         0         2
trans_table:  1152000:         1         0         0         0
1         4         0         0         1
trans_table:  1209600:         0         0         0         0
0         0         5         0         0
trans_table:  1363200:         1         0         0         0
0         1         1         5         0



> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

I haven't found a v2 of this patch incorporating Bjorn's suggestions
yet. Until then,

Tested-by: Amit Kucheria <amit.kucheria@linaro.org>



> ---
>  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 24 ++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/msm8916.dtsi     | 24 ++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/pm8916.dtsi      |  6 ++++++
>  3 files changed, 54 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> index 037e26b..f1c1216 100644
> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
> @@ -560,6 +560,30 @@
>         qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
>  };
>
> +&spm_regulators {
> +       vdd_cpu: s2 {
> +               regulator-always-on;
> +               regulator-min-microvolt = <1050000>;
> +               regulator-max-microvolt = <1350000>;
> +       };
> +};
> +
> +&CPU0 {
> +       cpu-supply = <&vdd_cpu>;
> +};
> +
> +&CPU1 {
> +       cpu-supply = <&vdd_cpu>;
> +};
> +
> +&CPU2 {
> +       cpu-supply = <&vdd_cpu>;
> +};
> +
> +&CPU3 {
> +       cpu-supply = <&vdd_cpu>;
> +};
> +
>  &smd_rpm_regulators {
>         vdd_l1_l2_l3-supply = <&pm8916_s3>;
>         vdd_l5-supply = <&pm8916_s3>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 9f31064..9805af0 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -342,15 +342,39 @@
>
>                 opp-200000000 {
>                         opp-hz = /bits/ 64 <200000000>;
> +                       opp-microvolt = <1050000>;
>                 };
>                 opp-400000000 {
>                         opp-hz = /bits/ 64 <400000000>;
> +                       opp-microvolt = <1050000>;
> +               };
> +               opp-533330000 {
> +                       opp-hz = /bits/ 64 <533330000>;
> +                       opp-microvolt = <1150000>;
>                 };
>                 opp-800000000 {
>                         opp-hz = /bits/ 64 <800000000>;
> +                       opp-microvolt = <1150000>;
>                 };
>                 opp-998400000 {
>                         opp-hz = /bits/ 64 <998400000>;
> +                       opp-microvolt = <1350000>;
> +               };
> +               opp-1094400000 {
> +                       opp-hz = /bits/ 64 <1094400000>;
> +                       opp-microvolt = <1350000>;
> +               };
> +               opp-1152000000 {
> +                       opp-hz = /bits/ 64 <1152000000>;
> +                       opp-microvolt = <1350000>;
> +               };
> +               opp-1209600000 {
> +                       opp-hz = /bits/ 64 <1209600000>;
> +                       opp-microvolt = <1350000>;
> +               };
> +               opp-1363200000 {
> +                       opp-hz = /bits/ 64 <1363200000>;
> +                       opp-microvolt = <1350000>;
>                 };
>         };
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8916.dtsi b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> index 0bcdf04..c9b9c4f 100644
> --- a/arch/arm64/boot/dts/qcom/pm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8916.dtsi
> @@ -157,5 +157,11 @@
>                         vdd-micbias-supply = <&pm8916_l13>;
>                         #sound-dai-cells = <1>;
>                 };
> +
> +               spm_regulators: spm_regulators  {
> +                       compatible = "qcom,pm8916-regulators";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +               };
>         };
>  };
> --
> 2.7.4
>