[Core] Clean up ISSI drivers, Add IS31FL3736 support (#20572)

Co-authored-by: Pablo Martínez <58857054+elpekenin@users.noreply.github.com>

builddefs/common_features.mk

@@ -419,7 +419,7 @@ endif
 
 RGB_MATRIX_ENABLE ?= no
 
-VALID_RGB_MATRIX_TYPES := AW20216 IS31FL3731 IS31FL3733 IS31FL3737 IS31FL3741 IS31FL3742A IS31FL3743A IS31FL3745 IS31FL3746A CKLED2001 WS2812 custom
+VALID_RGB_MATRIX_TYPES := AW20216 IS31FL3731 IS31FL3733 IS31FL3736 IS31FL3737 IS31FL3741 IS31FL3742A IS31FL3743A IS31FL3745 IS31FL3746A CKLED2001 WS2812 custom
 ifeq ($(strip $(RGB_MATRIX_ENABLE)), yes)
     ifeq ($(filter $(RGB_MATRIX_DRIVER),$(VALID_RGB_MATRIX_TYPES)),)
         $(call CATASTROPHIC_ERROR,Invalid RGB_MATRIX_DRIVER,RGB_MATRIX_DRIVER="$(RGB_MATRIX_DRIVER)" is not a valid matrix type)
@@ -460,6 +460,13 @@ endif
         QUANTUM_LIB_SRC += i2c_master.c
     endif
 
+    ifeq ($(strip $(RGB_MATRIX_DRIVER)), IS31FL3736)
+        OPT_DEFS += -DIS31FL3736 -DSTM32_I2C -DHAL_USE_I2C=TRUE
+        COMMON_VPATH += $(DRIVER_PATH)/led/issi
+        SRC += is31fl3736.c
+        QUANTUM_LIB_SRC += i2c_master.c
+    endif
+
     ifeq ($(strip $(RGB_MATRIX_DRIVER)), IS31FL3737)
         OPT_DEFS += -DIS31FL3737 -DSTM32_I2C -DHAL_USE_I2C=TRUE
         COMMON_VPATH += $(DRIVER_PATH)/led/issi

docs/feature_rgb_matrix.md

@@ -156,6 +156,82 @@ const is31_led PROGMEM g_is31_leds[RGB_MATRIX_LED_COUNT] = {
 Where `X_Y` is the location of the LED in the matrix defined by [the datasheet](https://www.issi.com/WW/pdf/31FL3733.pdf) and the header file `drivers/led/issi/is31fl3733.h`. The `driver` is the index of the driver you defined in your `config.h` (`0`, `1`, `2`, or `3` for now).
 
 ---
+### IS31FL3736 :id=is31fl3736
+
+There is basic support for addressable RGB matrix lighting with the I2C IS31FL3737 RGB controller. To enable it, add this to your `rules.mk`:
+
+```make
+RGB_MATRIX_ENABLE = yes
+RGB_MATRIX_DRIVER = IS31FL3736
+```
+You can use between 1 and 4 IS31FL3736 IC's. Do not specify `DRIVER_ADDR_<N>` defines for IC's that are not present on your keyboard.
+
+Configure the hardware via your `config.h`:
+
+| Variable | Description | Default |
+|----------|-------------|---------|
+| `ISSI_TIMEOUT` | (Optional) How long to wait for i2c messages, in milliseconds | 100 |
+| `ISSI_PERSISTENCE` | (Optional) Retry failed messages this many times | 0 |
+| `ISSI_PWM_FREQUENCY` | (Optional) PWM Frequency Setting - IS31FL3736B only | 0 |
+| `ISSI_GLOBALCURRENT` | (Optional) Configuration for the Global Current Register | 0xFF |
+| `ISSI_SWPULLUP` | (Optional) Set the value of the SWx lines on-chip de-ghosting resistors | PUR_0R (Disabled) |
+| `ISSI_CSPULLUP` | (Optional) Set the value of the CSx lines on-chip de-ghosting resistors | PUR_0R (Disabled) |
+| `DRIVER_COUNT` | (Required) How many RGB driver IC's are present | |
+| `RGB_MATRIX_LED_COUNT` | (Required) How many RGB lights are present across all drivers | |
+| `DRIVER_ADDR_1` | (Required) Address for the first RGB driver | |
+| `DRIVER_ADDR_2` | (Optional) Address for the second RGB driver | |
+| `DRIVER_ADDR_3` | (Optional) Address for the third RGB driver | |
+| `DRIVER_ADDR_4` | (Optional) Address for the fourth RGB driver | |
+
+The IS31FL3736 IC's have on-chip resistors that can be enabled to allow for de-ghosting of the RGB matrix. By default these resistors are not enabled (`ISSI_SWPULLUP`/`ISSI_CSPULLUP` are given the value of`PUR_0R`), the values that can be set to enable de-ghosting are as follows:
+
+| `ISSI_SWPULLUP/ISSI_CSPULLUP` | Description |
+|----------------------|-------------|
+| `PUR_0R` | (default) Do not use the on-chip resistors/enable de-ghosting |
+| `PUR_05KR` | The 0.5k Ohm resistor used during blanking period (t_NOL) |
+| `PUR_1KR` | The 1k Ohm resistor used during blanking period (t_NOL) |
+| `PUR_2KR` | The 2k Ohm resistor used during blanking period (t_NOL) |
+| `PUR_4KR` | The 4k Ohm resistor used during blanking period (t_NOL) |
+| `PUR_8KR` | The 8k Ohm resistor during blanking period (t_NOL) |
+| `PUR_16KR` | The 16k Ohm resistor during blanking period (t_NOL) |
+| `PUR_32KR` | The 32k Ohm resistor used during blanking period (t_NOL) |
+
+Here is an example using 2 drivers.
+
+```c
+// This is a 7-bit address, that gets left-shifted and bit 0
+// set to 0 for write, 1 for read (as per I2C protocol)
+// The address will vary depending on your wiring:
+// 0000 <-> GND
+// 0101 <-> SCL
+// 1010 <-> SDA
+// 1111 <-> VCC
+// ADDR represents A3:A0 of the 7-bit address.
+// The result is: 0b101(ADDR)
+#define DRIVER_ADDR_1 0b1010000
+#define DRIVER_ADDR_2 0b1010001
+
+#define DRIVER_COUNT 2
+#define DRIVER_1_LED_TOTAL 30
+#define DRIVER_2_LED_TOTAL 36
+#define RGB_MATRIX_LED_COUNT (DRIVER_1_LED_TOTAL + DRIVER_2_LED_TOTAL)
+```
+!> Note the parentheses, this is so when `RGB_MATRIX_LED_COUNT` is used in code and expanded, the values are added together before any additional math is applied to them. As an example, `rand() % (DRIVER_1_LED_TOTAL + DRIVER_2_LED_TOTAL)` will give very different results than `rand() % DRIVER_1_LED_TOTAL + DRIVER_2_LED_TOTAL`.
+
+Define these arrays listing all the LEDs in your `<keyboard>.c`:
+
+```c
+const is31_led PROGMEM g_is31_leds[RGB_MATRIX_LED_COUNT] = {
+/* Refer to IS31 manual for these locations
+ *   driver
+ *   |  R location
+ *   |  |       G location
+ *   |  |       |       B location
+ *   |  |       |       | */
+    {0, B_1,    A_1,    C_1},
+    ....
+}
+```
 ### IS31FL3737 :id=is31fl3737
 
 There is basic support for addressable RGB matrix lighting with the I2C IS31FL3737 RGB controller. To enable it, add this to your `rules.mk`:
@@ -218,8 +294,6 @@ Here is an example using 2 drivers.
 ```
 !> Note the parentheses, this is so when `RGB_MATRIX_LED_COUNT` is used in code and expanded, the values are added together before any additional math is applied to them. As an example, `rand() % (DRIVER_1_LED_TOTAL + DRIVER_2_LED_TOTAL)` will give very different results than `rand() % DRIVER_1_LED_TOTAL + DRIVER_2_LED_TOTAL`.
 
-Currently only 2 drivers are supported, but it would be trivial to support all 4 combinations.
-
 Define these arrays listing all the LEDs in your `<keyboard>.c`:
 
 ```c

drivers/led/issi/is31fl3218.c

@@ -45,9 +45,7 @@ void IS31FL3218_write_register(uint8_t reg, uint8_t data) {
 
 void IS31FL3218_write_pwm_buffer(uint8_t *pwm_buffer) {
     g_twi_transfer_buffer[0] = ISSI_REG_PWM;
-    for (int i = 0; i < 18; i++) {
+    memcpy(g_twi_transfer_buffer + 1, pwm_buffer, 18);
-        g_twi_transfer_buffer[1 + i] = pwm_buffer[i];
-    }
 
     i2c_transmit(ISSI_ADDRESS, g_twi_transfer_buffer, 19, ISSI_TIMEOUT);
 }

drivers/led/issi/is31fl3731-simple.c

@@ -123,9 +123,7 @@ void IS31FL3731_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
         // copy the data from i to i+15
         // device will auto-increment register for data after the first byte
         // thus this sets registers 0x24-0x33, 0x34-0x43, etc. in one transfer
-        for (int j = 0; j < 16; j++) {
+        memcpy(g_twi_transfer_buffer + 1, pwm_buffer + i, 16);
-            g_twi_transfer_buffer[1 + j] = pwm_buffer[i + j];
-        }
 
 #if ISSI_PERSISTENCE > 0
         for (uint8_t i = 0; i < ISSI_PERSISTENCE; i++) {

drivers/led/issi/is31fl3731.c

@@ -111,9 +111,7 @@ void IS31FL3731_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
         // copy the data from i to i+15
         // device will auto-increment register for data after the first byte
         // thus this sets registers 0x24-0x33, 0x34-0x43, etc. in one transfer
-        for (int j = 0; j < 16; j++) {
+        memcpy(g_twi_transfer_buffer + 1, pwm_buffer + i, 16);
-            g_twi_transfer_buffer[1 + j] = pwm_buffer[i + j];
-        }
 
 #if ISSI_PERSISTENCE > 0
         for (uint8_t i = 0; i < ISSI_PERSISTENCE; i++) {

drivers/led/issi/is31fl3733-simple.c

@@ -129,9 +129,7 @@ bool IS31FL3733_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
         // Copy the data from i to i+15.
         // Device will auto-increment register for data after the first byte
         // Thus this sets registers 0x00-0x0F, 0x10-0x1F, etc. in one transfer.
-        for (int j = 0; j < 16; j++) {
+        memcpy(g_twi_transfer_buffer + 1, pwm_buffer + i, 16);
-            g_twi_transfer_buffer[1 + j] = pwm_buffer[i + j];
-        }
 
 #if ISSI_PERSISTENCE > 0
         for (uint8_t i = 0; i < ISSI_PERSISTENCE; i++) {

drivers/led/issi/is31fl3736.c

@@ -107,9 +107,7 @@ void IS31FL3736_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
         // copy the data from i to i+15
         // device will auto-increment register for data after the first byte
         // thus this sets registers 0x00-0x0F, 0x10-0x1F, etc. in one transfer
-        for (int j = 0; j < 16; j++) {
+        memcpy(g_twi_transfer_buffer + 1, pwm_buffer + i, 16);
-            g_twi_transfer_buffer[1 + j] = pwm_buffer[i + j];
-        }
 
 #if ISSI_PERSISTENCE > 0
         for (uint8_t i = 0; i < ISSI_PERSISTENCE; i++) {
@@ -262,16 +260,15 @@ void IS31FL3736_mono_set_led_control_register(uint8_t index, bool enabled) {
     g_led_control_registers_update_required = true;
 }
 
-void IS31FL3736_update_pwm_buffers(uint8_t addr1, uint8_t addr2) {
+void IS31FL3736_update_pwm_buffers(uint8_t addr, uint8_t index) {
-    if (g_pwm_buffer_update_required) {
+    if (g_pwm_buffer_update_required[index]) {
         // Firstly we need to unlock the command register and select PG1
-        IS31FL3736_write_register(addr1, ISSI_COMMANDREGISTER_WRITELOCK, 0xC5);
+        IS31FL3736_write_register(addr, ISSI_COMMANDREGISTER_WRITELOCK, 0xC5);
-        IS31FL3736_write_register(addr1, ISSI_COMMANDREGISTER, ISSI_PAGE_PWM);
+        IS31FL3736_write_register(addr, ISSI_COMMANDREGISTER, ISSI_PAGE_PWM);
 
-        IS31FL3736_write_pwm_buffer(addr1, g_pwm_buffer[0]);
+        IS31FL3736_write_pwm_buffer(addr, g_pwm_buffer[index]);
-        // IS31FL3736_write_pwm_buffer(addr2, g_pwm_buffer[1]);
     }
-    g_pwm_buffer_update_required = false;
+    g_pwm_buffer_update_required[index] = false;
 }
 
 void IS31FL3736_update_led_control_registers(uint8_t addr1, uint8_t addr2) {

drivers/led/issi/is31fl3736.h

@@ -58,8 +58,8 @@ void IS31FL3736_mono_set_led_control_register(uint8_t index, bool enabled);
 // (eg. from a timer interrupt).
 // Call this while idle (in between matrix scans).
 // If the buffer is dirty, it will update the driver with the buffer.
-void IS31FL3736_update_pwm_buffers(uint8_t addr1, uint8_t addr2);
+void IS31FL3736_update_pwm_buffers(uint8_t addr, uint8_t index);
-void IS31FL3736_update_led_control_registers(uint8_t addr1, uint8_t addr2);
+void IS31FL3736_update_led_control_registers(uint8_t addr, uint8_t index);
 
 #define PUR_0R 0x00   // No PUR resistor
 #define PUR_05KR 0x01 // 0.5k Ohm resistor

drivers/led/issi/is31fl3737.c

@@ -114,9 +114,7 @@ void IS31FL3737_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
         // copy the data from i to i+15
         // device will auto-increment register for data after the first byte
         // thus this sets registers 0x00-0x0F, 0x10-0x1F, etc. in one transfer
-        for (int j = 0; j < 16; j++) {
+        memcpy(g_twi_transfer_buffer + 1, pwm_buffer + i, 16);
-            g_twi_transfer_buffer[1 + j] = pwm_buffer[i + j];
-        }
 
 #if ISSI_PERSISTENCE > 0
         for (uint8_t i = 0; i < ISSI_PERSISTENCE; i++) {

drivers/led/issi/is31fl3737.h

@@ -45,8 +45,8 @@ void IS31FL3737_set_led_control_register(uint8_t index, bool red, bool green, bo
 // (eg. from a timer interrupt).
 // Call this while idle (in between matrix scans).
 // If the buffer is dirty, it will update the driver with the buffer.
-void IS31FL3737_update_pwm_buffers(uint8_t addr1, uint8_t addr2);
+void IS31FL3737_update_pwm_buffers(uint8_t addr, uint8_t index);
-void IS31FL3737_update_led_control_registers(uint8_t addr1, uint8_t addr2);
+void IS31FL3737_update_led_control_registers(uint8_t addr, uint8_t index);
 
 #define PUR_0R 0x00   // No PUR resistor
 #define PUR_05KR 0x01 // 0.5k Ohm resistor in t_NOL

drivers/led/issi/is31fl3741.c

@@ -104,9 +104,7 @@ void IS31FL3741_write_register(uint8_t addr, uint8_t reg, uint8_t data) {
 }
 
 bool IS31FL3741_write_pwm_buffer(uint8_t addr, uint8_t *pwm_buffer) {
-    // unlock the command register and select PG2
+    // Assume PG1 is already selected
-    IS31FL3741_write_register(addr, ISSI_COMMANDREGISTER_WRITELOCK, 0xC5);
-    IS31FL3741_write_register(addr, ISSI_COMMANDREGISTER, ISSI_PAGE_PWM0);
 
     for (int i = 0; i < 342; i += 18) {
         if (i == 180) {
@@ -222,6 +220,10 @@ void IS31FL3741_set_led_control_register(uint8_t index, bool red, bool green, bo
 
 void IS31FL3741_update_pwm_buffers(uint8_t addr, uint8_t index) {
     if (g_pwm_buffer_update_required[index]) {
+        // unlock the command register and select PG2
+        IS31FL3741_write_register(addr, ISSI_COMMANDREGISTER_WRITELOCK, 0xC5);
+        IS31FL3741_write_register(addr, ISSI_COMMANDREGISTER, ISSI_PAGE_PWM0);
+
         IS31FL3741_write_pwm_buffer(addr, g_pwm_buffer[index]);
     }
 

quantum/rgb_matrix/rgb_matrix.h

@@ -28,6 +28,8 @@
 #    include "is31fl3731.h"
 #elif defined(IS31FL3733)
 #    include "is31fl3733.h"
+#elif defined(IS31FL3736)
+#    include "is31fl3736.h"
 #elif defined(IS31FL3737)
 #    include "is31fl3737.h"
 #elif defined(IS31FL3741)

quantum/rgb_matrix/rgb_matrix_drivers.c

@@ -23,7 +23,7 @@
  * be here if shared between boards.
  */
 
-#if defined(IS31FL3731) || defined(IS31FL3733) || defined(IS31FL3737) || defined(IS31FL3741) || defined(IS31FLCOMMON) || defined(CKLED2001)
+#if defined(IS31FL3731) || defined(IS31FL3733) || defined(IS31FL3736) || defined(IS31FL3737) || defined(IS31FL3741) || defined(IS31FLCOMMON) || defined(CKLED2001)
 #    include "i2c_master.h"
 
 // TODO: Remove this at some later date
@@ -72,6 +72,18 @@ static void init(void) {
 #            endif
 #        endif
 
+#    elif defined(IS31FL3736)
+    IS31FL3736_init(DRIVER_ADDR_1);
+#        if defined(DRIVER_ADDR_2)
+    IS31FL3736_init(DRIVER_ADDR_2);
+#            if defined(DRIVER_ADDR_3)
+    IS31FL3736_init(DRIVER_ADDR_3);
+#                if defined(DRIVER_ADDR_4)
+    IS31FL3736_init(DRIVER_ADDR_4);
+#                endif
+#            endif
+#        endif
+
 #    elif defined(IS31FL3737)
     IS31FL3737_init(DRIVER_ADDR_1);
 #        if defined(DRIVER_ADDR_2)
@@ -120,6 +132,8 @@ static void init(void) {
         IS31FL3731_set_led_control_register(index, enabled, enabled, enabled);
 #    elif defined(IS31FL3733)
         IS31FL3733_set_led_control_register(index, enabled, enabled, enabled);
+#    elif defined(IS31FL3736)
+        IS31FL3736_set_led_control_register(index, enabled, enabled, enabled);
 #    elif defined(IS31FL3737)
         IS31FL3737_set_led_control_register(index, enabled, enabled, enabled);
 #    elif defined(IS31FL3741)
@@ -156,6 +170,18 @@ static void init(void) {
 #            endif
 #        endif
 
+#    elif defined(IS31FL3736)
+    IS31FL3736_update_led_control_registers(DRIVER_ADDR_1, 0);
+#        if defined(DRIVER_ADDR_2)
+    IS31FL3736_update_led_control_registers(DRIVER_ADDR_2, 1);
+#            if defined(DRIVER_ADDR_3)
+    IS31FL3736_update_led_control_registers(DRIVER_ADDR_3, 2);
+#                if defined(DRIVER_ADDR_4)
+    IS31FL3736_update_led_control_registers(DRIVER_ADDR_4, 3);
+#                endif
+#            endif
+#        endif
+
 #    elif defined(IS31FL3737)
     IS31FL3737_update_led_control_registers(DRIVER_ADDR_1, 0);
 #        if defined(DRIVER_ADDR_2)
@@ -242,6 +268,27 @@ const rgb_matrix_driver_t rgb_matrix_driver = {
     .set_color_all = IS31FL3733_set_color_all,
 };
 
+#    elif defined(IS31FL3736)
+static void flush(void) {
+    IS31FL3736_update_pwm_buffers(DRIVER_ADDR_1, 0);
+#        if defined(DRIVER_ADDR_2)
+    IS31FL3736_update_pwm_buffers(DRIVER_ADDR_2, 1);
+#            if defined(DRIVER_ADDR_3)
+    IS31FL3736_update_pwm_buffers(DRIVER_ADDR_3, 2);
+#                if defined(DRIVER_ADDR_4)
+    IS31FL3736_update_pwm_buffers(DRIVER_ADDR_4, 3);
+#                endif
+#            endif
+#        endif
+}
+
+const rgb_matrix_driver_t rgb_matrix_driver = {
+    .init = init,
+    .flush = flush,
+    .set_color = IS31FL3736_set_color,
+    .set_color_all = IS31FL3736_set_color_all,
+};
+
 #    elif defined(IS31FL3737)
 static void flush(void) {
     IS31FL3737_update_pwm_buffers(DRIVER_ADDR_1, 0);