Re: [PATCHv4 11/14] remoteproc/omap: add support for system suspend/resume
From: Tero Kristo
Date: Wed Jan 15 2020 - 08:20:55 EST
On 08/01/2020 23:42, Suman Anna wrote:
On 1/8/20 1:57 PM, Suman Anna wrote:
On 1/2/20 7:18 AM, Tero Kristo wrote:
From: Suman Anna <s-anna@xxxxxx>
This patch adds the support for system suspend/resume to the
OMAP remoteproc driver so that the OMAP remoteproc devices can
be suspended/resumed during a system suspend/resume. The support
is added through the driver PM .suspend/.resume callbacks, and
requires appropriate support from the OS running on the remote
processors.
The IPU & DSP remote processors typically have their own private
modules like registers, internal memories, caches etc. The context
of these modules need to be saved and restored properly for a
suspend/resume to work. These are in general not accessible from
the MPU, so the remote processors themselves have to implement
the logic for the context save & restore of these modules.
The OMAP remoteproc driver initiates a suspend by sending a mailbox
message requesting the remote processor to save its context and
enter into an idle/standby state. The remote processor should
usually stop whatever processing it is doing to switch to a context
save mode. The OMAP remoteproc driver detects the completion of
the context save by checking the module standby status for the
remoteproc device. It also stops any resources used by the remote
processors like the timers. The timers need to be running only
when the processor is active and executing, and need to be stopped
otherwise to allow the timer driver to reach low-power states. The
IOMMUs are automatically suspended by the PM core during the late
suspend stage, after the remoteproc suspend process is completed by
putting the remote processor cores into reset. Thereafter, the Linux
kernel can put the domain into further lower power states as possible.
The resume sequence undoes the operations performed in the PM suspend
callback, by starting the timers and finally releasing the processors
from reset. This requires that the remote processor side OS be able to
distinguish a power-resume boot from a power-on/cold boot, restore the
context of its private modules saved during the suspend phase, and
resume executing code from where it was suspended. The IOMMUs would
have been resumed by the PM core during early resume, so they are
already enabled by the time remoteproc resume callback gets invoked.
The remote processors should save their context into System RAM (DDR),
as any internal memories are not guaranteed to retain context as it
depends on the lowest power domain that the remote processor device
is put into. The management of the DDR contents will be managed by
the Linux kernel.
Signed-off-by: Suman Anna <s-anna@xxxxxx>
[t-kristo@xxxxxx: converted to use ti-sysc instead of hwmod]
Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
v4:
- include headers sorted by name
- using #ifdef CONFIG_PM_SLEEP for suspend/resume ops
Yeah, guess you missed my response comment on this on v3. This is
actually old-style, so original code in v3 is good on this one.
Ok, will revert it back to v3 way of doing things.
regards
Suman
drivers/remoteproc/omap_remoteproc.c | 181 +++++++++++++++++++++++++++
drivers/remoteproc/omap_remoteproc.h | 18 ++-
2 files changed, 197 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index fd4700861a14..2a333565a9c1 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -15,13 +15,17 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/clk/ti.h>
#include <linux/err.h>
+#include <linux/io.h>
#include <linux/of_device.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/remoteproc.h>
#include <linux/mailbox_client.h>
+#include <linux/omap-iommu.h>
#include <linux/omap-mailbox.h>
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
@@ -81,6 +85,9 @@ struct omap_rproc_timer {
* @timers: timer(s) info used by rproc
* @rproc: rproc handle
* @reset: reset handle
+ * @pm_comp: completion primitive to sync for suspend response
+ * @fck: functional clock for the remoteproc
+ * @suspend_acked: state machine flag to store the suspend request ack
*/
struct omap_rproc {
struct mbox_chan *mbox;
@@ -92,6 +99,9 @@ struct omap_rproc {
struct omap_rproc_timer *timers;
struct rproc *rproc;
struct reset_control *reset;
+ struct completion pm_comp;
+ struct clk *fck;
+ bool suspend_acked;
};
/**
@@ -366,6 +376,12 @@ static void omap_rproc_mbox_callback(struct mbox_client *client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SUSPEND_ACK:
+ /* Fall through */
+ case RP_MBOX_SUSPEND_CANCEL:
+ oproc->suspend_acked = msg == RP_MBOX_SUSPEND_ACK;
+ complete(&oproc->pm_comp);
+ break;
default:
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
return;
@@ -554,6 +570,158 @@ static const struct rproc_ops omap_rproc_ops = {
.da_to_va = omap_rproc_da_to_va,
};
+#ifdef CONFIG_PM
+static bool _is_rproc_in_standby(struct omap_rproc *oproc)
+{
+ return ti_clk_is_in_standby(oproc->fck);
+}
+
+/* 1 sec is long enough time to let the remoteproc side suspend the device */
+#define DEF_SUSPEND_TIMEOUT 1000
+static int _omap_rproc_suspend(struct rproc *rproc)
+{
+ struct device *dev = rproc->dev.parent;
+ struct omap_rproc *oproc = rproc->priv;
+ unsigned long to = msecs_to_jiffies(DEF_SUSPEND_TIMEOUT);
+ unsigned long ta = jiffies + to;
+ int ret;
+
+ reinit_completion(&oproc->pm_comp);
+ oproc->suspend_acked = false;
+ ret = mbox_send_message(oproc->mbox, (void *)RP_MBOX_SUSPEND_SYSTEM);
+ if (ret < 0) {
+ dev_err(dev, "PM mbox_send_message failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = wait_for_completion_timeout(&oproc->pm_comp, to);
+ if (!oproc->suspend_acked)
+ return -EBUSY;
+
+ /*
+ * The remoteproc side is returning the ACK message before saving the
+ * context, because the context saving is performed within a SYS/BIOS
+ * function, and it cannot have any inter-dependencies against the IPC
+ * layer. Also, as the SYS/BIOS needs to preserve properly the processor
+ * register set, sending this ACK or signalling the completion of the
+ * context save through a shared memory variable can never be the
+ * absolute last thing to be executed on the remoteproc side, and the
+ * MPU cannot use the ACK message as a sync point to put the remoteproc
+ * into reset. The only way to ensure that the remote processor has
+ * completed saving the context is to check that the module has reached
+ * STANDBY state (after saving the context, the SYS/BIOS executes the
+ * appropriate target-specific WFI instruction causing the module to
+ * enter STANDBY).
+ */
+ while (!_is_rproc_in_standby(oproc)) {
+ if (time_after(jiffies, ta))
+ return -ETIME;
+ schedule();
+ }
+
+ reset_control_assert(oproc->reset);
+
+ ret = omap_rproc_disable_timers(rproc, false);
+ if (ret) {
+ dev_err(dev, "disabling timers during suspend failed %d\n",
+ ret);
+ goto enable_device;
+ }
+
+ return 0;
+
+enable_device:
+ reset_control_deassert(oproc->reset);
+ return ret;
+}
+
+static int _omap_rproc_resume(struct rproc *rproc)
+{
+ struct device *dev = rproc->dev.parent;
+ struct omap_rproc *oproc = rproc->priv;
+ int ret;
+
+ /* boot address could be lost after suspend, so restore it */
+ if (oproc->boot_data) {
+ ret = omap_rproc_write_dsp_boot_addr(rproc);
+ if (ret) {
+ dev_err(dev, "boot address restore failed %d\n", ret);
+ goto out;
+ }
+ }
+
+ ret = omap_rproc_enable_timers(rproc, false);
+ if (ret) {
+ dev_err(dev, "enabling timers during resume failed %d\n", ret);
+ goto out;
+ }
+
+ reset_control_deassert(oproc->reset);
So, I actually did not comment specifically on this on v3 hoping that my
comment on the lack of error checking in v3 Patch 2 would have been
taken care of here as well.
Ok, will add error handling on this one.
-Tero
Please do compare the overall code against our current downstream
release code as well. I still see some deltas, I have to double-check
the behavior as well once with the auto-suspend patch. Probably will do
that on the next version.
regards
Suman
+
+out:
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int omap_rproc_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ int ret = 0;
+
+ mutex_lock(&rproc->lock);
+ if (rproc->state == RPROC_OFFLINE)
+ goto out;
+
+ if (rproc->state == RPROC_SUSPENDED)
+ goto out;
+
+ if (rproc->state != RPROC_RUNNING) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ret = _omap_rproc_suspend(rproc);
+ if (ret) {
+ dev_err(dev, "suspend failed %d\n", ret);
+ goto out;
+ }
+
+ rproc->state = RPROC_SUSPENDED;
+out:
+ mutex_unlock(&rproc->lock);
+ return ret;
+}
+
+static int omap_rproc_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ int ret = 0;
+
+ mutex_lock(&rproc->lock);
+ if (rproc->state == RPROC_OFFLINE)
+ goto out;
+
+ if (rproc->state != RPROC_SUSPENDED) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ret = _omap_rproc_resume(rproc);
+ if (ret) {
+ dev_err(dev, "resume failed %d\n", ret);
+ goto out;
+ }
+
+ rproc->state = RPROC_RUNNING;
+out:
+ mutex_unlock(&rproc->lock);
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
+
static const struct omap_rproc_mem_data ipu_mems[] = {
{ .name = "l2ram", .dev_addr = 0x20000000 },
{ },
@@ -796,6 +964,14 @@ static int omap_rproc_probe(struct platform_device *pdev)
oproc->num_timers);
}
+ init_completion(&oproc->pm_comp);
+
+ oproc->fck = devm_clk_get(&pdev->dev, 0);
+ if (IS_ERR(oproc->fck)) {
+ ret = PTR_ERR(oproc->fck);
+ goto free_rproc;
+ }
+
ret = of_reserved_mem_device_init(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "device does not have specific CMA pool\n");
@@ -828,11 +1004,16 @@ static int omap_rproc_remove(struct platform_device *pdev)
return 0;
}
+static const struct dev_pm_ops omap_rproc_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(omap_rproc_suspend, omap_rproc_resume)
+};
+
static struct platform_driver omap_rproc_driver = {
.probe = omap_rproc_probe,
.remove = omap_rproc_remove,
.driver = {
.name = "omap-rproc",
+ .pm = &omap_rproc_pm_ops,
.of_match_table = omap_rproc_of_match,
},
};
diff --git a/drivers/remoteproc/omap_remoteproc.h b/drivers/remoteproc/omap_remoteproc.h
index 72f656c93caa..8af37e46aa73 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -1,7 +1,7 @@
/*
* Remote processor messaging
*
- * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011-2020 Texas Instruments, Inc.
* Copyright (C) 2011 Google, Inc.
* All rights reserved.
*
@@ -57,6 +57,16 @@
* @RP_MBOX_ABORT_REQUEST: a "please crash" request, used for testing the
* recovery mechanism (to some extent).
*
+ * @RP_MBOX_SUSPEND_AUTO: auto suspend request for the remote processor
+ *
+ * @RP_MBOX_SUSPEND_SYSTEM: system suspend request for the remote processor
+ *
+ * @RP_MBOX_SUSPEND_ACK: successful response from remote processor for a
+ * suspend request
+ *
+ * @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
+ * on a suspend request
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -70,7 +80,11 @@ enum omap_rp_mbox_messages {
RP_MBOX_ECHO_REQUEST = 0xFFFFFF03,
RP_MBOX_ECHO_REPLY = 0xFFFFFF04,
RP_MBOX_ABORT_REQUEST = 0xFFFFFF05,
- RP_MBOX_END_MSG = 0xFFFFFF06,
+ RP_MBOX_SUSPEND_AUTO = 0xFFFFFF10,
+ RP_MBOX_SUSPEND_SYSTEM = 0xFFFFFF11,
+ RP_MBOX_SUSPEND_ACK = 0xFFFFFF12,
+ RP_MBOX_SUSPEND_CANCEL = 0xFFFFFF13,
+ RP_MBOX_END_MSG = 0xFFFFFF14,
};
#endif /* _OMAP_RPMSG_H */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki