[RFC PATCH 0/9] accel/rocket: RK3576 NPU (RKNN) enablement

[RFC PATCH 0/9] accel/rocket: RK3576 NPU (RKNN) enablement

[Jiaxing Hu]

This adds RK3576 NPU support to accel/rocket. RFC: the NPU brings up and
runs, but full inference is not correct yet, and I would rather get
feedback now than sit on it.

Tested on a Radxa ROCK 4D. The NPU probes, powers on, brings up its
IOMMUs, and runs jobs to completion (rocket + Mesa Teflon). Getting there
took a few fixes outside the driver too: a power-domain settle delay and
reset cycling (else a cold power-on throws an async SError), two rk_iommu
fixes for banks firmware leaves faulted, taking the full clock set on the
MMU nodes, and both NPU power domains on the core.

Not solved: only the first operation in a power session gives the right
answer. Every later one engages and DMAs its input, but the compute
output never lands, so the readback is the zero-point. I chased it
through the whole software surface (registers, IOMMU/TLB, cache,
power/clock/reset, dispatch) and it reproduces even with the driver
environment matched to the vendor's on the same kernel, so it looks to be
below software, in the fixed-function block. Traces available.

Two asks: review of the bring-up and the shared pmdomain/iommu changes;
and if anyone knows the RK3576 NPU internals, whether that
per-power-session arm can be driven from software at all.

The series is split by subsystem (binding, pmdomain, iommu, driver, DT).
The board DT keeps vdd_npu_s0 always-on for now; happy to switch to
proper runtime control if preferred.

I used Claude Opus 4.8 to help trim this series out of my debugging tree
and generate the diffs.

Jiaxing Hu (9):
  dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  pmdomain/rockchip: add optional per-domain power-on settle delay
  pmdomain/rockchip: cycle optional power-domain resets on power-on
  iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active
  iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch
  iommu/rockchip: take all DT clocks
  accel/rocket: add RK3576 NPU (RKNN) support
  arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  arm64: dts: rockchip: rk3576-rock-4d: enable NPU

 .../npu/rockchip,rk3588-rknn-core.yaml        |   2 +-
 .../boot/dts/rockchip/rk3576-rock-4d.dts      |  16 ++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      |  78 ++++++++-
 drivers/accel/rocket/rocket_core.c            |  43 ++++-
 drivers/accel/rocket/rocket_core.h            |  22 ++-
 drivers/accel/rocket/rocket_device.c          |   4 +
 drivers/accel/rocket/rocket_drv.c             |   1 +
 drivers/accel/rocket/rocket_job.c             | 154 ++++++++++++++----
 drivers/iommu/rockchip-iommu.c                |  66 +++++---
 drivers/pmdomain/rockchip/pm-domains.c        |  68 +++++---
 10 files changed, 367 insertions(+), 87 deletions(-)

-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 0/9] accel/rocket: RK3576 NPU (RKNN) enablement

[Jiaxing Hu]

This adds RK3576 NPU support to accel/rocket. RFC: the NPU brings up and
runs, but full inference is not correct yet, and I would rather get
feedback now than sit on it.

Tested on a Radxa ROCK 4D. The NPU probes, powers on, brings up its
IOMMUs, and runs jobs to completion (rocket + Mesa Teflon). Getting there
took a few fixes outside the driver too: a power-domain settle delay and
reset cycling (else a cold power-on throws an async SError), two rk_iommu
fixes for banks firmware leaves faulted, taking the full clock set on the
MMU nodes, and both NPU power domains on the core.

Not solved: only the first operation in a power session gives the right
answer. Every later one engages and DMAs its input, but the compute
output never lands, so the readback is the zero-point. I chased it
through the whole software surface (registers, IOMMU/TLB, cache,
power/clock/reset, dispatch) and it reproduces even with the driver
environment matched to the vendor's on the same kernel, so it looks to be
below software, in the fixed-function block. Traces available.

Two asks: review of the bring-up and the shared pmdomain/iommu changes;
and if anyone knows the RK3576 NPU internals, whether that
per-power-session arm can be driven from software at all.

The series is split by subsystem (binding, pmdomain, iommu, driver, DT).
The board DT keeps vdd_npu_s0 always-on for now; happy to switch to
proper runtime control if preferred.

I used Claude Opus 4.8 to help trim this series out of my debugging tree
and generate the diffs.

Jiaxing Hu (9):
  dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  pmdomain/rockchip: add optional per-domain power-on settle delay
  pmdomain/rockchip: cycle optional power-domain resets on power-on
  iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active
  iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch
  iommu/rockchip: take all DT clocks
  accel/rocket: add RK3576 NPU (RKNN) support
  arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  arm64: dts: rockchip: rk3576-rock-4d: enable NPU

 .../npu/rockchip,rk3588-rknn-core.yaml        |   2 +-
 .../boot/dts/rockchip/rk3576-rock-4d.dts      |  16 ++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      |  78 ++++++++-
 drivers/accel/rocket/rocket_core.c            |  43 ++++-
 drivers/accel/rocket/rocket_core.h            |  22 ++-
 drivers/accel/rocket/rocket_device.c          |   4 +
 drivers/accel/rocket/rocket_drv.c             |   1 +
 drivers/accel/rocket/rocket_job.c             | 154 ++++++++++++++----
 drivers/iommu/rockchip-iommu.c                |  66 +++++---
 drivers/pmdomain/rockchip/pm-domains.c        |  68 +++++---
 10 files changed, 367 insertions(+), 87 deletions(-)

-- 
2.43.0

[RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Jiaxing Hu]

Document the RK3576 NPU core: add rockchip,rk3576-rknn-core to the
compatible enum and drop sram-supply from required (RK3576 has no NPU
SRAM supply).

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
index caca2a490..d904db3bf 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:
@@ -75,7 +76,6 @@ required:
   - resets
   - reset-names
   - npu-supply
-  - sram-supply
 
 additionalProperties: false
 
-- 
2.43.0

[RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Jiaxing Hu]

Document the RK3576 NPU core: add rockchip,rk3576-rknn-core to the
compatible enum and drop sram-supply from required (RK3576 has no NPU
SRAM supply).

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
index caca2a490..d904db3bf 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:
@@ -75,7 +76,6 @@ required:
   - resets
   - reset-names
   - npu-supply
-  - sram-supply
 
 additionalProperties: false
 
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing `sram-supply` from the global `required` list makes it optional for `rockchip,rk3588-rknn-core`, incorrectly weakening schema validation for existing RK3588 hardware.
--

--- Patch [1]: [RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core ---
commit 9a2dabf1434398eb090f8e36b12d42467c86de03
Author: Jiaxing Hu <gahing@gahingwoo.com>

dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

This commit documents the RK3576 NPU core by adding the new compatible
string rockchip,rk3576-rknn-core. It also drops sram-supply from the global
required list since the RK3576 has no NPU SRAM supply.

> diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> index caca2a4903cd1..d904db3bf9281 100644
> --- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> +++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
[ ... ]
> @@ -75,7 +76,6 @@ required:
>    - resets
>    - reset-names
>    - npu-supply
> -  - sram-supply

[Severity: Medium]
By dropping sram-supply from the global required list to accommodate the
RK3576, does this incorrectly weaken schema validation for existing
RK3588 hardware?

If an RK3588 device tree node is missing the sram-supply property, it
appears it will now silently pass make dtbs_check validation.

Could this instead use an allOf block with an if-then conditional to
enforce the sram-supply requirement specifically when the compatible string
is rockchip,rk3588-rknn-core?

>  
>  additionalProperties: false

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=1

Re: [RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Document the RK3576 NPU core: add rockchip,rk3576-rknn-core to the
> compatible enum and drop sram-supply from required (RK3576 has no NPU
> SRAM supply).
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  .../devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml      | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> index caca2a490..d904db3bf 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:
> @@ -75,7 +76,6 @@ required:
>    - resets
>    - reset-names
>    - npu-supply
> -  - sram-supply
>

You should not delete this item directly. Instead, use the compatible 
to distinguish whether it is required.

>  additionalProperties: false
>  

-- 
Best, 
Chaoyi

Re: [RFC PATCH 1/9] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Document the RK3576 NPU core: add rockchip,rk3576-rknn-core to the
> compatible enum and drop sram-supply from required (RK3576 has no NPU
> SRAM supply).
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  .../devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml      | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> index caca2a490..d904db3bf 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:
> @@ -75,7 +76,6 @@ required:
>    - resets
>    - reset-names
>    - npu-supply
> -  - sram-supply
>

You should not delete this item directly. Instead, use the compatible 
to distinguish whether it is required.

>  additionalProperties: false
>  

-- 
Best, 
Chaoyi

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 2/9] pmdomain/rockchip: add optional per-domain power-on settle delay

[Jiaxing Hu]

The RK3576 NPU domains need a short settle time after the idle
request is released before the QoS registers behind the domain are
written: restoring QoS immediately after de-idle can raise an SError
on the register access while the NoC is still settling (observed on
ROCK 4D as a panic inside rockchip_pd_power() during NPU runtime-PM
cycling).

The vendor kernel handles this with a per-domain delay_us field
applied between the de-idle request and the QoS restore, carrying
15us for the RK3576 NPUTOP domain.  Mirror that: add delay_us to
rockchip_domain_info, honor it in rockchip_pd_power() right before
rockchip_pmu_restore_qos(), and set 15us for NPUTOP, NPU0 and NPU1
(the two core domains power-cycle constantly under runtime PM and
showed the same symptom during bring-up).  All other domains keep 0,
so nothing changes for them.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 51 ++++++++++++++------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 490bbb1d1..1787f62ba 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -59,6 +59,7 @@ struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -185,7 +186,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(_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 +201,7 @@ struct rockchip_pmu {
 	.req_mask = (req),				\
 	.idle_mask = (idle),				\
 	.clk_ungate_mask = (g_mask),			\
+	.delay_us = (delay),				\
 	.ack_mask = (ack),				\
 	.active_wakeup = wakeup,			\
 }
@@ -244,8 +246,8 @@ struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup)		\
 	DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
 
-#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(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
@@ -612,7 +614,6 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
 			return ret;
 	}
 
-
 	ret = readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
 					is_on == on, 0, 10000);
 	if (ret) {
@@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (ret < 0)
 			goto out;
 
+		if (pd->info->delay_us)
+			udelay(pd->info->delay_us);
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -1286,25 +1290,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

[RFC PATCH 2/9] pmdomain/rockchip: add optional per-domain power-on settle delay

[Jiaxing Hu]

The RK3576 NPU domains need a short settle time after the idle
request is released before the QoS registers behind the domain are
written: restoring QoS immediately after de-idle can raise an SError
on the register access while the NoC is still settling (observed on
ROCK 4D as a panic inside rockchip_pd_power() during NPU runtime-PM
cycling).

The vendor kernel handles this with a per-domain delay_us field
applied between the de-idle request and the QoS restore, carrying
15us for the RK3576 NPUTOP domain.  Mirror that: add delay_us to
rockchip_domain_info, honor it in rockchip_pd_power() right before
rockchip_pmu_restore_qos(), and set 15us for NPUTOP, NPU0 and NPU1
(the two core domains power-cycle constantly under runtime PM and
showed the same symptom during bring-up).  All other domains keep 0,
so nothing changes for them.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 51 ++++++++++++++------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 490bbb1d1..1787f62ba 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -59,6 +59,7 @@ struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -185,7 +186,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(_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 +201,7 @@ struct rockchip_pmu {
 	.req_mask = (req),				\
 	.idle_mask = (idle),				\
 	.clk_ungate_mask = (g_mask),			\
+	.delay_us = (delay),				\
 	.ack_mask = (ack),				\
 	.active_wakeup = wakeup,			\
 }
@@ -244,8 +246,8 @@ struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup)		\
 	DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
 
-#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(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
@@ -612,7 +614,6 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
 			return ret;
 	}
 
-
 	ret = readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
 					is_on == on, 0, 10000);
 	if (ret) {
@@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (ret < 0)
 			goto out;
 
+		if (pd->info->delay_us)
+			udelay(pd->info->delay_us);
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -1286,25 +1290,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


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 2/9] pmdomain/rockchip: add optional per-domain power-on settle delay

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized State Overwrite in QoS registers on first power-on
--

--- Patch [2]: [RFC PATCH 2/9] pmdomain/rockchip: add optional per-domain power-on settle delay ---
commit a8d7f648552a5a45a8073245f6be06268bc14e7d
Author: Jiaxing Hu <gahing@gahingwoo.com>

pmdomain/rockchip: add optional per-domain power-on settle delay

This commit introduces a per-domain settle delay for the RK3576 NPU domains.
The delay is applied between releasing the idle request and writing to the
QoS registers to prevent an SError while the NoC is settling.

> 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

[ ... ]

> @@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>  		if (ret < 0)
>  			goto out;
>  
> +		if (pd->info->delay_us)
> +			udelay(pd->info->delay_us);
> +
>  		rockchip_pmu_restore_qos(pd);

[Severity: High]
This is a pre-existing issue, but does this unconditional restore overwrite
QoS registers with uninitialized state on the very first power-on?

If a power domain is initially OFF at boot, qos_save_regs remains
zero-initialized by devm_kcalloc(). On the first power-on request,
rockchip_pd_power() unconditionally calls rockchip_pmu_restore_qos(),
which writes these zeros directly to the hardware:

drivers/pmdomain/rockchip/pm-domains.c:rockchip_pmu_restore_qos() {
    ...
    regmap_write(pd->qos_regmap[i], QOS_PRIORITY, pd->qos_save_regs[0][i]);
    ...
}

If the hardware reset defaults for QoS bandwidth or priority are non-zero,
writing zeros could throttle or block interconnect traffic, potentially causing
the associated IP block to hang when accessing memory.

Is there a mechanism to ensure the QoS state is actually populated before it
is restored, or should the driver avoid restoring QoS state if a prior state
was never saved?

>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=2

[RFC PATCH 3/9] pmdomain/rockchip: cycle optional power-domain resets on power-on

[Jiaxing Hu]

