[PATCH v7 00/10] accel/rocket: RK3576 NPU (RKNN) enablement

[PATCH v7 00/10] accel/rocket: RK3576 NPU (RKNN) enablement

[Jiaxing Hu]

Based on Igor Paunovic's "[PATCH v2] accel/rocket: request the core
clocks by name", as v6 was:

  https://lore.kernel.org/linux-rockchip/20260729130743.128876-1-royalnet026@gmail.com/

Tested on a Radxa ROCK 4D, on next-20260730.

The thing this series has been describing as unsolved since v3 is solved,
and it was one register write. That changes the cover letter more than it
changes the patches, so the correction comes first.

The claim I have to withdraw
============================

Every version of this series from v1 to v6 has said that the RK3576's
completion interrupt is armed exactly as on RK3588 and never reaches the
GIC, and v6 shipped an hrtimer that samples INTERRUPT_RAW_STATUS instead
of waiting for it. That is not true. The interrupt works.

It never fired because the block believed it had 28672 tasks left to
run, so the job was never complete. PC_TASK_CON packs the task number
with three controls above it, and the field widths are not the same on
both SoCs:

  RK3588   BIT[11:0] task_number, BIT[12] pp_en, BIT[13] count_clear
  RK3576   BIT[15:0] task_number, BIT[16] pp_en, BIT[17] count_clear,
           BIT[18] last_layer_clear

rocket_registers.h is generated from the RK3588 description, so writing
it unchanged on an RK3576 asks for task_number 0x7001, that is 28673
tasks, and puts TASK_COUNT_CLEAR on a bit that does nothing. The counter
was then only ever cleared by a reset, which is precisely the "one task
per reset" shape v6 reported, and it is why every completion path I
added looked necessary.

It was found by taking an ordered trace of every register write during
one submit and diffing it against the same trace from the vendor driver
on the same board. Exactly one value differed.

Robin and Diederik, my apologies for the thread that premise cost you.
Robin's shortlist for an interrupt that never arrives was an extra clock
or power domain in the path, a masking control that had been overlooked,
a wrong description, or terminally broken hardware. It was none of those,
because the interrupt was not the thing that was wrong.

Chaoyi Chen of Rockchip confirmed the layout on the list, including the
fourth control at BIT(18) that the trace could not have named:

  https://lore.kernel.org/all/4f300b78-d96d-4d98-8819-dc292b0c9b97@rock-chips.com/

I sent a correction to the list when this landed rather than leaving it
until now:

  https://lore.kernel.org/all/20260807211629.1573228-1-gahing@gahingwoo.com/

So v7 drops the poll entirely. There is no hrtimer, no poll work, no
second completion path and no arbitration between two of them. A job is
retired by its interrupt, the way it is on RK3588.

Changes in v7
=============

  * accel/rocket: PC_TASK_CON is written with the RK3576 field layout.
    The defines are in rocket_job.c rather than in rocket_registers.h,
    which is generated and says not to edit it by hand.

  * accel/rocket: the completion poll and everything supporting it is
    gone: poll_completion, the hrtimer, the work, poll_seq and its
    mirror, poll_dying, the teardown ordering in rocket_job_fini() and
    the arbitration in rocket_job_handle_irq(). 7/9 in v6 was 142 added
    lines in rocket_job.c and 8/10 here is 90, most of which is the
    comment explaining the register.

  * accel/rocket: the job_lock fix is its own patch now, 1/10, with a
    Fixes tag. It is an RK3588 bug and it was buried in the middle of a
    preparation patch in v6, where nobody could backport it.

  * accel/rocket: the power domain list is attached before
    iommu_group_get() rather than after rocket_job_init(). In v6 a
    failure there returned without unwinding either of them. Igor caught
    it. Moving the call is better than adding an unwind path, since
    everything before that point is devres managed and a plain return is
    then correct. It also keeps commit f509a081f6a2
    ("accel/rocket: fix unwinding in error path in rocket_core_init")
    from having a second copy of itself to keep in step.

  * accel/rocket: struct rocket_core's clks[] no longer grows in 7/10.
    Igor pointed out that 7/10 claimed nothing changes for RK3588 while
    growing the array there, with the two extra names only arriving in
    8/10. The array now grows in the patch that adds the names.

  * The series is 10 patches rather than 9 because of the split above.
    Nothing else moved.

What this means for the split in 7/10 and 8/10
==============================================

Diederik asked for the enablement to be split and Igor seconded it, and
v6 did that. The split survives v7 unchanged: 7/10 is the soc_data
plumbing with RK3588 keeping four clocks and two resets, and 8/10 is the
RK3576 enablement. What changed is that 8/10 is now much smaller.

Where it stands
===============

With every debug knob off, on a ROCK 4D:

  * the NPU probes with the two domain list and no attach failure;
  * a convolution submitted three times with three different inputs is
    byte exact against the CPU reference each time, with no reset in
    between and with nothing retiring the job but the interrupt;
  * the NPU's line in /proc/interrupts goes from zero to three across
    those three submits, one each and no more;
  * unbind and rebind is clean with no warning;
  * every patch builds on its own;
  * dt_binding_check is clean on all three bindings the series touches.

The measurement in v6 that could not tell a recomputation from an
untouched output buffer has been replaced by one that can: the inputs
differ between submits, so a stale buffer cannot pass.

That was run on this branch with nothing else applied. The out of tree
work this hardware has needed for the userspace investigation, including
an rk_iommu flush_iotlb_all that is neither upstream nor in this series,
is not present, and the userspace results below are the same without it.

The userspace side is a separate matter and is not part of this series.
It runs regular convolutions, depthwise convolutions and the first
convolution of MobileNet V1 byte exact per output channel against the
CPU reference, and two chained operators come out at the accuracy the
hardware's own arithmetic allows. A whole MobileNet does not run yet.
Nothing that is still wrong there is in the kernel.

Igor, thank you again for the RK3588 review. Both of the things you
found in v6 are addressed above and the completion path has changed
enough that it needs another look rather than a carried tag. Your offer
to run this on all three RK3588 cores would be very welcome, since the
only behaviour change to RK3588 in the series is the job_lock ordering
in 1/10 and I cannot test it here.

Jiaxing Hu (10):
  accel/rocket: take the completion register writes under job_lock
  dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  dt-bindings: power: rockchip: allow resets in a power domain node
  dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set
  pmdomain/rockchip: add optional per-domain power-on settle delay
  pmdomain/rockchip: cycle optional power-domain resets on power-on
  accel/rocket: select the per-core clock and reset counts from match
    data
  accel/rocket: add RK3576 NPU (RKNN) support
  arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  arm64: dts: rockchip: rk3576-rock-4d: enable NPU

 .../bindings/iommu/rockchip,iommu.yaml        |  8 ++
 .../npu/rockchip,rk3588-rknn-core.yaml        | 47 +++++++++-
 .../power/rockchip,power-controller.yaml      |  8 ++
 .../boot/dts/rockchip/rk3576-rock-4d.dts      | 10 +++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      | 80 ++++++++++++++++-
 drivers/accel/rocket/rocket_core.c            | 28 +++++-
 drivers/accel/rocket/rocket_core.h            | 11 ++-
 drivers/accel/rocket/rocket_device.c          |  4 +
 drivers/accel/rocket/rocket_drv.c             | 22 ++++-
 drivers/accel/rocket/rocket_job.c             | 90 ++++++++++++++-----
 drivers/pmdomain/rockchip/pm-domains.c        | 71 ++++++++++-----
 11 files changed, 324 insertions(+), 55 deletions(-)

-- 
2.43.0

[PATCH v7 01/10] accel/rocket: take the completion register writes under job_lock

[Jiaxing Hu]

rocket_job_handle_irq() writes OPERATION_ENABLE and INTERRUPT_CLEAR before
taking job_lock, while rocket_job_hw_submit() writes OPERATION_ENABLE from
inside it. The two can therefore race: a completion being handled on one core
can write its zero after a submit on the same core has written its one, and
stop a task that has only just started.

Nothing in tree hits this often, because the interrupt is the only completion
path and it does not overlap its own submit, but the ordering is wrong on its
own terms.

Move both writes inside the existing scoped_guard() rather than adding a second
critical section, so stopping the block and deciding what to start next are one
atomic step.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_job.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index bb77b6bf0..4c01b703e 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -345,10 +345,15 @@ static void rocket_job_handle_irq(struct rocket_core *core)
 {
 	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) {
+		/*
+		 * Stopping the block belongs under the lock. hw_submit() writes
+		 * OPERATION_ENABLE too, and outside the lock this zero can land
+		 * after that one and stop a task that has only just started.
+		 */
+		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);
@@ -360,6 +365,7 @@ static void rocket_job_handle_irq(struct rocket_core *core)
 			pm_runtime_put_autosuspend(core->dev);
 			core->in_flight_job = NULL;
 		}
+	}
 }
 
 static void
-- 
2.43.0

Re: [PATCH v7 01/10] accel/rocket: take the completion register writes under job_lock

[Igor Paunovic]

Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, three cores

I ran this on an Orange Pi 5 Plus across all three NPU cores, against a
base without the series. Both modules were built the same way and
neither carried any local DVFS work.

  base:  v7.2 rocket
         + Guangshuo Li's "clear rdev on device init failure"
         + my "request the core clocks by name" v2
         + my lifecycle v2 1/2 and 2/2
  test:  the same, plus 1/10, 7/10 and 8/10 from this series

Six phases per module: all three cores bound; core 2 unbound and
rebound; core 0 unbound and rebound; all three unbound and all three
rebound. One MobileNet V1 run per phase through the Teflon delegate.
The oracle is the sha256 of the tensors that both change between
different inputs and stay stable across repeats, so a stale output
buffer cannot pass as a recomputation.

                          base    this series
    three cores           89.3       90.0   inf/s
    core 2 unbound        87.6       88.8
    core 2 rebound        88.9       88.2
    core 0 unbound        75.3       75.0
    core 0 rebound        88.6       88.4
    all three cycled      88.7       88.3

All twelve runs produce identical oracle hashes and the same
classification. Interrupts per inference are 42.75 in both, and the
distribution matches phase for phase: with core 0 bound it takes 41.7
of them and core 1 takes 1.02; with core 0 unbound the same work moves
to core 1. Neither round logged anything beyond the probe messages.

That comes to 2596 inferences and 111048 completion interrupts through
rocket_job_handle_irq() with the two writes moved under job_lock, with
no difference in result from the same count without them.

On the change itself: I could not construct the race on the normal
path. The scheduler runs one job at a time and the fence is signalled
under the same lock after the writes, so a submit cannot overlap the
completion it follows.

Where I think it is reachable is the reset path. rocket_reset() calls
drm_sched_stop() and then says "Remaining interrupts have been
handled", but drm_sched_stop() stops the scheduler, not the threaded
IRQ handler. A handler already in flight can therefore run alongside
rocket_reset(), and after drm_sched_start() alongside a fresh job.
Making the write and the decision one step is the right shape for
that. It does not stop a late handler from writing the zero into a job
that is not the one whose interrupt it is handling, though - would a
synchronize_irq(core->irq) before the guard in rocket_reset() be worth
having as well?

One note on the base, since it matters to anyone repeating this. The
core-0 rebind step needs my lifecycle series underneath. Without it,
that rebind hands the returning core the index of a core that is still
live: the driver prints "core 2" for fdab0000.npu, inference starts
returning a different answer, and the teardown that follows dies in
destroy_workqueue() under drm_sched_fini() with a poisoned list
pointer, leaving an unkillable D state. None of that is your series
doing - it reproduces with 1/10, 7/10 and 8/10 absent - but it does
mean the three-core test cannot run to completion on a tree without it.

Igor

[PATCH v7 02/10] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Jiaxing Hu]

The RK3576 NPU has two cores of the same RKNN block the RK3588 binding
already describes, but it wires them up differently: two extra CBUF
clocks, two power domains per core, and a single reset instead of two.
It also has no NPU SRAM supply.

Widen the property ranges to cover both, then pin each SoC back to its
own shape in allOf so nothing loosens for RK3588, and keep sram-supply
required for rockchip,rk3588-rknn-core only.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../npu/rockchip,rk3588-rknn-core.yaml        | 47 +++++++++++++++++--
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
index caca2a490..3b611b64c 100644
--- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
+++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
@@ -21,6 +21,7 @@ properties:
 
   compatible:
     enum:
