Smooth fan speed changes

src/board/system76/addw1/board.mk

@@ -30,8 +30,6 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -42,8 +40,6 @@ CFLAGS+=-DBOARD_FAN_POINTS="\
 
 # Enable DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/addw2/board.mk

@@ -30,8 +30,6 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -42,8 +40,6 @@ CFLAGS+=-DBOARD_FAN_POINTS="\
 
 # Enable DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/bonw14/board.mk

@@ -30,8 +30,6 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -42,8 +40,6 @@ CFLAGS+=-DBOARD_FAN_POINTS="\
 
 # Enable DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/common/acpi.c

@@ -94,7 +94,7 @@ uint8_t acpi_read(uint8_t addr) {
             }
             break;
 
-        ACPI_8(0x07, peci_temp >> 6);
+        ACPI_8(0x07, peci_temp);
 
         // Handle AC adapter and battery present
         case 0x10:

src/board/system76/common/dgpu.c

@@ -5,7 +5,6 @@
 
 #if HAVE_DGPU
 
-#include <board/fan.h>
 #include <board/gpio.h>
 #include <board/peci.h>
 #include <board/power.h>
@@ -16,7 +15,7 @@
 
 // Fan speed is the lowest requested over HEATUP seconds
 #ifndef BOARD_DGPU_HEATUP
-    #define BOARD_DGPU_HEATUP 10
+    #define BOARD_DGPU_HEATUP 4
 #endif
 
 static uint8_t FAN_HEATUP[BOARD_DGPU_HEATUP] = { 0 };
@@ -54,7 +53,7 @@ static struct Fan __code FAN = {
     .heatup_size = ARRAY_SIZE(FAN_HEATUP),
     .cooldown = FAN_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN_COOLDOWN),