Some power domains contain hardware bus interfaces that require an
explicit assert/deassert edge on their reset line after the domain
is powered on.  The CRU preserves reset state across power cycles
(stays deasserted), so the hardware never sees the edge it needs to
initialize unless the driver pulses it explicitly.

Add an optional resets property to rockchip_pm_domain.  When present,
the resets are asserted and deasserted (with a 10 µs hold) inside
rockchip_pd_power() after the idle bits are cleared and the
delay_us settling time has elapsed, but before any device in the
domain resumes.  This ordering guarantees the bus is fully
initialized before any device driver accesses registers.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 1787f62ba..7a88a9149 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/reset.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -102,6 +103,7 @@ struct rockchip_pm_domain {
 	struct clk_bulk_data *clks;
 	struct device_node *node;
 	struct regulator *supply;
+	struct reset_control *resets;
 };
 
 struct rockchip_pmu {
@@ -671,6 +673,12 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (pd->info->delay_us)
 			udelay(pd->info->delay_us);
 
+		if (pd->resets) {
+			reset_control_assert(pd->resets);
+			udelay(10);
+			reset_control_deassert(pd->resets);
+		}
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -840,6 +848,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);
 
@@ -910,6 +926,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;
 }
 
-- 
2.43.0

[RFC PATCH 3/9] pmdomain/rockchip: cycle optional power-domain resets on power-on

[Jiaxing Hu]

Some power domains contain hardware bus interfaces that require an
explicit assert/deassert edge on their reset line after the domain
is powered on.  The CRU preserves reset state across power cycles
(stays deasserted), so the hardware never sees the edge it needs to
initialize unless the driver pulses it explicitly.

Add an optional resets property to rockchip_pm_domain.  When present,
the resets are asserted and deasserted (with a 10 µs hold) inside
rockchip_pd_power() after the idle bits are cleared and the
delay_us settling time has elapsed, but before any device in the
domain resumes.  This ordering guarantees the bus is fully
initialized before any device driver accesses registers.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 1787f62ba..7a88a9149 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/reset.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -102,6 +103,7 @@ struct rockchip_pm_domain {
 	struct clk_bulk_data *clks;
 	struct device_node *node;
 	struct regulator *supply;
+	struct reset_control *resets;
 };
 
 struct rockchip_pmu {
@@ -671,6 +673,12 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (pd->info->delay_us)
 			udelay(pd->info->delay_us);
 
+		if (pd->resets) {
+			reset_control_assert(pd->resets);
+			udelay(10);
+			reset_control_deassert(pd->resets);
+		}
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -840,6 +848,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);
 
@@ -910,6 +926,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;
 }
 
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 3/9] pmdomain/rockchip: cycle optional power-domain resets on power-on

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `reset_control_put()` in `rockchip_pm_remove_one_domain()` leaks reset controls on probe failure.
--

--- Patch [3]: [RFC PATCH 3/9] pmdomain/rockchip: cycle optional power-domain resets on power-on ---
commit dbabfedb9da6564f3dfbd42512d2a456cfb08c18
Author: Jiaxing Hu <gahing@gahingwoo.com>

pmdomain/rockchip: cycle optional power-domain resets on power-on

This commit adds optional reset line handling to the Rockchip power domain
driver. It explicitly asserts and deasserts the reset lines during domain
power-on to ensure hardware bus interfaces are correctly initialized.

> 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

[ ... ]

> @@ -840,6 +848,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;
> +	}
> +

[Severity: Medium]
Since pd->resets is acquired here using a non-managed function, does it need
to be released in rockchip_pm_remove_one_domain()?

If a subsequent domain fails to probe, the error path tears down all
successfully added domains via rockchip_pm_remove_one_domain(). It looks like
that function destroys the domain and releases its clocks, but might miss
cleaning up these reset controls:

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);

        /* pd->resets does not appear to be freed here */
        ...
}

Would calling reset_control_put(pd->resets) in the removal path prevent a
resource leak here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=3

[RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

[Jiaxing Hu]

Boot firmware can leave a PAGE_FAULT_ACTIVE condition in an IOMMU bank
before the driver has configured paging.  Such a bank shows:

  PAGE_FAULT_ACTIVE=1  STALL_ACTIVE=0  IDLE=1

rk_iommu_is_stall_active() sees STALL_ACTIVE=0 and reports the whole
IOMMU as "not stalled", so any subsequent readx_poll_timeout() loop
waiting for the stall to complete never sees a passing result even
after the other banks have correctly entered stall mode.

Detect the orphaned-fault pattern (PAGE_FAULT_ACTIVE & !STALL_ACTIVE &
IDLE) and skip those banks in the stall check.  They have no in-flight
transaction, so they are already quiescent.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0013cf196..f42ffcafb 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -378,9 +378,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
 	bool active = true;
 	int i;
 
-	for (i = 0; i < iommu->num_mmu; i++)
-		active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
-					   RK_MMU_STATUS_STALL_ACTIVE);
+	for (i = 0; i < iommu->num_mmu; i++) {
+		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
+
+		/*
+		 * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
+		 * and with IDLE set has an orphaned fault left by firmware before
+		 * paging was configured.  It cannot enter stall mode but has no
+		 * transaction in flight, so it is already quiescent.  Skip it
+		 * rather than treating it as "not stalled."
+		 */
+		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
+		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
+		    (status & RK_MMU_STATUS_IDLE))
+			continue;
+
+		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
+	}
 
 	return active;
 }
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

[Jiaxing Hu]

Boot firmware can leave a PAGE_FAULT_ACTIVE condition in an IOMMU bank
before the driver has configured paging.  Such a bank shows:

  PAGE_FAULT_ACTIVE=1  STALL_ACTIVE=0  IDLE=1

rk_iommu_is_stall_active() sees STALL_ACTIVE=0 and reports the whole
IOMMU as "not stalled", so any subsequent readx_poll_timeout() loop
waiting for the stall to complete never sees a passing result even
after the other banks have correctly entered stall mode.

Detect the orphaned-fault pattern (PAGE_FAULT_ACTIVE & !STALL_ACTIVE &
IDLE) and skip those banks in the stall check.  They have no in-flight
transaction, so they are already quiescent.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0013cf196..f42ffcafb 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -378,9 +378,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
 	bool active = true;
 	int i;
 
-	for (i = 0; i < iommu->num_mmu; i++)
-		active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
-					   RK_MMU_STATUS_STALL_ACTIVE);
+	for (i = 0; i < iommu->num_mmu; i++) {
+		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
+
+		/*
+		 * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
+		 * and with IDLE set has an orphaned fault left by firmware before
+		 * paging was configured.  It cannot enter stall mode but has no
+		 * transaction in flight, so it is already quiescent.  Skip it
+		 * rather than treating it as "not stalled."
+		 */
+		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
+		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
+		    (status & RK_MMU_STATUS_IDLE))
+			continue;
+
+		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
+	}
 
 	return active;
 }
-- 
2.43.0

Re: [RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Skipping orphaned-fault banks in `rk_iommu_is_stall_active` causes `rk_iommu_disable_stall` to unconditionally time out if all banks are skipped (e.g., on a single-bank IOMMU).

Pre-existing issues:
- [High] Multi-bank disable operations (`rk_iommu_disable_stall` and `rk_iommu_disable_paging`) prematurely terminate their waits as soon as the first bank completes its operation.
--

--- Patch [4]: [RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active ---
commit 0709b1254bd6d532239176eba388c28b0ed8ea3c
Author: Jiaxing Hu <gahing@gahingwoo.com>

iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

This commit updates rk_iommu_is_stall_active() to detect and skip
orphaned-fault IOMMU banks that were left in a faulted state by boot
firmware. This ensures the driver doesn't treat them as "not stalled"
which would otherwise cause stall wait loops to fail.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 87ae036d64145..ad462cee6bd89 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -380,9 +380,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
>  	bool active = true;
>  	int i;
>  
> -	for (i = 0; i < iommu->num_mmu; i++)
> -		active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> -					   RK_MMU_STATUS_STALL_ACTIVE);
> +	for (i = 0; i < iommu->num_mmu; i++) {
> +		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
> +
> +		/*
> +		 * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
> +		 * and with IDLE set has an orphaned fault left by firmware before
> +		 * paging was configured.  It cannot enter stall mode but has no
> +		 * transaction in flight, so it is already quiescent.  Skip it
> +		 * rather than treating it as "not stalled."
> +		 */
> +		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
> +		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
> +		    (status & RK_MMU_STATUS_IDLE))
> +			continue;

[Severity: Medium]
Does skipping banks here cause a timeout if all banks are skipped?

If an IOMMU has a single bank (num_mmu == 1) and encounters this orphaned
fault, the loop skips the only bank and the active variable remains true.

When rk_iommu_disable_stall() polls for the stall to complete:

	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
				 !val, RK_MMU_POLL_PERIOD_US,
				 RK_MMU_POLL_TIMEOUT_US);

It expects rk_iommu_is_stall_active() to return false, but it will return
true, causing an unconditional timeout and a spurious error log.

> +
> +		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);

[Severity: High]
This is a pre-existing issue, but does this bitwise AND reduction cause
multi-bank disable operations to terminate their waits prematurely?

When disabling an IOMMU with multiple banks (num_mmu > 1), returning false
as soon as the first bank drops its STALL_ACTIVE status leaves subsequent
banks still active.

The same logic exists in rk_iommu_is_paging_enabled(), where the bitwise
AND causes readx_poll_timeout() to terminate its wait as soon as one bank
is disabled, rather than waiting for all banks:

	for (i = 0; i < iommu->num_mmu; i++)
		enable &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
					   RK_MMU_STATUS_PAGING_ENABLED);

Can this lead to modifying register state or disabling hardware clocks while
some MMU banks are still actively processing commands?

> +	}
>  
>  	return active;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=4

Re: [RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

[Chaoyi Chen]

Hello Jiaxing, 

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Boot firmware can leave a PAGE_FAULT_ACTIVE condition in an IOMMU bank
> before the driver has configured paging.  Such a bank shows:
> 

I'm not sure what's happening here. Does this issue still occur when
you reset the MMU?

>   PAGE_FAULT_ACTIVE=1  STALL_ACTIVE=0  IDLE=1
> 
> rk_iommu_is_stall_active() sees STALL_ACTIVE=0 and reports the whole
> IOMMU as "not stalled", so any subsequent readx_poll_timeout() loop
> waiting for the stall to complete never sees a passing result even
> after the other banks have correctly entered stall mode.
> 
> Detect the orphaned-fault pattern (PAGE_FAULT_ACTIVE & !STALL_ACTIVE &
> IDLE) and skip those banks in the stall check.  They have no in-flight
> transaction, so they are already quiescent.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/iommu/rockchip-iommu.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0013cf196..f42ffcafb 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -378,9 +378,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
>  	bool active = true;
>  	int i;
>  
> -	for (i = 0; i < iommu->num_mmu; i++)
> -		active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> -					   RK_MMU_STATUS_STALL_ACTIVE);
> +	for (i = 0; i < iommu->num_mmu; i++) {
> +		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
> +
> +		/*
> +		 * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
> +		 * and with IDLE set has an orphaned fault left by firmware before
> +		 * paging was configured.  It cannot enter stall mode but has no
> +		 * transaction in flight, so it is already quiescent.  Skip it
> +		 * rather than treating it as "not stalled."
> +		 */
> +		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
> +		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
> +		    (status & RK_MMU_STATUS_IDLE))
> +			continue;
> +
> +		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
> +	}
>  
>  	return active;
>  }

-- 
Best, 
Chaoyi

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 4/9] iommu/rockchip: skip orphaned-fault banks in rk_iommu_is_stall_active

[Chaoyi Chen]

Hello Jiaxing, 

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Boot firmware can leave a PAGE_FAULT_ACTIVE condition in an IOMMU bank
> before the driver has configured paging.  Such a bank shows:
> 

I'm not sure what's happening here. Does this issue still occur when
you reset the MMU?

>   PAGE_FAULT_ACTIVE=1  STALL_ACTIVE=0  IDLE=1
> 
> rk_iommu_is_stall_active() sees STALL_ACTIVE=0 and reports the whole
> IOMMU as "not stalled", so any subsequent readx_poll_timeout() loop
> waiting for the stall to complete never sees a passing result even
> after the other banks have correctly entered stall mode.
> 
> Detect the orphaned-fault pattern (PAGE_FAULT_ACTIVE & !STALL_ACTIVE &
> IDLE) and skip those banks in the stall check.  They have no in-flight
> transaction, so they are already quiescent.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/iommu/rockchip-iommu.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 0013cf196..f42ffcafb 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -378,9 +378,23 @@ static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
>  	bool active = true;
>  	int i;
>  
> -	for (i = 0; i < iommu->num_mmu; i++)
> -		active &= !!(rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> -					   RK_MMU_STATUS_STALL_ACTIVE);
> +	for (i = 0; i < iommu->num_mmu; i++) {
> +		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
> +
> +		/*
> +		 * A bank stuck with PAGE_FAULT_ACTIVE but without STALL_ACTIVE
> +		 * and with IDLE set has an orphaned fault left by firmware before
> +		 * paging was configured.  It cannot enter stall mode but has no
> +		 * transaction in flight, so it is already quiescent.  Skip it
> +		 * rather than treating it as "not stalled."
> +		 */
> +		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
> +		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
> +		    (status & RK_MMU_STATUS_IDLE))
> +			continue;
> +
> +		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
> +	}
>  
>  	return active;
>  }

-- 
Best, 
Chaoyi

[RFC PATCH 5/9] iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch

[Jiaxing Hu]

Sending CMD_ENABLE_STALL to a bank that carries an orphaned firmware
fault (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE) causes the command to
be silently dropped by that bank.  The dropped command also interferes
with the other banks' stall transition, delaying them past the poll
timeout even though they would otherwise stall within a few
microseconds.

Skip such banks when issuing CMD_ENABLE_STALL, matching the skip
condition already used in rk_iommu_is_stall_active().

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index f42ffcafb..ce586919b 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -434,7 +434,21 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
 	if (!rk_iommu_is_paging_enabled(iommu))
 		return 0;
 
