2022-01-09 17:25:24

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 00/10] Add APSS RSC to Cluster power domain

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/[email protected]/
[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(-)

--
2.7.4



2022-01-09 17:25:31

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc

Correct the TCS config by updating the number of TCSes for each type.

Cc: [email protected]
Fixes: d8cf9372b654 ("arm64: dts: qcom: sm8150: Add apps shared nodes")
Signed-off-by: Maulik Shah <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8150.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 6012322..7826564 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -3556,9 +3556,9 @@
qcom,tcs-offset = <0xd00>;
qcom,drv-id = <2>;
qcom,tcs-config = <ACTIVE_TCS 2>,
- <SLEEP_TCS 1>,
- <WAKE_TCS 1>,
- <CONTROL_TCS 0>;
+ <SLEEP_TCS 3>,
+ <WAKE_TCS 3>,
+ <CONTROL_TCS 1>;

rpmhcc: clock-controller {
compatible = "qcom,sm8150-rpmh-clk";
--
2.7.4


2022-01-09 17:25:33

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 02/10] arm64: dts: qcom: sm8250: Add cpuidle states

This change adds various idle states and add devices to power domains.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 105 +++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 5617a46..077d0ab 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -98,6 +98,8 @@
capacity-dmips-mhz = <448>;
dynamic-power-coefficient = <205>;
next-level-cache = <&L2_0>;
+ power-domains = <&CPU_PD0>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 0>;
operating-points-v2 = <&cpu0_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -120,6 +122,8 @@
capacity-dmips-mhz = <448>;
dynamic-power-coefficient = <205>;
next-level-cache = <&L2_100>;
+ power-domains = <&CPU_PD1>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 0>;
operating-points-v2 = <&cpu0_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -139,6 +143,8 @@
capacity-dmips-mhz = <448>;
dynamic-power-coefficient = <205>;
next-level-cache = <&L2_200>;
+ power-domains = <&CPU_PD2>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 0>;
operating-points-v2 = <&cpu0_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -158,6 +164,8 @@
capacity-dmips-mhz = <448>;
dynamic-power-coefficient = <205>;
next-level-cache = <&L2_300>;
+ power-domains = <&CPU_PD3>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 0>;
operating-points-v2 = <&cpu0_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -177,6 +185,8 @@
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <379>;
next-level-cache = <&L2_400>;
+ power-domains = <&CPU_PD4>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 1>;
operating-points-v2 = <&cpu4_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -196,6 +206,8 @@
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <379>;
next-level-cache = <&L2_500>;
+ power-domains = <&CPU_PD5>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 1>;
operating-points-v2 = <&cpu4_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -216,6 +228,8 @@
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <379>;
next-level-cache = <&L2_600>;
+ power-domains = <&CPU_PD6>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 1>;
operating-points-v2 = <&cpu4_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -235,6 +249,8 @@
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <444>;
next-level-cache = <&L2_700>;
+ power-domains = <&CPU_PD7>;
+ power-domain-names = "psci";
qcom,freq-domain = <&cpufreq_hw 2>;
operating-points-v2 = <&cpu7_opp_table>;
interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -281,6 +297,42 @@
};
};
};
+
+ 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 = <0x40000004>;
+ entry-latency-us = <360>;
+ exit-latency-us = <531>;
+ min-residency-us = <3934>;
+ 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 = <0x40000004>;
+ entry-latency-us = <702>;
+ exit-latency-us = <1061>;
+ min-residency-us = <4488>;
+ local-timer-stop;
+ };
+ };
+
+ domain-idle-states {
+ CLUSTER_SLEEP_0: cluster-sleep-0 {
+ compatible = "domain-idle-state";
+ idle-state-name = "cluster-llcc-off";
+ arm,psci-suspend-param = <0x4100c244>;
+ entry-latency-us = <3264>;
+ exit-latency-us = <6562>;
+ min-residency-us = <9987>;
+ local-timer-stop;
+ };
+ };
};

cpu0_opp_table: cpu0_opp_table {
@@ -594,6 +646,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>;
+ };
+
+ CPU_PD1: cpu1 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ };
+
+ CPU_PD2: cpu2 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ };
+
+ CPU_PD3: cpu3 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+ };
+
+ CPU_PD4: cpu4 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ };
+
+ CPU_PD5: cpu5 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ };
+
+ CPU_PD6: cpu6 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ };
+
+ CPU_PD7: cpu7 {
+ #power-domain-cells = <0>;
+ power-domains = <&CLUSTER_PD>;
+ domain-idle-states = <&BIG_CPU_SLEEP_0>;
+ };
+
+ CLUSTER_PD: cpu-cluster0 {
+ #power-domain-cells = <0>;
+ domain-idle-states = <&CLUSTER_SLEEP_0>;
+ };
};

