mbox series

[00/10] Add APSS RSC to Cluster power domain

Message ID 1641749107-31979-1-git-send-email-quic_mkshah@quicinc.com
Headers show
Series Add APSS RSC to Cluster power domain | expand

Message

Maulik Shah (mkshah) Jan. 9, 2022, 5:24 p.m. UTC
This series patches 1 to 4 adds/corrects the cpuidle states/
apps_rsc TCS configuration to make it same as downstream kernel.

The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
that when cluster is going to power down the cluster pre off notification
will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.

The patches 8, 9 and 10 are to program the next wakeup in CONTROL_TCS.

[1], [2] was older way of programming CONTROL_TCS (exporting an API and
calling when last CPU was entering deeper low power mode). Now with patch
number 5,6 and 7 the apps RSC is added to to cluster power domain and hence
these patches are no longer needed with this series.

The series is tested on SM8250 with latest linux-next tag next-20220107.

[1] https://patchwork.kernel.org/project/linux-arm-msm/patch/20190218140210.14631-3-rplsssn@codeaurora.org/
[2] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=59613

Lina Iyer (1):
  soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

Maulik Shah (9):
  arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
  arm64: dts: qcom: sm8250: Add cpuidle states
  arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
  arm64: dts: qcom: sm8450: Update cpuidle states parameters
  dt-bindings: soc: qcom: Update devicetree binding document for
    rpmh-rsc
  arm64: dts: qcom: Add power-domains property for apps_rsc
  PM: domains: Store the closest hrtimer event of the domain CPUs
  soc: qcom: rpmh-rsc: Save base address of drv
  soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

 .../devicetree/bindings/soc/qcom/rpmh-rsc.txt      |   6 +
 arch/arm64/boot/dts/qcom/sm8150.dtsi               |   7 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi               | 106 ++++++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi               |   3 +-
 arch/arm64/boot/dts/qcom/sm8450.dtsi               |  29 ++---
 drivers/base/power/domain_governor.c               |   1 +
 drivers/soc/qcom/rpmh-internal.h                   |   9 +-
 drivers/soc/qcom/rpmh-rsc.c                        | 138 +++++++++++++++++++--
 drivers/soc/qcom/rpmh.c                            |   4 +-
 include/linux/pm_domain.h                          |   1 +
 10 files changed, 271 insertions(+), 33 deletions(-)

Comments

Ulf Hansson Jan. 14, 2022, 12:31 p.m. UTC | #1
On Sun, 9 Jan 2022 at 18:25, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> The change documents power-domains property for RSC device.
> This optional property points to corresponding PM domain node.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

BTW, it would be nice to get the binding converted to the new DT yaml
formal, perhaps something we can look at doing on top?

Kind regards
Uffe


