Adds IS31FL3731 RGB Matrix Implementation

[jackhumbert]

This is a rework of #1857 and @Wilba6582's driver and RGB matrix implementation. Some points of improvement for future PRs (feel free to suggest):

• Storing the driver/RGB config in PROGMEM - I avoided this in hopes that it would speed up the loop, but I don't think it had much affect
• Reorganise the rgblight and rgb matrix integrations
• Allow for a separate EEPROM address
• Allow static effects like solid color to take advantage of the cache and not update every cycle
• Give better examples of the custom indicators

Below are the docs being added as well:

RGB Matrix Lighting

There is basic support for addressable RGB matrix lighting with the I2C IS31FL3731 RGB controller. To enable it, add this to your rules.mk:

RGB_MATRIX_ENABLE = yes

Configure the hardware via your config.h:

// 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:
// 0b1110100 AD <-> GND
// 0b1110111 AD <-> VCC
// 0b1110101 AD <-> SCL
// 0b1110110 AD <-> SDA
#define DRIVER_ADDR_1 0b1110100
#define DRIVER_ADDR_2 0b1110110

#define DRIVER_COUNT 2
#define DRIVER_1_LED_TOTAL 25
#define DRIVER_2_LED_TOTAL 24
#define DRIVER_LED_TOTAL 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:

const is31_led g_is31_leds[DRIVER_LED_TOTAL] = {
/* Refer to IS31 manual for these locations
 *   driver
 *   |  R location
 *   |  |      G location
 *   |  |      |      B location
 *   |  |      |      | */
    {0, C1_3,  C2_3,  C3_3},
    ....
}

Where Cx_y is the location of the LED in the matrix defined by the datasheet. The driver is the index of the driver you defined in your config.h (0 or 1 right now).

const rgb_led g_rgb_leds[DRIVER_LED_TOTAL] = {
/* {row | col << 4}
 *    |           {x=0..224, y=0..64}
 *    |              |               modifier
 *    |              |                 | */
    {{0|(0<<4)},   {20.36*0, 21.33*0}, 1},
    {{0|(1<<4)},   {20.36*1, 21.33*0}, 1},
    ....
}

The format for the matrix position used in this array is {row | (col << 4)}. The x is between (inclusive) 0-224, and y is between (inclusive) 0-64. The easiest way to calculate these positions is:

x = 224 / ( NUMBER_OF_ROWS - 1 ) * ROW_POSITION
y = 64 / (NUMBER_OF_COLS - 1 ) * COL_POSITION

Where all variables are decimels/floats.

modifier is a boolean, whether or not a certain key is considered a modifier (used in some effects).

Keycodes

All RGB keycodes are currently shared with the RGBLIGHT system:

* `RGB_TOG` - toggle
* `RGB_MOD` - cycle through modes
* `RGB_HUI` - increase hue
* `RGB_HUD` - decrease hue
* `RGB_SAI` - increase saturation
* `RGB_SAD` - decrease saturation
* `RGB_VAI` - increase value
* `RGB_VAD` - decrease value


* `RGB_MODE_*` keycodes will generally work, but are not currently mapped to the correct effects for the RGB Matrix system

RGB Matrix Effects

These are the effects that are currently available:

enum rgb_matrix_effects {
    RGB_MATRIX_SOLID_COLOR = 1,
    RGB_MATRIX_SOLID_REACTIVE,
    RGB_MATRIX_ALPHAS_MODS,
    RGB_MATRIX_DUAL_BEACON,
    RGB_MATRIX_GRADIENT_UP_DOWN,
    RGB_MATRIX_RAINDROPS,
    RGB_MATRIX_CYCLE_ALL,
    RGB_MATRIX_CYCLE_LEFT_RIGHT,
    RGB_MATRIX_CYCLE_UP_DOWN,
    RGB_MATRIX_RAINBOW_BEACON,
    RGB_MATRIX_RAINBOW_PINWHEELS,
    RGB_MATRIX_RAINBOW_MOVING_CHEVRON,
    RGB_MATRIX_JELLYBEAN_RAINDROPS,
#ifdef RGB_MATRIX_KEYPRESSES
    RGB_MATRIX_SPLASH,
    RGB_MATRIX_MULTISPLASH,
    RGB_MATRIX_SOLID_SPLASH,
    RGB_MATRIX_SOLID_MULTISPLASH,
#endif
    RGB_MATRIX_EFFECT_MAX
};