-    .interpolate = false,
+    .interpolate = SMOOTH_FANS != 0,
 };
 
 void dgpu_init(void) {

src/board/system76/common/fan.c

@@ -4,14 +4,19 @@
 #include <common/debug.h>
 #include <ec/pwm.h>
 
-bool fan_max = false;
+#if SMOOTH_FANS != 0
+    static const uint8_t max_jump_up = (MAX_FAN_SPEED - MIN_FAN_SPEED) / (uint8_t) SMOOTH_FANS_UP;
+    static const uint8_t max_jump_down = (MAX_FAN_SPEED - MIN_FAN_SPEED) / (uint8_t) SMOOTH_FANS_DOWN;
+#else
+    static const uint8_t max_jump_up = MAX_FAN_SPEED - MIN_FAN_SPEED;
+    static const uint8_t max_jump_down = MAX_FAN_SPEED - MIN_FAN_SPEED;
+#endif
 
-#define max_speed PWM_DUTY(100)
-#define min_speed PWM_DUTY(0)
+static const uint8_t min_speed_to_smooth = PWM_DUTY(SMOOTH_FANS_MIN);
 
-#ifndef SYNC_FANS
-  #define SYNC_FANS 1
-#endif
+bool fan_max = false;
+uint8_t last_duty_dgpu = 0;
+uint8_t last_duty_peci = 0;
 
 void fan_reset(void) {
     // Do not manually set fans to maximum speed
@@ -30,7 +35,7 @@ uint8_t fan_duty(const struct Fan * fan, int16_t temp) __reentrant {
         } else if (temp < cur->temp) {
             // If lower than first temp, return 0%
             if (i == 0) {
-                return min_speed;
+                return MIN_FAN_SPEED;
             } else {
                 const struct FanPoint * prev = &fan->points[i - 1];
 
@@ -52,26 +57,30 @@ uint8_t fan_duty(const struct Fan * fan, int16_t temp) __reentrant {
     }
 
     // If no point is found, return 100%
-    return max_speed;
+    return MAX_FAN_SPEED;
 }
 
 void fan_duty_set(uint8_t peci_fan_duty, uint8_t dgpu_fan_duty) __reentrant {
-  #if SYNC_FANS != 0
-    peci_fan_duty = peci_fan_duty > dgpu_fan_duty ? peci_fan_duty : dgpu_fan_duty;
-    dgpu_fan_duty = peci_fan_duty > dgpu_fan_duty ? peci_fan_duty : dgpu_fan_duty;
-  #endif
-
-  // set PECI fan duty
-  if (peci_fan_duty != DCR2) {
-      DCR2 = peci_fan_duty;
-      DEBUG("PECI fan_duty=%d\n", peci_fan_duty);
-  }
-
-  // set dGPU fan duty
-  if (dgpu_fan_duty != DCR4) {
-      DCR4 = dgpu_fan_duty;
-      DEBUG("DGPU fan_duty=%d\n", dgpu_fan_duty);
-  }
+    #if SYNC_FANS != 0
+        peci_fan_duty = peci_fan_duty > dgpu_fan_duty ? peci_fan_duty : dgpu_fan_duty;
+        dgpu_fan_duty = peci_fan_duty > dgpu_fan_duty ? peci_fan_duty : dgpu_fan_duty;
+    #endif
+
+    // set PECI fan duty
+    if (peci_fan_duty != DCR2) {
+        DEBUG("PECI fan_duty_raw=%d\n", peci_fan_duty);
+        last_duty_peci = peci_fan_duty = fan_smooth(last_duty_peci, peci_fan_duty);
+        DCR2 = fan_max ? MAX_FAN_SPEED : peci_fan_duty;
+        DEBUG("PECI fan_duty_smoothed=%d\n", peci_fan_duty);
+    }
+
+    // set dGPU fan duty
+    if (dgpu_fan_duty != DCR4) {
+        DEBUG("DGPU fan_duty_raw=%d\n", dgpu_fan_duty);
+        last_duty_dgpu = dgpu_fan_duty = fan_smooth(last_duty_dgpu, dgpu_fan_duty);
+        DCR4 = fan_max ? MAX_FAN_SPEED : dgpu_fan_duty;
+        DEBUG("DGPU fan_duty_smoothed=%d\n", dgpu_fan_duty);
+    }
 }
 
 uint8_t fan_heatup(const struct Fan * fan, uint8_t duty) __reentrant {
@@ -105,3 +114,35 @@ uint8_t fan_cooldown(const struct Fan * fan, uint8_t duty) __reentrant {
 
     return highest;
 }
+
+uint8_t fan_smooth(uint8_t last_duty, uint8_t duty) __reentrant {
+    uint8_t next_duty = duty;
+
+    // ramping down
+    if (duty < last_duty) {
+        // out of bounds (lower) safeguard
+        uint8_t smoothed = last_duty < MIN_FAN_SPEED + max_jump_down
+            ? MIN_FAN_SPEED
+            : last_duty - max_jump_down;
+
+        // use smoothed value if above min and if smoothed is closer than raw
+        if (last_duty > min_speed_to_smooth && smoothed > duty) {
+            next_duty = smoothed;
+        }
+    }
+
+    // ramping up
+    if (duty > last_duty) {
+        // out of bounds (higher) safeguard
+        uint8_t smoothed = last_duty > MAX_FAN_SPEED - max_jump_up
+            ? MAX_FAN_SPEED
+            : last_duty + max_jump_up;
+
+        // use smoothed value if above min and if smoothed is closer than raw
+        if (duty > min_speed_to_smooth && smoothed < duty) {
+            next_duty = smoothed;
+        }
+    }
+
+    return next_duty;
+}

src/board/system76/common/include/board/fan.h

@@ -7,6 +7,29 @@
 #include <stdint.h>
 
 #define PWM_DUTY(X) ((uint8_t)(((((uint16_t)(X)) * 255) + 99) / 100))
+#define MAX_FAN_SPEED PWM_DUTY(100)
+#define MIN_FAN_SPEED PWM_DUTY(0)
+
+#ifndef SMOOTH_FANS
+    #define SMOOTH_FANS 1 // default to fan smoothing
+#endif
+
+#ifndef SYNC_FANS
+    #define SYNC_FANS 1 // default to syncing fan speeds
+#endif
+
+#if SMOOTH_FANS != 0
+    #ifndef SMOOTH_FANS_UP
+        #define SMOOTH_FANS_UP 45 // default to ~11 seconds for full ramp-up
+    #endif
+    #ifndef SMOOTH_FANS_DOWN
+        #define SMOOTH_FANS_DOWN 100 // default to ~25 seconds for full ramp-down
+    #endif
+#endif
+
+#ifndef SMOOTH_FANS_MIN
+    #define SMOOTH_FANS_MIN 0 // default to smoothing all fan speed changes
+#endif
 
 struct FanPoint {
     int16_t temp;
@@ -31,5 +54,6 @@ uint8_t fan_duty(const struct Fan * fan, int16_t temp) __reentrant;
 void fan_duty_set(uint8_t peci_fan_duty, uint8_t dgpu_fan_duty) __reentrant;
 uint8_t fan_heatup(const struct Fan * fan, uint8_t duty) __reentrant;
 uint8_t fan_cooldown(const struct Fan * fan, uint8_t duty) __reentrant;
+uint8_t fan_smooth(uint8_t last_duty, uint8_t duty) __reentrant;
 
 #endif // _BOARD_FAN_H

src/board/system76/common/main.c

@@ -43,6 +43,10 @@ void serial(void) __interrupt(4) {}
 void timer_2(void) __interrupt(5) {}
 
 uint8_t main_cycle = 0;
+const uint16_t battery_interval = 1000;
+// update fan speed more frequently for smoother fans
+// NOTE: event loop is longer than 100ms and maybe even longer than 250
+const uint16_t fan_interval = SMOOTH_FANS != 0 ? 250 : 1000;
 
 void init(void) {
     // Must happen first
@@ -91,7 +95,9 @@ void main(void) {
 
     INFO("System76 EC board '%s', version '%s'\n", board(), version());
 
-    uint32_t last_time = 0;
+    uint32_t last_time_battery = 0;
+    uint32_t last_time_fan = 0;
+
     for(main_cycle = 0; ; main_cycle++) {
         switch (main_cycle % 3) {
             case 0:
@@ -115,12 +121,17 @@ void main(void) {
 
         if (main_cycle == 0) {
             uint32_t time = time_get();
-            // Only run the following once a second
-            if (last_time > time || (time - last_time) >= 1000) {
-                last_time = time;
+            // Only run the following once per interval
+            if (last_time_fan > time || (time - last_time_fan) >= fan_interval) {
+                last_time_fan = time;
 
                 // Update fan speeds
                 fan_duty_set(peci_get_fan_duty(), dgpu_get_fan_duty());
+            }
+
+            // Only run the following once per interval
+            if (last_time_battery > time || (time - last_time_battery) >= battery_interval) {
+                last_time_battery = time;
 
                 // Updates battery status
                 battery_event();

src/board/system76/common/peci.c

@@ -12,7 +12,7 @@
 
 // Fan speed is the lowest requested over HEATUP seconds
 #ifndef BOARD_HEATUP
-    #define BOARD_HEATUP 10
+    #define BOARD_HEATUP 4
 #endif
 
 static uint8_t FAN_HEATUP[BOARD_HEATUP] = { 0 };
@@ -30,7 +30,7 @@ static uint8_t FAN_COOLDOWN[BOARD_COOLDOWN] = { 0 };
 bool peci_on = false;
 int16_t peci_temp = 0;
 
-#define PECI_TEMP(X) (((int16_t)(X)) << 6)
+#define PECI_TEMP(X) ((int16_t)(X))
 
 #define FAN_POINT(T, D) { .temp = PECI_TEMP(T), .duty = PWM_DUTY(D) }
 
@@ -54,7 +54,7 @@ static struct Fan __code FAN = {
     .heatup_size = ARRAY_SIZE(FAN_HEATUP),
     .cooldown = FAN_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN_COOLDOWN),
-    .interpolate = false,
+    .interpolate = SMOOTH_FANS != 0,
 };
 
 void peci_init(void) {
@@ -156,7 +156,7 @@ uint8_t peci_get_fan_duty(void) {
             // Use result if finished successfully
             uint8_t low = HORDDR;
             uint8_t high = HORDDR;
-            uint16_t peci_offset = ((int16_t)high << 8) | (int16_t)low;
+            uint16_t peci_offset = (((int16_t)high << 8) | (int16_t)low) >> 6;
 
             peci_temp = PECI_TEMP(T_JUNCTION) + peci_offset;
             duty = fan_duty(&FAN, peci_temp);

src/board/system76/galp5/board.mk

@@ -36,23 +36,19 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
-	FAN_POINT(70, 40), \
-	FAN_POINT(75, 60), \
-	FAN_POINT(80, 75), \
-	FAN_POINT(85, 90), \
-	FAN_POINT(90, 100) \
+	FAN_POINT(70, 25), \
+	FAN_POINT(80, 25), \
+	FAN_POINT(80, 40), \
+	FAN_POINT(88, 40), \
+	FAN_POINT(88, 100) \
 "
 
 # DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
-	FAN_POINT(70, 40), \
-	FAN_POINT(75, 60), \
+	FAN_POINT(70, 25), \
+	FAN_POINT(75, 40), \
 	FAN_POINT(80, 75), \
 	FAN_POINT(85, 90), \
 	FAN_POINT(90, 100) \

src/board/system76/gaze15/board.mk

@@ -30,8 +30,6 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -42,8 +40,6 @@ CFLAGS+=-DBOARD_FAN_POINTS="\
 
 # Enable DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/oryp5/board.mk

@@ -31,8 +31,6 @@ CFLAGS+=\
 	-DPOWER_LIMIT_DC=28
 
 # Custom fan curve
-CFLAGS+=-DBOARD_HEATUP=5
-CFLAGS+=-DBOARD_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -43,8 +41,6 @@ CFLAGS+=-DBOARD_FAN_POINTS="\
 
 # Enable DGPU support
 CFLAGS+=-DHAVE_DGPU=1
-CFLAGS+=-DBOARD_DGPU_HEATUP=5
-CFLAGS+=-DBOARD_DGPU_COOLDOWN=20
 CFLAGS+=-DBOARD_DGPU_FAN_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \