rockchip64: opt-in helios64 CPU stability overlay (current + edge)#9822
rockchip64: opt-in helios64 CPU stability overlay (current + edge)#9822iav wants to merge 2 commits into
Conversation
iav
requested review from
ColorfulRhino,
HeyMeco,
JohnTheCoolingFan,
SuperKali,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
brentr,
catalinii,
chainsx,
efectn,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal,
pyavitz,
rpardini and
schwar3kat
as code owners
📝 WalkthroughWalkthroughAdds a new Rockchip DT overlay plugin ChangesHelios64 CPU stability overlay (DT + build + docs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
05
github-actions
Bot
added
Needs review
iav
force-pushed
the
feat/helios64-cpu-stability-overlay
branch
2 times, most recently
from
c723616 to
21c6fff
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
patch/kernel/archive/rockchip64-6.18/overlay/rockchip-rk3399-helios64-cpu-stability.dtso (2)
11-18: ⚡ Quick winConsider documenting frequencies for modified OPPs.
The comment for opp07 helpfully documents both frequency (1.8 GHz) and voltage (1.20 V). Consider adding similar documentation for opp00–opp06 to make the overlay more maintainable and easier to review. This would show the frequency-to-voltage mapping and make it clearer what the ~50–75 mV increases are relative to.
For example:
opp00 { opp-microvolt = < 900000 900000 1250000>; }; /* 600 MHz: 0.85V → 0.90V */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/rockchip64-6.18/overlay/rockchip-rk3399-helios64-cpu-stability.dtso` around lines 11 - 18, Add inline comments to the modified OPP nodes opp00 through opp06 documenting the intended frequency and original→new voltage mapping (e.g., "600 MHz: 0.85V → 0.90V") similar to the existing opp07 comment so reviewers can see frequency-to-voltage mapping and the ~50–75 mV increases; update the opp00, opp01, opp02, opp03, opp04, opp05, and opp06 node lines to include those brief comments describing frequency and old vs new voltages.
11-17: The triplet format is correct per Linux kernel OPP binding specification, but add frequency documentation for clarity.The
opp-microvolt = <target min max>triplet format used here is valid per the Linux kernel OPP device tree binding. The identical first two values (e.g.,<900000 900000 1250000>) mean the minimum voltage is fixed at that target, with the maximum allowed ceiling at 1.25V.However, the voltage configuration in this file is notably more conservative than other RK3399 OPP tables (e.g., the 2GHz variant uses 1.3V for the same cluster at 2.016GHz). Without documented frequencies for each opp entry, it's unclear whether these voltages align with the corresponding performance points. Add
opp-hzproperties (e.g.,opp-hz = /bits/ 64 <frequency>;) to each OPP node to clarify the frequency-voltage relationship and enable proper validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patch/kernel/archive/rockchip64-6.18/overlay/rockchip-rk3399-helios64-cpu-stability.dtso` around lines 11 - 17, Add explicit frequency documentation to each OPP node (opp00..opp06) by adding an opp-hz property in the 64-bit bits format (opp-hz = /bits/ 64 <frequency>;) so the voltage triplets in opp-microvolt can be correlated to their frequencies; update each node (opp00, opp01, opp02, opp03, opp04, opp05, opp06) with the correct frequency values in Hz (as 64-bit integers) that correspond to the intended performance points and ensure the units and syntax follow the DT binding (/bits/ 64) and kernel OPP conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@patch/kernel/archive/rockchip64-6.18/overlay/rockchip-rk3399-helios64-cpu-stability.dtso`:
- Around line 11-18: Add inline comments to the modified OPP nodes opp00 through
opp06 documenting the intended frequency and original→new voltage mapping (e.g.,
"600 MHz: 0.85V → 0.90V") similar to the existing opp07 comment so reviewers can
see frequency-to-voltage mapping and the ~50–75 mV increases; update the opp00,
opp01, opp02, opp03, opp04, opp05, and opp06 node lines to include those brief
comments describing frequency and old vs new voltages.
- Around line 11-17: Add explicit frequency documentation to each OPP node
(opp00..opp06) by adding an opp-hz property in the 64-bit bits format (opp-hz =
/bits/ 64 <frequency>;) so the voltage triplets in opp-microvolt can be
correlated to their frequencies; update each node (opp00, opp01, opp02, opp03,
opp04, opp05, opp06) with the correct frequency values in Hz (as 64-bit
integers) that correspond to the intended performance points and ensure the
units and syntax follow the DT binding (/bits/ 64) and kernel OPP conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1632bb13-1b7d-4386-94a7-b9822567eaea
📒 Files selected for processing (6)
patch/kernel/archive/rockchip64-6.18/overlay/Makefilepatch/kernel/archive/rockchip64-6.18/overlay/README.rockchip-overlayspatch/kernel/archive/rockchip64-6.18/overlay/rockchip-rk3399-helios64-cpu-stability.dtsopatch/kernel/archive/rockchip64-7.0/overlay/Makefilepatch/kernel/archive/rockchip64-7.0/overlay/README.rockchip-overlayspatch/kernel/archive/rockchip64-7.0/overlay/rockchip-rk3399-helios64-cpu-stability.dtso
✅ Files skipped from review due to trivial changes (3)
- patch/kernel/archive/rockchip64-6.18/overlay/README.rockchip-overlays
- patch/kernel/archive/rockchip64-7.0/overlay/README.rockchip-overlays
- patch/kernel/archive/rockchip64-7.0/overlay/Makefile
🚧 Files skipped from review as they are similar to previous changes (2)
- patch/kernel/archive/rockchip64-7.0/overlay/rockchip-rk3399-helios64-cpu-stability.dtso
- patch/kernel/archive/rockchip64-6.18/overlay/Makefile
iav
force-pushed
the
feat/helios64-cpu-stability-overlay
branch
from
21c6fff to
6348b9e
Compare
|
done |
1 participant
Description
Adds an opt-in DT overlay for the Helios64 NAS (Kobol) that raises the A72
big-cluster vcore by +75 mV on opp00..opp06; opp07 (1.8 GHz / 1.20 V)
is unchanged. Disabled by default — enable via
overlays=rk3399-helios64-cpu-stabilityin/boot/armbianEnv.txt(therockchip-prefix is implicit becauseoverlay_prefix=rockchipisalready set for this board) or through
armbian-config→ System →Kernel → Manage device tree overlays.
Voltage mapping (mainline rk3399-op1-opp.dtsi → this overlay):
max(third element of the triplet) is kept at 1.25 V on all entries,matching the forum workaround (post #237456 in topic 58597 by @ebin-dev).
Two commits, one per kernel tree:
rockchip64-current(6.18) androckchip64-edge(7.0).Motivation
Long-standing instability reports for Helios64 under sustained CPU load
are tracked on the Armbian forum:
Forum users (prahal, ebin-dev) have been distributing pre-built DTBs
with the same opp-microvolt bumps for years. No upstream patch exists
and Kobol is no longer maintaining helios* boards, so this will not
land in mainline.
Deliberately not a board-default change. On my own Helios64 the
stock mainline voltages run stable, and I don't want to push a
tree-wide voltage bump onto every user when only some units exhibit
the instability.
But the affected users currently have to maintain a hand-patched
DTB — copying
rk3399-kobol-helios64.dtbinto/boot/dtb/rockchip/and redoing it on every kernel package update. That workflow is
fragile and easy to forget. Armbian already has a clean, durable
mechanism for exactly this case — DT overlays selected via
armbianEnv.txt/armbian-config. This PR brings the forumworkaround into that mechanism.
How Has This Been Tested?
current(6.18) deb built —linux-dtb-current-rockchip64ships
rockchip/overlay/rockchip-rk3399-helios64-cpu-stability.dtbo(485 B);
dtc -I dtbdecompiles to the documented values.edge(7.0) deb built — identical overlay payload, verifiedthe same way.
Helios64 SD-card boot, current/6.18, Trixie minimal — overlay
enabled via
armbian-config. U-boot log:Applying kernel provided DT overlay rockchip-rk3399-helios64-cpu-stability.dtbo.Live DT after boot:
```
for n in 0 1 2 3 4 5 6 7; do
od -An -tx4 --endian=big
/sys/firmware/devicetree/base/opp-table-1/opp0$n/opp-microvolt
done
Overlay visible in
armbian-config→ System → Kernel →Manage device tree overlays (TUI writes
rk3399-helios64-cpu-stabilityto theoverlays=line inarmbianEnv.txt).Helios64 SD-card boot, edge/7.0 (kernel 7.0.7-edge-rockchip64,
Trixie minimal) — overlay enabled via the same TUI path, identical
end-to-end result:
opp00 000dbba0 000dbba0 001312d0 900000 900000 1250000 µV opp01 000dbba0 000dbba0 001312d0 opp02 000dbba0 000dbba0 001312d0 opp03 000e7ef0 000e7ef0 001312d0 950000 opp04 000fa3e8 000fa3e8 001312d0 1025000 opp05 0010c8e0 0010c8e0 001312d0 1100000 opp06 0011edd8 0011edd8 001312d0 1175000 opp07 00124f80 00124f80 001312d0 1200000 (unchanged)Changes since the latest review
is now documented inline in the
.dtso(a single-line commentabove each
opp0N { ... };), and the README block was tightenedto replace the imprecise "50-75 mV" phrasing with the actual
uniform +75 mV value plus the explicit mainline→overlay voltage
list. No functional change to the overlay payload.
Checklist