2020-03-05 11:48:32

by Maulik Shah

[permalink] [raw]
Subject: [PATCH v11 0/3] Invoke rpmh_flush for non OSI targets

Changes in v11:
- Address Doug's comments on change 2 and 3
- Include change to invalidate TCSes before flush from [4]

Changes in v10:
- Address Evan's comments to update commit message on change 2
- Add Evan's Reviewed by on change 2
- Remove comment from rpmh_flush() related to last CPU invoking it
- Rebase all changes on top of next-20200302

Changes in v9:
- Keep rpmh_flush() to invoke from within cache_lock
- Remove comments related to only last cpu invoking rpmh_flush()

Changes in v8:
- Address Stephen's comments on changes 2 and 3
- Add Reviewed by from Stephen on change 1

Changes in v7:
- Address Srinivas's comments to update commit text
- Add Reviewed by from Srinivas

Changes in v6:
- Drop 1 & 2 changes from v5 as they already landed in maintainer tree
- Drop 3 & 4 changes from v5 as no user at present for power domain in rsc
- Rename subject to appropriate since power domain changes are dropped
- Rebase other changes on top of next-20200221

Changes in v5:
- Add Rob's Acked by on dt-bindings change
- Drop firmware psci change
- Update cpuidle stats in dtsi to follow PC mode
- Include change to update dirty flag when data is updated from [4]
- Add change to invoke rpmh_flush when caches are dirty

Changes in v4:
- Add change to allow hierarchical topology in PC mode
- Drop hierarchical domain idle states converter from v3
- Address Merge sc7180 dtsi change to add low power modes

Changes in v3:
- Address Rob's comment on dt property value
- Address Stephen's comments on rpmh-rsc driver change
- Include sc7180 cpuidle low power mode changes from [1]
- Include hierarchical domain idle states converter change from [2]

Changes in v2:
- Add Stephen's Reviewed-By to the first three patches
- Addressed Stephen's comments on fourth patch
- Include changes to connect rpmh domain to cpuidle and genpds

Resource State Coordinator (RSC) is responsible for powering off/lowering
the requirements from CPU subsystem for the associated hardware like buses,
clocks, and regulators when all CPUs and cluster is powered down.

RSC power domain uses last-man activities provided by genpd framework based
on Ulf Hansoon's patch series[3], when the cluster of CPUs enter deepest
idle states. As a part of domain poweroff, RSC can lower resource state
requirements by flushing the cached sleep and wake state votes for various
resources.

[1] https://patchwork.kernel.org/patch/11218965
[2] https://patchwork.kernel.org/patch/10941671
[3] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=222355
[4] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=236503

Maulik Shah (3):
arm64: dts: qcom: sc7180: Add cpuidle low power states
soc: qcom: rpmh: Update dirty flag only when data changes
soc: qcom: rpmh: Invoke rpmh_flush for dirty caches

arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++
drivers/soc/qcom/rpmh.c | 27 ++++++++++---
2 files changed, 100 insertions(+), 5 deletions(-)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2020-03-05 11:48:39

by Maulik Shah

[permalink] [raw]
Subject: [PATCH v11 1/4] arm64: dts: qcom: sc7180: Add cpuidle low power states

