On Sun, 2 Apr 2023 at 01:28, Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote: > > On 01/04/2023 21:26, Bhupesh Sharma wrote: > > Hi Konrad, > > > > On Sat, 1 Apr 2023 at 17:51, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote: > >> > >> > >> > >> On 30.03.2023 21:33, Bhupesh Sharma wrote: > >>> Add CPU idle-state nodes and power-domains in Qualcomm sm6115 SoC dtsi. > >>> > >>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx> > >>> --- > >>> Changes since v1: > >>> - v1 can be viewed here: https://lore.kernel.org/lkml/e5cda4cf-5c2a-a7ed-9e1d-1fe9f2cbef40@xxxxxxxxxx > >>> - Addressed Konrad's comments on v1 and added GDHS and Power Collapse > >>> cluster power states. > >>> > >>> arch/arm64/boot/dts/qcom/sm6115.dtsi | 136 +++++++++++++++++++++++++++ > >>> 1 file changed, 136 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi > >>> index 2a51c938bbcb..b63395d476ed 100644 > >>> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi > >>> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi > >>> @@ -45,6 +45,8 @@ CPU0: cpu@0 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_0>; > >>> qcom,freq-domain = <&cpufreq_hw 0>; > >>> + power-domains = <&CPU_PD0>; > >>> + power-domain-names = "psci"; > >>> L2_0: l2-cache { > >>> compatible = "cache"; > >>> cache-level = <2>; > >>> @@ -61,6 +63,8 @@ CPU1: cpu@1 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_0>; > >>> qcom,freq-domain = <&cpufreq_hw 0>; > >>> + power-domains = <&CPU_PD1>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> CPU2: cpu@2 { > >>> @@ -73,6 +77,8 @@ CPU2: cpu@2 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_0>; > >>> qcom,freq-domain = <&cpufreq_hw 0>; > >>> + power-domains = <&CPU_PD2>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> CPU3: cpu@3 { > >>> @@ -85,6 +91,8 @@ CPU3: cpu@3 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_0>; > >>> qcom,freq-domain = <&cpufreq_hw 0>; > >>> + power-domains = <&CPU_PD3>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> CPU4: cpu@100 { > >>> @@ -97,6 +105,8 @@ CPU4: cpu@100 { > >>> dynamic-power-coefficient = <282>; > >>> next-level-cache = <&L2_1>; > >>> qcom,freq-domain = <&cpufreq_hw 1>; > >>> + power-domains = <&CPU_PD4>; > >>> + power-domain-names = "psci"; > >>> L2_1: l2-cache { > >>> compatible = "cache"; > >>> cache-level = <2>; > >>> @@ -113,6 +123,8 @@ CPU5: cpu@101 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_1>; > >>> qcom,freq-domain = <&cpufreq_hw 1>; > >>> + power-domains = <&CPU_PD5>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> CPU6: cpu@102 { > >>> @@ -125,6 +137,8 @@ CPU6: cpu@102 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_1>; > >>> qcom,freq-domain = <&cpufreq_hw 1>; > >>> + power-domains = <&CPU_PD6>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> CPU7: cpu@103 { > >>> @@ -137,6 +151,8 @@ CPU7: cpu@103 { > >>> enable-method = "psci"; > >>> next-level-cache = <&L2_1>; > >>> qcom,freq-domain = <&cpufreq_hw 1>; > >>> + power-domains = <&CPU_PD7>; > >>> + power-domain-names = "psci"; > >>> }; > >>> > >>> cpu-map { > >>> @@ -176,6 +192,68 @@ core3 { > >>> }; > >>> }; > >>> }; > >>> + > >>> + idle-states { > >>> + entry-method = "psci"; > >>> + > >>> + LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 { > >>> + compatible = "arm,idle-state"; > >>> + idle-state-name = "silver-rail-power-collapse"; > >>> + arm,psci-suspend-param = <0x40000003>; > >>> + entry-latency-us = <290>; > >>> + exit-latency-us = <376>; > >>> + min-residency-us = <1182>; > >>> + local-timer-stop; > >>> + }; > >>> + > >>> + BIG_CPU_SLEEP_0: cpu-sleep-1-0 { > >>> + compatible = "arm,idle-state"; > >>> + idle-state-name = "gold-rail-power-collapse"; > >>> + arm,psci-suspend-param = <0x40000003>; > >>> + entry-latency-us = <297>; > >>> + exit-latency-us = <324>; > >>> + min-residency-us = <1110>; > >>> + local-timer-stop; > >>> + }; > >>> + }; > >>> + > >>> + domain-idle-states { > >>> + CLUSTER_0_SLEEP_0: cluster-sleep-0-0 { > >>> + /* GDHS */ > >>> + compatible = "domain-idle-state"; > >>> + arm,psci-suspend-param = <0x40000022>; > >> This 0x22 ending seems very sus. > >> > >> The last nibble represents the core-level power state and the > >> penultimate one represents the same at cluster level. A value > >> of 2 in that cluster nibble is actually undefined by the PSCI spec, > >> whereas the value of 4 (as you have in all of the other idle > >> states, including D3G for the perf cluster) corresponds to > >> "Retention", so unless there's a very weird nuance in the > >> TZ for this SoC, it should probably end in 0x42. > >> > >> Otherwise I think this LGTM now! > > > > I am also learning by experiment about the exact values to use here, > > as the only ready reckoner of how these values are calculated, seems > > to be available via [1]. > > > > Also it seems the downstream code uses the following approach to > > calculate the LPM state suspend-param, which for example for > > CLUSTER_0_SLEEP_1 states turns out to be: > > > > state_id = get_cluster_id(cpu->parent, &affinity_level, from_idle); = 0x40 > > power_state = (is-reset << 30) = 0x40000000 > > affinity_level = (affinity level & 0x3) << 24 = 0x1000000 > > state_id += power_state + affinity_level + psci_id; > > > > = 0x40000000 + 0x1000000 + 0x40 + 0x4 = 0x41000044 > > > > For the D3G cases as well, I just used the 'qcom,psci-mode = <2>' > > value as provided in downstream code (see [2]), for the overall > > calculations. > > > > Also, the only usage of D3G state I could find upstream (in qcom dtsi > > files0 is for 'msm8916' (see [3]), which also uses the value with > > ending 0x2 -> 'arm,psci-suspend-param = <0x41000032>' > > D3G has min-child-idx = 1, so the end PSCI param should be 0x41000023 > D3 is 0x41000043 Ok, let me recheck at my end as well. Thanks Bhupesh