Custom layer effects

Custom layer effects can be done by defining this in your <keyboard>.c:

void rgb_matrix_indicators_kb(void) {
	// rgb_matrix_set_color(index, red, green, blue);
}

A similar function works in the keymap as rgb_matrix_indicators_user.

Additional config.h Options

#define RGB_MATRIX_KEYPRESSES // reacts to keypresses (will slow down matrix scan by a lot)
#define RGB_MATRIX_KEYRELEASES // reacts to keyreleases (not recommened)
#define RGB_DISABLE_AFTER_TIMEOUT 0 // number of ticks to wait until disabling effects
#define RGB_DISABLE_WHEN_USB_SUSPENDED false // turn off effects when suspended

EEPROM storage

The EEPROM for it is currently shared with the RGBLIGHT system (it's generally assumed only one RGB would be used at a time), but could be configured to use its own 32bit address with:

#define EECONFIG_RGB_MATRIX (uint32_t *)16

Where 16 is an unused index from eeconfig.h.

Suspended state

To use the suspend feature, add this to your <keyboard>.c:

void suspend_power_down_kb(void)
{
    rgb_matrix_set_suspend_state(true);
}

void suspend_wakeup_init_kb(void)
{
    rgb_matrix_set_suspend_state(false);
}

[algernon]

As far as I see, process_rgb_matrix is called every cycle, right? In that case... this part here may become a bit costy, because map_row_column_to_led iterates over all DRIVER_LED_TOTAL LEDs, for each key pressed. When chording or typing fast, this may have a significant impact on performance.

Would it not be better to have an array that maps rows and columns directly to LED indexes? One could do it with two lookups then: one row,column to LED index, and a LED index to that led[8] thing... That'd be a bit more RAM used, but O(1) lookups.

[wilba]

My original code had a map of row/col to LED indices... this can be a future optimization.

[algernon]

So... this gets called every cycle. rgb_matrix_update_pwm_buffers will call IS31FL3731_update_pwm_buffers and IS31FL3731_update_led_control_registers, both of which are guarded by dirty flags, and only update when things changed.

If we have animations that change something every cycle, updating every cycle will be slow. I'm not familiar with the hardware here, but I imagine updating here isn't cheap either (otherwise the dirty flags would be pointless). In Kaleidoscope, we were facing a similar situation, where the keyboard felt laggy (see keyboardio/Kaleidoscope#113 and keyboardio/Kaleidoscope-LEDControl#1 for details and some numbers), so we changed the LED sync to happen only every 16msec or so (still netting us ~60fps), and the results were amazing.

Perhaps it would be worth considering doing something similar here, too?

[wilba]

My original code would update backlight effect state using a 20Hz timer/interrupt, but then update the LED driver registers in between matrix scans.

[jackhumbert]

I moved it from the timer to the task to avoid conflicts with other timers (1 & 3 are used by audio), and I had played around with the counter that is commented out there a little to lighten the load - I think something like that might work well.

[wilba]

The setup via g_is31_leds is OK... originally I had it as a fixed mapping of index to RGB locations (because they can only go in fixed locations in the matrix anyway) but this at least exposes the actual pin out at the code level a little more obviously.

I think we can switch to the existing I2C library already in QMK but recommend doing this in a future PR. It won't hurt to use TWIlib for now.

The refactoring of my Zeal60/Zeal65 code into quantum/rgb_matrix.c loses some of the functionality and some effects are not very optimal (lots of floating point math in the effects, which I had avoided by pre-calculating and using integer math). There is no one-size-fits-all solution, though... it does integrate with the existing bottom light RGB stuff and uses only one RGB value in EEPROM (thus missing out on the concept of an alpha and mod color for two-tone RGB backlight) so in that respect, it's a very light-weight implementation. I totally understand the reasoning and not objecting... this needs to work independently of hardware (although AFAIK it's still coupled to an Atmega specific I2C driver).

I have been too slow to refactor my implementation into a generic one, as it is highly coupled with the use of EEPROM memory to store the current effect, colors, brightness, user-defined keycodes, etc. so I can't complain too much about this implementation not being "complete" enough for me to refactor my Zeal60/Zeal65/M60-A implementation to use it wholesale.

Can we come up with some compromise? Pull all this into master now, and I will refactor the Zeal60/Zeal65/M60-A implementation to use the low-level ISSI driver, but use its own higher-level RGB backlight effect/setting code. I can't see any other way, because to add more features to this RGB matrix driver would bloat it, especially with specific things that other keyboards wouldn't need. Yet I would also want to push something generalized for multiple keyboards to use, Zeal60, Zeal65, M60-A, the HS60 PCB by @yiancar, etc.

Not sure how that can be done other than adding a 2nd, alternate quantum/rgb_matrix.c

I'm not against the idea of merging and even improving this RGB matrix driver with the same optimizations... willing to share how I pre-calculate polar co-ordinates for radial effects without floating-point math, etc. Possibly even generalizing out the effects and letting them be pulled into either implementation, perhaps even letting the keyboard implementation decide which effects it wants in its list.... But I think it's better to just commit this now, and let me refactor and commit an alternative one, and work towards refactoring/generalizing them while side-by-side, than spend more time trying to merge them together before committing anything.

[yiancar]

I had a look over this and I think I generally agree with @Wilba6582.
The code seems good enough to be generalized. I see no reason for why an array of pre-calculated math cannot be used for some of the effects.

I personally am a big supported of storing rgb settings in EEPROM in order to be able to be updated over hid (rawhid etc).

I will implement the above commits on HS60 tonight and see how it goes. For my setup I had to use a non blocking version of TWIlib. This is mostly related to my I2C sometimes dropping a packet which hangs the system. For an application like this, missing a frame every 10 minutes is not a big deal and should not hang the system.

Will come back when I have more results to offer! Thank you for this :)

