Bugfix: urgent small fixes

CHANGELOG.md

@@ -4,6 +4,12 @@
 
 ## [v6.1.0-rc2]
 
+### Changed
+- Bug fixes
+- Partial support for per-profile CPU tunings (WIP)
+
+## [v6.1.0-rc2]
+
 ### Added
 - asus-armoury driver support. WIP, will be adjusted/changed further
 - More "Slash" display controls

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace.package]
-version = "6.1.0-rc2"
+version = "6.1.0-rc3"
 rust-version = "1.82"
 license = "MPL-2.0"
 readme = "README.md"

asusd/src/asus_armoury.rs

@@ -1,11 +1,18 @@
+use std::str::FromStr;
+use std::sync::Arc;
+
+use ::zbus::export::futures_util::lock::Mutex;
+use config_traits::StdConfig;
 use log::error;
 use rog_platform::firmware_attributes::{
     AttrValue, Attribute, FirmwareAttribute, FirmwareAttributes
 };
+use rog_platform::platform::{RogPlatform, ThrottlePolicy};
 use serde::{Deserialize, Serialize};
 use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue, Type, Value};
 use zbus::{fdo, interface, Connection};
 
+use crate::config::Config;
 use crate::error::RogError;
 use crate::ASUS_ZBUS_PATH;
 
@@ -21,18 +28,27 @@ fn dbus_path_for_attr(attr_name: &str) -> OwnedObjectPath {
     ObjectPath::from_str_unchecked(&format!("{ASUS_ZBUS_PATH}/{MOD_NAME}/{attr_name}")).into()
 }
 