+      - rockchip,rk3576-rknn-core
       - rockchip,rk3588-rknn-core
 
   reg:
@@ -33,14 +34,18 @@ properties:
       - const: core # Main NPU core processing unit registers
 
   clocks:
-    maxItems: 4
+    minItems: 4
+    maxItems: 6
 
   clock-names:
+    minItems: 4
     items:
       - const: aclk
       - const: hclk
       - const: npu
       - const: pclk
+      - const: aclk_cbuf
+      - const: hclk_cbuf
 
   interrupts:
     maxItems: 1
@@ -51,12 +56,15 @@ properties:
   npu-supply: true
 
   power-domains:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   resets:
+    minItems: 1
     maxItems: 2
 
   reset-names:
+    minItems: 1
     items:
       - const: srst_a
       - const: srst_h
@@ -75,7 +83,40 @@ required:
   - resets
   - reset-names
   - npu-supply
-  - sram-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3588-rknn-core
+    then:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          maxItems: 4
+        power-domains:
+          maxItems: 1
+        resets:
+          minItems: 2
+        reset-names:
+          minItems: 2
+      required:
+        - sram-supply
+    else:
+      properties:
+        clocks:
+          minItems: 6
+        clock-names:
+          minItems: 6
+        power-domains:
+          minItems: 2
+        resets:
+          maxItems: 1
+        reset-names:
+          maxItems: 1
+        sram-supply: false
 
 additionalProperties: false
 
-- 
2.43.0

Re: [PATCH v7 02/10] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Krzysztof Kozlowski]

On Wed, Aug 12, 2026 at 09:40:57PM +1200, Jiaxing Hu wrote:
> The RK3576 NPU has two cores of the same RKNN block the RK3588 binding
> already describes, but it wires them up differently: two extra CBUF
> clocks, two power domains per core, and a single reset instead of two.
> It also has no NPU SRAM supply.
> 
> Widen the property ranges to cover both, then pin each SoC back to its
> own shape in allOf so nothing loosens for RK3588, and keep sram-supply
> required for rockchip,rk3588-rknn-core only.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  .../npu/rockchip,rk3588-rknn-core.yaml        | 47 +++++++++++++++++--
>  1 file changed, 44 insertions(+), 3 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v7 03/10] dt-bindings: power: rockchip: allow resets in a power domain node

[Jiaxing Hu]

Some domains do not come up in a usable state on their own and need
their resets cycled once power is on. The RK3576 NPU domains are one
case: without it the first access after power-on takes an async SError.

pd-node has no resets property and every nesting level is
unevaluatedProperties: false, so describing that in DT is rejected
today. Add it alongside clocks.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../bindings/power/rockchip,power-controller.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
index b41db576f..f23c1a118 100644
--- a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
@@ -136,6 +136,14 @@ $defs:
           A number of phandles to clocks that need to be enabled
           while power domain switches state.
 
+      resets:
+        minItems: 1
+        maxItems: 30
+        description: |
+          A number of phandles to resets that need to be cycled once the power
+          domain has been switched on, for domains whose logic does not come up
+          in a usable state by itself.
+
       domain-supply:
         description: domain regulator supply.
 
-- 
2.43.0

Re: [PATCH v7 03/10] dt-bindings: power: rockchip: allow resets in a power domain node

[Krzysztof Kozlowski]

On Wed, Aug 12, 2026 at 09:40:58PM +1200, Jiaxing Hu wrote:
> Some domains do not come up in a usable state on their own and need
> their resets cycled once power is on. The RK3576 NPU domains are one
> case: without it the first access after power-on takes an async SError.
> 
> pd-node has no resets property and every nesting level is
> unevaluatedProperties: false, so describing that in DT is rejected
> today. Add it alongside clocks.

This paragraph is redundant. Why are you explaining correct syntax?

> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  .../bindings/power/rockchip,power-controller.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> index b41db576f..f23c1a118 100644
> --- a/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/rockchip,power-controller.yaml
> @@ -136,6 +136,14 @@ $defs:
>            A number of phandles to clocks that need to be enabled
>            while power domain switches state.
>  
> +      resets:
> +        minItems: 1
> +        maxItems: 30

30 resets per one power domain? and none got to the example in this
file?

> +        description: |

Do not need '|' unless you need to preserve formatting.

> +          A number of phandles to resets that need to be cycled once the power
> +          domain has been switched on, for domains whose logic does not come up
> +          in a usable state by itself.
> +
>        domain-supply:
>          description: domain regulator supply.
>  
> -- 
> 2.43.0
> 

Re: [PATCH v7 03/10] dt-bindings: power: rockchip: allow resets in a power domain node

[Jiaxing Hu]

Hi Krzysztof,

> This paragraph is redundant. Why are you explaining correct syntax?

No good reason. v8 drops it.

> 30 resets per one power domain? and none got to the example in this
> file?

One. Both RK3576 NPU domains carry exactly one, SRST_A_RKNN0_BIU and
SRST_A_RKNN1_BIU, and 30 came from the clocks property above it, copied
without asking what it would mean here. v8 has maxItems: 1, which is
what this series actually needs, and it can be widened by whoever turns
up with a domain that needs more.

And no, nothing got to the example, which it should have. v8 adds one.

> Do not need '|' unless you need to preserve formatting.

v8 drops it.

Nothing above is fixed yet, only decided. I told another reviewer on v6
that something was fixed for v7 and then sent v7 without it, so I would
rather say what v8 will contain than describe it as done.

Thanks for the review.

Jiaxing

[PATCH v7 04/10] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set

[Jiaxing Hu]

The RK3576 NPU MMUs need more than aclk and iface. With only those two
enabled the MMU accepts reads but silently drops register writes: a
DTE_ADDR value written from the power domain, while the domain clocks
are still on, reads back correctly, and the write rk_iommu_resume() does
microseconds later does not land at all. The vendor DT names the CBUF
clocks as that MMU's interface clocks and its driver keeps every NPU
clock on for as long as the device is powered.

The driver side of this is already upstream, commit 841363ebb508
("iommu/rockchip: Take all DT clocks"), which switched rk_iommu to
devm_clk_bulk_get_all(). Widen the schema to match so those nodes can
be described. minItems stays at 2, so every existing devicetree, which
all carry exactly aclk and iface, is unaffected.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../devicetree/bindings/iommu/rockchip,iommu.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
index 6ce41d11f..a3cedcaaa 100644
--- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
@@ -42,14 +42,22 @@ properties:
     minItems: 1
 
   clocks:
+    minItems: 2
     items:
       - description: Core clock
       - description: Interface clock
+      - description: Compute clock, RK3576 NPU MMUs only
+      - description: Convolution buffer core clock, RK3576 NPU MMUs only
+      - description: Convolution buffer interface clock, RK3576 NPU MMUs only
 
   clock-names:
+    minItems: 2
     items:
       - const: aclk
       - const: iface
+      - const: npu
+      - const: aclk_cbuf
+      - const: hclk_cbuf
 
   "#iommu-cells":
     const: 0
-- 
2.43.0

Re: [PATCH v7 04/10] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set

[Diederik de Haas]

Hi Jiaxing,

On Wed Aug 12, 2026 at 11:40 AM CEST, Jiaxing Hu wrote:
> The RK3576 NPU MMUs need more than aclk and iface. With only those two
> enabled the MMU accepts reads but silently drops register writes: a
> DTE_ADDR value written from the power domain, while the domain clocks
> are still on, reads back correctly, and the write rk_iommu_resume() does
> microseconds later does not land at all. The vendor DT names the CBUF
> clocks as that MMU's interface clocks and its driver keeps every NPU
> clock on for as long as the device is powered.
>
> The driver side of this is already upstream, commit 841363ebb508
> ("iommu/rockchip: Take all DT clocks"), which switched rk_iommu to
> devm_clk_bulk_get_all(). Widen the schema to match so those nodes can
> be described. minItems stays at 2, so every existing devicetree, which
> all carry exactly aclk and iface, is unaffected.
>
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  .../devicetree/bindings/iommu/rockchip,iommu.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> index 6ce41d11f..a3cedcaaa 100644
> --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> @@ -42,14 +42,22 @@ properties:
>      minItems: 1
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: Core clock
>        - description: Interface clock
> +      - description: Compute clock, RK3576 NPU MMUs only
> +      - description: Convolution buffer core clock, RK3576 NPU MMUs only
> +      - description: Convolution buffer interface clock, RK3576 NPU MMUs only

Drop the ", RK3576 NPU MMUs only" part as it is not future proof, not
needed, not enforceable and not enforced.

IIUC, only a RK3576 NPU MMU can and should have 5 clocks, but a non-NPU
RK3576 MMU should only have 2 clocks, just like any MMU for RK3568 and
RK3588.
So you'd need a new compatible for RK3576 NPU MMU and enforce that only
that one has exactly 5 clocks, while all other compatibles are only
allowed to have 2 clocks.
 
Right now, it is allowed that a ``rockchip,rk3568-iommu`` compatible has
5 clocks while a RK3576 NPU MMU only has 2. Both are incorrect.

Cheers,
  Diederik

>    clock-names:
> +    minItems: 2
>      items:
>        - const: aclk
>        - const: iface
> +      - const: npu
> +      - const: aclk_cbuf
> +      - const: hclk_cbuf
>  
>    "#iommu-cells":
>      const: 0

Re: [PATCH v7 04/10] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set

[Jiaxing Hu]

Hi Diederik,

You are right, and worse than that, this is the same thing you asked
for on v6 and I told you it was fixed.

My reply then said v7 would carry a compatible of its own and pin each
side with an allOf. It does not. What v7 actually contains is a
minItems of 2 and three descriptions ending in "RK3576 NPU MMUs only",
which is a comment, not a schema, and it leaves both of the cases you
name allowed: an rk3568-iommu with five clocks, and an RK3576 NPU MMU
with two. I do not have an explanation for the gap between what I said
and what I sent, only the fix.

v8 does what you and the bot asked for the first time. The MMU nodes
get their own compatible,

    compatible = "rockchip,rk3576-npu-iommu", "rockchip,rk3568-iommu";

the enum gains it, and an allOf pins both sides so neither can borrow
the other's clock set:

    allOf:
      - if:
          properties:
            compatible:
              contains:
                const: rockchip,rk3576-npu-iommu
        then:
          properties:
            clocks:
              minItems: 5
              maxItems: 5
            clock-names:
              items:
                - const: aclk
                - const: iface
                - const: npu
                - const: aclk_cbuf
                - const: hclk_cbuf
        else:
          properties:
            clocks:
              maxItems: 2
            clock-names:
              maxItems: 2

so every existing devicetree keeps exactly two clocks and only the new
compatible may have five. The ", RK3576 NPU MMUs only" wording goes
away with it, since the schema then says it.

The DTS patch changes with it, since v7's NPU MMU nodes use the plain
rockchip,rk3576-iommu string.

I will not claim it is fixed this time until the patch is in front of
you.

Thanks for catching it twice,
Jiaxing

[PATCH v7 05/10] 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 answer. Without
it rockchip_pmu_restore_qos() reads back zeroes, and the NPU throws an
async SError on the first cold power-on.

Give rockchip_domain_info an optional delay_us and wait for it between
releasing idle and restoring QoS. Rename DOMAIN_M_O_R_G to
DOMAIN_M_O_R_G_W, since the suffixes name the fields the macro sets and
this one now also carries a wakeup delay; RK3576 is its only user, so
the old spelling is not kept around.

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

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index ba66ae719..e1857f878 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -18,6 +18,7 @@
 #include <linux/of_address.h>
 #include <linux/of_clk.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -59,6 +60,7 @@ struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -185,7 +187,7 @@ struct rockchip_pmu {
 	.need_regulator = regulator,			\
 }
 
-#define DOMAIN_M_O_R_G(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, wakeup)	\
+#define DOMAIN_M_O_R_G_W(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, delay, wakeup)	\
 {							\
 	.name = _name,					\
 	.pwr_offset = p_offset,				\
@@ -200,6 +202,7 @@ struct rockchip_pmu {
 	.req_mask = (req),				\
 	.idle_mask = (idle),				\
 	.clk_ungate_mask = (g_mask),			\
+	.delay_us = (delay),				\
 	.ack_mask = (ack),				\
 	.active_wakeup = wakeup,			\
 }
@@ -258,8 +261,8 @@ struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup, regulator)		\
 	DOMAIN_M_R(name, pwr, pwr, req, req, req, wakeup, regulator)
 
