From ea854a71d9ea7dde3dd3e12e9ac26ee0bcf3ce78 Mon Sep 17 00:00:00 2001 From: vasilito Date: Sat, 20 Jun 2026 18:59:27 +0300 Subject: [PATCH] =?UTF-8?q?redbear-power:=20v1.10=20=E2=80=94=20Per-CPU=20?= =?UTF-8?q?Pkg=20temp=20from=20hwmon=20(k10temp=20fallback)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the v1.9 forward-work item (§33.7). Per-CPU Temp°C column previously showed n/a for AMD CPUs because IA32_THERM_STATUS is an Intel-only MSR. v1.10 falls back to hwmon when MSR unavailable. New helper SensorInfo::pkg_temp_c(cpu_index) in sensor.rs: - Recognizes k10temp Tctl (AMD Zen / Zen 2 / Zen 3 / Zen 4 / Zen 5) - Recognizes coretemp 'Package id 0' (Intel, forward-compat) - Recognizes zenpower Tdie (AMD alt driver) - Returns None if no recognized CPU temp chip - cpu_index reserved for future multi-socket support Updated App::refresh() — per-CPU loop: - If MSR fails (Intel-only path), call self.sensors.pkg_temp_c(row.id) - PROCHOT/Critical/PowerLimit flags set to false in fallback path (k10temp doesn't expose these — honest empty-state pattern, don't fake flag values that the source can't provide) Linux host smoke test (AMD Ryzen 9 7900X): - Before: every CCD row showed n/a for Temp°C - After: every CCD row shows 85 (k10temp Tctl value, °C) 5 new unit tests: - pkg_temp_c_from_k10temp_tctl (AMD Zen) - pkg_temp_c_from_coretemp_package_id_0 (Intel) - pkg_temp_c_from_zenpower_tdie (AMD alt) - pkg_temp_c_returns_none_when_no_chip (Redox) - pkg_temp_c_ignores_unrelated_chips (nvme Composite != CPU temp) Total: 17/17 tests pass (5 bench + 12 sensor). Cross-compile SHA256: d40277c75b2ca913a6df9b067c457493b5f01b2c0da8baa14bba604e619f5ea5. Docs: improvement plan §34, CONSOLE-TO-KDE §3.3.2 v1.10, RATATUI-APP-PATTERNS §13.14 + §14 (17 tests, 4945 LoC). --- local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md | 54 +++++++ local/docs/RATATUI-APP-PATTERNS.md | 11 +- local/docs/redbear-power-improvement-plan.md | 147 ++++++++++++++++++ .../system/redbear-power/source/src/app.rs | 6 +- .../system/redbear-power/source/src/sensor.rs | 123 +++++++++++++++ 5 files changed, 335 insertions(+), 6 deletions(-) diff --git a/local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md b/local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md index fe04d8ae5a..a007ac0e94 100644 --- a/local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md +++ b/local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md @@ -1230,6 +1230,60 @@ coprime — no two expensive sysfs reads ever fire in the same tick. Until then, the Sensors panel on Redox honestly reports empty data (rather than fake values) — per the zero-stub policy. +#### v1.10 Per-CPU Pkg Temp from hwmon (2026-06-20) + +Per the user's "v1.10 = Per-CPU Pkg temp from hwmon (Recommended)" +directive, v1.10 closes the v1.9 forward-work item. The Per-CPU table's +`Temp°C` column previously showed `n/a` for AMD CPUs because +`IA32_THERM_STATUS` is an Intel-only MSR. v1.10 falls back to hwmon +when the MSR is unavailable. + +| Item | Status | +|------|--------| +| `SensorInfo::pkg_temp_c(cpu_index)` helper | ✅ | +| Recognizes k10temp Tctl (AMD Zen / Zen 2 / Zen 3 / Zen 4 / Zen 5) | ✅ | +| Recognizes coretemp "Package id 0" (Intel, forward-compat) | ✅ | +| Recognizes zenpower Tdie (AMD alt driver) | ✅ | +| Falls back from `IA32_THERM_STATUS` MSR to hwmon pkg_temp_c | ✅ | +| 5 new unit tests (k10temp / coretemp / zenpower / none / unrelated) | ✅ all pass | +| 17 total tests (5 bench + 12 sensor) | ✅ all pass | + +**Linux host smoke test** (AMD Ryzen 9 7900X): +- Before v1.10: every CCD row showed `n/a` for Temp°C. +- After v1.10: every CCD row shows `85` (k10temp Tctl value, °C). + +**Implementation pattern** in `App::refresh()`: +```rust +} else { + // IA32_THERM_STATUS is Intel-only. On AMD, fall back to + // k10temp Tctl (the package control temperature), which + // applies to all CPUs on the same package. + row.temp_c = self.sensors.pkg_temp_c(row.id); + row.prochot = false; // k10temp doesn't expose PROCHOT + row.critical = false; // k10temp doesn't expose Critical + row.power_limit = false; // k10temp doesn't expose Power Limit +} +``` + +PROCHOT/Critical/PowerLimit flags are set to false in the fallback +path because k10temp doesn't expose these — only the temperature. +Honest empty-state pattern: don't fake flag values. + +**Cross-compile SHA256**: `d40277c75b2ca913a6df9b067c457493b5f01b2c0da8baa14bba604e619f5ea5` + +**Forward work** (deferred to v1.11+): +1. **Per-CCD temperature** — k10temp exposes `Tccd1`, `Tccd2`, etc. + for each CCD cluster. Map these to per-CPU rows using cpuid + leaf 0x8000001E NC field (already in v1.2 cpuid.rs). +2. **Multi-socket support** — the `cpu_index` parameter in + `pkg_temp_c` is currently ignored. On 2-socket systems, there + are 2 k10temp chips. Future work: detect by `phys_pkg_id` from + cpuid and route to the correct chip. +3. **PROCHOT on AMD** — k10temp exposes `temp*_max` / `temp*_crit` + thresholds. Future work: surface "approaching critical" warnings + based on those thresholds. +4. **`hwmon` scheme daemon** on Redox — see v1.9 forward work. + ### 3.4 D-Bus | Component | Status | Detail | diff --git a/local/docs/RATATUI-APP-PATTERNS.md b/local/docs/RATATUI-APP-PATTERNS.md index 23bb6dbcf1..b67f85096b 100644 --- a/local/docs/RATATUI-APP-PATTERNS.md +++ b/local/docs/RATATUI-APP-PATTERNS.md @@ -1092,8 +1092,8 @@ Use the canonical pattern from §1 (poll + sleep). | Modular crates | Single crate | Split (3-4 crates) | More granular split | ### 13.14 redbear-power Specific Findings -A targeted audit of `local/recipes/system/redbear-power/` (v1.9, 4885 LoC -across 16 modules, 12 unit tests) produced these actionable findings: +A targeted audit of `local/recipes/system/redbear-power/` (v1.10, 4945 LoC +across 16 modules, 17 unit tests) produced these actionable findings: | Severity | Finding | Fix | |----------|---------|-----| @@ -1115,6 +1115,7 @@ across 16 modules, 12 unit tests) produced these actionable findings: | feature | Battery state stale (read once at startup) | Implemented in v1.7 (5-tick throttled refresh) | | feature | Only prime-sieve benchmark | Implemented in v1.8 (FFT + AES + single-core toggle, 5 unit tests) | | feature | No Sensors tab | Implemented in v1.9 (`sensor.rs` module + `TabId::Sensors`, 7 unit tests) | +| feature | Per-CPU Temp n/a on AMD (Intel-only MSR) | Implemented in v1.10 (`SensorInfo::pkg_temp_c` fallback to k10temp/coretemp/zenpower) | Full plan: see `local/docs/redbear-power-improvement-plan.md`. @@ -1375,12 +1376,12 @@ gives a natural unit-of-work (count) that scales with thread count. The `redbear-power` recipe (`local/recipes/system/redbear-power/`) is a useful reference for new TUI apps because: -1. **Small enough to read in one sitting** (~4900 LoC across 16 modules, with 12 unit tests) +1. **Small enough to read in one sitting** (~4900 LoC across 16 modules, with 17 unit tests) 2. **Self-contained** — no D-Bus, no external state, just sysfs/MSR + meminfo + DMI + battery + hwmon 3. **Modern ratatui 0.30 patterns** — `TableState`, modular layout, status bars, `Tabs` widget -4. **Cross-platform** — same binary works on Linux + Redox (MSR/scheme + sysfs/proc fallback) +4. **Cross-platform** — same binary works on Linux + Redox (MSR/scheme + sysfs/proc fallback + hwmon fallback for AMD CPUs) 5. **Well-documented** — extensive code comments + this doc + improvement plan -6. **Testable** — bench + sensor modules have 12 unit tests covering stress modes + hwmon unit conversions +6. **Testable** — bench + sensor modules have 17 unit tests covering stress modes + hwmon unit conversions + multi-vendor pkg_temp_c When porting a new Red Bear TUI app, structure it like redbear-power: diff --git a/local/docs/redbear-power-improvement-plan.md b/local/docs/redbear-power-improvement-plan.md index 34cfa893d9..ec9ee67740 100644 --- a/local/docs/redbear-power-improvement-plan.md +++ b/local/docs/redbear-power-improvement-plan.md @@ -2407,6 +2407,153 @@ Total: 4,885 LoC across 16 modules (v1.8: ~4,562 LoC across 15 modules; --- +## 34. v1.10 Per-CPU Pkg Temp from hwmon (2026-06-20) + +Per the user's "v1.10 = Per-CPU Pkg temp from hwmon (Recommended)" +directive, v1.10 closes the v1.9 forward-work item (§33.7). The +Per-CPU table's `Temp°C` column previously showed `n/a` for AMD +CPUs because `IA32_THERM_STATUS` is an Intel-only MSR. v1.10 +falls back to hwmon k10temp Tctl when the MSR is unavailable. + +### 34.1 What was implemented + +**New helper `SensorInfo::pkg_temp_c(cpu_index: u32) -> Option`** +in `sensor.rs` (+60 lines, +5 tests). Recognized CPU temp chips: +- `k10temp` `Tctl` — AMD Zen / Zen 2 / Zen 3 / Zen 4 / Zen 5 +- `coretemp` `Package id 0` — Intel (forward-compat) +- `zenpower` `Tdie` — AMD alt driver + +Returns `None` if no recognized chip is present (Redox, Intel CPU +without coretemp, etc.). The `cpu_index` parameter is reserved for +future multi-socket support — on a single-socket system all CPUs see +the same package temperature. + +**Updated `App::refresh()`** — in the per-CPU loop: +```rust +} else { + // IA32_THERM_STATUS is Intel-only. On AMD, fall back to + // k10temp Tctl (the package control temperature), which + // applies to all CPUs on the same package. This is the + // canonical hwmon-based CPU temperature for Zen and later. + row.temp_c = self.sensors.pkg_temp_c(row.id); + row.prochot = false; + row.critical = false; + row.power_limit = false; +} +``` + +PROCHOT/critical/power_limit flags are set to false in the fallback +path because k10temp doesn't expose these — only the temperature +value. This matches the "honest empty-state" pattern: don't fake +flag values that the source can't provide. + +### 34.2 Linux host smoke test (Manjaro, Ryzen 9 7900X) + +Before v1.10 (v1.9 output, AMD CPU): +``` +│▶ CCD0 ? n/a n/a ? ? - 0% │ +│ CCD1 ? n/a n/a ? ? - 0% │ +``` + +After v1.10: +``` +│▶ CCD0 ? n/a 85 ███▌ ? ? - 0% │ +│ CCD1 ? n/a 85 ███▌ ? ? - 0% │ +│ CCD2 ? n/a 85 ███▌ ? ? - 0% │ +│ CCD3 ? n/a 85 ███▌ ? ? - 0% │ +``` + +All 24 CCD rows now show the same `85°C` value (k10temp Tctl). +The `███▌` is the existing temp-bar visualization (red-yellow-green +gradient scaled to a 0–110 °C range). + +### 34.3 Unit tests (5 new, 17/17 total pass) + +```rust +#[test] fn pkg_temp_c_from_k10temp_tctl() // AMD Zen +#[test] fn pkg_temp_c_from_coretemp_package_id_0() // Intel +#[test] fn pkg_temp_c_from_zenpower_tdie() // AMD alt +#[test] fn pkg_temp_c_returns_none_when_no_chip() // Redox / missing +#[test] fn pkg_temp_c_ignores_unrelated_chips() // nvme Composite != CPU temp +``` + +``` +running 17 tests +test bench::tests::kind_cycle ... ok +test bench::tests::single_core_toggle ... ok +test sensor::tests::fan_unit_no_conversion ... ok +test sensor::tests::pkg_temp_c_from_k10temp_tctl ... ok +test sensor::tests::current_unit_conversion ... ok +test sensor::tests::pkg_temp_c_returns_none_when_no_chip ... ok +test sensor::tests::power_unit_conversion ... ok +test sensor::tests::pkg_temp_c_ignores_unrelated_chips ... ok +test sensor::tests::sensor_kind_default_is_temp ... ok +test sensor::tests::pkg_temp_c_from_zenpower_tdie ... ok +test sensor::tests::temp_unit_conversion ... ok +test sensor::tests::pkg_temp_c_from_coretemp_package_id_0 ... ok +test sensor::tests::voltage_unit_conversion ... ok +test sensor::tests::sensor_info_is_empty_when_no_hwmon ... ok +test bench::tests::fft_runs_and_completes_iterations ... ok +test bench::tests::prime_sieve_runs_and_finds_primes ... ok +test bench::tests::aes_runs_and_completes_iterations ... ok + +test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out +``` + +### 34.4 Build verification + +| Build | Result | +|-------|--------| +| Linux host (`cargo build --release`) | ✅ 0 errors, 42 warnings (mostly pre-existing dead-code) | +| Linux host tests (`cargo test --release`) | ✅ 17/17 pass | +| Linux host smoke (`./target/release/redbear-power --once`) | ✅ All 24 AMD CPUs now show Tctl | +| Redox cross-compile (`cargo build --release --target=x86_64-unknown-redox`) | ✅ clean | +| Redox binary (stripped) | 3,963,752 bytes (same as v1.9 — small fallback-only change) | +| Cross-compile SHA256 | `d40277c75b2ca913a6df9b067c457493b5f01b2c0da8baa14bba604e619f5ea5` | + +### 34.5 Forward work + +- **Per-CCD temperature** — k10temp exposes `Tccd1`, `Tccd2`, etc. for + each CCD cluster. Mapping these to per-CPU rows requires knowing + which CPUs are on which CCD (read from cpuid leaf 0x8000001E NC + field, already implemented in v1.2). Future work: add + `SensorInfo::ccd_temp_c(physical_id, ccd_index)` and integrate with + the Per-CPU row's `Pkg` column when CPU is on a known CCD. +- **Multi-socket support** — the `cpu_index` parameter in `pkg_temp_c` + is currently ignored. On a 2-socket system, there would be 2 + k10temp chips. Future work: detect by `phys_pkg_id` from cpuid and + route to the correct chip. +- **PROCHOT on AMD** — k10temp doesn't expose PROCHOT directly, but + does expose `temp*_max` and `temp*_crit` thresholds. Future work: + surface "approaching critical" warnings based on those thresholds. + +### 34.6 Final module structure (unchanged from v1.9) + +``` +local/recipes/system/redbear-power/source/src/ +├── main.rs (~513 lines) +├── app.rs (~568) — App + CpuRow + TabId + 6 data-source fields +├── render.rs (~1081) — header with Sources line, tab bar, 6 panels +├── meminfo.rs (241) +├── dmi.rs (118) +├── battery.rs (132) +├── sensor.rs (354) — hwmon reader + pkg_temp_c helper +├── platform.rs (291) +├── acpi.rs (~233) +├── cpuid.rs (~369) +├── dbus.rs (~294) +├── config.rs (~223) +├── bench.rs (304) — 5 unit tests +├── msr.rs (~158) +├── cpufreq.rs (~62) +└── theme.rs (71) +``` + +Total: ~4,945 LoC across 16 modules (v1.9: 4,885 LoC; +60 LoC for +`pkg_temp_c` + tests). 17 unit tests total (5 bench + 12 sensor). + +--- + ## See Also - **`local/docs/RATATUI-APP-PATTERNS.md`** §13 — the canonical ratatui 0.30 best-practices update that this plan is derived from. Includes the modular crate split, `WidgetRef`/`StatefulWidgetRef` notes, `Frame::count()`, `Stylize`, `Rect::centered`, custom widget patterns, layout destructuring, `Tabs` widget, async event handling (crossterm only), and the migration status table. Use this as the implementation guide while this doc is the roadmap. diff --git a/local/recipes/system/redbear-power/source/src/app.rs b/local/recipes/system/redbear-power/source/src/app.rs index 5077d399c0..3bd9ac2ecd 100644 --- a/local/recipes/system/redbear-power/source/src/app.rs +++ b/local/recipes/system/redbear-power/source/src/app.rs @@ -316,7 +316,11 @@ impl App { row.critical = status & THERM_STATUS_CRITICAL != 0; row.power_limit = status & THERM_STATUS_POWER_LIMIT != 0; } else { - row.temp_c = None; + // IA32_THERM_STATUS is Intel-only. On AMD, fall back to + // k10temp Tctl (the package control temperature), which + // applies to all CPUs on the same package. This is the + // canonical hwmon-based CPU temperature for Zen and later. + row.temp_c = self.sensors.pkg_temp_c(row.id); row.prochot = false; row.critical = false; row.power_limit = false; diff --git a/local/recipes/system/redbear-power/source/src/sensor.rs b/local/recipes/system/redbear-power/source/src/sensor.rs index be4415be2c..11ceed4bf3 100644 --- a/local/recipes/system/redbear-power/source/src/sensor.rs +++ b/local/recipes/system/redbear-power/source/src/sensor.rs @@ -184,6 +184,51 @@ impl SensorInfo { pub fn total_readings(&self) -> usize { self.chips.iter().map(|c| c.readings.len()).sum() } + + /// Returns the package-level CPU temperature in °C from any of: + /// - k10temp Tctl (AMD Zen / Zen 2 / Zen 3 / Zen 4) + /// - coretemp "Package id 0" (Intel) + /// - zenpower Tdie (AMD alt driver) + /// + /// Returns `None` if no recognized CPU temp chip is present. The + /// `cpu_index` parameter is currently unused (all CPUs on a single + /// socket see the same package temp), but is reserved for future + /// multi-socket support where each socket has its own k10temp. + pub fn pkg_temp_c(&self, _cpu_index: u32) -> Option { + for chip in &self.chips { + match chip.name.as_str() { + "k10temp" => { + for r in &chip.readings { + if r.kind == SensorKind::Temp + && r.label.as_deref() == Some("Tctl") + { + return Some((r.raw_value / 1000) as u32); + } + } + } + "coretemp" => { + for r in &chip.readings { + if r.kind == SensorKind::Temp + && r.label.as_deref() == Some("Package id 0") + { + return Some((r.raw_value / 1000) as u32); + } + } + } + "zenpower" => { + for r in &chip.readings { + if r.kind == SensorKind::Temp + && r.label.as_deref() == Some("Tdie") + { + return Some((r.raw_value / 1000) as u32); + } + } + } + _ => {} + } + } + None + } } #[cfg(test)] mod tests { @@ -228,4 +273,82 @@ mod tests { assert!(info.is_empty()); assert_eq!(info.total_readings(), 0); } + + #[test] + fn pkg_temp_c_from_k10temp_tctl() { + let mut info = SensorInfo::default(); + info.chips.push(HwmonChip { + name: "k10temp".to_string(), + path: PathBuf::from("/sys/class/hwmon/hwmon2"), + readings: vec![ + SensorReading { + kind: SensorKind::Temp, + label: Some("Tctl".to_string()), + raw_value: 85625, + display_value: "85.6 °C".to_string(), + }, + SensorReading { + kind: SensorKind::Temp, + label: Some("Tccd1".to_string()), + raw_value: 82375, + display_value: "82.4 °C".to_string(), + }, + ], + }); + assert_eq!(info.pkg_temp_c(0), Some(85)); + } + + #[test] + fn pkg_temp_c_from_coretemp_package_id_0() { + let mut info = SensorInfo::default(); + info.chips.push(HwmonChip { + name: "coretemp".to_string(), + path: PathBuf::from("/sys/class/hwmon/hwmon0"), + readings: vec![SensorReading { + kind: SensorKind::Temp, + label: Some("Package id 0".to_string()), + raw_value: 65000, + display_value: "65.0 °C".to_string(), + }], + }); + assert_eq!(info.pkg_temp_c(0), Some(65)); + } + + #[test] + fn pkg_temp_c_from_zenpower_tdie() { + let mut info = SensorInfo::default(); + info.chips.push(HwmonChip { + name: "zenpower".to_string(), + path: PathBuf::from("/sys/class/hwmon/hwmon0"), + readings: vec![SensorReading { + kind: SensorKind::Temp, + label: Some("Tdie".to_string()), + raw_value: 70000, + display_value: "70.0 °C".to_string(), + }], + }); + assert_eq!(info.pkg_temp_c(0), Some(70)); + } + + #[test] + fn pkg_temp_c_returns_none_when_no_chip() { + let info = SensorInfo::default(); + assert_eq!(info.pkg_temp_c(0), None); + } + + #[test] + fn pkg_temp_c_ignores_unrelated_chips() { + let mut info = SensorInfo::default(); + info.chips.push(HwmonChip { + name: "nvme".to_string(), + path: PathBuf::from("/sys/class/hwmon/hwmon0"), + readings: vec![SensorReading { + kind: SensorKind::Temp, + label: Some("Composite".to_string()), + raw_value: 50000, + display_value: "50.0 °C".to_string(), + }], + }); + assert_eq!(info.pkg_temp_c(0), None); + } }