> ---
>  Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> index 9b86d1e..85b9859 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> @@ -78,6 +78,11 @@ Properties:
>                     CONTROL_TCS
>         - Cell #2 (Number of TCS): <u32>
>
> +- power-domains:
> +       Usage: optional
> +       Value type: <prop-encoded-power-domains>
> +       Definition: Phandle pointing to the corresponding PM domain node.
> +
>  - label:
>         Usage: optional
>         Value type: <string>
> @@ -112,6 +117,7 @@ TCS-OFFSET: 0xD00
>                                   <SLEEP_TCS   3>,
>                                   <WAKE_TCS    3>,
>                                   <CONTROL_TCS 1>;
> +               power-domains = <&CLUSTER_PD>;
>         };
>
>  Example 2:
> --
> 2.7.4
>
Ulf Hansson Jan. 14, 2022, 12:33 p.m. UTC | #2
On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Add power-domains property which allows apps_rsc device to attach
> to cluster power domain on sm8150, sm8250, sm8350 and sm8450.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
>  4 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 7826564..83a44f5 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -3559,6 +3559,7 @@
>                                           <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>,
>                                           <CONTROL_TCS 1>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8150-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 077d0ab..ebb4a4e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4593,6 +4593,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>, <CONTROL_TCS 1>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8250-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 665f79f..2c5dc305 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1803,6 +1803,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>, <CONTROL_TCS 0>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8350-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 5e329f8..acd122a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -910,6 +910,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  3>, <SLEEP_TCS   2>,
>                                           <WAKE_TCS    2>, <CONTROL_TCS 0>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         apps_bcm_voter: bcm-voter {
>                                 compatible = "qcom,bcm-voter";
> --
> 2.7.4
>
Ulf Hansson Jan. 14, 2022, 12:35 p.m. UTC | #3
On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Add changes to save drv's base address for rsc. This is
> used to read drv's configuration such as solver mode is
> supported or to write into CONTROL_TCS registers.
>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/soc/qcom/rpmh-internal.h |  2 ++
>  drivers/soc/qcom/rpmh-rsc.c      | 18 ++++++++----------
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 32ac117..6770bbb 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -91,6 +91,7 @@ struct rpmh_ctrlr {
>   * Resource State Coordinator controller (RSC)
>   *
>   * @name:               Controller identifier.
> + * @base:               Start address of the DRV registers in this controller.
>   * @tcs_base:           Start address of the TCS registers in this controller.
>   * @id:                 Instance id in the controller (Direct Resource Voter).
>   * @num_tcs:            Number of TCSes in this DRV.
> @@ -115,6 +116,7 @@ struct rpmh_ctrlr {
>   */
>  struct rsc_drv {
>         const char *name;
> +       void __iomem *base;
>         void __iomem *tcs_base;
>         int id;
>         int num_tcs;
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 5875ad5..c2a7c6c 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -882,8 +882,7 @@ static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
>         return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
>  }
>
> -static int rpmh_probe_tcs_config(struct platform_device *pdev,
> -                                struct rsc_drv *drv, void __iomem *base)
> +static int rpmh_probe_tcs_config(struct platform_device *pdev, struct rsc_drv *drv)
>  {
>         struct tcs_type_config {
>                 u32 type;
> @@ -897,9 +896,9 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
>         ret = of_property_read_u32(dn, "qcom,tcs-offset", &offset);
>         if (ret)
>                 return ret;
> -       drv->tcs_base = base + offset;
> +       drv->tcs_base = drv->base + offset;
>
> -       config = readl_relaxed(base + DRV_PRNT_CHLD_CONFIG);
> +       config = readl_relaxed(drv->base + DRV_PRNT_CHLD_CONFIG);
>
>         max_tcs = config;
>         max_tcs &= DRV_NUM_TCS_MASK << (DRV_NUM_TCS_SHIFT * drv->id);
> @@ -961,7 +960,6 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>         char drv_id[10] = {0};
>         int ret, irq;
>         u32 solver_config;
> -       void __iomem *base;
>
>         /*
>          * Even though RPMh doesn't directly use cmd-db, all of its children
> @@ -988,11 +986,11 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>                 drv->name = dev_name(&pdev->dev);
>
>         snprintf(drv_id, ARRAY_SIZE(drv_id), "drv-%d", drv->id);
> -       base = devm_platform_ioremap_resource_byname(pdev, drv_id);
> -       if (IS_ERR(base))
> -               return PTR_ERR(base);
> +       drv->base = devm_platform_ioremap_resource_byname(pdev, drv_id);
> +       if (IS_ERR(drv->base))
> +               return PTR_ERR(drv->base);
>
> -       ret = rpmh_probe_tcs_config(pdev, drv, base);
> +       ret = rpmh_probe_tcs_config(pdev, drv);
>         if (ret)
>                 return ret;
>
> @@ -1015,7 +1013,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>          * 'HW solver' mode where they can be in autonomous mode executing low
>          * power mode to power down.
>          */
> -       solver_config = readl_relaxed(base + DRV_SOLVER_CONFIG);
> +       solver_config = readl_relaxed(drv->base + DRV_SOLVER_CONFIG);
>         solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
>         solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
>         if (!solver_config) {
> --
> 2.7.4
>
Rob Herring (Arm) Jan. 21, 2022, 11:06 p.m. UTC | #4
On Sun, 09 Jan 2022 22:55:02 +0530, Maulik Shah wrote:
> The change documents power-domains property for RSC device.
> This optional property points to corresponding PM domain node.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>
Amit Pundir March 29, 2022, 9:55 a.m. UTC | #5
On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>
> On Sun, 9 Jan 2022 22:54:57 +0530, Maulik Shah wrote:
> > This series patches 1 to 4 adds/corrects the cpuidle states/
> > apps_rsc TCS configuration to make it same as downstream kernel.
> >
> > The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
> > that when cluster is going to power down the cluster pre off notification
> > will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.
> >
> > [...]
>
> Applied, thanks!
>
> [01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
>         commit: 17ac8af678b6da6a8f1df7da8ebf2c5198741827
> [02/10] arm64: dts: qcom: sm8250: Add cpuidle states
>         commit: 32bc936d732171d48c9c8f96c4fa25ac3ed7e1c7

Hi, These patches landed upstream and I see a boot regression on RB5
with this sm8250 patch:

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

Doesn't regress everytime but It is fairly reproducible. RB5 fails to
boot on every alternate reboot or so. But the device boots everytime
if I revert this patch.

> [03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
>         commit: a131255e4ad1ef8d4873ecba21561ba272b2547a
> [04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters
>         commit: 6574702b0d394d2acc9ff808c4a79df8b9999173
>
> Best regards,
> --
> Bjorn Andersson <bjorn.andersson@linaro.org>
Amit Pundir April 25, 2022, 4:56 p.m. UTC | #6
On Tue, 29 Mar 2022 at 15:25, Amit Pundir <amit.pundir@linaro.org> wrote:
>
> On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> >
> > On Sun, 9 Jan 2022 22:54:57 +0530, Maulik Shah wrote:
> > > This series patches 1 to 4 adds/corrects the cpuidle states/
> > > apps_rsc TCS configuration to make it same as downstream kernel.
> > >
> > > The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
> > > that when cluster is going to power down the cluster pre off notification
> > > will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
> >         commit: 17ac8af678b6da6a8f1df7da8ebf2c5198741827
> > [02/10] arm64: dts: qcom: sm8250: Add cpuidle states
> >         commit: 32bc936d732171d48c9c8f96c4fa25ac3ed7e1c7
>
> Hi, These patches landed upstream and I see a boot regression on RB5
> with this sm8250 patch:
>
> [    T1] vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> [    T1] qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators:
> ldo11: devm_regulator_register() failed, ret=-110
> [    T1] qcom-rpmh-regulator: probe of
> 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
>
> Doesn't regress everytime but It is fairly reproducible. RB5 fails to
> boot on every alternate reboot or so. But the device boots everytime
> if I revert this patch.

Hi, I'm still waiting on a resolution/fix for this. I'm happy to test
or run any debug patches, if it would help narrow the issue down.
Fwiw, this is the defconfig I'm using
https://git.linaro.org/people/amit.pundir/linux.git/plain/arch/arm64/configs/rbX_aosp_defconfig?h=rb5-rpmh-regression

Regards,
Amit Pundir

>
> > [03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
> >         commit: a131255e4ad1ef8d4873ecba21561ba272b2547a
> > [04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters
> >         commit: 6574702b0d394d2acc9ff808c4a79df8b9999173
> >
> > Best regards,
> > --
> > Bjorn Andersson <bjorn.andersson@linaro.org>