2022-05-11 17:19:20

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 0/6] Add APSS RSC to Cluster power domain

Changes in v2:
- First four changes from v1 are already in linux-next, drop them
- Update dt-bindings change to yaml format
- Address Ulf's comments from v1 patches

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

.../bindings/soc/qcom/qcom,rpmh-rsc.yaml | 5 +
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 +
drivers/base/power/domain.c | 24 ++++
drivers/base/power/domain_governor.c | 1 +
drivers/soc/qcom/rpmh-internal.h | 9 +-
drivers/soc/qcom/rpmh-rsc.c | 146 +++++++++++++++++++--
drivers/soc/qcom/rpmh.c | 4 +-
include/linux/pm_domain.h | 7 +
11 files changed, 184 insertions(+), 16 deletions(-)

--
2.7.4



2022-05-11 18:34:42

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 3/6] 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]>
Reviewed-by: Ulf Hansson <[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 8ea44c4..9c30b65 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -4018,6 +4018,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 cf0c97b..5ba98c7 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -4939,6 +4939,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 743cba9..1a5f9b0 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -2004,6 +2004,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 7d08fad..9008f61 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2929,6 +2929,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-05-11 19:25:39

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 4/6] 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.

Add dev_pm_genpd_get_next_hrtimer() to get the genpd wakeup time.

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

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 18cd796..f0d70d0 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -487,6 +487,29 @@ void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
}
EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup);

+/**
+ * dev_pm_genpd_get_next_hrtimer - Return genpd domain next_hrtimer.
+ *
+ * @dev: Device to handle
+ *
+ * Returns the aggregated domain wakeup time for CPU PM domain
+ * when all the subdomains are off.
+ */
+ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ genpd = dev_to_genpd_safe(dev);
+ if (!genpd)
+ return KTIME_MAX;
+
+ if (atomic_read(&genpd->sd_count) > 0)
+ return KTIME_MAX;
+
+ return genpd->next_hrtimer;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_get_next_hrtimer);
+
static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
{
unsigned int state_idx = genpd->state_idx;
@@ -1998,6 +2021,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
genpd->max_off_time_changed = true;
genpd->provider = NULL;
genpd->has_provider = false;
+ genpd->next_hrtimer = KTIME_MAX;
genpd->accounting_time = ktime_get_mono_fast_ns();
genpd->domain.ops.runtime_suspend = genpd_runtime_suspend;
genpd->domain.ops.runtime_resume = genpd_runtime_resume;
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 043d48e..6d9fb79 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -17,6 +17,7 @@
#include <linux/notifier.h>
#include <linux/spinlock.h>
#include <linux/cpumask.h>
+#include <linux/time64.h>

/*
* Flags to control the behaviour of a genpd.
@@ -136,6 +137,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; /* Next hrtimer for the CPU PM domain */
bool max_off_time_changed;
bool cached_power_down_ok;
bool cached_power_down_state_idx;
@@ -228,6 +230,7 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
int dev_pm_genpd_remove_notifier(struct device *dev);
void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next);
+ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev);

extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
@@ -289,6 +292,10 @@ static inline int dev_pm_genpd_remove_notifier(struct device *dev)
static inline void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
{ }

+static inline ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
+{
+ return KTIME_MAX;
+}
#define simple_qos_governor (*(struct dev_power_governor *)(NULL))
#define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
#endif
--
2.7.4


2022-05-11 20:01:50

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 1/6] 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/qcom,rpmh-rsc.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
index f5ecf4a..7683cc9 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
@@ -110,6 +110,9 @@ properties:
- const: drv-2
- const: drv-3

+ power-domains:
+ maxItems: 1
+
bcm-voter:
$ref: /schemas/interconnect/qcom,bcm-voter.yaml#

@@ -162,6 +165,7 @@ examples:
<SLEEP_TCS 3>,
<WAKE_TCS 3>,
<CONTROL_TCS 1>;
+ power-domains = <&CLUSTER_PD>;
};

