Implement persistent options for keyboard backlight

src/board/system76/common/acpi.c

@@ -21,49 +21,6 @@ enum EcOs acpi_ecos = EC_OS_NONE;
 
 extern bool pmc_s0_hack;
 
-static uint8_t fcmd = 0;
-static uint8_t fdat = 0;
-static uint8_t fbuf[4] = { 0, 0, 0, 0 };
-
-void fcommand(void) {
-    uint32_t color;
-    switch (fcmd) {
-    // Keyboard backlight
-    case 0xCA:
-        switch (fdat) {
-        // Set LED brightness
-        case 0x00:
-            kbled_set_brightness(fbuf[0]);
-            break;
-        // Get LED brightness
-        case 0x01:
-            fbuf[0] = kbled_get();
-            break;
-        // Get type
-        case 2:
-            fbuf[0] = kbled_kind;
-        // Set LED color
-        case 0x03:
-            kbled_set_color(
-                ((uint32_t)fbuf[0]) | ((uint32_t)fbuf[1] << 16) | ((uint32_t)fbuf[2] << 8)
-            );
-            break;
-        // Get LED color
-        case 0x04:
-            color = kbled_get_color();
-            fbuf[0] = color & 0x0000ff;
-            fbuf[1] = (color & 0xff0000) >> 16;
-            fbuf[2] = (color & 0x00ff00) >> 8;
-            break;
-        // Enable / disable LED
-        case 0x05:
-            kbled_enable(!!fbuf[0]);
-            break;
-        }
-        break;
-    }
-}
-
 void acpi_reset(void) {
     // Disable lid wake
     lid_wake = false;
@@ -178,13 +135,6 @@ uint8_t acpi_read(uint8_t addr) {
 
         // Set size of flash (from old firmware)
         ACPI_8 (0xE5, 0x80);
-
-        ACPI_8 (0xF8, fcmd);
-        ACPI_8 (0xF9, fdat);
-        ACPI_8 (0xFA, fbuf[0]);
-        ACPI_8 (0xFB, fbuf[1]);
-        ACPI_8 (0xFC, fbuf[2]);
-        ACPI_8 (0xFD, fbuf[3]);
     }
 
     TRACE("acpi_read %02X = %02X\n", addr, data);
@@ -219,25 +169,5 @@ void acpi_write(uint8_t addr, uint8_t data) {
         gpio_set(&LED_AIRPLANE_N, !(bool)(data & BIT(6)));
         break;
 #endif
-
-    case 0xF8:
-        fcmd = data;
-        fcommand();
-        break;
-    case 0xF9:
-        fdat = data;
-        break;
-    case 0xFA:
-        fbuf[0] = data;
-        break;
-    case 0xFB:
-        fbuf[1] = data;
-        break;
-    case 0xFC:
-        fbuf[2] = data;
-        break;
-    case 0xFD:
-        fbuf[3] = data;
-        break;
     }
 }

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

@@ -23,6 +23,7 @@ uint32_t kbled_get_color(void);
 void kbled_set_color(uint32_t color);
 
 // Provided by common code
+void kbled_restore(void);
 void kbled_enable(bool enabled);
 void kbled_set_brightness(uint8_t value);
 void kbled_hotkey_color(void);

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

@@ -8,12 +8,6 @@
 
 // Initialize the options
 void options_init(void);
-// Set the options to the default options
-void options_load_default(void);
-// Erase options in flash
-bool options_erase_config(void);
-// Load options from flash
-bool options_load_config(void);
 // Save options to flash
 bool options_save_config(void);
 // Get an option
@@ -23,6 +17,10 @@ bool options_set(uint16_t index, uint8_t value);
 
 enum {
     OPT_POWER_ON_AC = 0,
+    OPT_KBLED_BRIGHTNESS,
+    OPT_KBLED_COLOR_I,
+    OPT_BAT_THRESHOLD_START,
+    OPT_BAT_THRESHOLD_STOP,
     NUM_OPTIONS
 };
 

src/board/system76/common/kbled.c

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
 #include <board/kbled.h>
+#include <board/options.h>
 #include <common/macro.h>
 
 enum KbledKind kbled_kind = KBLED_NONE;
