feat: add support for custom fan curves and static fan speed on RDNA3 (#248)

* feat: add support for custom fan curves and static fan speed on RDNA3

* fix: use gpu-provided min and max fan speed values

* fix: set min temperature point too in static curve

* fix: reset fan curve when switching mode back

* fix: normalize fan speed values by taking min speed into account

* test

* Revert "test"

This reverts commit dbd37bdc0b.

* fix: dont use the upper range value as visibility trigger for oc scales

* fix visibility

* Revert "fix: normalize fan speed values by taking min speed into account"

This reverts commit 945cfd23e6.

* fix: get_adjustment_value

* doc: update README

Cargo.lock

@@ -41,9 +41,8 @@ dependencies = [
 
 [[package]]
 name = "amdgpu-sysfs"
-version = "0.13.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "04b3c8f53aaed3c9a8f9b71c7d261a4343703ed3a5758eda16646e9b57c4e263"
+version = "0.14.0"
+source = "git+https://github.com/ilya-zlobintsev/amdgpu-sysfs-rs?branch=feature/rnda3-fan-curve#0d270af398dc863d7f3f976887bb5810ed754abf"
 dependencies = [
  "enum_dispatch",
  "serde",

README.md

@@ -56,7 +56,7 @@ Tested GPU generations:
 - [X] Vega
 - [X] RDNA1 (RX 5000 series)
 - [X] RDNA2 (RX 6000 series)
-- [ ] RDNA3 (RX 7000 series) - basic support available. Fan control available via thermal target settings, but full custom curve support is currently missing. Requires Kernel 6.7+
+- [X] RDNA3 (RX 7000 series) - Requires Kernel 6.7+
 
 GPUs not listed here will still work, but might not have full functionality available.
 Monitoring/system info will be available everywhere. Integrated GPUs might also only have basic configuration available.

lact-daemon/src/server/gpu_controller/fan_control.rs

@@ -1,5 +1,10 @@
-use anyhow::anyhow;
-use lact_schema::{amdgpu_sysfs::hw_mon::Temperature, default_fan_curve, FanCurveMap};
+use std::cmp;
+
+use anyhow::{anyhow, Context};
+use lact_schema::{
+    amdgpu_sysfs::{gpu_handle::fan_control::FanCurve as PmfwCurve, hw_mon::Temperature},
+    default_fan_curve, FanCurveMap,
+};
 use serde::{Deserialize, Serialize};
 use tracing::warn;
 
@@ -39,6 +44,36 @@ impl FanCurve {
 
         (f32::from(u8::MAX) * percentage) as u8
     }
+
+    pub fn into_pmfw_curve(self, current_pmfw_curve: PmfwCurve) -> anyhow::Result<PmfwCurve> {
+        if current_pmfw_curve.points.len() != self.0.len() {
+            return Err(anyhow!(
+                "The GPU only supports {} curve points, given {}",
+                current_pmfw_curve.points.len(),
+                self.0.len()
+            ));
+        }
+        let allowed_ranges = current_pmfw_curve
+            .allowed_ranges
+            .context("The GPU does not allow fan curve modifications")?;
+        let min_pwm = *allowed_ranges.speed_range.start();
+        let max_pwm = f32::from(*allowed_ranges.speed_range.end());
+
+        let points = self
+            .0
+            .into_iter()
+            .map(|(temp, ratio)| {
+                let custom_pwm = (max_pwm * ratio) as u8;
+                let pwm = cmp::max(min_pwm, custom_pwm);
+                (temp, pwm)
+            })
+            .collect();
+
+        Ok(PmfwCurve {
+            points,
+            allowed_ranges: Some(allowed_ranges),
+        })
+    }
 }
 
 impl FanCurve {
@@ -60,8 +95,8 @@ impl Default for FanCurve {
 
 #[cfg(test)]
 mod tests {
-    use super::FanCurve;
-    use lact_schema::amdgpu_sysfs::hw_mon::Temperature;
+    use super::{FanCurve, PmfwCurve};
+    use lact_schema::amdgpu_sysfs::{gpu_handle::fan_control::FanCurveRanges, hw_mon::Temperature};
 
     fn simple_pwm(temp: f32) -> u8 {
         let curve = FanCurve([(0, 0.0), (100, 1.0)].into());
@@ -147,9 +182,7 @@ mod tests {
             };
             curve.pwm_at_temp(temp)
         };
-        assert_eq!(pwm_at_temp(20.0), 0);
-        assert_eq!(pwm_at_temp(30.0), 0);
-        assert_eq!(pwm_at_temp(33.0), 15);
+        assert_eq!(pwm_at_temp(40.0), 51);
         assert_eq!(pwm_at_temp(60.0), 127);
         assert_eq!(pwm_at_temp(65.0), 159);
         assert_eq!(pwm_at_temp(70.0), 191);
@@ -158,4 +191,19 @@ mod tests {
         assert_eq!(pwm_at_temp(100.0), 255);
         assert_eq!(pwm_at_temp(-5.0), 255);
     }
+
+    #[test]
+    fn default_curve_to_pmfw() {
+        let curve = FanCurve::default();
+        let current_pmfw_curve = PmfwCurve {
+            points: Box::new([(0, 0); 5]),
+            allowed_ranges: Some(FanCurveRanges {
+                temperature_range: 15..=90,
+                speed_range: 20..=100,
+            }),
+        };
+        let pmfw_curve = curve.into_pmfw_curve(current_pmfw_curve).unwrap();
+        let expected_points = [(40, 20), (50, 35), (60, 50), (70, 75), (80, 100)];
+        assert_eq!(&expected_points, pmfw_curve.points.as_ref());
+    }
 }

lact-daemon/src/server/gpu_controller/mod.rs

@@ -11,6 +11,7 @@ use lact_schema::{
     amdgpu_sysfs::{
         error::Error,
         gpu_handle::{
+            fan_control::FanCurve as PmfwCurve,
             overdrive::{ClocksTable, ClocksTableGen},
             GpuHandle, PerformanceLevel, PowerLevelKind, PowerLevels,
         },
@@ -24,6 +25,7 @@ use pciid_parser::Database;
 use std::{
     borrow::Cow,
     cell::RefCell,
+    cmp,
     path::{Path, PathBuf},
     rc::Rc,
     str::FromStr,
@@ -331,6 +333,36 @@ impl GpuController {
         // Stop existing task to set static speed
         self.stop_fan_control(false).await?;
 
+        // Use PMFW curve functionality for static speed when it is available
+        if let Ok(current_curve) = self.handle.get_fan_curve() {
+            let allowed_ranges = current_curve.allowed_ranges.ok_or_else(|| {
+                anyhow!("The GPU does not allow setting custom fan values (is overdrive enabled?)")
+            })?;
+            let min_temperature = allowed_ranges.temperature_range.start();
+            let max_temperature = allowed_ranges.temperature_range.end();
+
+            #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
+            let custom_pwm = (f64::from(*allowed_ranges.speed_range.end()) * static_speed) as u8;
+            let static_pwm = cmp::max(*allowed_ranges.speed_range.start(), custom_pwm);
+
+            let mut points = vec![(*min_temperature, static_pwm)];
+            for _ in 1..current_curve.points.len() {
+                points.push((*max_temperature, static_pwm));
+            }
+
+            let new_curve = PmfwCurve {
+                points: points.into_boxed_slice(),
+                allowed_ranges: Some(allowed_ranges),
+            };
+
+            debug!("setting static curve {new_curve:?}");
+
+            self.handle
+                .set_fan_curve(&new_curve)
+                .context("Could not set fan curve")?;
+
+            Ok(())
+        } else {
             let hw_mon = self
                 .handle
                 .hw_monitors
@@ -343,22 +375,51 @@ impl GpuController {
                 .context("Could not set fan control method")?;
 
             #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
-        let static_speed_converted = (f64::from(u8::MAX) * static_speed) as u8;
+            let static_pwm = (f64::from(u8::MAX) * static_speed) as u8;
 
             hw_mon
-            .set_fan_pwm(static_speed_converted)
+                .set_fan_pwm(static_pwm)
                 .context("could not set fan speed")?;
 
             debug!("set fan speed to {}", static_speed);
 
             Ok(())
         }
+    }
 
     async fn start_curve_fan_control(
         &self,
         curve: FanCurve,
         temp_key: String,
         interval: Duration,
+    ) -> anyhow::Result<()> {
+        // Use the PMFW curve functionality when it is available
+        // Otherwise, fall back to manual fan control via a task
+        match self.handle.get_fan_curve() {
+            Ok(current_curve) => {
+                let new_curve = curve
+                    .into_pmfw_curve(current_curve)
+                    .context("Invalid fan curve")?;
+                debug!("setting pmfw curve {new_curve:?}");
+
+                self.handle
+                    .set_fan_curve(&new_curve)
+                    .context("Could not set fan curve")?;
+
+                Ok(())
+            }
+            Err(_) => {
+                self.start_curve_fan_control_task(curve, temp_key, interval)
+                    .await
+            }
+        }
+    }
+
+    async fn start_curve_fan_control_task(
+        &self,
+        curve: FanCurve,
+        temp_key: String,
+        interval: Duration,
     ) -> anyhow::Result<()> {
         // Stop existing task to re-apply new curve
         self.stop_fan_control(false).await?;
@@ -425,6 +486,12 @@ impl GpuController {
         }
 
         if reset_mode {
+            if self.handle.get_fan_curve().is_ok() {
+                if let Err(err) = self.handle.reset_fan_curve() {
+                    warn!("could not reset fan curve: {err:#}");
+                }
+            }
+
             if let Some(hw_mon) = self.handle.hw_monitors.first().cloned() {
                 if let Ok(current_control) = hw_mon.get_fan_control_method() {
                     if !matches!(current_control, FanControlMethod::Auto) {
@@ -500,35 +567,6 @@ impl GpuController {
     }
 
     pub async fn apply_config(&self, config: &config::Gpu) -> anyhow::Result<()> {
-        if config.fan_control_enabled {
-            if let Some(ref settings) = config.fan_control_settings {
-                match settings.mode {
-                    lact_schema::FanControlMode::Static => {
-                        self.set_static_fan_control(settings.static_speed).await?;
-                    }
-                    lact_schema::FanControlMode::Curve => {
-                        if settings.curve.0.is_empty() {
-                            return Err(anyhow!("Cannot use empty fan curve"));
-                        }
-
-                        let interval = Duration::from_millis(settings.interval_ms);
-                        self.start_curve_fan_control(
-                            settings.curve.clone(),
-                            settings.temperature_key.clone(),
-                            interval,
-                        )
-                        .await?;
-                    }
-                }
-            } else {
-                return Err(anyhow!(
-                    "Trying to enable fan control with no settings provided"
-                ));
-            }
-        } else {
-            self.stop_fan_control(true).await?;
-        }
-
         if let Some(cap) = config.power_cap {
             let hw_mon = self.first_hw_mon()?;
 
@@ -629,7 +667,34 @@ impl GpuController {
                 .with_context(|| format!("Could not set {kind:?} power states"))?;
         }
 
-        if !config.fan_control_enabled {
+        if config.fan_control_enabled {
+            if let Some(ref settings) = config.fan_control_settings {
+                match settings.mode {
+                    lact_schema::FanControlMode::Static => {
+                        self.set_static_fan_control(settings.static_speed).await?;
+                    }
+                    lact_schema::FanControlMode::Curve => {
+                        if settings.curve.0.is_empty() {
+                            return Err(anyhow!("Cannot use empty fan curve"));
+                        }
+
+                        let interval = Duration::from_millis(settings.interval_ms);
+                        self.start_curve_fan_control(
+                            settings.curve.clone(),
+                            settings.temperature_key.clone(),
+                            interval,
+                        )
+                        .await?;
+                    }
+                }
+            } else {
+                return Err(anyhow!(
+                    "Trying to enable fan control with no settings provided"
+                ));
+            }
+        } else {
+            self.stop_fan_control(true).await?;
+
             let pmfw = &config.pmfw_options;
             if let Some(acoustic_limit) = pmfw.acoustic_limit {
                 self.handle

lact-gui/src/app/root_stack/oc_page/clocks_frame.rs

@@ -20,13 +20,13 @@ pub struct ClocksFrame {
     basic_togglebutton: ToggleButton,
     advanced_togglebutton: ToggleButton,
     min_values_grid: Grid,
-    min_sclk_adjustment: (Adjustment, Rc<AtomicBool>),
-    min_mclk_adjustment: (Adjustment, Rc<AtomicBool>),
-    min_voltage_adjustment: (Adjustment, Rc<AtomicBool>),
-    max_sclk_adjustment: (Adjustment, Rc<AtomicBool>),
-    max_mclk_adjustment: (Adjustment, Rc<AtomicBool>),
-    max_voltage_adjustment: (Adjustment, Rc<AtomicBool>),
-    voltage_offset_adjustment: (Adjustment, Rc<AtomicBool>),
+    min_sclk_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    min_mclk_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    min_voltage_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    max_sclk_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    max_mclk_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    max_voltage_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
+    voltage_offset_adjustment: (Adjustment, Scale, Rc<AtomicBool>),
     reset_button: Button,
     warning_label: Label,
     clocks_data_unavailable_label: Label,
@@ -134,8 +134,6 @@ impl ClocksFrame {
     pub fn set_table(&self, table: ClocksTableGen) -> anyhow::Result<()> {
         debug!("using clocks table {table:?}");
 
-        // The upper value "0.0" is used to hide the adjustment when info is not available
-
         if let Some((current_sclk_min, sclk_min, sclk_max)) =
             extract_value_and_range(&table, |table| {
                 (
@@ -149,8 +147,10 @@ impl ClocksFrame {
             self.min_sclk_adjustment
                 .0
                 .set_value(current_sclk_min.into());
+
+            self.min_sclk_adjustment.1.set_visible(true);
         } else {
-            self.min_sclk_adjustment.0.set_upper(0.0);
+            self.min_sclk_adjustment.1.set_visible(false);
         }
 
         if let Some((current_mclk_min, mclk_min, mclk_max)) =
@@ -166,8 +166,9 @@ impl ClocksFrame {
             self.min_mclk_adjustment
                 .0
                 .set_value(current_mclk_min.into());
+            self.min_mclk_adjustment.1.set_visible(true);
         } else {
-            self.min_mclk_adjustment.0.set_upper(0.0);
+            self.min_mclk_adjustment.1.set_visible(false);
         }
 
         if let Some((current_min_voltage, voltage_min, voltage_max)) =
@@ -185,8 +186,9 @@ impl ClocksFrame {
             self.min_voltage_adjustment
                 .0
                 .set_value(current_min_voltage.into());
+            self.min_voltage_adjustment.1.set_visible(true);
         } else {
-            self.min_voltage_adjustment.0.set_upper(0.0);
+            self.min_voltage_adjustment.1.set_visible(false);
         }
 
         if let Some((current_sclk_max, sclk_min, sclk_max)) =
@@ -199,8 +201,9 @@ impl ClocksFrame {
             self.max_sclk_adjustment
                 .0
                 .set_value(current_sclk_max.into());
+            self.max_sclk_adjustment.1.set_visible(true);
         } else {
-            self.max_sclk_adjustment.0.set_upper(0.0);
+            self.max_sclk_adjustment.1.set_visible(false);
         }
 
         if let Some((current_mclk_max, mclk_min, mclk_max)) =
@@ -213,8 +216,9 @@ impl ClocksFrame {
             self.max_mclk_adjustment
                 .0
                 .set_value(current_mclk_max.into());
+            self.max_mclk_adjustment.1.set_visible(true);
         } else {
-            self.max_mclk_adjustment.0.set_upper(0.0);
+            self.max_mclk_adjustment.1.set_visible(false);
         }
 
         if let Some((current_voltage_max, voltage_min, voltage_max)) =
@@ -227,8 +231,9 @@ impl ClocksFrame {
             self.max_voltage_adjustment
                 .0
                 .set_value(current_voltage_max.into());
+            self.max_voltage_adjustment.1.set_visible(true);
         } else {
-            self.max_voltage_adjustment.0.set_upper(0.0);
+            self.max_voltage_adjustment.1.set_visible(false);
         }
 
         if let ClocksTableGen::Vega20(table) = table {
@@ -246,11 +251,12 @@ impl ClocksFrame {
                     .0
                     .set_upper(max_offset as f64);
                 self.voltage_offset_adjustment.0.set_value(offset.into());
+                self.voltage_offset_adjustment.1.set_visible(true);
             } else {
-                self.voltage_offset_adjustment.0.set_upper(0.0);
+                self.voltage_offset_adjustment.1.set_visible(false);
             }
         } else {
-            self.voltage_offset_adjustment.0.set_upper(0.0);
+            self.voltage_offset_adjustment.1.set_visible(false);
         }
 
         emit_changed(&self.min_sclk_adjustment);
@@ -298,7 +304,7 @@ impl ClocksFrame {
     }
 
     pub fn get_settings(&self) -> ClocksSettings {
-        if self.tweaking_grid.is_visible() {
+        if self.tweaking_grid.get_visible() {
             let min_core_clock = get_adjustment_value(&self.min_sclk_adjustment);
             let min_memory_clock = get_adjustment_value(&self.min_mclk_adjustment);
             let min_voltage = get_adjustment_value(&self.min_voltage_adjustment);
@@ -306,10 +312,10 @@ impl ClocksFrame {
             let max_memory_clock = get_adjustment_value(&self.max_mclk_adjustment);
             let max_voltage = get_adjustment_value(&self.max_voltage_adjustment);
 
-            let voltage_offset = if self.voltage_offset_adjustment.0.upper() == 0.0 {
-                None
-            } else {
+            let voltage_offset = if self.voltage_offset_adjustment.1.get_visible() {
                 Some(self.voltage_offset_adjustment.0.value() as i32)
+            } else {
+                None
             };
 
             ClocksSettings {
@@ -349,7 +355,11 @@ fn extract_value_and_range(
     Some((value, min, max))
 }
 
-fn oc_adjustment(title: &'static str, grid: &Grid, row: i32) -> (Adjustment, Rc<AtomicBool>) {
+fn oc_adjustment(
+    title: &'static str,
+    grid: &Grid,
+    row: i32,
+) -> (Adjustment, Scale, Rc<AtomicBool>) {
     let label = Label::builder().label(title).halign(Align::Start).build();
 
     let adjustment = Adjustment::new(0.0, 0.0, 0.0, 1.0, 10.0, 0.0);
@@ -385,30 +395,19 @@ fn oc_adjustment(title: &'static str, grid: &Grid, row: i32) -> (Adjustment, Rc<
         }),
     );
 
-    adjustment.connect_changed(
-        clone!(@strong label, @strong value_label, @strong scale, @strong value_button => move |adjustment| {
-            let value = adjustment.value();
-            value_label.set_text(&value.to_string());
-
-            if adjustment.upper() == 0.0 {
-                label.hide();
-                value_label.hide();
-                scale.hide();
-                value_button.hide();
-            } else {
-                label.show();
-                value_label.show();
-                scale.show();
-                value_button.show();
-            }
-        }
-    ));
+    scale.connect_visible_notify(
+        clone!(@strong label, @strong value_label, @strong value_button => move |scale| {
+            label.set_visible(scale.get_visible());
+            value_button.set_visible(scale.get_visible());
+            value_label.set_visible(scale.get_visible());
+        }),
+    );
 
     grid.attach(&label, 0, row, 1, 1);
     grid.attach(&scale, 1, row, 4, 1);
     grid.attach(&value_button, 6, row, 4, 1);
 
-    (adjustment, changed)
+    (adjustment, scale, changed)
 }
 
 #[derive(Debug, Default)]
@@ -422,22 +421,25 @@ pub struct ClocksSettings {
     pub voltage_offset: Option<i32>,
 }
 
-fn get_adjustment_value((adjustment, changed): &(Adjustment, Rc<AtomicBool>)) -> Option<i32> {
+fn get_adjustment_value(
+    (adjustment, scale, changed): &(Adjustment, Scale, Rc<AtomicBool>),
+) -> Option<i32> {
     let changed = changed.load(Ordering::SeqCst);
 
     if changed {
         let value = adjustment.value();
-        if value == 0.0 {
-            None
-        } else {
+        if scale.get_visible() {
             Some(value as i32)
+        } else {
+            None
         }
     } else {
         None
     }
 }
 
-fn emit_changed(adjustment: &(Adjustment, Rc<AtomicBool>)) {
-    adjustment.0.emit_by_name::<()>("changed", &[]);
-    adjustment.1.store(false, Ordering::SeqCst);
+fn emit_changed(adjustment: &(Adjustment, Scale, Rc<AtomicBool>)) {
+    adjustment.0.emit_by_name::<()>("value-changed", &[]);
+    adjustment.1.notify("visible");
+    adjustment.2.store(false, Ordering::SeqCst);
 }

lact-schema/Cargo.toml

@@ -7,7 +7,9 @@ edition = "2021"
 args = ["clap"]
 
 [dependencies]
-amdgpu-sysfs = { version = "0.13.0", features = ["serde"] }
+amdgpu-sysfs = { git = "https://github.com/ilya-zlobintsev/amdgpu-sysfs-rs", branch = "feature/rnda3-fan-curve", features = [
+    "serde",
+] }
 serde = { version = "1.0", features = ["derive"] }
 indexmap = { version = "*", features = ["serde"] }
 clap = { version = "4.4.11", features = ["derive"], optional = true }

lact-schema/src/lib.rs

@@ -50,15 +50,7 @@ impl FromStr for FanControlMode {
 pub type FanCurveMap = BTreeMap<i32, f32>;
 
 pub fn default_fan_curve() -> FanCurveMap {
-    [
-        (30, 0.0),
-        (40, 0.2),
-        (50, 0.35),
-        (60, 0.5),
-        (70, 0.75),
-        (80, 1.0),
-    ]
-    .into()
+    [(40, 0.2), (50, 0.35), (60, 0.5), (70, 0.75), (80, 1.0)].into()
 }
 
 #[derive(Serialize, Deserialize, Debug)]