led_strip: ws2812: HACK: memory-hungry pulse timing bugfix

At least on nRF52 devices, we are taking too much time between pixels
dealing with overhead inside the SPI driver transceive calls. This is
leading to dropped frames, because the dead time between frames is
long enough (5000ns+) to look like a reset pulse to the LED strip.

Given this SPI driver limitation, it seems this LED driver's design
decision to rely on SPI peripherals as efficient pulse generators
doesn't work well in practice.

The right way to handle this is probably to switch from SPI to
efficient inline assembly which bit-bangs the pulses with interrupts
disabled.

This is what other efficient libraries do to drive this type of
LED (e.g. FastLED uses C++ templates that expand into such
assembly). The Zephyr GPIO API doesn't support doing that in a
portable fashion, unfortunately.

For now, we'll cheat by pre-allocating enough buffer space to send the
entire strip's worth of data.

This is preposterously inefficient (8x memory overhead since there's
one byte to make a SPI frame for each bit of color), but makes the
driver work correctly.

(Note that using timer peripherals as pulse generators, when combined
with DMA for efficiency, would also lead to similar levels of
overhead.)

Signed-off-by: Marti Bolivar <marti@opensourcefoundries.com>

drivers/led_strip/Kconfig.ws2812

@@ -29,6 +29,14 @@ config WS2812_STRIP_NAME
 	help
 		Device name for WS2812 LED strip.
 
+config WS2812_STRIP_MAX_PIXELS
+	int "Maximum number of pixels in a strip"
+	default 12
+	help
+		Set this to the maximum number of pixels you need
+		to control at once. There is an 8x memory penalty associated
+		with each increment of this value, so it's worth optimizing.
+
 config WS2812_STRIP_SPI_DEV_NAME
 	string "SPI master to use to drive the strip"
 	default ""

drivers/led_strip/ws2812.c

@@ -48,9 +48,16 @@ LOG_MODULE_REGISTER(ws2812);
  */
 #define RESET_NFRAMES ((size_t)ceiling_fraction(3 * SPI_FREQ, 4000000) + 1)
 
+#if CONFIG_WS2812_HAS_WHITE_CHANNEL
+#define PX_BUF_PER_PX 32
+#else
+#define PX_BUF_PER_PX 24
+#endif
+
 struct ws2812_data {
 	struct device *spi;
 	struct spi_config config;
+	u8_t px_buf[PX_BUF_PER_PX * CONFIG_WS2812_STRIP_MAX_PIXELS];
 };
 
 /*
@@ -69,16 +76,15 @@ static inline void ws2812_serialize_color(u8_t buf[8], u8_t color)
 /*
  * Convert a pixel into SPI frames, returning the number of bytes used.
  */
-static size_t ws2812_serialize_pixel(u8_t px[32], struct led_rgb *pixel)
+static void ws2812_serialize_pixel(u8_t px[PX_BUF_PER_PX],
+				   struct led_rgb *pixel)
 {
 	ws2812_serialize_color(px + RED_OFFSET, pixel->r);
 	ws2812_serialize_color(px + GRN_OFFSET, pixel->g);
 	ws2812_serialize_color(px + BLU_OFFSET, pixel->b);
 	if (IS_ENABLED(CONFIG_WS2812_HAS_WHITE_CHANNEL)) {
 		ws2812_serialize_color(px + WHT_OFFSET, 0); /* unused */
-		return 32;
 	}
-	return 24;
 }
 
 /*
@@ -106,9 +112,10 @@ static int ws2812_strip_update_rgb(struct device *dev, struct led_rgb *pixels,
 {
 	struct ws2812_data *drv_data = dev->driver_data;
 	struct spi_config *config = &drv_data->config;
-	u8_t px_buf[32]; /* 32 are needed when a white channel is present. */
+	u8_t *px_buf = drv_data->px_buf;
 	struct spi_buf buf = {
 		.buf = px_buf,
+		.len = PX_BUF_PER_PX * num_pixels,
 	};
 	const struct spi_buf_set tx = {
 		.buffers = &buf,
@@ -117,19 +124,18 @@ static int ws2812_strip_update_rgb(struct device *dev, struct led_rgb *pixels,
 	size_t i;
 	int rc;
 
+	if (num_pixels > CONFIG_WS2812_STRIP_MAX_PIXELS) {
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < num_pixels; i++) {
-		buf.len = ws2812_serialize_pixel(px_buf, &pixels[i]);
-		rc = spi_write(drv_data->spi, config, &tx);
-		if (rc) {
-			/*
-			 * Latch anything we've shifted out first, to
-			 * call visual attention to the problematic
-			 * pixel.
-			 */
-			(void)ws2812_reset_strip(drv_data);
-			LOG_ERR("can't set pixel %u: %d", i, rc);
-			return rc;
-		}
+		ws2812_serialize_pixel(&px_buf[PX_BUF_PER_PX * i], &pixels[i]);
+	}
+
+	rc = spi_write(drv_data->spi, config, &tx);
+	if (rc) {
+		(void)ws2812_reset_strip(drv_data);
+		return rc;
 	}
 
 	return ws2812_reset_strip(drv_data);