linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 00/14] accel/rocket: RK3576 NPU (RKNN) enablement
@ 2026-09-12  6:50 Jiaxing Hu
  2026-09-12  6:50 ` [PATCH v12 01/14] accel/rocket: request the core clocks by name Jiaxing Hu
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Jiaxing Hu @ 2026-09-12  6:50 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, joro, will, robin.murphy,
	ulfh, p.zabel, ogabbay, zhangqing
  Cc: royalnet026, abel.vesa, sebastian.reichel, sidong.yang,
	u.kleine-koenig, chaoyi.chen, diederik, alchark, dri-devel,
	linux-rockchip, iommu, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

This adds the RK3576 NPU to accel/rocket, which today supports RK3588
only. The RK3576 carries two cores of the same RKNN block, wired up
differently. Two extra convolution buffer clocks, two power domains per
core, one reset rather than two, no NPU SRAM rail, and a PC_TASK_CON
that packs the task number into sixteen bits rather than twelve.

1/14 is Igor Paunovic's "[PATCH v2] accel/rocket: request the core clocks
by name", carried in the series since v11 so it applies to a plain
next-20260911 with nothing outside it to follow. It keeps his authorship
and its four tags. Tested on a Radxa ROCK 4D.

Five things changed since v11:

  3/14   clears the raw interrupt status beside the mask. Masking alone
         left the DPU bit latched until rocket_core_reset(), and the
         hardirq decides on raw status alone, so a fault from the IOMMU
         sharing this core's line would wake the threaded handler again
         and "No handler is running now" would stop holding partway
         through the function.

         It still tests "> 0". An earlier draft of this version changed
         that to "!= 0", on the reasoning that -EINVAL means runtime PM
         is not managing the device and the block is therefore powered.
         That is wrong and the change is withdrawn.
         pm_runtime_get_conditional() tests power.disable_depth before
         power.runtime_status, so -EINVAL masks a suspended device
         rather than excluding one, and this driver makes that state
         twice: in pm_runtime_force_suspend(), its own system sleep
         callback, which disables runtime PM before turning the clocks
         off, and in rocket_core_fini(), which suspends then disables
         before cancelling the timeout worker. Either would have put a
         register write on a powered-down block.

  10/14  cycles the resets before the settle delay 9/14 adds, not after.
         PD_NPU0 and PD_NPU1 ask for both, and the old order spent the
         delay and then deasserted a reset with nothing between it and
         rockchip_pmu_restore_qos(). The reset is SRST_A_RKNN0/1_BIU,
         the bus interface those writes go through.

         This is a change to code Abel Vesa reviewed. His Reviewed-by
         is kept because the patch still does what he read, in a
         different order; say so if it should go. 9/14 is untouched.

  11/14  checks the match data at the top of rocket_probe(), before
         anything is allocated. of_device_get_match_data() returns NULL
         for a device bound by name rather than by compatible, and such
         a device has no of_node, so the walk of matching nodes that
         sizes rdev->cores[] never counted it. Storing into that array
         and checking afterwards writes past the end.

  13/14  gives CLK_RKNN_DSU0 a rate. Nothing in mainline sets it and the
         NPU comes up at 786 MHz, while Rockchip's OPP table asks 800 mV
         of its 800 MHz step and nothing sets the rail either. On the
         ROCK 4D at the 750 mV its PMIC boots with, two jobs at once
         make the second core write single words wrong, thirteen to
         twenty rows of a 5400 row pass, where either core alone is
         exact; at 594 MHz, or at 786 with the rail at 800 mV, four such
         passes are clean. 594 MHz is a divider off GPLL and sits
         between that table's 500 and 600 MHz steps, both of which ask
         725 mV, so it is inside the voltage a board that describes no
         NPU rail already provides.

  14/14  enables both cores and both IOMMUs. v11 enabled rknn_core_0
         alone, "left to whoever can test it". It has been tested.

Sashiko raised one thing on 4/14 that an argument does not settle.
pm_runtime_put_autosuspend() is asynchronous with a 50 ms delay, so on a
workload whose submit gap is shorter the domain does not cycle after a
reset, 10/14's power-on pulse never fires, and rocket_core_reset()'s own
resets are all that run.

Measured on the domain rather than on the device: over one 60.8 s decode
of Phi-3.5-mini on a ROCK 4D, genpd's npu domain took 46.8 s active and
14.2 s idle, which sum to the wall clock, and its idle-state usage count
rose by 202. npu0 and npu1 rose by 210 and 217, and current_state reads
off-0 either side. The domain cycles about two hundred times a minute
under a real decode, and the pulse fires.

Whether it cycles after a timed-out job is the half Sashiko is asking
about, and that depends on the gap to the next submit. Inducing one needs
a kernel with JOB_TIMEOUT_MS lowered and a flash, so that half is still
an argument rather than a measurement.

Findings Sashiko labelled pre-existing, for Tomeu. These five recur; it
raised thirteen across the nine mails, and the one not listed worth
naming is a 64-bit DMA address truncated to 32 bits, which faults the
IOMMU above 4 GB.

  - the job completion path takes iommu_group_get(core->dev) and never
    puts it, one group reference a job, flagged on seven of the nine.
    5/14 moves this call rather than adding or removing it, so the leak
    is pre-existing but not untouched;
  - the shared IRQ handler touches registers without checking the PM
    state;
  - runtime suspend has no synchronize_irq();
  - num_cores is used both as an array length and as the probe index;
  - rdev is leaked through devres on probe deferral.

5/14 moves the iommu_detach_group(NULL, iommu_group_get(core->dev)) line
out of the IRQ handler and into rocket_job_next_locked(). ZhaoJinming's
"accel/rocket: Fix iommu_group leak and unsafe IRQ register access"
changes the same line, is marked for stable, and carries the same Fixes
tag as ours. Whichever lands first the other conflicts; say which you
would rather take.

Still open from v9, no reply since. 9/14 does three things: it adds the
settle delay, renames a macro, and gives RK3576_PD_NPU a regulator, which
also makes every RK3576 board force that domain off at probe. I asked
whether it wants splitting; Abel Vesa's Reviewed-by may be the answer,
and I would rather split it than have it merged on my silence. 13/14 also
gives each core both NPU domains, which is the description that has been
tested here rather than the topology; if it should be one domain per
core, 6/14's minItems has to change with it.

The bindings are unchanged since v9, where dt_binding_check was clean on
all three with dtschema 2026.6 and yamllint 1.38.0, and CHECK_DTBS was
clean on all 13 rk3576 and all 48 rk3588 dtbs. 13/14 adds a property to a
dts rather than to a binding, and CHECK_DTBS is clean on all 13 rk3576
dtbs with it.

A correction to an earlier posting. On 11 September I replied to the v10
cover saying that the 102 induced resets and the all-0x80 buffer it
described were not Igor Paunovic's. They are his: he reported both on
25 August in the v9 02/13 thread, and the 45 resets of 19 August and the
102 of 25 August are two different runs of his. I had read only the
19 August thread, and should have searched before saying in public that
he had not measured it. That mail is withdrawn, and both of his
Tested-by lines stand as he sent them.

Link to v11: https://lore.kernel.org/all/20260831081956.84871-1-gahing@gahingwoo.com/

The tags:

  2/14   Tested-by: Igor Paunovic   # RK3588, three cores
  3/14   Tested-by: Igor Paunovic   # RK3588, three cores, induced
                                    # reset, differential base,
                                    # JOB_TIMEOUT_MS=2
  4/14   Tested-by: Igor Paunovic   # RK3588, three cores, induced
                                    # reset, JOB_TIMEOUT_MS=2
  5/14   Reviewed-by: Igor Paunovic
  6/14   Reviewed-by: Krzysztof Kozlowski
  7/14   Acked-by: Conor Dooley
  8/14   Acked-by: Conor Dooley
  9/14   Reviewed-by: Abel Vesa
  10/14  Reviewed-by: Abel Vesa

Igor Paunovic (1):
  accel/rocket: request the core clocks by name

Jiaxing Hu (13):
  accel/rocket: take the completion register writes under job_lock
  accel/rocket: wait for a running IRQ handler before resetting a core
  accel/rocket: let the core suspend after a reset
  accel/rocket: factor the completion tail out of the IRQ handler
  dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  dt-bindings: power: rockchip: allow resets in a power domain node
  dt-bindings: iommu: rockchip: describe the RK3576 NPU MMU
  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: add NPU (RKNN) nodes to rk3576
  arm64: dts: rockchip: enable the NPU on rk3576-rock-4d

 .../bindings/iommu/rockchip,iommu.yaml        |  28 ++++
 .../npu/rockchip,rk3588-rknn-core.yaml        |  47 +++++-
 .../power/rockchip,power-controller.yaml      |   8 ++
 .../boot/dts/rockchip/rk3576-rock-4d.dts      |  22 +++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      |  86 ++++++++++-
 drivers/accel/rocket/rocket_core.c            |  32 ++++-
 drivers/accel/rocket/rocket_core.h            |  11 +-
 drivers/accel/rocket/rocket_device.c          |   7 +-
 drivers/accel/rocket/rocket_drv.c             |  42 +++++-
 drivers/accel/rocket/rocket_drv.h             |   2 +
 drivers/accel/rocket/rocket_job.c             | 135 +++++++++++++++---
 drivers/pmdomain/rockchip/pm-domains.c        |  83 ++++++++---
 12 files changed, 440 insertions(+), 63 deletions(-)


base-commit: 68142f986ff04b2b70b31db00f719bf690f64a9a
-- 
2.43.0



^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-09-12 11:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-12  6:50 [PATCH v12 00/14] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 01/14] accel/rocket: request the core clocks by name Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 02/14] accel/rocket: take the completion register writes under job_lock Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 03/14] accel/rocket: wait for a running IRQ handler before resetting a core Jiaxing Hu
2026-09-12 11:37   ` Igor Paunovic
2026-09-12  6:50 ` [PATCH v12 04/14] accel/rocket: let the core suspend after a reset Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 05/14] accel/rocket: factor the completion tail out of the IRQ handler Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 06/14] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 07/14] dt-bindings: power: rockchip: allow resets in a power domain node Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 08/14] dt-bindings: iommu: rockchip: describe the RK3576 NPU MMU Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 09/14] pmdomain: rockchip: add optional per-domain power-on settle delay Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 10/14] pmdomain: rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 11/14] accel/rocket: select the per-core clock and reset counts from match data Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 12/14] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 13/14] arm64: dts: rockchip: add NPU (RKNN) nodes to rk3576 Jiaxing Hu
2026-09-12  6:50 ` [PATCH v12 14/14] arm64: dts: rockchip: enable the NPU on rk3576-rock-4d Jiaxing Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).