-	rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);
+	/*
+	 * Do not send CMD_ENABLE_STALL to orphaned-fault banks
+	 * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE): the command is ignored
+	 * by such a bank but its presence on the shared bus delays the other
+	 * banks from reaching STALL_ACTIVE within the poll timeout.
+	 */
+	for (i = 0; i < iommu->num_mmu; i++) {
+		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
+
+		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
+		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
+		    (status & RK_MMU_STATUS_IDLE))
+			continue;
+		writel(RK_MMU_CMD_ENABLE_STALL, iommu->bases[i] + RK_MMU_COMMAND);
+	}
 
 	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
 				 val, RK_MMU_POLL_PERIOD_US,
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 5/9] iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch

[Jiaxing Hu]

Sending CMD_ENABLE_STALL to a bank that carries an orphaned firmware
fault (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE) causes the command to
be silently dropped by that bank.  The dropped command also interferes
with the other banks' stall transition, delaying them past the poll
timeout even though they would otherwise stall within a few
microseconds.

Skip such banks when issuing CMD_ENABLE_STALL, matching the skip
condition already used in rk_iommu_is_stall_active().

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index f42ffcafb..ce586919b 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -434,7 +434,21 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
 	if (!rk_iommu_is_paging_enabled(iommu))
 		return 0;
 
-	rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);
+	/*
+	 * Do not send CMD_ENABLE_STALL to orphaned-fault banks
+	 * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE): the command is ignored
+	 * by such a bank but its presence on the shared bus delays the other
+	 * banks from reaching STALL_ACTIVE within the poll timeout.
+	 */
+	for (i = 0; i < iommu->num_mmu; i++) {
+		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
+
+		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
+		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
+		    (status & RK_MMU_STATUS_IDLE))
+			continue;
+		writel(RK_MMU_CMD_ENABLE_STALL, iommu->bases[i] + RK_MMU_COMMAND);
+	}
 
 	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
 				 val, RK_MMU_POLL_PERIOD_US,
-- 
2.43.0

Re: [RFC PATCH 5/9] iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Missing reset_control_put() causes a resource leak on PM domain removal.
- [Medium] rk_iommu_disable_stall() unconditionally times out if all MMU banks are in the orphaned-fault state.
- [Low] Relaxing the global 'required' list breaks validation for RK3588 NPU.

Pre-existing issues:
- [High] On multi-bank IOMMUs (num_mmu > 1), rk_iommu_disable_stall and rk_iommu_disable_paging return success prematurely when disabling, potentially leaving banks active.
- [High] The `rk_iommu_probe` function contains a critical flaw in how it populates and handles errors in the `iommu->bases` array, leading to a guaranteed kernel panic on malformed device trees and a sparse array bug resulting in `ERR_PTR` dereferences.
--

--- Patch [5]: [RFC PATCH 5/9] iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch ---
commit e1850eec170c4af87cf79a1f7f05257d02a8ccf8
Author: Jiaxing Hu <gahing@gahingwoo.com>

iommu/rockchip: skip orphaned-fault banks in CMD_ENABLE_STALL dispatch

Sending CMD_ENABLE_STALL to a bank that carries an orphaned firmware
fault (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE) causes the command to
be silently dropped by that bank.  The dropped command also interferes
with the other banks' stall transition, delaying them past the poll
timeout even though they would otherwise stall within a few
microseconds.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index ad462cee6bd89..7dbcc978ede79 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -436,7 +436,21 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
>  	if (!rk_iommu_is_paging_enabled(iommu))
>  		return 0;
>  
> -	rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);
> +	/*
> +	 * Do not send CMD_ENABLE_STALL to orphaned-fault banks
> +	 * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE): the command is ignored
> +	 * by such a bank but its presence on the shared bus delays the other
> +	 * banks from reaching STALL_ACTIVE within the poll timeout.
> +	 */
> +	for (i = 0; i < iommu->num_mmu; i++) {
> +		u32 status = rk_iommu_read(iommu->bases[i], RK_MMU_STATUS);
> +
> +		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
> +		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
> +		    (status & RK_MMU_STATUS_IDLE))
> +			continue;
> +		writel(RK_MMU_CMD_ENABLE_STALL, iommu->bases[i] + RK_MMU_COMMAND);
> +	}
>  
>  	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
>  				 val, RK_MMU_POLL_PERIOD_US,

[Severity: Medium]
Does rk_iommu_disable_stall() unconditionally timeout if all MMU banks are in
the orphaned-fault state?

Looking at rk_iommu_is_stall_active(), if all banks are orphaned, the loop
skips evaluating them:

drivers/iommu/rockchip-iommu.c:rk_iommu_is_stall_active() {
...
	for (i = 0; i < iommu->num_mmu; i++) {
...
		if ((status & RK_MMU_STATUS_PAGE_FAULT_ACTIVE) &&
		    !(status & RK_MMU_STATUS_STALL_ACTIVE) &&
		    (status & RK_MMU_STATUS_IDLE))
			continue;

		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
	}

	return active;
}

Since 'active' starts as true, it remains true and does not reflect their
bypassed state, meaning a fully-skipped IOMMU reports as stalled.

Then in rk_iommu_disable_stall():

drivers/iommu/rockchip-iommu.c:rk_iommu_disable_stall() {
...
	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
				 !val, RK_MMU_POLL_PERIOD_US,
				 RK_MMU_POLL_TIMEOUT_US);
...
}

The polling loop waits for !rk_iommu_is_stall_active(), which will
unconditionally exhaust its timeout if all banks are orphaned and continually
report as stalled.

[Severity: High]
This is a pre-existing issue, but do rk_iommu_disable_stall() and
rk_iommu_disable_paging() return success prematurely when disabling on
multi-bank IOMMUs?

Because of the bitwise AND reduction in rk_iommu_is_stall_active():

drivers/iommu/rockchip-iommu.c:rk_iommu_is_stall_active() {
...
	for (i = 0; i < iommu->num_mmu; i++) {
...
		active &= !!(status & RK_MMU_STATUS_STALL_ACTIVE);
	}

	return active;
}

The function returns false (indicating no stall) if ANY single bank has
finished its stall transition, ignoring banks that are still active.

Then in rk_iommu_disable_stall():

drivers/iommu/rockchip-iommu.c:rk_iommu_disable_stall() {
...
	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
				 !val, RK_MMU_POLL_PERIOD_US,
...
}

The poll condition !val incorrectly succeeds as soon as any single bank
unstalls, potentially leaving other hardware banks active.

[Severity: High]
This is a pre-existing issue, but is there a risk of a kernel panic in
rk_iommu_probe() due to how the iommu->bases array is handled?

If the device tree has 0 resources, or if devm_ioremap_resource() fails for an
early resource but succeeds for a later one in a multi-MMU configuration, the
array is populated sparsely:

drivers/iommu/rockchip-iommu.c:rk_iommu_probe() {
...
	for (i = 0; i < num_res; i++) {
		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
		if (!res)
			continue;
		iommu->bases[i] = devm_ioremap_resource(&pdev->dev, res);
		if (IS_ERR(iommu->bases[i]))
			continue;
		iommu->num_mmu++;
	}
	if (iommu->num_mmu == 0)
		return PTR_ERR(iommu->bases[0]);
...
}

If zero resources are provided, devm_kcalloc() returns ZERO_SIZE_PTR, which is
then invalidly dereferenced at index 0. Also, if an early resource fails
mapping, the array is populated sparsely using loop index 'i' instead of
'num_mmu', but runtime logic expects contiguous valid pointers, leading to a
dereference of ERR_PTR during register reads.

[Severity: Medium]
Does rockchip_pm_remove_one_domain() miss freeing reset controls on
PM domain removal?

When the PM domain is removed (e.g., during driver unbind) or if probing
fails for a later power domain:

drivers/pmdomain/rockchip/pm-domains.c:rockchip_pm_remove_one_domain() {
...
	clk_bulk_unprepare(pd->num_clks, pd->clks);
	clk_bulk_put(pd->num_clks, pd->clks);
...
}

This teardown function misses the reset_control_put() call to free the
reset controls retrieved during addition, causing a resource leak.

[Severity: Low]
Does relaxing the global 'required' list break validation for RK3588 NPU?

Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml:
...
  - reset-names
  - npu-supply

additionalProperties: false
...

The 'sram-supply' property was removed from the global 'required' list here.
Will DT schema validation fail to flag missing 'sram-supply' for RK3588
devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=5

[RFC PATCH 6/9] iommu/rockchip: take all DT clocks

[Jiaxing Hu]

rk_iommu only enabled a fixed {aclk,iface} pair. On the RK3576 NPU the
MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
dropped until those clocks run too (reads work, writes need more clocks).
Take every clock described in the DT for the IOMMU instead of a fixed
pair, so a platform can list the full set it needs.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ce586919b..49c841095 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -93,11 +93,6 @@ struct rk_iommu_domain {
 	struct iommu_domain domain;
 };
 
-/* list of clocks required by IOMMU */
-static const char * const rk_iommu_clocks[] = {
-	"aclk", "iface",
-};
-
 struct rk_iommu_ops {
 	phys_addr_t (*pt_address)(u32 dte);
 	u32 (*mk_dtentries)(dma_addr_t pt_dma);
@@ -1274,25 +1269,20 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	iommu->reset_disabled = device_property_read_bool(dev,
 					"rockchip,disable-mmu-reset");
 
-	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
-	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
-				     sizeof(*iommu->clocks), GFP_KERNEL);
-	if (!iommu->clocks)
-		return -ENOMEM;
-
-	for (i = 0; i < iommu->num_clocks; ++i)
-		iommu->clocks[i].id = rk_iommu_clocks[i];
-
 	/*
-	 * iommu clocks should be present for all new devices and devicetrees
-	 * but there are older devicetrees without clocks out in the wild.
-	 * So clocks as optional for the time being.
+	 * Take every clock the devicetree provides.  Most IOMMU instances
+	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
+	 * behind additional gates (CBUF/DSU) whose clocks must be running
+	 * for register writes to land.  Clocks stay optional because there
+	 * are older devicetrees without clocks out in the wild.
 	 */
-	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
-	if (err == -ENOENT)
+	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
+	if (err == -ENOENT || err == 0)
 		iommu->num_clocks = 0;
-	else if (err)
+	else if (err < 0)
 		return err;
+	else
+		iommu->num_clocks = err;
 
 	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
 	if (err)
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 6/9] iommu/rockchip: take all DT clocks

[Jiaxing Hu]

rk_iommu only enabled a fixed {aclk,iface} pair. On the RK3576 NPU the
MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
dropped until those clocks run too (reads work, writes need more clocks).
Take every clock described in the DT for the IOMMU instead of a fixed
pair, so a platform can list the full set it needs.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ce586919b..49c841095 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -93,11 +93,6 @@ struct rk_iommu_domain {
 	struct iommu_domain domain;
 };
 
-/* list of clocks required by IOMMU */
-static const char * const rk_iommu_clocks[] = {
-	"aclk", "iface",
-};
-
 struct rk_iommu_ops {
 	phys_addr_t (*pt_address)(u32 dte);
 	u32 (*mk_dtentries)(dma_addr_t pt_dma);
@@ -1274,25 +1269,20 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	iommu->reset_disabled = device_property_read_bool(dev,
 					"rockchip,disable-mmu-reset");
 
-	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
-	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
-				     sizeof(*iommu->clocks), GFP_KERNEL);
-	if (!iommu->clocks)
-		return -ENOMEM;
-
-	for (i = 0; i < iommu->num_clocks; ++i)
-		iommu->clocks[i].id = rk_iommu_clocks[i];
-
 	/*
-	 * iommu clocks should be present for all new devices and devicetrees
-	 * but there are older devicetrees without clocks out in the wild.
-	 * So clocks as optional for the time being.
+	 * Take every clock the devicetree provides.  Most IOMMU instances
+	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
+	 * behind additional gates (CBUF/DSU) whose clocks must be running
+	 * for register writes to land.  Clocks stay optional because there
+	 * are older devicetrees without clocks out in the wild.
 	 */
-	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
-	if (err == -ENOENT)
+	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
+	if (err == -ENOENT || err == 0)
 		iommu->num_clocks = 0;
-	else if (err)
+	else if (err < 0)
 		return err;
+	else
+		iommu->num_clocks = err;
 
 	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
 	if (err)
-- 
2.43.0

Re: [RFC PATCH 6/9] iommu/rockchip: take all DT clocks

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> rk_iommu only enabled a fixed {aclk,iface} pair. On the RK3576 NPU the
> MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
> dropped until those clocks run too (reads work, writes need more clocks).
> Take every clock described in the DT for the IOMMU instead of a fixed
> pair, so a platform can list the full set it needs.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/iommu/rockchip-iommu.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index ce586919b..49c841095 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -93,11 +93,6 @@ struct rk_iommu_domain {
>  	struct iommu_domain domain;
>  };
>  
> -/* list of clocks required by IOMMU */
> -static const char * const rk_iommu_clocks[] = {
> -	"aclk", "iface",
> -};
> -
>  struct rk_iommu_ops {
>  	phys_addr_t (*pt_address)(u32 dte);
>  	u32 (*mk_dtentries)(dma_addr_t pt_dma);
> @@ -1274,25 +1269,20 @@ static int rk_iommu_probe(struct platform_device *pdev)
>  	iommu->reset_disabled = device_property_read_bool(dev,
>  					"rockchip,disable-mmu-reset");
>  
> -	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
> -	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
> -				     sizeof(*iommu->clocks), GFP_KERNEL);
> -	if (!iommu->clocks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < iommu->num_clocks; ++i)
> -		iommu->clocks[i].id = rk_iommu_clocks[i];
> -
>  	/*
> -	 * iommu clocks should be present for all new devices and devicetrees
> -	 * but there are older devicetrees without clocks out in the wild.
> -	 * So clocks as optional for the time being.
> +	 * Take every clock the devicetree provides.  Most IOMMU instances
> +	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
> +	 * behind additional gates (CBUF/DSU) whose clocks must be running
> +	 * for register writes to land.  Clocks stay optional because there
> +	 * are older devicetrees without clocks out in the wild.
>  	 */

It does not seem necessary to modify the comments, as there are indeed
many MMUs with multiple clocks :)

> -	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
> -	if (err == -ENOENT)
> +	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
> +	if (err == -ENOENT || err == 0)
>  		iommu->num_clocks = 0;

