Conversation
TDP control did nothing on an Intel handheld. The plugin drives wattage through ryzenadj, which writes the AMD SMU mailbox over MMIO — on an Intel CPU that's a no-op then an error — so the whole feature was dead on Arc G3 / Xe3-class devices. Three new pure modules, each unit-tested: - lib/firmware-attributes.ts — discovers power rails by attribute name across every driver in /sys/class/firmware-attributes. A handheld whose vendor driver implements the interface publishes the exact envelope its firmware accepts (min_value/max_value on PL1), which is per-unit exact and works for a device that shipped after our table was last touched. applyFirmwareRange() folds that envelope into a device match: a known row keeps its hand-tuned battery cap and presets, clamped into the firmware interval; a fallback match derives presets from the range. - lib/rapl.ts — Intel RAPL powercap, resolving constraints by name rather than slot index and capturing both sustained and boost limits. - lib/gpu.ts — Intel GPU frequency control via the xe/i915 sysfs knobs, alongside the existing AMD path. AMD IS DELIBERATELY UNCHANGED. Generic rail discovery runs only when the CPU isn't AMD; ROG Ally and Legion Go keep their DMI-matched paths, in the same order, written in the same milliwatt unit, and ryzenadj keeps its precedence everywhere else. Letting discovery outrank ryzenadj on all AMD devices is a real improvement but it changes behaviour on hardware we can't test, so it stays a separate PR (#253) rather than riding along with Intel support. One deliberate difference on the DMI-matched paths: they carry scaleKnown: false, so a write the firmware rejects is retried once in the other unit and latches whichever is accepted, instead of throwing. That only fires where the old unconditional `watts * 1000` already failed outright — which is the state a modern-kernel ROG Ally appears to be in, since asus-armoury's ppt rails take watts and we send milliwatts. Split out of #253. Backend 3257 pass, UI 532 pass, typecheck clean. Untested on Intel hardware — I don't have an Intel handheld. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8sNiuexULKVgN3TyqAWsA
discoverPowerRails matched on the attribute *directory* name and never read current_value, so a stub or read-only driver publishing an empty ppt_pl1_spl/ would latch method="wmi" and starve the RAPL fallback — a device that works on main would lose TDP control entirely. Read the rail before claiming it, the way the DMI-matched probes already do. The unit-flip retry left the boost rails written in the unit it had just disproved: PL1 landed correctly while PL2/PL3 kept firmware defaults for that apply. Re-write them once the scale has latched. Generic discovery is now gated on a positive GenuineIntel rather than !isAmdCpu(). Both probes fail closed, so an unreadable /proc/cpuinfo on an AMD ROG Ally would have routed it into the new discovery path — exactly the AMD behaviour change this branch promises not to make. resolveZoneConstraints fell back to slot 0 only when nothing was named at all, so a zone naming peak_power/short_term but no long_term resolved to null where main took constraint_0. Fall back whenever long_term is missing — but never to the file already identified as short_term, since capping boost while believing we capped sustained draw is worse than reporting no RAPL control. Boost-rail write failures are warned-and-continued rather than thrown, which is a second (intended) difference from main on the WMI path: MSI's driver exposes only PL1 + PL2, and the old unconditional write of all three failed outright on anything without a PL3. Now stated in the code and in the PR description instead of being implicit. 5 new tests. Backend 3260 pass, typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K8sNiuexULKVgN3TyqAWsA
srsholmes
marked this pull request as draft
August 17, 2026 11:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split 3 of 3 out of #253, and the substantial one.
The problem
TDP control did nothing on an Intel handheld. The plugin drives wattage through
ryzenadj, which writes the AMD SMU mailbox over MMIO — on an Intel CPU that's a no-op then an error — so the whole feature was dead on Arc G3 / Xe3-class devices.What's added
Three new pure modules, each unit-tested:
lib/firmware-attributes.ts— discovers power rails by attribute name across every driver in/sys/class/firmware-attributes. A handheld whose vendor driver implements the interface publishes the exact envelope its firmware accepts (min_value/max_valueon PL1): per-unit exact, and the only correct source for a device that shipped after our device table was last touched.applyFirmwareRange()folds that envelope into a device match — a known row keeps its hand-tuned battery cap and presets clamped into the firmware interval, while a fallback match derives presets from the range.lib/rapl.ts— Intel RAPL powercap, resolving constraints by name rather than by slot index, and capturing both the sustained and boost limits.lib/gpu.ts— Intel GPU frequency control via thexe/i915sysfs knobs, alongside the existing AMD path.AMD is deliberately unchanged
This is the whole reason #253 was split. Generic rail discovery runs only on a positively identified Intel CPU — not merely "not AMD", so an unreadable
/proc/cpuinfokeeps every machine on the old detection order. ROG Ally and Legion Go keep their DMI-matched paths, probed in the same order and written in the same milliwatt unit; ryzenadj keeps its precedence everywhere else.Letting generic discovery outrank ryzenadj on all AMD devices is a real improvement, but it changes behaviour on hardware neither of us can test. That idea is written up in #253 rather than riding along with Intel support.
There are two deliberate differences on the DMI-matched paths, both stated here rather than left implicit:
scaleKnown: false, so a write the firmware rejects is retried once in the other unit and latches whichever is accepted, rather than throwing. That can only fire where the old unconditionalwatts * 1000already failed outright — which appears to be the state a modern-kernel ROG Ally is in today, sinceasus-armoury's ppt rails take watts and we send milliwatts. On a healthy write nothing changes. If the flip is also rejected, the original error propagates.fppt/sppt) write is warned and skipped instead of thrown. MSI's driver exposes only PL1 + PL2, so the old unconditional write of all three failed outright on anything without a PL3. Only observable where a write already fails.Review fixes
A review pass found four things, now fixed:
discoverPowerRailsmatched on the attribute directory name and never readcurrent_value, so a stub or read-only driver publishing an emptyppt_pl1_spl/would latchmethod="wmi"and starve the Intel RAPL fallback — losing TDP control on a device that works onmain.GenuineIntelrather than!isAmdCpu(). Both probes fail closed, so an unreadable/proc/cpuinfoon an AMD ROG Ally would otherwise have routed it into the new path — precisely the AMD change this PR promises not to make.resolveZoneConstraintsfell back to slot 0 only when nothing was named, so a zone namingpeak_power/short_termbut nolong_termresolved to null wheremaintookconstraint_0. It now falls back wheneverlong_termis missing, but never to the file already identified asshort_term.Testing
Backend 3260 pass, UI 532 pass, typecheck clean. The pure helpers are covered: firmware-attributes parsing, RAPL constraint resolution, GPU frequency writes, and
applyFirmwareRange.Not verified on hardware — I have no Intel handheld. The Intel paths are the ones wanting a real device, so OneXPlayer community testing would land here.
Related
No open issue tracks Intel handheld support — this came out of the work in #253.