Add device bindings for cpuidle states for cpu devices.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
Reviewed-by: Srinivas Rao L <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 253274d..f5c08ce 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -94,6 +94,9 @@
compatible = "arm,armv8";
reg = <0x0 0x0>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_0>;
@@ -113,6 +116,9 @@
compatible = "arm,armv8";
reg = <0x0 0x100>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_100>;
@@ -129,6 +135,9 @@
compatible = "arm,armv8";
reg = <0x0 0x200>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_200>;
@@ -145,6 +154,9 @@
compatible = "arm,armv8";
reg = <0x0 0x300>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_300>;
@@ -161,6 +173,9 @@
compatible = "arm,armv8";
reg = <0x0 0x400>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_400>;
@@ -177,6 +192,9 @@
compatible = "arm,armv8";
reg = <0x0 0x500>;
enable-method = "psci";
+ cpu-idle-states = <&LITTLE_CPU_SLEEP_0
+ &LITTLE_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1024>;
dynamic-power-coefficient = <100>;
next-level-cache = <&L2_500>;
@@ -193,6 +211,9 @@
compatible = "arm,armv8";
reg = <0x0 0x600>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0
+ &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1740>;
dynamic-power-coefficient = <405>;
next-level-cache = <&L2_600>;
@@ -209,6 +230,9 @@
compatible = "arm,armv8";
reg = <0x0 0x700>;
enable-method = "psci";
+ cpu-idle-states = <&BIG_CPU_SLEEP_0
+ &BIG_CPU_SLEEP_1
+ &CLUSTER_SLEEP_0>;
capacity-dmips-mhz = <1740>;
dynamic-power-coefficient = <405>;
next-level-cache = <&L2_700>;
@@ -255,6 +279,60 @@
};
};
};
+
+ idle-states {
+ entry-method = "psci";
+
+ LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-power-down";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <549>;
+ exit-latency-us = <901>;
+ min-residency-us = <1774>;
+ local-timer-stop;
+ };
+
+ LITTLE_CPU_SLEEP_1: cpu-sleep-0-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "little-rail-power-down";
+ arm,psci-suspend-param = <0x40000004>;
+ entry-latency-us = <702>;
+ exit-latency-us = <915>;
+ min-residency-us = <4001>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-power-down";
+ arm,psci-suspend-param = <0x40000003>;
+ entry-latency-us = <523>;
+ exit-latency-us = <1244>;
+ min-residency-us = <2207>;
+ local-timer-stop;
+ };
+
+ BIG_CPU_SLEEP_1: cpu-sleep-1-1 {
+ compatible = "arm,idle-state";
+ idle-state-name = "big-rail-power-down";
+ arm,psci-suspend-param = <0x40000004>;
+ entry-latency-us = <526>;
+ exit-latency-us = <1854>;
+ min-residency-us = <5555>;
+ local-timer-stop;
+ };
+
+ CLUSTER_SLEEP_0: cluster-sleep-0 {
+ compatible = "arm,idle-state";
+ idle-state-name = "cluster-power-down";
+ arm,psci-suspend-param = <0x40003444>;
+ entry-latency-us = <3263>;
+ exit-latency-us = <6562>;
+ min-residency-us = <9926>;
+ local-timer-stop;
+ };
+ };
};

memory@80000000 {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-03-05 11:48:50

by Maulik Shah

[permalink] [raw]
Subject: [PATCH v11 2/4] soc: qcom: rpmh: Update dirty flag only when data changes

Currently rpmh ctrlr dirty flag is set for all cases regardless of data
is really changed or not. Add changes to update dirty flag when data is
changed to newer values. Update dirty flag everytime when data in batch
cache is updated since rpmh_flush() may get invoked from any CPU instead
of only last CPU going to low power mode.

Also move dirty flag updates to happen from within cache_lock and remove
unnecessary INIT_LIST_HEAD() call and a default case from switch.

Fixes: 600513dfeef3 ("drivers: qcom: rpmh: cache sleep/wake state requests")
Signed-off-by: Maulik Shah <[email protected]>
Reviewed-by: Srinivas Rao L <[email protected]>
Reviewed-by: Evan Green <[email protected]>
---
drivers/soc/qcom/rpmh.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index eb0ded0..03630ae 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -119,6 +119,7 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
{
struct cache_req *req;
unsigned long flags;
+ u32 old_sleep_val, old_wake_val;

spin_lock_irqsave(&ctrlr->cache_lock, flags);
req = __find_req(ctrlr, cmd->addr);
@@ -133,26 +134,27 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,

req->addr = cmd->addr;
req->sleep_val = req->wake_val = UINT_MAX;
- INIT_LIST_HEAD(&req->list);
list_add_tail(&req->list, &ctrlr->cache);

existing:
+ old_sleep_val = req->sleep_val;
+ old_wake_val = req->wake_val;
+
switch (state) {
case RPMH_ACTIVE_ONLY_STATE:
- if (req->sleep_val != UINT_MAX)
- req->wake_val = cmd->data;
- break;
case RPMH_WAKE_ONLY_STATE:
req->wake_val = cmd->data;
break;
case RPMH_SLEEP_STATE:
req->sleep_val = cmd->data;
break;
- default:
- break;
}

- ctrlr->dirty = true;
+ ctrlr->dirty = (req->sleep_val != old_sleep_val ||
+ req->wake_val != old_wake_val) &&
+ req->sleep_val != UINT_MAX &&
+ req->wake_val != UINT_MAX;
+
unlock:
spin_unlock_irqrestore(&ctrlr->cache_lock, flags);

@@ -287,6 +289,7 @@ static void cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)

spin_lock_irqsave(&ctrlr->cache_lock, flags);
list_add_tail(&req->list, &ctrlr->batch_cache);
+ ctrlr->dirty = true;
spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
}

