Minor tweaks based on feedback

4 files changed, 68 insertions, 22 deletions

diff --git a/shared-module/displayio/Bitmap.c b/shared-module/displayio/Bitmap.c
index 453dc1c59..addc341a7 100644
--- a/shared-module/displayio/Bitmap.c
+++ b/shared-module/displayio/Bitmap.c
@@ -31,8 +31,8 @@
 #include "py/runtime.h"
 
 void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t width,
-    uint32_t height, uint32_t value_size) {
-    uint32_t row_width = width * value_size;
+    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);
@@ -43,16 +43,25 @@ void common_hal_displayio_bitmap_construct(displayio_bitmap_t *self, uint32_t wi
     self->height = height;
     self->data = m_malloc(self->stride * height * sizeof(uint32_t), false);
 
-    self->bits_per_value = value_size;
+    self->bits_per_value = bits_per_value;
 
-    self->x_shift = 0;
+    if (bits_per_value > 8) {
+        mp_raise_NotImplementedError(translate("Only bit maps of 8 bit color or less are supported"));
+    }
+
+    // Division and modulus can be slow because it has to handle any integer. We know bits_per_value
+    // is a power of two. We divide and mod by bits_per_value to compute the offset into the byte
+    // array. So, we can the offset computation to simplify to a shift for division and mask for mod.
+
+    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 / value_size ) {
+    while (power_of_two < 32 / bits_per_value ) {
         self->x_shift++;
         power_of_two <<= 1;
     }
     self->x_mask = (1 << self->x_shift) - 1; // Used as a modulus on the x value
-    self->bitmask = (1 << value_size) - 1;
+    self->bitmask = (1 << bits_per_value) - 1;
 }
 
 void common_hal_displayio_bitmap_load_row(displayio_bitmap_t *self, uint16_t y, uint8_t* data, uint16_t len) {
diff --git a/shared-module/displayio/Palette.c b/shared-module/displayio/Palette.c
index e2ec6e057..5f410d786 100644
--- a/shared-module/displayio/Palette.c
+++ b/shared-module/displayio/Palette.c
@@ -27,7 +27,7 @@
 #include "shared-bindings/displayio/Palette.h"
 
 void common_hal_displayio_palette_construct(displayio_palette_t* self, uint16_t color_count) {
-    self->max_value = color_count;
+    self->color_count = color_count;
     self->colors = (uint32_t *) m_malloc(color_count * sizeof(uint16_t), false);
     uint32_t opaque_byte_count = color_count / 8;
     if (color_count % 8 > 0) {
@@ -36,35 +36,35 @@ void common_hal_displayio_palette_construct(displayio_palette_t* self, uint16_t
     self->opaque = (uint32_t *) m_malloc(opaque_byte_count, false);
 }
 
-void common_hal_displayio_palette_make_opaque(displayio_palette_t* self, uint32_t value) {
-    self->opaque[value / 32] &= ~(0x1 << (value % 32));
+void common_hal_displayio_palette_make_opaque(displayio_palette_t* self, uint32_t palette_index) {
+    self->opaque[palette_index / 32] &= ~(0x1 << (palette_index % 32));
 }
 
-void common_hal_displayio_palette_make_transparent(displayio_palette_t* self, uint32_t value) {
-    self->opaque[value / 32] |= (0x1 << (value % 32));
+void common_hal_displayio_palette_make_transparent(displayio_palette_t* self, uint32_t palette_index) {
+    self->opaque[palette_index / 32] |= (0x1 << (palette_index % 32));
 }
 
-void common_hal_displayio_palette_set_color(displayio_palette_t* self, uint32_t value, uint32_t color) {
-    uint32_t shift = (value % 2) * 16;
-    uint32_t masked = self->colors[value / 2] & ~(0xffff << shift);
+void common_hal_displayio_palette_set_color(displayio_palette_t* self, uint32_t palette_index, uint32_t color) {
+    uint32_t shift = (palette_index % 2) * 16;
+    uint32_t masked = self->colors[palette_index / 2] & ~(0xffff << shift);
     uint32_t b5 = (color >> 19);
     uint32_t g6 = (color >> 10) & 0x3f;
     uint32_t r5 = (color >> 3) & 0x1f;
     uint32_t packed = r5 << 11 | g6 << 5 | b5;
     // swap bytes
-    packed = ((packed >> 8) & 0xff) | ((packed & 0xff) << 8);
-    self->colors[value / 2] = masked | packed << shift;
+    packed = __builtin_bswap16(packed);
+    self->colors[palette_index / 2] = masked | packed << shift;
     self->needs_refresh = true;
 }
 
-bool displayio_palette_get_color(displayio_palette_t *self, uint32_t value, uint16_t* color) {
-    if (value > self->max_value) {
+bool displayio_palette_get_color(displayio_palette_t *self, uint32_t palette_index, uint16_t* color) {
+    if (palette_index > self->color_count) {
         return false;
     }
-    if ((self->opaque[value / 32] & (0x1 << (value % 32))) != 0) {
+    if ((self->opaque[palette_index / 32] & (0x1 << (palette_index % 32))) != 0) {
         return false;
     }
-    *color = (self->colors[value / 2] >> (16 * (value % 2))) & 0xffff;
+    *color = (self->colors[palette_index / 2] >> (16 * (palette_index % 2))) & 0xffff;
 
     return true;
 }
diff --git a/shared-module/displayio/Palette.h b/shared-module/displayio/Palette.h
index 22c737c9d..5a5f1d287 100644
--- a/shared-module/displayio/Palette.h
+++ b/shared-module/displayio/Palette.h
@@ -36,11 +36,11 @@ typedef struct {
     mp_obj_base_t base;
     uint32_t* opaque;
     uint32_t* colors;
-    uint8_t max_value;
+    uint8_t color_count;
     bool needs_refresh;
 } displayio_palette_t;
 
-bool displayio_palette_get_color(displayio_palette_t *palette, uint32_t value, uint16_t* color);
+bool displayio_palette_get_color(displayio_palette_t *palette, uint32_t palette_index, uint16_t* color);
 bool displayio_palette_needs_refresh(displayio_palette_t *self);
 void displayio_palette_finish_refresh(displayio_palette_t *self);
 
diff --git a/shared-module/displayio/mipi_constants.h b/shared-module/displayio/mipi_constants.h
new file mode 100644
index 000000000..3cb7e4292
--- /dev/null
+++ b/shared-module/displayio/mipi_constants.h
@@ -0,0 +1,37 @@
+/*
+ * This file is part of the Micro Python project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2018 Scott Shawcroft for Adafruit Industries
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_MIPI_CONSTANTS_H
+#define MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_MIPI_CONSTANTS_H
+
+// More info here: https://www.tonylabs.com/wp-content/uploads/MIPI_DCS_specification_v1.02.00.pdf
+enum mipi_command {
+    MIPI_COMMAND_SET_COLUMN_ADDRESS = 0x2a,
+    MIPI_COMMAND_SET_PAGE_ADDRESS = 0x2b,
+    MIPI_COMMAND_WRITE_MEMORY_START = 0x2c,
+};
+
+#endif // MICROPY_INCLUDED_SHARED_BINDINGS_DISPLAYIO_MIPI_CONSTANTS_H