2022-10-27 12:11:55

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

To support the deeper cluster idle state for sm8250 platforms, some
additional synchronization is needed between the rpmh-rsc device and the
CPU cluster PM domain. Until that is supported, let's disable the cluster
idle state.

This fixes a problem that has been reported for the Qcom RB5 platform (see
below), but most likely other sm8250 platforms suffers from similar issues,
so let's make the fix generic for sm8250.

vreg_l11c_3p3: failed to enable: -ETIMEDOUT
qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110

Reported-by: Amit Pundir <[email protected]>
Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")
Signed-off-by: Ulf Hansson <[email protected]>
---

This problem was reported by Amit [1] together with an attempt to fix it. It
turned, that we wanted a more generic fix, hence I posted $subject patch.

Also note that, $subject patch is also depending (from functionality point of
view) on a another for genpd [2]. Although, that should soon reach v6.1-rc[n] and
stable kernels.

Bjorn, can you pick this up as a fix for v6.1-rc and tag it for stable kernels?

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/lkml/[email protected]/
[2]
https://lore.kernel.org/lkml/CAJZ5v0hJxRiL03XDFpU8FuabsHn5i6JMksJ=dfj8B+Dm=s3LYA@mail.gmail.com/

---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index a5b62cadb129..e276eed1f8e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -334,6 +334,7 @@ CLUSTER_SLEEP_0: cluster-sleep-0 {
exit-latency-us = <6562>;
min-residency-us = <9987>;
local-timer-stop;
+ status = "disabled";
};
};
};
--
2.34.1



2022-10-27 13:16:29

by Sudeep Holla

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

On Thu, Oct 27, 2022 at 01:57:45PM +0200, Ulf Hansson wrote:
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
>
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
>
> vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
> qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
>
> Reported-by: Amit Pundir <[email protected]>
> Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")

Thanks for the patience and fixing it this way :). I take it is only the
cluster states that has the issue because [1] only disables the CPU level
states which was confusing and I had asked the same.

Anyways for this,

Reviewed-by: Sudeep Holla <[email protected]>

--
Regards,
Sudeep

2022-10-27 15:55:29

by Amit Pundir

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

On Thu, 27 Oct 2022 at 17:27, Ulf Hansson <[email protected]> wrote:
>
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
>
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
>
> vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators: ldo11: devm_regulator_register() failed, ret=-110
> qcom-rpmh-regulator: probe of 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
>
> Reported-by: Amit Pundir <[email protected]>
> Fixes: 32bc936d7321 ("arm64: dts: qcom: sm8250: Add cpuidle states")
> Signed-off-by: Ulf Hansson <[email protected]>
> ---
>
> This problem was reported by Amit [1] together with an attempt to fix it. It
> turned, that we wanted a more generic fix, hence I posted $subject patch.

Thanks Ulf and Sudeep. This patch along with [2], fixes the above
mentioned crash on RB5 running AOSP.

Tested-by: Amit Pundir <[email protected]>

Regards,
Amit Pundir

>
> Also note that, $subject patch is also depending (from functionality point of
> view) on a another for genpd [2]. Although, that should soon reach v6.1-rc[n] and
> stable kernels.
>
> Bjorn, can you pick this up as a fix for v6.1-rc and tag it for stable kernels?
>
> Kind regards
> Ulf Hansson
>
> [1]
> https://lore.kernel.org/lkml/[email protected]/
> [2]
> https://lore.kernel.org/lkml/CAJZ5v0hJxRiL03XDFpU8FuabsHn5i6JMksJ=dfj8B+Dm=s3LYA@mail.gmail.com/
>
> ---
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index a5b62cadb129..e276eed1f8e2 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -334,6 +334,7 @@ CLUSTER_SLEEP_0: cluster-sleep-0 {
> exit-latency-us = <6562>;
> min-residency-us = <9987>;
> local-timer-stop;
> + status = "disabled";
> };
> };
> };
> --
> 2.34.1
>

2022-11-07 04:02:34

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> To support the deeper cluster idle state for sm8250 platforms, some
> additional synchronization is needed between the rpmh-rsc device and the
> CPU cluster PM domain. Until that is supported, let's disable the cluster
> idle state.
>
> This fixes a problem that has been reported for the Qcom RB5 platform (see
> below), but most likely other sm8250 platforms suffers from similar issues,
> so let's make the fix generic for sm8250.
>
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c

Best regards,
--
Bjorn Andersson <[email protected]>

2022-11-07 06:38:55

by Amit Pundir

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

On Mon, 7 Nov 2022 at 08:43, Bjorn Andersson <[email protected]> wrote:
>
> On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> > To support the deeper cluster idle state for sm8250 platforms, some
> > additional synchronization is needed between the rpmh-rsc device and the
> > CPU cluster PM domain. Until that is supported, let's disable the cluster
> > idle state.
> >
> > This fixes a problem that has been reported for the Qcom RB5 platform (see
> > below), but most likely other sm8250 platforms suffers from similar issues,
> > so let's make the fix generic for sm8250.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
> commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c

Hi Bjorn,

There seem to be some error while applying the patch "arm64: dts:
qcom: sm8250: Disable the not yet supported cluster idle state".
This patch is already applied in your arm64-fixes-for-6.1 tree
(commit: cadaa773bcf161184fa428180516bae33a7bc667)

The new commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c,
however, disables the Big cpu idle state instead. I'm not sure if that
is intentional though.

Regards,
Amit Pundir


>
> Best regards,
> --
> Bjorn Andersson <[email protected]>

2022-11-08 11:22:46

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state

On Mon, 7 Nov 2022 at 06:55, Amit Pundir <[email protected]> wrote:
>
> On Mon, 7 Nov 2022 at 08:43, Bjorn Andersson <[email protected]> wrote:
> >
> > On Thu, 27 Oct 2022 13:57:45 +0200, Ulf Hansson wrote:
> > > To support the deeper cluster idle state for sm8250 platforms, some
> > > additional synchronization is needed between the rpmh-rsc device and the
> > > CPU cluster PM domain. Until that is supported, let's disable the cluster
> > > idle state.
> > >
> > > This fixes a problem that has been reported for the Qcom RB5 platform (see
> > > below), but most likely other sm8250 platforms suffers from similar issues,
> > > so let's make the fix generic for sm8250.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] arm64: dts: qcom: sm8250: Disable the not yet supported cluster idle state
> > commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c
>
> Hi Bjorn,
>
> There seem to be some error while applying the patch "arm64: dts:
> qcom: sm8250: Disable the not yet supported cluster idle state".
> This patch is already applied in your arm64-fixes-for-6.1 tree
> (commit: cadaa773bcf161184fa428180516bae33a7bc667)
>
> The new commit: 5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c,
> however, disables the Big cpu idle state instead. I'm not sure if that
> is intentional though.

It's a mistake. There have been a lot of various patches/discussions
around this issue at LKML, not entirely easy to follow.

Anyway to make it clear, we shouldn't need to disable the
BIG_CPU_SLEEP_0 state, but only the CLUSTER_SLEEP_0.

Bjorn, can you please have a look and drop/revert commit
5c7fa5de12a31c1425cc87ba2ef27e6ae0a1788c ?

Kind regards
Uffe