-#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, wakeup)	\
-	DOMAIN_M_O_R_G(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, wakeup)
+#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, delay, wakeup)	\
+	DOMAIN_M_O_R_G_W(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, delay, wakeup)
 
 /*
  * Dynamic Memory Controller may need to coordinate with us -- see
@@ -681,6 +684,10 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (ret < 0)
 			goto out;
 
+		/* Some domains need to settle before the QoS registers answer. */
+		if (pd->info->delay_us)
+			udelay(pd->info->delay_us);
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -1300,25 +1307,26 @@ static const struct rockchip_domain_info rk3568_pm_domains[] = {
 };
 
 static const struct rockchip_domain_info rk3576_pm_domains[] = {
-	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       false),
-	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  false),
-	[RK3576_PD_SDGMAC]	= DOMAIN_RK3576("sdgmac", 0x0, BIT(7),  0,      BIT(7),  0x4, BIT(1),  BIT(17), 0x6,     false),
-	[RK3576_PD_AUDIO]	= DOMAIN_RK3576("audio",  0x0, BIT(8),  0,      BIT(8),  0x4, BIT(0),  BIT(16), BIT(0),  false),
-	[RK3576_PD_PHP]		= DOMAIN_RK3576("php",    0x0, BIT(9),  0,      BIT(9),  0x0, BIT(15), BIT(15), BIT(15), false),
-	[RK3576_PD_SUBPHP]	= DOMAIN_RK3576("subphp", 0x0, BIT(10), 0,      BIT(10), 0x0, 0,       0,       0,       false),
-	[RK3576_PD_VOP]		= DOMAIN_RK3576("vop",    0x0, BIT(11), 0,      BIT(11), 0x0, 0x6000,  0x6000,  0x6000,  false),
-	[RK3576_PD_VO1]		= DOMAIN_RK3576("vo1",    0x0, BIT(14), 0,      BIT(14), 0x0, BIT(12), BIT(12), 0x7000,  false),
-	[RK3576_PD_VO0]		= DOMAIN_RK3576("vo0",    0x0, BIT(15), 0,      BIT(15), 0x0, BIT(11), BIT(11), 0x6800,  false),
-	[RK3576_PD_USB]		= DOMAIN_RK3576("usb",    0x4, BIT(0),  0,      BIT(16), 0x0, BIT(10), BIT(10), 0x6400,  true),
-	[RK3576_PD_VI]		= DOMAIN_RK3576("vi",     0x4, BIT(1),  0,      BIT(17), 0x0, BIT(9),  BIT(9),  BIT(9),  false),
-	[RK3576_PD_VEPU0]	= DOMAIN_RK3576("vepu0",  0x4, BIT(2),  0,      BIT(18), 0x0, BIT(7),  BIT(7),  0x280,   false),
-	[RK3576_PD_VEPU1]	= DOMAIN_RK3576("vepu1",  0x4, BIT(3),  0,      BIT(19), 0x0, BIT(8),  BIT(8),  BIT(8),  false),
-	[RK3576_PD_VDEC]	= DOMAIN_RK3576("vdec",   0x4, BIT(4),  0,      BIT(20), 0x0, BIT(6),  BIT(6),  BIT(6),  false),
-	[RK3576_PD_VPU]		= DOMAIN_RK3576("vpu",    0x4, BIT(5),  0,      BIT(21), 0x0, BIT(5),  BIT(5),  BIT(5),  false),
-	[RK3576_PD_NPUTOP]	= DOMAIN_RK3576("nputop", 0x4, BIT(6),  0,      BIT(22), 0x0, 0x18,    0x18,    0x18,    false),
-	[RK3576_PD_NPU0]	= DOMAIN_RK3576("npu0",   0x4, BIT(7),  0,      BIT(23), 0x0, BIT(1),  BIT(1),  0x1a,    false),
-	[RK3576_PD_NPU1]	= DOMAIN_RK3576("npu1",   0x4, BIT(8),  0,      BIT(24), 0x0, BIT(2),  BIT(2),  0x1c,    false),
-	[RK3576_PD_GPU]		= DOMAIN_RK3576("gpu",    0x4, BIT(9),  0,      BIT(25), 0x0, BIT(0),  BIT(0),  BIT(0),  false),
+	/*                                            name    p_offset pwr      status  r_status r_offset req      idle     g_mask   delay wakeup */
+	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  0,    false),
+	[RK3576_PD_SDGMAC]	= DOMAIN_RK3576("sdgmac", 0x0, BIT(7),  0,      BIT(7),  0x4, BIT(1),  BIT(17), 0x6,     0,    false),
+	[RK3576_PD_AUDIO]	= DOMAIN_RK3576("audio",  0x0, BIT(8),  0,      BIT(8),  0x4, BIT(0),  BIT(16), BIT(0),  0,    false),
+	[RK3576_PD_PHP]		= DOMAIN_RK3576("php",    0x0, BIT(9),  0,      BIT(9),  0x0, BIT(15), BIT(15), BIT(15), 0,    false),
+	[RK3576_PD_SUBPHP]	= DOMAIN_RK3576("subphp", 0x0, BIT(10), 0,      BIT(10), 0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_VOP]		= DOMAIN_RK3576("vop",    0x0, BIT(11), 0,      BIT(11), 0x0, 0x6000,  0x6000,  0x6000,  0,    false),
+	[RK3576_PD_VO1]		= DOMAIN_RK3576("vo1",    0x0, BIT(14), 0,      BIT(14), 0x0, BIT(12), BIT(12), 0x7000,  0,    false),
+	[RK3576_PD_VO0]		= DOMAIN_RK3576("vo0",    0x0, BIT(15), 0,      BIT(15), 0x0, BIT(11), BIT(11), 0x6800,  0,    false),
+	[RK3576_PD_USB]		= DOMAIN_RK3576("usb",    0x4, BIT(0),  0,      BIT(16), 0x0, BIT(10), BIT(10), 0x6400,  0,    true),
+	[RK3576_PD_VI]		= DOMAIN_RK3576("vi",     0x4, BIT(1),  0,      BIT(17), 0x0, BIT(9),  BIT(9),  BIT(9),  0,    false),
+	[RK3576_PD_VEPU0]	= DOMAIN_RK3576("vepu0",  0x4, BIT(2),  0,      BIT(18), 0x0, BIT(7),  BIT(7),  0x280,   0,    false),
+	[RK3576_PD_VEPU1]	= DOMAIN_RK3576("vepu1",  0x4, BIT(3),  0,      BIT(19), 0x0, BIT(8),  BIT(8),  BIT(8),  0,    false),
+	[RK3576_PD_VDEC]	= DOMAIN_RK3576("vdec",   0x4, BIT(4),  0,      BIT(20), 0x0, BIT(6),  BIT(6),  BIT(6),  0,    false),
+	[RK3576_PD_VPU]		= DOMAIN_RK3576("vpu",    0x4, BIT(5),  0,      BIT(21), 0x0, BIT(5),  BIT(5),  BIT(5),  0,    false),
+	[RK3576_PD_NPUTOP]	= DOMAIN_RK3576("nputop", 0x4, BIT(6),  0,      BIT(22), 0x0, 0x18,    0x18,    0x18,    15,   false),
+	[RK3576_PD_NPU0]	= DOMAIN_RK3576("npu0",   0x4, BIT(7),  0,      BIT(23), 0x0, BIT(1),  BIT(1),  0x1a,    15,   false),
+	[RK3576_PD_NPU1]	= DOMAIN_RK3576("npu1",   0x4, BIT(8),  0,      BIT(24), 0x0, BIT(2),  BIT(2),  0x1c,    15,   false),
+	[RK3576_PD_GPU]		= DOMAIN_RK3576("gpu",    0x4, BIT(9),  0,      BIT(25), 0x0, BIT(0),  BIT(0),  BIT(0),  0,    false),
 };
 
 static const struct rockchip_domain_info rk3588_pm_domains[] = {
-- 
2.43.0

[PATCH v7 06/10] pmdomain/rockchip: cycle optional power-domain resets on power-on

[Jiaxing Hu]

Some Rockchip domains come out of power-on with their bus interface in
an undefined state. On the RK3576 NPU this shows up as a hang on the
first register access after the domain is switched on, and pulsing the
domain's resets at this point clears it.

Take the domain node's resets if it has any, and pulse them between
releasing idle and restoring QoS. The resets are optional, so domains
that do not list any are unaffected.

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

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index e1857f878..4eebb5d99 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -19,6 +19,7 @@
 #include <linux/of_clk.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/reset.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -103,6 +104,7 @@ struct rockchip_pm_domain {
 	struct clk_bulk_data *clks;
 	struct device_node *node;
 	struct regulator *supply;
+	struct reset_control *resets;
 };
 
 struct rockchip_pmu {
@@ -688,6 +690,13 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (pd->info->delay_us)
 			udelay(pd->info->delay_us);
 
+		/* Optional: some domains need their resets cycled after power-on. */
+		if (pd->resets) {
+			reset_control_assert(pd->resets);
+			udelay(10);
+			reset_control_deassert(pd->resets);
+		}
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -857,6 +866,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	if (error)
 		goto err_put_clocks;
 
+	pd->resets = of_reset_control_array_get_optional_exclusive(node);
+	if (IS_ERR(pd->resets)) {
+		error = dev_err_probe(pmu->dev, PTR_ERR(pd->resets),
+				      "%pOFn: failed to get resets\n", node);
+		pd->resets = NULL;
+		goto err_unprepare_clocks;
+	}
+
 	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
 						 NULL);
 
@@ -927,6 +944,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 err_put_clocks:
 	clk_bulk_put(pd->num_clks, pd->clks);
+	reset_control_put(pd->resets);
 	return error;
 }
 
@@ -945,6 +963,7 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
 
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 	clk_bulk_put(pd->num_clks, pd->clks);
+	reset_control_put(pd->resets);
 
 	/* protect the zeroing of pm->num_clks */
 	mutex_lock(&pd->pmu->mutex);
-- 
2.43.0

[PATCH v7 07/10] accel/rocket: select the per-core clock and reset counts from match data

[Jiaxing Hu]

The RK3576 carries the same RKNN block with a different set of clocks and
resets, so the counts cannot stay compile-time constants. Add a soc_data
struct to the of_device_id match data and take the bulk counts from it.
RK3588 keeps four clocks and two resets, so nothing changes for it, and
the arrays keep their present sizes: the SoC that needs a longer one
grows it in the patch that adds the names.

rocket_core_reset() is switched over as well. It is the same array, and
leaving it on ARRAY_SIZE() would walk entries that were never acquired
once a SoC asks for fewer.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c |  8 ++++----
 drivers/accel/rocket/rocket_core.h |  7 +++++++
 drivers/accel/rocket/rocket_drv.c  | 12 +++++++++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index 5dd260bac..b202d1581 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -23,7 +23,7 @@ int rocket_core_init(struct rocket_core *core)
 
 	core->resets[0].id = "srst_a";
 	core->resets[1].id = "srst_h";
-	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
 						    core->resets);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
@@ -32,7 +32,7 @@ int rocket_core_init(struct rocket_core *core)
 	core->clks[1].id = "hclk";
 	core->clks[2].id = "npu";
 	core->clks[3].id = "pclk";
-	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
+	err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
 
@@ -109,9 +109,9 @@ void rocket_core_fini(struct rocket_core *core)
 
 void rocket_core_reset(struct rocket_core *core)
 {
-	reset_control_bulk_assert(ARRAY_SIZE(core->resets), core->resets);
+	reset_control_bulk_assert(core->soc->num_resets, core->resets);
 
 	udelay(10);
 
-	reset_control_bulk_deassert(ARRAY_SIZE(core->resets), core->resets);
+	reset_control_bulk_deassert(core->soc->num_resets, core->resets);
 }
diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
index f6d738285..ba74c5339 100644
--- a/drivers/accel/rocket/rocket_core.h
+++ b/drivers/accel/rocket/rocket_core.h
@@ -27,9 +27,16 @@
 #define rocket_core_writel(core, reg, value) \
 	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
 
