-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
rockchip64-6.18: Enable the NPU on Turing RK1 #9023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rockchip64-6.18: Enable the NPU on Turing RK1 #9023
Conversation
- already had the regulator (`vdd_npu_s0`)
WalkthroughThis PR introduces a device tree patch that enables NPU (Neural Processing Unit) support on the Rockchip RK3588 Turing RK1 platform by defining power supplies for NPU cores and MMUs, and updating SD card configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
patch/kernel/archive/rockchip64-6.18/rk3588-1202-arm64-dts-rockchip-Enable-the-NPU-on-Turing-RK1.patch (1)
63-65: SDHCI bus-width/no-sdio change may impact SDIO users — confirm board wiringChanging
&sdhcitobus-width = <8>;and addingno-sdio;is appropriate if this controller is strictly used as 8‑bit eMMC on Turing RK1 with no SDIO consumers. Please verify that there are no board variants or add‑on modules relying on SDIO on this host, to avoid silently breaking WiFi or other peripherals.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
patch/kernel/archive/rockchip64-6.18/rk3588-1202-arm64-dts-rockchip-Enable-the-NPU-on-Turing-RK1.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
🔇 Additional comments (3)
patch/kernel/archive/rockchip64-6.18/rk3588-1202-arm64-dts-rockchip-Enable-the-NPU-on-Turing-RK1.patch (3)
1-12: Commit header and summary look consistent with the changeSubject, short description, and stats accurately describe enabling the NPU on Turing RK1 and only touching the board DTS include; nothing concerning here.
22-24: Power-domain supply hookup for&pd_npulooks correct; please confirm regulator presenceHooking
&pd_nputovdd_npu_s0matches how RK3588 boards usually express NPU power. Please double‑check thatvdd_npu_s0is indeed defined in this board’s DTS stack (and not left disabled) so the NPU domain can actually power up.
33-62: NPU cores/MMUs enablement matches RK3588 patterns; verify all three cores are really usable on Turing RK1Setting
npu-supplyandsram-supplyforrknn_core_[0-2]tovdd_npu_s0and flipping all cores and MMUs to"okay"is in line with typical RK3588 NPU enablement overrides. Please confirm that Turing RK1 actually exposes all three NPU cores (no SKU with fused‑off cores) and that there are no board‑specific reasons to keep any of these disabled (e.g., thermal or power constraints).
|
✅ This PR has been reviewed and approved — all set for merge! |
vdd_npu_s0)Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.