fix: add workaround for amd broken p-state indexes (#1097)

* fix: add workaround for amd broken p-state indexes

* upd snapshot

* reflect apply behaviour in rx9070 snapshot

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

@@ -6,7 +6,7 @@ use crate::server::gpu_controller::common::{
 use amdgpu_sysfs::{
     error::Error,
     gpu_handle::{
-        CommitHandle, GpuHandle, PerformanceLevel, PowerLevelId, PowerLevelKind,
+        CommitHandle, GpuHandle, PerformanceLevel, PowerLevelId, PowerLevelKind, PowerLevels,
         fan_control::FanCurve as PmfwCurve,
         overdrive::{ClocksTable, ClocksTableGen},
         power_profile_mode::PowerProfileModesTable,
@@ -441,11 +441,12 @@ impl AmdGpuController {
             .get_clock_levels(kind)
             .inspect(|power_levels| trace!("{kind:?} power states: {power_levels:?}"))
             .inspect_err(|err| debug!("could not get {kind:?} power states: {err:#}"))
-            .map(|power_levels| power_levels.levels)
+            .map(Self::normalize_power_level_indexes)
             .unwrap_or_default();
 
         if attempt < MAX_PSTATE_READ_ATTEMPTS
             && levels
+                .levels
                 .iter()
                 .any(|level| level.value >= u64::from(u16::MAX))
         {
@@ -454,6 +455,7 @@ impl AmdGpuController {
         }
 
         levels
+            .levels
             .into_iter()
             .map(|level| {
                 let enabled = match level.id {
@@ -472,6 +474,33 @@ impl AmdGpuController {
             .collect()
     }
 
+    // workaround for https://gitlab.freedesktop.org/drm/amd/-/work_items/5295
+    fn normalize_power_level_indexes<T>(mut power_levels: PowerLevels<T>) -> PowerLevels<T> {
+        // first numeric is not 1
+        if !matches!(
+            power_levels.levels.iter().find_map(|level| match level.id {
+                PowerLevelId::Index(index) => Some(index),
+                PowerLevelId::Sleep => None,
+            }),
+            Some(1)
+        ) {
+            return power_levels;
+        }
+
+        for level in &mut power_levels.levels {
+            level.id = Self::shift_power_level_index(level.id);
+        }
+        power_levels.active = power_levels.active.map(Self::shift_power_level_index);
+        power_levels
+    }
+
+    fn shift_power_level_index(id: PowerLevelId) -> PowerLevelId {
+        match id {
+            PowerLevelId::Index(index) => PowerLevelId::Index(index.saturating_sub(1)),
+            PowerLevelId::Sleep => PowerLevelId::Sleep,
+        }
+    }
+
     fn first_hw_mon(&self) -> anyhow::Result<&HwMon> {
         self.handle
             .hw_monitors
@@ -721,6 +750,100 @@ impl AmdGpuController {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use amdgpu_sysfs::gpu_handle::PowerLevel;
+
+    #[test]
+    fn normalize_power_level_indexes_keeps_zero_based_indexes_after_sleep_state() {
+        let levels = normalize_indexes(
+            vec![
+                PowerLevelId::Sleep,
+                PowerLevelId::Index(0),
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2),
+            ],
+            Some(PowerLevelId::Index(1)),
+        );
+
+        assert_eq!(
+            level_ids(&levels),
+            [
+                PowerLevelId::Sleep,
+                PowerLevelId::Index(0),
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2)
+            ]
+        );
+        assert_eq!(levels.active, Some(PowerLevelId::Index(1)));
+    }
+
+    #[test]
+    fn normalize_power_level_indexes_keeps_zero_based_indexes_without_sleep_state() {
+        let levels = normalize_indexes(
+            vec![
+                PowerLevelId::Index(0),
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2),
+            ],
+            Some(PowerLevelId::Index(2)),
+        );
+
+        assert_eq!(
+            level_ids(&levels),
+            [
+                PowerLevelId::Index(0),
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2)
+            ]
+        );
+        assert_eq!(levels.active, Some(PowerLevelId::Index(2)));
+    }
+
+    #[test]
+    fn normalize_power_level_indexes_shifts_one_based_indexes_after_sleep_state() {
+        let levels = normalize_indexes(
+            vec![
+                PowerLevelId::Sleep,
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2),
+                PowerLevelId::Index(3),
+            ],
+            Some(PowerLevelId::Index(2)),
+        );
+
+        assert_eq!(
+            level_ids(&levels),
+            [
+                PowerLevelId::Sleep,
+                PowerLevelId::Index(0),
+                PowerLevelId::Index(1),
+                PowerLevelId::Index(2)
+            ]
+        );
+        assert_eq!(levels.active, Some(PowerLevelId::Index(1)));
+    }
+
+    fn normalize_indexes(ids: Vec<PowerLevelId>, active: Option<PowerLevelId>) -> PowerLevels<u64> {
+        AmdGpuController::normalize_power_level_indexes(PowerLevels {
+            levels: ids
+                .into_iter()
+                .enumerate()
+                .map(|(value, id)| PowerLevel {
+                    id,
+                    value: value as u64,
+                })
+                .collect(),
+            active,
+        })
+    }
+
+    fn level_ids<T>(levels: &PowerLevels<T>) -> Vec<PowerLevelId> {
+        levels.levels.iter().map(|level| level.id).collect()
+    }
+}
+
 impl GpuController for AmdGpuController {
     fn controller_info(&self) -> &CommonControllerInfo {
         &self.common
@@ -1037,6 +1160,7 @@ impl GpuController for AmdGpuController {
                         .get_core_clock_levels()
                         .inspect_err(|err| debug!("could not get active core power state: {err:#}"))
                         .ok()
+                        .map(Self::normalize_power_level_indexes)
                         .and_then(|levels| levels.active),
                     memory: self
                         .handle
@@ -1045,12 +1169,14 @@ impl GpuController for AmdGpuController {
                             debug!("could not get active memory power state: {err:#}");
                         })
                         .ok()
+                        .map(Self::normalize_power_level_indexes)
                         .and_then(|levels| levels.active),
                     pcie: self
                         .handle
                         .get_pcie_clock_levels()
                         .inspect_err(|err| debug!("could not get active PCIe power state: {err:#}"))
                         .ok()
+                        .map(Self::normalize_power_level_indexes)
                         .and_then(|levels| levels.active),
                 };
                 (active_power_states.core.is_some()

lact-daemon/src/tests/snapshots/lact_daemon__tests__amd__rx9070.snap

@@ -353,22 +353,20 @@ expression: device_info
     "core": [
       {
         "enabled": true,
-        "id": {
-          "Index": 0
-        },
+        "id": "Sleep",
         "value": 500
       },
       {
         "enabled": true,
         "id": {
-          "Index": 1
+          "Index": 0
         },
         "value": 789
       },
       {
         "enabled": true,
         "id": {
-          "Index": 2
+          "Index": 1
         },
         "value": 2070
       }
@@ -421,7 +419,7 @@ expression: device_info
   "stats": {
     "active_power_states": {
       "core": {
-        "Index": 1
+        "Index": 0
       },
       "memory": {
         "Index": 4
@@ -488,7 +486,7 @@ expression: device_info
         100
       ]
     },
-    "performance_level": "auto",
+    "performance_level": "manual",
     "power": {
       "average": 49.0,
       "cap_current": 200.0,

lact-daemon/src/tests/snapshots/lact_daemon__tests__apply_config__amd__rx9070__config.yaml.snap

@@ -6,7 +6,7 @@ expression: write_commands
     "card1/device/pp_od_clk_voltage: r\n",
     "card1/device/pp_od_clk_voltage: s 0\n",
     "card1/device/pp_od_clk_voltage: vo 0\n",
-    "card1/device/power_dpm_force_performance_level: auto",
+    "card1/device/power_dpm_force_performance_level: manual",
     "card1/device/gpu_od/fan_ctrl/fan_curve: 0 40 25\n",
     "card1/device/gpu_od/fan_ctrl/fan_curve: 1 50 40\n",
     "card1/device/gpu_od/fan_ctrl/fan_curve: 2 60 60\n",
@@ -15,4 +15,5 @@ expression: write_commands
     "card1/device/hwmon/hwmon2/power1_cap: 200000000",
     "card1/device/gpu_od/fan_ctrl/fan_curve: c\n",
     "card1/device/pp_od_clk_voltage: c\n",
+    "card1/device/pp_dpm_sclk: 0 1 ",
 ]

tests/snapshots/amd/rx9070/card1/device/power_dpm_force_performance_level

@@ -1 +1 @@
-auto
+manual

tests/snapshots/amd/rx9070/card1/device/pp_dpm_mclk

@@ -1,6 +1,6 @@
-0: 96Mhz 
-1: 456Mhz 
-2: 772Mhz 
-3: 875Mhz 
+0: 96Mhz
+1: 456Mhz
+2: 772Mhz
+3: 875Mhz
 4: 1124Mhz *
-5: 1258Mhz 
+5: 1258Mhz

tests/snapshots/amd/rx9070/card1/device/pp_dpm_sclk

@@ -1,3 +1,3 @@
-0: 500Mhz
+S: 500Mhz
 1: 789Mhz *
 2: 2070Mhz

tests/snapshots/amd/rx9070/config.yaml

@@ -18,7 +18,11 @@ pmfw_options:
   minimum_pwm: 15
   target_temperature: 70
 power_cap: 200.0
-performance_level: auto
+performance_level: manual
 gpu_clock_offsets:
   0: 0
 voltage_offset: 0
+power_states:
+  core_clock:
+  - 0
+  - 1