* [RFC PATCH v5 v5 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 6:38 ` [RFC PATCH v5 v5 2/8] dt-bindings: power: rockchip: allow resets in a power domain node Jiaxing Hu
` (7 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
The RK3576 NPU has two cores of the same RKNN block the RK3588 binding
already describes, but it wires them up differently: two extra CBUF
clocks, two power domains per core, and a single reset instead of two.
It also has no NPU SRAM supply.
Widen the property ranges to cover both, then pin each SoC back to its
own shape in allOf so nothing loosens for RK3588, and keep sram-supply
required for rockchip,rk3588-rknn-core only.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
.../npu/rockchip,rk3588-rknn-core.yaml | 47 +++++++++++++++++--
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
index caca2a490..3b611b64c 100644
--- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
+++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
@@ -21,6 +21,7 @@ properties:
compatible:
enum:
+ - rockchip,rk3576-rknn-core
- rockchip,rk3588-rknn-core
reg:
@@ -33,14 +34,18 @@ properties:
- const: core # Main NPU core processing unit registers
clocks:
- maxItems: 4
+ minItems: 4
+ maxItems: 6
clock-names:
+ minItems: 4
items:
- const: aclk
- const: hclk
- const: npu
- const: pclk
+ - const: aclk_cbuf
+ - const: hclk_cbuf
interrupts:
maxItems: 1
@@ -51,12 +56,15 @@ properties:
npu-supply: true
power-domains:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
resets:
+ minItems: 1
maxItems: 2
reset-names:
+ minItems: 1
items:
- const: srst_a
- const: srst_h
@@ -75,7 +83,40 @@ required:
- resets
- reset-names
- npu-supply
- - sram-supply
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: rockchip,rk3588-rknn-core
+ then:
+ properties:
+ clocks:
+ maxItems: 4
+ clock-names:
+ maxItems: 4
+ power-domains:
+ maxItems: 1
+ resets:
+ minItems: 2
+ reset-names:
+ minItems: 2
+ required:
+ - sram-supply
+ else:
+ properties:
+ clocks:
+ minItems: 6
+ clock-names:
+ minItems: 6
+ power-domains:
+ minItems: 2
+ resets:
+ maxItems: 1
+ reset-names:
+ maxItems: 1
+ sram-supply: false
additionalProperties: false
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC PATCH v5 v5 2/8] dt-bindings: power: rockchip: allow resets in a power domain node
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
2026-08-05 6:38 ` [RFC PATCH v5 v5 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 6:38 ` [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set Jiaxing Hu
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
Some domains do not come up in a usable state on their own and need
their resets cycled once power is on. The RK3576 NPU domains are one
case: without it the first access after power-on takes an async SError.
pd-node has no resets property and every nesting level is
unevaluatedProperties: false, so describing that in DT is rejected
today. Add it alongside clocks.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
.../bindings/power/rockchip,power-controller.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
index b41db576f..f23c1a118 100644
--- a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -136,6 +136,14 @@ $defs:
A number of phandles to clocks that need to be enabled
while power domain switches state.
+ resets:
+ minItems: 1
+ maxItems: 30
+ description: |
+ A number of phandles to resets that need to be cycled once the power
+ domain has been switched on, for domains whose logic does not come up
+ in a usable state by itself.
+
domain-supply:
description: domain regulator supply.
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
2026-08-05 6:38 ` [RFC PATCH v5 v5 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
2026-08-05 6:38 ` [RFC PATCH v5 v5 2/8] dt-bindings: power: rockchip: allow resets in a power domain node Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 7:10 ` sashiko-bot
2026-08-05 6:38 ` [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
` (5 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
The RK3576 NPU MMUs need more than aclk and iface. With only those two
enabled the MMU accepts reads but silently drops register writes: a
DTE_ADDR value written from the power domain, while the domain clocks
are still on, reads back correctly, and the write rk_iommu_resume() does
microseconds later does not land at all. The vendor DT names the CBUF
clocks as that MMU's interface clocks and its driver keeps every NPU
clock on for as long as the device is powered.
The driver side of this is already upstream, commit 841363ebb508
("iommu/rockchip: Take all DT clocks"), which switched rk_iommu to
devm_clk_bulk_get_all(). Widen the schema to match so those nodes can
be described. minItems stays at 2, so every existing devicetree, which
all carry exactly aclk and iface, is unaffected.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
.../devicetree/bindings/iommu/rockchip,iommu.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
index 6ce41d11f..a3cedcaaa 100644
--- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
@@ -42,14 +42,22 @@ properties:
minItems: 1
clocks:
+ minItems: 2
items:
- description: Core clock
- description: Interface clock
+ - description: Compute clock, RK3576 NPU MMUs only
+ - description: Convolution buffer core clock, RK3576 NPU MMUs only
+ - description: Convolution buffer interface clock, RK3576 NPU MMUs only
clock-names:
+ minItems: 2
items:
- const: aclk
- const: iface
+ - const: npu
+ - const: aclk_cbuf
+ - const: hclk_cbuf
"#iommu-cells":
const: 0
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set
2026-08-05 6:38 ` [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set Jiaxing Hu
@ 2026-08-05 7:10 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-05 7:10 UTC (permalink / raw)
To: Jiaxing Hu; +Cc: robh, conor+dt, dri-devel, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Adding the three NPU clocks globally relaxes the maximum allowed clocks to 5 for all Rockchip IOMMUs, disabling strict validation for older hardware.
- [Medium] The failure to introduce a distinct compatible string for the NPU MMU hardware variant prevents the schema from enforcing its mandatory extra clocks.
--
--- Patch [3]: [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set ---
commit 1dfa4f75475ad14a765f82a9e6d6d985e8af62dc
Author: Jiaxing Hu <gahing@gahingwoo.com>
dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set
This commit expands the clocks and clock-names properties in the Rockchip IOMMU
binding to allow up to 5 items to support the RK3576 NPU MMU. It maintains a
minimum item count of 2 to keep existing device trees valid.
> diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> index 6ce41d11ff5e5..a3cedcaaa9792 100644
> --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> @@ -42,14 +42,22 @@ properties:
> minItems: 1
>
> clocks:
> + minItems: 2
> items:
> - description: Core clock
> - description: Interface clock
> + - description: Compute clock, RK3576 NPU MMUs only
> + - description: Convolution buffer core clock, RK3576 NPU MMUs only
> + - description: Convolution buffer interface clock, RK3576 NPU MMUs only
[Severity: Medium]
Does expanding the items list globally cause a regression by relaxing the
maximum allowed clocks to 5 for all Rockchip IOMMUs?
If older hardware variants specify 3, 4, or 5 clocks in their device trees,
they will now silently pass schema validation. Would it be better to restrict
the maximum clocks to 2 for older variants using an allOf: if-then
conditional block?
Additionally, because the generic rockchip,rk3576-iommu compatible string is
used for both standard and NPU MMUs, the schema forces minItems: 2 globally.
Does the lack of a distinct compatible string for the NPU MMU hardware variant
introduce a regression where the schema is prevented from enforcing its
mandatory extra clocks?
If a device tree node for the RK3576 NPU MMU is missing its required extra
clocks, won't it silently pass validation but fail at runtime by dropping
register writes?
>
> clock-names:
> + minItems: 2
> items:
> - const: aclk
> - const: iface
> + - const: npu
> + - const: aclk_cbuf
> + - const: hclk_cbuf
[Severity: Medium]
Similar to the clocks property above, does widening the clock-names property
without conditional constraints cause a regression that allows invalid
configurations for older hardware variants to silently pass validation?
Should an allOf: if-then block be used here as well?
>
> "#iommu-cells":
> const: 0
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805063826.95682-1-gahing@gahingwoo.com?part=3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (2 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 7:19 ` sashiko-bot
2026-08-05 6:38 ` [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
` (4 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
The RK3576 NPU domains need a short settle time after the idle request
is released before the QoS registers behind the domain answer. Without
it rockchip_pmu_restore_qos() reads back zeroes, and the NPU throws an
async SError on the first cold power-on.
Give rockchip_domain_info an optional delay_us and wait for it between
releasing idle and restoring QoS. Rename DOMAIN_M_O_R_G to
DOMAIN_M_O_R_G_W, since the suffixes name the fields the macro sets and
this one now also carries a wakeup delay; RK3576 is its only user, so
the old spelling is not kept around.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 52 +++++++++++++++-----------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index ba66ae719..e1857f878 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -18,6 +18,7 @@
#include <linux/of_address.h>
#include <linux/of_clk.h>
#include <linux/clk.h>
+#include <linux/delay.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/mfd/syscon.h>
@@ -59,6 +60,7 @@ struct rockchip_domain_info {
u32 pwr_offset;
u32 mem_offset;
u32 req_offset;
+ u32 delay_us;
};
struct rockchip_pmu_info {
@@ -185,7 +187,7 @@ struct rockchip_pmu {
.need_regulator = regulator, \
}
-#define DOMAIN_M_O_R_G(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, wakeup) \
+#define DOMAIN_M_O_R_G_W(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, delay, wakeup) \
{ \
.name = _name, \
.pwr_offset = p_offset, \
@@ -200,6 +202,7 @@ struct rockchip_pmu {
.req_mask = (req), \
.idle_mask = (idle), \
.clk_ungate_mask = (g_mask), \
+ .delay_us = (delay), \
.ack_mask = (ack), \
.active_wakeup = wakeup, \
}
@@ -258,8 +261,8 @@ struct rockchip_pmu {
#define DOMAIN_RK3568(name, pwr, req, wakeup, regulator) \
DOMAIN_M_R(name, pwr, pwr, req, req, req, wakeup, regulator)
-#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, wakeup) \
- DOMAIN_M_O_R_G(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, wakeup)
+#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, delay, wakeup) \
+ DOMAIN_M_O_R_G_W(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, delay, wakeup)
/*
* Dynamic Memory Controller may need to coordinate with us -- see
@@ -681,6 +684,10 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
if (ret < 0)
goto out;
+ /* Some domains need to settle before the QoS registers answer. */
+ if (pd->info->delay_us)
+ udelay(pd->info->delay_us);
+
rockchip_pmu_restore_qos(pd);
}
@@ -1300,25 +1307,26 @@ static const struct rockchip_domain_info rk3568_pm_domains[] = {
};
static const struct rockchip_domain_info rk3576_pm_domains[] = {
- [RK3576_PD_NPU] = DOMAIN_RK3576("npu", 0x0, BIT(0), BIT(0), 0, 0x0, 0, 0, 0, false),
- [RK3576_PD_NVM] = DOMAIN_RK3576("nvm", 0x0, BIT(6), 0, BIT(6), 0x4, BIT(2), BIT(18), BIT(2), false),
- [RK3576_PD_SDGMAC] = DOMAIN_RK3576("sdgmac", 0x0, BIT(7), 0, BIT(7), 0x4, BIT(1), BIT(17), 0x6, false),
- [RK3576_PD_AUDIO] = DOMAIN_RK3576("audio", 0x0, BIT(8), 0, BIT(8), 0x4, BIT(0), BIT(16), BIT(0), false),
- [RK3576_PD_PHP] = DOMAIN_RK3576("php", 0x0, BIT(9), 0, BIT(9), 0x0, BIT(15), BIT(15), BIT(15), false),
- [RK3576_PD_SUBPHP] = DOMAIN_RK3576("subphp", 0x0, BIT(10), 0, BIT(10), 0x0, 0, 0, 0, false),
- [RK3576_PD_VOP] = DOMAIN_RK3576("vop", 0x0, BIT(11), 0, BIT(11), 0x0, 0x6000, 0x6000, 0x6000, false),
- [RK3576_PD_VO1] = DOMAIN_RK3576("vo1", 0x0, BIT(14), 0, BIT(14), 0x0, BIT(12), BIT(12), 0x7000, false),
- [RK3576_PD_VO0] = DOMAIN_RK3576("vo0", 0x0, BIT(15), 0, BIT(15), 0x0, BIT(11), BIT(11), 0x6800, false),
- [RK3576_PD_USB] = DOMAIN_RK3576("usb", 0x4, BIT(0), 0, BIT(16), 0x0, BIT(10), BIT(10), 0x6400, true),
- [RK3576_PD_VI] = DOMAIN_RK3576("vi", 0x4, BIT(1), 0, BIT(17), 0x0, BIT(9), BIT(9), BIT(9), false),
- [RK3576_PD_VEPU0] = DOMAIN_RK3576("vepu0", 0x4, BIT(2), 0, BIT(18), 0x0, BIT(7), BIT(7), 0x280, false),
- [RK3576_PD_VEPU1] = DOMAIN_RK3576("vepu1", 0x4, BIT(3), 0, BIT(19), 0x0, BIT(8), BIT(8), BIT(8), false),
- [RK3576_PD_VDEC] = DOMAIN_RK3576("vdec", 0x4, BIT(4), 0, BIT(20), 0x0, BIT(6), BIT(6), BIT(6), false),
- [RK3576_PD_VPU] = DOMAIN_RK3576("vpu", 0x4, BIT(5), 0, BIT(21), 0x0, BIT(5), BIT(5), BIT(5), false),
- [RK3576_PD_NPUTOP] = DOMAIN_RK3576("nputop", 0x4, BIT(6), 0, BIT(22), 0x0, 0x18, 0x18, 0x18, false),
- [RK3576_PD_NPU0] = DOMAIN_RK3576("npu0", 0x4, BIT(7), 0, BIT(23), 0x0, BIT(1), BIT(1), 0x1a, false),
- [RK3576_PD_NPU1] = DOMAIN_RK3576("npu1", 0x4, BIT(8), 0, BIT(24), 0x0, BIT(2), BIT(2), 0x1c, false),
- [RK3576_PD_GPU] = DOMAIN_RK3576("gpu", 0x4, BIT(9), 0, BIT(25), 0x0, BIT(0), BIT(0), BIT(0), false),
+ /* name p_offset pwr status r_status r_offset req idle g_mask delay wakeup */
+ [RK3576_PD_NPU] = DOMAIN_RK3576("npu", 0x0, BIT(0), BIT(0), 0, 0x0, 0, 0, 0, 0, false),
+ [RK3576_PD_NVM] = DOMAIN_RK3576("nvm", 0x0, BIT(6), 0, BIT(6), 0x4, BIT(2), BIT(18), BIT(2), 0, false),
+ [RK3576_PD_SDGMAC] = DOMAIN_RK3576("sdgmac", 0x0, BIT(7), 0, BIT(7), 0x4, BIT(1), BIT(17), 0x6, 0, false),
+ [RK3576_PD_AUDIO] = DOMAIN_RK3576("audio", 0x0, BIT(8), 0, BIT(8), 0x4, BIT(0), BIT(16), BIT(0), 0, false),
+ [RK3576_PD_PHP] = DOMAIN_RK3576("php", 0x0, BIT(9), 0, BIT(9), 0x0, BIT(15), BIT(15), BIT(15), 0, false),
+ [RK3576_PD_SUBPHP] = DOMAIN_RK3576("subphp", 0x0, BIT(10), 0, BIT(10), 0x0, 0, 0, 0, 0, false),
+ [RK3576_PD_VOP] = DOMAIN_RK3576("vop", 0x0, BIT(11), 0, BIT(11), 0x0, 0x6000, 0x6000, 0x6000, 0, false),
+ [RK3576_PD_VO1] = DOMAIN_RK3576("vo1", 0x0, BIT(14), 0, BIT(14), 0x0, BIT(12), BIT(12), 0x7000, 0, false),
+ [RK3576_PD_VO0] = DOMAIN_RK3576("vo0", 0x0, BIT(15), 0, BIT(15), 0x0, BIT(11), BIT(11), 0x6800, 0, false),
+ [RK3576_PD_USB] = DOMAIN_RK3576("usb", 0x4, BIT(0), 0, BIT(16), 0x0, BIT(10), BIT(10), 0x6400, 0, true),
+ [RK3576_PD_VI] = DOMAIN_RK3576("vi", 0x4, BIT(1), 0, BIT(17), 0x0, BIT(9), BIT(9), BIT(9), 0, false),
+ [RK3576_PD_VEPU0] = DOMAIN_RK3576("vepu0", 0x4, BIT(2), 0, BIT(18), 0x0, BIT(7), BIT(7), 0x280, 0, false),
+ [RK3576_PD_VEPU1] = DOMAIN_RK3576("vepu1", 0x4, BIT(3), 0, BIT(19), 0x0, BIT(8), BIT(8), BIT(8), 0, false),
+ [RK3576_PD_VDEC] = DOMAIN_RK3576("vdec", 0x4, BIT(4), 0, BIT(20), 0x0, BIT(6), BIT(6), BIT(6), 0, false),
+ [RK3576_PD_VPU] = DOMAIN_RK3576("vpu", 0x4, BIT(5), 0, BIT(21), 0x0, BIT(5), BIT(5), BIT(5), 0, false),
+ [RK3576_PD_NPUTOP] = DOMAIN_RK3576("nputop", 0x4, BIT(6), 0, BIT(22), 0x0, 0x18, 0x18, 0x18, 15, false),
+ [RK3576_PD_NPU0] = DOMAIN_RK3576("npu0", 0x4, BIT(7), 0, BIT(23), 0x0, BIT(1), BIT(1), 0x1a, 15, false),
+ [RK3576_PD_NPU1] = DOMAIN_RK3576("npu1", 0x4, BIT(8), 0, BIT(24), 0x0, BIT(2), BIT(2), 0x1c, 15, false),
+ [RK3576_PD_GPU] = DOMAIN_RK3576("gpu", 0x4, BIT(9), 0, BIT(25), 0x0, BIT(0), BIT(0), BIT(0), 0, false),
};
static const struct rockchip_domain_info rk3588_pm_domains[] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay
2026-08-05 6:38 ` [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
@ 2026-08-05 7:19 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-05 7:19 UTC (permalink / raw)
To: Jiaxing Hu; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] On the first power-on from a cold boot, `rockchip_pmu_restore_qos()` incorrectly writes uninitialized zero values to the hardware QoS registers if the domain was powered off by the bootloader.
--
--- Patch [4]: [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay ---
commit c7d671d6283e8b2b3bf5a4fdce967757abdd8d2f
Author: Jiaxing Hu <gahing@gahingwoo.com>
pmdomain/rockchip: add optional per-domain power-on settle delay
Adds an optional delay_us parameter to rockchip_domain_info to allow the
RK3576 NPU domains to settle after the idle request is released. This ensures
the QoS registers answer properly and prevents an async SError on the first
cold power-on.
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
[ ... ]
> @@ -681,6 +684,10 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
> if (ret < 0)
> goto out;
>
> + /* Some domains need to settle before the QoS registers answer. */
> + if (pd->info->delay_us)
> + udelay(pd->info->delay_us);
> +
> rockchip_pmu_restore_qos(pd);
[Severity: High]
This isn't a bug introduced by this patch, but is it possible that
rockchip_pmu_restore_qos() writes uninitialized zeroes to the hardware
QoS registers on the first power-on?
If a domain is powered off by the bootloader, pm_genpd_init() initializes
it as off, and rockchip_pmu_save_qos() is never called to save the hardware
defaults.
Since qos_save_regs is zero-allocated in rockchip_pm_add_one_domain():
pd->qos_save_regs[j] = devm_kcalloc(pmu->dev, pd->num_qos,
sizeof(u32), GFP_KERNEL);
When the domain is powered on for the first time by a consumer, this path
is reached and rockchip_pmu_restore_qos() directly writes those zeroes to
the QoS registers, such as QOS_PRIORITY.
Can this cause performance degradation or starvation for IPs in the domain?
> }
>
> @@ -1300,25 +1307,26 @@ static const struct rockchip_domain_info rk3568_pm_domains[] = {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805063826.95682-1-gahing@gahingwoo.com?part=4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (3 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 4/8] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 7:27 ` sashiko-bot
2026-08-05 12:13 ` Philipp Zabel
2026-08-05 6:38 ` [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
` (3 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
Some Rockchip domains come out of power-on with their bus interface in
an undefined state. On the RK3576 NPU this shows up as a hang on the
first access after a cold power-on; the vendor kernel cycles the
domain's resets at this point and that clears it.
Take the domain node's resets if it has any, and pulse them between
releasing idle and restoring QoS. The resets are optional, so domains
that do not declare any behave exactly as before.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
drivers/pmdomain/rockchip/pm-domains.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index e1857f878..82051f600 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -19,6 +19,7 @@
#include <linux/of_clk.h>
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/reset.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/mfd/syscon.h>
@@ -103,6 +104,7 @@ struct rockchip_pm_domain {
struct clk_bulk_data *clks;
struct device_node *node;
struct regulator *supply;
+ struct reset_control *resets;
};
struct rockchip_pmu {
@@ -688,6 +690,13 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
if (pd->info->delay_us)
udelay(pd->info->delay_us);
+ /* Optional: some domains need their resets cycled after power-on. */
+ if (pd->resets) {
+ reset_control_assert(pd->resets);
+ udelay(10);
+ reset_control_deassert(pd->resets);
+ }
+
rockchip_pmu_restore_qos(pd);
}
@@ -857,6 +866,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
if (error)
goto err_put_clocks;
+ pd->resets = of_reset_control_array_get_optional_exclusive(node);
+ if (IS_ERR(pd->resets)) {
+ error = PTR_ERR(pd->resets);
+ dev_err(pmu->dev, "%pOFn: failed to get resets: %d\n", node, error);
+ pd->resets = NULL;
+ goto err_unprepare_clocks;
+ }
+
pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
NULL);
@@ -927,6 +944,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
clk_bulk_unprepare(pd->num_clks, pd->clks);
err_put_clocks:
clk_bulk_put(pd->num_clks, pd->clks);
+ reset_control_put(pd->resets);
return error;
}
@@ -945,6 +963,7 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
clk_bulk_unprepare(pd->num_clks, pd->clks);
clk_bulk_put(pd->num_clks, pd->clks);
+ reset_control_put(pd->resets);
/* protect the zeroing of pm->num_clks */
mutex_lock(&pd->pmu->mutex);
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on
2026-08-05 6:38 ` [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
@ 2026-08-05 7:27 ` sashiko-bot
2026-08-05 12:13 ` Philipp Zabel
1 sibling, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-05 7:27 UTC (permalink / raw)
To: Jiaxing Hu; +Cc: devicetree, robh, dri-devel, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using dev_err() instead of dev_err_probe() for reset acquisition leads to spurious boot warnings.
--
--- Patch [5]: [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on ---
commit a3723842ac037cc5513c93510e6b54bd305213be
Author: Jiaxing Hu <gahing@gahingwoo.com>
pmdomain/rockchip: cycle optional power-domain resets on power-on
This commit cycles the resets for Rockchip power domains when they are powered
on. This clears undefined bus interface states that can cause hangs on devices
like the RK3576 NPU on the first access after a cold power-on.
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
[ ... ]
> @@ -857,6 +866,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
> if (error)
> goto err_put_clocks;
>
> + pd->resets = of_reset_control_array_get_optional_exclusive(node);
> + if (IS_ERR(pd->resets)) {
> + error = PTR_ERR(pd->resets);
> + dev_err(pmu->dev, "%pOFn: failed to get resets: %d\n", node, error);
[Severity: Medium]
Could this cause spurious boot warnings if the reset controller hasn't probed
yet?
When of_reset_control_array_get_optional_exclusive() returns -EPROBE_DEFER,
dev_err() will log an unconditional error instead of handling the deferral
silently. Should this use dev_err_probe() to cleanly handle deferred probes
without adding spam to the kernel log?
> + pd->resets = NULL;
> + goto err_unprepare_clocks;
> + }
> +
> pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
> NULL);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805063826.95682-1-gahing@gahingwoo.com?part=5
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on
2026-08-05 6:38 ` [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
2026-08-05 7:27 ` sashiko-bot
@ 2026-08-05 12:13 ` Philipp Zabel
1 sibling, 0 replies; 21+ messages in thread
From: Philipp Zabel @ 2026-08-05 12:13 UTC (permalink / raw)
To: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel
On Mi, 2026-08-05 at 18:38 +1200, Jiaxing Hu wrote:
> Some Rockchip domains come out of power-on with their bus interface in
> an undefined state. On the RK3576 NPU this shows up as a hang on the
> first access after a cold power-on; the vendor kernel cycles the
> domain's resets at this point and that clears it.
>
> Take the domain node's resets if it has any, and pulse them between
> releasing idle and restoring QoS. The resets are optional, so domains
> that do not declare any behave exactly as before.
>
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
> drivers/pmdomain/rockchip/pm-domains.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index e1857f878..82051f600 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -19,6 +19,7 @@
> #include <linux/of_clk.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> +#include <linux/reset.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/mfd/syscon.h>
> @@ -103,6 +104,7 @@ struct rockchip_pm_domain {
> struct clk_bulk_data *clks;
> struct device_node *node;
> struct regulator *supply;
> + struct reset_control *resets;
> };
>
> struct rockchip_pmu {
> @@ -688,6 +690,13 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
> if (pd->info->delay_us)
> udelay(pd->info->delay_us);
>
> + /* Optional: some domains need their resets cycled after power-on. */
> + if (pd->resets) {
> + reset_control_assert(pd->resets);
> + udelay(10);
> + reset_control_deassert(pd->resets);
> + }
> +
> rockchip_pmu_restore_qos(pd);
> }
>
> @@ -857,6 +866,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
> if (error)
> goto err_put_clocks;
>
> + pd->resets = of_reset_control_array_get_optional_exclusive(node);
Why is this using of_reset_control_array_get_optional_exclusive()
instead of devm_reset_control_array_get_optional_exclusive() when pd is
allocated with devm_kzalloc()?
> + if (IS_ERR(pd->resets)) {
> + error = PTR_ERR(pd->resets);
> + dev_err(pmu->dev, "%pOFn: failed to get resets: %d\n", node, error);
dev_err_probe() to avoid printing on -EPROBE_DEFER.
regards
Philipp
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (4 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 5/8] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 7:39 ` sashiko-bot
` (2 more replies)
2026-08-05 6:38 ` [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
` (2 subsequent siblings)
8 siblings, 3 replies; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
The RK3576 carries the same RKNN block as the RK3588, with two cores
instead of three and a few platform differences:
- the CBUF (convolution buffer) has its own clock domain, so the core
needs six clocks rather than four;
- the BIU reset moved into the power domain, leaving one reset here;
- the NPU spans two power domains, and a device with more than one is
skipped by the driver-core single-domain auto-attach, so the list has
to be attached explicitly;
- the DPU completion interrupt is armed exactly as on RK3588 but never
reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
so sample that from an hrtimer rather than wait for an interrupt that
does not come. The interrupt stays armed, so if it ever does arrive
the normal handler finalises the job first.
Select all of that from of_device_id match data so the RK3588 path keeps
its existing counts and behaviour unchanged.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
drivers/accel/rocket/rocket_core.c | 26 +++++-
drivers/accel/rocket/rocket_core.h | 20 ++++-
drivers/accel/rocket/rocket_device.c | 4 +
drivers/accel/rocket/rocket_drv.c | 22 ++++-
drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
5 files changed, 176 insertions(+), 17 deletions(-)
diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b3b2fa9ba..e08288c8c 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -8,6 +8,7 @@
#include <linux/err.h>
#include <linux/iommu.h>
#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
@@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
u32 version;
int err = 0;
+ /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
core->resets[0].id = "srst_a";
core->resets[1].id = "srst_h";
- err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
+ err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
core->resets);
if (err)
return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
- err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
+ core->clks[0].id = "aclk";
+ core->clks[1].id = "hclk";
+ core->clks[2].id = "npu";
+ core->clks[3].id = "pclk";
+ /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
+ core->clks[4].id = "aclk_cbuf";
+ core->clks[5].id = "hclk_cbuf";
+ err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
if (err)
return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
@@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
return err;
}
+ /*
+ * RK3576 spans two power domains, and a multi-domain device is skipped
+ * by the driver-core single-domain auto-attach, so attach the list here.
+ */
+ if (core->soc->multi_power_domain) {
+ struct dev_pm_domain_list *pd_list;
+
+ err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
+ if (err < 0)
+ return dev_err_probe(dev, err,
+ "failed to attach NPU power domains\n");
+ }
+
pm_runtime_use_autosuspend(dev);
/*
diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
index f6d738285..205ff070d 100644
--- a/drivers/accel/rocket/rocket_core.h
+++ b/drivers/accel/rocket/rocket_core.h
@@ -6,6 +6,7 @@
#include <drm/gpu_scheduler.h>
#include <linux/clk.h>
+#include <linux/hrtimer.h>
#include <linux/io.h>
#include <linux/mutex_types.h>
#include <linux/reset.h>
@@ -27,16 +28,25 @@
#define rocket_core_writel(core, reg, value) \
writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
+/* Per-SoC differences, selected by the of_device_id match data. */
+struct rocket_soc_data {
+ unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
+ unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
+ bool multi_power_domain; /* device spans more than one PM domain */
+ bool poll_completion; /* completion IRQ never reaches the GIC */
+};
+
struct rocket_core {
struct device *dev;
struct rocket_device *rdev;
+ const struct rocket_soc_data *soc;
unsigned int index;
int irq;
void __iomem *pc_iomem;
void __iomem *cna_iomem;
void __iomem *core_iomem;
- struct clk_bulk_data clks[4];
+ struct clk_bulk_data clks[6];
struct reset_control_bulk_data resets[2];
struct iommu_group *iommu_group;
@@ -52,6 +62,14 @@ struct rocket_core {
atomic_t pending;
} reset;
+ struct hrtimer poll_timer;
+ struct work_struct poll_work;
+ atomic_t poll_active;
+ unsigned int poll_ticks;
+ unsigned int poll_seq;
+ unsigned int poll_work_seq;
+ bool poll_dying;
+
struct drm_gpu_scheduler sched;
u64 fence_context;
u64 emit_seqno;
diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
index 46e6ee1e7..bfb00f967 100644
--- a/drivers/accel/rocket/rocket_device.c
+++ b/drivers/accel/rocket/rocket_device.c
@@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
if (of_device_is_available(core_node))
num_cores++;
+ for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
+ if (of_device_is_available(core_node))
+ num_cores++;
+
rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
if (!rdev->cores)
return ERR_PTR(-ENOMEM);
diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 8bbbce594..7f7dfa374 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
rdev->cores[core].rdev = rdev;
rdev->cores[core].dev = &pdev->dev;
+ rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
rdev->cores[core].index = core;
rdev->num_cores++;
@@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
}
}
+static const struct rocket_soc_data rk3588_soc_data = {
+ .num_clks = 4,
+ .num_resets = 2,
+ .multi_power_domain = false,
+ .poll_completion = false,
+};
+
+static const struct rocket_soc_data rk3576_soc_data = {
+ .num_clks = 6,
+ .num_resets = 1,
+ .multi_power_domain = true,
+ .poll_completion = true,
+};
+
static const struct of_device_id dt_match[] = {
- { .compatible = "rockchip,rk3588-rknn-core" },
+ { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
+ { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
@@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
if (core < 0)
return -ENODEV;
- err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+ err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
if (err) {
dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
return err;
@@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
if (!rocket_job_is_idle(&rdev->cores[core]))
return -EBUSY;
- clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+ clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
return 0;
}
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index bb77b6bf0..28845ac4e 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -7,6 +7,7 @@
#include <drm/drm_file.h>
#include <drm/drm_gem.h>
#include <drm/rocket_accel.h>
+#include <linux/hrtimer.h>
#include <linux/interrupt.h>
#include <linux/overflow.h>
#include <linux/iommu.h>
@@ -21,6 +22,15 @@
#define JOB_TIMEOUT_MS 500
+/*
+ * RK3576 arms the same DPU completion as RK3588, but the interrupt never
+ * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
+ * so sample that instead. The tick cap bounds jobs that never raise it at all,
+ * which is the same open problem as the wrong inference results.
+ */
+#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
+#define RK3576_POLL_MAX_TICKS 8
+
static struct rocket_job *
to_rocket_job(struct drm_sched_job *sched_job)
{
@@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
+ if (core->soc->poll_completion) {
+ core->poll_ticks = 0;
+ core->poll_seq++;
+ atomic_set(&core->poll_active, 1);
+ hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
+ HRTIMER_MODE_REL);
+ }
+
dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
}
@@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
return ERR_PTR(ret);
}
+static void rocket_job_handle_irq(struct rocket_core *core);
+
+static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
+{
+ struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
+ u32 raw;
+
+ if (!atomic_read(&core->poll_active))
+ return HRTIMER_NORESTART;
+
+ core->poll_work_seq = core->poll_seq;
+
+ raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
+ if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
+ ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
+ atomic_set(&core->poll_active, 0);
+ schedule_work(&core->poll_work);
+ return HRTIMER_NORESTART;
+ }
+
+ hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
+ return HRTIMER_RESTART;
+}
+
+/* Start the job's next task, or retire it. Caller holds job_lock. */
+static void rocket_job_next_locked(struct rocket_core *core)
+{
+ lockdep_assert_held(&core->job_lock);
+
+ if (!core->in_flight_job)
+ return;
+
+ if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
+ rocket_job_hw_submit(core, core->in_flight_job);
+ return;
+ }
+
+ iommu_detach_group(NULL, iommu_group_get(core->dev));
+ dma_fence_signal(core->in_flight_job->done_fence);
+ pm_runtime_put_autosuspend(core->dev);
+ core->in_flight_job = NULL;
+}
+
+static void rocket_poll_work_fn(struct work_struct *work)
+{
+ struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
+
+ pm_runtime_mark_last_busy(core->dev);
+
+ scoped_guard(mutex, &core->job_lock) {
+ /*
+ * The interrupt can land while this work is queued, retire the job
+ * and start the next task. poll_seq only moves under job_lock, in
+ * hw_submit, so comparing it here says whether that happened. Doing
+ * it outside the lock would leave the window open rather than close
+ * it, and this work would then submit a task on top of a live one.
+ */
+ if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
+ return;
+
+ rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
+ rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
+
+ rocket_job_next_locked(core);
+ }
+}
+
static void rocket_job_handle_irq(struct rocket_core *core)
{
+ if (core->soc->poll_completion) {
+ atomic_set(&core->poll_active, 0);
+ hrtimer_cancel(&core->poll_timer);
+ }
+
pm_runtime_mark_last_busy(core->dev);
rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
scoped_guard(mutex, &core->job_lock)
- if (core->in_flight_job) {
- if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
- rocket_job_hw_submit(core, core->in_flight_job);
- return;
- }
-
- iommu_detach_group(NULL, iommu_group_get(core->dev));
- dma_fence_signal(core->in_flight_job->done_fence);
- pm_runtime_put_autosuspend(core->dev);
- core->in_flight_job = NULL;
- }
+ rocket_job_next_locked(core);
}
static void
@@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
int ret;
INIT_WORK(&core->reset.work, rocket_reset_work);
+ INIT_WORK(&core->poll_work, rocket_poll_work_fn);
+ hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL);
+ atomic_set(&core->poll_active, 0);
spin_lock_init(&core->fence_lock);
mutex_init(&core->job_lock);
@@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
void rocket_job_fini(struct rocket_core *core)
{
+ /*
+ * Stop the poll from starting hardware work before tearing anything
+ * down: it submits the next task, and drm_sched_fini() does not wait
+ * for work already queued. Cancel after the scheduler is gone, so a
+ * job running now cannot re-arm the timer behind the cancel.
+ */
+ if (core->soc->poll_completion)
+ WRITE_ONCE(core->poll_dying, true);
+
drm_sched_fini(&core->sched);
+ if (core->soc->poll_completion) {
+ atomic_set(&core->poll_active, 0);
+ hrtimer_cancel(&core->poll_timer);
+ cancel_work_sync(&core->poll_work);
+ }
+
cancel_work_sync(&core->reset.work);
destroy_workqueue(core->reset.wq);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 6:38 ` [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
@ 2026-08-05 7:39 ` sashiko-bot
2026-08-05 10:34 ` Igor Paunovic
2026-08-05 12:52 ` Diederik de Haas
2 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-05 7:39 UTC (permalink / raw)
To: Jiaxing Hu; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
New issues:
- [High] Hardware reset triggered while polling timer and workqueue are still active.
- [High] Unclocked MMIO writes in `rocket_poll_work_fn` due to stale sequence numbers.
- [High] Bypassing job completion in `rocket_poll_work_fn` during teardown leads to deadlocks and resource leaks.
- [High] Concurrent execution of `rocket_poll_work_fn` and `rocket_job_handle_irq` corrupts hardware state and skips tasks.
- [High] Late hardware interrupts re-arm the polling timer after teardown, causing a Use-After-Free.
Pre-existing issues:
- [High] Unclocked MMIO access in shared IRQ handler (`rocket_job_irq_handler`).
- [High] Reset workqueue can be used after destruction due to late IRQ firing.
- [High] IOMMU group reference leak on every job completion.
- [High] DRM device is registered and exposed to userspace before core hardware and schedulers are initialized.
- [High] Unbinding a non-terminal core corrupts `rdev->num_cores` and leads to state overwrite.
--
--- Patch [6]: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support ---
commit 49430ea8bb109098d159ff322aab30ccbfc4c20a
Author: Jiaxing Hu <gahing@gahingwoo.com>
accel/rocket: add RK3576 NPU (RKNN) support
The RK3576 carries the same RKNN block as the RK3588, with two cores
instead of three and a few platform differences: The patch adds RK3576
support, managing its separate clock domains, power domains, and polling
the completion interrupt via an hrtimer.
> diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> --- a/drivers/accel/rocket/rocket_device.c
> +++ b/drivers/accel/rocket/rocket_device.c
> @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
[ ... ]
> err = drm_dev_register(ddev, 0);
> if (err)
> return ERR_PTR(err);
>
> return rdev;
> }
[Severity: High]
This is a pre-existing issue, but does rocket_device_init() expose the
DRM device to userspace before the hardware is actually ready?
It looks like drm_dev_register() is called here, exposing the device,
but rocket_probe() only initializes the core hardware and schedulers via
rocket_core_init() afterward. Could userspace race to open the device and
submit jobs before the schedulers and memory maps are initialized?
> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
>
> rdev->cores[core].rdev = rdev;
> rdev->cores[core].dev = &pdev->dev;
> + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> rdev->cores[core].index = core;
>
> rdev->num_cores++;
[Severity: High]
This is a pre-existing issue, but does the core removal logic in
rocket_remove() corrupt the device state if a non-terminal core is unbound?
If rocket_remove() is called for core 0 on a 2-core setup, it blindly
decrements rdev->num_cores to 1. If another core is then bound,
rocket_probe() will use rdev->num_cores (1) as the index, overwriting the
active rdev->cores[1] structure currently in use.
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
[ ... ]
> +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> +{
> + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> + u32 raw;
> +
> + if (!atomic_read(&core->poll_active))
> + return HRTIMER_NORESTART;
> +
> + core->poll_work_seq = core->poll_seq;
> +
> + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
[Severity: High]
Can the hardware reset logic race with this polling timer and workqueue?
When a job times out, the DRM scheduler calls rocket_reset(), which
triggers rocket_core_reset() without first stopping the active poll timer
or work. If the timer fires or the work executes concurrently, will they
attempt to read or write MMIO registers on a device undergoing reset or
without active clocks?
> + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> + atomic_set(&core->poll_active, 0);
> + schedule_work(&core->poll_work);
> + return HRTIMER_NORESTART;
> + }
> +
> + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> + return HRTIMER_RESTART;
> +}
> +
> +/* Start the job's next task, or retire it. Caller holds job_lock. */
> +static void rocket_job_next_locked(struct rocket_core *core)
> +{
> + lockdep_assert_held(&core->job_lock);
> +
> + if (!core->in_flight_job)
> + return;
> +
> + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> + rocket_job_hw_submit(core, core->in_flight_job);
> + return;
> + }
> +
> + iommu_detach_group(NULL, iommu_group_get(core->dev));
[Severity: High]
This is a pre-existing issue, but does calling iommu_group_get() here leak
the IOMMU group reference?
It appears iommu_group_get() acquires a reference that is passed to
iommu_detach_group(), but iommu_detach_group() does not consume the
reference, and there is no matching iommu_group_put(). Will this cause the
reference count to overflow or permanently pin IOMMU resources?
> + dma_fence_signal(core->in_flight_job->done_fence);
> + pm_runtime_put_autosuspend(core->dev);
> + core->in_flight_job = NULL;
> +}
> +
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> + pm_runtime_mark_last_busy(core->dev);
> +
> + scoped_guard(mutex, &core->job_lock) {
> + /*
> + * The interrupt can land while this work is queued, retire the job
> + * and start the next task. poll_seq only moves under job_lock, in
> + * hw_submit, so comparing it here says whether that happened. Doing
> + * it outside the lock would leave the window open rather than close
> + * it, and this work would then submit a task on top of a live one.
> + */
> + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> + return;
[Severity: High]
Does returning early here on poll_dying cause a resource leak and deadlock?
If poll_work_fn() returns early without calling rocket_job_next_locked(), it
looks like the done_fence of the currently executing job is never signaled,
the PM runtime reference is never released, and the IOMMU group is never
detached.
[Severity: High]
Can this sequence check incorrectly pass if the last task of a job completes
via a shared IRQ?
If rocket_job_handle_irq() finishes the last task, it drops the PM runtime
reference and clears in_flight_job. However, because rocket_job_hw_submit()
isn't called, poll_seq is not updated.
When this work function subsequently runs, it passes the sequence check
(poll_work_seq == poll_seq) and proceeds to write to hardware. Since the
PM reference was already dropped, could this cause a fatal unclocked MMIO
access?
> +
> + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> +
> + rocket_job_next_locked(core);
> + }
> +}
> +
> static void rocket_job_handle_irq(struct rocket_core *core)
> {
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + }
> +
> pm_runtime_mark_last_busy(core->dev);
>
> rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
[Severity: High]
Can this hardware write outside the job_lock race with task submission?
The rocket_job_handle_irq() function writes 0 to OPERATION_ENABLE before
acquiring the job_lock. If poll_work_fn() is concurrently inside the lock
submitting the next task (which sets OPERATION_ENABLE to 1), the IRQ handler
can immediately overwrite it to 0, disabling the new task.
The IRQ handler would then acquire job_lock and call rocket_job_next_locked(),
which retires the newly started task without it ever executing.
> rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
>
> scoped_guard(mutex, &core->job_lock)
> - if (core->in_flight_job) {
> - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> - rocket_job_hw_submit(core, core->in_flight_job);
> - return;
> - }
> -
> - iommu_detach_group(NULL, iommu_group_get(core->dev));
> - dma_fence_signal(core->in_flight_job->done_fence);
> - pm_runtime_put_autosuspend(core->dev);
> - core->in_flight_job = NULL;
> - }
> + rocket_job_next_locked(core);
> }
[ ... ]
> @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> int ret;
>
> INIT_WORK(&core->reset.work, rocket_reset_work);
> + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> + HRTIMER_MODE_REL);
> + atomic_set(&core->poll_active, 0);
> spin_lock_init(&core->fence_lock);
> mutex_init(&core->job_lock);
>
> @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
[Severity: High]
This is a pre-existing issue, but does the shared IRQ handler perform
unclocked MMIO access?
The rocket_job_irq_handler (which calls rocket_job_handle_irq) is registered
with IRQF_SHARED. If it reads INTERRUPT_RAW_STATUS immediately without
checking if the device's power domain and clocks are active, a shared
interrupt firing while the NPU is runtime suspended could cause a
synchronous external abort.
> void rocket_job_fini(struct rocket_core *core)
> {
> + /*
> + * Stop the poll from starting hardware work before tearing anything
> + * down: it submits the next task, and drm_sched_fini() does not wait
> + * for work already queued. Cancel after the scheduler is gone, so a
> + * job running now cannot re-arm the timer behind the cancel.
> + */
> + if (core->soc->poll_completion)
> + WRITE_ONCE(core->poll_dying, true);
> +
> drm_sched_fini(&core->sched);
>
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + cancel_work_sync(&core->poll_work);
> + }
[Severity: High]
Can a late hardware interrupt re-arm the polling timer after teardown?
While hrtimer_cancel() is called here, the hardware IRQ is not disabled.
If an interrupt fires after this point, rocket_job_handle_irq() will execute,
progress the job, and call rocket_job_hw_submit(), which re-arms the timer
via hrtimer_start().
Once the rocket_core structure is freed, could the armed timer fire and
access freed memory?
> +
> cancel_work_sync(&core->reset.work);
> destroy_workqueue(core->reset.wq);
> }
[Severity: High]
This is a pre-existing issue, but can the reset workqueue be used after it
is destroyed?
Because the shared IRQ remains active until devres cleans it up after
rocket_remove(), a late interrupt hitting the MAC interrupt condition could
call queue_work() on this destroyed workqueue, leading to a kernel panic.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805063826.95682-1-gahing@gahingwoo.com?part=6
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 6:38 ` [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
2026-08-05 7:39 ` sashiko-bot
@ 2026-08-05 10:34 ` Igor Paunovic
2026-08-05 12:52 ` Diederik de Haas
2 siblings, 0 replies; 21+ messages in thread
From: Igor Paunovic @ 2026-08-05 10:34 UTC (permalink / raw)
To: Jiaxing Hu
Cc: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing, alchark, chaoyi.chen,
dri-devel, linux-rockchip, iommu, linux-pm, devicetree,
linux-arm-kernel, linux-kernel
Hi Jiaxing,
A few things on this patch. The first two are coordination and a small
inconsistency; the three after them come from reading the new poll and
probe paths rather than from the bench, and none of them are reachable
on RK3588, so I cannot put a measurement behind them.
First, coordination. 6/8 still carries the four clks[].id assignments,
and the commit message still describes the RK3588 path as keeping its
existing behaviour unchanged. That hunk is the standalone fix I have in
flight:
https://lore.kernel.org/linux-rockchip/20260729130743.128876-1-royalnet026@gmail.com/
which carries your Reviewed-by and a Tested-by from Sidong Yang, and is
waiting on Tomeu. Your v3 cover flagged the overlap; v4 and v5 do not,
and I assume that was an editing casualty rather than a decision. If the
standalone lands first, this hunk will not apply cleanly, so it may be
worth either noting the dependency in the cover again or dropping the
hunk here and rebasing on it. Tomeu is the one who has to sequence the
two, so it is probably best said where he will look for it.
Second, a real if currently harmless inconsistency. This patch moves the
reset acquisition to soc->num_resets, which is 1 on RK3576:
- err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
- ARRAY_SIZE(core->resets),
+ err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
+ core->soc->num_resets,
but rocket_core_reset() still asserts and deasserts
ARRAY_SIZE(core->resets), which stays 2:
void rocket_core_reset(struct rocket_core *core)
{
reset_control_bulk_assert(ARRAY_SIZE(core->resets), core->resets);
udelay(10);
reset_control_bulk_deassert(ARRAY_SIZE(core->resets), core->resets);
}
On RK3576 that walks resets[1], which was never acquired. It is benign
today because the reset core returns 0 for a NULL rstc, so it is not a
bug report. But it is the kind of thing that stops being benign the
moment someone adds a NULL check or a WARN, and given that patch 5 has
just made the reset path load bearing for the behaviour you are chasing,
I would rather it read soc->num_resets in both places.
Third, and this is the one I would most like you to check, because I
cannot run it: poll_dying is a one-way latch. rocket_job_fini() sets it
if (core->soc->poll_completion)
WRITE_ONCE(core->poll_dying, true);
and nothing ever clears it. rocket_job_init() re-initialises the rest of
the poll state on the same struct, but not that field:
INIT_WORK(&core->poll_work, rocket_poll_work_fn);
hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
HRTIMER_MODE_REL);
atomic_set(&core->poll_active, 0);
Across the whole series the flag has exactly three appearances: the
declaration in rocket_core.h, the write above, and the read in
rocket_poll_work_fn().
That matters because struct rocket_core can outlive a bind. rocket_probe()
hands rocket_device_init() the shared facade device rather than its own
pdev:
rdev = rocket_device_init(drm_dev, &rocket_drm_driver);
and rocket_device_init() allocates the core array on it:
rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores),
GFP_KERNEL);
drm_dev is the "rknn" platform device registered in rocket_register().
It never binds a driver, so its devres list is only released at module
exit. Unbinding one core while another stays bound therefore leaves rdev
and that array alive and untouched, and the rebinding core is handed the
same struct rocket_core back. The driver already concedes this:
rocket_core_fini() has to clear core->iommu_group by hand, and that line
only makes sense because the struct survives the unbind.
On RK3576 the poll is the only completion path, so a core that comes
back with poll_dying still set never retires anything through it.
rocket_poll_work_fn() takes job_lock, reads the flag and returns before
the OPERATION_ENABLE and INTERRUPT_CLEAR writes and before
rocket_job_next_locked(), so done_fence is never signalled, the IOMMU
group is never detached and the runtime PM reference is never dropped.
Every job on that core then ends in the 500 ms drm_sched timeout and a
core reset instead, which is also the path that walks resets[1] from the
point above.
Now the part that I think explains why your unbind/rebind test is clean
and this is still there. 8/8 enables only rknn_core_0 on the ROCK 4D,
"the driver binds one core per node and the second core is left to
whoever can test it", so num_cores is 1 and every unbind on that board is
the last one. rocket_remove() then takes
if (rdev->num_cores == 0) {
rocket_device_fini(rdev);
rdev = NULL;
}
and the next probe re-runs rocket_device_init(), which devm_kcalloc()s a
fresh, zeroed core array. The struct the rebinding core gets is a
different one, with poll_dying false. So your result is right and
honestly reported; the latch just is not reachable in that
configuration. It needs both rknn_core_0 and rknn_core_1 enabled, which
7/8 already ships the nodes for, and an unbind of one while the other
stays bound:
echo 27708000.npu > /sys/bus/platform/drivers/rocket/unbind
echo 27708000.npu > /sys/bus/platform/drivers/rocket/bind
followed by inference on a freshly opened fd, since the scheduler list is
built at open time. Unbinding both cores and rebinding both will not show
it either, for the same reason the single core case does not: that path
reallocates rdev and the array from scratch.
I should disclose that my own in-flight 2/2, "keep core slots stable
across unbind and rebind", is what would make this deterministic. Today
the rebinding core takes index rdev->num_cores, so which struct it lands
in depends on the unbind order; with that patch it always reclaims its
own just-fini'd slot, which is exactly the one rocket_job_fini() latched.
I would rather it were fixed here than found after both land. One line
next to the existing atomic_set() in rocket_job_init() covers it:
WRITE_ONCE(core->poll_dying, false);
Init side rather than fini side, so a slot left dirty by a failed probe
is covered as well. RK3588 is unaffected either way, since
poll_completion is false there and the flag is never written.
Fourth, still in rocket_job_fini() and connected to the above: the flag
is written without job_lock but read under it, and cancel_work_sync()
comes after hrtimer_cancel(). A poll_work that has already passed the
check inside job_lock can therefore go on to rocket_job_next_locked() ->
rocket_job_hw_submit() and run
hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
HRTIMER_MODE_REL);
after hrtimer_cancel() has already returned. cancel_work_sync() then
waits for that worker and returns with the timer armed. fini returns,
rocket_core_fini() and rocket_remove() return, devres unmaps the register
windows, and roughly a millisecond later the timer fires into
rocket_pc_readl(core, INTERRUPT_RAW_STATUS). That is the property the
comment above the flag claims, "a job running now cannot re-arm the timer
behind the cancel", and I do not think the code has it yet.
Taking job_lock around the WRITE_ONCE would close it: any worker past the
check holds the lock, so fini waits behind it, and every later one
observes the flag. Reordering the two cancels alone only narrows the
window, since the timer callback is what queues the work.
Fifth, an error path. The new attach sits after rocket_job_init():
err = rocket_job_init(core);
if (err) {
iommu_group_put(core->iommu_group);
core->iommu_group = NULL;
return err;
}
+ if (core->soc->multi_power_domain) {
+ struct dev_pm_domain_list *pd_list;
+
+ err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
+ if (err < 0)
+ return dev_err_probe(dev, err,
+ "failed to attach NPU power domains\n");
+ }
By that point rocket_job_init() has taken two things devres does not own,
core->reset.wq from alloc_ordered_workqueue() and core->sched from
drm_sched_init(), which allocates its own submit workqueue as well, plus
the iommu_group reference that the branch immediately above is careful to
put. The new return unwinds none of them; every other failure after
rocket_job_init() goes through rocket_core_fini(). -EPROBE_DEFER is the
realistic case here, and it retries, so it repeats.
Moving the attach up, above core->iommu_group = iommu_group_get(dev),
makes the bare return correct and also attaches the domains before the
IRQ and the scheduler exist, which reads better anyway.
And a nit: the forward declaration
static void rocket_job_handle_irq(struct rocket_core *core);
has no user. Between it and the definition are rocket_poll_timer_fn(),
rocket_job_next_locked() and rocket_poll_work_fn(), none of which call
it, and the only caller, rocket_job_irq_handler_thread(), sits below the
definition. A leftover from the earlier shape where the poll work called
handle_irq() directly, I think.
Igor
On Wed, Aug 5, 2026 at 8:39 AM Jiaxing Hu <gahing@gahingwoo.com> wrote:
>
> The RK3576 carries the same RKNN block as the RK3588, with two cores
> instead of three and a few platform differences:
>
> - the CBUF (convolution buffer) has its own clock domain, so the core
> needs six clocks rather than four;
> - the BIU reset moved into the power domain, leaving one reset here;
> - the NPU spans two power domains, and a device with more than one is
> skipped by the driver-core single-domain auto-attach, so the list has
> to be attached explicitly;
> - the DPU completion interrupt is armed exactly as on RK3588 but never
> reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
> so sample that from an hrtimer rather than wait for an interrupt that
> does not come. The interrupt stays armed, so if it ever does arrive
> the normal handler finalises the job first.
>
> Select all of that from of_device_id match data so the RK3588 path keeps
> its existing counts and behaviour unchanged.
>
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
> drivers/accel/rocket/rocket_core.c | 26 +++++-
> drivers/accel/rocket/rocket_core.h | 20 ++++-
> drivers/accel/rocket/rocket_device.c | 4 +
> drivers/accel/rocket/rocket_drv.c | 22 ++++-
> drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
> 5 files changed, 176 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba..e08288c8c 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -8,6 +8,7 @@
> #include <linux/err.h>
> #include <linux/iommu.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
>
> @@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
> u32 version;
> int err = 0;
>
> + /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
> core->resets[0].id = "srst_a";
> core->resets[1].id = "srst_h";
> - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> core->resets);
> if (err)
> return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
>
> - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> + core->clks[0].id = "aclk";
> + core->clks[1].id = "hclk";
> + core->clks[2].id = "npu";
> + core->clks[3].id = "pclk";
> + /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
> + core->clks[4].id = "aclk_cbuf";
> + core->clks[5].id = "hclk_cbuf";
> + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> if (err)
> return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
>
> @@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
> return err;
> }
>
> + /*
> + * RK3576 spans two power domains, and a multi-domain device is skipped
> + * by the driver-core single-domain auto-attach, so attach the list here.
> + */
> + if (core->soc->multi_power_domain) {
> + struct dev_pm_domain_list *pd_list;
> +
> + err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> + if (err < 0)
> + return dev_err_probe(dev, err,
> + "failed to attach NPU power domains\n");
> + }
> +
> pm_runtime_use_autosuspend(dev);
>
> /*
> diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> index f6d738285..205ff070d 100644
> --- a/drivers/accel/rocket/rocket_core.h
> +++ b/drivers/accel/rocket/rocket_core.h
> @@ -6,6 +6,7 @@
>
> #include <drm/gpu_scheduler.h>
> #include <linux/clk.h>
> +#include <linux/hrtimer.h>
> #include <linux/io.h>
> #include <linux/mutex_types.h>
> #include <linux/reset.h>
> @@ -27,16 +28,25 @@
> #define rocket_core_writel(core, reg, value) \
> writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
>
> +/* Per-SoC differences, selected by the of_device_id match data. */
> +struct rocket_soc_data {
> + unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
> + unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
> + bool multi_power_domain; /* device spans more than one PM domain */
> + bool poll_completion; /* completion IRQ never reaches the GIC */
> +};
> +
> struct rocket_core {
> struct device *dev;
> struct rocket_device *rdev;
> + const struct rocket_soc_data *soc;
> unsigned int index;
>
> int irq;
> void __iomem *pc_iomem;
> void __iomem *cna_iomem;
> void __iomem *core_iomem;
> - struct clk_bulk_data clks[4];
> + struct clk_bulk_data clks[6];
> struct reset_control_bulk_data resets[2];
>
> struct iommu_group *iommu_group;
> @@ -52,6 +62,14 @@ struct rocket_core {
> atomic_t pending;
> } reset;
>
> + struct hrtimer poll_timer;
> + struct work_struct poll_work;
> + atomic_t poll_active;
> + unsigned int poll_ticks;
> + unsigned int poll_seq;
> + unsigned int poll_work_seq;
> + bool poll_dying;
> +
> struct drm_gpu_scheduler sched;
> u64 fence_context;
> u64 emit_seqno;
> diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> index 46e6ee1e7..bfb00f967 100644
> --- a/drivers/accel/rocket/rocket_device.c
> +++ b/drivers/accel/rocket/rocket_device.c
> @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
> if (of_device_is_available(core_node))
> num_cores++;
>
> + for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
> + if (of_device_is_available(core_node))
> + num_cores++;
> +
> rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
> if (!rdev->cores)
> return ERR_PTR(-ENOMEM);
> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594..7f7dfa374 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
>
> rdev->cores[core].rdev = rdev;
> rdev->cores[core].dev = &pdev->dev;
> + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> rdev->cores[core].index = core;
>
> rdev->num_cores++;
> @@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
> }
> }
>
> +static const struct rocket_soc_data rk3588_soc_data = {
> + .num_clks = 4,
> + .num_resets = 2,
> + .multi_power_domain = false,
> + .poll_completion = false,
> +};
> +
> +static const struct rocket_soc_data rk3576_soc_data = {
> + .num_clks = 6,
> + .num_resets = 1,
> + .multi_power_domain = true,
> + .poll_completion = true,
> +};
> +
> static const struct of_device_id dt_match[] = {
> - { .compatible = "rockchip,rk3588-rknn-core" },
> + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
> + { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
> @@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> if (core < 0)
> return -ENODEV;
>
> - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> if (err) {
> dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> return err;
> @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> if (!rocket_job_is_idle(&rdev->cores[core]))
> return -EBUSY;
>
> - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
>
> return 0;
> }
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0..28845ac4e 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -7,6 +7,7 @@
> #include <drm/drm_file.h>
> #include <drm/drm_gem.h>
> #include <drm/rocket_accel.h>
> +#include <linux/hrtimer.h>
> #include <linux/interrupt.h>
> #include <linux/overflow.h>
> #include <linux/iommu.h>
> @@ -21,6 +22,15 @@
>
> #define JOB_TIMEOUT_MS 500
>
> +/*
> + * RK3576 arms the same DPU completion as RK3588, but the interrupt never
> + * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
> + * so sample that instead. The tick cap bounds jobs that never raise it at all,
> + * which is the same open problem as the wrong inference results.
> + */
> +#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
> +#define RK3576_POLL_MAX_TICKS 8
> +
> static struct rocket_job *
> to_rocket_job(struct drm_sched_job *sched_job)
> {
> @@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
>
> rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
>
> + if (core->soc->poll_completion) {
> + core->poll_ticks = 0;
> + core->poll_seq++;
> + atomic_set(&core->poll_active, 1);
> + hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> + HRTIMER_MODE_REL);
> + }
> +
> dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> }
>
> @@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> return ERR_PTR(ret);
> }
>
> +static void rocket_job_handle_irq(struct rocket_core *core);
> +
> +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> +{
> + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> + u32 raw;
> +
> + if (!atomic_read(&core->poll_active))
> + return HRTIMER_NORESTART;
> +
> + core->poll_work_seq = core->poll_seq;
> +
> + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> + atomic_set(&core->poll_active, 0);
> + schedule_work(&core->poll_work);
> + return HRTIMER_NORESTART;
> + }
> +
> + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> + return HRTIMER_RESTART;
> +}
> +
> +/* Start the job's next task, or retire it. Caller holds job_lock. */
> +static void rocket_job_next_locked(struct rocket_core *core)
> +{
> + lockdep_assert_held(&core->job_lock);
> +
> + if (!core->in_flight_job)
> + return;
> +
> + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> + rocket_job_hw_submit(core, core->in_flight_job);
> + return;
> + }
> +
> + iommu_detach_group(NULL, iommu_group_get(core->dev));
> + dma_fence_signal(core->in_flight_job->done_fence);
> + pm_runtime_put_autosuspend(core->dev);
> + core->in_flight_job = NULL;
> +}
> +
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> + pm_runtime_mark_last_busy(core->dev);
> +
> + scoped_guard(mutex, &core->job_lock) {
> + /*
> + * The interrupt can land while this work is queued, retire the job
> + * and start the next task. poll_seq only moves under job_lock, in
> + * hw_submit, so comparing it here says whether that happened. Doing
> + * it outside the lock would leave the window open rather than close
> + * it, and this work would then submit a task on top of a live one.
> + */
> + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> + return;
> +
> + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> +
> + rocket_job_next_locked(core);
> + }
> +}
> +
> static void rocket_job_handle_irq(struct rocket_core *core)
> {
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + }
> +
> pm_runtime_mark_last_busy(core->dev);
>
> rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
>
> scoped_guard(mutex, &core->job_lock)
> - if (core->in_flight_job) {
> - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> - rocket_job_hw_submit(core, core->in_flight_job);
> - return;
> - }
> -
> - iommu_detach_group(NULL, iommu_group_get(core->dev));
> - dma_fence_signal(core->in_flight_job->done_fence);
> - pm_runtime_put_autosuspend(core->dev);
> - core->in_flight_job = NULL;
> - }
> + rocket_job_next_locked(core);
> }
>
> static void
> @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> int ret;
>
> INIT_WORK(&core->reset.work, rocket_reset_work);
> + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> + HRTIMER_MODE_REL);
> + atomic_set(&core->poll_active, 0);
> spin_lock_init(&core->fence_lock);
> mutex_init(&core->job_lock);
>
> @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
>
> void rocket_job_fini(struct rocket_core *core)
> {
> + /*
> + * Stop the poll from starting hardware work before tearing anything
> + * down: it submits the next task, and drm_sched_fini() does not wait
> + * for work already queued. Cancel after the scheduler is gone, so a
> + * job running now cannot re-arm the timer behind the cancel.
> + */
> + if (core->soc->poll_completion)
> + WRITE_ONCE(core->poll_dying, true);
> +
> drm_sched_fini(&core->sched);
>
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + cancel_work_sync(&core->poll_work);
> + }
> +
> cancel_work_sync(&core->reset.work);
> destroy_workqueue(core->reset.wq);
> }
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 6:38 ` [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
2026-08-05 7:39 ` sashiko-bot
2026-08-05 10:34 ` Igor Paunovic
@ 2026-08-05 12:52 ` Diederik de Haas
2026-08-05 14:06 ` Igor Paunovic
2 siblings, 1 reply; 21+ messages in thread
From: Diederik de Haas @ 2026-08-05 12:52 UTC (permalink / raw)
To: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel
On Wed Aug 5, 2026 at 8:38 AM CEST, Jiaxing Hu wrote:
> The RK3576 carries the same RKNN block as the RK3588, with two cores
> instead of three and a few platform differences:
>
> - the CBUF (convolution buffer) has its own clock domain, so the core
> needs six clocks rather than four;
> - the BIU reset moved into the power domain, leaving one reset here;
> - the NPU spans two power domains, and a device with more than one is
> skipped by the driver-core single-domain auto-attach, so the list has
> to be attached explicitly;
> - the DPU completion interrupt is armed exactly as on RK3588 but never
> reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
> so sample that from an hrtimer rather than wait for an interrupt that
> does not come. The interrupt stays armed, so if it ever does arrive
> the normal handler finalises the job first.
>
> Select all of that from of_device_id match data so the RK3588 path keeps
> its existing counts and behaviour unchanged.
Would it make sense to split this patch up in 2 patches:
1) Prepare for different NPU/RKNN implementations
2) Add support for RK3576 NPU/RKNN
It seems like you're changing the implementation for RK3588 in this patch
AND add support for RK3576. It's often better to split those things out.
Cheers,
Diederik
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
> drivers/accel/rocket/rocket_core.c | 26 +++++-
> drivers/accel/rocket/rocket_core.h | 20 ++++-
> drivers/accel/rocket/rocket_device.c | 4 +
> drivers/accel/rocket/rocket_drv.c | 22 ++++-
> drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
> 5 files changed, 176 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba..e08288c8c 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -8,6 +8,7 @@
> #include <linux/err.h>
> #include <linux/iommu.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
>
> @@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
> u32 version;
> int err = 0;
>
> + /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
> core->resets[0].id = "srst_a";
> core->resets[1].id = "srst_h";
> - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> core->resets);
> if (err)
> return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
>
> - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> + core->clks[0].id = "aclk";
> + core->clks[1].id = "hclk";
> + core->clks[2].id = "npu";
> + core->clks[3].id = "pclk";
> + /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
> + core->clks[4].id = "aclk_cbuf";
> + core->clks[5].id = "hclk_cbuf";
> + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> if (err)
> return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
>
> @@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
> return err;
> }
>
> + /*
> + * RK3576 spans two power domains, and a multi-domain device is skipped
> + * by the driver-core single-domain auto-attach, so attach the list here.
> + */
> + if (core->soc->multi_power_domain) {
> + struct dev_pm_domain_list *pd_list;
> +
> + err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> + if (err < 0)
> + return dev_err_probe(dev, err,
> + "failed to attach NPU power domains\n");
> + }
> +
> pm_runtime_use_autosuspend(dev);
>
> /*
> diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> index f6d738285..205ff070d 100644
> --- a/drivers/accel/rocket/rocket_core.h
> +++ b/drivers/accel/rocket/rocket_core.h
> @@ -6,6 +6,7 @@
>
> #include <drm/gpu_scheduler.h>
> #include <linux/clk.h>
> +#include <linux/hrtimer.h>
> #include <linux/io.h>
> #include <linux/mutex_types.h>
> #include <linux/reset.h>
> @@ -27,16 +28,25 @@
> #define rocket_core_writel(core, reg, value) \
> writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
>
> +/* Per-SoC differences, selected by the of_device_id match data. */
> +struct rocket_soc_data {
> + unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
> + unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
> + bool multi_power_domain; /* device spans more than one PM domain */
> + bool poll_completion; /* completion IRQ never reaches the GIC */
> +};
> +
> struct rocket_core {
> struct device *dev;
> struct rocket_device *rdev;
> + const struct rocket_soc_data *soc;
> unsigned int index;
>
> int irq;
> void __iomem *pc_iomem;
> void __iomem *cna_iomem;
> void __iomem *core_iomem;
> - struct clk_bulk_data clks[4];
> + struct clk_bulk_data clks[6];
> struct reset_control_bulk_data resets[2];
>
> struct iommu_group *iommu_group;
> @@ -52,6 +62,14 @@ struct rocket_core {
> atomic_t pending;
> } reset;
>
> + struct hrtimer poll_timer;
> + struct work_struct poll_work;
> + atomic_t poll_active;
> + unsigned int poll_ticks;
> + unsigned int poll_seq;
> + unsigned int poll_work_seq;
> + bool poll_dying;
> +
> struct drm_gpu_scheduler sched;
> u64 fence_context;
> u64 emit_seqno;
> diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> index 46e6ee1e7..bfb00f967 100644
> --- a/drivers/accel/rocket/rocket_device.c
> +++ b/drivers/accel/rocket/rocket_device.c
> @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
> if (of_device_is_available(core_node))
> num_cores++;
>
> + for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
> + if (of_device_is_available(core_node))
> + num_cores++;
> +
> rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
> if (!rdev->cores)
> return ERR_PTR(-ENOMEM);
> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594..7f7dfa374 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
>
> rdev->cores[core].rdev = rdev;
> rdev->cores[core].dev = &pdev->dev;
> + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> rdev->cores[core].index = core;
>
> rdev->num_cores++;
> @@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
> }
> }
>
> +static const struct rocket_soc_data rk3588_soc_data = {
> + .num_clks = 4,
> + .num_resets = 2,
> + .multi_power_domain = false,
> + .poll_completion = false,
> +};
> +
> +static const struct rocket_soc_data rk3576_soc_data = {
> + .num_clks = 6,
> + .num_resets = 1,
> + .multi_power_domain = true,
> + .poll_completion = true,
> +};
> +
> static const struct of_device_id dt_match[] = {
> - { .compatible = "rockchip,rk3588-rknn-core" },
> + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
> + { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
> @@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> if (core < 0)
> return -ENODEV;
>
> - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> if (err) {
> dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> return err;
> @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> if (!rocket_job_is_idle(&rdev->cores[core]))
> return -EBUSY;
>
> - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
>
> return 0;
> }
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0..28845ac4e 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -7,6 +7,7 @@
> #include <drm/drm_file.h>
> #include <drm/drm_gem.h>
> #include <drm/rocket_accel.h>
> +#include <linux/hrtimer.h>
> #include <linux/interrupt.h>
> #include <linux/overflow.h>
> #include <linux/iommu.h>
> @@ -21,6 +22,15 @@
>
> #define JOB_TIMEOUT_MS 500
>
> +/*
> + * RK3576 arms the same DPU completion as RK3588, but the interrupt never
> + * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
> + * so sample that instead. The tick cap bounds jobs that never raise it at all,
> + * which is the same open problem as the wrong inference results.
> + */
> +#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
> +#define RK3576_POLL_MAX_TICKS 8
> +
> static struct rocket_job *
> to_rocket_job(struct drm_sched_job *sched_job)
> {
> @@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
>
> rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
>
> + if (core->soc->poll_completion) {
> + core->poll_ticks = 0;
> + core->poll_seq++;
> + atomic_set(&core->poll_active, 1);
> + hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> + HRTIMER_MODE_REL);
> + }
> +
> dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> }
>
> @@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> return ERR_PTR(ret);
> }
>
> +static void rocket_job_handle_irq(struct rocket_core *core);
> +
> +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> +{
> + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> + u32 raw;
> +
> + if (!atomic_read(&core->poll_active))
> + return HRTIMER_NORESTART;
> +
> + core->poll_work_seq = core->poll_seq;
> +
> + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> + atomic_set(&core->poll_active, 0);
> + schedule_work(&core->poll_work);
> + return HRTIMER_NORESTART;
> + }
> +
> + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> + return HRTIMER_RESTART;
> +}
> +
> +/* Start the job's next task, or retire it. Caller holds job_lock. */
> +static void rocket_job_next_locked(struct rocket_core *core)
> +{
> + lockdep_assert_held(&core->job_lock);
> +
> + if (!core->in_flight_job)
> + return;
> +
> + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> + rocket_job_hw_submit(core, core->in_flight_job);
> + return;
> + }
> +
> + iommu_detach_group(NULL, iommu_group_get(core->dev));
> + dma_fence_signal(core->in_flight_job->done_fence);
> + pm_runtime_put_autosuspend(core->dev);
> + core->in_flight_job = NULL;
> +}
> +
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> + pm_runtime_mark_last_busy(core->dev);
> +
> + scoped_guard(mutex, &core->job_lock) {
> + /*
> + * The interrupt can land while this work is queued, retire the job
> + * and start the next task. poll_seq only moves under job_lock, in
> + * hw_submit, so comparing it here says whether that happened. Doing
> + * it outside the lock would leave the window open rather than close
> + * it, and this work would then submit a task on top of a live one.
> + */
> + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> + return;
> +
> + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> +
> + rocket_job_next_locked(core);
> + }
> +}
> +
> static void rocket_job_handle_irq(struct rocket_core *core)
> {
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + }
> +
> pm_runtime_mark_last_busy(core->dev);
>
> rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
>
> scoped_guard(mutex, &core->job_lock)
> - if (core->in_flight_job) {
> - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> - rocket_job_hw_submit(core, core->in_flight_job);
> - return;
> - }
> -
> - iommu_detach_group(NULL, iommu_group_get(core->dev));
> - dma_fence_signal(core->in_flight_job->done_fence);
> - pm_runtime_put_autosuspend(core->dev);
> - core->in_flight_job = NULL;
> - }
> + rocket_job_next_locked(core);
> }
>
> static void
> @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> int ret;
>
> INIT_WORK(&core->reset.work, rocket_reset_work);
> + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> + HRTIMER_MODE_REL);
> + atomic_set(&core->poll_active, 0);
> spin_lock_init(&core->fence_lock);
> mutex_init(&core->job_lock);
>
> @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
>
> void rocket_job_fini(struct rocket_core *core)
> {
> + /*
> + * Stop the poll from starting hardware work before tearing anything
> + * down: it submits the next task, and drm_sched_fini() does not wait
> + * for work already queued. Cancel after the scheduler is gone, so a
> + * job running now cannot re-arm the timer behind the cancel.
> + */
> + if (core->soc->poll_completion)
> + WRITE_ONCE(core->poll_dying, true);
> +
> drm_sched_fini(&core->sched);
>
> + if (core->soc->poll_completion) {
> + atomic_set(&core->poll_active, 0);
> + hrtimer_cancel(&core->poll_timer);
> + cancel_work_sync(&core->poll_work);
> + }
> +
> cancel_work_sync(&core->reset.work);
> destroy_workqueue(core->reset.wq);
> }
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 12:52 ` Diederik de Haas
@ 2026-08-05 14:06 ` Igor Paunovic
2026-08-05 14:07 ` Igor Paunovic
0 siblings, 1 reply; 21+ messages in thread
From: Igor Paunovic @ 2026-08-05 14:06 UTC (permalink / raw)
To: Diederik de Haas
Cc: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, p.zabel, ogabbay, zhangqing, alchark,
chaoyi.chen, dri-devel, linux-rockchip, iommu, linux-pm,
devicetree, linux-arm-kernel, linux-kernel
Hi Jiaxing,
Here is the RK3588 characterisation I promised on the cover letter. Short
version: I cannot measure any effect from this patch on RK3588. The output
is bit-identical and the interrupt accounting is unchanged to within
0.0001 interrupts per inference.
What was compared
-----------------
Both sides were built from the same tree, from the same headers, with only
your 6/8 between them:
base: v7.2-rc6 rocket + the four fixes I have in flight
(Guangshuo Li's "clear rdev on device init failure", my two
lifecycle patches, and "request the core clocks by name")
test: the same tree + this patch, hand resolved, see below
running kernel 7.2.0-rc6-rk3588-igor-claude+, tree pinned at Collabora
rockchip-devel 8c70065a3207, which sits on 075b74841bd0 = Linux 7.2-rc6.
Not a v7.2 tag; that does not exist yet.
base module srcversion 97DECECF73741ABE56685D2
test module srcversion 57A4D5369E495870E651EEC
I deliberately left my local devfreq work out of both sides. It touches
rocket_job.c, you do not have it, and it would have changed the clock
during the runs. The consequence is that the NPU ran at the firmware
default rate rather than the rate I normally use, so the absolute numbers
below are around 89 inf/s where I would usually quote about 222. That does
not matter for a difference measurement, but it means these figures are not
comparable to any throughput number I have posted before.
One thing worth reporting before the numbers: 6/8 does not apply cleanly on
that base, for two separate reasons, and only one of them concerns you and
me.
The first is the coordination point from my earlier mail, now measured
rather than predicted. The clks[].id hunk in rocket_core.c conflicts with
my standalone "request the core clocks by name" fix. I resolved it by hand,
keeping your two CBUF entries and the soc->num_clks and soc->num_resets
changes on top of the four named assignments that are already there.
The second is not a conflict with anything I carry. The rocket_job.c hunk
has "#include <linux/overflow.h>" in its context, and that line is neither
in v7.2-rc6 rocket_job.c nor added by any patch in this series, so your
tree is simply newer than v7.2-rc6 there. Mentioning it so that a clean
apply on your side does not get read as a contradiction of the first point.
That means this Tested-by is on the hand resolved form of 6/8, not on the
posted hunks applied verbatim.
Method
------
Interleaved, with the order alternated per pair, because my first attempt
was not interleaved at all. That attempt measured base and test in separate
blocks and showed a 1.6% difference on the NPU part, once the first run of
each block is dropped. It is not your patch. The two blocks ran under
measurably different conditions: NPU thermals 43.5-49.0 C and one minute
load average 2.9-4.4 during the base block, against 39.8-44.4 C and 2.0-3.1
during the test block. And one test run behaved like a base run: same
module, CPU part 1.87 ms, where the other three test runs sat at 1.67-1.68
ms. I believe that was my desktop client compositing while I typed in it,
but I did not isolate it as a controlled variable, so what I can actually
say is that the block to block gap was environmental, and that the
interleaved rerun below shows nothing.
So: three pairs, order base/test, then test/base, then base/test, module
swapped between every single run, no interactive use of the machine, only
the usual system timers. One honesty note on the design: the alternation
only balances fully over an even number of pairs, and I ended up with
three, so base occupies series positions 1, 4 and 5 against test's 2, 3 and
6. The series does drift slowly downwards, NPU thermals falling from 39.8 C
to 37.9 C over the six runs, and that drift is therefore not entirely
cancelled. It is well below the effect size discussed below, but it is
there.
Environment held constant: CPU governor performance at 1800/2400/2400 MHz,
vdd_npu_s0 at 850 mV, NPU thermals 37.9-39.8 C. Those thermals are
snapshots taken at the start and end of each run, not a continuous trace.
MobileNetV1 through the Teflon TFLite delegate, effectively one core, 20 s
x 3 blocks per run, per-run block spread 0.29-1.09%.
Results, 3 runs per side
------------------------
base test delta
throughput 89.0093 +-0.3568 88.9546 +-0.2270 -0.06% inf/s
NPU part 9.7521 +-0.0434 9.7570 +-0.0364 +0.05% ms/inference
CPU part 1.4828 +-0.0022 1.4847 +-0.0106 +0.13% ms/inference
The largest standard deviation on the NPU part is 0.0434 ms, 0.45% of the
mean. The measured difference is 0.0049 ms, 0.05%, about an eighth of that
standard deviation. This does not say the patch costs nothing; it says any
cost is below what this bench resolves. With three runs per side the 95%
confidence bound on the difference is about +-0.9% of the mean, so one
percent is the honest resolution figure, not half a percent.
Interrupt accounting per inference:
base test delta
irq 99 41.11512 +-0.00046 41.11519 +-0.00029 +0.000071
irq 102 1.00281 +-0.00001 1.00281 +-0.00001 +0.000002
irq 103 0.00000 +-0.00000 0.00000 +-0.00000 0
These are integer counters. The residual scatter is a window alignment
artefact, because the counter window does not line up exactly with the
counted iterations, which is also why the figure is 41.115 rather than 41.
It is three orders of magnitude below one interrupt per inference, so a
dropped or a doubled task boundary would be unmissable. This is the number
I care about most: the restructured completion path neither double counts
nor drops a task boundary. For this model 41 completions per inference land
on core 0 and one on core 1; irq 103 stays at zero because the third core
is idle in this workload.
The bit exact oracle passed on every run, and the reference hashes over the
five tracked tensors are identical across all six runs, 32044 inferences
in total. Top-1 unchanged at "military uniform", 0.867.
The module was loaded 13 times over the session, reporting "[drm]
Initialized rocket 0.0.0 for rknn on minor 0" with minor 0 every time, and
there is no rocket error or warning anywhere in that boot. Twelve of those
loads were unloaded normally. The thirteenth was still resident when the
machine stopped, which is the last caveat below.
What I did not test, and why
----------------------------
Three things, and they are not the same kind of "not".
The poll_dying latch and rocket_poll_work_fn() cannot be exercised on this
SoC at all. The flag is written only under
if (core->soc->poll_completion)
WRITE_ONCE(core->poll_dying, true);
poll_completion is false in rk3588_soc_data, and the poll timer is started
under the same guard in rocket_job_hw_submit(), so neither the timer
function nor the work function can run here. The fini path race I raised on
this patch needs RK3576 for the same reason.
The per-core unbind/rebind I promised on the cover was runnable here and I
did not run it. That distinction matters, so: the rocket platform driver
exposes bind and unbind in sysfs, all three cores are bound here, and
unbinding one while the other two stay bound does reach patched code on
RK3588, through rocket_core_fini() and rocket_job_fini() on the way out,
and soc->num_clks and soc->num_resets on the way back in. That is not
covered by an insmod/rmmod of the whole module. It is a gap in my testing,
not a limitation of the hardware, and I will run it before v6 if you want
it.
The module unload under load I also promised is not runnable in that form:
rmmod is refused by the module refcount while a DRM fd is open, and my
harness declines the swap rather than fight it. The runnable equivalent is
an unbind under load, which I also did not run. All 13 loads above happened
between benches with nothing attached.
So the Tested-by covers what I actually measured: that this patch does not
regress the RK3588 path, in timing, in interrupt accounting or in output.
The RK3576 specific paths are untested by me.
Two caveats on the numbers.
First, three runs per side rather than four. The machine went down partway
through the fourth pair, roughly 25 seconds into the test side, with your
module loaded and inference in flight. I am describing it that way because
that is what the logs support: the journal for that boot ends mid-run with
no shutdown sequence at all, the bench's output file for that run is zero
bytes, my script's exit trap never ran, and the following boot logs an
orphan cleanup and a journal marked uncleanly shut down. Whether the
desktop or the whole machine went first is not recorded. There is no oops,
no kernel WARNING, no call trace and no rocket error anywhere in that boot,
and nothing that ties it to this patch, but it did happen with this patch
loaded and I have no explanation for it. I power cycled and did not
resume the series rather than mix a rebooted machine into it. I would
rather report it than not.
Second, the resolution bound of about one percent is a property of my
bench, not of the patch.
If there is an ordering or a workload you would rather I ran, say so and I
will run it.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, Orange Pi 5 Plus
Igor
On Wed, Aug 5, 2026 at 2:52 PM Diederik de Haas <diederik@cknow-tech.com> wrote:
>
> On Wed Aug 5, 2026 at 8:38 AM CEST, Jiaxing Hu wrote:
> > The RK3576 carries the same RKNN block as the RK3588, with two cores
> > instead of three and a few platform differences:
> >
> > - the CBUF (convolution buffer) has its own clock domain, so the core
> > needs six clocks rather than four;
> > - the BIU reset moved into the power domain, leaving one reset here;
> > - the NPU spans two power domains, and a device with more than one is
> > skipped by the driver-core single-domain auto-attach, so the list has
> > to be attached explicitly;
> > - the DPU completion interrupt is armed exactly as on RK3588 but never
> > reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
> > so sample that from an hrtimer rather than wait for an interrupt that
> > does not come. The interrupt stays armed, so if it ever does arrive
> > the normal handler finalises the job first.
> >
> > Select all of that from of_device_id match data so the RK3588 path keeps
> > its existing counts and behaviour unchanged.
>
> Would it make sense to split this patch up in 2 patches:
> 1) Prepare for different NPU/RKNN implementations
> 2) Add support for RK3576 NPU/RKNN
>
> It seems like you're changing the implementation for RK3588 in this patch
> AND add support for RK3576. It's often better to split those things out.
>
> Cheers,
> Diederik
>
> > Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> > ---
> > drivers/accel/rocket/rocket_core.c | 26 +++++-
> > drivers/accel/rocket/rocket_core.h | 20 ++++-
> > drivers/accel/rocket/rocket_device.c | 4 +
> > drivers/accel/rocket/rocket_drv.c | 22 ++++-
> > drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
> > 5 files changed, 176 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> > index b3b2fa9ba..e08288c8c 100644
> > --- a/drivers/accel/rocket/rocket_core.c
> > +++ b/drivers/accel/rocket/rocket_core.c
> > @@ -8,6 +8,7 @@
> > #include <linux/err.h>
> > #include <linux/iommu.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/reset.h>
> >
> > @@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
> > u32 version;
> > int err = 0;
> >
> > + /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
> > core->resets[0].id = "srst_a";
> > core->resets[1].id = "srst_h";
> > - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> > + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> > core->resets);
> > if (err)
> > return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
> >
> > - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> > + core->clks[0].id = "aclk";
> > + core->clks[1].id = "hclk";
> > + core->clks[2].id = "npu";
> > + core->clks[3].id = "pclk";
> > + /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
> > + core->clks[4].id = "aclk_cbuf";
> > + core->clks[5].id = "hclk_cbuf";
> > + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> > if (err)
> > return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
> >
> > @@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
> > return err;
> > }
> >
> > + /*
> > + * RK3576 spans two power domains, and a multi-domain device is skipped
> > + * by the driver-core single-domain auto-attach, so attach the list here.
> > + */
> > + if (core->soc->multi_power_domain) {
> > + struct dev_pm_domain_list *pd_list;
> > +
> > + err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> > + if (err < 0)
> > + return dev_err_probe(dev, err,
> > + "failed to attach NPU power domains\n");
> > + }
> > +
> > pm_runtime_use_autosuspend(dev);
> >
> > /*
> > diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> > index f6d738285..205ff070d 100644
> > --- a/drivers/accel/rocket/rocket_core.h
> > +++ b/drivers/accel/rocket/rocket_core.h
> > @@ -6,6 +6,7 @@
> >
> > #include <drm/gpu_scheduler.h>
> > #include <linux/clk.h>
> > +#include <linux/hrtimer.h>
> > #include <linux/io.h>
> > #include <linux/mutex_types.h>
> > #include <linux/reset.h>
> > @@ -27,16 +28,25 @@
> > #define rocket_core_writel(core, reg, value) \
> > writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
> >
> > +/* Per-SoC differences, selected by the of_device_id match data. */
> > +struct rocket_soc_data {
> > + unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
> > + unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
> > + bool multi_power_domain; /* device spans more than one PM domain */
> > + bool poll_completion; /* completion IRQ never reaches the GIC */
> > +};
> > +
> > struct rocket_core {
> > struct device *dev;
> > struct rocket_device *rdev;
> > + const struct rocket_soc_data *soc;
> > unsigned int index;
> >
> > int irq;
> > void __iomem *pc_iomem;
> > void __iomem *cna_iomem;
> > void __iomem *core_iomem;
> > - struct clk_bulk_data clks[4];
> > + struct clk_bulk_data clks[6];
> > struct reset_control_bulk_data resets[2];
> >
> > struct iommu_group *iommu_group;
> > @@ -52,6 +62,14 @@ struct rocket_core {
> > atomic_t pending;
> > } reset;
> >
> > + struct hrtimer poll_timer;
> > + struct work_struct poll_work;
> > + atomic_t poll_active;
> > + unsigned int poll_ticks;
> > + unsigned int poll_seq;
> > + unsigned int poll_work_seq;
> > + bool poll_dying;
> > +
> > struct drm_gpu_scheduler sched;
> > u64 fence_context;
> > u64 emit_seqno;
> > diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> > index 46e6ee1e7..bfb00f967 100644
> > --- a/drivers/accel/rocket/rocket_device.c
> > +++ b/drivers/accel/rocket/rocket_device.c
> > @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
> > if (of_device_is_available(core_node))
> > num_cores++;
> >
> > + for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
> > + if (of_device_is_available(core_node))
> > + num_cores++;
> > +
> > rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
> > if (!rdev->cores)
> > return ERR_PTR(-ENOMEM);
> > diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> > index 8bbbce594..7f7dfa374 100644
> > --- a/drivers/accel/rocket/rocket_drv.c
> > +++ b/drivers/accel/rocket/rocket_drv.c
> > @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
> >
> > rdev->cores[core].rdev = rdev;
> > rdev->cores[core].dev = &pdev->dev;
> > + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> > rdev->cores[core].index = core;
> >
> > rdev->num_cores++;
> > @@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
> > }
> > }
> >
> > +static const struct rocket_soc_data rk3588_soc_data = {
> > + .num_clks = 4,
> > + .num_resets = 2,
> > + .multi_power_domain = false,
> > + .poll_completion = false,
> > +};
> > +
> > +static const struct rocket_soc_data rk3576_soc_data = {
> > + .num_clks = 6,
> > + .num_resets = 1,
> > + .multi_power_domain = true,
> > + .poll_completion = true,
> > +};
> > +
> > static const struct of_device_id dt_match[] = {
> > - { .compatible = "rockchip,rk3588-rknn-core" },
> > + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
> > + { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, dt_match);
> > @@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> > if (core < 0)
> > return -ENODEV;
> >
> > - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> > if (err) {
> > dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> > return err;
> > @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> > if (!rocket_job_is_idle(&rdev->cores[core]))
> > return -EBUSY;
> >
> > - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> >
> > return 0;
> > }
> > diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> > index bb77b6bf0..28845ac4e 100644
> > --- a/drivers/accel/rocket/rocket_job.c
> > +++ b/drivers/accel/rocket/rocket_job.c
> > @@ -7,6 +7,7 @@
> > #include <drm/drm_file.h>
> > #include <drm/drm_gem.h>
> > #include <drm/rocket_accel.h>
> > +#include <linux/hrtimer.h>
> > #include <linux/interrupt.h>
> > #include <linux/overflow.h>
> > #include <linux/iommu.h>
> > @@ -21,6 +22,15 @@
> >
> > #define JOB_TIMEOUT_MS 500
> >
> > +/*
> > + * RK3576 arms the same DPU completion as RK3588, but the interrupt never
> > + * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
> > + * so sample that instead. The tick cap bounds jobs that never raise it at all,
> > + * which is the same open problem as the wrong inference results.
> > + */
> > +#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
> > +#define RK3576_POLL_MAX_TICKS 8
> > +
> > static struct rocket_job *
> > to_rocket_job(struct drm_sched_job *sched_job)
> > {
> > @@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
> >
> > rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
> >
> > + if (core->soc->poll_completion) {
> > + core->poll_ticks = 0;
> > + core->poll_seq++;
> > + atomic_set(&core->poll_active, 1);
> > + hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> > + HRTIMER_MODE_REL);
> > + }
> > +
> > dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> > }
> >
> > @@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> > return ERR_PTR(ret);
> > }
> >
> > +static void rocket_job_handle_irq(struct rocket_core *core);
> > +
> > +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> > +{
> > + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> > + u32 raw;
> > +
> > + if (!atomic_read(&core->poll_active))
> > + return HRTIMER_NORESTART;
> > +
> > + core->poll_work_seq = core->poll_seq;
> > +
> > + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> > + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> > + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> > + atomic_set(&core->poll_active, 0);
> > + schedule_work(&core->poll_work);
> > + return HRTIMER_NORESTART;
> > + }
> > +
> > + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> > + return HRTIMER_RESTART;
> > +}
> > +
> > +/* Start the job's next task, or retire it. Caller holds job_lock. */
> > +static void rocket_job_next_locked(struct rocket_core *core)
> > +{
> > + lockdep_assert_held(&core->job_lock);
> > +
> > + if (!core->in_flight_job)
> > + return;
> > +
> > + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > + rocket_job_hw_submit(core, core->in_flight_job);
> > + return;
> > + }
> > +
> > + iommu_detach_group(NULL, iommu_group_get(core->dev));
> > + dma_fence_signal(core->in_flight_job->done_fence);
> > + pm_runtime_put_autosuspend(core->dev);
> > + core->in_flight_job = NULL;
> > +}
> > +
> > +static void rocket_poll_work_fn(struct work_struct *work)
> > +{
> > + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> > +
> > + pm_runtime_mark_last_busy(core->dev);
> > +
> > + scoped_guard(mutex, &core->job_lock) {
> > + /*
> > + * The interrupt can land while this work is queued, retire the job
> > + * and start the next task. poll_seq only moves under job_lock, in
> > + * hw_submit, so comparing it here says whether that happened. Doing
> > + * it outside the lock would leave the window open rather than close
> > + * it, and this work would then submit a task on top of a live one.
> > + */
> > + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> > + return;
> > +
> > + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> > +
> > + rocket_job_next_locked(core);
> > + }
> > +}
> > +
> > static void rocket_job_handle_irq(struct rocket_core *core)
> > {
> > + if (core->soc->poll_completion) {
> > + atomic_set(&core->poll_active, 0);
> > + hrtimer_cancel(&core->poll_timer);
> > + }
> > +
> > pm_runtime_mark_last_busy(core->dev);
> >
> > rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> >
> > scoped_guard(mutex, &core->job_lock)
> > - if (core->in_flight_job) {
> > - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > - rocket_job_hw_submit(core, core->in_flight_job);
> > - return;
> > - }
> > -
> > - iommu_detach_group(NULL, iommu_group_get(core->dev));
> > - dma_fence_signal(core->in_flight_job->done_fence);
> > - pm_runtime_put_autosuspend(core->dev);
> > - core->in_flight_job = NULL;
> > - }
> > + rocket_job_next_locked(core);
> > }
> >
> > static void
> > @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> > int ret;
> >
> > INIT_WORK(&core->reset.work, rocket_reset_work);
> > + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> > + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> > + HRTIMER_MODE_REL);
> > + atomic_set(&core->poll_active, 0);
> > spin_lock_init(&core->fence_lock);
> > mutex_init(&core->job_lock);
> >
> > @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
> >
> > void rocket_job_fini(struct rocket_core *core)
> > {
> > + /*
> > + * Stop the poll from starting hardware work before tearing anything
> > + * down: it submits the next task, and drm_sched_fini() does not wait
> > + * for work already queued. Cancel after the scheduler is gone, so a
> > + * job running now cannot re-arm the timer behind the cancel.
> > + */
> > + if (core->soc->poll_completion)
> > + WRITE_ONCE(core->poll_dying, true);
> > +
> > drm_sched_fini(&core->sched);
> >
> > + if (core->soc->poll_completion) {
> > + atomic_set(&core->poll_active, 0);
> > + hrtimer_cancel(&core->poll_timer);
> > + cancel_work_sync(&core->poll_work);
> > + }
> > +
> > cancel_work_sync(&core->reset.work);
> > destroy_workqueue(core->reset.wq);
> > }
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 14:06 ` Igor Paunovic
@ 2026-08-05 14:07 ` Igor Paunovic
2026-08-05 14:35 ` Igor Paunovic
0 siblings, 1 reply; 21+ messages in thread
From: Igor Paunovic @ 2026-08-05 14:07 UTC (permalink / raw)
To: Diederik de Haas
Cc: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, p.zabel, ogabbay, zhangqing, alchark,
chaoyi.chen, dri-devel, linux-rockchip, iommu, linux-pm,
devicetree, linux-arm-kernel, linux-kernel
Hi Diederik,
Seconding this, with a data point that may help make the case.
I have my own fix in flight that touches the same function, "accel/rocket:
request the core clocks by name":
https://lore.kernel.org/linux-rockchip/20260729130743.128876-1-royalnet026@gmail.com/
It carries Jiaxing's Reviewed-by and a Tested-by from Sidong Yang and is
waiting on Tomeu. Earlier today I built 6/8 on top of a tree that already
has it, to characterise the RK3588 side on hardware. git apply refused the
patch outright, and with fuzz everything applied except one hunk, the
resets-and-clocks hunk in rocket_core_init(), which I had to resolve by
hand. So the two RK3588-affecting changes in this patch are already
colliding in practice, not just in principle.
That hunk is doing three separate things at once: it re-adds the four
clks[].id assignments that my standalone fix adds, it grows the array and
switches both bulk counts to soc->num_clks and soc->num_resets, and it adds
the two RK3576 CBUF entries. Only the last of those is RK3576 enablement.
The first is a bugfix that exists independently, and the middle one is
exactly the "prepare for different implementations" step you are asking
for.
Split the way you suggest, the ordering problem goes away on its own. The
preparation patch would carry the soc_data plumbing and the count changes,
rebased on whatever landed first, and the RK3576 patch would carry the CBUF
entries and the poll path. Nothing would need to be sequenced by hand, and
the commit message could stop claiming the RK3588 path is behaviourally
unchanged, which is the part I raised separately on this patch and which is
only true once the bugfix is not being carried along with the enablement.
Igor
On Wed, Aug 5, 2026 at 4:06 PM Igor Paunovic <royalnet026@gmail.com> wrote:
>
> Hi Jiaxing,
>
> Here is the RK3588 characterisation I promised on the cover letter. Short
> version: I cannot measure any effect from this patch on RK3588. The output
> is bit-identical and the interrupt accounting is unchanged to within
> 0.0001 interrupts per inference.
>
> What was compared
> -----------------
>
> Both sides were built from the same tree, from the same headers, with only
> your 6/8 between them:
>
> base: v7.2-rc6 rocket + the four fixes I have in flight
> (Guangshuo Li's "clear rdev on device init failure", my two
> lifecycle patches, and "request the core clocks by name")
> test: the same tree + this patch, hand resolved, see below
>
> running kernel 7.2.0-rc6-rk3588-igor-claude+, tree pinned at Collabora
> rockchip-devel 8c70065a3207, which sits on 075b74841bd0 = Linux 7.2-rc6.
> Not a v7.2 tag; that does not exist yet.
>
> base module srcversion 97DECECF73741ABE56685D2
> test module srcversion 57A4D5369E495870E651EEC
>
> I deliberately left my local devfreq work out of both sides. It touches
> rocket_job.c, you do not have it, and it would have changed the clock
> during the runs. The consequence is that the NPU ran at the firmware
> default rate rather than the rate I normally use, so the absolute numbers
> below are around 89 inf/s where I would usually quote about 222. That does
> not matter for a difference measurement, but it means these figures are not
> comparable to any throughput number I have posted before.
>
> One thing worth reporting before the numbers: 6/8 does not apply cleanly on
> that base, for two separate reasons, and only one of them concerns you and
> me.
>
> The first is the coordination point from my earlier mail, now measured
> rather than predicted. The clks[].id hunk in rocket_core.c conflicts with
> my standalone "request the core clocks by name" fix. I resolved it by hand,
> keeping your two CBUF entries and the soc->num_clks and soc->num_resets
> changes on top of the four named assignments that are already there.
>
> The second is not a conflict with anything I carry. The rocket_job.c hunk
> has "#include <linux/overflow.h>" in its context, and that line is neither
> in v7.2-rc6 rocket_job.c nor added by any patch in this series, so your
> tree is simply newer than v7.2-rc6 there. Mentioning it so that a clean
> apply on your side does not get read as a contradiction of the first point.
>
> That means this Tested-by is on the hand resolved form of 6/8, not on the
> posted hunks applied verbatim.
>
> Method
> ------
>
> Interleaved, with the order alternated per pair, because my first attempt
> was not interleaved at all. That attempt measured base and test in separate
> blocks and showed a 1.6% difference on the NPU part, once the first run of
> each block is dropped. It is not your patch. The two blocks ran under
> measurably different conditions: NPU thermals 43.5-49.0 C and one minute
> load average 2.9-4.4 during the base block, against 39.8-44.4 C and 2.0-3.1
> during the test block. And one test run behaved like a base run: same
> module, CPU part 1.87 ms, where the other three test runs sat at 1.67-1.68
> ms. I believe that was my desktop client compositing while I typed in it,
> but I did not isolate it as a controlled variable, so what I can actually
> say is that the block to block gap was environmental, and that the
> interleaved rerun below shows nothing.
>
> So: three pairs, order base/test, then test/base, then base/test, module
> swapped between every single run, no interactive use of the machine, only
> the usual system timers. One honesty note on the design: the alternation
> only balances fully over an even number of pairs, and I ended up with
> three, so base occupies series positions 1, 4 and 5 against test's 2, 3 and
> 6. The series does drift slowly downwards, NPU thermals falling from 39.8 C
> to 37.9 C over the six runs, and that drift is therefore not entirely
> cancelled. It is well below the effect size discussed below, but it is
> there.
>
> Environment held constant: CPU governor performance at 1800/2400/2400 MHz,
> vdd_npu_s0 at 850 mV, NPU thermals 37.9-39.8 C. Those thermals are
> snapshots taken at the start and end of each run, not a continuous trace.
>
> MobileNetV1 through the Teflon TFLite delegate, effectively one core, 20 s
> x 3 blocks per run, per-run block spread 0.29-1.09%.
>
> Results, 3 runs per side
> ------------------------
>
> base test delta
> throughput 89.0093 +-0.3568 88.9546 +-0.2270 -0.06% inf/s
> NPU part 9.7521 +-0.0434 9.7570 +-0.0364 +0.05% ms/inference
> CPU part 1.4828 +-0.0022 1.4847 +-0.0106 +0.13% ms/inference
>
> The largest standard deviation on the NPU part is 0.0434 ms, 0.45% of the
> mean. The measured difference is 0.0049 ms, 0.05%, about an eighth of that
> standard deviation. This does not say the patch costs nothing; it says any
> cost is below what this bench resolves. With three runs per side the 95%
> confidence bound on the difference is about +-0.9% of the mean, so one
> percent is the honest resolution figure, not half a percent.
>
> Interrupt accounting per inference:
>
> base test delta
> irq 99 41.11512 +-0.00046 41.11519 +-0.00029 +0.000071
> irq 102 1.00281 +-0.00001 1.00281 +-0.00001 +0.000002
> irq 103 0.00000 +-0.00000 0.00000 +-0.00000 0
>
> These are integer counters. The residual scatter is a window alignment
> artefact, because the counter window does not line up exactly with the
> counted iterations, which is also why the figure is 41.115 rather than 41.
> It is three orders of magnitude below one interrupt per inference, so a
> dropped or a doubled task boundary would be unmissable. This is the number
> I care about most: the restructured completion path neither double counts
> nor drops a task boundary. For this model 41 completions per inference land
> on core 0 and one on core 1; irq 103 stays at zero because the third core
> is idle in this workload.
>
> The bit exact oracle passed on every run, and the reference hashes over the
> five tracked tensors are identical across all six runs, 32044 inferences
> in total. Top-1 unchanged at "military uniform", 0.867.
>
> The module was loaded 13 times over the session, reporting "[drm]
> Initialized rocket 0.0.0 for rknn on minor 0" with minor 0 every time, and
> there is no rocket error or warning anywhere in that boot. Twelve of those
> loads were unloaded normally. The thirteenth was still resident when the
> machine stopped, which is the last caveat below.
>
> What I did not test, and why
> ----------------------------
>
> Three things, and they are not the same kind of "not".
>
> The poll_dying latch and rocket_poll_work_fn() cannot be exercised on this
> SoC at all. The flag is written only under
>
> if (core->soc->poll_completion)
> WRITE_ONCE(core->poll_dying, true);
>
> poll_completion is false in rk3588_soc_data, and the poll timer is started
> under the same guard in rocket_job_hw_submit(), so neither the timer
> function nor the work function can run here. The fini path race I raised on
> this patch needs RK3576 for the same reason.
>
> The per-core unbind/rebind I promised on the cover was runnable here and I
> did not run it. That distinction matters, so: the rocket platform driver
> exposes bind and unbind in sysfs, all three cores are bound here, and
> unbinding one while the other two stay bound does reach patched code on
> RK3588, through rocket_core_fini() and rocket_job_fini() on the way out,
> and soc->num_clks and soc->num_resets on the way back in. That is not
> covered by an insmod/rmmod of the whole module. It is a gap in my testing,
> not a limitation of the hardware, and I will run it before v6 if you want
> it.
>
> The module unload under load I also promised is not runnable in that form:
> rmmod is refused by the module refcount while a DRM fd is open, and my
> harness declines the swap rather than fight it. The runnable equivalent is
> an unbind under load, which I also did not run. All 13 loads above happened
> between benches with nothing attached.
>
> So the Tested-by covers what I actually measured: that this patch does not
> regress the RK3588 path, in timing, in interrupt accounting or in output.
> The RK3576 specific paths are untested by me.
>
> Two caveats on the numbers.
>
> First, three runs per side rather than four. The machine went down partway
> through the fourth pair, roughly 25 seconds into the test side, with your
> module loaded and inference in flight. I am describing it that way because
> that is what the logs support: the journal for that boot ends mid-run with
> no shutdown sequence at all, the bench's output file for that run is zero
> bytes, my script's exit trap never ran, and the following boot logs an
> orphan cleanup and a journal marked uncleanly shut down. Whether the
> desktop or the whole machine went first is not recorded. There is no oops,
> no kernel WARNING, no call trace and no rocket error anywhere in that boot,
> and nothing that ties it to this patch, but it did happen with this patch
> loaded and I have no explanation for it. I power cycled and did not
> resume the series rather than mix a rebooted machine into it. I would
> rather report it than not.
>
> Second, the resolution bound of about one percent is a property of my
> bench, not of the patch.
>
> If there is an ordering or a workload you would rather I ran, say so and I
> will run it.
>
> Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, Orange Pi 5 Plus
>
> Igor
>
> On Wed, Aug 5, 2026 at 2:52 PM Diederik de Haas <diederik@cknow-tech.com> wrote:
> >
> > On Wed Aug 5, 2026 at 8:38 AM CEST, Jiaxing Hu wrote:
> > > The RK3576 carries the same RKNN block as the RK3588, with two cores
> > > instead of three and a few platform differences:
> > >
> > > - the CBUF (convolution buffer) has its own clock domain, so the core
> > > needs six clocks rather than four;
> > > - the BIU reset moved into the power domain, leaving one reset here;
> > > - the NPU spans two power domains, and a device with more than one is
> > > skipped by the driver-core single-domain auto-attach, so the list has
> > > to be attached explicitly;
> > > - the DPU completion interrupt is armed exactly as on RK3588 but never
> > > reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
> > > so sample that from an hrtimer rather than wait for an interrupt that
> > > does not come. The interrupt stays armed, so if it ever does arrive
> > > the normal handler finalises the job first.
> > >
> > > Select all of that from of_device_id match data so the RK3588 path keeps
> > > its existing counts and behaviour unchanged.
> >
> > Would it make sense to split this patch up in 2 patches:
> > 1) Prepare for different NPU/RKNN implementations
> > 2) Add support for RK3576 NPU/RKNN
> >
> > It seems like you're changing the implementation for RK3588 in this patch
> > AND add support for RK3576. It's often better to split those things out.
> >
> > Cheers,
> > Diederik
> >
> > > Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> > > ---
> > > drivers/accel/rocket/rocket_core.c | 26 +++++-
> > > drivers/accel/rocket/rocket_core.h | 20 ++++-
> > > drivers/accel/rocket/rocket_device.c | 4 +
> > > drivers/accel/rocket/rocket_drv.c | 22 ++++-
> > > drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
> > > 5 files changed, 176 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> > > index b3b2fa9ba..e08288c8c 100644
> > > --- a/drivers/accel/rocket/rocket_core.c
> > > +++ b/drivers/accel/rocket/rocket_core.c
> > > @@ -8,6 +8,7 @@
> > > #include <linux/err.h>
> > > #include <linux/iommu.h>
> > > #include <linux/platform_device.h>
> > > +#include <linux/pm_domain.h>
> > > #include <linux/pm_runtime.h>
> > > #include <linux/reset.h>
> > >
> > > @@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
> > > u32 version;
> > > int err = 0;
> > >
> > > + /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
> > > core->resets[0].id = "srst_a";
> > > core->resets[1].id = "srst_h";
> > > - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> > > + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> > > core->resets);
> > > if (err)
> > > return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
> > >
> > > - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> > > + core->clks[0].id = "aclk";
> > > + core->clks[1].id = "hclk";
> > > + core->clks[2].id = "npu";
> > > + core->clks[3].id = "pclk";
> > > + /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
> > > + core->clks[4].id = "aclk_cbuf";
> > > + core->clks[5].id = "hclk_cbuf";
> > > + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> > > if (err)
> > > return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
> > >
> > > @@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
> > > return err;
> > > }
> > >
> > > + /*
> > > + * RK3576 spans two power domains, and a multi-domain device is skipped
> > > + * by the driver-core single-domain auto-attach, so attach the list here.
> > > + */
> > > + if (core->soc->multi_power_domain) {
> > > + struct dev_pm_domain_list *pd_list;
> > > +
> > > + err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> > > + if (err < 0)
> > > + return dev_err_probe(dev, err,
> > > + "failed to attach NPU power domains\n");
> > > + }
> > > +
> > > pm_runtime_use_autosuspend(dev);
> > >
> > > /*
> > > diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> > > index f6d738285..205ff070d 100644
> > > --- a/drivers/accel/rocket/rocket_core.h
> > > +++ b/drivers/accel/rocket/rocket_core.h
> > > @@ -6,6 +6,7 @@
> > >
> > > #include <drm/gpu_scheduler.h>
> > > #include <linux/clk.h>
> > > +#include <linux/hrtimer.h>
> > > #include <linux/io.h>
> > > #include <linux/mutex_types.h>
> > > #include <linux/reset.h>
> > > @@ -27,16 +28,25 @@
> > > #define rocket_core_writel(core, reg, value) \
> > > writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
> > >
> > > +/* Per-SoC differences, selected by the of_device_id match data. */
> > > +struct rocket_soc_data {
> > > + unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
> > > + unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
> > > + bool multi_power_domain; /* device spans more than one PM domain */
> > > + bool poll_completion; /* completion IRQ never reaches the GIC */
> > > +};
> > > +
> > > struct rocket_core {
> > > struct device *dev;
> > > struct rocket_device *rdev;
> > > + const struct rocket_soc_data *soc;
> > > unsigned int index;
> > >
> > > int irq;
> > > void __iomem *pc_iomem;
> > > void __iomem *cna_iomem;
> > > void __iomem *core_iomem;
> > > - struct clk_bulk_data clks[4];
> > > + struct clk_bulk_data clks[6];
> > > struct reset_control_bulk_data resets[2];
> > >
> > > struct iommu_group *iommu_group;
> > > @@ -52,6 +62,14 @@ struct rocket_core {
> > > atomic_t pending;
> > > } reset;
> > >
> > > + struct hrtimer poll_timer;
> > > + struct work_struct poll_work;
> > > + atomic_t poll_active;
> > > + unsigned int poll_ticks;
> > > + unsigned int poll_seq;
> > > + unsigned int poll_work_seq;
> > > + bool poll_dying;
> > > +
> > > struct drm_gpu_scheduler sched;
> > > u64 fence_context;
> > > u64 emit_seqno;
> > > diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> > > index 46e6ee1e7..bfb00f967 100644
> > > --- a/drivers/accel/rocket/rocket_device.c
> > > +++ b/drivers/accel/rocket/rocket_device.c
> > > @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
> > > if (of_device_is_available(core_node))
> > > num_cores++;
> > >
> > > + for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
> > > + if (of_device_is_available(core_node))
> > > + num_cores++;
> > > +
> > > rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
> > > if (!rdev->cores)
> > > return ERR_PTR(-ENOMEM);
> > > diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> > > index 8bbbce594..7f7dfa374 100644
> > > --- a/drivers/accel/rocket/rocket_drv.c
> > > +++ b/drivers/accel/rocket/rocket_drv.c
> > > @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
> > >
> > > rdev->cores[core].rdev = rdev;
> > > rdev->cores[core].dev = &pdev->dev;
> > > + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> > > rdev->cores[core].index = core;
> > >
> > > rdev->num_cores++;
> > > @@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
> > > }
> > > }
> > >
> > > +static const struct rocket_soc_data rk3588_soc_data = {
> > > + .num_clks = 4,
> > > + .num_resets = 2,
> > > + .multi_power_domain = false,
> > > + .poll_completion = false,
> > > +};
> > > +
> > > +static const struct rocket_soc_data rk3576_soc_data = {
> > > + .num_clks = 6,
> > > + .num_resets = 1,
> > > + .multi_power_domain = true,
> > > + .poll_completion = true,
> > > +};
> > > +
> > > static const struct of_device_id dt_match[] = {
> > > - { .compatible = "rockchip,rk3588-rknn-core" },
> > > + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
> > > + { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
> > > {}
> > > };
> > > MODULE_DEVICE_TABLE(of, dt_match);
> > > @@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> > > if (core < 0)
> > > return -ENODEV;
> > >
> > > - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > > + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> > > if (err) {
> > > dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> > > return err;
> > > @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> > > if (!rocket_job_is_idle(&rdev->cores[core]))
> > > return -EBUSY;
> > >
> > > - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > > + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> > >
> > > return 0;
> > > }
> > > diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> > > index bb77b6bf0..28845ac4e 100644
> > > --- a/drivers/accel/rocket/rocket_job.c
> > > +++ b/drivers/accel/rocket/rocket_job.c
> > > @@ -7,6 +7,7 @@
> > > #include <drm/drm_file.h>
> > > #include <drm/drm_gem.h>
> > > #include <drm/rocket_accel.h>
> > > +#include <linux/hrtimer.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/overflow.h>
> > > #include <linux/iommu.h>
> > > @@ -21,6 +22,15 @@
> > >
> > > #define JOB_TIMEOUT_MS 500
> > >
> > > +/*
> > > + * RK3576 arms the same DPU completion as RK3588, but the interrupt never
> > > + * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
> > > + * so sample that instead. The tick cap bounds jobs that never raise it at all,
> > > + * which is the same open problem as the wrong inference results.
> > > + */
> > > +#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
> > > +#define RK3576_POLL_MAX_TICKS 8
> > > +
> > > static struct rocket_job *
> > > to_rocket_job(struct drm_sched_job *sched_job)
> > > {
> > > @@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
> > >
> > > rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
> > >
> > > + if (core->soc->poll_completion) {
> > > + core->poll_ticks = 0;
> > > + core->poll_seq++;
> > > + atomic_set(&core->poll_active, 1);
> > > + hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> > > + HRTIMER_MODE_REL);
> > > + }
> > > +
> > > dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> > > }
> > >
> > > @@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> > > return ERR_PTR(ret);
> > > }
> > >
> > > +static void rocket_job_handle_irq(struct rocket_core *core);
> > > +
> > > +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> > > +{
> > > + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> > > + u32 raw;
> > > +
> > > + if (!atomic_read(&core->poll_active))
> > > + return HRTIMER_NORESTART;
> > > +
> > > + core->poll_work_seq = core->poll_seq;
> > > +
> > > + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> > > + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> > > + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> > > + atomic_set(&core->poll_active, 0);
> > > + schedule_work(&core->poll_work);
> > > + return HRTIMER_NORESTART;
> > > + }
> > > +
> > > + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> > > + return HRTIMER_RESTART;
> > > +}
> > > +
> > > +/* Start the job's next task, or retire it. Caller holds job_lock. */
> > > +static void rocket_job_next_locked(struct rocket_core *core)
> > > +{
> > > + lockdep_assert_held(&core->job_lock);
> > > +
> > > + if (!core->in_flight_job)
> > > + return;
> > > +
> > > + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > > + rocket_job_hw_submit(core, core->in_flight_job);
> > > + return;
> > > + }
> > > +
> > > + iommu_detach_group(NULL, iommu_group_get(core->dev));
> > > + dma_fence_signal(core->in_flight_job->done_fence);
> > > + pm_runtime_put_autosuspend(core->dev);
> > > + core->in_flight_job = NULL;
> > > +}
> > > +
> > > +static void rocket_poll_work_fn(struct work_struct *work)
> > > +{
> > > + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> > > +
> > > + pm_runtime_mark_last_busy(core->dev);
> > > +
> > > + scoped_guard(mutex, &core->job_lock) {
> > > + /*
> > > + * The interrupt can land while this work is queued, retire the job
> > > + * and start the next task. poll_seq only moves under job_lock, in
> > > + * hw_submit, so comparing it here says whether that happened. Doing
> > > + * it outside the lock would leave the window open rather than close
> > > + * it, and this work would then submit a task on top of a live one.
> > > + */
> > > + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> > > + return;
> > > +
> > > + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > > + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> > > +
> > > + rocket_job_next_locked(core);
> > > + }
> > > +}
> > > +
> > > static void rocket_job_handle_irq(struct rocket_core *core)
> > > {
> > > + if (core->soc->poll_completion) {
> > > + atomic_set(&core->poll_active, 0);
> > > + hrtimer_cancel(&core->poll_timer);
> > > + }
> > > +
> > > pm_runtime_mark_last_busy(core->dev);
> > >
> > > rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > > rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> > >
> > > scoped_guard(mutex, &core->job_lock)
> > > - if (core->in_flight_job) {
> > > - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > > - rocket_job_hw_submit(core, core->in_flight_job);
> > > - return;
> > > - }
> > > -
> > > - iommu_detach_group(NULL, iommu_group_get(core->dev));
> > > - dma_fence_signal(core->in_flight_job->done_fence);
> > > - pm_runtime_put_autosuspend(core->dev);
> > > - core->in_flight_job = NULL;
> > > - }
> > > + rocket_job_next_locked(core);
> > > }
> > >
> > > static void
> > > @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> > > int ret;
> > >
> > > INIT_WORK(&core->reset.work, rocket_reset_work);
> > > + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> > > + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> > > + HRTIMER_MODE_REL);
> > > + atomic_set(&core->poll_active, 0);
> > > spin_lock_init(&core->fence_lock);
> > > mutex_init(&core->job_lock);
> > >
> > > @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
> > >
> > > void rocket_job_fini(struct rocket_core *core)
> > > {
> > > + /*
> > > + * Stop the poll from starting hardware work before tearing anything
> > > + * down: it submits the next task, and drm_sched_fini() does not wait
> > > + * for work already queued. Cancel after the scheduler is gone, so a
> > > + * job running now cannot re-arm the timer behind the cancel.
> > > + */
> > > + if (core->soc->poll_completion)
> > > + WRITE_ONCE(core->poll_dying, true);
> > > +
> > > drm_sched_fini(&core->sched);
> > >
> > > + if (core->soc->poll_completion) {
> > > + atomic_set(&core->poll_active, 0);
> > > + hrtimer_cancel(&core->poll_timer);
> > > + cancel_work_sync(&core->poll_work);
> > > + }
> > > +
> > > cancel_work_sync(&core->reset.work);
> > > destroy_workqueue(core->reset.wq);
> > > }
> >
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support
2026-08-05 14:07 ` Igor Paunovic
@ 2026-08-05 14:35 ` Igor Paunovic
0 siblings, 0 replies; 21+ messages in thread
From: Igor Paunovic @ 2026-08-05 14:35 UTC (permalink / raw)
To: Diederik de Haas
Cc: Jiaxing Hu, tomeu, heiko, robh, krzk+dt, conor+dt, joro, will,
robin.murphy, ulfh, p.zabel, ogabbay, zhangqing, alchark,
chaoyi.chen, dri-devel, linux-rockchip, iommu, linux-pm,
devicetree, linux-arm-kernel, linux-kernel
Hi Jiaxing,
Following up on my own caveat: I said the per-core unbind/rebind was
runnable here and that I had not run it. I have now. It passes, and it
also measures the thing I had only argued from the code.
Same test module as before, srcversion 57A4D5369E495870E651EEC, which is
v7.2-rc6 rocket plus my four in-flight fixes plus your 6/8 hand resolved.
All three cores bound to start with. Between every step, one inference
through the Teflon delegate on a freshly opened fd.
step inf/s interrupts per inference oracle
all three bound 88.3 irq70 41.74 irq71 1.02 irq74 0
core 2 unbound 88.1 irq70 41.74 irq71 1.02 pass
core 2 rebound 88.8 irq70 41.74 irq71 1.02 irq74 0 pass
core 0 unbound 74.7 irq71 41.88 irq74 1.02 pass
core 0 rebound 89.2 irq70 41.74 irq71 1.02 irq74 0 pass
all three, cycled 90.0 irq70 41.73 irq71 1.02 irq74 0 pass
The five reference tensor hashes are byte identical at every one of those
six points, /dev/accel/accel0 keeps its minor throughout, and there is no
rocket error, no WARNING, no oops and no call trace anywhere in the boot.
The interesting row is core 0 unbound with cores 1 and 2 still bound. The
work retargets correctly: the 41 completions move to core 1 and the single
one to core 2, and the result is still bit exact. That is the out-of-order
unbind case, so it exercises the slot bookkeeping as well as your fini and
probe changes.
Now the part that is worth more than the pass. It confirms, on hardware,
the mechanism I described when I raised poll_dying, and it does so from the
timestamps rather than from reading the code. "[drm] Initialized rocket
0.0.0 for rknn on minor 0" appears only when the shared device is actually
built:
16:23:20 module loaded, all three cores probe -> Initialized
16:23:33 core 2 rebound, cores 0 and 1 stayed -> no Initialized
16:23:46 core 0 rebound, cores 1 and 2 stayed -> no Initialized
16:23:54 all three unbound, then all three rebound -> Initialized
So a partial unbind leaves rdev and the devm_kcalloc'd core array alive and
hands the rebinding core the same struct rocket_core back, while unbinding
the last core runs rocket_device_fini(), nulls rdev, and makes the next
probe allocate a fresh zeroed array. That is exactly the difference between
a latch surviving and a latch being wiped, and it is why your ROCK 4D
result is clean and correct: 8/8 enables one core, so every unbind there is
the last one and always takes the reallocating path.
On RK3576 with both cores enabled, the sequence in the third row above is
the one that would leave poll_dying set on the rebinding core.
What this still does not cover: poll_dying and rocket_poll_work_fn remain
unreachable on RK3588, for the reason in my previous mail, so this says
nothing about the latch itself or about the fini race, only about the
struct lifetime that makes them reachable. And these unbinds were done
between inferences, not during one, so "unbind under load" is still not
tested.
One observation that is about the driver rather than your patch, in case it
matters for RK3576 where you have two cores rather than three. Unbinding
core 2 costs nothing, 88.1 against 88.3, because this workload leaves it
idle. Unbinding core 0 costs about 15%, 74.7 against 88.3, even though the
work does move over correctly. Whatever the reason, on a two core part the
core that goes away is more likely to be one that was carrying work.
Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, Orange Pi 5 Plus
Igor
On Wed, Aug 5, 2026 at 4:07 PM Igor Paunovic <royalnet026@gmail.com> wrote:
>
> Hi Diederik,
>
> Seconding this, with a data point that may help make the case.
>
> I have my own fix in flight that touches the same function, "accel/rocket:
> request the core clocks by name":
>
> https://lore.kernel.org/linux-rockchip/20260729130743.128876-1-royalnet026@gmail.com/
>
> It carries Jiaxing's Reviewed-by and a Tested-by from Sidong Yang and is
> waiting on Tomeu. Earlier today I built 6/8 on top of a tree that already
> has it, to characterise the RK3588 side on hardware. git apply refused the
> patch outright, and with fuzz everything applied except one hunk, the
> resets-and-clocks hunk in rocket_core_init(), which I had to resolve by
> hand. So the two RK3588-affecting changes in this patch are already
> colliding in practice, not just in principle.
>
> That hunk is doing three separate things at once: it re-adds the four
> clks[].id assignments that my standalone fix adds, it grows the array and
> switches both bulk counts to soc->num_clks and soc->num_resets, and it adds
> the two RK3576 CBUF entries. Only the last of those is RK3576 enablement.
> The first is a bugfix that exists independently, and the middle one is
> exactly the "prepare for different implementations" step you are asking
> for.
>
> Split the way you suggest, the ordering problem goes away on its own. The
> preparation patch would carry the soc_data plumbing and the count changes,
> rebased on whatever landed first, and the RK3576 patch would carry the CBUF
> entries and the poll path. Nothing would need to be sequenced by hand, and
> the commit message could stop claiming the RK3588 path is behaviourally
> unchanged, which is the part I raised separately on this patch and which is
> only true once the bugfix is not being carried along with the enablement.
>
> Igor
>
> On Wed, Aug 5, 2026 at 4:06 PM Igor Paunovic <royalnet026@gmail.com> wrote:
> >
> > Hi Jiaxing,
> >
> > Here is the RK3588 characterisation I promised on the cover letter. Short
> > version: I cannot measure any effect from this patch on RK3588. The output
> > is bit-identical and the interrupt accounting is unchanged to within
> > 0.0001 interrupts per inference.
> >
> > What was compared
> > -----------------
> >
> > Both sides were built from the same tree, from the same headers, with only
> > your 6/8 between them:
> >
> > base: v7.2-rc6 rocket + the four fixes I have in flight
> > (Guangshuo Li's "clear rdev on device init failure", my two
> > lifecycle patches, and "request the core clocks by name")
> > test: the same tree + this patch, hand resolved, see below
> >
> > running kernel 7.2.0-rc6-rk3588-igor-claude+, tree pinned at Collabora
> > rockchip-devel 8c70065a3207, which sits on 075b74841bd0 = Linux 7.2-rc6.
> > Not a v7.2 tag; that does not exist yet.
> >
> > base module srcversion 97DECECF73741ABE56685D2
> > test module srcversion 57A4D5369E495870E651EEC
> >
> > I deliberately left my local devfreq work out of both sides. It touches
> > rocket_job.c, you do not have it, and it would have changed the clock
> > during the runs. The consequence is that the NPU ran at the firmware
> > default rate rather than the rate I normally use, so the absolute numbers
> > below are around 89 inf/s where I would usually quote about 222. That does
> > not matter for a difference measurement, but it means these figures are not
> > comparable to any throughput number I have posted before.
> >
> > One thing worth reporting before the numbers: 6/8 does not apply cleanly on
> > that base, for two separate reasons, and only one of them concerns you and
> > me.
> >
> > The first is the coordination point from my earlier mail, now measured
> > rather than predicted. The clks[].id hunk in rocket_core.c conflicts with
> > my standalone "request the core clocks by name" fix. I resolved it by hand,
> > keeping your two CBUF entries and the soc->num_clks and soc->num_resets
> > changes on top of the four named assignments that are already there.
> >
> > The second is not a conflict with anything I carry. The rocket_job.c hunk
> > has "#include <linux/overflow.h>" in its context, and that line is neither
> > in v7.2-rc6 rocket_job.c nor added by any patch in this series, so your
> > tree is simply newer than v7.2-rc6 there. Mentioning it so that a clean
> > apply on your side does not get read as a contradiction of the first point.
> >
> > That means this Tested-by is on the hand resolved form of 6/8, not on the
> > posted hunks applied verbatim.
> >
> > Method
> > ------
> >
> > Interleaved, with the order alternated per pair, because my first attempt
> > was not interleaved at all. That attempt measured base and test in separate
> > blocks and showed a 1.6% difference on the NPU part, once the first run of
> > each block is dropped. It is not your patch. The two blocks ran under
> > measurably different conditions: NPU thermals 43.5-49.0 C and one minute
> > load average 2.9-4.4 during the base block, against 39.8-44.4 C and 2.0-3.1
> > during the test block. And one test run behaved like a base run: same
> > module, CPU part 1.87 ms, where the other three test runs sat at 1.67-1.68
> > ms. I believe that was my desktop client compositing while I typed in it,
> > but I did not isolate it as a controlled variable, so what I can actually
> > say is that the block to block gap was environmental, and that the
> > interleaved rerun below shows nothing.
> >
> > So: three pairs, order base/test, then test/base, then base/test, module
> > swapped between every single run, no interactive use of the machine, only
> > the usual system timers. One honesty note on the design: the alternation
> > only balances fully over an even number of pairs, and I ended up with
> > three, so base occupies series positions 1, 4 and 5 against test's 2, 3 and
> > 6. The series does drift slowly downwards, NPU thermals falling from 39.8 C
> > to 37.9 C over the six runs, and that drift is therefore not entirely
> > cancelled. It is well below the effect size discussed below, but it is
> > there.
> >
> > Environment held constant: CPU governor performance at 1800/2400/2400 MHz,
> > vdd_npu_s0 at 850 mV, NPU thermals 37.9-39.8 C. Those thermals are
> > snapshots taken at the start and end of each run, not a continuous trace.
> >
> > MobileNetV1 through the Teflon TFLite delegate, effectively one core, 20 s
> > x 3 blocks per run, per-run block spread 0.29-1.09%.
> >
> > Results, 3 runs per side
> > ------------------------
> >
> > base test delta
> > throughput 89.0093 +-0.3568 88.9546 +-0.2270 -0.06% inf/s
> > NPU part 9.7521 +-0.0434 9.7570 +-0.0364 +0.05% ms/inference
> > CPU part 1.4828 +-0.0022 1.4847 +-0.0106 +0.13% ms/inference
> >
> > The largest standard deviation on the NPU part is 0.0434 ms, 0.45% of the
> > mean. The measured difference is 0.0049 ms, 0.05%, about an eighth of that
> > standard deviation. This does not say the patch costs nothing; it says any
> > cost is below what this bench resolves. With three runs per side the 95%
> > confidence bound on the difference is about +-0.9% of the mean, so one
> > percent is the honest resolution figure, not half a percent.
> >
> > Interrupt accounting per inference:
> >
> > base test delta
> > irq 99 41.11512 +-0.00046 41.11519 +-0.00029 +0.000071
> > irq 102 1.00281 +-0.00001 1.00281 +-0.00001 +0.000002
> > irq 103 0.00000 +-0.00000 0.00000 +-0.00000 0
> >
> > These are integer counters. The residual scatter is a window alignment
> > artefact, because the counter window does not line up exactly with the
> > counted iterations, which is also why the figure is 41.115 rather than 41.
> > It is three orders of magnitude below one interrupt per inference, so a
> > dropped or a doubled task boundary would be unmissable. This is the number
> > I care about most: the restructured completion path neither double counts
> > nor drops a task boundary. For this model 41 completions per inference land
> > on core 0 and one on core 1; irq 103 stays at zero because the third core
> > is idle in this workload.
> >
> > The bit exact oracle passed on every run, and the reference hashes over the
> > five tracked tensors are identical across all six runs, 32044 inferences
> > in total. Top-1 unchanged at "military uniform", 0.867.
> >
> > The module was loaded 13 times over the session, reporting "[drm]
> > Initialized rocket 0.0.0 for rknn on minor 0" with minor 0 every time, and
> > there is no rocket error or warning anywhere in that boot. Twelve of those
> > loads were unloaded normally. The thirteenth was still resident when the
> > machine stopped, which is the last caveat below.
> >
> > What I did not test, and why
> > ----------------------------
> >
> > Three things, and they are not the same kind of "not".
> >
> > The poll_dying latch and rocket_poll_work_fn() cannot be exercised on this
> > SoC at all. The flag is written only under
> >
> > if (core->soc->poll_completion)
> > WRITE_ONCE(core->poll_dying, true);
> >
> > poll_completion is false in rk3588_soc_data, and the poll timer is started
> > under the same guard in rocket_job_hw_submit(), so neither the timer
> > function nor the work function can run here. The fini path race I raised on
> > this patch needs RK3576 for the same reason.
> >
> > The per-core unbind/rebind I promised on the cover was runnable here and I
> > did not run it. That distinction matters, so: the rocket platform driver
> > exposes bind and unbind in sysfs, all three cores are bound here, and
> > unbinding one while the other two stay bound does reach patched code on
> > RK3588, through rocket_core_fini() and rocket_job_fini() on the way out,
> > and soc->num_clks and soc->num_resets on the way back in. That is not
> > covered by an insmod/rmmod of the whole module. It is a gap in my testing,
> > not a limitation of the hardware, and I will run it before v6 if you want
> > it.
> >
> > The module unload under load I also promised is not runnable in that form:
> > rmmod is refused by the module refcount while a DRM fd is open, and my
> > harness declines the swap rather than fight it. The runnable equivalent is
> > an unbind under load, which I also did not run. All 13 loads above happened
> > between benches with nothing attached.
> >
> > So the Tested-by covers what I actually measured: that this patch does not
> > regress the RK3588 path, in timing, in interrupt accounting or in output.
> > The RK3576 specific paths are untested by me.
> >
> > Two caveats on the numbers.
> >
> > First, three runs per side rather than four. The machine went down partway
> > through the fourth pair, roughly 25 seconds into the test side, with your
> > module loaded and inference in flight. I am describing it that way because
> > that is what the logs support: the journal for that boot ends mid-run with
> > no shutdown sequence at all, the bench's output file for that run is zero
> > bytes, my script's exit trap never ran, and the following boot logs an
> > orphan cleanup and a journal marked uncleanly shut down. Whether the
> > desktop or the whole machine went first is not recorded. There is no oops,
> > no kernel WARNING, no call trace and no rocket error anywhere in that boot,
> > and nothing that ties it to this patch, but it did happen with this patch
> > loaded and I have no explanation for it. I power cycled and did not
> > resume the series rather than mix a rebooted machine into it. I would
> > rather report it than not.
> >
> > Second, the resolution bound of about one percent is a property of my
> > bench, not of the patch.
> >
> > If there is an ordering or a workload you would rather I ran, say so and I
> > will run it.
> >
> > Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, Orange Pi 5 Plus
> >
> > Igor
> >
> > On Wed, Aug 5, 2026 at 2:52 PM Diederik de Haas <diederik@cknow-tech.com> wrote:
> > >
> > > On Wed Aug 5, 2026 at 8:38 AM CEST, Jiaxing Hu wrote:
> > > > The RK3576 carries the same RKNN block as the RK3588, with two cores
> > > > instead of three and a few platform differences:
> > > >
> > > > - the CBUF (convolution buffer) has its own clock domain, so the core
> > > > needs six clocks rather than four;
> > > > - the BIU reset moved into the power domain, leaving one reset here;
> > > > - the NPU spans two power domains, and a device with more than one is
> > > > skipped by the driver-core single-domain auto-attach, so the list has
> > > > to be attached explicitly;
> > > > - the DPU completion interrupt is armed exactly as on RK3588 but never
> > > > reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
> > > > so sample that from an hrtimer rather than wait for an interrupt that
> > > > does not come. The interrupt stays armed, so if it ever does arrive
> > > > the normal handler finalises the job first.
> > > >
> > > > Select all of that from of_device_id match data so the RK3588 path keeps
> > > > its existing counts and behaviour unchanged.
> > >
> > > Would it make sense to split this patch up in 2 patches:
> > > 1) Prepare for different NPU/RKNN implementations
> > > 2) Add support for RK3576 NPU/RKNN
> > >
> > > It seems like you're changing the implementation for RK3588 in this patch
> > > AND add support for RK3576. It's often better to split those things out.
> > >
> > > Cheers,
> > > Diederik
> > >
> > > > Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> > > > ---
> > > > drivers/accel/rocket/rocket_core.c | 26 +++++-
> > > > drivers/accel/rocket/rocket_core.h | 20 ++++-
> > > > drivers/accel/rocket/rocket_device.c | 4 +
> > > > drivers/accel/rocket/rocket_drv.c | 22 ++++-
> > > > drivers/accel/rocket/rocket_job.c | 121 ++++++++++++++++++++++++---
> > > > 5 files changed, 176 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> > > > index b3b2fa9ba..e08288c8c 100644
> > > > --- a/drivers/accel/rocket/rocket_core.c
> > > > +++ b/drivers/accel/rocket/rocket_core.c
> > > > @@ -8,6 +8,7 @@
> > > > #include <linux/err.h>
> > > > #include <linux/iommu.h>
> > > > #include <linux/platform_device.h>
> > > > +#include <linux/pm_domain.h>
> > > > #include <linux/pm_runtime.h>
> > > > #include <linux/reset.h>
> > > >
> > > > @@ -21,14 +22,22 @@ int rocket_core_init(struct rocket_core *core)
> > > > u32 version;
> > > > int err = 0;
> > > >
> > > > + /* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
> > > > core->resets[0].id = "srst_a";
> > > > core->resets[1].id = "srst_h";
> > > > - err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
> > > > + err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
> > > > core->resets);
> > > > if (err)
> > > > return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
> > > >
> > > > - err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
> > > > + core->clks[0].id = "aclk";
> > > > + core->clks[1].id = "hclk";
> > > > + core->clks[2].id = "npu";
> > > > + core->clks[3].id = "pclk";
> > > > + /* RK3576 clocks the CBUF separately; the compute path stalls without these. */
> > > > + core->clks[4].id = "aclk_cbuf";
> > > > + core->clks[5].id = "hclk_cbuf";
> > > > + err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
> > > > if (err)
> > > > return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
> > > >
> > > > @@ -65,6 +74,19 @@ int rocket_core_init(struct rocket_core *core)
> > > > return err;
> > > > }
> > > >
> > > > + /*
> > > > + * RK3576 spans two power domains, and a multi-domain device is skipped
> > > > + * by the driver-core single-domain auto-attach, so attach the list here.
> > > > + */
> > > > + if (core->soc->multi_power_domain) {
> > > > + struct dev_pm_domain_list *pd_list;
> > > > +
> > > > + err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> > > > + if (err < 0)
> > > > + return dev_err_probe(dev, err,
> > > > + "failed to attach NPU power domains\n");
> > > > + }
> > > > +
> > > > pm_runtime_use_autosuspend(dev);
> > > >
> > > > /*
> > > > diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
> > > > index f6d738285..205ff070d 100644
> > > > --- a/drivers/accel/rocket/rocket_core.h
> > > > +++ b/drivers/accel/rocket/rocket_core.h
> > > > @@ -6,6 +6,7 @@
> > > >
> > > > #include <drm/gpu_scheduler.h>
> > > > #include <linux/clk.h>
> > > > +#include <linux/hrtimer.h>
> > > > #include <linux/io.h>
> > > > #include <linux/mutex_types.h>
> > > > #include <linux/reset.h>
> > > > @@ -27,16 +28,25 @@
> > > > #define rocket_core_writel(core, reg, value) \
> > > > writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
> > > >
> > > > +/* Per-SoC differences, selected by the of_device_id match data. */
> > > > +struct rocket_soc_data {
> > > > + unsigned int num_clks; /* clk_bulk count: 4 base, 6 with CBUF */
> > > > + unsigned int num_resets; /* reset_bulk count: 2 base, 1 on RK3576 */
> > > > + bool multi_power_domain; /* device spans more than one PM domain */
> > > > + bool poll_completion; /* completion IRQ never reaches the GIC */
> > > > +};
> > > > +
> > > > struct rocket_core {
> > > > struct device *dev;
> > > > struct rocket_device *rdev;
> > > > + const struct rocket_soc_data *soc;
> > > > unsigned int index;
> > > >
> > > > int irq;
> > > > void __iomem *pc_iomem;
> > > > void __iomem *cna_iomem;
> > > > void __iomem *core_iomem;
> > > > - struct clk_bulk_data clks[4];
> > > > + struct clk_bulk_data clks[6];
> > > > struct reset_control_bulk_data resets[2];
> > > >
> > > > struct iommu_group *iommu_group;
> > > > @@ -52,6 +62,14 @@ struct rocket_core {
> > > > atomic_t pending;
> > > > } reset;
> > > >
> > > > + struct hrtimer poll_timer;
> > > > + struct work_struct poll_work;
> > > > + atomic_t poll_active;
> > > > + unsigned int poll_ticks;
> > > > + unsigned int poll_seq;
> > > > + unsigned int poll_work_seq;
> > > > + bool poll_dying;
> > > > +
> > > > struct drm_gpu_scheduler sched;
> > > > u64 fence_context;
> > > > u64 emit_seqno;
> > > > diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
> > > > index 46e6ee1e7..bfb00f967 100644
> > > > --- a/drivers/accel/rocket/rocket_device.c
> > > > +++ b/drivers/accel/rocket/rocket_device.c
> > > > @@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
> > > > if (of_device_is_available(core_node))
> > > > num_cores++;
> > > >
> > > > + for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
> > > > + if (of_device_is_available(core_node))
> > > > + num_cores++;
> > > > +
> > > > rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
> > > > if (!rdev->cores)
> > > > return ERR_PTR(-ENOMEM);
> > > > diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> > > > index 8bbbce594..7f7dfa374 100644
> > > > --- a/drivers/accel/rocket/rocket_drv.c
> > > > +++ b/drivers/accel/rocket/rocket_drv.c
> > > > @@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
> > > >
> > > > rdev->cores[core].rdev = rdev;
> > > > rdev->cores[core].dev = &pdev->dev;
> > > > + rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
> > > > rdev->cores[core].index = core;
> > > >
> > > > rdev->num_cores++;
> > > > @@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
> > > > }
> > > > }
> > > >
> > > > +static const struct rocket_soc_data rk3588_soc_data = {
> > > > + .num_clks = 4,
> > > > + .num_resets = 2,
> > > > + .multi_power_domain = false,
> > > > + .poll_completion = false,
> > > > +};
> > > > +
> > > > +static const struct rocket_soc_data rk3576_soc_data = {
> > > > + .num_clks = 6,
> > > > + .num_resets = 1,
> > > > + .multi_power_domain = true,
> > > > + .poll_completion = true,
> > > > +};
> > > > +
> > > > static const struct of_device_id dt_match[] = {
> > > > - { .compatible = "rockchip,rk3588-rknn-core" },
> > > > + { .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
> > > > + { .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
> > > > {}
> > > > };
> > > > MODULE_DEVICE_TABLE(of, dt_match);
> > > > @@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
> > > > if (core < 0)
> > > > return -ENODEV;
> > > >
> > > > - err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > > > + err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> > > > if (err) {
> > > > dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
> > > > return err;
> > > > @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
> > > > if (!rocket_job_is_idle(&rdev->cores[core]))
> > > > return -EBUSY;
> > > >
> > > > - clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> > > > + clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
> > > >
> > > > return 0;
> > > > }
> > > > diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> > > > index bb77b6bf0..28845ac4e 100644
> > > > --- a/drivers/accel/rocket/rocket_job.c
> > > > +++ b/drivers/accel/rocket/rocket_job.c
> > > > @@ -7,6 +7,7 @@
> > > > #include <drm/drm_file.h>
> > > > #include <drm/drm_gem.h>
> > > > #include <drm/rocket_accel.h>
> > > > +#include <linux/hrtimer.h>
> > > > #include <linux/interrupt.h>
> > > > #include <linux/overflow.h>
> > > > #include <linux/iommu.h>
> > > > @@ -21,6 +22,15 @@
> > > >
> > > > #define JOB_TIMEOUT_MS 500
> > > >
> > > > +/*
> > > > + * RK3576 arms the same DPU completion as RK3588, but the interrupt never
> > > > + * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
> > > > + * so sample that instead. The tick cap bounds jobs that never raise it at all,
> > > > + * which is the same open problem as the wrong inference results.
> > > > + */
> > > > +#define RK3576_POLL_INTERVAL_NS 1000000LL /* 1 ms */
> > > > +#define RK3576_POLL_MAX_TICKS 8
> > > > +
> > > > static struct rocket_job *
> > > > to_rocket_job(struct drm_sched_job *sched_job)
> > > > {
> > > > @@ -151,6 +161,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
> > > >
> > > > rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
> > > >
> > > > + if (core->soc->poll_completion) {
> > > > + core->poll_ticks = 0;
> > > > + core->poll_seq++;
> > > > + atomic_set(&core->poll_active, 1);
> > > > + hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> > > > + HRTIMER_MODE_REL);
> > > > + }
> > > > +
> > > > dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> > > > }
> > > >
> > > > @@ -341,25 +359,87 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> > > > return ERR_PTR(ret);
> > > > }
> > > >
> > > > +static void rocket_job_handle_irq(struct rocket_core *core);
> > > > +
> > > > +static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
> > > > +{
> > > > + struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
> > > > + u32 raw;
> > > > +
> > > > + if (!atomic_read(&core->poll_active))
> > > > + return HRTIMER_NORESTART;
> > > > +
> > > > + core->poll_work_seq = core->poll_seq;
> > > > +
> > > > + raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> > > > + if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
> > > > + ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
> > > > + atomic_set(&core->poll_active, 0);
> > > > + schedule_work(&core->poll_work);
> > > > + return HRTIMER_NORESTART;
> > > > + }
> > > > +
> > > > + hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
> > > > + return HRTIMER_RESTART;
> > > > +}
> > > > +
> > > > +/* Start the job's next task, or retire it. Caller holds job_lock. */
> > > > +static void rocket_job_next_locked(struct rocket_core *core)
> > > > +{
> > > > + lockdep_assert_held(&core->job_lock);
> > > > +
> > > > + if (!core->in_flight_job)
> > > > + return;
> > > > +
> > > > + if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > > > + rocket_job_hw_submit(core, core->in_flight_job);
> > > > + return;
> > > > + }
> > > > +
> > > > + iommu_detach_group(NULL, iommu_group_get(core->dev));
> > > > + dma_fence_signal(core->in_flight_job->done_fence);
> > > > + pm_runtime_put_autosuspend(core->dev);
> > > > + core->in_flight_job = NULL;
> > > > +}
> > > > +
> > > > +static void rocket_poll_work_fn(struct work_struct *work)
> > > > +{
> > > > + struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> > > > +
> > > > + pm_runtime_mark_last_busy(core->dev);
> > > > +
> > > > + scoped_guard(mutex, &core->job_lock) {
> > > > + /*
> > > > + * The interrupt can land while this work is queued, retire the job
> > > > + * and start the next task. poll_seq only moves under job_lock, in
> > > > + * hw_submit, so comparing it here says whether that happened. Doing
> > > > + * it outside the lock would leave the window open rather than close
> > > > + * it, and this work would then submit a task on top of a live one.
> > > > + */
> > > > + if (READ_ONCE(core->poll_dying) || core->poll_work_seq != core->poll_seq)
> > > > + return;
> > > > +
> > > > + rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > > > + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> > > > +
> > > > + rocket_job_next_locked(core);
> > > > + }
> > > > +}
> > > > +
> > > > static void rocket_job_handle_irq(struct rocket_core *core)
> > > > {
> > > > + if (core->soc->poll_completion) {
> > > > + atomic_set(&core->poll_active, 0);
> > > > + hrtimer_cancel(&core->poll_timer);
> > > > + }
> > > > +
> > > > pm_runtime_mark_last_busy(core->dev);
> > > >
> > > > rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> > > > rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> > > >
> > > > scoped_guard(mutex, &core->job_lock)
> > > > - if (core->in_flight_job) {
> > > > - if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
> > > > - rocket_job_hw_submit(core, core->in_flight_job);
> > > > - return;
> > > > - }
> > > > -
> > > > - iommu_detach_group(NULL, iommu_group_get(core->dev));
> > > > - dma_fence_signal(core->in_flight_job->done_fence);
> > > > - pm_runtime_put_autosuspend(core->dev);
> > > > - core->in_flight_job = NULL;
> > > > - }
> > > > + rocket_job_next_locked(core);
> > > > }
> > > >
> > > > static void
> > > > @@ -460,6 +540,10 @@ int rocket_job_init(struct rocket_core *core)
> > > > int ret;
> > > >
> > > > INIT_WORK(&core->reset.work, rocket_reset_work);
> > > > + INIT_WORK(&core->poll_work, rocket_poll_work_fn);
> > > > + hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
> > > > + HRTIMER_MODE_REL);
> > > > + atomic_set(&core->poll_active, 0);
> > > > spin_lock_init(&core->fence_lock);
> > > > mutex_init(&core->job_lock);
> > > >
> > > > @@ -501,8 +585,23 @@ int rocket_job_init(struct rocket_core *core)
> > > >
> > > > void rocket_job_fini(struct rocket_core *core)
> > > > {
> > > > + /*
> > > > + * Stop the poll from starting hardware work before tearing anything
> > > > + * down: it submits the next task, and drm_sched_fini() does not wait
> > > > + * for work already queued. Cancel after the scheduler is gone, so a
> > > > + * job running now cannot re-arm the timer behind the cancel.
> > > > + */
> > > > + if (core->soc->poll_completion)
> > > > + WRITE_ONCE(core->poll_dying, true);
> > > > +
> > > > drm_sched_fini(&core->sched);
> > > >
> > > > + if (core->soc->poll_completion) {
> > > > + atomic_set(&core->poll_active, 0);
> > > > + hrtimer_cancel(&core->poll_timer);
> > > > + cancel_work_sync(&core->poll_work);
> > > > + }
> > > > +
> > > > cancel_work_sync(&core->reset.work);
> > > > destroy_workqueue(core->reset.wq);
> > > > }
> > >
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (5 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 6/8] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 7:46 ` sashiko-bot
2026-08-05 6:38 ` [RFC PATCH v5 v5 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
2026-08-05 9:06 ` [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Igor Paunovic
8 siblings, 1 reply; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
Add the two RKNN cores and their IOMMUs, plus the NPU power-domain
resets the pmdomain driver now cycles on power-on. Both cores are
disabled by default; boards enable what they wire up.
Each core lists both NPU power domains, its own first. The compute path
needs NPU1 powered even when only core 0 runs, and a node with a single
domain would be auto-attached by the driver core before the driver can
attach the list itself. The IOMMUs keep one domain each, since they rely
on that same auto-attach.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
arch/arm64/boot/dts/rockchip/rk3576.dtsi | 80 +++++++++++++++++++++++-
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index b0c0d3c8b..1e6dd039f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1070,14 +1070,22 @@ power-domain@RK3576_PD_NPUTOP {
power-domain@RK3576_PD_NPU0 {
reg = <RK3576_PD_NPU0>;
clocks = <&cru HCLK_RKNN_ROOT>,
- <&cru ACLK_RKNN0>;
+ <&cru ACLK_RKNN0>,
+ <&cru CLK_RKNN_DSU0>,
+ <&cru ACLK_RKNN_CBUF>,
+ <&cru HCLK_RKNN_CBUF>;
+ resets = <&cru SRST_A_RKNN0_BIU>;
pm_qos = <&qos_npu_m0>;
#power-domain-cells = <0>;
};
power-domain@RK3576_PD_NPU1 {
reg = <RK3576_PD_NPU1>;
clocks = <&cru HCLK_RKNN_ROOT>,
- <&cru ACLK_RKNN1>;
+ <&cru ACLK_RKNN1>,
+ <&cru CLK_RKNN_DSU0>,
+ <&cru ACLK_RKNN_CBUF>,
+ <&cru HCLK_RKNN_CBUF>;
+ resets = <&cru SRST_A_RKNN1_BIU>;
pm_qos = <&qos_npu_m1>;
#power-domain-cells = <0>;
};
@@ -1832,6 +1840,74 @@ qos_npu_m1ro: qos@27f22100 {
reg = <0x0 0x27f22100 0x0 0x20>;
};
+ rknn_core_0: npu@27700000 {
+ compatible = "rockchip,rk3576-rknn-core";
+ reg = <0x0 0x27700000 0x0 0x1000>,
+ <0x0 0x27701000 0x0 0x1000>,
+ <0x0 0x27703000 0x0 0x1000>;
+ reg-names = "pc", "cna", "core";
+ interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
+ <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>,
+ <&cru ACLK_RKNN_CBUF>, <&cru HCLK_RKNN_CBUF>;
+ clock-names = "aclk", "hclk", "npu", "pclk",
+ "aclk_cbuf", "hclk_cbuf";
+ resets = <&cru SRST_A_RKNN0>;
+ reset-names = "srst_a";
+ power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
+ iommus = <&rknn_mmu_0>;
+ status = "disabled";
+ };
+
+ rknn_mmu_0: iommu@27702000 {
+ compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0x27702000 0x0 0x100>,
+ <0x0 0x27702100 0x0 0x100>;
+ interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
+ <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
+ <&cru HCLK_RKNN_CBUF>;
+ clock-names = "aclk", "iface", "npu",
+ "aclk_cbuf", "hclk_cbuf";
+ #iommu-cells = <0>;
+ power-domains = <&power RK3576_PD_NPU0>;
+ status = "disabled";
+ };
+
+ rknn_core_1: npu@27708000 {
+ compatible = "rockchip,rk3576-rknn-core";
+ reg = <0x0 0x27708000 0x0 0x1000>,
+ <0x0 0x27709000 0x0 0x1000>,
+ <0x0 0x2770b000 0x0 0x1000>;
+ reg-names = "pc", "cna", "core";
+ interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+ <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>,
+ <&cru ACLK_RKNN_CBUF>, <&cru HCLK_RKNN_CBUF>;
+ clock-names = "aclk", "hclk", "npu", "pclk",
+ "aclk_cbuf", "hclk_cbuf";
+ resets = <&cru SRST_A_RKNN1>;
+ reset-names = "srst_a";
+ power-domains = <&power RK3576_PD_NPU1>, <&power RK3576_PD_NPU0>;
+ iommus = <&rknn_mmu_1>;
+ status = "disabled";
+ };
+
+ rknn_mmu_1: iommu@2770a000 {
+ compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0x2770a000 0x0 0x100>,
+ <0x0 0x2770a100 0x0 0x100>;
+ interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+ <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
+ <&cru HCLK_RKNN_CBUF>;
+ clock-names = "aclk", "iface", "npu",
+ "aclk_cbuf", "hclk_cbuf";
+ #iommu-cells = <0>;
+ power-domains = <&power RK3576_PD_NPU1>;
+ status = "disabled";
+ };
+
gmac0: ethernet@2a220000 {
compatible = "rockchip,rk3576-gmac", "snps,dwmac-4.20a";
reg = <0x0 0x2a220000 0x0 0x10000>;
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
2026-08-05 6:38 ` [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
@ 2026-08-05 7:46 ` sashiko-bot
0 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-08-05 7:46 UTC (permalink / raw)
To: Jiaxing Hu; +Cc: devicetree, dri-devel, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The device tree nodes are padded with fake power-domain dependencies to bypass the Linux driver core's single-domain auto-attach mechanism.
--
--- Patch [7]: [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes ---
commit 2901ef2a9f2683429a8cc97e3be5786a1b6b21b5
Author: Jiaxing Hu <gahing@gahingwoo.com>
arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
Add the two RKNN cores and their IOMMUs for the rk3576. Both cores are
disabled by default, and boards can enable what they wire up. The NPU
power-domain resets are also added.
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> --- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
[ ... ]
> @@ -1832,6 +1840,74 @@ qos_npu_m1ro: qos@27f22100 {
[ ... ]
> + rknn_core_0: npu@27700000 {
> + compatible = "rockchip,rk3576-rknn-core";
[ ... ]
> + resets = <&cru SRST_A_RKNN0>;
> + reset-names = "srst_a";
> + power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
[Severity: Medium]
Are these power domains accurately describing the hardware topology, or are they
encoding a software workaround?
The commit message mentions that listing both domains prevents the driver core
from auto-attaching a single domain. Device trees should accurately describe the
hardware instead of encoding software workarounds to bypass driver core behavior.
[ ... ]
> + rknn_core_1: npu@27708000 {
> + compatible = "rockchip,rk3576-rknn-core";
[ ... ]
> + resets = <&cru SRST_A_RKNN1>;
> + reset-names = "srst_a";
> + power-domains = <&power RK3576_PD_NPU1>, <&power RK3576_PD_NPU0>;
[Severity: Medium]
Will padding the power-domains list with RK3576_PD_NPU0 on rknn_core_1 cause
unintended side effects?
Since this artificially inflates the power-domain count to bypass the OS
auto-attach mechanism, it appears it will cause the system to unnecessarily
power on NPU0 when only Core 1 is active, wasting energy.
> + iommus = <&rknn_mmu_1>;
> + status = "disabled";
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260805063826.95682-1-gahing@gahingwoo.com?part=7
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH v5 v5 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (6 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
@ 2026-08-05 6:38 ` Jiaxing Hu
2026-08-05 9:06 ` [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Igor Paunovic
8 siblings, 0 replies; 21+ messages in thread
From: Jiaxing Hu @ 2026-08-05 6:38 UTC (permalink / raw)
To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing
Cc: royalnet026, alchark, chaoyi.chen, dri-devel, linux-rockchip,
iommu, linux-pm, devicetree, linux-arm-kernel, linux-kernel,
Jiaxing Hu
Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
core from vdd_npu_s0.
The supply is marked always-on because the NPU power domains are what
gate the block here, and dropping the rail underneath them takes an
async SError on the next power-on rather than a clean retry. Only
rknn_core_0 is enabled: the driver binds one core per node and the
second core is left to whoever can test it.
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 272af1012..965e0906b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -442,6 +442,7 @@ regulator-state-mem {
};
vdd_npu_s0: dcdc-reg2 {
+ regulator-always-on;
regulator-boot-on;
regulator-enable-ramp-delay = <400>;
regulator-min-microvolt = <550000>;
@@ -869,3 +870,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
remote-endpoint = <&hdmi_in_vp0>;
};
};
+
+&rknn_core_0 {
+ npu-supply = <&vdd_npu_s0>;
+ status = "okay";
+};
+
+&rknn_mmu_0 {
+ status = "okay";
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement
2026-08-05 6:38 [RFC PATCH v5 0/8] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
` (7 preceding siblings ...)
2026-08-05 6:38 ` [RFC PATCH v5 v5 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
@ 2026-08-05 9:06 ` Igor Paunovic
8 siblings, 0 replies; 21+ messages in thread
From: Igor Paunovic @ 2026-08-05 9:06 UTC (permalink / raw)
To: Jiaxing Hu
Cc: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
ulfh, p.zabel, ogabbay, zhangqing, alchark, chaoyi.chen,
dri-devel, linux-rockchip, iommu, linux-pm, devicetree,
linux-arm-kernel, linux-kernel
Hi Jiaxing,
Thank you for the fast respin, and for the correction in the cover letter.
Retracting the zero point reading in public, in the same cover that carries
the fix, is the right way to handle it, and it makes the series easier to
reason about rather than harder.
On the two items that touch rocket_job_handle_irq(): understood, and agreed
that a repeat of the v3 bench would not say anything useful about them. I
will characterise the shared path on RK3588 rather than re-run it, and
report separately on 6/8. What I plan to look at, so you can tell me if I
am aiming at the wrong thing:
- the bit exact oracle across a long run rather than a short one, since
the poll_seq move under job_lock changes when the completion path
observes a submit rather than whether it does;
- the interrupt accounting per inference, which on RK3588 is a stable
41/1/0 for MobileNetV1 on a single core, and is the cheapest signal I
have that the restructured completion path is not double counting or
dropping a task boundary;
- throughput against my current baseline, to catch a regression hiding
behind a still correct result;
- and an unbind/rebind cycle plus a module unload under load, which is
where the new dying flag actually earns its place.
If there is a specific ordering you want exercised, say so and I will add
it.
Now the open question, what condition makes the block load its
configuration at all. Three things, in the order I would spend time on
them.
1. The complement of your marker test.
Your marker byte covers the failing job's own output BOs. Would it be cheap
to run the mirror image: fill the *resident* convolution's output BOs with
a marker, then submit the different convolution that walls, then read the
resident job's buffers back rather than the new one's?
Everything you have measured is also consistent with the block never having
stopped executing the resident configuration. It computes byte exact from a
regcmd buffer full of 0xdeadbeef, so on a repeat submit it is demonstrably
not fetching. The failing job's output is untouched in 100 percent of the
buffer, which is exactly what you would see if the block wrote to the
previous task's addresses rather than to nothing at all. The resident
convolution keeps working across the failure, and going back to it is byte
exact. And the only case where corruption changes the result is the first
submit after a resume, which is the one submit that follows the domain
reset cycling patch 5 adds.
If the resident BO's marker is gone after a walling submit, then the
failing submit is not a no-op at all, it is a re-run of the last
configuration the block ever loaded, and the question narrows from "why
does it not start" to "why does the configuration fetch stop happening
after the first post-reset submit". If the marker survives there too, your
no-op reading stands and this costs you one run to confirm it.
2. Read pc_task_status (0x003C).
This is the one register that speaks directly to "does the block load at
all", and as far as I can tell nothing reads it: not the driver, not Mesa,
not this series. rocket_registers.h already defines REG_PC_TASK_STATUS, and
the driver has exactly three readl calls in the whole tree, for
INTERRUPT_RAW_STATUS and the two VERSION registers.
The RK3588 TRM describes 0x003C as bits [11:0] current task counter value,
plus flags for the first and last task operating and, separately, for the
first and last task's register *fetching*. (The bit numbering in that table
is visibly mistyped, 12 and 13 appear twice, so the flags would need
probing rather than trusting.)
Would a read of 0x003C before TASK_CON, after TASK_CON and after OP_EN, on
a computing submit and on a walling one, separate your two remaining cases?
A counter that still reads non-zero on the walling submit would say the
TASK_COUNT_CLEAR pulse that hw_submit issues on every submit is not landing
on RK3576, which is the same failure shape as the MMU that accepted reads
and silently dropped register writes until the right clocks were on. A
counter of zero with no fetch flag would say the PC never started, which
points somewhere else entirely.
If it helps, tell me where you want the reads and I will run the same three
on RK3588 and post what a known good submit looks like, so you have a
reference rather than an isolated number for a register neither of us has
ever seen the contents of.
3. The pc_sel readback, and one control run that only I can do.
hw_submit writes PC_BASE_ADDRESS twice: first 0x1, which the TRM defines as
pc_sel = 1, slave mode, "use AHB to set register", and then the regcmd
IOVA, whose bit 0 is 0 and which puts the block back into PC mode with AXI
DMA fetching the config. Nothing reads that register back. Given that you
already found S_POINTER bit 0 reads back 1 whatever is written, is it worth
reading 0x0010 back after the second write and checking both that pc_sel is
0 and that the address matches the IOVA you wrote? A block left in slave
mode fetches no configuration over AXI and keeps whatever register state is
resident, which is the shape of what you measured.
Separately, and this is the part where I can be useful rather than
speculative: I have RK3588 and the TRM, you have neither. If you tell me
precisely where you patch the regcmd head, I will run your 0xdeadbeef
corruption on a repeat submit on RK3588 and report whether it produces a
wrong result there. If RK3588 always fetches and always goes wrong under
that corruption, then "computes byte exact from a dead buffer" is already
anomalous on RK3576 for the submits you are counting as working, not only
for the one that walls, and that would move the resident state question to
the centre of the picture.
One smaller thing, since it is two lines. The TRM marks bits 16:0 of
pc_interrupt_raw_status (0x002C) and pc_interrupt_status (0x0028) as W1C in
their own right, not read only, and the driver only ever writes
pc_interrupt_clear (0x0024). Three write-one-to-clear registers, one of
which is written. Since the bits your poll tests, DPU_0 and DPU_1, survive
both the clear in hw_submit and the 0x1ffff clear in the completion path,
would writing 0x1ffff directly to 0x002C and re-reading it behave any
differently on RK3576? To be clear about what that would and would not buy
you: it is about the completion signalling, not about the fetch, so it
cannot explain why the block does not load. It would only tell you whether
the stale latch is a separate clear-path quirk you can take off the table
while you chase the real thing. And I am reading the RK3588 TRM, so I have
no way to know whether the RK3576 block keeps the same attributes there.
Thanks again for the credit in the cover letter. It was a pleasure to have
something useful to say.
Igor
P.S. The subject lines carry the version twice, "[RFC PATCH v5 v5 1/8]"
through 8/8; the cover is the only one tagged correctly. Harmless to read,
but b4 and patchwork both parse that field, so it is worth fixing before
the next spin.
On Wed, Aug 5, 2026 at 8:38 AM Jiaxing Hu <gahing@gahingwoo.com> wrote:
>
> v4 was supposed to be a fixes only revision and its patch 4 was not.
> I trimmed the series out of my debugging tree and the trim missed
> rocket_job.c entirely, so 202 lines of ping-pong experiment went out
> with it, including a rocket_core_state_init() call that runs
> unconditionally from rocket_device_runtime_resume() and would therefore
> have replayed the RK3576 vendor init on RK3588 as well. Igor Paunovic
> caught it when he went to re-test, before it cost anyone else time.
> Sorry for the noise.
>
> That code is gone. rocket_job.c is back to +72 lines, which is v3 plus
> the two fixes v4 was meant to carry.
>
> Tested on a Radxa ROCK 4D, on next-20260730.
>
> Changes in v5
> -------------
>
> * accel/rocket: the experiment code v4 shipped by mistake is removed.
> No module parameters, no snapshot ioremap, no regcmd patching.
>
> * dt-bindings: the RK3576 nodes never validated against the RK3588
> binding, which still described RK3588's shape only. Igor ran
> dt-validate and found ten failures across the two cores: six clocks
> and two power domains where the schema allowed four and one, and a
> single reset where dtschema infers minItems from maxItems and so
> requires two. The property ranges are widened and each SoC is pinned
> back to its own shape in allOf, so nothing loosens for RK3588. I
> checked that by giving an RK3588 node a fifth clock and confirming
> the schema still rejects it.
>
> * dt-bindings: new patch. The resets that patch 5 adds to the NPU power
> domain nodes had no binding at all, and pd-node is
> unevaluatedProperties: false at every level, so the DTS could not
> validate. Also found by Igor.
>
> * dt-bindings: new patch. The NPU MMU nodes carry five clocks and no
> clock-names, which rockchip,iommu.yaml does not allow either. This
> one is not cosmetic: with only aclk and iface enabled the MMU accepts
> reads and silently drops register writes, which is what commit
> 841363ebb508 ("iommu/rockchip: Take all DT clocks") was for. The
> schema is widened to match, minItems stays at 2 so every existing
> devicetree is unaffected, and the DTS gets its clock-names back.
> Found by the Sashiko bot.
>
> * accel/rocket: the poll work checked its sequence number outside
> job_lock, which narrowed the race it was meant to close rather than
> closing it. poll_seq only moves under job_lock, in hw_submit, so the
> check belongs there too. The shared part of the completion path is
> split into a helper both callers use. Also from the bot.
>
> * accel/rocket: rocket_job_fini() cancelled the poll after
> drm_sched_fini(), but the completion path submits the job's next
> task, and drm_sched_fini() does not wait for work already queued, so
> a poll could arm the hardware while teardown was disabling clocks.
> A dying flag now stops that before the scheduler goes away, and the
> cancel stays after it so a job running at that moment cannot re-arm
> the timer behind it. Also from the bot.
>
> * rk3576-rock-4d.dts: the commit message now says why the supply is
> marked always-on and why only core 0 is enabled, rather than leaving
> both to be asked about.
>
> Igor, this is not the mechanical respin I said it would be. The last two
> items restructure rocket_job_handle_irq(), which is the RK3588 path as
> well, so it needs characterising rather than a repeat of your v3 bench.
> Both are gated on soc->poll_completion for behaviour, but the code
> underneath is shared. Thanks for reading the diff instead of trusting
> the cover letter.
>
> Verified on hardware, twice, with every debug knob off: the NPU probes
> with the two domain list, a known byte exact convolution stays byte exact
> six times over, running a different one and coming back is unchanged, and
> unbind/rebind rebinds cleanly and runs again with no warning.
>
> What is still wrong
> -------------------
>
> A single convolution is byte exact, and re-running it is byte exact every
> time. A different one after it computes nothing, while the resident one
> keeps working, and going back to it is byte exact again.
>
> Igor suggested on the v4 thread that this reads less like a register we
> fail to write and more like something the block never re-fetches, and
> that the useful question is whether the regcmd is read at all. That
> turned out to be the right question, and the answer is now measured.
>
> Overwriting the head of the regcmd buffer in place, just before OP_EN,
> changes nothing for a repeat submit: it computes byte exact from a buffer
> full of 0xdeadbeef. The same corruption on the first submit after a
> resume makes that job wrong. So some submits load their configuration and
> some run from resident state, and the write itself is confirmed by
> reading it back.
>
> Filling the output BOs with a marker byte just before OP_EN says what the
> failing submit does with that state. On a submit that computes, the
> marker is gone from every byte and the result is correct. On the submit
> that walls, the marker survives in 100% of the buffer.
>
> So the failing submit is a no-op. It does not read its configuration, it
> does not compute, and it never writes its output, which means it does not
> know where the output goes. It still looks like a completion, because
> INTERRUPT_RAW_STATUS PC_DONE is permanently latched and the poll
> condition is therefore always already true.
>
> I have to correct something in the v3 and v4 cover letters here. Both
> said the failing job "writes out a zero point surface", and I read that
> as the MAC producing nothing. That was wrong. A fresh shmem BO is zeroed,
> 0x00 plus the +0x80 that teflon applies on readback is 128, and 128 is
> exactly what I had been calling the zero point fill. The buffer was never
> written at all. Nothing was ever measured about the MAC on this path.
>
> That also retires the ping-pong lead from the v3 thread, and not because
> the observation was wrong. The pointer is stuck, S_POINTER bit 0 reads
> back 1 whatever we write, but flipping it, selecting a bank the way
> rk3576_state_init does, and pulsing POINTER_PP_CLEAR are all null, the
> vendor does not switch banks per submit either, and adding its state_init
> verbatim changes nothing. The bank is about where a configuration lands,
> and the configuration is not being read in the first place.
>
> What is left is the condition under which the block loads at all. That is
> what I am chasing now, and suggestions are very welcome.
>
> I used Claude Opus 5 to trim this series out of my debugging tree and
> generate the diffs. It is also what missed the hunk in v4, so this time
> the trimmed tree was diffed against v3 patch by patch and grepped for
> every experiment symbol before sending.
>
> Jiaxing Hu (8):
> dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
> dt-bindings: power: rockchip: allow resets in a power domain node
> dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set
> pmdomain/rockchip: add optional per-domain power-on settle delay
> pmdomain/rockchip: cycle optional power-domain resets on power-on
> accel/rocket: add RK3576 NPU (RKNN) support
> arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
> arm64: dts: rockchip: rk3576-rock-4d: enable NPU
>
> .../devicetree/bindings/iommu/rockchip,iommu.yaml | 8 ++
> .../bindings/npu/rockchip,rk3588-rknn-core.yaml | 47 +++++++-
> .../bindings/power/rockchip,power-controller.yaml | 8 ++
> arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 10 ++
> arch/arm64/boot/dts/rockchip/rk3576.dtsi | 80 +++++++++++++-
> drivers/accel/rocket/rocket_core.c | 26 ++++-
> drivers/accel/rocket/rocket_core.h | 20 +++-
> drivers/accel/rocket/rocket_device.c | 4 +
> drivers/accel/rocket/rocket_drv.c | 22 +++-
> drivers/accel/rocket/rocket_job.c | 121 +++++++++++++++++++--
> drivers/pmdomain/rockchip/pm-domains.c | 71 ++++++++----
> 11 files changed, 373 insertions(+), 44 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply [flat|nested] 21+ messages in thread