Skip to content

fix: platform profile handling when quiet/low-power is missing #280

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.

Sign up for GitHub

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

Open
Boogie61 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: platform profile handling when quiet/low-power is missing #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a258a4
fix(rog-platform): cycle platform profiles only through available cho…
Boogie61 Aug 8, 2026
d5368bc
fix(asusd): don't leave EPP applied when the platform profile is reje…
Boogie61 Aug 8, 2026
ed67a23
fix(asusd): fall back to any available profile, not just LowPower
Boogie61 Aug 8, 2026
647e726
fix(asusd): only persist a profile once the hardware accepted it
Boogie61 Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 76 additions & 49 deletions asusd/src/ctrl_platform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,37 +273,70 @@ 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<PlatformProfile> {
if configured == PlatformProfile::Custom {
return None;
Comment on lines +285 to +286

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect Custom profile semantics and all profile-application call paths.
ast-grep outline rog-platform/src/platform.rs --items all
rg -n -C 5 'PlatformProfile::Custom|normalize_profile|set_platform_profile_on_(battery|ac)|update_policy_ac_or_bat|platform_profile_choices' \
  asusd/src/ctrl_platform.rs rog-platform/src/platform.rs

Repository: OpenGamingCollective/asusctl

Length of output: 19169

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the complete policy update and profile setter implementations,
# plus configuration defaults and existing tests for Custom.
sed -n '250,460p' asusd/src/ctrl_platform.rs
sed -n '550,670p' asusd/src/ctrl_platform.rs
sed -n '850,915p' asusd/src/ctrl_platform.rs
rg -n -C 4 'profile_custom|platform_profile_on_(ac|battery)|Custom' asusd rog-platform | head -n 240

Repository: OpenGamingCollective/asusctl

Length of output: 31603

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Trace AC/battery tuning application and identify test coverage for the two
# power-source setters and update_policy_ac_or_bat.
rg -n -C 8 'async fn run_ac_or_bat_cmd|run_ac_or_bat_cmd|ac_profile_tunings|dc_profile_tunings|set_platform_profile_on_(ac|battery)|update_policy_ac_or_bat' \
  asusd/src/ctrl_platform.rs asusd/src
rg -n --glob '*.{rs,toml}' 'ctrl_platform|platform_profile_on_ac|platform_profile_on_battery|update_policy_ac_or_bat|Custom' \
  asusd rog-platform | head -n 260

Repository: OpenGamingCollective/asusctl

Length of output: 34994

Do not pass PlatformProfile::Custom to the kernel profile setter.

normalize_profile preserves Custom, but both power-source setters pass it to set_platform_profile, which rejects it because it is absent from platform_profile_choices. A stored Custom AC or battery policy also fails during update_policy_ac_or_bat, so its custom EPP is not applied.

Add a userspace-only Custom application path, or reject Custom before it enters an AC or battery policy. Add tests for both power sources.

🤖 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 `@asusd/src/ctrl_platform.rs` around lines 285 - 286, Prevent
PlatformProfile::Custom from reaching the kernel setter: update the AC and
battery policy flows, including update_policy_ac_or_bat and both power-source
setters, to apply Custom entirely in userspace or reject it before
set_platform_profile. Preserve custom EPP application for stored Custom
policies, and add coverage for Custom handling on both AC and battery sources.

}

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
} else {
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)
{
let mut cfg = self.config.lock().await;
if power_plugged {
cfg.platform_profile_on_ac = PlatformProfile::LowPower;
} else {
cfg.platform_profile_on_battery = PlatformProfile::LowPower;
}
cfg.write();
warn!(
"Configured profile Quiet is unavailable, falling back to LowPower for {}",
if power_plugged { "AC" } else { "battery" }
);
return PlatformProfile::LowPower;
}
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.
Expand Down Expand Up @@ -368,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);
}
}
Expand Down Expand Up @@ -491,15 +526,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 {
Expand Down Expand Up @@ -528,12 +565,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!(
Expand All @@ -548,6 +581,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 {
Expand Down Expand Up @@ -580,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(())
}

Expand Down Expand Up @@ -620,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(())
}

Expand Down
109 changes: 99 additions & 10 deletions rog-platform/src/platform.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> = 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
);
}
}

Expand Down