reserved-memory {
--
2.7.4


2022-01-09 17:25:40

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc

Correct the TCS config by updating the number of TCSes for each type.

Cc: [email protected]
Fixes: b7e8f433a673 ("arm64: dts: qcom: Add basic devicetree support for SM8350 SoC")
Signed-off-by: Maulik Shah <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 53b39e7..665f79f 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1802,7 +1802,7 @@
qcom,tcs-offset = <0xd00>;
qcom,drv-id = <2>;
qcom,tcs-config = <ACTIVE_TCS 2>, <SLEEP_TCS 3>,
- <WAKE_TCS 3>, <CONTROL_TCS 1>;
+ <WAKE_TCS 3>, <CONTROL_TCS 0>;

rpmhcc: clock-controller {
compatible = "qcom,sm8350-rpmh-clk";
--
2.7.4


2022-01-09 17:25:44

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

This change updates/corrects below cpuidle parameters

1. entry-latency, exit-latency and residency for various idle states.
2. arm,psci-suspend-param which is same for CLUSTER_SLEEP_0/1 states.
3. Add CLUSTER_SLEEP_1 in CLUSTER_PD.

Cc: [email protected]
Fixes: 5188049c9b36 ("arm64: dts: qcom: Add base SM8450 DTSI")
Signed-off-by: Maulik Shah <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 10c25ad..5e329f8 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -203,9 +203,9 @@
compatible = "arm,idle-state";
idle-state-name = "silver-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
- entry-latency-us = <274>;
- exit-latency-us = <480>;
- min-residency-us = <3934>;
+ entry-latency-us = <800>;
+ exit-latency-us = <750>;
+ min-residency-us = <4090>;
local-timer-stop;
};

@@ -213,9 +213,9 @@
compatible = "arm,idle-state";
idle-state-name = "gold-rail-power-collapse";
arm,psci-suspend-param = <0x40000004>;
- entry-latency-us = <327>;
- exit-latency-us = <1502>;
- min-residency-us = <4488>;
+ entry-latency-us = <600>;
+ exit-latency-us = <1550>;
+ min-residency-us = <4791>;
local-timer-stop;
};
};
@@ -224,10 +224,10 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "domain-idle-state";
idle-state-name = "cluster-l3-off";
- arm,psci-suspend-param = <0x4100c344>;
- entry-latency-us = <584>;
- exit-latency-us = <2332>;
- min-residency-us = <6118>;
+ arm,psci-suspend-param = <0x41000044>;
+ entry-latency-us = <1050>;
+ exit-latency-us = <2500>;
+ min-residency-us = <5309>;
local-timer-stop;
};

@@ -235,9 +235,9 @@
compatible = "domain-idle-state";
idle-state-name = "cluster-power-collapse";
arm,psci-suspend-param = <0x4100c344>;
- entry-latency-us = <2893>;
- exit-latency-us = <4023>;
- min-residency-us = <9987>;
+ entry-latency-us = <2700>;
+ exit-latency-us = <3500>;
+ min-residency-us = <13959>;
local-timer-stop;
};
};
@@ -315,7 +315,7 @@

CLUSTER_PD: cpu-cluster0 {
#power-domain-cells = <0>;
- domain-idle-states = <&CLUSTER_SLEEP_0>;
+ domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;
};
};

--
2.7.4


2022-01-09 17:25:50

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

The change documents power-domains property for RSC device.
This optional property points to corresponding PM domain node.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
---
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


2022-01-09 17:26:21

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 10/10] soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

The next wakeup timer value needs to be set in always on domain timer
as the arch timer interrupt can not wakeup the SoC if after the deepest
CPUidle states the SoC also enters deepest low power state.

To wakeup the SoC in such scenarios the earliest wakeup time is set in
CONTROL_TCS and the firmware takes care of setting up its own timer in
always on domain with next wakeup time. The timer wakes up the RSC and
sets resources back to wake state.

Signed-off-by: Maulik Shah <[email protected]>
---
drivers/soc/qcom/rpmh-internal.h | 1 +
drivers/soc/qcom/rpmh-rsc.c | 60 ++++++++++++++++++++++++++++++++++++++++
drivers/soc/qcom/rpmh.c | 4 ++-
3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 6770bbb..04789a37 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -135,6 +135,7 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg);
int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
const struct tcs_request *msg);
void rpmh_rsc_invalidate(struct rsc_drv *drv);
+void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv);

void rpmh_tx_done(const struct tcs_request *msg, int r);
int rpmh_flush(struct rpmh_ctrlr *ctrlr);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index c2a7c6c..b3b85f1 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -12,6 +12,7 @@
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
+#include <linux/ktime.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/notifier.h>
@@ -25,6 +26,7 @@
#include <linux/spinlock.h>
#include <linux/wait.h>

+#include <clocksource/arm_arch_timer.h>
#include <soc/qcom/cmd-db.h>
#include <soc/qcom/tcs.h>
#include <dt-bindings/soc/qcom,rpmh-rsc.h>
@@ -49,6 +51,14 @@
#define DRV_NCPT_MASK 0x1F
#define DRV_NCPT_SHIFT 27

+/* Offsets for CONTROL TCS Registers */
+#define RSC_DRV_CTL_TCS_DATA_HI 0x38
+#define RSC_DRV_CTL_TCS_DATA_HI_MASK 0xFFFFFF
+#define RSC_DRV_CTL_TCS_DATA_HI_VALID BIT(31)
+#define RSC_DRV_CTL_TCS_DATA_LO 0x40
+#define RSC_DRV_CTL_TCS_DATA_LO_MASK 0xFFFFFFFF
+#define RSC_DRV_CTL_TCS_DATA_SIZE 32
+
/* Offsets for common TCS Registers, one bit per TCS */
#define RSC_DRV_IRQ_ENABLE 0x00
#define RSC_DRV_IRQ_STATUS 0x04
@@ -142,6 +152,14 @@
* +---------------------------------------------------+
*/

+#define USECS_TO_CYCLES(time_usecs) \
+ xloops_to_cycles((time_usecs) * 0x10C7UL)
+
+static inline unsigned long xloops_to_cycles(unsigned long xloops)
+{
+ return (xloops * loops_per_jiffy * HZ) >> 32;
+}
+
static inline void __iomem *
tcs_reg_addr(const struct rsc_drv *drv, int reg, int tcs_id)
{
@@ -757,6 +775,48 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
}

/**
+ * rpmh_rsc_write_next_wakeup() - Write next wakeup in CONTROL_TCS.
+ * @drv: The controller
+ *
+ * Writes maximum wakeup cycles when called from suspend.
+ * Writes earliest hrtimer wakeup when called from idle.
+ */
+void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
+{
+ ktime_t now, wakeup;
+ u64 wakeup_us, wakeup_cycles = ~0;
+ u32 lo, hi;
+
+ if (!drv->tcs[CONTROL_TCS].num_tcs || !drv->genpd)
+ return;
+
+ /* Set highest time when system (timekeeping) is suspended */
+ if (system_state == SYSTEM_SUSPEND)
+ goto exit;
+
+ /* Find the earliest hrtimer wakeup from online cpus */
+ wakeup = drv->genpd->next_hrtimer;
+
+ /* Find the relative wakeup in kernel time scale */
+ now = ktime_get();
+ wakeup = ktime_sub(wakeup, now);
+ wakeup_us = ktime_to_us(wakeup);
+
+ /* Convert the wakeup to arch timer scale */
+ wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
+ wakeup_cycles += arch_timer_read_counter();
+
+exit:
+ lo = wakeup_cycles & RSC_DRV_CTL_TCS_DATA_LO_MASK;
+ hi = wakeup_cycles >> RSC_DRV_CTL_TCS_DATA_SIZE;
+ hi &= RSC_DRV_CTL_TCS_DATA_HI_MASK;
+ hi |= RSC_DRV_CTL_TCS_DATA_HI_VALID;
+
+ writel_relaxed(lo, drv->base + RSC_DRV_CTL_TCS_DATA_LO);
+ writel_relaxed(hi, drv->base + RSC_DRV_CTL_TCS_DATA_HI);
+}
+
+/**
* rpmh_rsc_cpu_pm_callback() - Check if any of the AMCs are busy.
* @nfb: Pointer to the notifier block in struct rsc_drv.
* @action: CPU_PM_ENTER, CPU_PM_ENTER_FAILED, or CPU_PM_EXIT.
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 01765ee..3a53ed9 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -450,7 +450,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)

if (!ctrlr->dirty) {
pr_debug("Skipping flush, TCS has latest data.\n");
- goto exit;
+ goto write_next_wakeup;
}

/* Invalidate the TCSes first to avoid stale data */
@@ -479,6 +479,8 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)

ctrlr->dirty = false;

+write_next_wakeup:
+ rpmh_rsc_write_next_wakeup(ctrlr_to_drv(ctrlr));
exit:
spin_unlock(&ctrlr->cache_lock);
return ret;
--
2.7.4


2022-01-09 17:26:24

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 07/10] arm64: dts: qcom: Add power-domains property for apps_rsc

Add power-domains property which allows apps_rsc device to attach
to cluster power domain on sm8150, sm8250, sm8350 and sm8450.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
---
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


2022-01-09 17:26:29

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
SoCs when the deepest CPUidle modes results in the SoC also to enter
the low power mode.

RSC is part of CPU subsystem and APSS rsc device is attached to cluster
power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
can wakeup the SoC from deepest low power states. The CONTROL_TCS does
this by writing next wakeup in always on domain timer when the SoC is
entering the low power state.

Store the domain wakeup time from all the CPUs which can be used from
domain power off callback by RSC device.

Signed-off-by: Maulik Shah <[email protected]>
---
drivers/base/power/domain_governor.c | 1 +
include/linux/pm_domain.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index cd08c58..a4c7dd8 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
domain_wakeup = next_hrtimer;
}
}
+ genpd->next_hrtimer = domain_wakeup;

/* The minimum idle duration is from now - until the next wakeup. */
idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 67017c9..682b372 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -136,6 +136,7 @@ struct generic_pm_domain {
struct gpd_dev_ops dev_ops;
s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
ktime_t next_wakeup; /* Maintained by the domain governor */
+ ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */
bool max_off_time_changed;
bool cached_power_down_ok;
bool cached_power_down_state_idx;
--
2.7.4


2022-01-09 17:26:35

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 09/10] soc: qcom: rpmh-rsc: Save base address of drv

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 <[email protected]>
---
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


2022-01-09 17:26:44

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH 06/10] soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

From: Lina Iyer <[email protected]>

RSC is part the CPU subsystem and powers off the CPU domains when all
the CPUs and no RPMH transactions are pending from any of the drivers.
The RSC needs to flush the 'sleep' and 'wake' votes that are critical
for saving power when all the CPUs are in idle.

Let's make RSC part of the CPU PM domains, by attaching it to the
cluster power domain. Registering for PM domain notifications, RSC
driver can be notified that the last CPU is powering down. When the last
CPU is powering down the domain, let's flush the 'sleep' and 'wake'
votes that are stored in the data buffers into the hardware and also
write next wakeup in CONTROL_TCS.