[jackhumbert]

@Wilba6582 that sounds good - its own file alongside rgb_matrix.c would be fine, but I think a vendor/keyboard-specific one in the keyboard folder would work well too. I'd like to have the rgb_matrix extendable in a way (with a separate rgb_matrix_effects enum and the rgb_matrix_indicators_kb/user function) so each keyboard can capitalise on the full effect system without needing to add keyboard-specific effects to the core code. I'd be happy to make any changes to it that would make that easier.

The polar cors optimisations are very welcome :) thanks for the feedback everybody! I'll merge this now.

[yiancar]

I am porting HS60 now to this to see how its going. I would personally suggest adding a warning for the polar effect as well due to high computational power

[jackhumbert]

@yiancar I didn't notice any major lag with that effect, but I'll add a note. I'm merging in the changes from master right now if you wanna wait a little bit.

[yiancar]

@jackhumbert oke making my config ready as we speak:)

[jackhumbert]

@yiancar should be good now :) merging!

Failures are unrelated to changes

[drashna]

What about the Teensy ++ 2.0?

[jackhumbert]

That case is already handled here :)

[drashna]

Maybe, but the bootloader size is 1024, not 512 bytes. :)

[jackhumbert]

Yeah, that code handles the address without that variable - mainly just defined there to avoid errors I think :)

[drashna]

Ah, okay.

Because I have seen issues with this in the past. ..... not that I can find them now. So clearly, it was in gitter

[yiancar]

@jackhumbert Ok so I have to report the following:

• Mapping of leds works pretty smoothly
• Effects seem to be sensible (even tho I can't see how to change the speed of the effect I think this can be easily added).
• Update rate seems to be jerky (to an unacceptable level I think) sometimes ic 2 updates 1-2 seconds after ic1. Leds seem to flicker while playing a relatively fast effect.

I think that the most universal way to solve the update rate jerkiness is using a timer.