err == 0 is redundant.

> -	else if (err)
> +	else if (err < 0)
>  		return err;
> +	else
> +		iommu->num_clocks = err;
>  
>  	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
>  	if (err)

-- 
Best, 
Chaoyi

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 6/9] iommu/rockchip: take all DT clocks

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> rk_iommu only enabled a fixed {aclk,iface} pair. On the RK3576 NPU the
> MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
> dropped until those clocks run too (reads work, writes need more clocks).
> Take every clock described in the DT for the IOMMU instead of a fixed
> pair, so a platform can list the full set it needs.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/iommu/rockchip-iommu.c | 30 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index ce586919b..49c841095 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -93,11 +93,6 @@ struct rk_iommu_domain {
>  	struct iommu_domain domain;
>  };
>  
> -/* list of clocks required by IOMMU */
> -static const char * const rk_iommu_clocks[] = {
> -	"aclk", "iface",
> -};
> -
>  struct rk_iommu_ops {
>  	phys_addr_t (*pt_address)(u32 dte);
>  	u32 (*mk_dtentries)(dma_addr_t pt_dma);
> @@ -1274,25 +1269,20 @@ static int rk_iommu_probe(struct platform_device *pdev)
>  	iommu->reset_disabled = device_property_read_bool(dev,
>  					"rockchip,disable-mmu-reset");
>  
> -	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
> -	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
> -				     sizeof(*iommu->clocks), GFP_KERNEL);
> -	if (!iommu->clocks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < iommu->num_clocks; ++i)
> -		iommu->clocks[i].id = rk_iommu_clocks[i];
> -
>  	/*
> -	 * iommu clocks should be present for all new devices and devicetrees
> -	 * but there are older devicetrees without clocks out in the wild.
> -	 * So clocks as optional for the time being.
> +	 * Take every clock the devicetree provides.  Most IOMMU instances
> +	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
> +	 * behind additional gates (CBUF/DSU) whose clocks must be running
> +	 * for register writes to land.  Clocks stay optional because there
> +	 * are older devicetrees without clocks out in the wild.
>  	 */

It does not seem necessary to modify the comments, as there are indeed
many MMUs with multiple clocks :)

> -	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
> -	if (err == -ENOENT)
> +	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
> +	if (err == -ENOENT || err == 0)
>  		iommu->num_clocks = 0;

err == 0 is redundant.

> -	else if (err)
> +	else if (err < 0)
>  		return err;
> +	else
> +		iommu->num_clocks = err;
>  
>  	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
>  	if (err)

-- 
Best, 
Chaoyi

[RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Add RK3576 support to the rocket DRM accelerator driver (used with the
Mesa Teflon TFLite delegate).

  - match rockchip,rk3576-rknn-core; iterate its nodes at probe
  - add named clock IDs (aclk/hclk/npu/pclk) and the CBUF clock domain
    (ACLK/HCLK_RKNN_CBUF); the CNA fills the CBUF and CORE reads from it,
    so the compute path stalls if those clocks are not held for a job
  - guard rocket_job_timedout() MMIO behind pm_runtime_active()
  - hrtimer completion poll: OP_EN never clears on RK3576 (unlike
    RK3588), so poll INTERRUPT_RAW_STATUS PC_DONE bits instead
  - map the DPU (0x4000) and DPU_RDMA (0x5000) blocks and pre-arm them
  - attach both NPU power domains (PD_NPU0 + PD_NPU1): the CBUF->CMAC
    read path is only fully powered with NPU1 up

Tested on a Radxa ROCK 4D: the NPU probes, powers on, brings up its
IOMMUs and runs submitted jobs to completion. Full multi-layer inference
is not yet correct on this SoC (only the first operation per power
session produces valid output); see the cover letter.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c   |  43 +++++++-
 drivers/accel/rocket/rocket_core.h   |  22 +++-
 drivers/accel/rocket/rocket_device.c |   4 +
 drivers/accel/rocket/rocket_drv.c    |   1 +
 drivers/accel/rocket/rocket_job.c    | 154 +++++++++++++++++++++------
 5 files changed, 187 insertions(+), 37 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b3b2fa9ba..7b0c48e4d 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>
 
@@ -22,12 +23,23 @@ int rocket_core_init(struct rocket_core *core)
 	int err = 0;
 
 	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),
 						    core->resets);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
 
+	core->clks[0].id = "aclk";
+	core->clks[1].id = "hclk";
+	core->clks[2].id = "npu";
+	core->clks[3].id = "pclk";
+	/*
+	 * RK3576: the CBUF (convolution buffer) has its own clock domain. The CNA
+	 * fills the CBUF and CORE reads from it; without these the compute path
+	 * stalls after loading one slice (RDMA, which bypasses the CBUF, still
+	 * runs). The vendor keeps all NPU clocks on whenever powered.
+	 */
+	core->clks[4].id = "aclk_cbuf";
+	core->clks[5].id = "hclk_cbuf";
 	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
@@ -50,6 +62,18 @@ int rocket_core_init(struct rocket_core *core)
 		return PTR_ERR(core->core_iomem);
 	}
 
+	core->dpu_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu");
+	if (IS_ERR(core->dpu_iomem)) {
+		dev_warn(dev, "no DPU registers; DPU S_POINTER won't be pre-armed\n");
+		core->dpu_iomem = NULL;
+	}
+
+	core->dpu_rdma_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu_rdma");
+	if (IS_ERR(core->dpu_rdma_iomem)) {
+		dev_warn(dev, "no DPU_RDMA registers; DPU_RDMA S_POINTER won't be pre-armed\n");
+		core->dpu_rdma_iomem = NULL;
+	}
+
 	dma_set_max_seg_size(dev, UINT_MAX);
 
 	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
@@ -65,6 +89,23 @@ int rocket_core_init(struct rocket_core *core)
 		return err;
 	}
 
