diff --git a/lact-client/src/lib.rs b/lact-client/src/lib.rs index 3b40d2b..5de138e 100644 --- a/lact-client/src/lib.rs +++ b/lact-client/src/lib.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Context}; use nix::unistd::getuid; use schema::{ amdgpu_sysfs::gpu_handle::{power_profile_mode::PowerProfileModesTable, PerformanceLevel}, - request::SetClocksCommand, + request::{ConfirmCommand, SetClocksCommand}, ClocksInfo, DeviceInfo, DeviceListEntry, DeviceStats, FanCurveMap, Request, Response, SystemInfo, }; @@ -104,13 +104,12 @@ impl DaemonClient { id: &str, enabled: bool, curve: Option, - ) -> anyhow::Result<()> { - self.make_request::<()>(Request::SetFanControl { id, enabled, curve })? - .inner()?; - Ok(()) + ) -> anyhow::Result { + self.make_request(Request::SetFanControl { id, enabled, curve })? + .inner() } - pub fn set_power_cap(&self, id: &str, cap: Option) -> anyhow::Result<()> { + pub fn set_power_cap(&self, id: &str, cap: Option) -> anyhow::Result { self.make_request(Request::SetPowerCap { id, cap })?.inner() } @@ -129,7 +128,7 @@ impl DaemonClient { &self, id: &str, performance_level: PerformanceLevel, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { self.make_request(Request::SetPerformanceLevel { id, performance_level, @@ -137,15 +136,29 @@ impl DaemonClient { .inner() } - pub fn set_clocks_value(&self, id: &str, command: SetClocksCommand) -> anyhow::Result<()> { + pub fn set_clocks_value(&self, id: &str, command: SetClocksCommand) -> anyhow::Result { self.make_request(Request::SetClocksValue { id, command })? .inner() } - pub fn set_power_profile_mode(&self, id: &str, index: Option) -> anyhow::Result<()> { + pub fn batch_set_clocks_value( + &self, + id: &str, + commands: Vec, + ) -> anyhow::Result { + self.make_request(Request::BatchSetClocksValue { id, commands })? + .inner() + } + + pub fn set_power_profile_mode(&self, id: &str, index: Option) -> anyhow::Result { self.make_request(Request::SetPowerProfileMode { id, index })? .inner() } + + pub fn confirm_pending_config(&self, command: ConfirmCommand) -> anyhow::Result<()> { + self.make_request(Request::ConfirmPendingConfig(command))? + .inner() + } } fn get_socket_path() -> Option { diff --git a/lact-daemon/src/config.rs b/lact-daemon/src/config.rs index e23c4a8..18f0766 100644 --- a/lact-daemon/src/config.rs +++ b/lact-daemon/src/config.rs @@ -1,6 +1,6 @@ use crate::server::gpu_controller::fan_control::FanCurve; use anyhow::Context; -use lact_schema::amdgpu_sysfs::gpu_handle::PerformanceLevel; +use lact_schema::{amdgpu_sysfs::gpu_handle::PerformanceLevel, request::SetClocksCommand}; use nix::unistd::getuid; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; @@ -10,12 +10,24 @@ use tracing::debug; const FILE_NAME: &str = "config.yaml"; const DEFAULT_ADMIN_GROUPS: [&str; 2] = ["wheel", "sudo"]; -#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Config { pub daemon: Daemon, + #[serde(default = "default_apply_settings_timer")] + pub apply_settings_timer: u64, pub gpus: HashMap, } +impl Default for Config { + fn default() -> Self { + Self { + daemon: Daemon::default(), + apply_settings_timer: default_apply_settings_timer(), + gpus: HashMap::new(), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct Daemon { pub log_level: String, @@ -64,6 +76,28 @@ impl Gpu { .iter() .any(Option::is_some) } + + pub fn apply_clocks_command(&mut self, command: &SetClocksCommand) { + match command { + SetClocksCommand::MaxCoreClock(clock) => self.max_core_clock = Some(*clock), + SetClocksCommand::MaxMemoryClock(clock) => self.max_memory_clock = Some(*clock), + SetClocksCommand::MaxVoltage(voltage) => self.max_voltage = Some(*voltage), + SetClocksCommand::MinCoreClock(clock) => self.min_core_clock = Some(*clock), + SetClocksCommand::MinMemoryClock(clock) => self.min_memory_clock = Some(*clock), + SetClocksCommand::MinVoltage(voltage) => self.min_voltage = Some(*voltage), + SetClocksCommand::VoltageOffset(offset) => self.voltage_offset = Some(*offset), + SetClocksCommand::Reset => { + self.min_core_clock = None; + self.min_memory_clock = None; + self.min_voltage = None; + self.max_core_clock = None; + self.max_memory_clock = None; + self.max_voltage = None; + + assert!(!self.is_core_clocks_used()); + } + } + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -119,6 +153,10 @@ fn get_path() -> PathBuf { } } +fn default_apply_settings_timer() -> u64 { + 5 +} + #[cfg(test)] mod tests { use super::{Config, Daemon, FanControlSettings, Gpu}; @@ -141,6 +179,7 @@ mod tests { }, )] .into(), + ..Default::default() }; let data = serde_yaml::to_string(&config).unwrap(); let deserialized_config: Config = serde_yaml::from_str(&data).unwrap(); diff --git a/lact-daemon/src/server/gpu_controller/mod.rs b/lact-daemon/src/server/gpu_controller/mod.rs index da11af6..0ce3123 100644 --- a/lact-daemon/src/server/gpu_controller/mod.rs +++ b/lact-daemon/src/server/gpu_controller/mod.rs @@ -407,6 +407,9 @@ impl GpuController { self.handle.set_active_power_profile_mode(mode_index)?; } + // Reset the clocks table in case the settings get reverted back to not having a clocks value configured + self.handle.reset_clocks_table().ok(); + if config.is_core_clocks_used() { let mut table = self.handle.get_clocks_table()?; diff --git a/lact-daemon/src/server/handler.rs b/lact-daemon/src/server/handler.rs index 892ce4c..96b8623 100644 --- a/lact-daemon/src/server/handler.rs +++ b/lact-daemon/src/server/handler.rs @@ -3,17 +3,17 @@ use crate::config::{self, Config, FanControlSettings}; use anyhow::{anyhow, Context}; use lact_schema::{ amdgpu_sysfs::gpu_handle::{power_profile_mode::PowerProfileModesTable, PerformanceLevel}, - request::SetClocksCommand, + request::{ConfirmCommand, SetClocksCommand}, ClocksInfo, DeviceInfo, DeviceListEntry, DeviceStats, FanCurveMap, }; use std::{ collections::HashMap, env, path::PathBuf, - sync::{Arc, RwLock}, + sync::{Arc, Mutex, RwLock}, time::Duration, }; -use tokio::time::sleep; +use tokio::{sync::oneshot, time::sleep}; use tracing::{debug, error, info, trace, warn}; const CONTROLLERS_LOAD_RETRY_ATTEMPTS: u8 = 5; @@ -23,6 +23,7 @@ const CONTROLLERS_LOAD_RETRY_INTERVAL: u64 = 1; pub struct Handler { pub config: Arc>, pub gpu_controllers: Arc>, + confirm_config_tx: Arc>>>, } impl<'a> Handler { @@ -46,6 +47,7 @@ impl<'a> Handler { let handler = Self { gpu_controllers: Arc::new(controllers), config: Arc::new(RwLock::new(config)), + confirm_config_tx: Arc::new(Mutex::new(None)), }; handler.load_config().await; @@ -70,38 +72,105 @@ impl<'a> Handler { &self, id: String, f: F, - ) -> anyhow::Result<()> { - let current_config = self - .config - .read() + ) -> anyhow::Result { + if self + .confirm_config_tx + .try_lock() .map_err(|err| anyhow!("{err}"))? - .gpus - .get(&id) - .cloned() - .unwrap_or_default(); + .is_some() + { + return Err(anyhow!( + "There is an unconfirmed configuration change pending" + )); + } - let mut new_config = current_config.clone(); + let (gpu_config, apply_timer) = { + let config = self.config.read().map_err(|err| anyhow!("{err}"))?; + let apply_timer = config.apply_settings_timer; + let gpu_config = config.gpus.get(&id).cloned().unwrap_or_default(); + (gpu_config, apply_timer) + }; + + let mut new_config = gpu_config.clone(); f(&mut new_config); let controller = self.controller_by_id(&id)?; match controller.apply_config(&new_config).await { Ok(()) => { - let mut config_guard = self.config.write().unwrap(); - config_guard.gpus.insert(id, new_config); - config_guard.save()?; - Ok(()) + self.wait_config_confirm(id, gpu_config, new_config, apply_timer) + .await?; + Ok(apply_timer) } Err(apply_err) => { - error!("Could not apply settings: {apply_err:#}"); - match controller.apply_config(¤t_config).await { - Ok(()) => Err(apply_err.context("Could not apply settings")), - Err(err) => Err(anyhow!("Could not apply settings, and could not reset to default settings: {err:#}")), - } + error!("could not apply settings: {apply_err:#}"); + match controller.apply_config(&gpu_config).await { + Ok(()) => Err(apply_err.context("Could not apply settings")), + Err(err) => Err(anyhow!("Could not apply settings, and could not reset to default settings: {err:#}")), + } } } } + /// Should be called after applying new config without writing it + async fn wait_config_confirm( + &self, + id: String, + previous_config: config::Gpu, + new_config: config::Gpu, + apply_timer: u64, + ) -> anyhow::Result<()> { + let (tx, rx) = oneshot::channel(); + *self + .confirm_config_tx + .try_lock() + .map_err(|err| anyhow!("{err}"))? = Some(tx); + + let handler = self.clone(); + + tokio::task::spawn(async move { + let controller = handler + .controller_by_id(&id) + .expect("GPU controller disappeared"); + + tokio::select! { + _ = tokio::time::sleep(Duration::from_secs(apply_timer)) => { + info!("no confirmation received, reverting settings"); + + if let Err(err) = controller.apply_config(&previous_config).await { + error!("could not revert settings: {err:#}"); + } + } + result = rx => { + match result { + Ok(ConfirmCommand::Confirm) => { + info!("saving updated config"); + + let mut config_guard = handler.config.write().unwrap(); + config_guard.gpus.insert(id, new_config); + + if let Err(err) = config_guard.save() { + error!("{err}"); + } + } + Ok(ConfirmCommand::Revert) | Err(_) => { + if let Err(err) = controller.apply_config(&previous_config).await { + error!("could not revert settings: {err:#}"); + } + } + } + } + } + + match handler.confirm_config_tx.try_lock() { + Ok(mut guard) => *guard = None, + Err(err) => error!("{err}"), + } + }); + + Ok(()) + } + fn controller_by_id(&self, id: &str) -> anyhow::Result<&GpuController> { Ok(self .gpu_controllers @@ -145,7 +214,7 @@ impl<'a> Handler { id: &str, enabled: bool, curve: Option, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { let settings = match curve { Some(raw_curve) => { let curve = FanCurve(raw_curve); @@ -175,7 +244,7 @@ impl<'a> Handler { .await } - pub async fn set_power_cap(&'a self, id: &str, maybe_cap: Option) -> anyhow::Result<()> { + pub async fn set_power_cap(&'a self, id: &str, maybe_cap: Option) -> anyhow::Result { self.edit_gpu_config(id.to_owned(), |gpu_config| { gpu_config.power_cap = maybe_cap; }) @@ -186,7 +255,7 @@ impl<'a> Handler { &self, id: &str, level: PerformanceLevel, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { self.edit_gpu_config(id.to_owned(), |gpu_config| { gpu_config.performance_level = Some(level); }) @@ -197,28 +266,25 @@ impl<'a> Handler { &self, id: &str, command: SetClocksCommand, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { if let SetClocksCommand::Reset = command { self.controller_by_id(id)?.handle.reset_clocks_table()?; } - self.edit_gpu_config(id.to_owned(), |gpu_config| match command { - SetClocksCommand::MaxCoreClock(clock) => gpu_config.max_core_clock = Some(clock), - SetClocksCommand::MaxMemoryClock(clock) => gpu_config.max_memory_clock = Some(clock), - SetClocksCommand::MaxVoltage(voltage) => gpu_config.max_voltage = Some(voltage), - SetClocksCommand::MinCoreClock(clock) => gpu_config.min_core_clock = Some(clock), - SetClocksCommand::MinMemoryClock(clock) => gpu_config.min_memory_clock = Some(clock), - SetClocksCommand::MinVoltage(voltage) => gpu_config.min_voltage = Some(voltage), - SetClocksCommand::VoltageOffset(offset) => gpu_config.voltage_offset = Some(offset), - SetClocksCommand::Reset => { - gpu_config.min_core_clock = None; - gpu_config.min_memory_clock = None; - gpu_config.min_voltage = None; - gpu_config.max_core_clock = None; - gpu_config.max_memory_clock = None; - gpu_config.max_voltage = None; + self.edit_gpu_config(id.to_owned(), |gpu_config| { + gpu_config.apply_clocks_command(&command); + }) + .await + } - assert!(!gpu_config.is_core_clocks_used()); + pub async fn batch_set_clocks_value( + &self, + id: &str, + commands: Vec, + ) -> anyhow::Result { + self.edit_gpu_config(id.to_owned(), |gpu_config| { + for command in commands { + gpu_config.apply_clocks_command(&command); } }) .await @@ -232,13 +298,31 @@ impl<'a> Handler { Ok(modes_table) } - pub async fn set_power_profile_mode(&self, id: &str, index: Option) -> anyhow::Result<()> { + pub async fn set_power_profile_mode( + &self, + id: &str, + index: Option, + ) -> anyhow::Result { self.edit_gpu_config(id.to_owned(), |gpu_config| { gpu_config.power_profile_mode_index = index; }) .await } + pub fn confirm_pending_config(&self, command: ConfirmCommand) -> anyhow::Result<()> { + if let Some(tx) = self + .confirm_config_tx + .try_lock() + .map_err(|err| anyhow!("{err}"))? + .take() + { + tx.send(command) + .map_err(|_| anyhow!("Could not confirm config")) + } else { + Err(anyhow!("No pending config changes")) + } + } + pub async fn cleanup(self) { let disable_clocks_cleanup = self .config diff --git a/lact-daemon/src/server/mod.rs b/lact-daemon/src/server/mod.rs index 5bda86e..56e03e1 100644 --- a/lact-daemon/src/server/mod.rs +++ b/lact-daemon/src/server/mod.rs @@ -99,10 +99,16 @@ async fn handle_request<'a>(request: Request<'a>, handler: &'a Handler) -> anyho Request::SetClocksValue { id, command } => { ok_response(handler.set_clocks_value(id, command).await?) } + Request::BatchSetClocksValue { id, commands } => { + ok_response(handler.batch_set_clocks_value(id, commands).await?) + } Request::SetPowerProfileMode { id, index } => { ok_response(handler.set_power_profile_mode(id, index).await?) } Request::EnableOverdrive => ok_response(system::enable_overdrive()?), + Request::ConfirmPendingConfig(command) => { + ok_response(handler.confirm_pending_config(command)?) + } } } diff --git a/lact-gui/src/app/mod.rs b/lact-gui/src/app/mod.rs index dd5c6b3..152058c 100644 --- a/lact-gui/src/app/mod.rs +++ b/lact-gui/src/app/mod.rs @@ -8,7 +8,7 @@ use apply_revealer::ApplyRevealer; use glib::clone; use gtk::{gio::ApplicationFlags, prelude::*, *}; use header::Header; -use lact_client::schema::request::SetClocksCommand; +use lact_client::schema::request::{ConfirmCommand, SetClocksCommand}; use lact_client::schema::DeviceStats; use lact_client::DaemonClient; use lact_daemon::MODULE_CONF_PATH; @@ -101,7 +101,9 @@ impl App { let gpu_id = current_gpu_id.read().unwrap(); - match app.daemon_client.set_clocks_value(&gpu_id, SetClocksCommand::Reset) { + match app.daemon_client.set_clocks_value(&gpu_id, SetClocksCommand::Reset) + .and_then(|_| app.daemon_client.confirm_pending_config(ConfirmCommand::Confirm)) + { Ok(()) => { app.set_initial(&gpu_id); } @@ -275,6 +277,7 @@ impl App { fn apply_settings(&self, current_gpu_id: Arc>) -> anyhow::Result<()> { debug!("applying settings"); + // TODO: Ask confirmation for everything, not just clocks let gpu_id = current_gpu_id.read().unwrap(); @@ -282,17 +285,27 @@ impl App { self.daemon_client .set_power_cap(&gpu_id, Some(cap)) .context("Failed to set power cap")?; + + self.daemon_client + .confirm_pending_config(ConfirmCommand::Confirm) + .context("Could not commit config")?; } // Reset the power profile mode for switching to/from manual performance level self.daemon_client .set_power_profile_mode(&gpu_id, None) .context("Could not set default power profile mode")?; + self.daemon_client + .confirm_pending_config(ConfirmCommand::Confirm) + .context("Could not commit config")?; if let Some(level) = self.root_stack.oc_page.get_performance_level() { self.daemon_client .set_performance_level(&gpu_id, level) .context("Failed to set power profile")?; + self.daemon_client + .confirm_pending_config(ConfirmCommand::Confirm) + .context("Could not commit config")?; let mode_index = self .root_stack @@ -302,6 +315,9 @@ impl App { self.daemon_client .set_power_profile_mode(&gpu_id, mode_index) .context("Could not set active power profile mode")?; + self.daemon_client + .confirm_pending_config(ConfirmCommand::Confirm) + .context("Could not commit config")?; } if let Some(thermals_settings) = self.root_stack.thermals_page.get_thermals_settings() { @@ -314,52 +330,50 @@ impl App { thermals_settings.curve, ) .context("Could not set fan control")?; + self.daemon_client + .confirm_pending_config(ConfirmCommand::Confirm) + .context("Could not commit config")?; } let clocks_settings = self.root_stack.oc_page.clocks_frame.get_settings(); + let mut clocks_commands = Vec::new(); debug!("applying clocks settings {clocks_settings:#?}"); if let Some(clock) = clocks_settings.min_core_clock { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MinCoreClock(clock)) - .context("Could not set the minimum core clock")?; + clocks_commands.push(SetClocksCommand::MinCoreClock(clock)); } if let Some(clock) = clocks_settings.min_memory_clock { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MinMemoryClock(clock)) - .context("Could not set the minimum memory clock")?; + clocks_commands.push(SetClocksCommand::MinMemoryClock(clock)); } if let Some(voltage) = clocks_settings.min_voltage { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MinVoltage(voltage)) - .context("Could not set the minimum voltage")?; + clocks_commands.push(SetClocksCommand::MinVoltage(voltage)); } if let Some(clock) = clocks_settings.max_core_clock { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MaxCoreClock(clock)) - .context("Could not set the maximum core clock")?; + clocks_commands.push(SetClocksCommand::MaxCoreClock(clock)); } if let Some(clock) = clocks_settings.max_memory_clock { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MaxMemoryClock(clock)) - .context("Could not set the maximum memory clock")?; + clocks_commands.push(SetClocksCommand::MaxMemoryClock(clock)); } if let Some(voltage) = clocks_settings.max_voltage { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::MaxVoltage(voltage)) - .context("Could not set the maximum voltage")?; + clocks_commands.push(SetClocksCommand::MaxVoltage(voltage)); } if let Some(offset) = clocks_settings.voltage_offset { - self.daemon_client - .set_clocks_value(&gpu_id, SetClocksCommand::VoltageOffset(offset)) - .context("Could not set the voltage offset")?; + clocks_commands.push(SetClocksCommand::VoltageOffset(offset)); + } + + if !clocks_commands.is_empty() { + let delay = self + .daemon_client + .batch_set_clocks_value(&gpu_id, clocks_commands) + .context("Could not commit clocks settins")?; + self.ask_confirmation(gpu_id.clone(), delay); } self.set_initial(&gpu_id); @@ -400,6 +414,50 @@ impl App { diag.hide(); })); } + + fn ask_confirmation(&self, gpu_id: String, mut delay: u64) { + let text = confirmation_text(delay); + let dialog = MessageDialog::builder() + .title("Confirm settings") + .text(text) + .message_type(MessageType::Question) + .buttons(ButtonsType::YesNo) + .build(); + + glib::source::timeout_add_local( + Duration::from_secs(1), + clone!(@strong dialog, @strong self as app, @strong gpu_id => move || { + delay -= 1; + + let text = confirmation_text(delay); + dialog.set_text(Some(&text)); + + if delay == 0 { + dialog.hide(); + app.set_initial(&gpu_id); + + Continue(false) + } else { + Continue(true) + } + }), + ); + + dialog.run_async(clone!(@strong self as app => move |diag, response| { + let command = match response { + ResponseType::Yes => ConfirmCommand::Confirm, + ResponseType::No => ConfirmCommand::Revert, + _ => unreachable!(), + }; + + diag.hide(); + + if let Err(err) = app.daemon_client.confirm_pending_config(command) { + show_error(&app.window, err); + } + app.set_initial(&gpu_id); + })); + } } enum GuiUpdateMsg { @@ -420,3 +478,7 @@ fn show_error(parent: &ApplicationWindow, err: anyhow::Error) { diag.hide(); }) } + +fn confirmation_text(seconds_left: u64) -> String { + format!("Do you want to keep the new settings? (Reverting in {seconds_left} seconds)") +} diff --git a/lact-schema/src/request.rs b/lact-schema/src/request.rs index 11c53c2..3c9bb80 100644 --- a/lact-schema/src/request.rs +++ b/lact-schema/src/request.rs @@ -37,11 +37,23 @@ pub enum Request<'a> { id: &'a str, command: SetClocksCommand, }, + BatchSetClocksValue { + id: &'a str, + commands: Vec, + }, SetPowerProfileMode { id: &'a str, index: Option, }, EnableOverdrive, + ConfirmPendingConfig(ConfirmCommand), +} + +#[derive(Serialize, Deserialize, Debug, PartialEq)] +#[serde(tag = "command", rename_all = "snake_case")] +pub enum ConfirmCommand { + Confirm, + Revert, } #[derive(Serialize, Deserialize, Debug, PartialEq)]