+/* Per-SoC differences, selected by the of_device_id match data. */
+struct rocket_soc_data {
+	unsigned int num_clks;		/* clk_bulk count */
+	unsigned int num_resets;	/* reset_bulk count */
+};
+
 struct rocket_core {
 	struct device *dev;
 	struct rocket_device *rdev;
+	const struct rocket_soc_data *soc;
 	unsigned int index;
 
 	int irq;
diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 8bbbce594..6e7dc91c5 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
 
 	rdev->cores[core].rdev = rdev;
 	rdev->cores[core].dev = &pdev->dev;
+	rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
 	rdev->cores[core].index = core;
 
 	rdev->num_cores++;
@@ -213,8 +214,13 @@ static void rocket_remove(struct platform_device *pdev)
 	}
 }
 
+static const struct rocket_soc_data rk3588_soc_data = {
+	.num_clks = 4,
+	.num_resets = 2,
+};
+
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "rockchip,rk3588-rknn-core" },
+	{ .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
@@ -240,7 +246,7 @@ static int rocket_device_runtime_resume(struct device *dev)
 	if (core < 0)
 		return -ENODEV;
 
-	err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+	err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
 	if (err) {
 		dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
 		return err;
@@ -260,7 +266,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
 	if (!rocket_job_is_idle(&rdev->cores[core]))
 		return -EBUSY;
 
-	clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+	clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

The RK3576 has two cores of the same RKNN block and a few platform
differences:

 - the CBUF (convolution buffer) has its own clock domain, so the core
   needs six clocks rather than four;
 - the BIU reset moved into the power domain, leaving one reset here;
 - the NPU spans two power domains, and a device with more than one is
   skipped by the driver-core single-domain auto-attach, so the list has
   to be attached explicitly;
 - PC_TASK_CON packs the task number with sixteen bits rather than
   twelve, moving the three controls above it up by four.

That last one is the reason this series has been reporting, since v3,
that the block accepts exactly one task per reset. rocket_registers.h is
generated from the RK3588 description, so writing it unchanged to an
RK3576 asks for task_number 0x7001, which is 28673 tasks, and puts
TASK_COUNT_CLEAR on a bit that does nothing. The counter is then only
ever cleared by a reset.

The layout was confirmed by Chaoyi Chen of Rockchip, including a fourth
control at BIT(18), task_last_layer_clear, which belongs on every submit
alongside the count clear:

  https://lore.kernel.org/all/4f300b78-d96d-4d98-8819-dc292b0c9b97@rock-chips.com/

With that written correctly a job of several tasks runs to completion,
the completion interrupt arrives, and /proc/interrupts counts up. A
convolution submitted three times with three different inputs is byte
exact against the CPU reference each time, with no reset in between and
with nothing retiring the job but the interrupt.

All of it hangs off the soc_data added earlier, so the RK3588 path keeps
its existing counts and behaviour.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c   | 20 ++++++++
 drivers/accel/rocket/rocket_core.h   |  8 +--
 drivers/accel/rocket/rocket_device.c |  4 ++
 drivers/accel/rocket/rocket_drv.c    | 10 ++++
 drivers/accel/rocket/rocket_job.c    | 76 ++++++++++++++++++++++------
 5 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b202d1581..5f3155135 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 
@@ -21,6 +22,7 @@ int rocket_core_init(struct rocket_core *core)
 	u32 version;
 	int err = 0;
 
+	/* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
 	core->resets[0].id = "srst_a";
 	core->resets[1].id = "srst_h";
 	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
@@ -32,6 +34,9 @@ int rocket_core_init(struct rocket_core *core)
 	core->clks[1].id = "hclk";
 	core->clks[2].id = "npu";
 	core->clks[3].id = "pclk";
+	/* RK3576 clocks the CBUF separately; the compute path stalls without these. */
+	core->clks[4].id = "aclk_cbuf";
+	core->clks[5].id = "hclk_cbuf";
 	err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
@@ -60,6 +65,21 @@ int rocket_core_init(struct rocket_core *core)
 	if (err)
 		return err;
 
+	/*
+	 * RK3576 spans two power domains, and a multi-domain device is skipped
+	 * by the driver-core single-domain auto-attach, so attach the list here.
+	 * This goes before the first thing that would have to be unwound, so a
+	 * failure can simply return.
+	 */
+	if (core->soc->multi_power_domain) {
+		struct dev_pm_domain_list *pd_list;
+
+		err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
+		if (err < 0)
+			return dev_err_probe(dev, err,
+					     "failed to attach NPU power domains\n");
+	}
+
 	core->iommu_group = iommu_group_get(dev);
 
 	err = rocket_job_init(core);
diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
index ba74c5339..8c8d1f453 100644
--- a/drivers/accel/rocket/rocket_core.h
+++ b/drivers/accel/rocket/rocket_core.h
@@ -29,8 +29,10 @@
 
 /* Per-SoC differences, selected by the of_device_id match data. */
 struct rocket_soc_data {
-	unsigned int num_clks;		/* clk_bulk count */
-	unsigned int num_resets;	/* reset_bulk count */
+	unsigned int num_clks;		/* clk_bulk count: 4 base, 6 with CBUF */
+	unsigned int num_resets;	/* reset_bulk count: 2 base, 1 on RK3576 */
+	bool multi_power_domain;	/* device spans more than one PM domain */
+	bool task_con_16bit;		/* PC_TASK_CON uses the 16-bit task number */
 };
 
 struct rocket_core {
@@ -43,7 +45,7 @@ struct rocket_core {
 	void __iomem *pc_iomem;
 	void __iomem *cna_iomem;
 	void __iomem *core_iomem;
-	struct clk_bulk_data clks[4];
+	struct clk_bulk_data clks[6];
 	struct reset_control_bulk_data resets[2];
 
 	struct iommu_group *iommu_group;
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 6e7dc91c5..f333fe466 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -217,10 +217,20 @@ static void rocket_remove(struct platform_device *pdev)
 static const struct rocket_soc_data rk3588_soc_data = {
 	.num_clks = 4,
 	.num_resets = 2,
+	.multi_power_domain = false,
+	.task_con_16bit = false,
+};
+
+static const struct rocket_soc_data rk3576_soc_data = {
+	.num_clks = 6,
+	.num_resets = 1,
+	.multi_power_domain = true,
+	.task_con_16bit = true,
 };
 
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
+	{ .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 4c01b703e..493b3bf97 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -21,6 +21,35 @@
 
 #define JOB_TIMEOUT_MS 500
 
+/*
+ * RK3576 arms the same DPU completion as RK3588, but the interrupt never
+ * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
+ * so sample that instead. The tick cap bounds jobs that never raise it at all,
+ * which is the same open problem as the wrong inference results.
+ */
+/*
+ * PC_TASK_CON packs the task number with three controls, and the field widths
+ * are not the same on every SoC. rocket_registers.h is generated from the
+ * RK3588 description, where the task number is twelve bits:
+ *
+ *   RK3588   BIT[11:0] task_number, BIT[12] pp_en, BIT[13] count_clear
+ *   RK3576   BIT[15:0] task_number, BIT[16] pp_en, BIT[17] count_clear,
+ *            BIT[18] last_layer_clear
+ *
+ * The RK3576 layout was confirmed by Chaoyi Chen of Rockchip:
+ * https://lore.kernel.org/all/4f300b78-d96d-4d98-8819-dc292b0c9b97@rock-chips.com/
+ *
+ * Writing the RK3588 layout to an RK3576 therefore asks for task_number
+ * 0x7001, that is 28673 tasks, and lands the count clear on a bit that does
+ * nothing. The task counter is then only ever cleared by a reset, which is
+ * exactly the "one task per reset" behaviour this series has been reporting
+ * since v3.
+ */
+#define RK3576_PC_TASK_CON_TASK_NUMBER(n)	((n) & 0xffff)
+#define RK3576_PC_TASK_CON_PP_EN		BIT(16)
+#define RK3576_PC_TASK_CON_COUNT_CLEAR		BIT(17)
+#define RK3576_PC_TASK_CON_LAST_LAYER_CLEAR	BIT(18)
+
 static struct rocket_job *
 to_rocket_job(struct drm_sched_job *sched_job)
 {
@@ -142,10 +171,17 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 	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);
 
-	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
-					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
-					 PC_TASK_CON_TASK_NUMBER(1) |
-					 PC_TASK_CON_TASK_PP_EN(1));
+	if (core->soc->task_con_16bit)
+		rocket_pc_writel(core, TASK_CON,
+				 RK3576_PC_TASK_CON_LAST_LAYER_CLEAR |
+				 RK3576_PC_TASK_CON_COUNT_CLEAR |
+				 RK3576_PC_TASK_CON_PP_EN |
+				 RK3576_PC_TASK_CON_TASK_NUMBER(1));
+	else
+		rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
+						 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
+						 PC_TASK_CON_TASK_NUMBER(1) |
+						 PC_TASK_CON_TASK_PP_EN(1));
 
 	rocket_pc_writel(core, TASK_DMA_BASE_ADDR, PC_TASK_DMA_BASE_ADDR_DMA_BASE_ADDR(0x0));
 
@@ -341,6 +377,25 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
 	return ERR_PTR(ret);
 }
 
+/* Start the job's next task, or retire it. Caller holds job_lock. */
+static void rocket_job_next_locked(struct rocket_core *core)
+{
+	lockdep_assert_held(&core->job_lock);
+
+	if (!core->in_flight_job)
+		return;
+
+	if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
+		rocket_job_hw_submit(core, core->in_flight_job);
+		return;
+	}
+
+	iommu_detach_group(NULL, iommu_group_get(core->dev));
+	dma_fence_signal(core->in_flight_job->done_fence);
+	pm_runtime_put_autosuspend(core->dev);
+	core->in_flight_job = NULL;
+}
+
 static void rocket_job_handle_irq(struct rocket_core *core)
 {
 	pm_runtime_mark_last_busy(core->dev);
@@ -354,17 +409,7 @@ static void rocket_job_handle_irq(struct rocket_core *core)
 		rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
 		rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
 
-		if (core->in_flight_job) {
-			if (core->in_flight_job->next_task_idx < core->in_flight_job->task_count) {
-				rocket_job_hw_submit(core, core->in_flight_job);
-				return;
-			}
-
-			iommu_detach_group(NULL, iommu_group_get(core->dev));
-			dma_fence_signal(core->in_flight_job->done_fence);
-			pm_runtime_put_autosuspend(core->dev);
-			core->in_flight_job = NULL;
-		}
+		rocket_job_next_locked(core);
 	}
 }
 
@@ -644,7 +689,6 @@ int rocket_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *fil
 		}
 	}
 
-
 	for (i = 0; i < args->job_count; i++)
 		rocket_ioctl_submit_job(dev, file, &jobs[i]);
 
-- 
2.43.0

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Two things here, one of which I think has to be fixed before this
lands.

The first is a comment that outlived its subject. This patch adds the
following just above the PC_TASK_CON block:

  /*
   * RK3576 arms the same DPU completion as RK3588, but the interrupt
   * never reaches the GIC. The completion itself is visible in
   * INTERRUPT_RAW_STATUS, so sample that instead. The tick cap bounds
   * jobs that never raise it at all, which is the same open problem as
   * the wrong inference results.
   */

That is the v6 comment for the poll. It states the premise your cover
letter withdraws, it describes machinery this version deletes, and it
has no code under it - the next line opens the second comment block.
Left in, the driver would carry a claim that contradicts both the
commit introducing it and the register description two paragraphs
below it.

The second is placement rather than correctness. This patch also
factors the completion tail out of rocket_job_handle_irq() into
rocket_job_next_locked(). I read that as behaviour-neutral on RK3588 -
the return that used to leave handle_irq() now leaves the helper, and
scoped_guard drops the lock either way - and the numbers I posted on
1/10 bear it out. But it restructures the shared completion path in a
patch whose subject is adding RK3576, which puts a bisect in the wrong
place if it ever turns out not to be neutral. It would sit more
naturally in 1/10, which already touches that function, or in a patch
of its own.

Both of the things I raised on v6 are right in this version. The power
domain list is attached before anything that would have to be unwound,
and the comment saying why a plain return is correct there is a good
addition. clks[] grows in the same patch that adds the two names.

I also went looking for an ARRAY_SIZE(core->clks) or
ARRAY_SIZE(core->resets) left behind, since that would walk six entries
on a four-clock RK3588. All six are converted in 7/10, including the
two in rocket_drv.c's runtime PM callbacks, which are the easiest pair
to miss.

Igor

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

Thank you for the run, and for reading the patch rather than only
testing it. Both of your points are right and both are fixed for v8.
I am answering your 1/10 question here as well so it stays in one
place.

> That is the v6 comment for the poll.

Yes, and it should not have shipped. The cover letter withdraws the
premise that comment states, the patch under it deletes the machinery
it describes, and it has no code beneath it at all. It is gone in v8.

For the record on how it survived: it was fixed in my tree the day the
correction went to the list and the fix never made it into the series I
formatted. That is the second time a fix has existed here and not
reached what I sent, so I now diff the posted patches against the tree
before sending rather than trusting that they match.

> It would sit more naturally in 1/10, which already touches that
> function, or in a patch of its own.

A patch of its own, placed after 1/10 rather than before it. 1/10 is a
fix with a Fixes tag that someone may want to backport, and it should
stay the smallest thing that fixes the bug. Refactoring the function
first would put the backport on top of a restructure it does not need.
So v8 is 1/10 unchanged, then the extraction on its own, then the
RK3576 patch with no shared-path changes left in it.

> would a synchronize_irq(core->irq) before the guard in
> rocket_reset() be worth having as well?

I think yes, and before the guard is the only place it can go. The
handler takes job_lock, so calling it inside the scoped_guard would
wait for a handler that is waiting for the lock we hold. Before the
guard nothing is held, and both callers, the timedout_job callback and
reset_work, are process context, so it is safe there.

It also closes exactly the window you describe rather than a different
one. drm_sched_stop() stops the scheduler and returns; a threaded
handler already running is untouched by it, and the comment sitting
above that code says "Remaining interrupts have been handled", which is
the assumption your reading breaks. synchronize_irq() makes that
sentence true instead of hopeful.

What it does not do is stop a handler that has already read
in_flight_job from finishing its work on a job the reset is about to
drop. That one wants the check and the write to be one step under the
lock, which is what 1/10 does. The two changes are complementary and I
will send them as such, with the comment reworded to say what is
actually guaranteed.

Your RK3588 numbers are also the only evidence anyone has that 1/10
costs nothing on the path it protects, since I cannot run three cores
here. Carrying the tag to v8:

  Tested-by: Igor Paunovic <royalnet026@gmail.com> # RK3588, three cores

One piece of news from the userspace side, since you run MobileNet
through Teflon yourself. As of today the whole of MobileNet V1 runs on
the RK3576 with the open stack: 995 of its 1001 outputs land within one
count of the CPU reference, against 1001 channels of zero in every run
before this. The kernel side of that is this series unchanged; what
moved was four Mesa faults, the last of which was a coefficient buffer
whose second operand has to be 16 byte aligned, which is why every
layer whose output channel count was not a multiple of eight came back
empty.

Thanks again,
Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

Two short things.

On synchronize_irq(): your placement is right and my question was not
thought through that far. Inside the guard it would wait for a handler
that is itself waiting for job_lock, which is a deadlock rather than a
fence. Before it, with nothing held and both callers in process
context, is the only place it works. I had the window right and the
location wrong.

I will run v8 on the three cores here when it is out, and I will drive
the reset path deliberately rather than wait for a timeout to happen on
its own, since that is the path the two changes are there for.

On MobileNet: 995 of 1001 within one count is a different kind of
number from what this series has been reporting, and it took four Mesa
faults to get there. That is worth saying out loud.

If it would help to know whether the remaining six are RK3576 specific
or common to the stack, I can run the same comparison on RK3588. I
already run MobileNet V1 through the Teflon delegate here, but my
oracle is bit-exactness across repeated runs rather than a per-output
comparison against the CPU, so it would not have noticed six outputs
being off by more than a count. Say the word and I will point it at the
CPU reference the way you did.

Igor

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

> I had the window right and the location wrong.

That is the useful half. v8 carries synchronize_irq(core->irq) before the
guard in rocket_reset(), with the comment above it saying what is
actually guaranteed rather than "Remaining interrupts have been handled".
Driving the reset path deliberately rather than waiting for a timeout is
worth more than the rest of the run put together, since that is the only
path either change is for.

> Say the word and I will point it at the CPU reference the way you did.

Please do, and thank you. It is the one comparison I cannot produce, and
it separates two things that look identical from here: a defect specific
to this SoC, and the reference's own rounding compounding through a
chain.

Two things before you spend time on it.

The number moved. When I wrote 995 of 1001 there was still one fault
left in Mesa, an output channel count that is not a multiple of two,
which the CNA reads in pairs. With that fixed it is 1000 of 1001, so it
is one output rather than six, and whether one output is even worth
chasing is a fair question. The comparison is still worth having for the
LAYERS rather than the final vector.

And the oracle matters more than the run. A per output comparison
against the CPU is not enough on its own past the first layer or two,
because tflite's requant and the hardware's disagree by design and that
disagreement compounds: at operator 6 a flawless accelerator scores 4 of
128 channels against the CPU. vendor-capture/chainmodel.py in

  https://github.com/gahingwoo/linux-rk3576-npu

runs the graph twice from the model file, once with tflite's
SaturatingRoundingDoublingHighMul and RoundingDivideByPOT and once with
the hardware's single half up shift, and prints what a perfect
accelerator would score at every operator. Read your numbers against
that column rather than against 128 of 128, or every deep layer will
look broken on both SoCs.

If it is easier, mn_L00 through mn_L26 in that repository are MobileNet
with its graph output moved to each operator's output, which is a four
byte patch of the flatbuffer and needs no converter. Those are what the
per layer table came from.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

Here is the RK3588 column, all 27 operators, ROCKET_SEED=7, scored the
way perch.py scores: a channel is good when its maxdiff (md below)
against max(cpu, output zero point) is at most 1.

Setup: Orange Pi 5 Plus (RK3588), kernel 7.2.0-rc6, the rocket driver
from this kernel's tree rebuilt with my clocks-by-name and devfreq
patches on top, Mesa at bf70ab68a21, teflon delegate, model
mobilenet_v1_1_224_quant.tflite from the Mesa test suite
(md5 4f348b87dca3315d2b3646cf5a3b31cf), per-operator models generated
with the four byte output patch you described. The "correct hw" column
is your chainmodel.py against the same model file. One difference to
flag up front: against this model file chainmodel prints 36/256 for
operator 8 where your table has 34/256, so our model files are not
byte-identical, and the columns below should be read against each
other rather than against your FINDINGS numbers.

  op  kind        correct hw       RK3588
  0   conv         32/32   md 1     32/32   md 1
  1   depthwise    28/32   md 3     28/32   md 3
  2   1x1          22/64   md 6     22/64   md 6
  3   depthwise    21/64   md 13    21/64   md 13
  4   1x1          18/128  md 14     8/128  md 15
  5   depthwise     9/128  md 13     3/128  md 15
  6   1x1           4/128  md 23     1/128  md 24
  7   depthwise     7/128  md 10     3/128  md 12
  8   1x1          36/256  md 7     20/256  md 17
  9   depthwise    32/256  md 11    22/256  md 13
  10  1x1          29/256  md 9     28/256  md 10
  11  depthwise    53/256  md 8     48/256  md 11
  12  1x1         166/512  md 11   121/512  md 13
  13  depthwise   142/512  md 9    137/512  md 12
  14  1x1          82/512  md 7     72/512  md 10
  15  depthwise   154/512  md 10   115/512  md 15
  16  1x1          82/512  md 7     71/512  md 10
  17  depthwise   156/512  md 14   141/512  md 21
  18  1x1          92/512  md 7     77/512  md 10
  19  depthwise   170/512  md 7    152/512  md 12
  20  1x1         102/512  md 8     84/512  md 10
  21  depthwise   166/512  md 13   150/512  md 12
  22  1x1         174/512  md 6    161/512  md 7
  23  depthwise   293/512  md 5    261/512  md 7
  24  1x1         671/1024 md 6    636/1024 md 5
  25  depthwise   718/1024 md 9    684/1024 md 8
  26  1x1         572/1024 md 25   574/1024 md 23

Three things stand out from here.

Operators 0 through 3 score identically to your chain simulation --
same good-channel counts, same maxdiff -- including operator 3, the
stride 2 depthwise with the asymmetric padding you suspect for the
first RK3576 divergence. They are not byte-identical to the simulated
hardware: diffing the raw tensors against the requant_hw chain shows a
few hundred elements per surface already off by 1-4 at operators 0-3.
That looks like the same small extra rounding difference that pushes
the scores below your column from operator 4 on; through operator 3 it
just stays under the maxdiff <= 1 scoring threshold.

There is no md 255 anywhere. From operator 4 on, RK3588 sits somewhat
below the simulation (8 vs 18 at op 4, 1 vs 4 at op 6), but the
maxdiff never exceeds 24 across all 27 operators and the deep layers
track the simulation closely (574 vs 572 at op 26).

A control run with ROCKET_SEED=11 keeps the same character: operator 0
still 32/32, no saturated maxdiff anywhere, worst case md 35 at
operator 6.

So from the RK3588 side your read looks right: the deep-layer
compounding is the reference artifact, and the RK3576 collapse from
operator 4 with maxdiff 255 has no counterpart here.

Igor

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

Thank you. This is the column I cannot produce, and it settles the
question it was aimed at.

First, what you were comparing against has moved. The RK3576 collapse
from operator 4 with maxdiff 255 is gone; four Mesa faults came out
between that mail and yours. Against the same simulation RK3576 now
reads 21/64 md 13, 18/128 md 14, 9/128 md 13, 4/128 md 23 and 7/128
md 10 at operators 3 to 7, which is the simulation exactly, and
MobileNet end to end is 1000 of 1001 outputs within one count of the
CPU. So the thing your run was built to characterise no longer exists,
and your table is measuring something else.

That something is worth a note, and I do not have an explanation for it.
From operator 4 on RK3588 sits below the simulation where RK3576 now
sits on it, 8 of 128 against 18 at operator 4 and 20 of 256 against 36
at operator 8, with a few hundred elements per surface already off by 1
to 4 at operators 0 to 3.

My first thought was that this is my Mesa tree rather than your silicon,
since you ran upstream and four fixes are not posted yet. Checking it
before writing it: three of those four are not gated on the SoC, but
none of them bite at MobileNet's channel counts.

  the CBUF row cost fires when ceil(ic/16) is 3 modulo 4, which is 33 to
  48 or 97 to 112 input channels. MobileNet has 3, 32, 64, 128, 256,
  512 and 1024, so 1, 2, 4, 8, 16, 32 and 64 atoms. Never 3.

  the output channel pair rounding fires on an odd count, and the
  coefficient operand alignment on a count that is not a multiple of 8.
  In this model that is only operator 28, the 1001 channel classifier,
  which is past the end of your table.

  the fourth, the tiled 1x1 weight layout, is inside the RK3576 path.

So my hypothesis does not survive its own arithmetic and I am not going
to send it as one. Operators 4 to 26 on your board are unexplained by
anything I have, which means either RK3588's own path carries a rounding
difference RK3576 does not, or my simulation is closer to RK3576 than to
the hardware in general because that is the chip I tuned it against. The
second is the more likely and the less flattering.

If you ever want to close it, the cheapest probe is operator 28 rather
than any of the ones you ran: 1001 output channels is odd AND not a
multiple of 8, so it is the one operator in this model where two of my
unposted fixes would change anything on RK3588. On RK3576 without them
it came back an empty convolution.

One more thing, since it cuts against my own earlier note: operator 3
scoring identically on both chips retires the suspicion in my round 104
write-up that its asymmetric stride 2 padding was the first RK3576
divergence. It was not, on either SoC.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

Here is operator 28 on RK3588, and it changes the picture: it is not an
empty convolution here, and once the reference is right it is the
cleanest operator in the whole model.

Setup as before - same board, same upstream Mesa at bf70ab68a21, none of
your four unposted fixes, ROCKET_SEED=7, model truncated the same way as
the table (subgraph output repointed to the operator's output tensor,
here the 1x1x1001 BiasAdd). One change to disclose: the kernel moved
from 7.2.0-rc6 to 7.2.0-rc7 as my daily since the table. The rc6 run also
had my devfreq patches in the rocket driver; this rc7 rocket has no
devfreq node registered, so it runs like yours does. Before running
operator 28 I re-ran operators 0-4 as a control and all five reproduce
the published column exactly (32/32 md 1, 28/32 md 3, 22/64 md 6, 21/64
md 13, 8/128 md 15), so neither the kernel change nor the devfreq
difference moves these numbers.

