From: Igor Paunovic <royalnet026@gmail.com>
To: Tomeu Vizoso <tomeu@tomeuvizoso.net>,
Oded Gabbay <ogabbay@kernel.org>,
Heiko Stuebner <heiko@sntech.de>
Cc: Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Sidong Yang <sidong.yang@furiosa.ai>,
Diederik de Haas <diederik@cknow-tech.com>,
Sebastian Reichel <sebastian.reichel@collabora.com>,
Jiaxing Hu <gahing@gahingwoo.com>,
Nicolas Dufresne <nicolas@ndufresne.ca>,
Jonas Karlman <jonas@kwiboo.se>,
dri-devel@lists.freedesktop.org,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
Igor Paunovic <royalnet026@gmail.com>
Subject: [PATCH 4/7] accel/rocket: restore the NPU clock boot rate before powering the cores down
Date: Fri, 4 Sep 2026 15:08:55 +0200 [thread overview]
Message-ID: <20260904130858.27803-5-royalnet026@gmail.com> (raw)
In-Reply-To: <20260904130858.27803-1-royalnet026@gmail.com>
The compute clock is generated by a PVTPLL that lives inside the NPU power
island. Powering an island up while that clock is above the rate the
bootloader left it at does not work: the domain never acks the power-on,
and the first register access into it afterwards takes an asynchronous
SError. So the rate has to be back down before the last core goes away.
Nothing in the driver raises the clock today, which makes this a no-op on
its own, but it is the guard that has to be in the tree before anything
does, and the next patches do. The .shutdown hook is the same guard for the
handover: once devfreq is driving the clock, a reboot or a kexec would
otherwise pass the raised rate to the next kernel, which powers the islands
up before it looks at it. What this cannot do is rescue a rate it did not
set - the rate read at probe is taken as the boot rate whatever it is.
The rate is read at probe rather than hardcoded. Mainline pins the RK3588
cores at 200 MHz with assigned-clock-rates, but that is a devicetree
property, not a property of the hardware, and a SoC whose devicetree does
not set it would be left running at a rate this driver had invented.
All three cores share the clock, so only the last core to suspend may lower
it; the others just drop the count. Lowering it is safe with the islands
already down, because the firmware serves the boot rate from GPLL and
writes only CRU clock selectors to get there, never a register inside the
NPU.
Signed-off-by: Igor Paunovic <royalnet026@gmail.com>
Assisted-by: LLM sparse checkpatch
---
drivers/accel/rocket/rocket_core.c | 12 +++++++++
drivers/accel/rocket/rocket_device.h | 10 +++++++
drivers/accel/rocket/rocket_drv.c | 40 ++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index 5dd260bacbff6..61200e5d5ac0d 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -12,6 +12,7 @@
#include <linux/reset.h>
#include "rocket_core.h"
+#include "rocket_device.h"
#include "rocket_job.h"
int rocket_core_init(struct rocket_core *core)
@@ -36,6 +37,17 @@ int rocket_core_init(struct rocket_core *core)
if (err)
return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
+ /*
+ * Record what the compute clock was running at before anything here
+ * touched it, on the first core to probe. Reading it rather than
+ * hardcoding a rate keeps this working on a SoC whose devicetree does
+ * not pin the clock with assigned-clock-rates.
+ */
+ if (!core->rdev->npu_clk) {
+ core->rdev->npu_clk = core->clks[2].clk;
+ core->rdev->npu_boot_rate = clk_get_rate(core->rdev->npu_clk);
+ }
+
core->pc_iomem = devm_platform_ioremap_resource_byname(pdev, "pc");
if (IS_ERR(core->pc_iomem)) {
dev_err(dev, "couldn't find PC registers %ld\n", PTR_ERR(core->pc_iomem));
diff --git a/drivers/accel/rocket/rocket_device.h b/drivers/accel/rocket/rocket_device.h
index c62d567010696..466ebc4c26a8a 100644
--- a/drivers/accel/rocket/rocket_device.h
+++ b/drivers/accel/rocket/rocket_device.h
@@ -20,6 +20,16 @@ struct rocket_device {
struct rocket_core *cores;
unsigned int num_cores;
unsigned int max_cores;
+
+ /*
+ * The cores have no clock of their own: one clock feeds all of them,
+ * so any core's handle refers to the same thing. npu_boot_rate is the
+ * rate it was left at before the driver touched it, and active_cores
+ * counts the cores that are runtime resumed right now.
+ */
+ struct clk *npu_clk;
+ unsigned long npu_boot_rate;
+ atomic_t active_cores;
};
struct rocket_device *rocket_device_init(struct platform_device *pdev,
diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 2bcfe4ab3c68f..b7199de57ccc7 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -231,6 +231,28 @@ static int find_core_for_dev(struct device *dev)
return -1;
}
+/*
+ * Put the compute clock back where the bootloader had it. The cores share
+ * this clock, so this is only correct once none of them is running any more.
+ *
+ * Lowering the rate is safe with the power islands down: the firmware serves
+ * the boot rate from GPLL and touches only the CRU clock selectors on the way
+ * there, none of the NPU's own registers.
+ */
+static void rocket_npu_restore_boot_rate(struct rocket_device *rdev)
+{
+ int err;
+
+ if (!rdev->npu_clk)
+ return;
+
+ err = clk_set_rate(rdev->npu_clk, rdev->npu_boot_rate);
+ if (err)
+ dev_warn(rdev->cores[0].dev,
+ "failed to restore the NPU boot rate of %lu Hz: %d\n",
+ rdev->npu_boot_rate, err);
+}
+
static int rocket_device_runtime_resume(struct device *dev)
{
struct rocket_device *rdev = dev_get_drvdata(dev);
@@ -246,6 +268,8 @@ static int rocket_device_runtime_resume(struct device *dev)
return err;
}
+ atomic_inc(&rdev->active_cores);
+
return 0;
}
@@ -262,6 +286,9 @@ static int rocket_device_runtime_suspend(struct device *dev)
clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+ if (atomic_dec_and_test(&rdev->active_cores))
+ rocket_npu_restore_boot_rate(rdev);
+
return 0;
}
@@ -270,9 +297,22 @@ EXPORT_GPL_DEV_PM_OPS(rocket_pm_ops) = {
SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
};
+/*
+ * A kexec or a reboot hands the next kernel whatever rate is set here, and
+ * that kernel will power the islands up before it looks at the clock.
+ */
+static void rocket_shutdown(struct platform_device *pdev)
+{
+ struct rocket_device *rdev = dev_get_drvdata(&pdev->dev);
+
+ if (rdev)
+ rocket_npu_restore_boot_rate(rdev);
+}
+
static struct platform_driver rocket_driver = {
.probe = rocket_probe,
.remove = rocket_remove,
+ .shutdown = rocket_shutdown,
.driver = {
.name = "rocket",
.pm = pm_ptr(&rocket_pm_ops),
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2026-09-04 13:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 13:08 [PATCH 0/7] accel/rocket: DVFS for the RK3588 NPU Igor Paunovic
2026-09-04 13:08 ` [PATCH 1/7] accel/rocket: request the core clocks by name Igor Paunovic
2026-09-04 13:08 ` [PATCH 2/7] dt-bindings: npu: rockchip: allow DVFS and thermal properties Igor Paunovic
2026-09-04 15:11 ` Conor Dooley
2026-09-04 13:08 ` [PATCH 3/7] arm64: dts: rockchip: rk3588: add an OPP table for the NPU Igor Paunovic
2026-09-04 13:08 ` Igor Paunovic [this message]
2026-09-04 13:08 ` [PATCH 5/7] accel/rocket: add devfreq support Igor Paunovic
2026-09-04 13:08 ` [PATCH 6/7] accel/rocket: register a devfreq cooling device Igor Paunovic
2026-09-04 13:08 ` [PATCH 7/7] arm64: dts: rockchip: rk3588: add passive cooling to the NPU thermal zone Igor Paunovic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260904130858.27803-5-royalnet026@gmail.com \
--to=royalnet026@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=diederik@cknow-tech.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gahing@gahingwoo.com \
--cc=heiko@sntech.de \
--cc=jonas@kwiboo.se \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=nicolas@ndufresne.ca \
--cc=ogabbay@kernel.org \
--cc=robh@kernel.org \
--cc=sebastian.reichel@collabora.com \
--cc=sidong.yang@furiosa.ai \
--cc=tomeu@tomeuvizoso.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).