From 4a258a43401337b6ec851fde617c76a1eab92675 Mon Sep 17 00:00:00 2001 From: Boogie61 Date: Sat, 8 Aug 2026 19:40:16 +0300 Subject: [PATCH 1/4] fix(rog-platform): cycle platform profiles only through available choices PlatformProfile::next() took `choices` but only consulted it to decide between LowPower and Quiet. From Performance it therefore returned Quiet whenever LowPower was absent - including on hardware that has neither. The ROG Zephyrus G16 GU606AX exposes only balanced and performance: $ cat /sys/firmware/acpi/platform_profile_choices balanced performance so cycling was a dead end. Once on Performance the user could not get back to Balanced at all, by CLI or by hotkey: $ asusctl profile next Error: org.freedesktop.DBus.Error.NotSupported: RogPlatform: platform_profile: (quiet) not supported next() now filters a canonical cycle order by what the kernel actually reports, so the cycle always stays inside the supported set. Behaviour on hardware that does offer Quiet or LowPower is unchanged. If choices comes back empty the previous logic is kept rather than inventing a profile. Adds unit tests for the two/three-profile cases and for a current profile that is not part of the cycle. --- rog-platform/src/platform.rs | 109 +++++++++++++++++++++++++++++++---- 1 file changed, 99 insertions(+), 10 deletions(-) diff --git a/rog-platform/src/platform.rs b/rog-platform/src/platform.rs index 07d8760c5..0ec409347 100644 --- a/rog-platform/src/platform.rs +++ b/rog-platform/src/platform.rs @@ -154,18 +154,107 @@ pub enum PlatformProfile { } impl PlatformProfile { + /// Cycle order. `Custom` is excluded, it never appears in + /// `platform_profile_choices`. + const CYCLE_ORDER: [Self; 4] = [ + Self::Balanced, + Self::Performance, + Self::Quiet, + Self::LowPower, + ]; + + /// Next profile to cycle to, limited to what the kernel offers. Laptops + /// with only balanced and performance used to get Quiet here and fail. pub fn next(current: Self, choices: &[Self]) -> Self { - match current { - Self::Balanced => Self::Performance, - Self::Performance => { - if choices.contains(&Self::LowPower) { - Self::LowPower - } else { - Self::Quiet - } - } - Self::Quiet | Self::LowPower | Self::Custom => Self::Balanced, + let available: Vec = Self::CYCLE_ORDER + .iter() + .copied() + .filter(|p| choices.contains(p)) + .collect(); + + if available.is_empty() { + // Nothing reported, keep the old behaviour. + return match current { + Self::Balanced => Self::Performance, + Self::Performance => Self::Quiet, + Self::Quiet | Self::LowPower | Self::Custom => Self::Balanced, + }; } + + match available.iter().position(|p| *p == current) { + Some(idx) => available[(idx + 1) % available.len()], + // Not in the cycle (e.g. Custom): start over. + None => available[0], + } + } +} + +#[cfg(test)] +mod platform_profile_tests { + use super::PlatformProfile; + + #[test] + fn cycles_only_through_available_profiles() { + // Only balanced + performance, as on the GU606AX. + let choices = [ + PlatformProfile::Balanced, + PlatformProfile::Performance, + ]; + assert_eq!( + PlatformProfile::next(PlatformProfile::Balanced, &choices), + PlatformProfile::Performance + ); + // Was Quiet before, which this hardware doesn't have. + assert_eq!( + PlatformProfile::next(PlatformProfile::Performance, &choices), + PlatformProfile::Balanced + ); + } + + #[test] + fn cycles_through_low_power_when_offered() { + let choices = [ + PlatformProfile::Balanced, + PlatformProfile::Performance, + PlatformProfile::LowPower, + ]; + assert_eq!( + PlatformProfile::next(PlatformProfile::Performance, &choices), + PlatformProfile::LowPower + ); + assert_eq!( + PlatformProfile::next(PlatformProfile::LowPower, &choices), + PlatformProfile::Balanced + ); + } + + #[test] + fn cycles_through_quiet_on_older_kernels() { + let choices = [ + PlatformProfile::Balanced, + PlatformProfile::Performance, + PlatformProfile::Quiet, + ]; + assert_eq!( + PlatformProfile::next(PlatformProfile::Performance, &choices), + PlatformProfile::Quiet + ); + assert_eq!( + PlatformProfile::next(PlatformProfile::Quiet, &choices), + PlatformProfile::Balanced + ); + } + + #[test] + fn unknown_current_profile_restarts_the_cycle() { + let choices = [ + PlatformProfile::Balanced, + PlatformProfile::Performance, + ]; + assert_eq!( + PlatformProfile::next(PlatformProfile::Custom, &choices), + PlatformProfile::Balanced + ); } } From d5368bcb9253241dab6afc43cea65544ab8f1355 Mon Sep 17 00:00:00 2001 From: Boogie61 Date: Sat, 8 Aug 2026 19:40:45 +0300 Subject: [PATCH 2/4] fix(asusd): don't leave EPP applied when the platform profile is rejected Both next_platform_profile() and set_platform_profile() wrote the CPU energy-performance preference for the requested profile before trying to apply the profile itself, then returned early on failure. The EPP change was never rolled back, so a rejected profile left the machine in a state that matches neither profile. Measured on a GU606AX, which offers only balanced and performance. A single `asusctl profile next` from Performance: platform_profile performance (unchanged, the set failed) EPP power (applied anyway - Quiet's EPP) The CPU is left on the most power-saving preference while the profile still reads Performance, with no error visible to the user beyond the one-line D-Bus failure. Reorder both paths so the profile is validated and applied first, and EPP and the config write only happen once that succeeded. set_platform_profile already validated against platform_profile_choices - that check just ran too late to protect anything. --- asusd/src/ctrl_platform.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/asusd/src/ctrl_platform.rs b/asusd/src/ctrl_platform.rs index 9ca42fcc2..6e7ad58bb 100644 --- a/asusd/src/ctrl_platform.rs +++ b/asusd/src/ctrl_platform.rs @@ -491,15 +491,17 @@ impl CtrlPlatform { let policy = PlatformProfile::next(policy, &choices); if self.platform.has_platform_profile() { - let change_epp = self.config.lock().await.platform_profile_linked_epp; - let epp = self.get_config_epp_for_throttle(policy).await; - self.check_and_set_epp(epp, change_epp); + // Profile first: with EPP first, a rejected profile leaves the + // machine on the EPP of a profile it never applied. self.platform .set_platform_profile(policy.into()) .map_err(|err| { warn!("platform_profile {}", err); FdoErr::Failed(format!("RogPlatform: platform_profile: {err}")) })?; + let change_epp = self.config.lock().await.platform_profile_linked_epp; + let epp = self.get_config_epp_for_throttle(policy).await; + self.check_and_set_epp(epp, change_epp); self.enable_ppt_group_changed(&ctxt).await?; Ok(self.platform_profile_changed(&ctxt).await?) } else { @@ -528,12 +530,8 @@ impl CtrlPlatform { ) -> Result<(), FdoErr> { // TODO: watch for external changes if self.platform.has_platform_profile() { - let change_epp = self.config.lock().await.platform_profile_linked_epp; - let epp = self.get_config_epp_for_throttle(policy).await; - self.check_and_set_epp(epp, change_epp); - - self.config.lock().await.write(); - + // Check before writing anything, so a rejected profile can't + // leave EPP behind. let choices = self.platform.get_platform_profile_choices()?; if !choices.contains(&policy) { return Err(FdoErr::NotSupported(format!( @@ -548,6 +546,13 @@ impl CtrlPlatform { warn!("platform_profile {}", err); FdoErr::Failed(format!("RogPlatform: platform_profile: {err}")) })?; + + let change_epp = self.config.lock().await.platform_profile_linked_epp; + let epp = self.get_config_epp_for_throttle(policy).await; + self.check_and_set_epp(epp, change_epp); + + self.config.lock().await.write(); + self.enable_ppt_group_changed(&ctxt).await?; Ok(()) } else { From ed67a23da2491e639e1d26d0f4eaa8bdfe08f4f1 Mon Sep 17 00:00:00 2001 From: Boogie61 Date: Sat, 8 Aug 2026 19:41:15 +0300 Subject: [PATCH 3/4] fix(asusd): fall back to any available profile, not just LowPower The apply-time normalization only handled the Quiet -> LowPower rename from kernel 6.11. Its condition required LowPower to be present, so on hardware offering neither Quiet nor LowPower the configured profile was passed through unchanged and the write simply failed. The default config ships platform_profile_on_battery: Quiet, so on a GU606AX (balanced + performance only) a fresh install never switched profile on battery at all. Measured by unplugging: platform_profile performance -> performance (no change) throttle_thermal_policy 1 -> 1 (no change) The laptop stays at full performance on battery, silently. asusd sees the AC/DC transition but the profile write is rejected every time. Generalize the check: if the configured profile is not in platform_profile_choices, fall back to the first available of LowPower, Quiet, Balanced, Performance and persist that. Custom is excluded - it is a userspace concept for custom fan curves and never appears in choices, so normalizing it would clobber a deliberate user setting. --- asusd/src/ctrl_platform.rs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/asusd/src/ctrl_platform.rs b/asusd/src/ctrl_platform.rs index 6e7ad58bb..07f487042 100644 --- a/asusd/src/ctrl_platform.rs +++ b/asusd/src/ctrl_platform.rs @@ -280,25 +280,39 @@ impl CtrlPlatform { self.config.lock().await.platform_profile_on_battery }; - // Older configs may still contain Quiet on devices that only support LowPower. - // Normalize at apply-time so AC/BAT transitions still work correctly. - if configured == PlatformProfile::Quiet { - if let Ok(choices) = self.platform.get_platform_profile_choices() { - if !choices.contains(&PlatformProfile::Quiet) - && choices.contains(&PlatformProfile::LowPower) - { + // The configured profile may not exist here: the default is Quiet, + // 6.11+ renamed it to LowPower, and some laptops have neither. + // Normalize at apply-time so AC/BAT transitions still work. Custom is + // userspace-only and never in choices, so leave it alone. + if let Ok(choices) = self.platform.get_platform_profile_choices() { + if configured != PlatformProfile::Custom + && !choices.is_empty() + && !choices.contains(&configured) + { + // Prefer low-power, else the least aggressive one available. + let fallback = [ + PlatformProfile::LowPower, + PlatformProfile::Quiet, + PlatformProfile::Balanced, + PlatformProfile::Performance, + ] + .into_iter() + .find(|p| choices.contains(p)); + + if let Some(fallback) = fallback { let mut cfg = self.config.lock().await; if power_plugged { - cfg.platform_profile_on_ac = PlatformProfile::LowPower; + cfg.platform_profile_on_ac = fallback; } else { - cfg.platform_profile_on_battery = PlatformProfile::LowPower; + cfg.platform_profile_on_battery = fallback; } cfg.write(); warn!( - "Configured profile Quiet is unavailable, falling back to LowPower for {}", + "Configured profile {configured} is unavailable on this hardware, falling \ + back to {fallback} for {}", if power_plugged { "AC" } else { "battery" } ); - return PlatformProfile::LowPower; + return fallback; } } } From 647e726e58e6b2d4df0f46ee99626228f3ef26f8 Mon Sep 17 00:00:00 2001 From: Boogie61 Date: Sat, 8 Aug 2026 21:13:19 +0300 Subject: [PATCH 4/4] fix(asusd): only persist a profile once the hardware accepted it Review feedback: select_power_profile_for_source() wrote the normalized profile to the config before returning it, but the caller can still fail to apply it. That records a profile the hardware never took - the same write-before-confirm mistake this series fixes for EPP. Split the logic: normalize_profile() only computes, and the config write moves to update_policy_ac_or_bat() after set_platform_profile() succeeds. The AC and battery D-Bus setters had their own narrower Quiet -> LowPower normalization that missed the case where neither exists, and also wrote the config before applying. Both now share normalize_profile() and the same store-after-apply helper. --- asusd/src/ctrl_platform.rs | 116 ++++++++++++++++++++----------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/asusd/src/ctrl_platform.rs b/asusd/src/ctrl_platform.rs index 07f487042..18c40a223 100644 --- a/asusd/src/ctrl_platform.rs +++ b/asusd/src/ctrl_platform.rs @@ -273,6 +273,35 @@ impl CtrlPlatform { } } + /// Map a configured profile onto one the kernel actually offers. + /// + /// The default config uses Quiet, 6.11+ renamed it to LowPower, and some + /// laptops have neither. Returns `None` when `configured` is usable as-is. + /// Custom is userspace-only and never in choices, so it is left alone. + /// + /// This only computes: nothing is written here, so a profile that turns + /// out to be unappliable is never persisted. + fn normalize_profile(&self, configured: PlatformProfile) -> Option { + if configured == PlatformProfile::Custom { + return None; + } + + let choices = self.platform.get_platform_profile_choices().ok()?; + if choices.is_empty() || choices.contains(&configured) { + return None; + } + + // Prefer low-power, else the least aggressive one available. + [ + PlatformProfile::LowPower, + PlatformProfile::Quiet, + PlatformProfile::Balanced, + PlatformProfile::Performance, + ] + .into_iter() + .find(|p| choices.contains(p)) + } + async fn select_power_profile_for_source(&self, power_plugged: bool) -> PlatformProfile { let configured = if power_plugged { self.config.lock().await.platform_profile_on_ac @@ -280,44 +309,34 @@ impl CtrlPlatform { self.config.lock().await.platform_profile_on_battery }; - // The configured profile may not exist here: the default is Quiet, - // 6.11+ renamed it to LowPower, and some laptops have neither. - // Normalize at apply-time so AC/BAT transitions still work. Custom is - // userspace-only and never in choices, so leave it alone. - if let Ok(choices) = self.platform.get_platform_profile_choices() { - if configured != PlatformProfile::Custom - && !choices.is_empty() - && !choices.contains(&configured) - { - // Prefer low-power, else the least aggressive one available. - let fallback = [ - PlatformProfile::LowPower, - PlatformProfile::Quiet, - PlatformProfile::Balanced, - PlatformProfile::Performance, - ] - .into_iter() - .find(|p| choices.contains(p)); - - if let Some(fallback) = fallback { - let mut cfg = self.config.lock().await; - if power_plugged { - cfg.platform_profile_on_ac = fallback; - } else { - cfg.platform_profile_on_battery = fallback; - } - cfg.write(); - warn!( - "Configured profile {configured} is unavailable on this hardware, falling \ - back to {fallback} for {}", - if power_plugged { "AC" } else { "battery" } - ); - return fallback; - } + match self.normalize_profile(configured) { + Some(fallback) => { + warn!( + "Configured profile {configured} is unavailable on this hardware, falling \ + back to {fallback} for {}", + if power_plugged { "AC" } else { "battery" } + ); + fallback } + None => configured, } + } - configured + /// Persist the profile that was actually applied for this power source. + async fn store_applied_profile(&self, power_plugged: bool, profile: PlatformProfile) { + let mut cfg = self.config.lock().await; + let changed = if power_plugged { + let changed = cfg.platform_profile_on_ac != profile; + cfg.platform_profile_on_ac = profile; + changed + } else { + let changed = cfg.platform_profile_on_battery != profile; + cfg.platform_profile_on_battery = profile; + changed + }; + if changed { + cfg.write(); + } } /// Manage nvidia-powerd service based on current power state and config. @@ -382,6 +401,8 @@ impl CtrlPlatform { warn!("Failed to set platform profile {throttle:?} on AC/BAT change: {err}"); return; } + // Only now that the hardware took it is it worth recording. + self.store_applied_profile(power_plugged, throttle).await; self.check_and_set_epp(epp, change_epp); } } @@ -599,19 +620,12 @@ impl CtrlPlatform { #[zbus(signal_context)] ctxt: SignalEmitter<'_>, policy: PlatformProfile, ) -> Result<(), FdoErr> { - // If the requested profile isn't available on this platform, and it's - // `Quiet`, fall back to `LowPower` so we don't write an unavailable - // profile into the config file. - let mut chosen = policy; - if let Ok(choices) = self.platform.get_platform_profile_choices() { - if chosen == PlatformProfile::Quiet && !choices.contains(&PlatformProfile::Quiet) { - chosen = PlatformProfile::LowPower; - } - } + // Map onto something this hardware has, so an unavailable profile is + // never written to the config file. + let chosen = self.normalize_profile(policy).unwrap_or(policy); - self.config.lock().await.platform_profile_on_battery = chosen; self.set_platform_profile(ctxt, chosen).await?; - self.config.lock().await.write(); + self.store_applied_profile(false, chosen).await; Ok(()) } @@ -639,16 +653,10 @@ impl CtrlPlatform { policy: PlatformProfile, ) -> Result<(), FdoErr> { // Mirror the same fallback behavior for AC profile changes. - let mut chosen = policy; - if let Ok(choices) = self.platform.get_platform_profile_choices() { - if chosen == PlatformProfile::Quiet && !choices.contains(&PlatformProfile::Quiet) { - chosen = PlatformProfile::LowPower; - } - } + let chosen = self.normalize_profile(policy).unwrap_or(policy); - self.config.lock().await.platform_profile_on_ac = chosen; self.set_platform_profile(ctxt, chosen).await?; - self.config.lock().await.write(); + self.store_applied_profile(true, chosen).await; Ok(()) }