Skip to contentSkip to conversation

Invoke rpmh_flush for non OSI targets

PATCH Superseded Maulik Shah 3 participants 11 messages Mar 5, 2020 – Mar 9, 2020

Subsystems (4)

v12 of 17 Superseded Latest is v17 →

lore
Share & tools
lore
#0 lore raw

v12 cover Invoke rpmh_flush for non OSI targets

Changes in v12:
- Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it.

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(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

PATCH v12 1/4 arm64: dts: qcom: sc7180: Add cpuidle low power states

Add device bindings for cpuidle states for cpu devices.

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 78 ++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
arch/arm64/boot/dts/qcom/sc7180.dtsi +78 -0
index 253274d..f5c08ce 100644
@@ -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 {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#2 lore raw

PATCH v12 2/4 soc: qcom: rpmh: Update dirty flag only when data changes

In reply to Maulik Shah

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")
---
 drivers/soc/qcom/rpmh.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
drivers/soc/qcom/rpmh.c +11 -8
index eb0ded0..03630ae 100644
@@ -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));
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#3 lore raw

PATCH v12 3/4 soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches

In reply to Maulik Shah

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.

---
 drivers/soc/qcom/Kconfig |  2 +-
 drivers/soc/qcom/rpmh.c  | 49 ++++++++++++++++++++++++------------------------
 2 files changed, 26 insertions(+), 25 deletions(-)
drivers/soc/qcom/Kconfig +1 -1
index d0a73e7..2e581bc 100644
@@ -105,7 +105,7 @@ config QCOM_RMTFS_MEM
 
 config QCOM_RPMH
 	bool "Qualcomm RPM-Hardened (RPMH) Communication"
-	depends on ARCH_QCOM && ARM64 || COMPILE_TEST
+	depends on ARCH_QCOM && ARM64
 	help
 	  Support for communication with the hardened-RPM blocks in
 	  Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an
drivers/soc/qcom/rpmh.c +25 -24

136 lines not on this page · raw message

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#4 lore raw

PATCH v12 4/4 soc: qcom: rpmh: Invalidate SLEEP and WAKE TCSes before flushing new data

In reply to Maulik Shah

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")
---
 drivers/soc/qcom/rpmh.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/qcom/rpmh.c +5 -0
index 1951f6a..63364ce 100644
@@ -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)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#5 lore raw

Re: [PATCH v12 4/4] soc: qcom: rpmh: Invalidate SLEEP and WAKE TCSes before flushing new data

Hi,

Quoted 18 lines · Maulik Shah

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")
---
 drivers/soc/qcom/rpmh.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/qcom/rpmh.c +0 -0
index 1951f6a..63364ce 100644
@@ -472,6 +472,11 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
                return 0;

drivers/soc/qcom/rpmh.c int rpmh_flush(struct rpmh_ctrlr *ctrlr) · line 482

Quoted 10 lines
drivers/soc/qcom/rpmh.c +5 -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)

I think you should make this patch 3/4 instead of 4/4, and then:

1. In this patch remove the call to rpmh_rsc_invalidate() in rpmh_invalidate(). You've already marked things "dirty" in invalidate_batch() so no need to actually program the hardware--it'll happen in the flush.

2. In patch 4/4 (the flushing patch) add a call to rpmh_flush() to rpmh_invalidate() if you're in non-OSI mode. Presumably you'll need a spinlock around the rpmh_flush() call?

The end result of that will be that rpmh_invalidate() will properly leave the non-batch sleep/wake sets programmed.

-Doug

#6 lore raw

Re: [PATCH v12 4/4] soc: qcom: rpmh: Invalidate SLEEP and WAKE TCSes before flushing new data

Quoted 32 lines · Doug Anderson

Hi,

Quoted 17 lines · Maulik Shah

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")
---
 drivers/soc/qcom/rpmh.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/soc/qcom/rpmh.c +0 -0
index 1951f6a..63364ce 100644
@@ -472,6 +472,11 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
                return 0;

