fan smoothing and syncing

src/board/system76/common/dgpu.c

@@ -1,10 +1,9 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
 #include <board/dgpu.h>
+#include <board/fan.h>
 
 #if HAVE_DGPU
-
-#include <board/fan.h>
 #include <board/gpio.h>
 #include <board/power.h>
 #include <common/debug.h>
@@ -52,15 +51,19 @@ static struct Fan __code FAN = {
     .heatup_size = ARRAY_SIZE(FAN_HEATUP),
     .cooldown = FAN_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN_COOLDOWN),
-    .interpolate = false,
+    #if defined(FAN_SMOOTHING) || defined(FAN_SMOOTHING_UP) || defined(FAN_SMOOTHING_DOWN)
+      .interpolate = true,
+    #else
+      .interpolate = false,
+    #endif
 };
 
 void dgpu_init(void) {
     // Set up for i2c usage
     i2c_reset(&I2C_DGPU, true);
 }
 
-void dgpu_event(void) {
+uint8_t dgpu_get_fan_duty(void) {
     uint8_t duty;
     if (power_state == POWER_STATE_S0 && gpio_get(&DGPU_PWR_EN) && !gpio_get(&GC6_FB_EN)) {
         // Use I2CS if in S0 state
@@ -90,16 +93,16 @@ void dgpu_event(void) {
         duty = fan_cooldown(&FAN, duty);
     }
 
-    if (duty != DCR4) {
-        DCR4 = duty;
-        DEBUG("DGPU temp=%d = %d\n", dgpu_temp, duty);
-    }
+    DEBUG("DGPU temp=%d\n", dgpu_temp);
+    return duty;
 }
 
 #else
 
 void dgpu_init(void) {}
 
-void dgpu_event(void) {}
+uint8_t dgpu_get_fan_duty(void) {
+  return PWM_DUTY(0);
+}
 
 #endif // HAVE_DGPU

src/board/system76/common/fan.c

@@ -1,8 +1,34 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
 #include <board/fan.h>
+#include <common/debug.h>
+#include <ec/pwm.h>
 
 bool fan_max = false;
+uint8_t last_duty_dgpu = 0;
+uint8_t last_duty_peci = 0;
+
+#define max_speed PWM_DUTY(100)
+#define min_speed PWM_DUTY(0)
+
+#if !defined(FAN_SMOOTHING) && (defined(FAN_SMOOTHING_UP) || defined(FAN_SMOOTHING_DOWN))
+  #define FAN_SMOOTHING 40
+#endif
+
+#ifdef FAN_SMOOTHING
+  #ifndef FAN_SMOOTHING_UP
+    #define FAN_SMOOTHING_UP FAN_SMOOTHING
+  #endif
+  #ifndef FAN_SMOOTHING_DOWN
+    #define FAN_SMOOTHING_DOWN FAN_SMOOTHING
+  #endif
+
+  const uint8_t max_jump_up = (max_speed - min_speed) / (uint8_t) FAN_SMOOTHING_UP;
+  const uint8_t max_jump_down = (max_speed - min_speed) / (uint8_t) FAN_SMOOTHING_DOWN;
+#else
+  const uint8_t max_jump_up = max_speed - min_speed;
+  const uint8_t max_jump_down = max_speed - min_speed;
+#endif
 
 void fan_reset(void) {
     // Do not manually set fans to maximum speed
@@ -21,7 +47,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 PWM_DUTY(0);
+                return min_speed;
             } else {
                 const struct FanPoint * prev = &fan->points[i - 1];
 
@@ -43,7 +69,30 @@ uint8_t fan_duty(const struct Fan * fan, int16_t temp) __reentrant {
     }
 
     // If no point is found, return 100%
-    return PWM_DUTY(100);
+    return max_speed;
+}
+
+void fan_duty_set(uint8_t peci_fan_duty, uint8_t dgpu_fan_duty) __reentrant {
+  #ifdef SYNC_FANS
+    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_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_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 {
@@ -77,3 +126,33 @@ 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_speed + max_jump_down
+      ? min_speed
+      : last_duty - max_jump_down;
+
+    if (smoothed > duty) {
+      next_duty = smoothed;
+    }
+  }
+
+  // ramping up
+  if (duty > last_duty) {
+    // out of bounds (higher) safeguard
+    uint8_t smoothed = last_duty > max_speed - max_jump_up
+      ? max_speed
+      : last_duty + max_jump_up;
+
+    if (smoothed < duty) {
+      next_duty = smoothed;
+    }
+  }
+
+  return next_duty;
+}

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

@@ -14,6 +14,6 @@
 #endif // HAVE_DGPU
 
 void dgpu_init(void);
-void dgpu_event(void);
+uint8_t dgpu_get_fan_duty(void);
 
 #endif // _BOARD_DGPU_H

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

@@ -28,7 +28,9 @@ extern bool fan_max;
 void fan_reset(void);
 
 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/include/board/peci.h

@@ -9,6 +9,6 @@ extern int16_t peci_temp;
 
 void peci_init(void);
 int peci_wr_pkg_config(uint8_t index, uint16_t param, uint32_t data);
-void peci_event(void);
+uint8_t peci_get_fan_duty(void);
 
 #endif // _BOARD_PECI_H

src/board/system76/common/main.c

@@ -10,6 +10,7 @@
 #include <board/board.h>
 #include <board/dgpu.h>
 #include <board/ecpm.h>
+#include <board/fan.h>
 #include <board/gpio.h>
 #include <board/gctrl.h>
 #include <board/kbc.h>
@@ -42,6 +43,12 @@ void serial(void) __interrupt(4) {}
 void timer_2(void) __interrupt(5) {}
 
 uint8_t main_cycle = 0;
+const uint16_t battery_interval = 1000;
+#if defined(FAN_SMOOTHING) || defined(FAN_SMOOTHING_UP) || defined(FAN_SMOOTHING_DOWN)
+  const uint16_t fan_interval = 250; // event loop is longer than 100ms and maybe even longer than 250
+#else
+  const uint16_t fan_interval = 1000;
+#endif
 
 void init(void) {
     // Must happen first
@@ -90,7 +97,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:
@@ -114,17 +123,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;
 
-                // Updates fan status and temps
-                peci_event();
+                // Update fan speeds
+                fan_duty_set(peci_get_fan_duty(), dgpu_get_fan_duty());
+            }
 
-#if HAVE_DGPU
-                // Updates discrete GPU fan status and temps
-                dgpu_event();
-#endif
+            // 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

@@ -29,7 +29,7 @@ static uint8_t FAN_COOLDOWN[BOARD_COOLDOWN] = { 0 };
 
 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) }
 
@@ -53,7 +53,11 @@ static struct Fan __code FAN = {
     .heatup_size = ARRAY_SIZE(FAN_HEATUP),
     .cooldown = FAN_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN_COOLDOWN),
-    .interpolate = false,
+    #if defined(FAN_SMOOTHING) || defined(FAN_SMOOTHING_UP) || defined(FAN_SMOOTHING_DOWN)
+      .interpolate = true,
+    #else
+      .interpolate = false,
+    #endif
 };
 
 void peci_init(void) {
@@ -118,7 +122,7 @@ int peci_wr_pkg_config(uint8_t index, uint16_t param, uint32_t data) {
 }
 
 // PECI information can be found here: https://www.intel.com/content/dam/www/public/us/en/documents/design-guides/core-i7-lga-2011-guide.pdf
-void peci_event(void) {
+uint8_t peci_get_fan_duty(void) {
     uint8_t duty;
 
 #if EC_ESPI
@@ -154,10 +158,11 @@ void peci_event(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);
+            DEBUG("PECI offset=%d temp=%d duty=%d\n", peci_offset, peci_temp, duty);
         } else {
             // Default to 50% if there is an error
             peci_temp = 0;
@@ -178,8 +183,6 @@ void peci_event(void) {
         duty = fan_cooldown(&FAN, duty);
     }
 
-    if (duty != DCR2) {
-        DCR2 = duty;
-        DEBUG("PECI temp=%d = %d\n", peci_temp, duty);
-    }
+    DEBUG("PECI temp=%d\n", peci_temp);
+    return duty;
 }

src/board/system76/common/pwm.c

@@ -22,7 +22,7 @@ void pwm_init(void) {
     // Set cycle time to 255 + 1
     CTR0 = 255;
 
-    // Turn off CPU fan (temperature control in peci_event)
+    // Turn off CPU fan (temperature control in peci_get_fan_duty)
     DCR2 = 0;
 
     // Enable PWM

src/board/system76/galp5/board.mk

@@ -40,6 +40,12 @@ CFLAGS+=\
 	-DPOWER_LIMIT_AC=65 \
 	-DPOWER_LIMIT_DC=28
 
+# sync GPU fan speed to CPU fan speed (great for galp5 w/o dGPU)
+CFLAGS+=-DSYNC_FANS=1
+# smooth the fan speed updates such that 0 to full speed happens over this period (divide by 4 for seconds)
+CFLAGS+=-DFAN_SMOOTHING_UP=45
+CFLAGS+=-DFAN_SMOOTHING_DOWN=100
+
 # Custom fan curve
 CFLAGS+=-DBOARD_HEATUP=5
 CFLAGS+=-DBOARD_COOLDOWN=20