On Mon, Jun 12, 2023 at 08:39:19AM +0300, Dmitry Baryshkov wrote:
> APQ8064 has 4 speed bins, each of them having from 4 to 6 categorization
> kinds. Provide tables necessary to handle voltage scaling on this SoC.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 1017 +++++++++++++++++++++++++++
> 1 file changed, 1017 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 4ef13f3d702b..f35853b59544 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -49,6 +49,9 @@ CPU0: cpu@0 {
> clocks = <&kraitcc KRAIT_CPU_0>;
> clock-names = "cpu";
> clock-latency = <100000>;
> + vdd-mem-supply = <&pm8921_l24>;
> + vdd-dig-supply = <&pm8921_s3>;
> + vdd-core-supply = <&saw0_vreg>;
> interconnects = <&kraitcc MASTER_KRAIT_L2 &kraitcc SLAVE_KRAIT_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -66,6 +69,9 @@ CPU1: cpu@1 {
> clocks = <&kraitcc KRAIT_CPU_1>;
> clock-names = "cpu";
> clock-latency = <100000>;
> + vdd-mem-supply = <&pm8921_l24>;
> + vdd-dig-supply = <&pm8921_s3>;
> + vdd-core-supply = <&saw1_vreg>;
> interconnects = <&kraitcc MASTER_KRAIT_L2 &kraitcc SLAVE_KRAIT_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -83,6 +89,9 @@ CPU2: cpu@2 {
> clocks = <&kraitcc KRAIT_CPU_2>;
> clock-names = "cpu";
> clock-latency = <100000>;
> + vdd-mem-supply = <&pm8921_l24>;
> + vdd-dig-supply = <&pm8921_s3>;
> + vdd-core-supply = <&saw2_vreg>;
> interconnects = <&kraitcc MASTER_KRAIT_L2 &kraitcc SLAVE_KRAIT_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -100,6 +109,9 @@ CPU3: cpu@3 {
> clocks = <&kraitcc KRAIT_CPU_3>;
> clock-names = "cpu";
> clock-latency = <100000>;
> + vdd-mem-supply = <&pm8921_l24>;
> + vdd-dig-supply = <&pm8921_s3>;
> + vdd-core-supply = <&saw3_vreg>;
> interconnects = <&kraitcc MASTER_KRAIT_L2 &kraitcc SLAVE_KRAIT_L2>;
> operating-points-v2 = <&cpu_opp_table>;
> #cooling-cells = <2>;
> @@ -132,6 +144,81 @@ cpu_opp_table: opp-table-cpu {
> opp-384000000 {
> opp-hz = /bits/ 64 <384000000>;
> opp-peak-kBps = <384000>;
> + opp-microvolt-speed0-pvs0 = <1050000 1050000 1150000>,
> + <950000 950000 1150000>,
> + <950000 950000 975000>;
I think this won't result in the correct switch order without making
some changes to the OPP core. In _set_opp() the OPP core does
/* Scaling up? Configure required OPPs before frequency */
if (!scaling_down) {
_set_required_opps();
_set_opp_bw();
opp_table->config_regulators();
}
opp_table->config_clks();
/* Scaling down? Configure required OPPs after frequency */
if (scaling_down) {
opp_table->config_regulators();
_set_opp_bw();
_set_required_opps();
}
Since the "bandwidth" for the L2 cache is set before the regulators
there is a short window where the L2 clock is running at a high
frequency with too low voltage, which could potentially cause
instability. On downstream this seems to be done in the proper order [1].
I'm not sure if the order in the OPP core is on purpose. If not, you
could propose moving the config_regulators() first (for scaling up)
and last (for scaling down). This would resolve the problem.
The alternative that I've already argued for on IRC in #linux-msm a
couple of days ago would be to give the L2 cache (here: "interconnect")
an own OPP table where it can describe its voltage requirements,
independent from the CPU. That way the icc_set_bw() would be guaranteed
to apply the correct voltage before adjusting the L2 cache clock. It
looks like the "l2_level" voltages for vdd_dig and vdd_mem are not
speedbin/PVS-specific [2] so this would also significantly reduce the DT
size, since you wouldn't need to repeat the same vdd_dig/vdd_mem
voltages for all of them.
Thanks,
Stephan
[1]: https://git.codelinaro.org/clo/la/kernel/msm/-/blob/LA.AF.1.2.1-08410-8064.0/arch/arm/mach-msm/acpuclock-krait.c#L529-588
[2]: https://git.codelinaro.org/clo/la/kernel/msm/-/blob/LA.AF.1.2.1-08410-8064.0/arch/arm/mach-msm/acpuclock-8064.c#L118-135