Signed-off-by: Lina Iyer <[email protected]>
Signed-off-by: Maulik Shah <[email protected]>
---
drivers/soc/qcom/rpmh-internal.h | 6 +++-
drivers/soc/qcom/rpmh-rsc.c | 60 ++++++++++++++++++++++++++++++++++++++--
2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 344ba68..32ac117 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -97,7 +97,9 @@ struct rpmh_ctrlr {
* @rsc_pm: CPU PM notifier for controller.
* Used when solver mode is not present.
* @cpus_in_pm: Number of CPUs not in idle power collapse.
- * Used when solver mode is not present.
+ * Used when solver mode and "power-domains" is not present.
+ * @genpd_nb: PM Domain notifier for cluster genpd notifications.
+ * @genpdb: PM Domain for cluster genpd.
* @tcs: TCS groups.
* @tcs_in_use: S/W state of the TCS; only set for ACTIVE_ONLY
* transfers, but might show a sleep/wake TCS in use if
@@ -117,6 +119,8 @@ struct rsc_drv {
int id;
int num_tcs;
struct notifier_block rsc_pm;
+ struct notifier_block genpd_nb;
+ struct generic_pm_domain *genpd;
atomic_t cpus_in_pm;
struct tcs_group tcs[TCS_TYPE_NR];
DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 01c2f50c..5875ad5 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -14,10 +14,13 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/module.h>
+#include <linux/notifier.h>
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/wait.h>
@@ -834,6 +837,51 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb,
return ret;
}

+/**
+ * rpmh_rsc_pd_callback() - Check if any of the AMCs are busy.
+ * @nfb: Pointer to the genpd notifier block in struct rsc_drv.
+ * @action: GENPD_NOTIFY_PRE_OFF, GENPD_NOTIFY_OFF, GENPD_NOTIFY_PRE_ON or GENPD_NOTIFY_ON.
+ * @v: Unused
+ *
+ * This function is given to dev_pm_genpd_add_notifier() so we can be informed
+ * about when cluster-pd is going down. When cluster go down we know no more active
+ * transfers will be started so we write sleep/wake sets. This function gets
+ * called from cpuidle code paths and also at system suspend time.
+ *
+ * If AMCs are not busy then writes cached sleep and wake messages to TCSes.
+ * The firmware then takes care of triggering them when entering deepest low power modes.
+ *
+ * Return:
+ * * NOTIFY_OK - success
+ * * NOTIFY_BAD - failure
+ */
+static int rpmh_rsc_pd_callback(struct notifier_block *nfb,
+ unsigned long action, void *v)
+{
+ struct rsc_drv *drv = container_of(nfb, struct rsc_drv, genpd_nb);
+
+ /* We don't need to lock as domin on/off are serialized */
+ if ((action == GENPD_NOTIFY_PRE_OFF) &&
+ (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)))
+ return NOTIFY_BAD;
+
+ return NOTIFY_OK;
+}
+
+static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
+{
+ int ret;
+
+ pm_runtime_enable(dev);
+ ret = dev_pm_domain_attach(dev, false);
+ if (ret)
+ return ret;
+
+ drv->genpd = pd_to_genpd(dev->pm_domain);
+ drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
+ 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)
{
@@ -963,7 +1011,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
return ret;

/*
- * CPU PM notification are not required for controllers that support
+ * CPU PM/genpd notification are not required for controllers that support
* 'HW solver' mode where they can be in autonomous mode executing low
* power mode to power down.
*/
@@ -971,8 +1019,14 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
if (!solver_config) {
- drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
- cpu_pm_register_notifier(&drv->rsc_pm);
+ if (of_find_property(dn, "power-domains", NULL)) {
+ ret = rpmh_rsc_pd_attach(drv, &pdev->dev);
+ if (ret)
+ return ret;
+ } else {
+ drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
+ cpu_pm_register_notifier(&drv->rsc_pm);
+ }
}

/* Enable the active TCS to send requests immediately */
--
2.7.4


2022-01-14 12:30:48

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 06/10] soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> wrote:
>
> From: Lina Iyer <[email protected]>
>
> RSC is part the CPU subsystem and powers off the CPU domains when all
> the CPUs and no RPMH transactions are pending from any of the drivers.
> The RSC needs to flush the 'sleep' and 'wake' votes that are critical
> for saving power when all the CPUs are in idle.
>
> Let's make RSC part of the CPU PM domains, by attaching it to the
> cluster power domain. Registering for PM domain notifications, RSC
> driver can be notified that the last CPU is powering down. When the last
> CPU is powering down the domain, let's flush the 'sleep' and 'wake'
> votes that are stored in the data buffers into the hardware and also
> write next wakeup in CONTROL_TCS.
>
> Signed-off-by: Lina Iyer <[email protected]>
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> drivers/soc/qcom/rpmh-internal.h | 6 +++-
> drivers/soc/qcom/rpmh-rsc.c | 60 ++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 344ba68..32ac117 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -97,7 +97,9 @@ struct rpmh_ctrlr {
> * @rsc_pm: CPU PM notifier for controller.
> * Used when solver mode is not present.
> * @cpus_in_pm: Number of CPUs not in idle power collapse.
> - * Used when solver mode is not present.
> + * Used when solver mode and "power-domains" is not present.
> + * @genpd_nb: PM Domain notifier for cluster genpd notifications.
> + * @genpdb: PM Domain for cluster genpd.

/s/genpdb/genpd

> * @tcs: TCS groups.
> * @tcs_in_use: S/W state of the TCS; only set for ACTIVE_ONLY
> * transfers, but might show a sleep/wake TCS in use if
> @@ -117,6 +119,8 @@ struct rsc_drv {
> int id;
> int num_tcs;
> struct notifier_block rsc_pm;
> + struct notifier_block genpd_nb;
> + struct generic_pm_domain *genpd;
> atomic_t cpus_in_pm;
> struct tcs_group tcs[TCS_TYPE_NR];
> DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR);
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 01c2f50c..5875ad5 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -14,10 +14,13 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/module.h>
> +#include <linux/notifier.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/wait.h>
> @@ -834,6 +837,51 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb,
> return ret;
> }
>
> +/**
> + * rpmh_rsc_pd_callback() - Check if any of the AMCs are busy.
> + * @nfb: Pointer to the genpd notifier block in struct rsc_drv.
> + * @action: GENPD_NOTIFY_PRE_OFF, GENPD_NOTIFY_OFF, GENPD_NOTIFY_PRE_ON or GENPD_NOTIFY_ON.
> + * @v: Unused
> + *
> + * This function is given to dev_pm_genpd_add_notifier() so we can be informed
> + * about when cluster-pd is going down. When cluster go down we know no more active
> + * transfers will be started so we write sleep/wake sets. This function gets
> + * called from cpuidle code paths and also at system suspend time.
> + *
> + * If AMCs are not busy then writes cached sleep and wake messages to TCSes.
> + * The firmware then takes care of triggering them when entering deepest low power modes.
> + *
> + * Return:
> + * * NOTIFY_OK - success
> + * * NOTIFY_BAD - failure
> + */
> +static int rpmh_rsc_pd_callback(struct notifier_block *nfb,
> + unsigned long action, void *v)
> +{
> + struct rsc_drv *drv = container_of(nfb, struct rsc_drv, genpd_nb);
> +
> + /* We don't need to lock as domin on/off are serialized */

/s/domin/genpd

> + if ((action == GENPD_NOTIFY_PRE_OFF) &&
> + (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)))
> + return NOTIFY_BAD;
> +
> + return NOTIFY_OK;
> +}
> +
> +static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
> +{
> + int ret;
> +
> + pm_runtime_enable(dev);
> + ret = dev_pm_domain_attach(dev, false);

Unless I have missed something, this should not be needed.

This is because it's a regular platform driver and we only have a
single PM domain to attach for the rsc device. In this case, the
platform bus is capable of managing the attach to the genpd. See
platform_probe() in drivers/base/platform.c.

> + if (ret)
> + return ret;
> +
> + drv->genpd = pd_to_genpd(dev->pm_domain);

I couldn't find where this pointer is being used later in the driver.
In any case, you can probably use dev->pm_domain directly wherever
needed instead.

> + drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
> + return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);

