Merge pull request #43 from system76/wip/fan

Fan curve improvements

.gitignore

@@ -1 +1,3 @@
 /build/
+backup.rom
+power.csv

power.sh

@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+
+set -e
+
+./ectool.sh info
+
+header=1
+if [ -e power.csv ]
+then
+    header=0
+fi
+
+while true
+do
+    if [ "${header}" == "1" ]
+    then
+        F="Time    "
+        F="${F}\tBAT W"
+        F="${F}\tCPU W"
+        F="${F}\tCPU C"
+        F="${F}\tFAN %"
+    else
+        F="$(date "+%T")"
+
+        uV="$(cat /sys/class/power_supply/BAT0/voltage_now)"
+        V="$(echo "${uV}/1000000" | bc -lq)"
+        uA="$(cat /sys/class/power_supply/BAT0/current_now)"
+        A="$(echo "${uA}/1000000" | bc -lq)"
+        bat_W="$(echo "${V} * ${A}" | bc -lq)"
+        F="${F}\t$(printf "%.2f" "${bat_W}")"
+
+        last_E="$(cat /sys/class/powercap/intel-rapl\:0/energy_uj)"
+        sleep 1
+        next_E="$(cat /sys/class/powercap/intel-rapl\:0/energy_uj)"
+        W="$(echo "(${next_E} - ${last_E})/1000000" | bc -lq)"
+        F="${F}\t$(printf "%.1f" "${W}")"
+
+        T="$(cat /sys/devices/platform/coretemp.0/hwmon/hwmon*/temp1_input)"
+        C="$(echo "${T}/1000" | bc -lq)"
+        F="${F}\t$(printf "%.1f" "${C}")"
+
+        D="$(sudo tool/target/release/system76_ectool fan 0)"
+        P="$(echo "(${D} * 100)/255" | bc -lq)"
+        F="${F}\t$(printf "%.0f" "${P}")"
+    fi
+
+    for file in /sys/devices/system/cpu/cpu*/cpufreq/scaling_cur_freq
+    do
+        if [ "${header}" == "1" ]
+        then
+            id="$(basename "$(dirname "$(dirname "${file}")")")"
+            F="${F}\t${id}"
+        else
+            KHz="$(cat "${file}")"
+            MHz="$(echo "${KHz}/1000" | bc -lq)"
+            F="${F}\t$(printf "%.0f" "${MHz}")"
+        fi
+    done
+
+    echo -e "${F}" | tee -a power.csv
+
+    header=0
+done

src/board/system76/darp5/peci.c

@@ -7,6 +7,12 @@
 #include <ec/gpio.h>
 #include <ec/pwm.h>
 
+// Fan speed is the lowest requested over HEATUP seconds
+#define HEATUP 5
+
+// Fan speed is the highest HEATUP speed over COOLDOWN seconds
+#define COOLDOWN 10
+
 // Tjunction = 100C for i7-8565U (and probably the same for all WHL-U)
 #define T_JUNCTION 100
 
@@ -30,7 +36,7 @@ struct FanPoint __code FAN_POINTS[] = {
     FAN_POINT(70,  45),
     FAN_POINT(75,  55),
     FAN_POINT(80,  75),
-    FAN_POINT(84, 100),
+    FAN_POINT(84, 100)
 };
 
 // Get duty cycle based on temperature, adapted from
@@ -51,12 +57,14 @@ uint8_t fan_duty(int16_t temp) {
 
                 // If in between current temp and previous temp, interpolate
                 if (temp > prev->temp) {
-                    int16_t dtemp = (cur->temp - prev->temp);
+                    return prev->duty;
-                    int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
+
-                    return (uint8_t)(
+                    // int16_t dtemp = (cur->temp - prev->temp);
-                        ((int16_t)prev->duty) +
+                    // int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
-                        ((temp - prev->temp) * dduty) / dtemp
+                    // return (uint8_t)(
-                    );
+                    //     ((int16_t)prev->duty) +
+                    //     ((temp - prev->temp) * dduty) / dtemp
+                    // );
                 }
             }
         }
@@ -66,6 +74,40 @@ uint8_t fan_duty(int16_t temp) {
     return PWM_DUTY(100);
 }
 
+uint8_t fan_heatup(uint8_t duty) {
+    static uint8_t history[HEATUP] = { 0 };
+    uint8_t lowest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value < lowest) {
+            lowest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return lowest;
+}
+
+uint8_t fan_cooldown(uint8_t duty) {
+    static uint8_t history[COOLDOWN] = { 0 };
+    uint8_t highest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value > highest) {
+            highest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return highest;
+}
+
 void peci_init(void) {
     // Allow PECI pin to be used
     GCR2 |= (1 << 4);
@@ -123,8 +165,10 @@ void peci_event(void) {
         peci_duty = PWM_DUTY(0);
     }
 
-    if (peci_duty != DCR2) {
+    uint8_t heatup_duty = fan_heatup(peci_duty);
-        DCR2 = peci_duty;
+    uint8_t cooldown_duty = fan_cooldown(heatup_duty);
-        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, peci_duty);
+    if (cooldown_duty != DCR2) {
+        DCR2 = cooldown_duty;
+        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, cooldown_duty);
     }
 }

src/board/system76/darp5/power.c

@@ -386,11 +386,10 @@ void power_event(void) {
     // state is S3
     static bool ack_last = false;
     bool ack_new = gpio_get(&SUSWARN_N);
-    if (ack_new && !ack_last) {
-        DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
-    }
     #if LEVEL >= LEVEL_DEBUG
-        else if (!ack_new && ack_last) {
+        if (ack_new && !ack_last) {
+            DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
+        } else if (!ack_new && ack_last) {
             DEBUG("%02X: SUSPWRDNACK de-asserted\n", main_cycle);
         }
     #endif

src/board/system76/darp5/smfi.c

@@ -9,6 +9,7 @@
 #include <common/command.h>
 #include <common/macro.h>
 #include <common/version.h>
+#include <ec/pwm.h>
 
 // Shared memory host semaphore
 volatile uint8_t __xdata __at(0x1022) SMHSR;
@@ -143,6 +144,30 @@ static enum Result cmd_reset(void) {
     return RES_ERR;
 }
 
+static enum Result cmd_fan_get(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Get duty of fan 0
+        smfi_cmd[3] = DCR2;
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
+static enum Result cmd_fan_set(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Set duty cycle of fan 0
+        DCR2 = smfi_cmd[3];
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
 void smfi_event(void) {
     if (smfi_cmd[0]) {
         switch (smfi_cmd[0]) {
@@ -174,6 +199,14 @@ void smfi_event(void) {
             case CMD_RESET:
                 smfi_cmd[1] = cmd_reset();
                 break;
+#ifndef __SCRATCH__
+            case CMD_FAN_GET:
+                smfi_cmd[1] = cmd_fan_get();
+                break;
+            case CMD_FAN_SET:
+                smfi_cmd[1] = cmd_fan_set();
+                break;
+#endif // __SCRATCH__
             default:
                 // Command not found
                 smfi_cmd[1] = RES_ERR;

src/board/system76/galp3-c/peci.c

@@ -7,6 +7,12 @@
 #include <ec/gpio.h>
 #include <ec/pwm.h>
 
+// Fan speed is the lowest requested over HEATUP seconds
+#define HEATUP 5
+
+// Fan speed is the highest HEATUP speed over COOLDOWN seconds
+#define COOLDOWN 10
+
 // Tjunction = 100C for i7-8565U (and probably the same for all WHL-U)
 #define T_JUNCTION 100
 
@@ -30,7 +36,7 @@ struct FanPoint __code FAN_POINTS[] = {
     FAN_POINT(70,  45),
     FAN_POINT(75,  55),
     FAN_POINT(80,  75),
-    FAN_POINT(84, 100),
+    FAN_POINT(84, 100)
 };
 
 // Get duty cycle based on temperature, adapted from
@@ -51,12 +57,14 @@ uint8_t fan_duty(int16_t temp) {
 
                 // If in between current temp and previous temp, interpolate
                 if (temp > prev->temp) {
-                    int16_t dtemp = (cur->temp - prev->temp);
+                    return prev->duty;
-                    int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
+
-                    return (uint8_t)(
+                    // int16_t dtemp = (cur->temp - prev->temp);
-                        ((int16_t)prev->duty) +
+                    // int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
-                        ((temp - prev->temp) * dduty) / dtemp
+                    // return (uint8_t)(
-                    );
+                    //     ((int16_t)prev->duty) +
+                    //     ((temp - prev->temp) * dduty) / dtemp
+                    // );
                 }
             }
         }
@@ -66,6 +74,40 @@ uint8_t fan_duty(int16_t temp) {
     return PWM_DUTY(100);
 }
 
+uint8_t fan_heatup(uint8_t duty) {
+    static uint8_t history[HEATUP] = { 0 };
+    uint8_t lowest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value < lowest) {
+            lowest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return lowest;
+}
+
+uint8_t fan_cooldown(uint8_t duty) {
+    static uint8_t history[COOLDOWN] = { 0 };
+    uint8_t highest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value > highest) {
+            highest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return highest;
+}
+
 void peci_init(void) {
     // Allow PECI pin to be used
     GCR2 |= (1 << 4);
@@ -123,8 +165,10 @@ void peci_event(void) {
         peci_duty = PWM_DUTY(0);
     }
 
-    if (peci_duty != DCR2) {
+    uint8_t heatup_duty = fan_heatup(peci_duty);
-        DCR2 = peci_duty;
+    uint8_t cooldown_duty = fan_cooldown(heatup_duty);
-        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, peci_duty);
+    if (cooldown_duty != DCR2) {
+        DCR2 = cooldown_duty;
+        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, cooldown_duty);
     }
 }

src/board/system76/galp3-c/power.c

@@ -383,11 +383,10 @@ void power_event(void) {
     // state is S3
     static bool ack_last = false;
     bool ack_new = gpio_get(&SUSWARN_N);
-    if (ack_new && !ack_last) {
-        DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
-    }
     #if LEVEL >= LEVEL_DEBUG
-        else if (!ack_new && ack_last) {
+        if (ack_new && !ack_last) {
+            DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
+        } else if (!ack_new && ack_last) {
             DEBUG("%02X: SUSPWRDNACK de-asserted\n", main_cycle);
         }
     #endif

src/board/system76/galp3-c/smfi.c

@@ -9,6 +9,7 @@
 #include <common/command.h>
 #include <common/macro.h>
 #include <common/version.h>
+#include <ec/pwm.h>
 
 // Shared memory host semaphore
 volatile uint8_t __xdata __at(0x1022) SMHSR;
@@ -143,6 +144,30 @@ static enum Result cmd_reset(void) {
     return RES_ERR;
 }
 
+static enum Result cmd_fan_get(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Get duty of fan 0
+        smfi_cmd[3] = DCR2;
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
+static enum Result cmd_fan_set(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Set duty cycle of fan 0
+        DCR2 = smfi_cmd[3];
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
 void smfi_event(void) {
     if (smfi_cmd[0]) {
         switch (smfi_cmd[0]) {
@@ -174,6 +199,14 @@ void smfi_event(void) {
             case CMD_RESET:
                 smfi_cmd[1] = cmd_reset();
                 break;
+#ifndef __SCRATCH__
+            case CMD_FAN_GET:
+                smfi_cmd[1] = cmd_fan_get();
+                break;
+            case CMD_FAN_SET:
+                smfi_cmd[1] = cmd_fan_set();
+                break;
+#endif // __SCRATCH__
             default:
                 // Command not found
                 smfi_cmd[1] = RES_ERR;

src/board/system76/lemp9/peci.c

@@ -7,6 +7,12 @@
 #include <ec/gpio.h>
 #include <ec/pwm.h>
 
+// Fan speed is the lowest requested over HEATUP seconds
+#define HEATUP 5
+
+// Fan speed is the highest HEATUP speed over COOLDOWN seconds
+#define COOLDOWN 10
+
 // Tjunction = 100C for i7-8565U (and probably the same for all WHL-U)
 #define T_JUNCTION 100
 
@@ -30,7 +36,7 @@ struct FanPoint __code FAN_POINTS[] = {
     FAN_POINT(70,  45),
     FAN_POINT(75,  55),
     FAN_POINT(80,  75),
-    FAN_POINT(84, 100),
+    FAN_POINT(84, 100)
 };
 
 // Get duty cycle based on temperature, adapted from
@@ -51,12 +57,14 @@ uint8_t fan_duty(int16_t temp) {
 
                 // If in between current temp and previous temp, interpolate
                 if (temp > prev->temp) {
-                    int16_t dtemp = (cur->temp - prev->temp);
+                    return prev->duty;
-                    int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
+
-                    return (uint8_t)(
+                    // int16_t dtemp = (cur->temp - prev->temp);
-                        ((int16_t)prev->duty) +
+                    // int16_t dduty = ((int16_t)cur->duty) - ((int16_t)prev->duty);
-                        ((temp - prev->temp) * dduty) / dtemp
+                    // return (uint8_t)(
-                    );
+                    //     ((int16_t)prev->duty) +
+                    //     ((temp - prev->temp) * dduty) / dtemp
+                    // );
                 }
             }
         }
@@ -66,6 +74,40 @@ uint8_t fan_duty(int16_t temp) {
     return PWM_DUTY(100);
 }
 
+uint8_t fan_heatup(uint8_t duty) {
+    static uint8_t history[HEATUP] = { 0 };
+    uint8_t lowest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value < lowest) {
+            lowest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return lowest;
+}
+
+uint8_t fan_cooldown(uint8_t duty) {
+    static uint8_t history[COOLDOWN] = { 0 };
+    uint8_t highest = duty;
+
+    int i;
+    for (i = 0; (i + 1) < ARRAY_SIZE(history); i++) {
+        uint8_t value = history[i + 1];
+        if (value > highest) {
+            highest = value;
+        }
+        history[i] = value;
+    }
+    history[i] = duty;
+
+    return highest;
+}
+
 void peci_init(void) {
     // Allow PECI pin to be used
     GCR2 |= (1 << 4);
@@ -123,8 +165,10 @@ void peci_event(void) {
         peci_duty = PWM_DUTY(0);
     }
 
-    if (peci_duty != DCR2) {
+    uint8_t heatup_duty = fan_heatup(peci_duty);
-        DCR2 = peci_duty;
+    uint8_t cooldown_duty = fan_cooldown(heatup_duty);
-        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, peci_duty);
+    if (cooldown_duty != DCR2) {
+        DCR2 = cooldown_duty;
+        DEBUG("PECI offset=%d, temp=%d = %d\n", peci_offset, peci_temp, cooldown_duty);
     }
 }

src/board/system76/lemp9/power.c

@@ -383,11 +383,10 @@ void power_event(void) {
     // state is S3
     static bool ack_last = false;
     bool ack_new = gpio_get(&SUSWARN_N);
-    if (ack_new && !ack_last) {
-        DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
-    }
     #if LEVEL >= LEVEL_DEBUG
-        else if (!ack_new && ack_last) {
+        if (ack_new && !ack_last) {
+            DEBUG("%02X: SUSPWRDNACK asserted\n", main_cycle);
+        } else if (!ack_new && ack_last) {
             DEBUG("%02X: SUSPWRDNACK de-asserted\n", main_cycle);
         }
     #endif

src/board/system76/lemp9/smfi.c

@@ -9,6 +9,7 @@
 #include <common/command.h>
 #include <common/macro.h>
 #include <common/version.h>
+#include <ec/pwm.h>
 
 // Shared memory host semaphore
 volatile uint8_t __xdata __at(0x1022) SMHSR;
@@ -143,6 +144,30 @@ static enum Result cmd_reset(void) {
     return RES_ERR;
 }
 
+static enum Result cmd_fan_get(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Get duty of fan 0
+        smfi_cmd[3] = DCR2;
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
+static enum Result cmd_fan_set(void) {
+    // If setting fan 0
+    if (smfi_cmd[2] == 0) {
+        // Set duty cycle of fan 0
+        DCR2 = smfi_cmd[3];
+        return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
 void smfi_event(void) {
     if (smfi_cmd[0]) {
         switch (smfi_cmd[0]) {
@@ -174,6 +199,14 @@ void smfi_event(void) {
             case CMD_RESET:
                 smfi_cmd[1] = cmd_reset();
                 break;
+#ifndef __SCRATCH__
+            case CMD_FAN_GET:
+                smfi_cmd[1] = cmd_fan_get();
+                break;
+            case CMD_FAN_SET:
+                smfi_cmd[1] = cmd_fan_set();
+                break;
+#endif // __SCRATCH__
             default:
                 // Command not found
                 smfi_cmd[1] = RES_ERR;

src/common/include/common/command.h

@@ -16,6 +16,10 @@ enum Command {
     CMD_SPI = 5,
     // Reset EC
     CMD_RESET = 6,
+    // Get fan speeds
+    CMD_FAN_GET = 7,
+    // Set fan speeds
+    CMD_FAN_SET = 8,
     //TODO
 };
 

tool/src/ec.rs

@@ -19,12 +19,14 @@ pub enum Cmd {
     Print = 4,
     Spi = 5,
     Reset = 6,
+    FanGet = 7,
+    FanSet = 8,
 }
 
-pub const CMD_SPI_FLAG_READ: u8 = (1 << 0);
+pub const CMD_SPI_FLAG_READ: u8 = 1 << 0;
-pub const CMD_SPI_FLAG_DISABLE: u8 = (1 << 1);
+pub const CMD_SPI_FLAG_DISABLE: u8 = 1 << 1;
-pub const CMD_SPI_FLAG_SCRATCH: u8 = (1 << 2);
+pub const CMD_SPI_FLAG_SCRATCH: u8 = 1 << 2;
-pub const CMD_SPI_FLAG_BACKUP: u8 = (1 << 3);
+pub const CMD_SPI_FLAG_BACKUP: u8 = 1 << 3;
 
 pub struct Ec<T: Timeout> {
     cmd: u16,
@@ -181,6 +183,18 @@ impl<T: Timeout> Ec<T> {
     pub unsafe fn reset(&mut self) -> Result<(), Error> {
         self.command(Cmd::Reset)
     }
+
+    pub unsafe fn fan_get(&mut self, index: u8) -> Result<u8, Error> {
+        self.write(2, index);
+        self.command(Cmd::FanGet)?;
+        Ok(self.read(3))
+    }
+
+    pub unsafe fn fan_set(&mut self, index: u8, duty: u8) -> Result<(), Error> {
+        self.write(2, index);
+        self.write(3, duty);
+        self.command(Cmd::FanSet)
+    }
 }
 
 pub struct EcSpi<'a, T: Timeout> {

tool/src/main.rs

@@ -274,9 +274,33 @@ unsafe fn print(message: &[u8]) -> Result<(), Error> {
     Ok(())
 }
 
+unsafe fn fan_get(index: u8) -> Result<(), Error> {
+    iopl();
+
+    let mut ec = Ec::new(
+        StdTimeout::new(Duration::new(1, 0)),
+    )?;
+
+    let duty = ec.fan_get(index)?;
+    println!("{}", duty);
+
+    Ok(())
+}
+
+unsafe fn fan_set(index: u8, duty: u8) -> Result<(), Error> {
+    iopl();
+
+    let mut ec = Ec::new(
+        StdTimeout::new(Duration::new(1, 0)),
+    )?;
+
+    ec.fan_set(index, duty)
+}
+
 fn usage() {
     eprintln!("  console");
     eprintln!("  flash [file]");
+    eprintln!("  fan [index] <duty>");
     eprintln!("  info");
     eprintln!("  print [message]");
 }
@@ -293,6 +317,40 @@ fn main() {
                     process::exit(1);
                 },
             },
+            "fan" => match args.next() {
+                Some(index_str) => match index_str.parse::<u8>() {
+                    Ok(index) => match args.next() {
+                        Some(duty_str) => match duty_str.parse::<u8>() {
+                            Ok(duty) => match unsafe { fan_set(index, duty) } {
+                                Ok(()) => (),
+                                Err(err) => {
+                                    eprintln!("failed to set fan {} to {}: {:X?}", index, duty, err);
+                                    process::exit(1);
+                                },
+                            },
+                            Err(err) => {
+                                eprintln!("failed to parse '{}': {:X?}", duty_str, err);
+                                process::exit(1);
+                            },
+                        },
+                        None => match unsafe { fan_get(index) } {
+                            Ok(()) => (),
+                            Err(err) => {
+                                eprintln!("failed to get fan {}: {:X?}", index, err);
+                                process::exit(1);
+                            },
+                        },
+                    },
+                    Err(err) => {
+                        eprintln!("failed to parse '{}': {:X?}", index_str, err);
+                        process::exit(1);
+                    },
+                },
+                None => {
+                    eprintln!("no index provided");
+                    process::exit(1);
+                },
+            },
             "flash" => match args.next() {
                 Some(path) => match unsafe { flash(&path) } {
                     Ok(()) => (),