fan: Split interval for actual/target duty

src/board/system76/addw3/board.mk

@@ -49,8 +49,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -60,8 +58,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS += -DBOARD_FAN2_HEATUP=5
-CFLAGS += -DBOARD_FAN2_COOLDOWN=20
 CFLAGS += -DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/addw4/board.mk

@@ -45,8 +45,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 28), \
 	FAN_POINT(65, 28), \
@@ -58,8 +56,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS += -DBOARD_FAN2_HEATUP=5
-CFLAGS += -DBOARD_FAN2_COOLDOWN=20
 CFLAGS += -DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 28), \
 	FAN_POINT(65, 28), \

src/board/system76/bonw15-b/board.mk

@@ -56,8 +56,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -67,8 +65,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS += -DBOARD_FAN2_HEATUP=5
-CFLAGS += -DBOARD_FAN2_COOLDOWN=20
 CFLAGS += -DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/bonw15/board.mk

@@ -54,8 +54,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -65,8 +63,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS += -DBOARD_FAN2_HEATUP=5
-CFLAGS += -DBOARD_FAN2_COOLDOWN=20
 CFLAGS += -DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/common/dgpu.c

@@ -9,6 +9,9 @@
 
 int16_t dgpu_temp = 0;
 
+// Update interval is 250ms, so average over 1s period
+static int16_t dgpu_temps[4] = { 0 };
+
 void dgpu_init(void) {
     // Set up for i2c usage
     i2c_reset(&I2C_DGPU, true);
@@ -33,11 +36,18 @@ bool dgpu_get_temp(int16_t *const data) {
 }
 
 void dgpu_read_temp(void) {
+    dgpu_temps[0] = dgpu_temps[1];
+    dgpu_temps[1] = dgpu_temps[2];
+    dgpu_temps[2] = dgpu_temps[3];
+
     if (power_state == POWER_STATE_S0) {
-        if (dgpu_get_temp(&dgpu_temp)) {
-            return;
+        if (!dgpu_get_temp(&dgpu_temps[3])) {
+            dgpu_temps[3] = 0;
         }
+    } else {
+        dgpu_temps[3] = 0;
     }
 
-    dgpu_temp = 0;
+    dgpu_temp = (dgpu_temps[0] + dgpu_temps[1] + dgpu_temps[2] + dgpu_temps[3]) /
+        ARRAY_SIZE(dgpu_temps);
 }

src/board/system76/common/fan.c

@@ -36,19 +36,10 @@ uint16_t fan2_rpm = 0;
 #define FAN1_PWM_MIN 0
 #endif
 
-// Fan speed is the lowest requested over HEATUP seconds
-#ifndef BOARD_FAN1_HEATUP
-#define BOARD_FAN1_HEATUP 4
-#endif
-
-static uint8_t FAN1_HEATUP[BOARD_FAN1_HEATUP] = { 0 };
+// Use highest duty over last COOLDOWN seconds.
+#define FAN1_COOLDOWN_SEC 5
 
-// Fan speed is the highest HEATUP speed over COOLDOWN seconds
-#ifndef BOARD_FAN1_COOLDOWN
-#define BOARD_FAN1_COOLDOWN 10
-#endif
-
-static uint8_t FAN1_COOLDOWN[BOARD_FAN1_COOLDOWN] = { 0 };
+static uint8_t FAN1_COOLDOWN[FAN1_COOLDOWN_SEC] = { 0 };
 
 // Fan curve with temperature in degrees C, duty cycle in percent
 static const struct FanPoint __code FAN1_POINTS[] = {
@@ -62,8 +53,6 @@ static const struct FanPoint __code FAN1_POINTS[] = {
 static const struct Fan __code FAN1 = {
     .points = FAN1_POINTS,
     .points_size = ARRAY_SIZE(FAN1_POINTS),
-    .heatup = FAN1_HEATUP,
-    .heatup_size = ARRAY_SIZE(FAN1_HEATUP),
     .cooldown = FAN1_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN1_COOLDOWN),
     .pwm_min = PWM_DUTY(FAN1_PWM_MIN),
@@ -75,19 +64,10 @@ static const struct Fan __code FAN1 = {
 #define FAN2_PWM_MIN 0
 #endif
 
-// Fan speed is the lowest requested over HEATUP seconds
-#ifndef BOARD_FAN2_HEATUP
-#define BOARD_FAN2_HEATUP 4
-#endif
+// Use highest duty over last COOLDOWN seconds.
+#define FAN2_COOLDOWN_SEC 5
 
-static uint8_t FAN2_HEATUP[BOARD_FAN2_HEATUP] = { 0 };
-
-// Fan speed is the highest HEATUP speed over COOLDOWN seconds
-#ifndef BOARD_FAN2_COOLDOWN
-#define BOARD_FAN2_COOLDOWN 10
-#endif
-
-static uint8_t FAN2_COOLDOWN[BOARD_FAN2_COOLDOWN] = { 0 };
+static uint8_t FAN2_COOLDOWN[FAN2_COOLDOWN_SEC] = { 0 };
 
 // Fan curve with temperature in degrees C, duty cycle in percent
 static const struct FanPoint __code FAN2_POINTS[] = {
@@ -101,8 +81,6 @@ static const struct FanPoint __code FAN2_POINTS[] = {
 static const struct Fan __code FAN2 = {
     .points = FAN2_POINTS,
     .points_size = ARRAY_SIZE(FAN2_POINTS),
-    .heatup = FAN2_HEATUP,
-    .heatup_size = ARRAY_SIZE(FAN2_HEATUP),
     .cooldown = FAN2_COOLDOWN,
     .cooldown_size = ARRAY_SIZE(FAN2_COOLDOWN),
     .pwm_min = PWM_DUTY(FAN2_PWM_MIN),
@@ -137,22 +115,11 @@ static uint8_t fan_duty(const struct Fan *const fan, int16_t temp) {
     return CTR0;
 }
 
-static uint8_t fan_heatup(const struct Fan *const fan, uint8_t duty) {
-    uint8_t lowest = duty;
-
-    uint8_t i;
-    for (i = 0; (i + 1) < fan->heatup_size; i++) {
-        uint8_t value = fan->heatup[i + 1];
-        if (value < lowest) {
-            lowest = value;
-        }
-        fan->heatup[i] = value;
-    }
-    fan->heatup[i] = duty;
-
-    return lowest;
-}
-
+// Apply hysteresis: Use the highest duty over the last COOLDOWN seconds.
+// Keep the fans at higher duties for a short time as the system cools to help
+// reduce temps below the target duty.
+// Useful for addressing the case where fans will constantly start/stop at the
+// point 0 temperature threshold.
 static uint8_t fan_cooldown(const struct Fan *const fan, uint8_t duty) {
     uint8_t highest = duty;
 
@@ -173,7 +140,6 @@ static uint8_t fan_get_duty(const struct Fan *const fan, int16_t temp) {
     uint8_t duty;
 
     duty = fan_duty(fan, temp);
-    duty = fan_heatup(fan, duty);
     duty = fan_cooldown(fan, duty);
 
     return duty;
@@ -197,7 +163,9 @@ static uint16_t fan_get_tach1_rpm(void) {
     return rpm;
 }
 
-void fan_event(void) {
+// Update the target duty of the fans based on system temps.
+// Interval: 1sec
+void fan_update_target(void) {
 #if CONFIG_PLATFORM_INTEL
 #if CONFIG_HAVE_DGPU
     int16_t sys_temp = MAX(peci_temp, dgpu_temp);
@@ -209,19 +177,38 @@ void fan_event(void) {
     int16_t sys_temp = 50;
 #endif
 
-    // Fan update interval is 100ms (main.c). The event changes PWM duty
-    // by 1 every interval to give a smoothing effect.
+    // Set FAN1 target duty.
+    if (fan_max) {
+        fan1_pwm_target = CTR0;
+    } else if (power_state != POWER_STATE_S0) {
+        fan1_pwm_target = 0;
+    } else {
+        fan1_pwm_target = fan_get_duty(&FAN1, sys_temp);
+    }
 
-    // Enabling fan max toggle and exiting S0 will cause duty to immediately
-    // change instead of stepping to provide the desired effects.
+#ifdef FAN2_PWM
+    // Set FAN2 target duty.
+    if (fan_max) {
+        fan2_pwm_target = CTR0;
+    } else if (power_state != POWER_STATE_S0) {
+        fan2_pwm_target = 0;
+    } else {
+        fan2_pwm_target = fan_get_duty(&FAN2, sys_temp);
+    }
+#endif
+}
 
-    // Set FAN1 duty
-    fan1_pwm_target = fan_get_duty(&FAN1, sys_temp);
+// Update the actual duty of the fans to move towards the target duty.
+// The duty is changed by 1 every interval to give a smoothing effect and to
+// avoid large, sudden changes.
+// Enabling fan max toggle and exiting S0 will cause duty to immediately
+// change instead of stepping to provide the desired effects.
+// Interval: 100ms
+void fan_update_duty(void) {
+    // Move FAN1 duty towards target.
     if (fan_max) {
-        fan1_pwm_target = CTR0;
         fan1_pwm_actual = CTR0;
     } else if (power_state != POWER_STATE_S0) {
-        fan1_pwm_target = 0;
         fan1_pwm_actual = 0;
     } else {
         if (fan1_pwm_actual < fan1_pwm_target) {
@@ -242,16 +229,14 @@ void fan_event(void) {
     }
     TRACE("FAN1 duty=%d\n", fan1_pwm_actual);
     FAN1_PWM = fan1_pwm_actual;
+    // Update RPM value for reporting to ACPI.
     fan1_rpm = fan_get_tach0_rpm();
 
 #ifdef FAN2_PWM
-    // set FAN2 duty
-    fan2_pwm_target = fan_get_duty(&FAN2, sys_temp);
+    // Move FAN2 duty towards target.
     if (fan_max) {
-        fan2_pwm_target = CTR0;
         fan2_pwm_actual = CTR0;
     } else if (power_state != POWER_STATE_S0) {
-        fan2_pwm_target = 0;
         fan2_pwm_actual = 0;
     } else {
         if (fan2_pwm_actual < fan2_pwm_target) {
@@ -272,6 +257,7 @@ void fan_event(void) {
     }
     TRACE("FAN2 duty=%d\n", fan2_pwm_actual);
     FAN2_PWM = fan2_pwm_actual;
+    // Update RPM value for reporting to ACPI.
     fan2_rpm = fan_get_tach1_rpm();
 #endif
 }

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

@@ -16,8 +16,6 @@ struct FanPoint {
 struct Fan {
     const struct FanPoint *points;
     uint8_t points_size;
-    uint8_t *heatup;
-    uint8_t heatup_size;
     uint8_t *cooldown;
     uint8_t cooldown_size;
     uint8_t pwm_min;
@@ -33,6 +31,7 @@ extern uint8_t fan2_pwm_target;
 extern uint16_t fan2_rpm;
 
 void fan_reset(void);
-void fan_event(void);
+void fan_update_duty(void);
+void fan_update_target(void);
 
 #endif // _BOARD_FAN_H

src/board/system76/common/main.c

@@ -150,7 +150,7 @@ void main(void) {
         if ((time - last_time_100ms) >= INTERVAL_100MS) {
             last_time_100ms = time;
 
-            fan_event();
+            fan_update_duty();
         }
 
         if ((time - last_time_250ms) >= INTERVAL_250MS) {
@@ -173,6 +173,7 @@ void main(void) {
             last_time_1sec = time;
 
             battery_event();
+            fan_update_target();
         }
 
         // Idle until next timer interrupt

src/board/system76/common/peci.c

@@ -17,6 +17,10 @@
 bool peci_on = false;
 int16_t peci_temp = 0;
 
+// Update interval is 250ms, so average over 1s period.
+// NOTE: PECI GetTemp() internally uses a moving average over 256ms.
+static int16_t peci_temps[4] = { 0 };
+
 // Tjunction = 100C for i7-8565U (and probably the same for all WHL-U)
 #define T_JUNCTION ((int16_t)100)
 
@@ -382,12 +386,19 @@ int16_t peci_wr_pkg_config(uint8_t index, uint16_t param, uint32_t data) {
 #endif // CONFIG_PECI_OVER_ESPI
 
 void peci_read_temp(void) {
+    peci_temps[0] = peci_temps[1];
+    peci_temps[1] = peci_temps[2];
+    peci_temps[2] = peci_temps[3];
+
     peci_on = peci_available();
     if (peci_on) {
-        if (peci_get_temp(&peci_temp)) {
-            return;
+        if (!peci_get_temp(&peci_temps[3])) {
+            peci_temps[3] = 0;
         }
+    } else {
+        peci_temps[3] = 0;
     }
 
-    peci_temp = 0;
+    peci_temp = (peci_temps[0] + peci_temps[1] + peci_temps[2] + peci_temps[3]) /
+        ARRAY_SIZE(peci_temps);
 }

src/board/system76/gaze16-3050/board.mk

@@ -45,8 +45,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -56,8 +54,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS += -DBOARD_FAN2_HEATUP=5
-CFLAGS += -DBOARD_FAN2_COOLDOWN=20
 CFLAGS += -DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \

src/board/system76/gaze16-3060-b/board.mk

@@ -48,8 +48,6 @@ CFLAGS += -DI2C_DGPU=I2C_1
 
 # Fan configs
 CFLAGS += -DFAN1_PWM=DCR2
-CFLAGS += -DBOARD_FAN1_HEATUP=5
-CFLAGS += -DBOARD_FAN1_COOLDOWN=20
 CFLAGS += -DBOARD_FAN1_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \
@@ -59,8 +57,6 @@ CFLAGS += -DBOARD_FAN1_POINTS="\
 "
 
 CFLAGS += -DFAN2_PWM=DCR4
-CFLAGS+=-DBOARD_FAN2_HEATUP=5
-CFLAGS+=-DBOARD_FAN2_COOLDOWN=20
 CFLAGS+=-DBOARD_FAN2_POINTS="\
 	FAN_POINT(60, 40), \
 	FAN_POINT(65, 60), \