You should call pm_runtime_disable() in the error path.

> +}
> +
> static int rpmh_probe_tcs_config(struct platform_device *pdev,
> struct rsc_drv *drv, void __iomem *base)
> {
> @@ -963,7 +1011,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
> return ret;
>
> /*
> - * CPU PM notification are not required for controllers that support
> + * CPU PM/genpd notification are not required for controllers that support
> * 'HW solver' mode where they can be in autonomous mode executing low
> * power mode to power down.
> */
> @@ -971,8 +1019,14 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
> solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
> solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
> if (!solver_config) {
> - drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
> - cpu_pm_register_notifier(&drv->rsc_pm);
> + if (of_find_property(dn, "power-domains", NULL)) {

Rather than parsing the DT, I think it's better to check if
"dev->pm_domain" has been assigned. As I indicated above, the platform
bus manages the attach before the driver's ->probe() callback is
invoked.

> + ret = rpmh_rsc_pd_attach(drv, &pdev->dev);
> + if (ret)
> + return ret;
> + } else {
> + drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
> + cpu_pm_register_notifier(&drv->rsc_pm);
> + }
> }
>
> /* Enable the active TCS to send requests immediately */

Beyond this point, you need to call the below to manage the error path
correctly:
dev_pm_genpd_remove_notifier()
pm_runtime_disable()

> --
> 2.7.4
>

Kind regards
Uffe

2022-01-14 12:31:02

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 02/10] arm64: dts: qcom: sm8250: Add cpuidle states

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <[email protected]> wrote:
>
> This change adds various idle states and add devices to power domains.
>
> Cc: [email protected]
> Signed-off-by: Maulik Shah <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe


> ---
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 105 +++++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 5617a46..077d0ab 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -98,6 +98,8 @@
> capacity-dmips-mhz = <448>;
> dynamic-power-coefficient = <205>;
> next-level-cache = <&L2_0>;
> + power-domains = <&CPU_PD0>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 0>;
> operating-points-v2 = <&cpu0_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -120,6 +122,8 @@
> capacity-dmips-mhz = <448>;
> dynamic-power-coefficient = <205>;
> next-level-cache = <&L2_100>;
> + power-domains = <&CPU_PD1>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 0>;
> operating-points-v2 = <&cpu0_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -139,6 +143,8 @@
> capacity-dmips-mhz = <448>;
> dynamic-power-coefficient = <205>;
> next-level-cache = <&L2_200>;
> + power-domains = <&CPU_PD2>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 0>;
> operating-points-v2 = <&cpu0_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -158,6 +164,8 @@
> capacity-dmips-mhz = <448>;
> dynamic-power-coefficient = <205>;
> next-level-cache = <&L2_300>;
> + power-domains = <&CPU_PD3>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 0>;
> operating-points-v2 = <&cpu0_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -177,6 +185,8 @@
> capacity-dmips-mhz = <1024>;
> dynamic-power-coefficient = <379>;
> next-level-cache = <&L2_400>;
> + power-domains = <&CPU_PD4>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 1>;
> operating-points-v2 = <&cpu4_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -196,6 +206,8 @@
> capacity-dmips-mhz = <1024>;
> dynamic-power-coefficient = <379>;
> next-level-cache = <&L2_500>;
> + power-domains = <&CPU_PD5>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 1>;
> operating-points-v2 = <&cpu4_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -216,6 +228,8 @@
> capacity-dmips-mhz = <1024>;
> dynamic-power-coefficient = <379>;
> next-level-cache = <&L2_600>;
> + power-domains = <&CPU_PD6>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 1>;
> operating-points-v2 = <&cpu4_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -235,6 +249,8 @@
> capacity-dmips-mhz = <1024>;
> dynamic-power-coefficient = <444>;
> next-level-cache = <&L2_700>;
> + power-domains = <&CPU_PD7>;
> + power-domain-names = "psci";
> qcom,freq-domain = <&cpufreq_hw 2>;
> operating-points-v2 = <&cpu7_opp_table>;
> interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -281,6 +297,42 @@
> };
> };
> };
> +
> + 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 = <0x40000004>;
> + entry-latency-us = <360>;
> + exit-latency-us = <531>;
> + min-residency-us = <3934>;
> + 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 = <0x40000004>;
> + entry-latency-us = <702>;
> + exit-latency-us = <1061>;
> + min-residency-us = <4488>;
> + local-timer-stop;
> + };
> + };
> +
> + domain-idle-states {
> + CLUSTER_SLEEP_0: cluster-sleep-0 {
> + compatible = "domain-idle-state";
> + idle-state-name = "cluster-llcc-off";
> + arm,psci-suspend-param = <0x4100c244>;
> + entry-latency-us = <3264>;
> + exit-latency-us = <6562>;
> + min-residency-us = <9987>;
> + local-timer-stop;
> + };
> + };
> };
>
> cpu0_opp_table: cpu0_opp_table {
> @@ -594,6 +646,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>;
> + };
> +
> + CPU_PD1: cpu1 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD2: cpu2 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD3: cpu3 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD4: cpu4 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD5: cpu5 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD6: cpu6 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
> + };
> +
> + CPU_PD7: cpu7 {
> + #power-domain-cells = <0>;
> + power-domains = <&CLUSTER_PD>;
> + domain-idle-states = <&BIG_CPU_SLEEP_0>;
> + };
> +
> + CLUSTER_PD: cpu-cluster0 {
> + #power-domain-cells = <0>;
> + domain-idle-states = <&CLUSTER_SLEEP_0>;
> + };
> };
>
> reserved-memory {
> --
> 2.7.4
>

2022-01-14 12:31:19

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <[email protected]> wrote:
>
> This change updates/corrects below cpuidle parameters
>
> 1. entry-latency, exit-latency and residency for various idle states.
> 2. arm,psci-suspend-param which is same for CLUSTER_SLEEP_0/1 states.
> 3. Add CLUSTER_SLEEP_1 in CLUSTER_PD.
>
> Cc: [email protected]
> Fixes: 5188049c9b36 ("arm64: dts: qcom: Add base SM8450 DTSI")
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 10c25ad..5e329f8 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -203,9 +203,9 @@
> compatible = "arm,idle-state";
> idle-state-name = "silver-rail-power-collapse";
> arm,psci-suspend-param = <0x40000004>;
> - entry-latency-us = <274>;
> - exit-latency-us = <480>;
> - min-residency-us = <3934>;
> + entry-latency-us = <800>;
> + exit-latency-us = <750>;
> + min-residency-us = <4090>;
> local-timer-stop;
> };
>
> @@ -213,9 +213,9 @@
> compatible = "arm,idle-state";
> idle-state-name = "gold-rail-power-collapse";
> arm,psci-suspend-param = <0x40000004>;
> - entry-latency-us = <327>;
> - exit-latency-us = <1502>;
> - min-residency-us = <4488>;
> + entry-latency-us = <600>;
> + exit-latency-us = <1550>;
> + min-residency-us = <4791>;
> local-timer-stop;
> };
> };
> @@ -224,10 +224,10 @@
> CLUSTER_SLEEP_0: cluster-sleep-0 {
> compatible = "domain-idle-state";
> idle-state-name = "cluster-l3-off";
> - arm,psci-suspend-param = <0x4100c344>;
> - entry-latency-us = <584>;
> - exit-latency-us = <2332>;
> - min-residency-us = <6118>;
> + arm,psci-suspend-param = <0x41000044>;
> + entry-latency-us = <1050>;
> + exit-latency-us = <2500>;
> + min-residency-us = <5309>;
> local-timer-stop;
> };
>
> @@ -235,9 +235,9 @@
> compatible = "domain-idle-state";
> idle-state-name = "cluster-power-collapse";
> arm,psci-suspend-param = <0x4100c344>;
> - entry-latency-us = <2893>;
> - exit-latency-us = <4023>;
> - min-residency-us = <9987>;
> + entry-latency-us = <2700>;
> + exit-latency-us = <3500>;
> + min-residency-us = <13959>;
> local-timer-stop;
> };
> };
> @@ -315,7 +315,7 @@
>
> CLUSTER_PD: cpu-cluster0 {
> #power-domain-cells = <0>;
> - domain-idle-states = <&CLUSTER_SLEEP_0>;
> + domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;

Should this be like the below instead?

<&CLUSTER_SLEEP_0>, <&CLUSTER_SLEEP_1>;

> };
> };
>
> --
> 2.7.4
>

Other than the above, feel free to add:

Reviewed-by: Ulf Hansson <[email protected]>

Kind regards
Uffe

2022-01-14 12:31:57

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <[email protected]> wrote:
>
> The change documents power-domains property for RSC device.
> This optional property points to corresponding PM domain node.
>
> Cc: [email protected]
> Signed-off-by: Maulik Shah <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

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
>