Scored exactly the way perch.py scores, operator 28 reads:

  674/1001 channels match, maxdiff 29
  NOT empty: only 36 of 1001 channels sit at the output zero point,
  71 distinct output values, min 37, max 117 (zp = 66)

But that maxdiff 29 is the reference, not the hardware. perch.py scores
against max(cpu, zp), which is right for operators 0-26 with their fused
ReLU6 - the hardware really does floor those at the zero point. Operator
28 has no fused activation (it is the logits BiasAdd), and RK3588 does
not floor it: the raw CPU output has 362 channels below the zero point
(down to 37) and the NPU follows them down instead of clamping.

Against the raw, unclamped CPU output:

  seed 7: 1001/1001 channels within 1, maxdiff 1 (708 exact, 293 off
          by one)
  seed 8: 1001/1001 within 1, maxdiff 1
  three seed-7 runs byte-identical

So on RK3588 with upstream Mesa, 1001 output channels - odd AND not a
multiple of 8 - come out clean without either of your unposted fixes.
Whatever those two fixes change, this SoC's path does not need it for
correctness at this operator. It may be worth re-scoring your RK3576
operator 28 against the unclamped CPU as well before reading its "empty
convolution" - if RK3576 also skips the floor on a no-activation conv,
the max(cpu, zp) reference alone moves several hundred channels.

Two small notes for reproduction: perch.py prints the headline and then
crashes on this operator at its spatial-profile section (got[1:-1,1:-1]
on a 1x1 surface is empty) - the numbers above it are unaffected. And I
find it a nice detail that my board reads md 23 at operator 26, yet
after the CPU average-pool the classifier lands within 1 of the CPU -
the pooling averages the deep-layer noise away, which fits your 1000 of
1001 end-to-end result.

This reply was prepared with the help of Claude (Anthropic). The board,
the runs and the numbers are mine, and each number above comes from a
run on this machine today.

Best regards,
Igor

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

Thank you for running it, and for the unclamped reference.

Your 1001 does not discriminate between the two forms, which I should have said
when I described them. DIV_ROUND_UP(1001,16) is 63, odd, so the parity form
gives 0x80011011 there and so does oc % 32. They differ only where a count is a
multiple of neither 32 nor 16. On RK3576 the failing one was 56, and 88 or 120
would do as well. So your clean result rules nothing out, and I am not reading
it as support either way.

On my side that register turned out to be the whole of it. A 1x1 layer with 56
output channels had timed out in every round it ever ran, 32 of 56, and with
the parity form it is 56 of 56 every channel correct, with the old form
reproducing the timeout in the same log.

You are right about perch.py and there were three of them rather than one. The
inner maxdiff, the interior statistics and the raw row dump all assume a
surface with an interior, and all three raise on a 1x1x1001 classifier after
the useful lines have printed. Fixed here, and checked on the host at 1x1x1001,
5x5x8 and 56x56x64 before flashing, which is a check I should have been running
all along.

And I will re-score operator 28 against the unclamped output. The max(cpu, zp)
reference does move channels that are not wrong, and reading an empty
convolution off it is the kind of mistake I would rather not make twice.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

I ran the discriminating counts on RK3588. Details below, but first a
source-level observation that changes what my result can tell you.

Neither mesa-25.3.0 nor current main contains the modulo form in
rkt_regcmd.c. In both, BS_OW_CFG (0x4050) is emitted conditionally on
operation->depthwise only, with no dependence on output channel count:

    if (operation->depthwise) {
       EMIT(REG_DPU_BS_OW_CFG, DPU_BS_OW_CFG_SIZE_E_2(3) |
                               DPU_BS_OW_CFG_SIZE_E_1(3) |
                               DPU_BS_OW_CFG_SIZE_E_0(3));   /* 0x36c */
    } else {
       EMIT(REG_DPU_BS_OW_CFG, DPU_BS_OW_CFG_SIZE_E_2(1) |
                               DPU_BS_OW_CFG_SIZE_E_1(1) |
                               DPU_BS_OW_CFG_SIZE_E_0(1));   /* 0x124 */
    }

(mesa-25.3.0, src/gallium/drivers/rocket/rkt_regcmd.c around line 234;
unchanged in main as of 26.1-branchpoint.)

So I take it the (oc % 32) expression is from one of your 102 commits,
fitted from vendor captures? That would also explain the extra bits in
your two constants: decoded against registers.xml, 0x80011111 vs
0x80011011 differ only in bit 8, which is the LSB of SIZE_E_2 (bits
8-10). Your working-at-56 form has SIZE_E_2=1 and the failing form has
SIZE_E_2=0 - and upstream's non-depthwise value always has SIZE_E_2=1.
The three data points are consistent with SIZE_E_2 being what matters,
rather than the modulo-vs-parity predicate as such. Your values also
carry RGP_CNTER=8 (bit 31), OW_SRC=1 (bit 0) and bits 12/16 in the
reserved range, which upstream never sets; if those came from a vendor
capture it may be worth checking which of them RK3576 actually needs.

Now the measurement. My runs are with upstream Mesa (main,
26.1-branchpoint-5505, clean checkout), i.e. the constant 0x124 form,
on RK3588 with the in-tree rocket driver (7.2-rc7, rockchip-devel
base). Standalone minimal models: CONV_2D 1x1, 64 input channels, 8x8
surface, uint8 asymmetric quantisation in the mobilenet style, output
zero point 0 so the hardware's clamp-at-zp cannot mask anything:

  1x1 conv, 64 in, 56 out:   56/56 channels within +/-1 (47
bit-exact), maxdiff 1
  1x1 conv, 64 in, 88 out:   88/88 channels within +/-1 (81
bit-exact), maxdiff 1
  1x1 conv, 64 in, 120 out:  120/120 channels within +/-1 (106
bit-exact), maxdiff 1

Reference is the raw CPU implementation per channel (not clamped to the
zero point - same lesson as operator 28). No channel had a constant
reference, so the matches are computed rather than trivial, and no
channel sat pinned at the output zero point. The three jobs also
happened to land one per NPU core (per-core IRQ counters went 0->1),
so the result is not specific to a single core.

Since upstream emits the same word for every oc, a
clean result at 56/88/120 says RK3588 does not need an oc-dependent
toggle at 0x4050 at all, at least for these shapes - which is the third
possibility your two forms could not distinguish. It does not tell us
which of your two fitted forms is closer to what RK3576 wants, but it
does suggest comparing your fitted values against the field composition
above rather than as opaque constants.

Happy to run other shapes if that helps narrowing it down.

Best regards,
Igor


On Sun, Aug 16, 2026 at 6:12 AM Jiaxing Hu <gahing@gahingwoo.com> wrote:
>
> Hi Igor,
>
> Thank you for running it, and for the unclamped reference.
>
> Your 1001 does not discriminate between the two forms, which I should have said
> when I described them. DIV_ROUND_UP(1001,16) is 63, odd, so the parity form
> gives 0x80011011 there and so does oc % 32. They differ only where a count is a
> multiple of neither 32 nor 16. On RK3576 the failing one was 56, and 88 or 120
> would do as well. So your clean result rules nothing out, and I am not reading
> it as support either way.
>
> On my side that register turned out to be the whole of it. A 1x1 layer with 56
> output channels had timed out in every round it ever ran, 32 of 56, and with
> the parity form it is 56 of 56 every channel correct, with the old form
> reproducing the timeout in the same log.
>
> You are right about perch.py and there were three of them rather than one. The
> inner maxdiff, the interior statistics and the raw row dump all assume a
> surface with an interior, and all three raise on a 1x1x1001 classifier after
> the useful lines have printed. Fixed here, and checked on the host at 1x1x1001,
> 5x5x8 and 56x56x64 before flashing, which is a check I should have been running
> all along.
>
> And I will re-score operator 28 against the unclamped output. The max(cpu, zp)
> reference does move channels that are not wrong, and reading an empty
> convolution off it is the kind of mistake I would rather not make twice.
>
> Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

You are right that it is fitted, and decoding it against registers.xml was
worth more than the fit was. I had been treating those two words as opaque
constants and comparing predicates, when the thing that varies is one bit.

Your reading also named an experiment I had never run. Every measurement this
board has produced was of the value my predicate happens to emit, so 16 and 41
output channels had only ever run with SIZE_E_2 0, and 56, 64 and 128 only with
1. The cell nobody had filled in was SIZE_E_2 1 at the small counts. Forcing
the constant fills it.

  output channels    predicate     forced SIZE_E_2 1
   16                16 of 16      NPU job timed out, 0 of 16
   41                41 of 41      0 of 41
   56                56 of 56      56 of 56
  128              128 of 128    128 of 128

So each form fails on the shapes the other handles, which makes it a two sided
measurement rather than the one sided fit it started as. Taken with your RK3588
result, RK3588 does not need the toggle and RK3576 does, and what the toggle
selects is SIZE_E_2 rather than anything about the channel count as such. I
will write it that way, as the field with a reason, rather than as a modulo or
a parity of something.

The other bits you flagged I cannot defend yet. RGP_CNTER 8, OW_SRC 1 and the
two in the reserved range came from vendor captures and have never been varied
one at a time. That is a sweep this board can run and I will do it before the
Mesa series goes out, since a value nobody can explain is a value nobody should
be asked to review.

On the unclamped reference, thank you for using it. It changed what I thought I
had here as well. The clamp is only free where the quantised output range
starts at zero, and on a middle zero point layer it rewrites about half the
surface. conv2d-cal is out_zp 128 with no fused activation, and against the
unclamped output it is 0 of 128 channels rather than 128 of 128, with the
hardware equal to max(cpu, out_zp) to within one everywhere. Where that clamp
comes from is still open. It is not the output offset, not the BS block, not
any register in the stream, and not the coefficient records, all four measured
rather than argued.

If you want another shape, an output channel count of 88 or 120 on RK3576 would
tell us whether the toggle follows the same rule up there, and those are exactly
the counts you already have on the other SoC.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

Your conv2d-cal result made me curious whether the clamp is a property of
the RKNN output stage in general, so I built the same shape here: 1x1
conv, 64 input channels, 8x8 surface, uint8, out_zp 128, no fused
activation - and compared against both references. On RK3588 the answer
is the opposite of yours.

  output channels    vs raw CPU              vs max(cpu, out_zp)
   56                56 of 56 (47 exact)      0 of 56
  128               128 of 128 (113 exact)    6 of 128

Maxdiff against the raw reference is 1 in both cases; against the clamped
reference it is 128. Where the raw reference falls below the zero point,
the hardware output spans 0..127 rather than sitting at 128, and only
0.6-0.8% of hardware values land exactly on the zero point, which is
about what the distribution gives you by chance.

So RK3588 does not clamp at the output zero point, and the clamp you are
seeing is not something the DPU output stage does everywhere. That is one
more thing it is not, and I think it is the expensive one to rule out by
inspection.

Three controls, since a "hardware equals CPU" result is exactly what a
silent delegate fallback would also produce:

  - the per-core NPU interrupt counters advance by one per run, so the
    job did reach the hardware
  - 15 of the 128 channels differ from the CPU by one, which a CPU
    fallback could not produce - it would be bit-identical
  - 46% (oc=128) and 59% (oc=56) of the raw reference lies below the zero
    point, so there was something for a clamp to remove

One thing I cannot tell from here: whether the difference is the silicon
or the userspace path. My runs are upstream Mesa main with no RK3576
patches, and yours are your working tree, so the two differ in more than
the SoC. If you still have the RK3576 board on a stock upstream Mesa, the
same shape there would separate those two explanations - and if it turns
out to be userspace rather than silicon, that is a much easier bug to
find than a hardware one.

Yes to the 88 and 120 sweep on RK3576, and thank you for offering it. If
the toggle follows the same rule at those counts I can re-run the RK3588
side with the forced constant for symmetry, so we would have both forms
on both SoCs at four channel counts.

Best regards,
Igor