-pub struct AsusArmouryAttribute(Attribute);
+pub struct AsusArmouryAttribute {
+    attr: Attribute,
+    config: Arc<Mutex<Config>>,
+    /// platform control required here for access to PPD or Throttle profile
+    platform: RogPlatform
+}
 
 impl AsusArmouryAttribute {
-    pub fn new(attr: Attribute) -> Self {
-        Self(attr)
+    pub fn new(attr: Attribute, platform: RogPlatform, config: Arc<Mutex<Config>>) -> Self {
+        Self {
+            attr,
+            config,
+            platform
+        }
     }
 
     pub async fn start_tasks(self, connection: &Connection) -> Result<(), RogError> {
         // self.reload()
         //     .await
         //     .unwrap_or_else(|err| warn!("Controller error: {}", err));
-        let path = dbus_path_for_attr(self.0.name());
+        let path = dbus_path_for_attr(self.attr.name());
         connection
             .object_server()
             .at(path.clone(), self)
@@ -48,30 +64,30 @@ impl AsusArmouryAttribute {
 #[interface(name = "xyz.ljones.AsusArmoury")]
 impl AsusArmouryAttribute {
     #[zbus(property)]
-    async fn name(&self) -> FirmwareAttribute {
-        self.0.name().into()
+    fn name(&self) -> FirmwareAttribute {
+        self.attr.name().into()
     }
 
     #[zbus(property)]
     async fn available_attrs(&self) -> Vec<String> {
         let mut attrs = Vec::new();
-        if !matches!(self.0.default_value(), AttrValue::None) {
+        if !matches!(self.attr.default_value(), AttrValue::None) {
             attrs.push("default_value".to_string());
         }
-        if !matches!(self.0.min_value(), AttrValue::None) {
+        if !matches!(self.attr.min_value(), AttrValue::None) {
             attrs.push("min_value".to_string());
         }
-        if !matches!(self.0.max_value(), AttrValue::None) {
+        if !matches!(self.attr.max_value(), AttrValue::None) {
             attrs.push("max_value".to_string());
         }
-        if !matches!(self.0.scalar_increment(), AttrValue::None) {
+        if !matches!(self.attr.scalar_increment(), AttrValue::None) {
             attrs.push("scalar_increment".to_string());
         }
-        if !matches!(self.0.possible_values(), AttrValue::None) {
+        if !matches!(self.attr.possible_values(), AttrValue::None) {
             attrs.push("possible_values".to_string());
         }
         // TODO: Don't unwrap, use error
-        if let Ok(value) = self.0.current_value().map_err(|e| {
+        if let Ok(value) = self.attr.current_value().map_err(|e| {
             error!("Failed to read: {e:?}");
             e
         }) {
@@ -85,7 +101,7 @@ impl AsusArmouryAttribute {
     /// If return is `-1` then there is no default value
     #[zbus(property)]
     async fn default_value(&self) -> i32 {
-        match self.0.default_value() {
+        match self.attr.default_value() {
             AttrValue::Integer(i) => *i,
             _ => -1
         }
@@ -93,7 +109,7 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn min_value(&self) -> i32 {
-        match self.0.min_value() {
+        match self.attr.min_value() {
             AttrValue::Integer(i) => *i,
             _ => -1
         }
@@ -101,7 +117,7 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn max_value(&self) -> i32 {
-        match self.0.max_value() {
+        match self.attr.max_value() {
             AttrValue::Integer(i) => *i,
             _ => -1
         }
@@ -109,7 +125,7 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn scalar_increment(&self) -> i32 {
-        match self.0.scalar_increment() {
+        match self.attr.scalar_increment() {
             AttrValue::Integer(i) => *i,
             _ => -1
         }
@@ -117,7 +133,7 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn possible_values(&self) -> Vec<i32> {
-        match self.0.possible_values() {
+        match self.attr.possible_values() {
             AttrValue::EnumInt(i) => i.clone(),
             _ => Vec::default()
         }
@@ -125,7 +141,7 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn current_value(&self) -> fdo::Result<i32> {
-        if let Ok(AttrValue::Integer(i)) = self.0.current_value() {
+        if let Ok(AttrValue::Integer(i)) = self.attr.current_value() {
             return Ok(i);
         }
         Err(fdo::Error::Failed(
@@ -135,19 +151,31 @@ impl AsusArmouryAttribute {
 
     #[zbus(property)]
     async fn set_current_value(&mut self, value: i32) -> fdo::Result<()> {
-        Ok(self
-            .0
+        self.attr
             .set_current_value(AttrValue::Integer(value))
             .map_err(|e| {
                 error!("Could not set value: {e:?}");
                 e
-            })?)
+            })?;
+        let profile: ThrottlePolicy =
+            ThrottlePolicy::from_str(self.platform.get_platform_profile()?.as_str())?;
+        if let Some(tunings) = self.config.lock().await.tunings.get_mut(&profile) {
+            if let Some(tune) = tunings.get_mut(&self.name()) {
+                *tune = value;
+            }
+        }
+        self.config.lock().await.write();
+        Ok(())
     }
 }
 
-pub async fn start_attributes_zbus(server: &Connection) -> Result<(), RogError> {
+pub async fn start_attributes_zbus(
+    server: &Connection,
+    platform: RogPlatform,
+    config: Arc<Mutex<Config>>
+) -> Result<(), RogError> {
     for attr in FirmwareAttributes::new().attributes() {
-        AsusArmouryAttribute::new(attr.clone())
+        AsusArmouryAttribute::new(attr.clone(), platform.clone(), config.clone())
             .start_tasks(server)
             .await?;
     }

asusd/src/config.rs

@@ -1,11 +1,14 @@
+use std::collections::HashMap;
+
 use config_traits::{StdConfig, StdConfigLoad1};
 use rog_platform::cpu::CPUEPP;
+use rog_platform::firmware_attributes::FirmwareAttribute;
 use rog_platform::platform::ThrottlePolicy;
 use serde::{Deserialize, Serialize};
 
 const CONFIG_FILE: &str = "asusd.ron";
 
-#[derive(Deserialize, Serialize, Debug, PartialEq, PartialOrd)]
+#[derive(Deserialize, Serialize)]
 pub struct Config {
     // The current charge limit applied
     pub charge_control_end_threshold: u8,
@@ -37,27 +40,7 @@ pub struct Config {
     pub throttle_balanced_epp: CPUEPP,
     /// The energy_performance_preference for this throttle/platform profile
     pub throttle_performance_epp: CPUEPP,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub ppt_pl1_spl: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub ppt_pl2_sppt: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub ppt_fppt: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub ppt_apu_sppt: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub ppt_platform_sppt: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub nv_dynamic_boost: Option<u8>,
-    /// Defaults to `None` if not supported
-    #[serde(skip_serializing_if = "Option::is_none", default)]
-    pub nv_temp_target: Option<u8>,
+    pub tunings: HashMap<ThrottlePolicy, HashMap<FirmwareAttribute, i32>>,
     /// Temporary state for AC/Batt
     #[serde(skip)]
     pub last_power_plugged: u8
@@ -82,13 +65,7 @@ impl Default for Config {
             throttle_quiet_epp: CPUEPP::Power,
             throttle_balanced_epp: CPUEPP::BalancePower,
             throttle_performance_epp: CPUEPP::Performance,
-            ppt_pl1_spl: Default::default(),
-            ppt_pl2_sppt: Default::default(),
-            ppt_fppt: Default::default(),
-            ppt_apu_sppt: Default::default(),
-            ppt_platform_sppt: Default::default(),
-            nv_dynamic_boost: Default::default(),
-            nv_temp_target: Default::default(),
+            tunings: HashMap::default(),
             last_power_plugged: Default::default()
         }
     }
@@ -116,58 +93,69 @@ impl StdConfig for Config {
     }
 }
 
-impl StdConfigLoad1<Config507> for Config {}
+impl StdConfigLoad1<Config601> for Config {}
 
 #[derive(Deserialize, Serialize)]
-pub struct Config507 {
-    // The current charge limit applied
+pub struct Config601 {
     pub charge_control_end_threshold: u8,
+    #[serde(skip)]
+    pub base_charge_control_end_threshold: u8,
     pub panel_od: bool,
+    pub boot_sound: bool,
     pub mini_led_mode: bool,
     pub disable_nvidia_powerd_on_battery: bool,
     pub ac_command: String,
     pub bat_command: String,
-    pub platform_policy_linked_epp: bool,
-    pub platform_policy_on_battery: ThrottlePolicy,
-    pub platform_policy_on_ac: ThrottlePolicy,
-    //
+    pub throttle_policy_linked_epp: bool,
+    pub throttle_policy_on_battery: ThrottlePolicy,
+    pub change_throttle_policy_on_battery: bool,
+    pub throttle_policy_on_ac: ThrottlePolicy,
+    pub change_throttle_policy_on_ac: bool,
+    pub throttle_quiet_epp: CPUEPP,
+    pub throttle_balanced_epp: CPUEPP,
+    pub throttle_performance_epp: CPUEPP,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub ppt_pl1_spl: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub ppt_pl2_sppt: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    pub ppt_pl3_fppt: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub ppt_fppt: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub ppt_apu_sppt: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub ppt_platform_sppt: Option<u8>,
+    #[serde(skip_serializing_if = "Option::is_none", default)]
     pub nv_dynamic_boost: Option<u8>,
-    pub nv_temp_target: Option<u8>
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    pub nv_temp_target: Option<u8>,
+    #[serde(skip)]
+    pub last_power_plugged: u8
 }
 
-impl From<Config507> for Config {
-    fn from(c: Config507) -> Self {
+impl From<Config601> for Config {
+    fn from(c: Config601) -> Self {
         Self {
             // Restore the base charge limit
             charge_control_end_threshold: c.charge_control_end_threshold,
             base_charge_control_end_threshold: c.charge_control_end_threshold,
             panel_od: c.panel_od,
-            boot_sound: false,
+            boot_sound: c.boot_sound,
             disable_nvidia_powerd_on_battery: c.disable_nvidia_powerd_on_battery,
             ac_command: c.ac_command,
             bat_command: c.bat_command,
             mini_led_mode: c.mini_led_mode,
-            throttle_policy_linked_epp: true,
-            throttle_policy_on_battery: c.platform_policy_on_battery,
-            change_throttle_policy_on_battery: true,
-            throttle_policy_on_ac: c.platform_policy_on_ac,
-            change_throttle_policy_on_ac: true,
-            throttle_quiet_epp: CPUEPP::Power,
-            throttle_balanced_epp: CPUEPP::BalancePower,
-            throttle_performance_epp: CPUEPP::Performance,
-            ppt_pl1_spl: c.ppt_pl1_spl,
-            ppt_pl2_sppt: c.ppt_pl2_sppt,
-            ppt_fppt: c.ppt_fppt,
-            ppt_apu_sppt: c.ppt_apu_sppt,
-            ppt_platform_sppt: c.ppt_platform_sppt,
-            nv_dynamic_boost: c.nv_dynamic_boost,
-            nv_temp_target: c.nv_temp_target,
-            last_power_plugged: 0
+            throttle_policy_linked_epp: c.throttle_policy_linked_epp,
+            throttle_policy_on_battery: c.throttle_policy_on_battery,
+            change_throttle_policy_on_battery: c.change_throttle_policy_on_battery,
+            throttle_policy_on_ac: c.throttle_policy_on_ac,
+            change_throttle_policy_on_ac: c.change_throttle_policy_on_ac,
+            throttle_quiet_epp: c.throttle_quiet_epp,
+            throttle_balanced_epp: c.throttle_balanced_epp,
+            throttle_performance_epp: c.throttle_performance_epp,
+            last_power_plugged: c.last_power_plugged,
+            tunings: HashMap::default()
         }
     }
 }

asusd/src/ctrl_platform.rs

@@ -619,7 +619,6 @@ impl ReloadAndNotify for CtrlPlatform {
         data: Self::Data
     ) -> Result<(), RogError> {
         let mut config = self.config.lock().await;
-        if *config != data {
         info!("asusd.ron updated externally, reloading and updating internal copy");
 
         let mut base_charge_control_end_threshold = None;
@@ -662,7 +661,6 @@ impl ReloadAndNotify for CtrlPlatform {
         *config = data;
         config.base_charge_control_end_threshold =
             base_charge_control_end_threshold.unwrap_or_default();
-        }
 
         Ok(())
     }

asusd/src/daemon.rs

@@ -12,6 +12,7 @@ use asusd::ctrl_platform::CtrlPlatform;
 use asusd::{print_board_info, start_tasks, CtrlTask, DBUS_NAME};
 use config_traits::{StdConfig, StdConfigLoad1};
 use log::{error, info};
+use rog_platform::platform::RogPlatform;
 use zbus::fdo::ObjectManager;
 
 #[tokio::main]
@@ -65,7 +66,8 @@ async fn start_daemon() -> Result<(), Box<dyn Error>> {
     let config = Arc::new(Mutex::new(config));
 
     // supported.add_to_server(&mut connection).await;
-    start_attributes_zbus(&server).await?;
+    let platform = RogPlatform::new()?; // TODO: maybe needs async mutex?
+    start_attributes_zbus(&server, platform, config.clone()).await?;
 
     match CtrlFanCurveZbus::new() {
         Ok(ctrl) => {

config-traits/src/lib.rs

@@ -203,7 +203,7 @@ macro_rules! std_config_load {
         /// new one created
         pub trait $trait_name<$($generic),*>
         where
-            Self: $crate::StdConfig +std::fmt::Debug + DeserializeOwned + Serialize,
+            Self: $crate::StdConfig + DeserializeOwned + Serialize,
             $($generic: DeserializeOwned + Into<Self>),*
         {
             fn load(mut self) -> Self {

rog-platform/src/firmware_attributes.rs

@@ -230,7 +230,20 @@ define_attribute_getters!(
 
 /// CamelCase names of the properties. Intended for use with DBUS
 #[repr(u8)]
-#[derive(Clone, Copy, Serialize, Deserialize, Type, Value, OwnedValue, PartialEq, PartialOrd)]
+#[derive(
+    Clone,
+    Copy,
+    Serialize,
+    Deserialize,
+    Type,
+    Value,
+    OwnedValue,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Hash,
+)]
 #[zvariant(signature = "s")]
 pub enum FirmwareAttribute {
     ApuMem = 0,

rog-platform/src/platform.rs

@@ -204,7 +204,6 @@ impl Display for GpuMode {
     PartialEq,
     Eq,
     PartialOrd,
-    Ord,
     Hash,
     Clone,
     Copy,