@@ -323,6 +326,7 @@ static void invalidate_batch(struct rpmh_ctrlr *ctrlr)
list_for_each_entry_safe(req, tmp, &ctrlr->batch_cache, list)
kfree(req);
INIT_LIST_HEAD(&ctrlr->batch_cache);
+ ctrlr->dirty = true;
spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
}

@@ -507,7 +511,6 @@ int rpmh_invalidate(const struct device *dev)
int ret;

invalidate_batch(ctrlr);
- ctrlr->dirty = true;

do {
ret = rpmh_rsc_invalidate(ctrlr_to_drv(ctrlr));
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-03-05 11:49:16

by Maulik Shah

[permalink] [raw]
Subject: [PATCH v11 3/4] soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches

Add changes to invoke rpmh flush() from within cache_lock when the data
in cache is dirty.

This is done only if OSI is not supported in PSCI. If OSI is supported
rpmh_flush can get invoked when the last cpu going to power collapse
deepest low power mode.

Also remove "depends on COMPILE_TEST" for Kconfig option QCOM_RPMH so the
driver is only compiled for arm64 which supports psci_has_osi_support()
API.

Signed-off-by: Maulik Shah <[email protected]>
Reviewed-by: Srinivas Rao L <[email protected]>
---
drivers/soc/qcom/rpmh.c | 49 +++++++++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 03630ae..1951f6a 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/psci.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/types.h>
@@ -113,13 +114,14 @@ static struct cache_req *__find_req(struct rpmh_ctrlr *ctrlr, u32 addr)
return req;
}

-static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
+static int cache_rpm_request(struct rpmh_ctrlr *ctrlr,
enum rpmh_state state,
struct tcs_cmd *cmd)
{
struct cache_req *req;
unsigned long flags;
u32 old_sleep_val, old_wake_val;
+ int ret = 0;

spin_lock_irqsave(&ctrlr->cache_lock, flags);
req = __find_req(ctrlr, cmd->addr);
@@ -155,10 +157,13 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
req->sleep_val != UINT_MAX &&
req->wake_val != UINT_MAX;

+ if (ctrlr->dirty && !psci_has_osi_support())
+ ret = rpmh_flush(ctrlr);
+
unlock:
spin_unlock_irqrestore(&ctrlr->cache_lock, flags);

- return req;
+ return ret;
}

/**
@@ -176,17 +181,16 @@ static int __rpmh_write(const struct device *dev, enum rpmh_state state,
struct rpmh_request *rpm_msg)
{
struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
- int ret = -EINVAL;
- struct cache_req *req;
+ int ret;
int i;

rpm_msg->msg.state = state;

/* Cache the request in our store and link the payload */
for (i = 0; i < rpm_msg->msg.num_cmds; i++) {
- req = cache_rpm_request(ctrlr, state, &rpm_msg->msg.cmds[i]);
- if (IS_ERR(req))
- return PTR_ERR(req);
+ ret = cache_rpm_request(ctrlr, state, &rpm_msg->msg.cmds[i]);
+ if (ret)
+ return ret;
}