On Sun, Aug 16, 2026 at 9:58 PM Jiaxing Hu <gahing@gahingwoo.com> wrote:
>
> Hi Igor,
>
> You are right that it is fitted, and decoding it against registers.xml was
> worth more than the fit was. I had been treating those two words as opaque
> constants and comparing predicates, when the thing that varies is one bit.
>
> Your reading also named an experiment I had never run. Every measurement this
> board has produced was of the value my predicate happens to emit, so 16 and 41
> output channels had only ever run with SIZE_E_2 0, and 56, 64 and 128 only with
> 1. The cell nobody had filled in was SIZE_E_2 1 at the small counts. Forcing
> the constant fills it.
>
>   output channels    predicate     forced SIZE_E_2 1
>    16                16 of 16      NPU job timed out, 0 of 16
>    41                41 of 41      0 of 41
>    56                56 of 56      56 of 56
>   128              128 of 128    128 of 128
>
> So each form fails on the shapes the other handles, which makes it a two sided
> measurement rather than the one sided fit it started as. Taken with your RK3588
> result, RK3588 does not need the toggle and RK3576 does, and what the toggle
> selects is SIZE_E_2 rather than anything about the channel count as such. I
> will write it that way, as the field with a reason, rather than as a modulo or
> a parity of something.
>
> The other bits you flagged I cannot defend yet. RGP_CNTER 8, OW_SRC 1 and the
> two in the reserved range came from vendor captures and have never been varied
> one at a time. That is a sweep this board can run and I will do it before the
> Mesa series goes out, since a value nobody can explain is a value nobody should
> be asked to review.
>
> On the unclamped reference, thank you for using it. It changed what I thought I
> had here as well. The clamp is only free where the quantised output range
> starts at zero, and on a middle zero point layer it rewrites about half the
> surface. conv2d-cal is out_zp 128 with no fused activation, and against the
> unclamped output it is 0 of 128 channels rather than 128 of 128, with the
> hardware equal to max(cpu, out_zp) to within one everywhere. Where that clamp
> comes from is still open. It is not the output offset, not the BS block, not
> any register in the stream, and not the coefficient records, all four measured
> rather than argued.
>
> If you want another shape, an output channel count of 88 or 120 on RK3576 would
> tell us whether the toggle follows the same rule up there, and those are exactly
> the counts you already have on the other SoC.
>
> Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

That is the measurement I could not make from here, and between your row and one
of mine the question is answered.

The control you propose is not one I can run in that form. Upstream Mesa has no
RK3576 path, so its encoder emits the RK3588 register layout, and this SoC does
not share that layout at the same offsets. I would not expect the result to be a
convolution at all, and I have not run it to find out.

What I can run is the vendor userspace, which is the same trade the other way
around, same silicon and an entirely different stack. Five models, output tensor
read as int8, the NPU interrupt count advancing by exactly one per run so
each of them reached the hardware.

  model    zero point   values below it            sitting exactly on it
  a_lin       17          2478 of 4096    60.50%      32
  a_lin2     -14          2420 of 4096    59.08%      41
  g_cal       -8        108564 of 204800  53.01%    2551
  pq_oc        0         62720 of 128576  48.78%    3136
  w_160        0        236287 of 409600  57.69%    2877

g_cal is conv2d-cal's geometry exactly, 16 input channels to 128 output over an
80x80 surface, 5x5 at stride 2. pq_oc and w_160 carry conv2d-cal's zero point
exactly, 0 in int8 being the 128 my tables report in uint8. So the geometry and
the zero point are each covered by a model that does not clamp. The counts
sitting on the zero point are 0.7 to 2.4 percent of the surface, which is close
to what you measured on the other side and close to what the distribution gives.

The grid now reads

  RK3576, vendor userspace     does not clamp
  RK3588, upstream Mesa        does not clamp, your run
  RK3576, my Mesa              clamps

The first and third rows are the same silicon. So the clamp is mine, and there
is no hardware behaviour left for me to appeal to. Your third control is the one
that makes your row carry weight, since fifteen channels off by one is something
a delegate falling back to the CPU could not produce.

I have not found it yet. The register stream is byte identical to the vendor's
at this geometry apart from addresses, the requantisation, the pad value and the
padding. A, B and C swapped one at a time from the vendor's records each leave
the floor where it is. The weight buffer is the last thing I have not compared.

The 88 and 120 sweep is queued for the next time the board is flashed, and I
will send what it says either way.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

Here are 88 and 120 on RK3576, with the 56 from earlier in this thread as
a third point. A channel counts as matching when its maxdiff against the
reference is at most 1, which is how perch.py scores.

  oc    parity form    modulo form, ROCKET_DPU4050_MOD32=1
   56   56 of 56       32 of 56, timed out, lost 32 to 55
   88   88 of 88       64 of 88, timed out, lost 64 to 87
  120  120 of 120      96 of 120, timed out, lost 96 to 119

Under the modulo form each count keeps whole groups of 32 output channels
and drops the remainder, and all three of those runs also raise the
driver's job timeout. Under the parity form all three compute the whole
output at a maxdiff of 1.

What the three points cannot tell you. 56, 88 and 120 are all 24 modulo
32, so the twenty four lost channels are forced by the arithmetic and are
not corroboration, and the rule is untested at every other remainder. A
count of 40 or 72 would say more than a fourth one at 24.

The modulo numbers are read back from a job the driver declared timed out,
so the obvious alternative is that the job died before writing the tail of
the output. The log argues against it. At 120, nine of the twenty four
wrong channels carry enough varying output to fit a line against the
reference, and the surface reports 45 constant channels of 120 with 37 of
those pinned at the output zero point, so the lost region was written
rather than left untouched.

ROCKET_DPU4050_MOD32 occurs once in the whole Mesa tree, inside the value
expression for 0x4050, so at a given count the two register streams differ
in that word and nothing else. That is an argument from the source, not a
measurement. I meant to measure it at 64 output channels, where the two
forms emit the same value by construction, and only got one side of it.
Without the knob it was 64 of 64. The run with the knob did not complete,
the entry hung after the 88 run had timed out.

One confound to disclose. 88 both ways and 120 under the parity form are
one boot. 120 under the modulo form is the next boot, on a kernel that
also carries an unrelated power domain change, and the NPU rail reads
enabled in the first and disabled in the second. The Mesa build is byte
identical across the two and the modulo form times out under either
kernel, but the 120 row is not a single variable comparison and should be
read as two.

If you still want the RK3588 side, ROCKET_DPU4050_MOD32=1 against your own
build is the comparison.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

You asked for a count that is not 24 modulo 32, so here are five, on
RK3588 with upstream Mesa. Same generator and same scoring as the 56, 88
and 120 rows earlier in this thread: 1x1 conv, 64 input channels,
uint8, output zero point 0, scored against the raw CPU reference with a
channel counting as matching at maxdiff 1 or less.

  oc    oc mod 32   matching      bit exact
   33       1        33 of 33        31
   40       8        40 of 40        34
   48      16        48 of 48        40
   56      24        56 of 56        47     (16 Aug)
   72       8        72 of 72        62
   88      24        88 of 88        81     (16 Aug)
  100       4       100 of 100       86
  120      24       120 of 120      106     (16 Aug)

Eight counts across five distinct remainders. The upstream constant
computes the whole output at every one of them, and the global maxdiff
is 1 in all eight runs. So on this SoC BS_OW_CFG needs no term in the
output channel count at any remainder, not just at 24.

Controls, same as before. No channel of the reference is constant in any
of the eight models - the generator refuses to emit a model where one
is, since a constant reference makes a trivial match. No NPU channel is
pinned at the output zero point. The NPU interrupt counters went from
zero to five across the three cores over the five new models, one job per
model, so each of them reached the hardware. And the counts that are not
bit exact are off by exactly 1, which as you noted is not something a
delegate falling back to the CPU produces.

What this does not say: it measures the upstream form only. It is not a
statement about how the modulo form would behave on RK3588, and it does
not touch your timeout observation.

Which brings me to your last line. I cannot run
ROCKET_DPU4050_MOD32=1 against my build, because the knob is not there.
Upstream Mesa has no occurrence of it anywhere in the tree, and
src/gallium/drivers/rocket/rkt_regcmd.c emits BS_OW_CFG from the
depthwise flag alone:

	if (operation->depthwise) {
	   EMIT(REG_DPU_BS_OW_CFG, DPU_BS_OW_CFG_SIZE_E_2(3) |
	                              DPU_BS_OW_CFG_SIZE_E_1(3) |
	                              DPU_BS_OW_CFG_SIZE_E_0(3));
	} else {
	   EMIT(REG_DPU_BS_OW_CFG, DPU_BS_OW_CFG_SIZE_E_2(1) |
	                              DPU_BS_OW_CFG_SIZE_E_1(1) |
	                              DPU_BS_OW_CFG_SIZE_E_0(1));
	}

which is 0x36c and 0x124, with no output channel term in either. Setting
the variable on my build would be silently ignored, and I would have
reported a null result as if it meant something.

So: send me the value expression your knob selects at 0x4050 and I will
patch it into my tree locally and run the RK3588 side of the comparison.
I would want to run your oc=64 control at the same time, both ways, since
that is the one where the two forms emit the same value by construction
and it is the cleanest discriminator you have. You got one side of it and
the other hung. I can run both here, and I can also give you 40 and 72
under the modulo form, where the remainder is 8 rather than 24.

Two things I want to acknowledge rather than skip past. You flagged the
mod-32 confound yourself before anyone asked, and you were right to,
the twenty four lost channels at 56, 88 and 120 really are forced by the
arithmetic and are not three independent confirmations. And you disclosed
that the 120 modulo row is a different boot with an unrelated power
domain change and a different NPU rail state. Reading that row as two
points rather than one is the correct call, and saying so in the mail
rather than after being asked is worth more than the row.

If it is useful to anyone, I can send the generator and the scorer. The
generator builds the flatbuffer by hand and self-checks against the CPU
interpreter before emitting a model, and the scorer is your perch.py
logic reduced to this one case.

Regards,
Igor

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Hi Igor,

First, I sent you to the wrong counts. The two predicates disagree only
where oc mod 32 falls between 17 and 31, since modulo true implies parity
true and never the reverse. At 40 and 72 both forms emit the same word, so
I withdraw them. By the same arithmetic only 56, 88 and 120 of your eight
carry any parity versus modulo content. The rest still establish something
better, that the upstream constant computes whole at five remainders on
RK3588. Discriminating counts nobody has run are 20, 50, 60, 90 and 114.

Take the code from the merge request rather than my working branch, it has
no knobs in it.

  https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/43804

  R_DPU(0x4050, (DIV_ROUND_UP(output_channels, FEATURE_ATOMIC_SIZE) & 1)
                   ? 0x80011011 : 0x80011111);

output_channels is task->output_channels_real, the unpadded count, and
FEATURE_ATOMIC_SIZE is 16.

Do not put my two words in an RK3588 build. Against registers.xml

  field         upstream 0x124   mine 0x80011111   mine 0x80011011
  RGP_CNTER            0                8                 8
  RESERVED_0           0               34                34
  SIZE_E_2             1                1                 0
  SIZE_E_1             1                0                 0
  SIZE_E_0             1                4                 4
  OW_SRC               0                1                 1

Mine differ from each other in SIZE_E_2 alone and from yours in five
further fields. Those five are common to both of my words so they do not
confound an A against B, but none of them has ever run on an RK3588, and
if both arms fail for that reason the oc 64 control goes down with them.

Use your own constant with bit 8 cleared instead, 0x124 where the
predicate is true and 0x024 where it is false. That takes SIZE_E_2 from 1
to 0, the same move my words make. It assumes the three SIZE_E fields move
independently, which neither of us has shown.

Bit 8 does something on RK3576. The table I sent on the 16th forced
SIZE_E_2 to 1 with every other field identical, and 16 output channels
went from 16 of 16 to a job timeout, 41 from 41 of 41 to 0 of 41. Whether
it is the only field that matters is open, and the sweep I promised before
the series went out covers RGP_CNTER, OW_SRC and two reserved bits but not
SIZE_E_1 or SIZE_E_0, which are just as fitted.

Of your three offers the oc 64 control is the one I want, both ways, since
that is the side I lost. A count at 20 or 60 would be the first
discriminating point outside 24 modulo 32. And yes to the generator and
the scorer, the refusal to emit a constant reference channel is a better
guard than mine.

