Merge branch 'master' into master

11 files changed, 131 insertions, 62 deletions

diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c
index 2b2c70ab6..f8dc24c15 100644
--- a/shared-module/displayio/Bitmap.c
+++ b/shared-module/displayio/Bitmap.c
@@ -33,20 +33,21 @@
 void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t width,
     uint32_t height, uint32_t bits_per_value) {
     uint32_t row_width = width * bits_per_value;
-    // word align
-    if (row_width % 32 != 0) {
-        self->stride = (row_width / 32 + 1);
+    // align to size_t
+    uint8_t align_bits = 8 * sizeof(size_t);
+    if (row_width % align_bits != 0) {
+        self->stride = (row_width / align_bits + 1);
     } else {
-        self->stride = row_width / 32;
+        self->stride = row_width / align_bits;
     }
     self->width = width;
     self->height = height;
-    self->data = m_malloc(self->stride * height * sizeof(uint32_t), false);
+    self->data = m_malloc(self->stride * height * sizeof(size_t), false);
     self->read_only = false;
     self->bits_per_value = bits_per_value;
 
-    if (bits_per_value > 8) {
-        mp_raise_NotImplementedError(translate("Only bit maps of 8 bit color or less are supported"));
+    if (bits_per_value > 8 && bits_per_value != 16 && bits_per_value != 32) {
+        mp_raise_NotImplementedError(translate("Invalid bits per value"));
     }
 
     // Division and modulus can be slow because it has to handle any integer. We know bits_per_value
@@ -56,7 +57,7 @@ void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t wi
     self->x_shift = 0; // Used to divide the index by the number of pixels per word. Its used in a
                        // shift which effectively divides by 2 ** x_shift.
     uint32_t power_of_two = 1;
-    while (power_of_two < 32 / bits_per_value ) {
+    while (power_of_two < align_bits / bits_per_value ) {
         self->x_shift++;
         power_of_two <<= 1;
     }
@@ -76,41 +77,27 @@ uint32_t common_hal_displayio_bitmap_get_bits_per_value(displayio_bitmap_t *self
     return self->bits_per_value;
 }
 
-void common_hal_displayio_bitmap_load_row(displayio_bitmap_t *self, uint16_t y, uint8_t* data, uint16_t len) {
-    if (len != self->stride * sizeof(uint32_t)) {
-        mp_raise_ValueError(translate("row must be packed and word aligned"));
-    }
-    uint32_t* row_value = self->data + (y * self->stride);
-    // Do the memcpy ourselves since we may want to flip endianness.
-    for (uint32_t i = 0; i < self->stride; i++) {
-        #pragma GCC diagnostic push
-        #pragma GCC diagnostic ignored "-Wcast-align"
-        uint32_t value = ((uint32_t *)data)[i];
-        #pragma GCC diagnostic pop
-        if (self->bits_per_value < 16) {
-            value = ((value >> 24) & 0xff) |
-                    ((value << 8) & 0xff0000) |
-                    ((value >> 8) & 0xff00) |
-                    ((value << 24) & 0xff000000);
-        }
-        *row_value = value;
-        row_value++;
-    }
-}
-
 uint32_t common_hal_displayio_bitmap_get_pixel(displayio_bitmap_t *self, int16_t x, int16_t y) {
     if (x >= self->width || x < 0 || y >= self->height || y < 0) {
         return 0;
     }
     int32_t row_start = y * self->stride;
-    if (self->bits_per_value < 8) {
-        uint32_t word = self->data[row_start + (x >> self->x_shift)];
+    uint32_t bytes_per_value = self->bits_per_value / 8;
+    if (bytes_per_value < 1) {
+        size_t word = self->data[row_start + (x >> self->x_shift)];
 
-        return (word >> (32 - ((x & self->x_mask) + 1) * self->bits_per_value)) & self->bitmask;
+        return (word >> (sizeof(size_t) * 8 - ((x & self->x_mask) + 1) * self->bits_per_value)) & self->bitmask;
     } else {
-        uint32_t bytes_per_value = self->bits_per_value / 8;
-        return self->data[row_start + x * bytes_per_value];
+        size_t* row = self->data + row_start;
+        if (bytes_per_value == 1) {
+            return ((uint8_t*) row)[x];
+        } else if (bytes_per_value == 2) {
+            return ((uint16_t*) row)[x];
+        } else if (bytes_per_value == 4) {
+            return ((uint32_t*) row)[x];
+        }
     }
+    return 0;
 }
 
 void common_hal_displayio_bitmap_set_pixel(displayio_bitmap_t *self, int16_t x, int16_t y, uint32_t value) {
@@ -118,15 +105,22 @@ void common_hal_displayio_bitmap_set_pixel(displayio_bitmap_t *self, int16_t x,
         mp_raise_RuntimeError(translate("Read-only object"));
     }
     int32_t row_start = y * self->stride;
-    if (self->bits_per_value < 8) {
-        uint32_t bit_position = (32 - ((x & self->x_mask) + 1) * self->bits_per_value);
+    uint32_t bytes_per_value = self->bits_per_value / 8;
+    if (bytes_per_value < 1) {
+        uint32_t bit_position = (sizeof(size_t) * 8 - ((x & self->x_mask) + 1) * self->bits_per_value);
         uint32_t index = row_start + (x >> self->x_shift);
         uint32_t word = self->data[index];
         word &= ~(self->bitmask << bit_position);
         word |= (value & self->bitmask) << bit_position;
         self->data[index] = word;
     } else {
-        uint32_t bytes_per_value = self->bits_per_value / 8;
-        self->data[row_start + x * bytes_per_value] = value;
+        size_t* row = self->data + row_start;
+        if (bytes_per_value == 1) {
+            ((uint8_t*) row)[x] = value;
+        } else if (bytes_per_value == 2) {
+            ((uint16_t*) row)[x] = value;
+        } else if (bytes_per_value == 4) {
+            ((uint32_t*) row)[x] = value;
+        }
     }
 }
diff --git a/shared-module/displayio/Bitmap.h b/shared-module/displayio/Bitmap.h
index 485b57daf..48ca9e2cf 100644
--- a/shared-module/displayio/Bitmap.h
+++ b/shared-module/displayio/Bitmap.h
@@ -36,11 +36,11 @@ typedef struct {
     mp_obj_base_t base;
     uint16_t width;
     uint16_t height;
-    uint32_t* data;
-    uint16_t stride; // words
+    size_t* data;
+    uint16_t stride; // size_t's
     uint8_t bits_per_value;
     uint8_t x_shift;
-    uint8_t x_mask;
+    size_t x_mask;
     uint16_t bitmask;
     bool read_only;
 } displayio_bitmap_t;
diff --git a/shared-module/displayio/ColorConverter.c b/shared-module/displayio/ColorConverter.c
index d20b24c01..3928e115a 100644
--- a/shared-module/displayio/ColorConverter.c
+++ b/shared-module/displayio/ColorConverter.c
@@ -39,3 +39,11 @@ bool common_hal_displayio_colorconverter_convert(displayio_colorconverter_t *sel
     *output_color = __builtin_bswap16(packed);
     return true;
 }
+
+// Currently no refresh logic is needed for a ColorConverter.
+bool displayio_colorconverter_needs_refresh(displayio_colorconverter_t *self) {
+    return false;
+}
+
+void displayio_colorconverter_finish_refresh(displayio_colorconverter_t *self) {
+}
diff --git a/shared-module/displayio/ColorConverter.h b/shared-module/displayio/ColorConverter.h
index 62ef2eaab..7f3c1a0a0 100644
--- a/shared-module/displayio/ColorConverter.h
+++ b/shared-module/displayio/ColorConverter.h
@@ -36,4 +36,7 @@ typedef struct {
     mp_obj_base_t base;
 } displayio_colorconverter_t;
 
+bool displayio_colorconverter_needs_refresh(displayio_colorconverter_t *self);
+void displayio_colorconverter_finish_refresh(displayio_colorconverter_t *self);
+
 #endif // MICROPY_INCLUDED_SHARED_MODULE_DISPLAYIO_COLORCONVERTER_H
diff --git a/shared-module/displayio/Display.c b/shared-module/displayio/Display.c
index 0e76a868f..28df062f8 100644
--- a/shared-module/displayio/Display.c
+++ b/shared-module/displayio/Display.c
@@ -44,7 +44,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
         mp_obj_t bus, uint16_t width, uint16_t height, int16_t colstart, int16_t rowstart, uint16_t rotation,
         uint16_t color_depth, uint8_t set_column_command, uint8_t set_row_command,
         uint8_t write_ram_command, uint8_t set_vertical_scroll, uint8_t* init_sequence, uint16_t init_sequence_len,
-        const mcu_pin_obj_t* backlight_pin) {
+        const mcu_pin_obj_t* backlight_pin, bool single_byte_bounds, bool data_as_commands) {
     self->color_depth = color_depth;
     self->set_column_command = set_column_command;
     self->set_row_command = set_row_command;
@@ -54,6 +54,8 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
     self->colstart = colstart;
     self->rowstart = rowstart;
     self->auto_brightness = false;
+    self->data_as_commands = data_as_commands;
+    self->single_byte_bounds = single_byte_bounds;
 
     if (MP_OBJ_IS_TYPE(bus, &displayio_parallelbus_type)) {
         self->begin_transaction = common_hal_displayio_parallelbus_begin_transaction;
@@ -81,7 +83,14 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
         data_size &= ~DELAY;
         uint8_t *data = cmd + 2;
         self->send(self->bus, true, cmd, 1);
-        self->send(self->bus, false, data, data_size);
+        if (self->data_as_commands) {
+            // Loop through each parameter to force a CS toggle
+            for (uint32_t j=0; j < data_size; j++) {
+                self->send(self->bus, true, data + j, 1);
+            }
+        } else {
+            self->send(self->bus, false, data, data_size);
+        }
         uint16_t delay_length_ms = 10;
         if (delay) {
             data_size++;
@@ -211,17 +220,35 @@ void displayio_display_end_transaction(displayio_display_obj_t* self) {
 }
 
 void displayio_display_set_region_to_update(displayio_display_obj_t* self, uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) {
-    // TODO(tannewt): Handle displays with single byte bounds.
-    uint16_t data[2];
+
     self->send(self->bus, true, &self->set_column_command, 1);
-    data[0] = __builtin_bswap16(x0 + self->colstart);
-    data[1] = __builtin_bswap16(x1 - 1 + self->colstart);
-    self->send(self->bus, false, (uint8_t*) data, 4);
+    bool isCommand = self->data_as_commands;
+    if (self->single_byte_bounds) {
+        uint8_t data[2];
+        data[0] = x0 + self->colstart;
+        data[1] = x1 - 1 + self->colstart;
+        self->send(self->bus, isCommand, (uint8_t*) data, 2);
+    } else {
+        uint16_t data[2];
+        data[0] = __builtin_bswap16(x0 + self->colstart);
+        data[1] = __builtin_bswap16(x1 - 1 + self->colstart);
+        self->send(self->bus, isCommand, (uint8_t*) data, 4);
+    }
     self->send(self->bus, true, &self->set_row_command, 1);
-    data[0] = __builtin_bswap16(y0  + self->rowstart);
-    data[1] = __builtin_bswap16(y1 - 1 + self->rowstart);
-    self->send(self->bus, false, (uint8_t*) data, 4);
-    self->send(self->bus, true, &self->write_ram_command, 1);
+    if (self->single_byte_bounds) {
+        uint8_t data[2];
+        data[0] = y0 + self->rowstart;
+        data[1] = y1 - 1 + self->rowstart;
+        self->send(self->bus, isCommand, (uint8_t*) data, 2);
+    } else {
+        uint16_t data[2];
+        data[0] = __builtin_bswap16(y0  + self->rowstart);
+        data[1] = __builtin_bswap16(y1 - 1 + self->rowstart);
+        self->send(self->bus, isCommand, (uint8_t*) data, 4);
+    }
+    if (!self->data_as_commands) {
+        self->send(self->bus, true, &self->write_ram_command, 1);
+    }
 }
 
 bool displayio_display_frame_queued(displayio_display_obj_t* self) {
diff --git a/shared-module/displayio/Display.h b/shared-module/displayio/Display.h
index 04da68b63..52f98a252 100644
--- a/shared-module/displayio/Display.h
+++ b/shared-module/displayio/Display.h
@@ -49,6 +49,8 @@ typedef struct {
     uint64_t last_refresh;
     int16_t colstart;
     int16_t rowstart;
+    bool single_byte_bounds;
+    bool data_as_commands;
     display_bus_begin_transaction begin_transaction;
     display_bus_send send;
     display_bus_end_transaction end_transaction;
diff --git a/shared-module/displayio/FourWire.c b/shared-module/displayio/FourWire.c
index 9da5c0701..b01456bba 100644
--- a/shared-module/displayio/FourWire.c
+++ b/shared-module/displayio/FourWire.c
@@ -28,8 +28,10 @@
 
 #include <stdint.h>
 
+#include "py/gc.h"
 #include "shared-bindings/busio/SPI.h"
 #include "shared-bindings/digitalio/DigitalInOut.h"
+#include "shared-bindings/time/__init__.h"
 
 #include "tick.h"
 
@@ -39,6 +41,13 @@ void common_hal_displayio_fourwire_construct(displayio_fourwire_obj_t* self,
 
     self->bus = spi;
     common_hal_busio_spi_never_reset(self->bus);
+    // Our object is statically allocated off the heap so make sure the bus object lives to the end
+    // of the heap as well.
+    gc_never_free(self->bus);
+
+    self->frequency = common_hal_busio_spi_get_frequency(spi);
+    self->polarity = common_hal_busio_spi_get_polarity(spi);
+    self->phase = common_hal_busio_spi_get_phase(spi);
 
     common_hal_digitalio_digitalinout_construct(&self->command, command);
     common_hal_digitalio_digitalinout_switch_to_output(&self->command, true, DRIVE_MODE_PUSH_PULL);
@@ -70,14 +79,19 @@ bool common_hal_displayio_fourwire_begin_transaction(mp_obj_t obj) {
     if (!common_hal_busio_spi_try_lock(self->bus)) {
         return false;
     }
-    // TODO(tannewt): Stop hardcoding SPI frequency, polarity and phase.
-    common_hal_busio_spi_configure(self->bus, 12000000, 0, 0, 8);
+    common_hal_busio_spi_configure(self->bus, self->frequency, self->polarity,
+                                   self->phase, 8);
     common_hal_digitalio_digitalinout_set_value(&self->chip_select, false);
     return true;
 }
 
 void common_hal_displayio_fourwire_send(mp_obj_t obj, bool command, uint8_t *data, uint32_t data_length) {
     displayio_fourwire_obj_t* self = MP_OBJ_TO_PTR(obj);
+    if (command) {
+        common_hal_digitalio_digitalinout_set_value(&self->chip_select, true);
+        common_hal_time_delay_ms(1);
+        common_hal_digitalio_digitalinout_set_value(&self->chip_select, false);
+    }
     common_hal_digitalio_digitalinout_set_value(&self->command, !command);
     common_hal_busio_spi_write(self->bus, data, data_length);
 }
diff --git a/shared-module/displayio/FourWire.h b/shared-module/displayio/FourWire.h
index 234bcf794..743139e62 100644
--- a/shared-module/displayio/FourWire.h
+++ b/shared-module/displayio/FourWire.h
@@ -38,6 +38,9 @@ typedef struct {
     digitalio_digitalinout_obj_t command;
     digitalio_digitalinout_obj_t chip_select;
     digitalio_digitalinout_obj_t reset;
+    uint32_t frequency;
+    uint8_t polarity;
+    uint8_t phase;
 } displayio_fourwire_obj_t;
 
 #endif // MICROPY_INCLUDED_ATMEL_SAMD_COMMON_HAL_DISPLAYIO_FOURWIRE_H
diff --git a/shared-module/displayio/Group.c b/shared-module/displayio/Group.c
index 20182863e..12f80cac4 100644
--- a/shared-module/displayio/Group.c
+++ b/shared-module/displayio/Group.c
@@ -73,8 +73,8 @@ void common_hal_displayio_group_insert(displayio_group_t* self, size_t index, mp
         mp_raise_ValueError(translate("Layer must be a Group or TileGrid subclass."));
     }
     // Shift everything right.
-    for (size_t i = index; i < self->size; i++) {
-        self->children[i + 1] = self->children[i];
+    for (size_t i = self->size; i > index; i--) {
+        self->children[i] = self->children[i - 1];
     }
     self->children[index].native = native_layer;
     self->children[index].original = layer;
diff --git a/shared-module/displayio/TileGrid.c b/shared-module/displayio/TileGrid.c
index bbbb11924..3212dfe8b 100644
--- a/shared-module/displayio/TileGrid.c
+++ b/shared-module/displayio/TileGrid.c
@@ -170,12 +170,24 @@ bool displayio_tilegrid_get_pixel(displayio_tilegrid_t *self, int16_t x, int16_t
 }
 
 bool displayio_tilegrid_needs_refresh(displayio_tilegrid_t *self) {
-    return self->needs_refresh || displayio_palette_needs_refresh(self->pixel_shader);
+    if (self->needs_refresh) {
+        return true;
+    } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_palette_type)) {
+        return displayio_palette_needs_refresh(self->pixel_shader);
+    } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_colorconverter_type)) {
+        return displayio_colorconverter_needs_refresh(self->pixel_shader);
+    }
+
+    return false;
 }
 
 void displayio_tilegrid_finish_refresh(displayio_tilegrid_t *self) {
     self->needs_refresh = false;
-    displayio_palette_finish_refresh(self->pixel_shader);
+    if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_palette_type)) {
+        displayio_palette_finish_refresh(self->pixel_shader);
+    } else if (MP_OBJ_IS_TYPE(self->pixel_shader, &displayio_colorconverter_type)) {
+        displayio_colorconverter_finish_refresh(self->pixel_shader);
+    }
     // TODO(tannewt): We could double buffer changes to position and move them over here.
     // That way they won't change during a refresh and tear.
 }
diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c
index ced5b0fef..156640440 100644
--- a/shared-module/displayio/__init__.c
+++ b/shared-module/displayio/__init__.c
@@ -5,6 +5,7 @@
 #include "lib/utils/interrupt_char.h"
 #include "py/reload.h"
 #include "py/runtime.h"
+#include "shared-bindings/board/__init__.h"
 #include "shared-bindings/displayio/Bitmap.h"
 #include "shared-bindings/displayio/Display.h"
 #include "shared-bindings/displayio/Group.h"
@@ -190,9 +191,14 @@ void reset_displays(void) {
         if (((uint32_t) fourwire->bus) < ((uint32_t) &displays) ||
             ((uint32_t) fourwire->bus) > ((uint32_t) &displays + CIRCUITPY_DISPLAY_LIMIT)) {
             busio_spi_obj_t* original_spi = fourwire->bus;
-            if (original_spi == board_spi()) {
-                continue;
-            }
+            #if BOARD_SPI
+                // We don't need to move original_spi if it is the board.SPI object because it is
+                // statically allocated already. (Doing so would also make it impossible to reference in
+                // a subsequent VM run.)
+                if (original_spi == common_hal_board_get_spi()) {
+                    continue;
+                }
+            #endif
             memcpy(&fourwire->inline_bus, original_spi, sizeof(busio_spi_obj_t));
             fourwire->bus = &fourwire->inline_bus;
             // Check for other displays that use the same spi bus and swap them too.