@@ -43,6 +44,11 @@ static const uint32_t __code COLORS[] = {
 static bool enabled = false;
 static uint8_t brightness = 0;
 
+void kbled_restore(void) {
+    kbled_set_brightness(options_get(OPT_KBLED_BRIGHTNESS));
+    kbled_set_color(COLORS[options_get(OPT_KBLED_COLOR_I)]);
+}
+
 void kbled_enable(bool enable) {
     enabled = enable;
 
@@ -58,6 +64,7 @@ void kbled_set_brightness(uint8_t value) {
     if (enabled) {
         kbled_set(brightness);
     }
+    options_set(OPT_KBLED_BRIGHTNESS, brightness);
 }
 
 void kbled_hotkey_color(void) {
@@ -67,6 +74,7 @@ void kbled_hotkey_color(void) {
         COLOR_I = 0;
     }
     kbled_set_color(COLORS[COLOR_I]);
+    options_set(OPT_KBLED_COLOR_I, COLOR_I);
 }
 
 void kbled_hotkey_down(void) {

src/board/system76/common/kbled/rgb_pwm.c

@@ -2,6 +2,7 @@
 
 #include <board/gpio.h>
 #include <board/kbled.h>
+#include <board/options.h>
 #include <ec/pwm.h>
 
 void kbled_init(void) {
@@ -17,11 +18,7 @@ void kbled_init(void) {
 }
 
 void kbled_reset(void) {
-    // Set brightness and color
-    kbled_set_brightness(0);
-    if (gpio_get(&LID_SW_N))
-        kbled_enable(true);
-    kbled_set_color(0xFFFFFF);
+    kbled_restore();
 }
 
 uint8_t kbled_get(void) {

src/board/system76/common/kbled/white_dac.c

@@ -20,9 +20,7 @@ void kbled_init(void) {
 }
 
 void kbled_reset(void) {
-    kbled_set_brightness(0);
-    if (gpio_get(&LID_SW_N))
-        kbled_enable(true);
+    kbled_restore();
 }
 
 uint8_t kbled_get(void) {

src/board/system76/common/kbscan.c

@@ -185,16 +185,13 @@ static void hardware_hotkey(uint16_t key) {
         kbled_hotkey_color();
         break;
     case K_KBD_DOWN:
-        if (acpi_ecos != EC_OS_FULL)
-            kbled_hotkey_down();
+        kbled_hotkey_down();
         break;
     case K_KBD_UP:
-        if (acpi_ecos != EC_OS_FULL)
-            kbled_hotkey_up();
+        kbled_hotkey_up();
         break;
     case K_KBD_TOGGLE:
-        if (acpi_ecos != EC_OS_FULL)
-            kbled_hotkey_toggle();
+        kbled_hotkey_toggle();
         break;
     }
 }

src/board/system76/common/lid.c

@@ -26,8 +26,6 @@ void lid_event(void) {
                 pmc_swi();
                 lid_wake = false;
             }
-
-            kbled_enable(true);
         } else {
             DEBUG("closed\n");
 

src/board/system76/common/options.c

@@ -5,44 +5,74 @@
 #include <common/debug.h>
 
 uint8_t __xdata OPTIONS[NUM_OPTIONS];
-uint8_t DEFAULT_OPTIONS[NUM_OPTIONS] = { 0 };
+
+// clang-format off
+uint8_t DEFAULT_OPTIONS[NUM_OPTIONS] = {
+    [OPT_POWER_ON_AC] = 0,
+    [OPT_KBLED_BRIGHTNESS] = 0xFF,
+    [OPT_KBLED_COLOR_I] = 0,
+    [OPT_BAT_THRESHOLD_START] = 95,
+    [OPT_BAT_THRESHOLD_STOP] = 98,
+};
+// clang-format on
 
 // Config is in the second to last sector of flash
 const uint32_t OPTIONS_ADDR = 0x1F800;
 // Signature is the size of the config
 const uint16_t OPTIONS_SIGNATURE = sizeof(OPTIONS);
 
-void options_init(void) {
-    if (!options_load_config()) {
-        options_load_default();
-    }
-}
-
-void options_load_default(void) {
+static void options_load_default() {
     for (uint8_t opt = 0; opt < NUM_OPTIONS; opt++) {
         OPTIONS[opt] = DEFAULT_OPTIONS[opt];
     }
 }
 
-bool options_erase_config(void) {
+static bool options_load_config() {
+    // Check signature
+    if (flash_read_u16(OPTIONS_ADDR) != OPTIONS_SIGNATURE)
+        return false;
+
+    // Read the options if signature is valid
+    flash_read(OPTIONS_ADDR + sizeof(OPTIONS_SIGNATURE), (uint8_t *)OPTIONS, sizeof(OPTIONS));
+    return true;
+}
+
+static bool options_erase_config(void) {
     // This will erase 1024 bytes
     flash_erase(OPTIONS_ADDR);
 
     // Verify signature is erased
     return flash_read_u16(OPTIONS_ADDR) == 0xFFFF;
 }
 
-bool options_load_config(void) {
-    // Check signature
-    if (flash_read_u16(OPTIONS_ADDR) != OPTIONS_SIGNATURE)
-        return false;
+void options_init(void) {
+    if (!options_load_config()) {
+        options_load_default();
+    }
+}
 
-    // Read the options if signature is valid
-    flash_read(OPTIONS_ADDR + sizeof(OPTIONS_SIGNATURE), (uint8_t *)OPTIONS, sizeof(OPTIONS));
-    return true;
+static bool options_changed() {
+    uint8_t current[NUM_OPTIONS];
+    // Check if anything changed
+    if (flash_read_u16(OPTIONS_ADDR) != OPTIONS_SIGNATURE) {
+        return (true);
+    } else {
+        // Read the options if signature is valid
+        flash_read(OPTIONS_ADDR + sizeof(OPTIONS_SIGNATURE), current, sizeof(current));
+        for (uint8_t i = 0; i < NUM_OPTIONS; ++i) {
+            if (current[i] != OPTIONS[i]) {
+                return (true);
+            }
+        }
+    }
+    return false;
 }
 
 bool options_save_config(void) {
+    // Bail if no settings changed to save flash write cycles
+    if (!options_changed())
+        return true;
+
     // Erase config region
     if (!options_erase_config())
         return false;
@@ -68,10 +98,10 @@ uint8_t options_get(uint16_t index) {
 }
 
 bool options_set(uint16_t index, uint8_t value) {
-    if (index < NUM_OPTIONS && OPTIONS[value] != value) {
+    if (index < NUM_OPTIONS) {
         OPTIONS[index] = value;
         TRACE("OPTION %x WRITE %x\n", index, value);
-        return options_save_config();
+        return true;
     } else {
         return false;
     }

src/board/system76/common/power.c

@@ -293,6 +293,9 @@ void power_on(void) {
 void power_off(void) {
     DEBUG("%02X: power_off\n", main_cycle);
 
+    // Commit settings to flash on shutdown
+    options_save_config();
+
 #if HAVE_PCH_PWROK_EC
     // De-assert SYS_PWROK
     GPIO_SET_DEBUG(PCH_PWROK_EC, false);
@@ -411,8 +414,8 @@ static void update_s0ix_state() {
     if (!gpio_get(&SLP_S0_N)) {
         last_sleep_time = time;
     }
-    // Allow for sub-500ms wakeups
-    in_s0ix = (time - last_sleep_time) < 500;
+    // Allow for sub-1s wakeups
+    in_s0ix = (time - last_sleep_time) < 1000;
 }
 #endif
 
@@ -656,13 +659,17 @@ void power_event(void) {
                 gpio_set(&LED_PWR, !gpio_get(&LED_PWR));
                 last_time = time;
             }
+            kbled_enable(false);
             gpio_set(&LED_ACIN, false);
         } else
 #endif
         {
             // CPU on, green light
             gpio_set(&LED_PWR, true);
             gpio_set(&LED_ACIN, false);
+
+            if (gpio_get(&LID_SW_N))
+                kbled_enable(true);
         }
     } else if (power_state == POWER_STATE_S3) {
         // Suspended, flashing green light
@@ -671,6 +678,7 @@ void power_event(void) {
             last_time = time;
         }
         gpio_set(&LED_ACIN, false);
+        kbled_enable(false);
     } else if (!ac_new) {
         // AC plugged in, orange light
         gpio_set(&LED_PWR, false);