| Message ID | 20230703085555.30285-4-quic_mkshah@quicinc.com |
|---|---|
| State | Accepted |
| Commit | 7925ca85e956191a6a522e0a31a877b98cb5096c |
| Headers | show |
| Series | Use PSCI OS initiated mode for sc7280 | expand |
Hi, On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > > Add power-domains for cpuidle states to use psci os-initiated idle states. > > Cc: devicetree@vger.kernel.org > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > 1 file changed, 73 insertions(+), 25 deletions(-) FWIW, I dug up an old sc7280-herobrine board to test some other change and found it no longer booted. :( I bisected it and this is the change that breaks it. Specifically, I can make mainline boot with: git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update domain-idle-states for cluster sleep git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add power-domains for cpuidle states (I get an ath11k crash after that, but that's easy to hack out since I don't need WiFi) I suppose the two options here are to either: 1. Track the problem down and figure out why the breaks boot and then fix it. I'm personally not going to track this down, but if someone wants me to test a patch I can do that. 2. Delete all the herobrine dts files. So far we've been keeping the herobrine dts files alive because I thought some graphics folks (Rob, Abhinav, Jessica, for instance) were still using it. ...but Rob says he hasn't booted his in a while. No idea if Abhinav and Jessica are using theirs. Any opinions? Is herobrine hardware support near and dear to anyone these days? -Doug
Hi Doug On 3/14/2024 4:20 PM, Doug Anderson wrote: > Hi, > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: >> >> Add power-domains for cpuidle states to use psci os-initiated idle states. >> >> Cc: devicetree@vger.kernel.org >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> >> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- >> 1 file changed, 73 insertions(+), 25 deletions(-) > > FWIW, I dug up an old sc7280-herobrine board to test some other change > and found it no longer booted. :( I bisected it and this is the change > that breaks it. Specifically, I can make mainline boot with: > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > domain-idle-states for cluster sleep > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > power-domains for cpuidle states > We noticed that some variants of sc7280 herobrine boards didnt boot but some did atleast till linux 6.8 rc-6. I have not tested linux 6.9 rc-1 yet. We did not narrow down which change broke some of the boards, I can go back and confirm if its this one next week. > (I get an ath11k crash after that, but that's easy to hack out since I > don't need WiFi) > hmm, wifi worked alright on 6.8 rc-6 for us. > I suppose the two options here are to either: > > 1. Track the problem down and figure out why the breaks boot and then > fix it. I'm personally not going to track this down, but if someone > wants me to test a patch I can do that. > Can Maulik help us do that? > 2. Delete all the herobrine dts files. > > So far we've been keeping the herobrine dts files alive because I > thought some graphics folks (Rob, Abhinav, Jessica, for instance) were > still using it. ...but Rob says he hasn't booted his in a while. No > idea if Abhinav and Jessica are using theirs. Any opinions? Is > herobrine hardware support near and dear to anyone these days? > Yes, so we have been using sc7280 herobrine devices even till the last cycle and quite a bit of feature development was actually done on that. It was the only device having eDP other than sc8280xp till x1e80100 landed last cycle. I do want to start using sc8280xp as well because from the experience we got, it has more visibility in terms of users. So that will address my eDP concern. But, the nice thing about chromebooks is we really like to use them for IGT development / CI debug as CrOS provides a nice environment to cros-deploy IGT. We can continue to use sc7180 for IGT development but if we want to debug issues with eDP + IGT, sc7280 is a really useful platform for that. sc8280xp or x1e80100 is not a CrOS supported device. So we will have to develop and test IGT directly on the device (which is a bit of a pain) unless someone has a better way of "cross-compilation" for IGT on non-CrOS images. > > -Doug
Hi, On Thu, Mar 14, 2024 at 4:39 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Hi Doug > > On 3/14/2024 4:20 PM, Doug Anderson wrote: > > Hi, > > > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > >> > >> Add power-domains for cpuidle states to use psci os-initiated idle states. > >> > >> Cc: devicetree@vger.kernel.org > >> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > >> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > >> --- > >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > >> 1 file changed, 73 insertions(+), 25 deletions(-) > > > > FWIW, I dug up an old sc7280-herobrine board to test some other change > > and found it no longer booted. :( I bisected it and this is the change > > that breaks it. Specifically, I can make mainline boot with: > > > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > > domain-idle-states for cluster sleep > > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > > power-domains for cpuidle states > > > > We noticed that some variants of sc7280 herobrine boards didnt boot but > some did atleast till linux 6.8 rc-6. I have not tested linux 6.9 rc-1 yet. Wow, really? This doesn't seem like it would be related to the variant. Maybe the firmware version? FWIW, the device I was having problems with was a "villager-rev2" with FW 15368.0.0. OK, so I just pulled out a `hoglin-rev5` with 15432.0.0 and v6.8-rc6 boots and WiFi comes up. However, when I move to full mainline (b0546776ad3f (HEAD, linux/master) Merge tag 'printk-for-6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux) I get the ath11k crash. OK, so I updated my villager to 15432.0.0 and things work even without reverting ${SUBJECT} patch. I guess that's the answer: this patch broke things with some old firmwares but with the newer firmware it's fixed. Hopefully that doesn't happen again since I don't think there will ever be a newer firmware than 15432.0.0. > > (I get an ath11k crash after that, but that's easy to hack out since I > > don't need WiFi) > > > > hmm, wifi worked alright on 6.8 rc-6 for us. I guess I'll leave it to you to track down / report as needed. > > I suppose the two options here are to either: > > > > 1. Track the problem down and figure out why the breaks boot and then > > fix it. I'm personally not going to track this down, but if someone > > wants me to test a patch I can do that. > > > > Can Maulik help us do that? OK, sounds like we don't need to, as long as everyone updates their firmware. This should be OK. > > 2. Delete all the herobrine dts files. > > > > So far we've been keeping the herobrine dts files alive because I > > thought some graphics folks (Rob, Abhinav, Jessica, for instance) were > > still using it. ...but Rob says he hasn't booted his in a while. No > > idea if Abhinav and Jessica are using theirs. Any opinions? Is > > herobrine hardware support near and dear to anyone these days? > > > > Yes, so we have been using sc7280 herobrine devices even till the last > cycle and quite a bit of feature development was actually done on that. > > It was the only device having eDP other than sc8280xp till x1e80100 > landed last cycle. OK, thanks for confirming that they're still useful to you. When I got the failures I feared that nobody was using them anymore. > I do want to start using sc8280xp as well because from the experience we > got, it has more visibility in terms of users. So that will address my > eDP concern. > > But, the nice thing about chromebooks is we really like to use them for > IGT development / CI debug as CrOS provides a nice environment to > cros-deploy IGT. > > We can continue to use sc7180 for IGT development but if we want to > debug issues with eDP + IGT, sc7280 is a really useful platform for that. > > sc8280xp or x1e80100 is not a CrOS supported device. So we will have to > develop and test IGT directly on the device (which is a bit of a pain) > unless someone has a better way of "cross-compilation" for IGT on > non-CrOS images. I'd have to let others comment on IGT. -Doug
On Thu, Mar 14, 2024 at 04:20:59PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > Add power-domains for cpuidle states to use psci os-initiated idle states. > > > > Cc: devicetree@vger.kernel.org > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > > --- > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > > 1 file changed, 73 insertions(+), 25 deletions(-) > > FWIW, I dug up an old sc7280-herobrine board to test some other change > and found it no longer booted. :( I bisected it and this is the change > that breaks it. Specifically, I can make mainline boot with: > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > domain-idle-states for cluster sleep > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > power-domains for cpuidle states > IIRC, this could be issue with psci firmware. There were some known issues which were discussed few years back but I am not aware of the details and if/how it is applicable here. Not sure if you are getting any logs during the boot, if you do have worth look at logs related to PSCI/OSI/Idle/...
Hi, On Fri, Mar 15, 2024 at 8:24 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Thu, Mar 14, 2024 at 04:20:59PM -0700, Doug Anderson wrote: > > Hi, > > > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > Add power-domains for cpuidle states to use psci os-initiated idle states. > > > > > > Cc: devicetree@vger.kernel.org > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > > > --- > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > > > 1 file changed, 73 insertions(+), 25 deletions(-) > > > > FWIW, I dug up an old sc7280-herobrine board to test some other change > > and found it no longer booted. :( I bisected it and this is the change > > that breaks it. Specifically, I can make mainline boot with: > > > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > > domain-idle-states for cluster sleep > > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > > power-domains for cpuidle states > > > > IIRC, this could be issue with psci firmware. There were some known > issues which were discussed few years back but I am not aware of the > details and if/how it is applicable here. > > Not sure if you are getting any logs during the boot, if you do have > worth look at logs related to PSCI/OSI/Idle/... Given that the new firmware fixes it I'm going to say it's not worth looking into any longer. -Doug
On Fri, Mar 15, 2024 at 10:12:12AM -0700, Doug Anderson wrote: > Hi, > > On Fri, Mar 15, 2024 at 8:24 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Thu, Mar 14, 2024 at 04:20:59PM -0700, Doug Anderson wrote: > > > Hi, > > > > > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > > > Add power-domains for cpuidle states to use psci os-initiated idle states. > > > > > > > > Cc: devicetree@vger.kernel.org > > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > > > > --- > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > > > > 1 file changed, 73 insertions(+), 25 deletions(-) > > > > > > FWIW, I dug up an old sc7280-herobrine board to test some other change > > > and found it no longer booted. :( I bisected it and this is the change > > > that breaks it. Specifically, I can make mainline boot with: > > > > > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > > > domain-idle-states for cluster sleep > > > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > > > power-domains for cpuidle states > > > > > > > IIRC, this could be issue with psci firmware. There were some known > > issues which were discussed few years back but I am not aware of the > > details and if/how it is applicable here. > > > > Not sure if you are getting any logs during the boot, if you do have > > worth look at logs related to PSCI/OSI/Idle/... > > Given that the new firmware fixes it I'm going to say it's not worth > looking into any longer. > But it would be good to know if that is the exact reason, see if it can be upgraded, or else we can disable them by default. The bootloader or something else can detect the f/w version and enable them so that the board with old f/w(if can't be upgraded) can still be used. Otherwise it is a regression IMO.
Hi, On Fri, Mar 15, 2024 at 10:26 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Fri, Mar 15, 2024 at 10:12:12AM -0700, Doug Anderson wrote: > > Hi, > > > > On Fri, Mar 15, 2024 at 8:24 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > On Thu, Mar 14, 2024 at 04:20:59PM -0700, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Mon, Jul 3, 2023 at 1:56 AM Maulik Shah <quic_mkshah@quicinc.com> wrote: > > > > > > > > > > Add power-domains for cpuidle states to use psci os-initiated idle states. > > > > > > > > > > Cc: devicetree@vger.kernel.org > > > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > > > > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com> > > > > > --- > > > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 98 +++++++++++++++++++++------- > > > > > 1 file changed, 73 insertions(+), 25 deletions(-) > > > > > > > > FWIW, I dug up an old sc7280-herobrine board to test some other change > > > > and found it no longer booted. :( I bisected it and this is the change > > > > that breaks it. Specifically, I can make mainline boot with: > > > > > > > > git revert --no-edit db5d137e81bc # arm64: dts: qcom: sc7280: Update > > > > domain-idle-states for cluster sleep > > > > git revert --no-edit 7925ca85e956 # arm64: dts: qcom: sc7280: Add > > > > power-domains for cpuidle states > > > > > > > > > > IIRC, this could be issue with psci firmware. There were some known > > > issues which were discussed few years back but I am not aware of the > > > details and if/how it is applicable here. > > > > > > Not sure if you are getting any logs during the boot, if you do have > > > worth look at logs related to PSCI/OSI/Idle/... > > > > Given that the new firmware fixes it I'm going to say it's not worth > > looking into any longer. > > > > But it would be good to know if that is the exact reason, see if it can > be upgraded, or else we can disable them by default. The bootloader or > something else can detect the f/w version and enable them so that the > board with old f/w(if can't be upgraded) can still be used. > > Otherwise it is a regression IMO. I think it only would really matter if the problematic firmware actually made it out into the real world. In this case the only people who run into this are developers at Google and Qualcomm who had early versions of hardware and had old firmware sitting around on them. I can count the number of folks affected on one hand, and that's even if one of my fingers gets cut off. All of those folks can just upgrade their firmware since there is no downside in doing so. -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index a0e8db8270e7..84208a6c673d 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -170,9 +170,8 @@ reg = <0x0 0x0>; clocks = <&cpufreq_hw 0>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD0>; + power-domain-names = "psci"; next-level-cache = <&L2_0>; operating-points-v2 = <&cpu0_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -198,9 +197,8 @@ reg = <0x0 0x100>; clocks = <&cpufreq_hw 0>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD1>; + power-domain-names = "psci"; next-level-cache = <&L2_100>; operating-points-v2 = <&cpu0_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -221,9 +219,8 @@ reg = <0x0 0x200>; clocks = <&cpufreq_hw 0>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD2>; + power-domain-names = "psci"; next-level-cache = <&L2_200>; operating-points-v2 = <&cpu0_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -244,9 +241,8 @@ reg = <0x0 0x300>; clocks = <&cpufreq_hw 0>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD3>; + power-domain-names = "psci"; next-level-cache = <&L2_300>; operating-points-v2 = <&cpu0_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -267,9 +263,8 @@ reg = <0x0 0x400>; clocks = <&cpufreq_hw 1>; enable-method = "psci"; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD4>; + power-domain-names = "psci"; next-level-cache = <&L2_400>; operating-points-v2 = <&cpu4_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -290,9 +285,8 @@ reg = <0x0 0x500>; clocks = <&cpufreq_hw 1>; enable-method = "psci"; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD5>; + power-domain-names = "psci"; next-level-cache = <&L2_500>; operating-points-v2 = <&cpu4_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -313,9 +307,8 @@ reg = <0x0 0x600>; clocks = <&cpufreq_hw 1>; enable-method = "psci"; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD6>; + power-domain-names = "psci"; next-level-cache = <&L2_600>; operating-points-v2 = <&cpu4_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -336,9 +329,8 @@ reg = <0x0 0x700>; clocks = <&cpufreq_hw 2>; enable-method = "psci"; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD7>; + power-domain-names = "psci"; next-level-cache = <&L2_700>; operating-points-v2 = <&cpu7_opp_table>; interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, @@ -431,9 +423,11 @@ min-residency-us = <5555>; local-timer-stop; }; + }; + domain-idle-states { CLUSTER_SLEEP_0: cluster-sleep-0 { - compatible = "arm,idle-state"; + compatible = "domain-idle-state"; idle-state-name = "cluster-power-down"; arm,psci-suspend-param = <0x40003444>; entry-latency-us = <3263>; @@ -811,6 +805,59 @@ psci { compatible = "arm,psci-1.0"; method = "smc"; + + CPU_PD0: cpu0 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD1: cpu1 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD2: cpu2 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD3: cpu3 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0 &LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD4: cpu4 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>; + }; + + CPU_PD5: cpu5 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>; + }; + + CPU_PD6: cpu6 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>; + }; + + CPU_PD7: cpu7 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0 &BIG_CPU_SLEEP_1>; + }; + + CLUSTER_PD: cpu-cluster0 { + #power-domain-cells = <0>; + domain-idle-states = <&CLUSTER_SLEEP_0>; + }; }; qspi_opp_table: opp-table-qspi { @@ -5291,6 +5338,7 @@ <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + power-domains = <&CLUSTER_PD>; apps_bcm_voter: bcm-voter { compatible = "qcom,bcm-voter";