rpm_msg->msg.state = state;
@@ -283,26 +287,32 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
}
EXPORT_SYMBOL(rpmh_write);

-static void cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)
+static int cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)
{
unsigned long flags;
+ int ret = 0;

spin_lock_irqsave(&ctrlr->cache_lock, flags);
+
list_add_tail(&req->list, &ctrlr->batch_cache);
ctrlr->dirty = true;
+
+ if (!psci_has_osi_support())
+ ret = rpmh_flush(ctrlr);
+
spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+
+ return ret;
}

static int flush_batch(struct rpmh_ctrlr *ctrlr)
{
struct batch_cache_req *req;
const struct rpmh_request *rpm_msg;
- unsigned long flags;
int ret = 0;
int i;

/* Send Sleep/Wake requests to the controller, expect no response */
- spin_lock_irqsave(&ctrlr->cache_lock, flags);
list_for_each_entry(req, &ctrlr->batch_cache, list) {
for (i = 0; i < req->count; i++) {
rpm_msg = req->rpm_msgs + i;
@@ -312,7 +322,6 @@ static int flush_batch(struct rpmh_ctrlr *ctrlr)
break;
}
}
- spin_unlock_irqrestore(&ctrlr->cache_lock, flags);

return ret;
}
@@ -384,10 +393,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
cmd += n[i];
}

- if (state != RPMH_ACTIVE_ONLY_STATE) {
- cache_batch(ctrlr, req);
- return 0;
- }
+ if (state != RPMH_ACTIVE_ONLY_STATE)
+ return cache_batch(ctrlr, req);

for (i = 0; i < count; i++) {
struct completion *compl = &compls[i];
@@ -450,12 +457,10 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state,
*
* @ctrlr: controller making request to flush cached data
*
- * Return: -EBUSY if the controller is busy, probably waiting on a response
- * to a RPMH request sent earlier.
+ * Return: 0 on success, error number otherwise.
*
- * This function is always called from the sleep code from the last CPU
- * that is powering down the entire system. Since no other RPMH API would be
- * executing at this time, it is safe to run lockless.
+ * This function can either be called from sleep code on the last CPU
+ * (thus no spinlock needed) or with the ctrlr->cache_lock already held.
*/
int rpmh_flush(struct rpmh_ctrlr *ctrlr)
{
@@ -472,10 +477,6 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
if (ret)
return ret;

- /*
- * Nobody else should be calling this function other than system PM,
- * hence we can run without locks.
- */
list_for_each_entry(p, &ctrlr->cache, list) {
if (!is_req_valid(p)) {
pr_debug("%s: skipping RPMH req: a:%#x s:%#x w:%#x",
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-03-05 11:50:28

by Maulik Shah

[permalink] [raw]
Subject: [PATCH v11 4/4] soc: qcom: rpmh: Invalidate SLEEP and WAKE TCSes before flushing new data

TCSes have previously programmed data when rpmh_flush is called.
This can cause old data to trigger along with newly flushed.

Fix this by cleaning SLEEP and WAKE TCSes before new data is flushed.

Fixes: 600513dfeef3 ("drivers: qcom: rpmh: cache sleep/wake state requests")
Signed-off-by: Maulik Shah <[email protected]>
---
drivers/soc/qcom/rpmh.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 1951f6a..63364ce 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -472,6 +472,11 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
return 0;
}

+ /* Invalidate the TCSes first to avoid stale data */
+ do {
+ ret = rpmh_rsc_invalidate(ctrlr_to_drv(ctrlr));
+ } while (ret == -EAGAIN);
+
/* First flush the cached batch requests */
ret = flush_batch(ctrlr);
if (ret)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation