diff mbox series

[v3,2/3] cpufreq: qcom-nvmem: Preserve PM domain votes in system suspend

Message ID 20231114-msm8909-cpufreq-v3-2-926097a6e5c1@kernkonzept.com
State New
Headers
Series cpufreq: qcom-nvmem: Fix power domain scaling |

Commit Message

Stephan Gerhold Nov. 14, 2023, 10:07 a.m. UTC 
  From the Linux point of view, the power domains used by the CPU must
stay always-on. This is because we still need the CPU to keep running
until the last instruction, which will typically be a firmware call that
shuts down the CPU cleanly.

At the moment the power domain votes (enable + performance state) are
dropped during system suspend, which means the CPU could potentially
malfunction while entering suspend.

We need to distinguish between two different setups used with
qcom-cpufreq-nvmem:

 1. CPR power domain: The backing regulator used by CPR should stay
    always-on in Linux; it is typically disabled automatically by
    hardware when the CPU enters a deep idle state. However, we
    should pause the CPR state machine during system suspend.

 2. RPMPD: The power domains used by the CPU should stay always-on
    in Linux (also across system suspend). The CPU typically only
    uses the *_AO ("active-only") variants of the power domains in
    RPMPD. For those, the RPM firmware will automatically drop
    the votes internally when the CPU enters a deep idle state.

Make this work correctly by calling device_set_awake_path() on the
virtual genpd devices, so that the votes are maintained across system
suspend. The power domain drivers need to set GENPD_FLAG_ACTIVE_WAKEUP
to opt into staying on during system suspend.

For now we only set this for the RPMPD case. For CPR, not setting it
will ensure the state machine is still paused during system suspend,
while the backing regulator will stay on with "regulator-always-on".

Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
---
This patch can be merged independently from the pmdomain one for RPMPD.
Both are needed to actually preserve the votes during system suspend but
there is no compile-time dependency.
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Ulf Hansson Nov. 22, 2023, 9:48 a.m. UTC | #1 
On Tue, 14 Nov 2023 at 11:08, Stephan Gerhold
<stephan.gerhold@kernkonzept.com> wrote:
>
> From the Linux point of view, the power domains used by the CPU must
> stay always-on. This is because we still need the CPU to keep running
> until the last instruction, which will typically be a firmware call that
> shuts down the CPU cleanly.
>
> At the moment the power domain votes (enable + performance state) are
> dropped during system suspend, which means the CPU could potentially
> malfunction while entering suspend.
>
> We need to distinguish between two different setups used with
> qcom-cpufreq-nvmem:
>
>  1. CPR power domain: The backing regulator used by CPR should stay
>     always-on in Linux; it is typically disabled automatically by
>     hardware when the CPU enters a deep idle state. However, we
>     should pause the CPR state machine during system suspend.
>
>  2. RPMPD: The power domains used by the CPU should stay always-on
>     in Linux (also across system suspend). The CPU typically only
>     uses the *_AO ("active-only") variants of the power domains in
>     RPMPD. For those, the RPM firmware will automatically drop
>     the votes internally when the CPU enters a deep idle state.
>
> Make this work correctly by calling device_set_awake_path() on the
> virtual genpd devices, so that the votes are maintained across system
> suspend. The power domain drivers need to set GENPD_FLAG_ACTIVE_WAKEUP
> to opt into staying on during system suspend.
>
> For now we only set this for the RPMPD case. For CPR, not setting it
> will ensure the state machine is still paused during system suspend,
> while the backing regulator will stay on with "regulator-always-on".
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>

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

Kind regards
Uffe


> ---
> This patch can be merged independently from the pmdomain one for RPMPD.
> Both are needed to actually preserve the votes during system suspend but
> there is no compile-time dependency.
> ---
>  drivers/cpufreq/qcom-cpufreq-nvmem.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> index d239a45ed497..ea05d9d67490 100644
> --- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
> @@ -23,6 +23,7 @@
>  #include <linux/nvmem-consumer.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_opp.h>
>  #include <linux/pm_runtime.h>
> @@ -426,6 +427,18 @@ static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
>         .get_version = qcom_cpufreq_ipq8074_name_version,
>  };
>
> +static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
> +{
> +       const char * const *name = drv->data->genpd_names;
> +       int i;
> +
> +       if (!drv->cpus[cpu].virt_devs)
> +               return;
> +
> +       for (i = 0; *name; i++, name++)
> +               device_set_awake_path(drv->cpus[cpu].virt_devs[i]);
> +}
> +
>  static void qcom_cpufreq_put_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
>  {
>         const char * const *name = drv->data->genpd_names;
> @@ -578,11 +591,25 @@ static void qcom_cpufreq_remove(struct platform_device *pdev)
>         }
>  }
>
> +static int qcom_cpufreq_suspend(struct device *dev)
> +{
> +       struct qcom_cpufreq_drv *drv = dev_get_drvdata(dev);
> +       unsigned int cpu;
> +
> +       for_each_possible_cpu(cpu)
> +               qcom_cpufreq_suspend_virt_devs(drv, cpu);
> +
> +       return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(qcom_cpufreq_pm_ops, qcom_cpufreq_suspend, NULL);
> +
>  static struct platform_driver qcom_cpufreq_driver = {
>         .probe = qcom_cpufreq_probe,
>         .remove_new = qcom_cpufreq_remove,
>         .driver = {
>                 .name = "qcom-cpufreq-nvmem",
> +               .pm = pm_sleep_ptr(&qcom_cpufreq_pm_ops),
>         },
>  };
>
>
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index d239a45ed497..ea05d9d67490 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -23,6 +23,7 @@ 
 #include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
@@ -426,6 +427,18 @@  static const struct qcom_cpufreq_match_data match_data_ipq8074 = {
 	.get_version = qcom_cpufreq_ipq8074_name_version,
 };
 
+static void qcom_cpufreq_suspend_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
+{
+	const char * const *name = drv->data->genpd_names;
+	int i;
+
+	if (!drv->cpus[cpu].virt_devs)
+		return;
+
+	for (i = 0; *name; i++, name++)
+		device_set_awake_path(drv->cpus[cpu].virt_devs[i]);
+}
+
 static void qcom_cpufreq_put_virt_devs(struct qcom_cpufreq_drv *drv, unsigned int cpu)
 {
 	const char * const *name = drv->data->genpd_names;
@@ -578,11 +591,25 @@  static void qcom_cpufreq_remove(struct platform_device *pdev)
 	}
 }
 
+static int qcom_cpufreq_suspend(struct device *dev)
+{
+	struct qcom_cpufreq_drv *drv = dev_get_drvdata(dev);
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu)
+		qcom_cpufreq_suspend_virt_devs(drv, cpu);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(qcom_cpufreq_pm_ops, qcom_cpufreq_suspend, NULL);
+
 static struct platform_driver qcom_cpufreq_driver = {
 	.probe = qcom_cpufreq_probe,
 	.remove_new = qcom_cpufreq_remove,
 	.driver = {
 		.name = "qcom-cpufreq-nvmem",
+		.pm = pm_sleep_ptr(&qcom_cpufreq_pm_ops),
 	},
 };