2022-01-14 12:34:33

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 07/10] arm64: dts: qcom: Add power-domains property for apps_rsc

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> wrote:
>
> Add power-domains property which allows apps_rsc device to attach
> to cluster power domain on sm8150, sm8250, sm8350 and sm8450.
>
> Cc: [email protected]
> Signed-off-by: Maulik Shah <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

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
>

2022-01-14 12:36:27

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 09/10] soc: qcom: rpmh-rsc: Save base address of drv

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> 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 <[email protected]>

Reviewed-by: Ulf Hansson <[email protected]>

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
>

2022-01-14 13:35:37

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 10/10] soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> wrote:
>
> The next wakeup timer value needs to be set in always on domain timer
> as the arch timer interrupt can not wakeup the SoC if after the deepest
> CPUidle states the SoC also enters deepest low power state.
>
> To wakeup the SoC in such scenarios the earliest wakeup time is set in
> CONTROL_TCS and the firmware takes care of setting up its own timer in
> always on domain with next wakeup time. The timer wakes up the RSC and
> sets resources back to wake state.
>
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> drivers/soc/qcom/rpmh-internal.h | 1 +
> drivers/soc/qcom/rpmh-rsc.c | 60 ++++++++++++++++++++++++++++++++++++++++
> drivers/soc/qcom/rpmh.c | 4 ++-
> 3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 6770bbb..04789a37 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -135,6 +135,7 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg);
> int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
> const struct tcs_request *msg);
> void rpmh_rsc_invalidate(struct rsc_drv *drv);
> +void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv);
>
> void rpmh_tx_done(const struct tcs_request *msg, int r);
> int rpmh_flush(struct rpmh_ctrlr *ctrlr);
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index c2a7c6c..b3b85f1 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -12,6 +12,7 @@
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/kernel.h>
> +#include <linux/ktime.h>
> #include <linux/list.h>
> #include <linux/module.h>
> #include <linux/notifier.h>
> @@ -25,6 +26,7 @@
> #include <linux/spinlock.h>
> #include <linux/wait.h>
>
> +#include <clocksource/arm_arch_timer.h>
> #include <soc/qcom/cmd-db.h>
> #include <soc/qcom/tcs.h>
> #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> @@ -49,6 +51,14 @@
> #define DRV_NCPT_MASK 0x1F
> #define DRV_NCPT_SHIFT 27
>
> +/* Offsets for CONTROL TCS Registers */
> +#define RSC_DRV_CTL_TCS_DATA_HI 0x38
> +#define RSC_DRV_CTL_TCS_DATA_HI_MASK 0xFFFFFF
> +#define RSC_DRV_CTL_TCS_DATA_HI_VALID BIT(31)
> +#define RSC_DRV_CTL_TCS_DATA_LO 0x40
> +#define RSC_DRV_CTL_TCS_DATA_LO_MASK 0xFFFFFFFF
> +#define RSC_DRV_CTL_TCS_DATA_SIZE 32
> +
> /* Offsets for common TCS Registers, one bit per TCS */
> #define RSC_DRV_IRQ_ENABLE 0x00
> #define RSC_DRV_IRQ_STATUS 0x04
> @@ -142,6 +152,14 @@
> * +---------------------------------------------------+
> */
>
> +#define USECS_TO_CYCLES(time_usecs) \
> + xloops_to_cycles((time_usecs) * 0x10C7UL)
> +
> +static inline unsigned long xloops_to_cycles(unsigned long xloops)
> +{
> + return (xloops * loops_per_jiffy * HZ) >> 32;
> +}
> +
> static inline void __iomem *
> tcs_reg_addr(const struct rsc_drv *drv, int reg, int tcs_id)
> {
> @@ -757,6 +775,48 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
> }
>
> /**
> + * rpmh_rsc_write_next_wakeup() - Write next wakeup in CONTROL_TCS.
> + * @drv: The controller
> + *
> + * Writes maximum wakeup cycles when called from suspend.
> + * Writes earliest hrtimer wakeup when called from idle.
> + */
> +void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
> +{
> + ktime_t now, wakeup;
> + u64 wakeup_us, wakeup_cycles = ~0;
> + u32 lo, hi;
> +
> + if (!drv->tcs[CONTROL_TCS].num_tcs || !drv->genpd)

Just curious, but in case you don't have a genpd attached, but are
rather using the CPU PM notifiers to determine the last CPU - in that
case, don't you need to write a new value for the timer/wakeup?

> + return;
> +
> + /* Set highest time when system (timekeeping) is suspended */
> + if (system_state == SYSTEM_SUSPEND)
> + goto exit;
> +
> + /* Find the earliest hrtimer wakeup from online cpus */
> + wakeup = drv->genpd->next_hrtimer;
> +
> + /* Find the relative wakeup in kernel time scale */
> + now = ktime_get();
> + wakeup = ktime_sub(wakeup, now);
> + wakeup_us = ktime_to_us(wakeup);
> +
> + /* Convert the wakeup to arch timer scale */
> + wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
> + wakeup_cycles += arch_timer_read_counter();
> +
> +exit:
> + lo = wakeup_cycles & RSC_DRV_CTL_TCS_DATA_LO_MASK;
> + hi = wakeup_cycles >> RSC_DRV_CTL_TCS_DATA_SIZE;
> + hi &= RSC_DRV_CTL_TCS_DATA_HI_MASK;
> + hi |= RSC_DRV_CTL_TCS_DATA_HI_VALID;
> +
> + writel_relaxed(lo, drv->base + RSC_DRV_CTL_TCS_DATA_LO);
> + writel_relaxed(hi, drv->base + RSC_DRV_CTL_TCS_DATA_HI);
> +}
> +
> +/**
> * rpmh_rsc_cpu_pm_callback() - Check if any of the AMCs are busy.
> * @nfb: Pointer to the notifier block in struct rsc_drv.
> * @action: CPU_PM_ENTER, CPU_PM_ENTER_FAILED, or CPU_PM_EXIT.
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index 01765ee..3a53ed9 100644
> --- a/drivers/soc/qcom/rpmh.c
> +++ b/drivers/soc/qcom/rpmh.c
> @@ -450,7 +450,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
>
> if (!ctrlr->dirty) {
> pr_debug("Skipping flush, TCS has latest data.\n");
> - goto exit;
> + goto write_next_wakeup;
> }
>
> /* Invalidate the TCSes first to avoid stale data */
> @@ -479,6 +479,8 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
>
> ctrlr->dirty = false;
>
> +write_next_wakeup:
> + rpmh_rsc_write_next_wakeup(ctrlr_to_drv(ctrlr));
> exit:
> spin_unlock(&ctrlr->cache_lock);
> return ret;