- |
@@ -208,6 +212,7 @@ examples:
<SLEEP_TCS 3>,
<WAKE_TCS 3>,
<CONTROL_TCS 0>;
+ power-domains = <&CLUSTER_PD>;

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


2022-05-11 22:07:34

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 6/6] 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 | 3 ++
drivers/soc/qcom/rpmh-rsc.c | 61 ++++++++++++++++++++++++++++++++++++++++
drivers/soc/qcom/rpmh.c | 4 ++-
3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 7866bb1..39f5358 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -112,6 +112,7 @@ struct rpmh_ctrlr {
* @tcs_wait: Wait queue used to wait for @tcs_in_use to free up a
* slot
* @client: Handle to the DRV's client.
+ * @dev: RSC device.
*/
struct rsc_drv {
const char *name;
@@ -127,12 +128,14 @@ struct rsc_drv {
spinlock_t lock;
wait_queue_head_t tcs_wait;
struct rpmh_ctrlr client;
+ struct device *dev;
};

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 8e01697..25b838b 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_nb.notifier_call)
+ 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 = dev_pm_genpd_get_next_hrtimer(drv->dev);
+
+ /* 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.
@@ -1035,6 +1095,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&drv->client.batch_cache);

dev_set_drvdata(&pdev->dev, drv);
+ drv->dev = &pdev->dev;

ret = devm_of_platform_populate(&pdev->dev);
if (ret && pdev->dev.pm_domain) {
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-05-12 04:45:26

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 5/6] 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]>
Reviewed-by: Ulf Hansson <[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 cd3d6ce..7866bb1 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.
@@ -114,6 +115,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 050b5f5c..8e01697 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -881,8 +881,7 @@ static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
return ret;
}

-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;
@@ -896,9 +895,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);
@@ -960,7 +959,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
@@ -987,11 +985,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;

@@ -1014,7 +1012,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-05-12 08:40:02

by Maulik Shah (mkshah)

[permalink] [raw]
Subject: [PATCH v2 2/6] 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 | 4 ++-
drivers/soc/qcom/rpmh-rsc.c | 67 +++++++++++++++++++++++++++++++++++++---
2 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 344ba68..cd3d6ce 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -97,7 +97,8 @@ 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.
* @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 +118,7 @@ struct rsc_drv {
int id;
int num_tcs;
struct notifier_block rsc_pm;
+ struct notifier_block genpd_nb;
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..050b5f5c 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,50 @@ 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 genpd 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);
+ drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
+ ret = dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
+ if (ret)
+ pm_runtime_disable(dev);
+
+ return ret;
+}
+
static int rpmh_probe_tcs_config(struct platform_device *pdev,
struct rsc_drv *drv, void __iomem *base)
{
@@ -963,7 +1010,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 +1018,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 (pdev->dev.pm_domain) {
+ 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 */
@@ -985,7 +1038,13 @@ static int rpmh_rsc_probe(struct platform_device *pdev)

dev_set_drvdata(&pdev->dev, drv);

- return devm_of_platform_populate(&pdev->dev);
+ ret = devm_of_platform_populate(&pdev->dev);
+ if (ret && pdev->dev.pm_domain) {
+ dev_pm_genpd_remove_notifier(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ }
+
+ return ret;
}

static const struct of_device_id rpmh_drv_match[] = {
--
2.7.4


2022-05-17 22:01:12

by Rob Herring

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

On Wed, 11 May 2022 18:46:51 +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/qcom,rpmh-rsc.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>

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

2022-05-20 01:52:23

by Ulf Hansson

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

On Wed, 11 May 2022 at 15:17, 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]>

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

Kind regards
Uffe

> ---
> drivers/soc/qcom/rpmh-internal.h | 4 ++-
> drivers/soc/qcom/rpmh-rsc.c | 67 +++++++++++++++++++++++++++++++++++++---
> 2 files changed, 66 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 344ba68..cd3d6ce 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -97,7 +97,8 @@ 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.
> * @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 +118,7 @@ struct rsc_drv {
> int id;
> int num_tcs;
> struct notifier_block rsc_pm;
> + struct notifier_block genpd_nb;
> 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..050b5f5c 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,50 @@ 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 genpd 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);
> + drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
> + ret = dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
> + if (ret)
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> static int rpmh_probe_tcs_config(struct platform_device *pdev,
> struct rsc_drv *drv, void __iomem *base)
> {
> @@ -963,7 +1010,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 +1018,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 (pdev->dev.pm_domain) {
> + 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 */
> @@ -985,7 +1038,13 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>
> dev_set_drvdata(&pdev->dev, drv);
>
> - return devm_of_platform_populate(&pdev->dev);
> + ret = devm_of_platform_populate(&pdev->dev);
> + if (ret && pdev->dev.pm_domain) {
> + dev_pm_genpd_remove_notifier(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + }
> +
> + return ret;
> }
>
> static const struct of_device_id rpmh_drv_match[] = {
> --
> 2.7.4
>

2022-05-20 09:44:26

by Ulf Hansson

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

On Wed, 11 May 2022 at 15:17, 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]>

I didn't forget to review this, but please allow me a few more days to
think a little bit more about this.

Kind regards
Uffe

> ---
> drivers/soc/qcom/rpmh-internal.h | 3 ++
> drivers/soc/qcom/rpmh-rsc.c | 61 ++++++++++++++++++++++++++++++++++++++++
> drivers/soc/qcom/rpmh.c | 4 ++-
> 3 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 7866bb1..39f5358 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -112,6 +112,7 @@ struct rpmh_ctrlr {
> * @tcs_wait: Wait queue used to wait for @tcs_in_use to free up a
> * slot
> * @client: Handle to the DRV's client.
> + * @dev: RSC device.
> */
> struct rsc_drv {
> const char *name;
> @@ -127,12 +128,14 @@ struct rsc_drv {
> spinlock_t lock;
> wait_queue_head_t tcs_wait;
> struct rpmh_ctrlr client;
> + struct device *dev;
> };
>
> 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 8e01697..25b838b 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_nb.notifier_call)
> + 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 = dev_pm_genpd_get_next_hrtimer(drv->dev);
> +
> + /* 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.
> @@ -1035,6 +1095,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
> INIT_LIST_HEAD(&drv->client.batch_cache);
>
> dev_set_drvdata(&pdev->dev, drv);
> + drv->dev = &pdev->dev;
>
> ret = devm_of_platform_populate(&pdev->dev);
> if (ret && pdev->dev.pm_domain) {
> 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-05-23 07:20:39

by Ulf Hansson

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

On Wed, 11 May 2022 at 15:17, 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]>

Kind regards
Uffe


> ---
> Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
> index f5ecf4a..7683cc9 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rpmh-rsc.yaml
> @@ -110,6 +110,9 @@ properties:
> - const: drv-2
> - const: drv-3
>
> + power-domains:
> + maxItems: 1
> +
> bcm-voter:
> $ref: /schemas/interconnect/qcom,bcm-voter.yaml#
>
> @@ -162,6 +165,7 @@ examples:
> <SLEEP_TCS 3>,
> <WAKE_TCS 3>,
> <CONTROL_TCS 1>;
> + power-domains = <&CLUSTER_PD>;
> };
>
> - |
> @@ -208,6 +212,7 @@ examples:
> <SLEEP_TCS 3>,
> <WAKE_TCS 3>,
> <CONTROL_TCS 0>;
> + power-domains = <&CLUSTER_PD>;
>
> clock-controller {
> compatible = "qcom,sm8350-rpmh-clk";
> --
> 2.7.4
>

2022-05-23 07:42:56

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] PM: domains: Store the closest hrtimer event of the domain CPUs

On Wed, 11 May 2022 at 15:17, 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.
>
> Add dev_pm_genpd_get_next_hrtimer() to get the genpd wakeup time.
>
> Signed-off-by: Maulik Shah <[email protected]>
> ---
> drivers/base/power/domain.c | 24 ++++++++++++++++++++++++
> drivers/base/power/domain_governor.c | 1 +
> include/linux/pm_domain.h | 7 +++++++
> 3 files changed, 32 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 18cd796..f0d70d0 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -487,6 +487,29 @@ void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
> }
> EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup);
>
> +/**
> + * dev_pm_genpd_get_next_hrtimer - Return genpd domain next_hrtimer.
> + *
> + * @dev: Device to handle
> + *
> + * Returns the aggregated domain wakeup time for CPU PM domain
> + * when all the subdomains are off.

To further clarify when this function should be used, I think that we
should state that it should typically be called from a consumer of a
genpd on/off-notifier at GENPD_NOTIFY_PRE_OFF. This also means that
the genpd's lock is being held across the function.

> + */
> +ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (!genpd)
> + return KTIME_MAX;
> +
> + if (atomic_read(&genpd->sd_count) > 0)
> + return KTIME_MAX;

This above isn't needed, assuming we clarify the description of the
function and when it should be called.

> +
> + return genpd->next_hrtimer;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_get_next_hrtimer);
> +
> static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
> {
> unsigned int state_idx = genpd->state_idx;
> @@ -1998,6 +2021,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
> genpd->max_off_time_changed = true;
> genpd->provider = NULL;
> genpd->has_provider = false;
> + genpd->next_hrtimer = KTIME_MAX;
> genpd->accounting_time = ktime_get_mono_fast_ns();
> genpd->domain.ops.runtime_suspend = genpd_runtime_suspend;
> genpd->domain.ops.runtime_resume = genpd_runtime_resume;
> 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;

There should be no point to set this, unless cpu_power_down_ok() are
returning true. Therefore I suggest you move this a few lines further
down, where cpu_power_down_ok() actually returns true.

>
> /* 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 043d48e..6d9fb79 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -17,6 +17,7 @@
> #include <linux/notifier.h>
> #include <linux/spinlock.h>
> #include <linux/cpumask.h>
> +#include <linux/time64.h>
>
> /*
> * Flags to control the behaviour of a genpd.
> @@ -136,6 +137,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; /* Next hrtimer for the CPU PM domain */
> bool max_off_time_changed;
> bool cached_power_down_ok;
> bool cached_power_down_state_idx;
> @@ -228,6 +230,7 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
> int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
> int dev_pm_genpd_remove_notifier(struct device *dev);
> void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next);
> +ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev);
>
> extern struct dev_power_governor simple_qos_governor;
> extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -289,6 +292,10 @@ static inline int dev_pm_genpd_remove_notifier(struct device *dev)
> static inline void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
> { }
>
> +static inline ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
> +{
> + return KTIME_MAX;
> +}
> #define simple_qos_governor (*(struct dev_power_governor *)(NULL))
> #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
> #endif
> --
> 2.7.4
>

Kind regards
Uffe

2022-06-08 10:35:21

by Ulf Hansson

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

On Fri, 20 May 2022 at 11:39, Ulf Hansson <[email protected]> wrote:
>
> On Wed, 11 May 2022 at 15:17, 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]>
>
> I didn't forget to review this, but please allow me a few more days to
> think a little bit more about this.

Apologize for the delay, this looks good to me!

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

Kind regards
Uffe

>
> > ---
> > drivers/soc/qcom/rpmh-internal.h | 3 ++
> > drivers/soc/qcom/rpmh-rsc.c | 61 ++++++++++++++++++++++++++++++++++++++++
> > drivers/soc/qcom/rpmh.c | 4 ++-
> > 3 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> > index 7866bb1..39f5358 100644
> > --- a/drivers/soc/qcom/rpmh-internal.h
> > +++ b/drivers/soc/qcom/rpmh-internal.h
> > @@ -112,6 +112,7 @@ struct rpmh_ctrlr {
> > * @tcs_wait: Wait queue used to wait for @tcs_in_use to free up a
> > * slot
> > * @client: Handle to the DRV's client.
> > + * @dev: RSC device.
> > */
> > struct rsc_drv {
> > const char *name;
> > @@ -127,12 +128,14 @@ struct rsc_drv {
> > spinlock_t lock;
> > wait_queue_head_t tcs_wait;
> > struct rpmh_ctrlr client;
> > + struct device *dev;
> > };
> >
> > 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 8e01697..25b838b 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_nb.notifier_call)
> > + 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 = dev_pm_genpd_get_next_hrtimer(drv->dev);
> > +
> > + /* 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.
> > @@ -1035,6 +1095,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
> > INIT_LIST_HEAD(&drv->client.batch_cache);
> >
> > dev_set_drvdata(&pdev->dev, drv);
> > + drv->dev = &pdev->dev;
> >
> > ret = devm_of_platform_populate(&pdev->dev);
> > if (ret && pdev->dev.pm_domain) {
> > 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-06-29 05:01:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] PM: domains: Store the closest hrtimer event of the domain CPUs

On Thu 19 May 07:56 CDT 2022, Ulf Hansson wrote:

> On Wed, 11 May 2022 at 15:17, 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.
> >
> > Add dev_pm_genpd_get_next_hrtimer() to get the genpd wakeup time.
> >
> > Signed-off-by: Maulik Shah <[email protected]>
> > ---
> > drivers/base/power/domain.c | 24 ++++++++++++++++++++++++
> > drivers/base/power/domain_governor.c | 1 +
> > include/linux/pm_domain.h | 7 +++++++
> > 3 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 18cd796..f0d70d0 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -487,6 +487,29 @@ void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next)
> > }
> > EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup);
> >
> > +/**
> > + * dev_pm_genpd_get_next_hrtimer - Return genpd domain next_hrtimer.
> > + *
> > + * @dev: Device to handle
> > + *
> > + * Returns the aggregated domain wakeup time for CPU PM domain
> > + * when all the subdomains are off.
>
> To further clarify when this function should be used, I think that we
> should state that it should typically be called from a consumer of a
> genpd on/off-notifier at GENPD_NOTIFY_PRE_OFF. This also means that
> the genpd's lock is being held across the function.
>

Maulik, it seems the only things remaining for this series it resolve
these three comments. Looking forward to be able to merge the next
revision.

Regards,
Bjorn

2022-07-04 16:17:37

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Add APSS RSC to Cluster power domain

On 11/05/2022 16:16, Maulik Shah wrote:
> Changes in v2:
> - First four changes from v1 are already in linux-next, drop them
> - Update dt-bindings change to yaml format
> - Address Ulf's comments from v1 patches
>
> 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

Tested-by: Dmitry Baryshkov <[email protected]> # SM8450

Also please note, that these patches fix the regression on sm8[1234]50,
which dates back to 5.18 (because the dts parts were merged at that
point). Amit has responded rpmh clock timeouts on RB5. On SM8450 we
observed random board stalls. Could you please describe this in the
cover letter and follow the process described in stable-kernel-rules.rst
to get these patches backported into 5.18/5.19. It would be critical to
get them in through the stable queue.

>
> Lina Iyer (1):
> soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain
>
> Maulik Shah (5):
> 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
>
> .../bindings/soc/qcom/qcom,rpmh-rsc.yaml | 5 +
> 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 +
> drivers/base/power/domain.c | 24 ++++
> drivers/base/power/domain_governor.c | 1 +
> drivers/soc/qcom/rpmh-internal.h | 9 +-
> drivers/soc/qcom/rpmh-rsc.c | 146 +++++++++++++++++++--
> drivers/soc/qcom/rpmh.c | 4 +-
> include/linux/pm_domain.h | 7 +
> 11 files changed, 184 insertions(+), 16 deletions(-)
>


--
With best wishes
Dmitry