drivers/soc/qcom/rpmh.c int rpmh_flush(struct rpmh_ctrlr *ctrlr) · line 482

Quoted 10 lines
drivers/soc/qcom/rpmh.c +5 -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)

I think you should make this patch 3/4 instead of 4/4, and then:

Quoted 4 lines

1. In this patch remove the call to rpmh_rsc_invalidate() in rpmh_invalidate(). You've already marked things "dirty" in invalidate_batch() so no need to actually program the hardware--it'll happen in the flush.

Done.

Quoted 4 lines

2. In patch 4/4 (the flushing patch) add a call to rpmh_flush() to rpmh_invalidate() if you're in non-OSI mode. Presumably you'll need a spinlock around the rpmh_flush() call?

With (1) addressed and rpmh_start_transaction and rpmh_end_transaction introduced in v13, this is not required.

Thanks,
Maulik

Quoted 7 lines

The end result of that will be that rpmh_invalidate() will properly leave the non-batch sleep/wake sets programmed.

-Doug

Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
#7 lore raw
In reply to Maulik Shah

Quoting Maulik Shah (2020-03-05 09:06:59)

Quoted 2 lines

Changes in v12:
- Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it.

What is the difficulty in maintaining COMPILE_TEST support? If it's purely making a stub function for OSI support then I'm lost why we can't have another patch at the start of the series that makes the function return false when PSCI is disabled.

#8 lore raw
Quoted 7 lines · Stephen Boyd

Quoting Maulik Shah (2020-03-05 09:06:59)

Quoted 2 lines

Changes in v12:
- Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it.

What is the difficulty in maintaining COMPILE_TEST support? If it's purely making a stub function for OSI support then I'm lost why we can't have another patch at the start of the series that makes the function return false when PSCI is disabled.

Addressed in v13 to maintain COMPILE_TEST for rpmh driver.
Althought its done from rpmh-rsc driver itself rather thatn by not making a new stub function in PSCI.

Thanks,
Maulik

Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
#9 lore raw
In reply to Maulik Shah

Hi,

Quoted 70 lines · Maulik Shah

Changes in v12:
- Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it.

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

Quoted 8 lines

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(-)

One overall optimization idea?

Should we add two API calls:

rpmh_start_operations()
rpmh_end_operations()

These optional API calls would be an optimization a client could use. When rpmh_start_operations() is called then RPMH code will inhibit flushing (but will still update the "dirty" flag). When rpmh_end_operations() is called then the RPMH will flush if the dirty flag is set.

This is a pretty simple concept but should have a huge impact in the number of times we program hardware in non-OSI mode. Specifically, if we don't do that and we look at what happens in the interconnect code:

1. We "invalidate" the batch. We have to flush the non-batch commands back into the hardware.

2. We program the "wake only" commands. We have to flush the batch wake-only commands and also the non-batch commands back into the hardware.

3. We program the "sleep only" commands. We have to flush yet again with everything.

-Doug

#10 lore raw
Quoted 104 lines · Doug Anderson

Hi,

Quoted 69 lines · Maulik Shah

Changes in v12:
- Kconfig change to remove COMPILE_TEST was dropped in v11, reinclude it.

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

Quoted 8 lines

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(-)

One overall optimization idea?

Should we add two API calls:

rpmh_start_operations()
rpmh_end_operations()

These optional API calls would be an optimization a client could use. When rpmh_start_operations() is called then RPMH code will inhibit flushing (but will still update the "dirty" flag). When rpmh_end_operations() is called then the RPMH will flush if the dirty flag is set.

This is a pretty simple concept but should have a huge impact in the number of times we program hardware in non-OSI mode. Specifically, if we don't do that and we look at what happens in the interconnect code:

1. We "invalidate" the batch. We have to flush the non-batch commands back into the hardware.

2. We program the "wake only" commands. We have to flush the batch wake-only commands and also the non-batch commands back into the hardware.

Quoted 6 lines

3. We program the "sleep only" commands. We have to flush yet again with everything.

-Doug

Implemented in v13.

Thanks,
Maulik

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