Kind regards
Uffe

2022-01-14 13:38:58

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> wrote:
>
> The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
> SoCs when the deepest CPUidle modes results in the SoC also to enter
> the low power mode.
>
> RSC is part of CPU subsystem and APSS rsc device is attached to cluster
> power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
> can wakeup the SoC from deepest low power states. The CONTROL_TCS does
> this by writing next wakeup in always on domain timer when the SoC is
> entering the low power state.
>
> Store the domain wakeup time from all the CPUs which can be used from
> domain power off callback by RSC device.
>
> Signed-off-by: Maulik Shah <[email protected]>

I need to think a little bit more about this one, so I have to get
back with some more detailed comments next week.

Kind regards
Uffe

> ---
> drivers/base/power/domain_governor.c | 1 +
> include/linux/pm_domain.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index cd08c58..a4c7dd8 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> domain_wakeup = next_hrtimer;
> }
> }
> + genpd->next_hrtimer = domain_wakeup;
>
> /* The minimum idle duration is from now - until the next wakeup. */
> idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 67017c9..682b372 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -136,6 +136,7 @@ struct generic_pm_domain {
> struct gpd_dev_ops dev_ops;
> s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
> ktime_t next_wakeup; /* Maintained by the domain governor */
> + ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */
> bool max_off_time_changed;
> bool cached_power_down_ok;
> bool cached_power_down_state_idx;
> --
> 2.7.4
>

2022-01-17 17:00:45

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: Re: [PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

Hi,

On 1/14/2022 6:00 PM, Ulf Hansson wrote:
>
>> @@ -315,7 +315,7 @@
>>
>> CLUSTER_PD: cpu-cluster0 {
>> #power-domain-cells = <0>;
>> - domain-idle-states = <&CLUSTER_SLEEP_0>;
>> + domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;
> Should this be like the below instead?
>
> <&CLUSTER_SLEEP_0>, <&CLUSTER_SLEEP_1>;

Thanks for catching this. Will correct in v2.

Thanks,
Maulik

2022-01-22 18:11:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

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: [email protected]
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>

Acked-by: Rob Herring <[email protected]>

2022-01-26 06:33:44

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <[email protected]> wrote:
>
> The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
> SoCs when the deepest CPUidle modes results in the SoC also to enter
> the low power mode.
>
> RSC is part of CPU subsystem and APSS rsc device is attached to cluster
> power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
> can wakeup the SoC from deepest low power states. The CONTROL_TCS does
> this by writing next wakeup in always on domain timer when the SoC is
> entering the low power state.
>
> Store the domain wakeup time from all the CPUs which can be used from
> domain power off callback by RSC device.
>
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> drivers/base/power/domain_governor.c | 1 +
> include/linux/pm_domain.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index cd08c58..a4c7dd8 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
> domain_wakeup = next_hrtimer;
> }
> }
> + genpd->next_hrtimer = domain_wakeup;
>
> /* The minimum idle duration is from now - until the next wakeup. */
> idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 67017c9..682b372 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -136,6 +136,7 @@ struct generic_pm_domain {
> struct gpd_dev_ops dev_ops;
> s64 max_off_time_ns; /* Maximum allowed "suspended" time. */
> ktime_t next_wakeup; /* Maintained by the domain governor */
> + ktime_t next_hrtimer; /* Closest hrtimer event of the domain CPUs */

Would you mind clarifying the comment into something along the lines
of: "/* Next hrtimer for the CPU PM domain */

> bool max_off_time_changed;
> bool cached_power_down_ok;
> bool cached_power_down_state_idx;
> --
> 2.7.4
>

Beside the nitpick above, I have a few additional minor comments.

*) Users of genpd->next_hrtimer should not access this member in the
struct generic_pm_domain themselves. Instead, I suggest we add a genpd
helper function to deal with this. In this regard, we should also add
a description to the helper function to explain under what *specific*
conditions it's allowed to be called for.

**) We should assign genpd->next_hrtimer a default value in
pm_genpd_init(). Perhaps that can also be used in a way to make sure
the helper function always returns a valid value!?

Other than this, I think the approach looks sane to me!

Kind regards
Uffe

2022-02-02 16:57:10

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

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
[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 <[email protected]>

2022-03-29 11:38:08

by Amit Pundir

[permalink] [raw]
Subject: Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <[email protected]> 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 <[email protected]>

2022-04-26 12:44:16

by Amit Pundir

[permalink] [raw]
Subject: Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

On Tue, 29 Mar 2022 at 15:25, Amit Pundir <[email protected]> wrote:
>
> On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <[email protected]> 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 <[email protected]>