You are right that none of this touches the timeout. Every modulo run that
lost channels also raised one, at three counts, and I cannot yet tell
whether the register causes it or both follow from something else.

Jiaxing

Re: [PATCH v7 08/10] accel/rocket: add RK3576 NPU (RKNN) support

[Igor Paunovic]

Hi Jiaxing,

Thank you for withdrawing 40 and 72 before I built anything on them, and for
the field table. I checked the table against registers.xml and it is right in
all eighteen cells, including RESERVED_0 = 34, which is bits 12 and 16 and is
not obvious. I also confirmed the part of your reasoning that matters most:
your two words XOR to 0x100, and 0x124 against 0x80011111 and 0x024 against
0x80011011 both XOR to 0x80011035. The five-field offset really is common mode
across the two arms and cancels in an A against B.

One thing I cannot resolve on my own before I write code.

You wrote: "Use your own constant with bit 8 cleared instead, 0x124 where the
predicate is true and 0x024 where it is false." The merge request line you told
me to take instead of your branch reads

  R_DPU(0x4050, (DIV_ROUND_UP(output_channels, FEATURE_ATOMIC_SIZE) & 1)
                   ? 0x80011011 : 0x80011111);

which puts the SIZE_E_2 = 0 word on the true arm. Read with that predicate your
sentence gives me 0x024 at oc = 64, where your merge request emits 0x80011111
and current upstream emits 0x124, both SIZE_E_2 = 1. Read with "the predicate"
meaning output_channels % 32 == 0, your sentence is exactly your modulo form at
every count. Both readings agree at every count with oc mod 32 in 17..31, so
your five discriminating counts are unaffected either way. They disagree only at
oc mod 32 == 0 and 1..16 - and oc = 64 is the count you said you most want.

So, concretely, so that I do not guess: at oc = 64, do you want SIZE_E_2 = 1 or
SIZE_E_2 = 0? And if you meant your own form rather than the merge request's,
send me the expression.

I would rather we name the arms by value and by count than by the word
predicate, since it now refers to two different things in this thread.

Now the part I think is worth more than the A against B.

I went looking for what SIZE_E means rather than which value works, and I think
it falls out of the padding. In upstream rkt_task.c the output channel count is
padded to a multiple of 32 for the normal case (line 87,
align(MAX2(oc, 32), 32)) and to 64 for depthwise (lines 88-91), and
FEATURE_ATOMIC_SIZE is 16 (rkt_ml.h:18). So the padded bank is 2 atoms of 16 in
the normal case and 4 in the depthwise case. Upstream writes SIZE_E = 1 and
SIZE_E = 3 respectively. That is 2 - 1 and 4 - 1.

  SIZE_E_n + 1 = the number of 16-channel atoms in the final output channel bank

If that reading is right, your parity predicate is not a fit at all, it is this
same statement: I checked over oc = 1..512 with no exceptions that
DIV_ROUND_UP(oc, 16) being odd is exactly the condition that the final
32-channel bank holds one atom rather than two. Odd atom count means a
half-populated last bank means SIZE_E_2 should be 0. That is the reason you said
on 16 August you wanted for the field, and it is arithmetically identical to
what you already wrote.

It also explains why my eight counts said nothing, which had been bothering me.
Upstream hands the padded count to the write path, not the real one -
rkt_regcmd.c lines 201, 226 and 247 all take task->output_channels, and only
ORIG_CHANNEL at line 225 takes output_channels_real. So on RK3588 the final bank
is always declared full, the atom count is always 2, and SIZE_E_2 = 1 is simply
correct at every remainder. My five new counts could not have discriminated
anything; they were confirming that upstream never under-declares the last bank.

And it predicts your losses arithmetically rather than describing them. An
under-declared last bank keeps floor(oc/32) * 32 channels, which is 32, 64 and
96 at oc = 56, 88 and 120 - your three numbers, including the lost ranges 32-55,
64-87 and 96-119.

Please treat that as a hypothesis. I derived it from the encoder's arithmetic,
not from a TRM or a vendor document, and it assumes the three SIZE_E fields are
independent, which is the same assumption you flagged. It also does not touch
the timeout, and I agree with you that the timeout is unexplained.

What I will run, once the polarity above is settled:

  - oc = 64 both ways with the constant forced, no predicate in the build at
    all, so the ambiguity cannot reach it. I will patch only the non-depthwise
    branch at rkt_regcmd.c:239-241; clearing bit 8 in the depthwise arm would be
    0x36c -> 0x26c, SIZE_E 3 -> 2, a different move. I will verify the emitted
    word from the submitted register stream rather than the source, because at
    oc = 64 real and padded are equal and a build that read the wrong one would
    be invisible.
  - oc = 20 and oc = 60 on stock upstream as the reference arm. Those need no
    patch and no answer from you, so I can send them whenever. They also add
    residues 20 and 28 to the table, which so far only has 1, 4, 8, 16 and 24.

Generator and scorer are on the way in a separate mail.

Two small things. Your sentence "modulo true implies parity true and never the
reverse" is the wrong way round - parity true is residues 1..16, which is a
subset of not-a-multiple-of-32, so the implication runs the other way. Nothing
downstream of it changes. And your five are a sample rather than the set: below
128 there are sixty counts with oc mod 32 in 17..31, and odd ones are fine since
33 already ran, so 17, 25, 51, 83 and 117 would do just as well as 20 and 50.

Regards,
Igor

[PATCH v7 09/10] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

[Jiaxing Hu]

Add the two RKNN cores and their IOMMUs, plus the NPU power-domain
resets the pmdomain driver now cycles on power-on. Both cores are
disabled by default; boards enable what they wire up.

Each core lists both NPU power domains, its own first. The compute path
needs NPU1 powered even when only core 0 runs, and a node with a single
domain would be auto-attached by the driver core before the driver can
attach the list itself. The IOMMUs keep one domain each, since they rely
on that same auto-attach.

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index b0c0d3c8b..1e6dd039f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
@@ -1070,14 +1070,22 @@ power-domain@RK3576_PD_NPUTOP {
 						power-domain@RK3576_PD_NPU0 {
 							reg = <RK3576_PD_NPU0>;
 							clocks = <&cru HCLK_RKNN_ROOT>,
-								 <&cru ACLK_RKNN0>;
+								 <&cru ACLK_RKNN0>,
+								 <&cru CLK_RKNN_DSU0>,
+								 <&cru ACLK_RKNN_CBUF>,
+								 <&cru HCLK_RKNN_CBUF>;
+							resets = <&cru SRST_A_RKNN0_BIU>;
 							pm_qos = <&qos_npu_m0>;
 							#power-domain-cells = <0>;
 						};
 						power-domain@RK3576_PD_NPU1 {
 							reg = <RK3576_PD_NPU1>;
 							clocks = <&cru HCLK_RKNN_ROOT>,
-								 <&cru ACLK_RKNN1>;
+								 <&cru ACLK_RKNN1>,
+								 <&cru CLK_RKNN_DSU0>,
+								 <&cru ACLK_RKNN_CBUF>,
+								 <&cru HCLK_RKNN_CBUF>;
+							resets = <&cru SRST_A_RKNN1_BIU>;
 							pm_qos = <&qos_npu_m1>;
 							#power-domain-cells = <0>;
 						};
@@ -1832,6 +1840,74 @@ qos_npu_m1ro: qos@27f22100 {
 			reg = <0x0 0x27f22100 0x0 0x20>;
 		};
 
+		rknn_core_0: npu@27700000 {
+			compatible = "rockchip,rk3576-rknn-core";
+			reg = <0x0 0x27700000 0x0 0x1000>,
+			      <0x0 0x27701000 0x0 0x1000>,
+			      <0x0 0x27703000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core";
+			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>,
+				 <&cru ACLK_RKNN_CBUF>, <&cru HCLK_RKNN_CBUF>;
+			clock-names = "aclk", "hclk", "npu", "pclk",
+				      "aclk_cbuf", "hclk_cbuf";
+			resets = <&cru SRST_A_RKNN0>;
+			reset-names = "srst_a";
+			power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
+			iommus = <&rknn_mmu_0>;
+			status = "disabled";
+		};
+
+		rknn_mmu_0: iommu@27702000 {
+			compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
+			reg = <0x0 0x27702000 0x0 0x100>,
+			      <0x0 0x27702100 0x0 0x100>;
+			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
+				 <&cru HCLK_RKNN_CBUF>;
+			clock-names = "aclk", "iface", "npu",
+				      "aclk_cbuf", "hclk_cbuf";
+			#iommu-cells = <0>;
+			power-domains = <&power RK3576_PD_NPU0>;
+			status = "disabled";
+		};
+
+		rknn_core_1: npu@27708000 {
+			compatible = "rockchip,rk3576-rknn-core";
+			reg = <0x0 0x27708000 0x0 0x1000>,
+			      <0x0 0x27709000 0x0 0x1000>,
+			      <0x0 0x2770b000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core";
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>,
+				 <&cru ACLK_RKNN_CBUF>, <&cru HCLK_RKNN_CBUF>;
+			clock-names = "aclk", "hclk", "npu", "pclk",
+				      "aclk_cbuf", "hclk_cbuf";
+			resets = <&cru SRST_A_RKNN1>;
+			reset-names = "srst_a";
+			power-domains = <&power RK3576_PD_NPU1>, <&power RK3576_PD_NPU0>;
+			iommus = <&rknn_mmu_1>;
+			status = "disabled";
+		};
+
+		rknn_mmu_1: iommu@2770a000 {
+			compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
+			reg = <0x0 0x2770a000 0x0 0x100>,
+			      <0x0 0x2770a100 0x0 0x100>;
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
+				 <&cru HCLK_RKNN_CBUF>;
+			clock-names = "aclk", "iface", "npu",
+				      "aclk_cbuf", "hclk_cbuf";
+			#iommu-cells = <0>;
+			power-domains = <&power RK3576_PD_NPU1>;
+			status = "disabled";
+		};
+
 		gmac0: ethernet@2a220000 {
 			compatible = "rockchip,rk3576-gmac", "snps,dwmac-4.20a";
 			reg = <0x0 0x2a220000 0x0 0x10000>;
-- 
2.43.0

[PATCH v7 10/10] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[Jiaxing Hu]

Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
core from vdd_npu_s0.

The supply is marked always-on because the NPU power domains are what
gate the block here, and dropping the rail underneath them takes an
async SError on the next power-on rather than a clean retry. Only
rknn_core_0 is enabled: the driver binds one core per node and the
second core is left to whoever can test it.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 272af1012..965e0906b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -442,6 +442,7 @@ regulator-state-mem {
 			};
 
 			vdd_npu_s0: dcdc-reg2 {
+				regulator-always-on;
 				regulator-boot-on;
 				regulator-enable-ramp-delay = <400>;
 				regulator-min-microvolt = <550000>;
@@ -869,3 +870,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
 		remote-endpoint = <&hdmi_in_vp0>;
 	};
 };
+
+&rknn_core_0 {
+	npu-supply = <&vdd_npu_s0>;
+	status = "okay";
+};
+
+&rknn_mmu_0 {
+	status = "okay";
+};
-- 
2.43.0

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

[Chaoyi Chen]

Hi Jiaxing,

On 8/12/2026 5:41 PM, Jiaxing Hu wrote:
> Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
> core from vdd_npu_s0.
> 
> The supply is marked always-on because the NPU power domains are what
> gate the block here, and dropping the rail underneath them takes an
> async SError on the next power-on rather than a clean retry. Only
> rknn_core_0 is enabled: the driver binds one core per node and the
> second core is left to whoever can test it.
> 
> Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 272af1012..965e0906b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> @@ -442,6 +442,7 @@ regulator-state-mem {
>  			};
>  
>  			vdd_npu_s0: dcdc-reg2 {
> +				regulator-always-on;
>  				regulator-boot-on;
>  				regulator-enable-ramp-delay = <400>;
>  				regulator-min-microvolt = <550000>;
> @@ -869,3 +870,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
>  		remote-endpoint = <&hdmi_in_vp0>;
>  	};
>  };
> +
> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;

Out of curiosity, I searched for code about this supply in the rocket
driver and found nothing.

Then what is the consumer of this regulator? I have reason to suspect 
they were automatically disabled.


> +	status = "okay";
> +};
> +
> +&rknn_mmu_0 {
> +	status = "okay";
> +};

-- 
Best, 
Chaoyi