+	/*
+	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1). The vendor
+	 * powers BOTH from its single NPU node even when computing on one core --
+	 * the CBUF->CMAC read path only works fully with NPU1 powered. The board DT
+	 * lists both power-domains on rknn_core_0; a multi-PD device skips the
+	 * driver-core single-PD auto-attach, so attach the list explicitly. With
+	 * one PD in DT this is a no-op (returns 1) and behaves as before.
+	 */
+	{
+		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..e959b26dd 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,6 +28,16 @@
 #define rocket_core_writel(core, reg, value) \
 	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
 
+#define rocket_dpu_readl(core, reg) \
+	readl((core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
+#define rocket_dpu_writel(core, reg, value) \
+	writel(value, (core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
+
+#define rocket_dpu_rdma_readl(core, reg) \
+	readl((core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
+#define rocket_dpu_rdma_writel(core, reg, value) \
+	writel(value, (core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
+
 struct rocket_core {
 	struct device *dev;
 	struct rocket_device *rdev;
@@ -36,8 +47,10 @@ struct rocket_core {
 	void __iomem *pc_iomem;
 	void __iomem *cna_iomem;
 	void __iomem *core_iomem;
-	struct clk_bulk_data clks[4];
-	struct reset_control_bulk_data resets[2];
+	void __iomem *dpu_iomem;
+	void __iomem *dpu_rdma_iomem;
+	struct clk_bulk_data clks[6];
+	struct reset_control_bulk_data resets[1];
 
 	struct iommu_group *iommu_group;
 
@@ -52,6 +65,11 @@ struct rocket_core {
 		atomic_t pending;
 	} reset;
 
+	/* RK3576 has no completion IRQ; poll for PC_DONE via hrtimer. */
+	struct hrtimer poll_timer;
+	struct work_struct poll_work;
+	atomic_t poll_active;
+
 	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..8f6a02d50 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -215,6 +215,7 @@ static void rocket_remove(struct platform_device *pdev)
 
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "rockchip,rk3588-rknn-core" },
+	{ .compatible = "rockchip,rk3576-rknn-core" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 2f1861f96..94461bf71 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/iommu.h>
 #include <linux/platform_device.h>
@@ -20,6 +21,16 @@
 
 #define JOB_TIMEOUT_MS 500
 
+/*
+ * RK3576: INTERRUPT_MASK bits 28-29 are read-only (hardware rejects the write),
+ * so the PC_DONE completion signal cannot be routed to the GIC via the normal
+ * interrupt-mask path.  We poll OPERATION_ENABLE every RK3576_POLL_INTERVAL_NS
+ * instead of waiting for a completion IRQ.
+ */
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_0  0x10000000u
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_1  0x20000000u
+#define RK3576_POLL_INTERVAL_NS  1000000LL  /* 1 ms */
+
 static struct rocket_job *
 to_rocket_job(struct drm_sched_job *sched_job)
 {
@@ -137,8 +148,21 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 	rocket_pc_writel(core, REGISTER_AMOUNTS,
 			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
 
-	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
-	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
+	/* Enable DMA-error interrupts; PC_DONE (bits 28-29) is polled, see above. */
+	rocket_pc_writel(core, INTERRUPT_MASK,
+			 PC_INTERRUPT_MASK_DPU_0    | PC_INTERRUPT_MASK_DPU_1    |
+			 PC_INTERRUPT_MASK_CORE_0   | PC_INTERRUPT_MASK_CORE_1   |
+			 PC_INTERRUPT_MASK_PPU_0    | PC_INTERRUPT_MASK_PPU_1    |
+			 PC_INTERRUPT_MASK_CNA_CSC_0 | PC_INTERRUPT_MASK_CNA_CSC_1 |
+			 PC_INTERRUPT_MASK_DMA_READ_ERROR |
+			 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);
+	rocket_pc_writel(core, INTERRUPT_CLEAR,
+			 PC_INTERRUPT_CLEAR_DPU_0    | PC_INTERRUPT_CLEAR_DPU_1    |
+			 PC_INTERRUPT_CLEAR_CORE_0   | PC_INTERRUPT_CLEAR_CORE_1   |
+			 PC_INTERRUPT_CLEAR_PPU_0    | PC_INTERRUPT_CLEAR_PPU_1    |
+			 PC_INTERRUPT_CLEAR_CNA_CSC_0 | PC_INTERRUPT_CLEAR_CNA_CSC_1 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
 
 	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
 					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
@@ -149,7 +173,9 @@ 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));
 
-	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
+	atomic_set(&core->poll_active, 1);
+	hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
+		      HRTIMER_MODE_REL);
 }
 
 static int rocket_acquire_object_fences(struct drm_gem_object **bos,
@@ -326,56 +352,99 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
 	return fence;
 }
 
+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);
+
+	if (!atomic_read(&core->poll_active))
+		return HRTIMER_NORESTART;
+
+	/*
+	 * On RK3576, OPERATION_ENABLE is not cleared by hardware on completion;
+	 * check INTERRUPT_RAW_STATUS bits 28-29 (PC_DONE_0/1) instead.
+	 */
+	if (rocket_pc_readl(core, OPERATION_ENABLE) == 0 ||
+	    (rocket_pc_readl(core, INTERRUPT_RAW_STATUS) &
+	     (PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+	      PC_INTERRUPT_MASK_RK3576_PC_DONE_1))) {
+		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;
+}
+
+static void rocket_poll_work_fn(struct work_struct *work)
+{
+	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
+
+	rocket_job_handle_irq(core);
+}
+
 static void rocket_job_handle_irq(struct rocket_core *core)
 {
+	struct rocket_job *job;
+
+	/* Stop the completion poll — we're handling it now. */
+	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_pc_writel(core, INTERRUPT_CLEAR,
+			 0x1ffff |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
+
+	scoped_guard(mutex, &core->job_lock) {
+		job = core->in_flight_job;
+		if (!job)
+			return;
+
+		if (job->next_task_idx < job->task_count) {
+			rocket_job_hw_submit(core, job);
+			return;
 		}
+
+		iommu_detach_group(job->domain->domain, core->iommu_group);
+		dma_fence_signal(job->done_fence);
+		pm_runtime_put_autosuspend(core->dev);
+		core->in_flight_job = NULL;
+	}
 }
 
 static void
 rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
 {
+	struct rocket_job *job;
+
 	if (!atomic_read(&core->reset.pending))
 		return;
 
+	atomic_set(&core->poll_active, 0);
+	hrtimer_cancel(&core->poll_timer);
+	cancel_work(&core->poll_work);
+
 	drm_sched_stop(&core->sched, bad);
 
-	/*
-	 * Remaining interrupts have been handled, but we might still have
-	 * stuck jobs. Let's make sure the PM counters stay balanced by
-	 * manually calling pm_runtime_put_noidle().
-	 */
 	scoped_guard(mutex, &core->job_lock) {
-		if (core->in_flight_job)
+		job = core->in_flight_job;
+		if (job) {
 			pm_runtime_put_noidle(core->dev);
-
-		iommu_detach_group(NULL, core->iommu_group);
-
-		core->in_flight_job = NULL;
+			iommu_detach_group(job->domain->domain, core->iommu_group);
+			core->in_flight_job = NULL;
+		}
 	}
 
-	/* Proceed with reset now. */
 	rocket_core_reset(core);
 
-	/* NPU has been reset, we can clear the reset pending bit. */
 	atomic_set(&core->reset.pending, 0);
 
-	/* Restart the scheduler */
 	drm_sched_start(&core->sched, 0);
 }
 
@@ -385,7 +454,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
 	struct rocket_device *rdev = job->rdev;
 	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
 
-	dev_err(core->dev, "NPU job timed out");
+	if (pm_runtime_active(core->dev))
+		dev_err(core->dev,
+			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
+			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
+			rocket_pc_readl(core, INTERRUPT_MASK),
+			rocket_pc_readl(core, OPERATION_ENABLE));
+	else
+		dev_err(core->dev, "NPU job timed out (device not active)\n");
 
 	atomic_set(&core->reset.pending, 1);
 	rocket_reset(core, sched_job);
@@ -420,14 +496,16 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
 {
 	struct rocket_core *core = data;
 	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
+	/* Only bits 0-13 (DMA errors) can raise this IRQ; PC_DONE is polled. */
+	u32 active = raw_status & 0x3fff;
+
+	if (!active)
+		return IRQ_NONE;
 
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_READ_ERROR);
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_WRITE_ERROR);
 
-	if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
-	      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
-		return IRQ_NONE;
-
+	rocket_pc_writel(core, INTERRUPT_CLEAR, active);
 	rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
 
 	return IRQ_WAKE_THREAD;
@@ -445,6 +523,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);
 
@@ -486,6 +568,10 @@ int rocket_job_init(struct rocket_core *core)
 
 void rocket_job_fini(struct rocket_core *core)
 {
+	atomic_set(&core->poll_active, 0);
+	hrtimer_cancel(&core->poll_timer);
+	cancel_work_sync(&core->poll_work);
+
 	drm_sched_fini(&core->sched);
 
 	cancel_work_sync(&core->reset.work);
-- 
2.43.0

[RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Add RK3576 support to the rocket DRM accelerator driver (used with the
Mesa Teflon TFLite delegate).

  - match rockchip,rk3576-rknn-core; iterate its nodes at probe
  - add named clock IDs (aclk/hclk/npu/pclk) and the CBUF clock domain
    (ACLK/HCLK_RKNN_CBUF); the CNA fills the CBUF and CORE reads from it,
    so the compute path stalls if those clocks are not held for a job
  - guard rocket_job_timedout() MMIO behind pm_runtime_active()
  - hrtimer completion poll: OP_EN never clears on RK3576 (unlike
    RK3588), so poll INTERRUPT_RAW_STATUS PC_DONE bits instead
  - map the DPU (0x4000) and DPU_RDMA (0x5000) blocks and pre-arm them
  - attach both NPU power domains (PD_NPU0 + PD_NPU1): the CBUF->CMAC
    read path is only fully powered with NPU1 up

Tested on a Radxa ROCK 4D: the NPU probes, powers on, brings up its
IOMMUs and runs submitted jobs to completion. Full multi-layer inference
is not yet correct on this SoC (only the first operation per power
session produces valid output); see the cover letter.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c   |  43 +++++++-
 drivers/accel/rocket/rocket_core.h   |  22 +++-
 drivers/accel/rocket/rocket_device.c |   4 +
 drivers/accel/rocket/rocket_drv.c    |   1 +
 drivers/accel/rocket/rocket_job.c    | 154 +++++++++++++++++++++------
 5 files changed, 187 insertions(+), 37 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b3b2fa9ba..7b0c48e4d 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>
 
@@ -22,12 +23,23 @@ int rocket_core_init(struct rocket_core *core)
 	int err = 0;
 
 	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),
 						    core->resets);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
 
+	core->clks[0].id = "aclk";
+	core->clks[1].id = "hclk";
+	core->clks[2].id = "npu";
+	core->clks[3].id = "pclk";
+	/*
+	 * RK3576: the CBUF (convolution buffer) has its own clock domain. The CNA
+	 * fills the CBUF and CORE reads from it; without these the compute path
+	 * stalls after loading one slice (RDMA, which bypasses the CBUF, still
+	 * runs). The vendor keeps all NPU clocks on whenever powered.
+	 */
+	core->clks[4].id = "aclk_cbuf";
+	core->clks[5].id = "hclk_cbuf";
 	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
@@ -50,6 +62,18 @@ int rocket_core_init(struct rocket_core *core)
 		return PTR_ERR(core->core_iomem);
 	}
 
+	core->dpu_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu");
+	if (IS_ERR(core->dpu_iomem)) {
+		dev_warn(dev, "no DPU registers; DPU S_POINTER won't be pre-armed\n");
+		core->dpu_iomem = NULL;
+	}
+
+	core->dpu_rdma_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu_rdma");
+	if (IS_ERR(core->dpu_rdma_iomem)) {
+		dev_warn(dev, "no DPU_RDMA registers; DPU_RDMA S_POINTER won't be pre-armed\n");
+		core->dpu_rdma_iomem = NULL;
+	}
+
 	dma_set_max_seg_size(dev, UINT_MAX);
 
 	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
@@ -65,6 +89,23 @@ int rocket_core_init(struct rocket_core *core)
 		return err;
 	}
 
+	/*
+	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1). The vendor
+	 * powers BOTH from its single NPU node even when computing on one core --
+	 * the CBUF->CMAC read path only works fully with NPU1 powered. The board DT
+	 * lists both power-domains on rknn_core_0; a multi-PD device skips the
+	 * driver-core single-PD auto-attach, so attach the list explicitly. With
+	 * one PD in DT this is a no-op (returns 1) and behaves as before.
+	 */
+	{
+		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..e959b26dd 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,6 +28,16 @@
 #define rocket_core_writel(core, reg, value) \
 	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
 
+#define rocket_dpu_readl(core, reg) \
+	readl((core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
+#define rocket_dpu_writel(core, reg, value) \
+	writel(value, (core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
+
+#define rocket_dpu_rdma_readl(core, reg) \
+	readl((core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
+#define rocket_dpu_rdma_writel(core, reg, value) \
+	writel(value, (core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
+
 struct rocket_core {
 	struct device *dev;
 	struct rocket_device *rdev;
@@ -36,8 +47,10 @@ struct rocket_core {
 	void __iomem *pc_iomem;
 	void __iomem *cna_iomem;
 	void __iomem *core_iomem;
-	struct clk_bulk_data clks[4];
-	struct reset_control_bulk_data resets[2];
+	void __iomem *dpu_iomem;
+	void __iomem *dpu_rdma_iomem;
+	struct clk_bulk_data clks[6];
+	struct reset_control_bulk_data resets[1];
 
 	struct iommu_group *iommu_group;
 
@@ -52,6 +65,11 @@ struct rocket_core {
 		atomic_t pending;
 	} reset;
 
+	/* RK3576 has no completion IRQ; poll for PC_DONE via hrtimer. */
+	struct hrtimer poll_timer;
+	struct work_struct poll_work;
+	atomic_t poll_active;
+
 	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..8f6a02d50 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -215,6 +215,7 @@ static void rocket_remove(struct platform_device *pdev)
 
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "rockchip,rk3588-rknn-core" },
+	{ .compatible = "rockchip,rk3576-rknn-core" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 2f1861f96..94461bf71 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/iommu.h>
 #include <linux/platform_device.h>
@@ -20,6 +21,16 @@
 
 #define JOB_TIMEOUT_MS 500
 
+/*
+ * RK3576: INTERRUPT_MASK bits 28-29 are read-only (hardware rejects the write),
+ * so the PC_DONE completion signal cannot be routed to the GIC via the normal
+ * interrupt-mask path.  We poll OPERATION_ENABLE every RK3576_POLL_INTERVAL_NS
+ * instead of waiting for a completion IRQ.
+ */
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_0  0x10000000u
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_1  0x20000000u
+#define RK3576_POLL_INTERVAL_NS  1000000LL  /* 1 ms */
+
 static struct rocket_job *
 to_rocket_job(struct drm_sched_job *sched_job)
 {
@@ -137,8 +148,21 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 	rocket_pc_writel(core, REGISTER_AMOUNTS,
 			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
 
-	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
-	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
+	/* Enable DMA-error interrupts; PC_DONE (bits 28-29) is polled, see above. */
+	rocket_pc_writel(core, INTERRUPT_MASK,
+			 PC_INTERRUPT_MASK_DPU_0    | PC_INTERRUPT_MASK_DPU_1    |
+			 PC_INTERRUPT_MASK_CORE_0   | PC_INTERRUPT_MASK_CORE_1   |
+			 PC_INTERRUPT_MASK_PPU_0    | PC_INTERRUPT_MASK_PPU_1    |
+			 PC_INTERRUPT_MASK_CNA_CSC_0 | PC_INTERRUPT_MASK_CNA_CSC_1 |
+			 PC_INTERRUPT_MASK_DMA_READ_ERROR |
+			 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);
+	rocket_pc_writel(core, INTERRUPT_CLEAR,
+			 PC_INTERRUPT_CLEAR_DPU_0    | PC_INTERRUPT_CLEAR_DPU_1    |
+			 PC_INTERRUPT_CLEAR_CORE_0   | PC_INTERRUPT_CLEAR_CORE_1   |
+			 PC_INTERRUPT_CLEAR_PPU_0    | PC_INTERRUPT_CLEAR_PPU_1    |
+			 PC_INTERRUPT_CLEAR_CNA_CSC_0 | PC_INTERRUPT_CLEAR_CNA_CSC_1 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
 
 	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
 					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
@@ -149,7 +173,9 @@ 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));
 
-	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
+	atomic_set(&core->poll_active, 1);
+	hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
+		      HRTIMER_MODE_REL);
 }
 
 static int rocket_acquire_object_fences(struct drm_gem_object **bos,
@@ -326,56 +352,99 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
 	return fence;
 }
 
+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);
+
+	if (!atomic_read(&core->poll_active))
+		return HRTIMER_NORESTART;
+
+	/*
+	 * On RK3576, OPERATION_ENABLE is not cleared by hardware on completion;
+	 * check INTERRUPT_RAW_STATUS bits 28-29 (PC_DONE_0/1) instead.
+	 */
+	if (rocket_pc_readl(core, OPERATION_ENABLE) == 0 ||
+	    (rocket_pc_readl(core, INTERRUPT_RAW_STATUS) &
+	     (PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+	      PC_INTERRUPT_MASK_RK3576_PC_DONE_1))) {
+		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;
+}
+
+static void rocket_poll_work_fn(struct work_struct *work)
+{
+	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
+
+	rocket_job_handle_irq(core);
+}
+
 static void rocket_job_handle_irq(struct rocket_core *core)
 {
+	struct rocket_job *job;
+
+	/* Stop the completion poll — we're handling it now. */
+	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_pc_writel(core, INTERRUPT_CLEAR,
+			 0x1ffff |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
+
+	scoped_guard(mutex, &core->job_lock) {
+		job = core->in_flight_job;
+		if (!job)
+			return;
+
+		if (job->next_task_idx < job->task_count) {
+			rocket_job_hw_submit(core, job);
+			return;
 		}
+
+		iommu_detach_group(job->domain->domain, core->iommu_group);
+		dma_fence_signal(job->done_fence);
+		pm_runtime_put_autosuspend(core->dev);
+		core->in_flight_job = NULL;
+	}
 }
 
 static void
 rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
 {
+	struct rocket_job *job;
+
 	if (!atomic_read(&core->reset.pending))
 		return;
 
+	atomic_set(&core->poll_active, 0);
+	hrtimer_cancel(&core->poll_timer);
+	cancel_work(&core->poll_work);
+
 	drm_sched_stop(&core->sched, bad);
 
-	/*
-	 * Remaining interrupts have been handled, but we might still have
-	 * stuck jobs. Let's make sure the PM counters stay balanced by
-	 * manually calling pm_runtime_put_noidle().
-	 */
 	scoped_guard(mutex, &core->job_lock) {
-		if (core->in_flight_job)
+		job = core->in_flight_job;
+		if (job) {
 			pm_runtime_put_noidle(core->dev);
-
-		iommu_detach_group(NULL, core->iommu_group);
-
-		core->in_flight_job = NULL;
+			iommu_detach_group(job->domain->domain, core->iommu_group);
+			core->in_flight_job = NULL;
+		}
 	}
 
-	/* Proceed with reset now. */
 	rocket_core_reset(core);
 
-	/* NPU has been reset, we can clear the reset pending bit. */
 	atomic_set(&core->reset.pending, 0);
 
-	/* Restart the scheduler */
 	drm_sched_start(&core->sched, 0);
 }
 
@@ -385,7 +454,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
 	struct rocket_device *rdev = job->rdev;
 	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
 
-	dev_err(core->dev, "NPU job timed out");
+	if (pm_runtime_active(core->dev))
+		dev_err(core->dev,
+			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
+			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
+			rocket_pc_readl(core, INTERRUPT_MASK),
+			rocket_pc_readl(core, OPERATION_ENABLE));
+	else
+		dev_err(core->dev, "NPU job timed out (device not active)\n");
 
 	atomic_set(&core->reset.pending, 1);
 	rocket_reset(core, sched_job);
@@ -420,14 +496,16 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
 {
 	struct rocket_core *core = data;
 	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
+	/* Only bits 0-13 (DMA errors) can raise this IRQ; PC_DONE is polled. */
+	u32 active = raw_status & 0x3fff;
+
+	if (!active)
+		return IRQ_NONE;
 
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_READ_ERROR);
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_WRITE_ERROR);
 
-	if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
-	      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
-		return IRQ_NONE;
-
+	rocket_pc_writel(core, INTERRUPT_CLEAR, active);
 	rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
 
 	return IRQ_WAKE_THREAD;
@@ -445,6 +523,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);
 
@@ -486,6 +568,10 @@ int rocket_job_init(struct rocket_core *core)
 
 void rocket_job_fini(struct rocket_core *core)
 {
+	atomic_set(&core->poll_active, 0);
+	hrtimer_cancel(&core->poll_timer);
+	cancel_work_sync(&core->poll_work);
+
 	drm_sched_fini(&core->sched);
 
 	cancel_work_sync(&core->reset.work);
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [High] Unconditional clock and reset requests break driver initialization on RK3588.
- [High] Unhandled completion interrupts cause an interrupt storm on RK3588.
- [High] Using cancel_work() in rocket_reset() causes a race condition and hardware state corruption.
- [High] TOCTOU race in rocket_job_timedout() allows MMIO reads on a suspended device.
- [Medium] DPU and DPU_RDMA pre-arming logic is missing despite claims in the commit message.

Pre-existing issues:
- [High] Shared IRQ handler unconditionally accesses MMIO without verifying the device is active.
--

--- Patch [7]: [RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support ---
commit 44e533d6c3c461a28b2f0fb3ee35a8a9e75c974a
Author: Jiaxing Hu <gahing@gahingwoo.com>

accel/rocket: add RK3576 NPU (RKNN) support

Add RK3576 support to the rocket DRM accelerator driver (used with the
Mesa Teflon TFLite delegate).

[ ... ]
>   - map the DPU (0x4000) and DPU_RDMA (0x5000) blocks and pre-arm them

[Severity: Medium]
The commit message mentions mapping and pre-arming the DPU and DPU_RDMA
blocks, and the accessor macros like rocket_dpu_writel are defined in
rocket_core.h. However, looking at rocket_job_hw_submit() in
rocket_job.c, there are no writes to these registers. Was the pre-arming
logic omitted by accident? Without it, might the compute path stall after
loading one slice on RK3576?

> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -22,12 +23,23 @@ int rocket_core_init(struct rocket_core *core)
>  	int err = 0;
>  
>  	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),
>  						    core->resets);

[Severity: High]
Will this break initialization on RK3588? The srst_h reset line has been
removed from the array, meaning the RK3588 hardware might not be fully
reset during probe.

>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
>  
> +	core->clks[0].id = "aclk";
> +	core->clks[1].id = "hclk";
> +	core->clks[2].id = "npu";
> +	core->clks[3].id = "pclk";
> +	/*
> +	 * RK3576: the CBUF (convolution buffer) has its own clock domain. The CNA
> +	 * fills the CBUF and CORE reads from it; without these the compute path
> +	 * stalls after loading one slice (RDMA, which bypasses the CBUF, still
> +	 * runs). The vendor keeps all NPU clocks on whenever powered.
> +	 */
> +	core->clks[4].id = "aclk_cbuf";
> +	core->clks[5].id = "hclk_cbuf";
>  	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);

[Severity: High]
Does this unconditional request for RK3576-specific clocks cause the probe
to fail on RK3588? If aclk_cbuf and hclk_cbuf do not exist in the RK3588
device tree, devm_clk_bulk_get() will return -ENOENT and break support for
the older platform.

[ ... ]
> 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
[ ... ]
> @@ -137,8 +148,21 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
>  	rocket_pc_writel(core, REGISTER_AMOUNTS,
>  			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
>  
> -	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
> -	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
> +	/* Enable DMA-error interrupts; PC_DONE (bits 28-29) is polled, see above. */
> +	rocket_pc_writel(core, INTERRUPT_MASK,
> +			 PC_INTERRUPT_MASK_DPU_0    | PC_INTERRUPT_MASK_DPU_1    |
> +			 PC_INTERRUPT_MASK_CORE_0   | PC_INTERRUPT_MASK_CORE_1   |
> +			 PC_INTERRUPT_MASK_PPU_0    | PC_INTERRUPT_MASK_PPU_1    |
> +			 PC_INTERRUPT_MASK_CNA_CSC_0 | PC_INTERRUPT_MASK_CNA_CSC_1 |
> +			 PC_INTERRUPT_MASK_DMA_READ_ERROR |
> +			 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);

[Severity: High]
Could this lead to an interrupt storm on RK3588? In rocket_job_hw_submit(),
completion interrupts like DPU_0 and CORE_0 are unmasked in hardware.

However, rocket_job_irq_handler() now masks INTERRUPT_RAW_STATUS to only
check DMA errors and returns IRQ_NONE. This means the completion bits are
never cleared and the interrupt is never acknowledged.

[ ... ]
> @@ -326,56 +352,99 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
[ ... ]
>  static void
>  rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
>  {
> +	struct rocket_job *job;
> +
>  	if (!atomic_read(&core->reset.pending))
>  		return;
>  
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work(&core->poll_work);

[Severity: High]
Is it safe to use the non-blocking cancel_work() here instead of
cancel_work_sync()? 

If a job timeout occurs while poll_work is actively executing on another
CPU, it will continue running alongside the reset sequence.

Since rocket_job_handle_irq() writes to OPERATION_ENABLE outside of the
job_lock, this unguarded write could abruptly halt a newly submitted job
after the reset completes.

[ ... ]
> @@ -385,7 +454,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
>  	struct rocket_device *rdev = job->rdev;
>  	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
>  
> -	dev_err(core->dev, "NPU job timed out");
> +	if (pm_runtime_active(core->dev))
> +		dev_err(core->dev,
> +			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
> +			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
> +			rocket_pc_readl(core, INTERRUPT_MASK),
> +			rocket_pc_readl(core, OPERATION_ENABLE));
> +	else
> +		dev_err(core->dev, "NPU job timed out (device not active)\n");

[Severity: High]
Is there a race condition here that could allow MMIO reads on a suspended
device? 

The lockless pm_runtime_active() check does not take a reference to
prevent suspension. If the device autosuspends in the narrow window between
the check and the rocket_pc_readl() calls, this might trigger a bus fault.
Would using pm_runtime_get_if_active() be safer?

[ ... ]
> @@ -420,14 +496,16 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
>  {
>  	struct rocket_core *core = data;
>  	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);

[Severity: High]
This is a pre-existing issue, but since the IRQ handler is registered
with IRQF_SHARED, could reading INTERRUPT_RAW_STATUS unconditionally cause
a kernel panic if the device is runtime suspended? 

If another device on the same IRQ line fires, reading unpowered MMIO
registers will trigger a synchronous external abort. Should this check
pm_runtime_get_if_active() first?

> +	/* Only bits 0-13 (DMA errors) can raise this IRQ; PC_DONE is polled. */
> +	u32 active = raw_status & 0x3fff;
> +
> +	if (!active)
> +		return IRQ_NONE;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=7

Re: [RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Add RK3576 support to the rocket DRM accelerator driver (used with the
> Mesa Teflon TFLite delegate).
> 
>   - match rockchip,rk3576-rknn-core; iterate its nodes at probe
>   - add named clock IDs (aclk/hclk/npu/pclk) and the CBUF clock domain
>     (ACLK/HCLK_RKNN_CBUF); the CNA fills the CBUF and CORE reads from it,
>     so the compute path stalls if those clocks are not held for a job
>   - guard rocket_job_timedout() MMIO behind pm_runtime_active()
>   - hrtimer completion poll: OP_EN never clears on RK3576 (unlike
>     RK3588), so poll INTERRUPT_RAW_STATUS PC_DONE bits instead

This doesn't look like a correct implementation.

>   - map the DPU (0x4000) and DPU_RDMA (0x5000) blocks and pre-arm them
>   - attach both NPU power domains (PD_NPU0 + PD_NPU1): the CBUF->CMAC
>     read path is only fully powered with NPU1 up
> 
> Tested on a Radxa ROCK 4D: the NPU probes, powers on, brings up its
> IOMMUs and runs submitted jobs to completion. Full multi-layer inference
> is not yet correct on this SoC (only the first operation per power
> session produces valid output); see the cover letter.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/accel/rocket/rocket_core.c   |  43 +++++++-
>  drivers/accel/rocket/rocket_core.h   |  22 +++-
>  drivers/accel/rocket/rocket_device.c |   4 +
>  drivers/accel/rocket/rocket_drv.c    |   1 +
>  drivers/accel/rocket/rocket_job.c    | 154 +++++++++++++++++++++------
>  5 files changed, 187 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba..7b0c48e4d 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>
>  
> @@ -22,12 +23,23 @@ int rocket_core_init(struct rocket_core *core)
>  	int err = 0;
>  
>  	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),
>  						    core->resets);
>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
>  
> +	core->clks[0].id = "aclk";
> +	core->clks[1].id = "hclk";
> +	core->clks[2].id = "npu";
> +	core->clks[3].id = "pclk";
> +	/*
> +	 * RK3576: the CBUF (convolution buffer) has its own clock domain. The CNA
> +	 * fills the CBUF and CORE reads from it; without these the compute path
> +	 * stalls after loading one slice (RDMA, which bypasses the CBUF, still
> +	 * runs). The vendor keeps all NPU clocks on whenever powered.
> +	 */
> +	core->clks[4].id = "aclk_cbuf";
> +	core->clks[5].id = "hclk_cbuf";
>  	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
> @@ -50,6 +62,18 @@ int rocket_core_init(struct rocket_core *core)
>  		return PTR_ERR(core->core_iomem);
>  	}
>  
> +	core->dpu_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu");
> +	if (IS_ERR(core->dpu_iomem)) {
> +		dev_warn(dev, "no DPU registers; DPU S_POINTER won't be pre-armed\n");
> +		core->dpu_iomem = NULL;
> +	}
> +
> +	core->dpu_rdma_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu_rdma");
> +	if (IS_ERR(core->dpu_rdma_iomem)) {
> +		dev_warn(dev, "no DPU_RDMA registers; DPU_RDMA S_POINTER won't be pre-armed\n");
> +		core->dpu_rdma_iomem = NULL;
> +	}
> +
>  	dma_set_max_seg_size(dev, UINT_MAX);
>  
>  	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
> @@ -65,6 +89,23 @@ int rocket_core_init(struct rocket_core *core)
>  		return err;
>  	}
>  
> +	/*
> +	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1). The vendor
> +	 * powers BOTH from its single NPU node even when computing on one core --
> +	 * the CBUF->CMAC read path only works fully with NPU1 powered. The board DT
> +	 * lists both power-domains on rknn_core_0; a multi-PD device skips the
> +	 * driver-core single-PD auto-attach, so attach the list explicitly. With
> +	 * one PD in DT this is a no-op (returns 1) and behaves as before.
> +	 */
> +	{
> +		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..e959b26dd 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,6 +28,16 @@
>  #define rocket_core_writel(core, reg, value) \
>  	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
>  
> +#define rocket_dpu_readl(core, reg) \
> +	readl((core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
> +#define rocket_dpu_writel(core, reg, value) \
> +	writel(value, (core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
> +
> +#define rocket_dpu_rdma_readl(core, reg) \
> +	readl((core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
> +#define rocket_dpu_rdma_writel(core, reg, value) \
> +	writel(value, (core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
> +

Where are these macros used?

>  struct rocket_core {
>  	struct device *dev;
>  	struct rocket_device *rdev;
> @@ -36,8 +47,10 @@ struct rocket_core {
>  	void __iomem *pc_iomem;
>  	void __iomem *cna_iomem;
>  	void __iomem *core_iomem;
> -	struct clk_bulk_data clks[4];
> -	struct reset_control_bulk_data resets[2];
> +	void __iomem *dpu_iomem;
> +	void __iomem *dpu_rdma_iomem;
> +	struct clk_bulk_data clks[6];
> +	struct reset_control_bulk_data resets[1];
>  
>  	struct iommu_group *iommu_group;
>  
> @@ -52,6 +65,11 @@ struct rocket_core {
>  		atomic_t pending;
>  	} reset;
>  
> +	/* RK3576 has no completion IRQ; poll for PC_DONE via hrtimer. */
> +	struct hrtimer poll_timer;
> +	struct work_struct poll_work;
> +	atomic_t poll_active;
> +
>  	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..8f6a02d50 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -215,6 +215,7 @@ static void rocket_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id dt_match[] = {
>  	{ .compatible = "rockchip,rk3588-rknn-core" },
> +	{ .compatible = "rockchip,rk3576-rknn-core" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, dt_match);
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 2f1861f96..94461bf71 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/iommu.h>
>  #include <linux/platform_device.h>
> @@ -20,6 +21,16 @@
>  
>  #define JOB_TIMEOUT_MS 500
>  
> +/*
> + * RK3576: INTERRUPT_MASK bits 28-29 are read-only (hardware rejects the write),
> + * so the PC_DONE completion signal cannot be routed to the GIC via the normal
> + * interrupt-mask path.  We poll OPERATION_ENABLE every RK3576_POLL_INTERVAL_NS
> + * instead of waiting for a completion IRQ.
> + */
> +#define PC_INTERRUPT_MASK_RK3576_PC_DONE_0  0x10000000u
> +#define PC_INTERRUPT_MASK_RK3576_PC_DONE_1  0x20000000u
> +#define RK3576_POLL_INTERVAL_NS  1000000LL  /* 1 ms */
> +
>  static struct rocket_job *
>  to_rocket_job(struct drm_sched_job *sched_job)
>  {
> @@ -137,8 +148,21 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
>  	rocket_pc_writel(core, REGISTER_AMOUNTS,
>  			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
>  
> -	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
> -	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
> +	/* Enable DMA-error interrupts; PC_DONE (bits 28-29) is polled, see above. */
> +	rocket_pc_writel(core, INTERRUPT_MASK,
> +			 PC_INTERRUPT_MASK_DPU_0    | PC_INTERRUPT_MASK_DPU_1    |
> +			 PC_INTERRUPT_MASK_CORE_0   | PC_INTERRUPT_MASK_CORE_1   |
> +			 PC_INTERRUPT_MASK_PPU_0    | PC_INTERRUPT_MASK_PPU_1    |
> +			 PC_INTERRUPT_MASK_CNA_CSC_0 | PC_INTERRUPT_MASK_CNA_CSC_1 |
> +			 PC_INTERRUPT_MASK_DMA_READ_ERROR |
> +			 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);
> +	rocket_pc_writel(core, INTERRUPT_CLEAR,
> +			 PC_INTERRUPT_CLEAR_DPU_0    | PC_INTERRUPT_CLEAR_DPU_1    |
> +			 PC_INTERRUPT_CLEAR_CORE_0   | PC_INTERRUPT_CLEAR_CORE_1   |
> +			 PC_INTERRUPT_CLEAR_PPU_0    | PC_INTERRUPT_CLEAR_PPU_1    |
> +			 PC_INTERRUPT_CLEAR_CNA_CSC_0 | PC_INTERRUPT_CLEAR_CNA_CSC_1 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
>  
>  	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
>  					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
> @@ -149,7 +173,9 @@ 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));
>  
> -	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> +	atomic_set(&core->poll_active, 1);
> +	hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> +		      HRTIMER_MODE_REL);
>  }
>  
>  static int rocket_acquire_object_fences(struct drm_gem_object **bos,
> @@ -326,56 +352,99 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
>  	return fence;
>  }
>  
> +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);
> +
> +	if (!atomic_read(&core->poll_active))
> +		return HRTIMER_NORESTART;
> +
> +	/*
> +	 * On RK3576, OPERATION_ENABLE is not cleared by hardware on completion;
> +	 * check INTERRUPT_RAW_STATUS bits 28-29 (PC_DONE_0/1) instead.
> +	 */
> +	if (rocket_pc_readl(core, OPERATION_ENABLE) == 0 ||
> +	    (rocket_pc_readl(core, INTERRUPT_RAW_STATUS) &
> +	     (PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +	      PC_INTERRUPT_MASK_RK3576_PC_DONE_1))) {
> +		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;
> +}
> +
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> +	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> +	rocket_job_handle_irq(core);
> +}
> +
>  static void rocket_job_handle_irq(struct rocket_core *core)
>  {
> +	struct rocket_job *job;
> +
> +	/* Stop the completion poll — we're handling it now. */
> +	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_pc_writel(core, INTERRUPT_CLEAR,
> +			 0x1ffff |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
> +
> +	scoped_guard(mutex, &core->job_lock) {
> +		job = core->in_flight_job;
> +		if (!job)
> +			return;
> +
> +		if (job->next_task_idx < job->task_count) {
> +			rocket_job_hw_submit(core, job);
> +			return;
>  		}
> +
> +		iommu_detach_group(job->domain->domain, core->iommu_group);
> +		dma_fence_signal(job->done_fence);
> +		pm_runtime_put_autosuspend(core->dev);
> +		core->in_flight_job = NULL;
> +	}
>  }
>  
>  static void
>  rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
>  {
> +	struct rocket_job *job;
> +
>  	if (!atomic_read(&core->reset.pending))
>  		return;
>  
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work(&core->poll_work);
> +
>  	drm_sched_stop(&core->sched, bad);
>  
> -	/*
> -	 * Remaining interrupts have been handled, but we might still have
> -	 * stuck jobs. Let's make sure the PM counters stay balanced by
> -	 * manually calling pm_runtime_put_noidle().
> -	 */
>  	scoped_guard(mutex, &core->job_lock) {
> -		if (core->in_flight_job)
> +		job = core->in_flight_job;
> +		if (job) {
>  			pm_runtime_put_noidle(core->dev);
> -
> -		iommu_detach_group(NULL, core->iommu_group);
> -
> -		core->in_flight_job = NULL;
> +			iommu_detach_group(job->domain->domain, core->iommu_group);
> +			core->in_flight_job = NULL;
> +		}
>  	}
>  
> -	/* Proceed with reset now. */
>  	rocket_core_reset(core);
>  
> -	/* NPU has been reset, we can clear the reset pending bit. */
>  	atomic_set(&core->reset.pending, 0);
>  
> -	/* Restart the scheduler */
>  	drm_sched_start(&core->sched, 0);
>  }
>  
> @@ -385,7 +454,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
>  	struct rocket_device *rdev = job->rdev;
>  	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
>  
> -	dev_err(core->dev, "NPU job timed out");
> +	if (pm_runtime_active(core->dev))
> +		dev_err(core->dev,
> +			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
> +			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
> +			rocket_pc_readl(core, INTERRUPT_MASK),
> +			rocket_pc_readl(core, OPERATION_ENABLE));
> +	else
> +		dev_err(core->dev, "NPU job timed out (device not active)\n");
>  
>  	atomic_set(&core->reset.pending, 1);
>  	rocket_reset(core, sched_job);
> @@ -420,14 +496,16 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
>  {
>  	struct rocket_core *core = data;
>  	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> +	/* Only bits 0-13 (DMA errors) can raise this IRQ; PC_DONE is polled. */
> +	u32 active = raw_status & 0x3fff;
> +
> +	if (!active)
> +		return IRQ_NONE;
>  
>  	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_READ_ERROR);
>  	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_WRITE_ERROR);
>  
> -	if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
> -	      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
> -		return IRQ_NONE;
> -
> +	rocket_pc_writel(core, INTERRUPT_CLEAR, active);
>  	rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
>  
>  	return IRQ_WAKE_THREAD;
> @@ -445,6 +523,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);
>  
> @@ -486,6 +568,10 @@ int rocket_job_init(struct rocket_core *core)
>  
>  void rocket_job_fini(struct rocket_core *core)
>  {
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work_sync(&core->poll_work);
> +
>  	drm_sched_fini(&core->sched);
>  
>  	cancel_work_sync(&core->reset.work);

-- 
Best, 
Chaoyi

Re: [RFC PATCH 7/9] accel/rocket: add RK3576 NPU (RKNN) support

[Chaoyi Chen]

Hello Jiaxing,

On 7/17/2026 4:50 PM, Jiaxing Hu wrote:
> Add RK3576 support to the rocket DRM accelerator driver (used with the
> Mesa Teflon TFLite delegate).
> 
>   - match rockchip,rk3576-rknn-core; iterate its nodes at probe
>   - add named clock IDs (aclk/hclk/npu/pclk) and the CBUF clock domain
>     (ACLK/HCLK_RKNN_CBUF); the CNA fills the CBUF and CORE reads from it,
>     so the compute path stalls if those clocks are not held for a job
>   - guard rocket_job_timedout() MMIO behind pm_runtime_active()
>   - hrtimer completion poll: OP_EN never clears on RK3576 (unlike
>     RK3588), so poll INTERRUPT_RAW_STATUS PC_DONE bits instead

This doesn't look like a correct implementation.

>   - map the DPU (0x4000) and DPU_RDMA (0x5000) blocks and pre-arm them
>   - attach both NPU power domains (PD_NPU0 + PD_NPU1): the CBUF->CMAC
>     read path is only fully powered with NPU1 up
> 
> Tested on a Radxa ROCK 4D: the NPU probes, powers on, brings up its
> IOMMUs and runs submitted jobs to completion. Full multi-layer inference
> is not yet correct on this SoC (only the first operation per power
> session produces valid output); see the cover letter.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  drivers/accel/rocket/rocket_core.c   |  43 +++++++-
>  drivers/accel/rocket/rocket_core.h   |  22 +++-
>  drivers/accel/rocket/rocket_device.c |   4 +
>  drivers/accel/rocket/rocket_drv.c    |   1 +
>  drivers/accel/rocket/rocket_job.c    | 154 +++++++++++++++++++++------
>  5 files changed, 187 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba..7b0c48e4d 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>
>  
> @@ -22,12 +23,23 @@ int rocket_core_init(struct rocket_core *core)
>  	int err = 0;
>  
>  	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),
>  						    core->resets);
>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
>  
> +	core->clks[0].id = "aclk";
> +	core->clks[1].id = "hclk";
> +	core->clks[2].id = "npu";
> +	core->clks[3].id = "pclk";
> +	/*
> +	 * RK3576: the CBUF (convolution buffer) has its own clock domain. The CNA
> +	 * fills the CBUF and CORE reads from it; without these the compute path
> +	 * stalls after loading one slice (RDMA, which bypasses the CBUF, still
> +	 * runs). The vendor keeps all NPU clocks on whenever powered.
> +	 */
> +	core->clks[4].id = "aclk_cbuf";
> +	core->clks[5].id = "hclk_cbuf";
>  	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
> @@ -50,6 +62,18 @@ int rocket_core_init(struct rocket_core *core)
>  		return PTR_ERR(core->core_iomem);
>  	}
>  
> +	core->dpu_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu");
> +	if (IS_ERR(core->dpu_iomem)) {
> +		dev_warn(dev, "no DPU registers; DPU S_POINTER won't be pre-armed\n");
> +		core->dpu_iomem = NULL;
> +	}
> +
> +	core->dpu_rdma_iomem = devm_platform_ioremap_resource_byname(pdev, "dpu_rdma");
> +	if (IS_ERR(core->dpu_rdma_iomem)) {
> +		dev_warn(dev, "no DPU_RDMA registers; DPU_RDMA S_POINTER won't be pre-armed\n");
> +		core->dpu_rdma_iomem = NULL;
> +	}
> +
>  	dma_set_max_seg_size(dev, UINT_MAX);
>  
>  	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
> @@ -65,6 +89,23 @@ int rocket_core_init(struct rocket_core *core)
>  		return err;
>  	}
>  
> +	/*
> +	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1). The vendor
> +	 * powers BOTH from its single NPU node even when computing on one core --
> +	 * the CBUF->CMAC read path only works fully with NPU1 powered. The board DT
> +	 * lists both power-domains on rknn_core_0; a multi-PD device skips the
> +	 * driver-core single-PD auto-attach, so attach the list explicitly. With
> +	 * one PD in DT this is a no-op (returns 1) and behaves as before.
> +	 */
> +	{
> +		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..e959b26dd 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,6 +28,16 @@
>  #define rocket_core_writel(core, reg, value) \
>  	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
>  
> +#define rocket_dpu_readl(core, reg) \
> +	readl((core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
> +#define rocket_dpu_writel(core, reg, value) \
> +	writel(value, (core)->dpu_iomem + (REG_DPU_##reg) - REG_DPU_S_STATUS)
> +
> +#define rocket_dpu_rdma_readl(core, reg) \
> +	readl((core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
> +#define rocket_dpu_rdma_writel(core, reg, value) \
> +	writel(value, (core)->dpu_rdma_iomem + (REG_DPU_RDMA_##reg) - REG_DPU_RDMA_RDMA_S_STATUS)
> +

Where are these macros used?

>  struct rocket_core {
>  	struct device *dev;
>  	struct rocket_device *rdev;
> @@ -36,8 +47,10 @@ struct rocket_core {
>  	void __iomem *pc_iomem;
>  	void __iomem *cna_iomem;
>  	void __iomem *core_iomem;
> -	struct clk_bulk_data clks[4];
> -	struct reset_control_bulk_data resets[2];
> +	void __iomem *dpu_iomem;
> +	void __iomem *dpu_rdma_iomem;
> +	struct clk_bulk_data clks[6];
> +	struct reset_control_bulk_data resets[1];
>  
>  	struct iommu_group *iommu_group;
>  
> @@ -52,6 +65,11 @@ struct rocket_core {
>  		atomic_t pending;
>  	} reset;
>  
> +	/* RK3576 has no completion IRQ; poll for PC_DONE via hrtimer. */
> +	struct hrtimer poll_timer;
> +	struct work_struct poll_work;
> +	atomic_t poll_active;
> +
>  	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..8f6a02d50 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -215,6 +215,7 @@ static void rocket_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id dt_match[] = {
>  	{ .compatible = "rockchip,rk3588-rknn-core" },
> +	{ .compatible = "rockchip,rk3576-rknn-core" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, dt_match);
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 2f1861f96..94461bf71 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/iommu.h>
>  #include <linux/platform_device.h>
> @@ -20,6 +21,16 @@
>  
>  #define JOB_TIMEOUT_MS 500
>  
> +/*
> + * RK3576: INTERRUPT_MASK bits 28-29 are read-only (hardware rejects the write),
> + * so the PC_DONE completion signal cannot be routed to the GIC via the normal
> + * interrupt-mask path.  We poll OPERATION_ENABLE every RK3576_POLL_INTERVAL_NS
> + * instead of waiting for a completion IRQ.
> + */
> +#define PC_INTERRUPT_MASK_RK3576_PC_DONE_0  0x10000000u
> +#define PC_INTERRUPT_MASK_RK3576_PC_DONE_1  0x20000000u
> +#define RK3576_POLL_INTERVAL_NS  1000000LL  /* 1 ms */
> +
>  static struct rocket_job *
>  to_rocket_job(struct drm_sched_job *sched_job)
>  {
> @@ -137,8 +148,21 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
>  	rocket_pc_writel(core, REGISTER_AMOUNTS,
>  			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
>  
> -	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
> -	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
> +	/* Enable DMA-error interrupts; PC_DONE (bits 28-29) is polled, see above. */
> +	rocket_pc_writel(core, INTERRUPT_MASK,
> +			 PC_INTERRUPT_MASK_DPU_0    | PC_INTERRUPT_MASK_DPU_1    |
> +			 PC_INTERRUPT_MASK_CORE_0   | PC_INTERRUPT_MASK_CORE_1   |
> +			 PC_INTERRUPT_MASK_PPU_0    | PC_INTERRUPT_MASK_PPU_1    |
> +			 PC_INTERRUPT_MASK_CNA_CSC_0 | PC_INTERRUPT_MASK_CNA_CSC_1 |
> +			 PC_INTERRUPT_MASK_DMA_READ_ERROR |
> +			 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);
> +	rocket_pc_writel(core, INTERRUPT_CLEAR,
> +			 PC_INTERRUPT_CLEAR_DPU_0    | PC_INTERRUPT_CLEAR_DPU_1    |
> +			 PC_INTERRUPT_CLEAR_CORE_0   | PC_INTERRUPT_CLEAR_CORE_1   |
> +			 PC_INTERRUPT_CLEAR_PPU_0    | PC_INTERRUPT_CLEAR_PPU_1    |
> +			 PC_INTERRUPT_CLEAR_CNA_CSC_0 | PC_INTERRUPT_CLEAR_CNA_CSC_1 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
>  
>  	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
>  					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
> @@ -149,7 +173,9 @@ 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));
>  
> -	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
> +	atomic_set(&core->poll_active, 1);
> +	hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
> +		      HRTIMER_MODE_REL);
>  }
>  
>  static int rocket_acquire_object_fences(struct drm_gem_object **bos,
> @@ -326,56 +352,99 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
>  	return fence;
>  }
>  
> +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);
> +
> +	if (!atomic_read(&core->poll_active))
> +		return HRTIMER_NORESTART;
> +
> +	/*
> +	 * On RK3576, OPERATION_ENABLE is not cleared by hardware on completion;
> +	 * check INTERRUPT_RAW_STATUS bits 28-29 (PC_DONE_0/1) instead.
> +	 */
> +	if (rocket_pc_readl(core, OPERATION_ENABLE) == 0 ||
> +	    (rocket_pc_readl(core, INTERRUPT_RAW_STATUS) &
> +	     (PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +	      PC_INTERRUPT_MASK_RK3576_PC_DONE_1))) {
> +		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;
> +}
> +
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> +	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> +	rocket_job_handle_irq(core);
> +}
> +
>  static void rocket_job_handle_irq(struct rocket_core *core)
>  {
> +	struct rocket_job *job;
> +
> +	/* Stop the completion poll — we're handling it now. */
> +	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_pc_writel(core, INTERRUPT_CLEAR,
> +			 0x1ffff |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
> +
> +	scoped_guard(mutex, &core->job_lock) {
> +		job = core->in_flight_job;
> +		if (!job)
> +			return;
> +
> +		if (job->next_task_idx < job->task_count) {
> +			rocket_job_hw_submit(core, job);
> +			return;
>  		}
> +
> +		iommu_detach_group(job->domain->domain, core->iommu_group);
> +		dma_fence_signal(job->done_fence);
> +		pm_runtime_put_autosuspend(core->dev);
> +		core->in_flight_job = NULL;
> +	}
>  }
>  
>  static void
>  rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
>  {
> +	struct rocket_job *job;
> +
>  	if (!atomic_read(&core->reset.pending))
>  		return;
>  
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work(&core->poll_work);
> +
>  	drm_sched_stop(&core->sched, bad);
>  
> -	/*
> -	 * Remaining interrupts have been handled, but we might still have
> -	 * stuck jobs. Let's make sure the PM counters stay balanced by
> -	 * manually calling pm_runtime_put_noidle().
> -	 */
>  	scoped_guard(mutex, &core->job_lock) {
> -		if (core->in_flight_job)
> +		job = core->in_flight_job;
> +		if (job) {
>  			pm_runtime_put_noidle(core->dev);
> -
> -		iommu_detach_group(NULL, core->iommu_group);
> -
> -		core->in_flight_job = NULL;
> +			iommu_detach_group(job->domain->domain, core->iommu_group);
> +			core->in_flight_job = NULL;
> +		}
>  	}
>  
> -	/* Proceed with reset now. */
>  	rocket_core_reset(core);
>  
> -	/* NPU has been reset, we can clear the reset pending bit. */
>  	atomic_set(&core->reset.pending, 0);
>  
> -	/* Restart the scheduler */
>  	drm_sched_start(&core->sched, 0);
>  }
>  
> @@ -385,7 +454,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
>  	struct rocket_device *rdev = job->rdev;
>  	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
>  
> -	dev_err(core->dev, "NPU job timed out");
> +	if (pm_runtime_active(core->dev))
> +		dev_err(core->dev,
> +			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
> +			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
> +			rocket_pc_readl(core, INTERRUPT_MASK),
> +			rocket_pc_readl(core, OPERATION_ENABLE));
> +	else
> +		dev_err(core->dev, "NPU job timed out (device not active)\n");
>  
>  	atomic_set(&core->reset.pending, 1);
>  	rocket_reset(core, sched_job);
> @@ -420,14 +496,16 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
>  {
>  	struct rocket_core *core = data;
>  	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
> +	/* Only bits 0-13 (DMA errors) can raise this IRQ; PC_DONE is polled. */
> +	u32 active = raw_status & 0x3fff;
> +
> +	if (!active)
> +		return IRQ_NONE;
>  
>  	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_READ_ERROR);
>  	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_WRITE_ERROR);
>  
> -	if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
> -	      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
> -		return IRQ_NONE;
> -
> +	rocket_pc_writel(core, INTERRUPT_CLEAR, active);
>  	rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
>  
>  	return IRQ_WAKE_THREAD;
> @@ -445,6 +523,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);
>  
> @@ -486,6 +568,10 @@ int rocket_job_init(struct rocket_core *core)
>  
>  void rocket_job_fini(struct rocket_core *core)
>  {
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work_sync(&core->poll_work);
> +
>  	drm_sched_fini(&core->sched);
>  
>  	cancel_work_sync(&core->reset.work);

-- 
Best, 
Chaoyi

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[RFC PATCH 8/9] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

[Jiaxing Hu]

Add the RK3576 NPU: rknn_core_0/1 and rknn_mmu_0/1, with register
addresses, GIC_SPI 247/248 interrupts, clocks, resets and power-domains.

Run the full RKNN clock set (CLK_RKNN_DSU0, ACLK_RKNN0/1, HCLK_RKNN_ROOT,
ACLK/HCLK_RKNN_CBUF) during the NPU0/NPU1 power transitions and on the
MMU nodes; without the functional and CBUF clocks the block powers on but
its registers (including the IOMMU banks) read back dead. Drive the RKNN
BIU reset (SRST_A_RKNN0/1_BIU) from the power domain so it fires before
the IOMMU resumes.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 78 +++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index e12a2a0cf..5033f7628 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>;
 						};
@@ -1804,6 +1812,72 @@ 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>,
+			      <0x0 0x27704000 0x0 0x1000>,
+			      <0x0 0x27705000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
+			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>;
+			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>;
+			#iommu-cells = <0>;
+			power-domains = <&power RK3576_PD_NPU0>;
+			status = "disabled";
+		};
+
+		rknn_core_1: npu@27710000 {
+			compatible = "rockchip,rk3576-rknn-core";
+			reg = <0x0 0x27710000 0x0 0x1000>,
+			      <0x0 0x27711000 0x0 0x1000>,
+			      <0x0 0x27713000 0x0 0x1000>,
+			      <0x0 0x27714000 0x0 0x1000>,
+			      <0x0 0x27715000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>;
+			clock-names = "aclk", "hclk", "npu", "pclk";
+			resets = <&cru SRST_A_RKNN1>;
+			reset-names = "srst_a";
+			power-domains = <&power RK3576_PD_NPU1>;
+			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>;
+			#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

[RFC PATCH 8/9] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

[Jiaxing Hu]

Add the RK3576 NPU: rknn_core_0/1 and rknn_mmu_0/1, with register
addresses, GIC_SPI 247/248 interrupts, clocks, resets and power-domains.

Run the full RKNN clock set (CLK_RKNN_DSU0, ACLK_RKNN0/1, HCLK_RKNN_ROOT,
ACLK/HCLK_RKNN_CBUF) during the NPU0/NPU1 power transitions and on the
MMU nodes; without the functional and CBUF clocks the block powers on but
its registers (including the IOMMU banks) read back dead. Drive the RKNN
BIU reset (SRST_A_RKNN0/1_BIU) from the power domain so it fires before
the IOMMU resumes.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 78 +++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index e12a2a0cf..5033f7628 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>;
 						};
@@ -1804,6 +1812,72 @@ 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>,
+			      <0x0 0x27704000 0x0 0x1000>,
+			      <0x0 0x27705000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
+			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>;
+			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>;
+			#iommu-cells = <0>;
+			power-domains = <&power RK3576_PD_NPU0>;
+			status = "disabled";
+		};
+
+		rknn_core_1: npu@27710000 {
+			compatible = "rockchip,rk3576-rknn-core";
+			reg = <0x0 0x27710000 0x0 0x1000>,
+			      <0x0 0x27711000 0x0 0x1000>,
+			      <0x0 0x27713000 0x0 0x1000>,
+			      <0x0 0x27714000 0x0 0x1000>,
+			      <0x0 0x27715000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>;
+			clock-names = "aclk", "hclk", "npu", "pclk";
+			resets = <&cru SRST_A_RKNN1>;
+			reset-names = "srst_a";
+			power-domains = <&power RK3576_PD_NPU1>;
+			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>;
+			#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


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 8/9] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `aclk_cbuf` and `hclk_cbuf` clocks in `rknn_core_1` node.
- [High] Single power-domain definition for NPU cores causes probe failure (`-EEXIST`) and violates hardware constraints.
- [High] The physical register address for `rknn_mmu_1` is incorrectly mapped to `0x2770a000` instead of `0x27712000`.
--

--- Patch [8]: [RFC PATCH 8/9] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes ---
commit 5396e10743f01d595779ccc7127139fe5f89e996
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
    
Add the RK3576 NPU: rknn_core_0/1 and rknn_mmu_0/1, with register
addresses, GIC_SPI 247/248 interrupts, clocks, resets and power-domains.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> index e12a2a0cfb891..5033f762840ae 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
[ ... ]
> @@ -1804,6 +1812,72 @@ 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>,
> +			      <0x0 0x27704000 0x0 0x1000>,
> +			      <0x0 0x27705000 0x0 0x1000>;
> +			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
> +			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>;

[Severity: High]
Does listing a single power domain here cause probe to fail?

The rocket_core_init driver expects both RK3576_PD_NPU0 and RK3576_PD_NPU1 on
rknn_core_0 to satisfy a hardware constraint where the CBUF->CMAC read path
requires both.

Because the DT only lists one power domain per core, the kernel platform bus
auto-attaches it and sets dev->pm_domain. When the driver then calls
devm_pm_domain_attach_list(), it will return -EEXIST, which the driver treats
as a fatal error and aborts probe. Missing RK3576_PD_NPU1 on rknn_core_0 also
breaks runtime PM, leading to AXI lockups.

> +			iommus = <&rknn_mmu_0>;
> +			status = "disabled";
> +		};
> +
[ ... ]
> +		rknn_core_1: npu@27710000 {
> +			compatible = "rockchip,rk3576-rknn-core";
> +			reg = <0x0 0x27710000 0x0 0x1000>,
> +			      <0x0 0x27711000 0x0 0x1000>,
> +			      <0x0 0x27713000 0x0 0x1000>,
> +			      <0x0 0x27714000 0x0 0x1000>,
> +			      <0x0 0x27715000 0x0 0x1000>;
> +			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
> +			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
> +				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>;
> +			clock-names = "aclk", "hclk", "npu", "pclk";

[Severity: High]
Will the driver fail to probe here due to missing CBUF clocks?

The rocket_core_init function explicitly requests 6 clocks (aclk, hclk, npu,
pclk, aclk_cbuf, hclk_cbuf) using devm_clk_bulk_get(). Since rknn_core_1 only
defines 4 clocks, the API will fail with -ENOENT, making the second NPU core
unusable.

> +			resets = <&cru SRST_A_RKNN1>;
> +			reset-names = "srst_a";
> +			power-domains = <&power RK3576_PD_NPU1>;
> +			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>;

[Severity: High]
Is this physical register address mapped correctly?

It looks like the base address is misplaced by 0x8000 relative to where it
belongs inside the rknn_core_1 memory footprint. The rknn_core_1 registers
span 0x27710000-0x27715000, with a 1-page gap at 0x27712000 for the IOMMU
(which is structurally symmetric to rknn_core_0 having its IOMMU at
0x27702000).

When the IOMMU driver attempts to access 0x2770a000 instead of 0x27712000, it
will likely cause a Synchronous External Abort or fail to translate memory
for the NPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=8

[RFC PATCH 9/9] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[Jiaxing Hu]

Enable rknn_core_0 and rknn_mmu_0 on the Radxa ROCK 4D. Supply the NPU
rail (vdd_npu_s0) via npu-supply; it is marked regulator-always-on for
now because the NPU power sequencing is still being brought up (see the
cover letter) -- happy to switch to proper runtime control once the
compute path is sorted.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 16 ++++++++++++++++
 1 file changed, 16 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..09d2fc98b 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,18 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
 		remote-endpoint = <&hdmi_in_vp0>;
 	};
 };
+
+&rknn_core_0 {
+	npu-supply = <&vdd_npu_s0>;
+	/*
+	 * Power BOTH NPU domains (NPU0 + NPU1) like the vendor's single NPU node,
+	 * even though rocket computes only on core 0: the CBUF->CMAC read path is
+	 * only fully powered with NPU1 up. rocket attaches the multi-PD list.
+	 */
+	power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
+	status = "okay";
+};
+
+&rknn_mmu_0 {
+	status = "okay";
+};
-- 
2.43.0

[RFC PATCH 9/9] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[Jiaxing Hu]

Enable rknn_core_0 and rknn_mmu_0 on the Radxa ROCK 4D. Supply the NPU
rail (vdd_npu_s0) via npu-supply; it is marked regulator-always-on for
now because the NPU power sequencing is still being brought up (see the
cover letter) -- happy to switch to proper runtime control once the
compute path is sorted.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 16 ++++++++++++++++
 1 file changed, 16 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..09d2fc98b 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,18 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
 		remote-endpoint = <&hdmi_in_vp0>;
 	};
 };
+
+&rknn_core_0 {
+	npu-supply = <&vdd_npu_s0>;
+	/*
+	 * Power BOTH NPU domains (NPU0 + NPU1) like the vendor's single NPU node,
+	 * even though rocket computes only on core 0: the CBUF->CMAC read path is
+	 * only fully powered with NPU1 up. rocket attaches the multi-PD list.
+	 */
+	power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
+	status = "okay";
+};
+
+&rknn_mmu_0 {
+	status = "okay";
+};
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH 9/9] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional call to `devm_pm_domain_attach_list()` breaks probe on single power-domain platforms (e.g., RK3588).
- [Medium] Devicetree binding schema violation: `power-domains` has multiple items.
--

--- Patch [9]: [RFC PATCH 9/9] arm64: dts: rockchip: rk3576-rock-4d: enable NPU ---
commit b202e27a2b4bfe1725b5bf2c08f62041f816c518
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: rk3576-rock-4d: enable NPU

This commit enables rknn_core_0 and rknn_mmu_0 on the Radxa ROCK 4D. It
supplies the NPU rail and configures the power domains.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 272af1012ab03..09d2fc98bb4c1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts

[ ... ]

> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;
> +	/*
> +	 * Power BOTH NPU domains (NPU0 + NPU1) like the vendor's single NPU node,
> +	 * even though rocket computes only on core 0: the CBUF->CMAC read path is
> +	 * only fully powered with NPU1 up. rocket attaches the multi-PD list.
> +	 */
> +	power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;

[Severity: High]
Does the driver's handling of the multi-PD list break probe on single-PD
platforms like RK3588?

The comment here notes that rocket attaches the multi-PD list. When a device
has exactly one power-domains entry, the driver core automatically populates
dev->pm_domain before rocket_core_init() runs.

If the driver unconditionally calls devm_pm_domain_attach_list():

drivers/accel/rocket/rocket_core.c:rocket_core_init() {
    ...
		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");
    ...
}

In this scenario, devm_pm_domain_attach_list() returns -EEXIST because
dev->pm_domain is already set. Since -EEXIST is less than 0, won't this abort
the probe via dev_err_probe(), fatally failing initialization for platforms
with only one power domain?

[Severity: Medium]
Will this multiple power domain configuration cause dtbs_check validation
errors?

The schema currently restricts the power-domains property to a single item:

Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml:
  power-domains:
    maxItems: 1

Since this adds both RK3576_PD_NPU0 and RK3576_PD_NPU1, could the schema
be updated to support maxItems: 2 before adding this configuration to avoid
build-time tooling errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717085220.3212274-1-gahing@gahingwoo.com?part=9