Mostly-working-version with comments

7 files changed, 64 insertions, 30 deletions

diff --git a/shared-bindings/displayio/Display.c b/shared-bindings/displayio/Display.c
index 2ae237350..a81736e46 100644
--- a/shared-bindings/displayio/Display.c
+++ b/shared-bindings/displayio/Display.c
@@ -105,14 +105,22 @@
 //|         :param bool auto_brightness: If True, brightness is controlled via an ambient light sensor or other mechanism.
 //|         :param bool single_byte_bounds: Display column and row commands use single bytes
 //|         :param bool data_as_commands: Treat all init and boundary data as SPI commands. Certain displays require this.
-//|         :param bool column_and_page_addressing: Special quirk for SH1107, use upper/lower column set and page set
+//|         :param bool SH1107_addressing: Special quirk for SH1107, use upper/lower column set and page set
 //|         :param bool auto_refresh: Automatically refresh the screen
 //|         :param int native_frames_per_second: Number of display refreshes per second that occur with the given init_sequence.
 //|         :param bool backlight_on_high: If True, pulling the backlight pin high turns the backlight on."""
 //|         ...
 //|
-STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
-    enum { ARG_display_bus, ARG_init_sequence, ARG_width, ARG_height, ARG_colstart, ARG_rowstart, ARG_rotation, ARG_color_depth, ARG_grayscale, ARG_pixels_in_byte_share_row, ARG_bytes_per_cell, ARG_reverse_pixels_in_byte, ARG_reverse_bytes_in_word, ARG_set_column_command, ARG_set_row_command, ARG_write_ram_command, ARG_set_vertical_scroll, ARG_backlight_pin, ARG_brightness_command, ARG_brightness, ARG_auto_brightness, ARG_single_byte_bounds, ARG_data_as_commands, ARG_auto_refresh, ARG_native_frames_per_second, ARG_backlight_on_high, ARG_column_and_page_addressing };
+STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_args, 
+                const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    enum { ARG_display_bus, ARG_init_sequence, ARG_width, ARG_height, ARG_colstart, ARG_rowstart, 
+                ARG_rotation, ARG_color_depth, ARG_grayscale, ARG_pixels_in_byte_share_row, 
+                ARG_bytes_per_cell, ARG_reverse_pixels_in_byte, ARG_reverse_bytes_in_word, 
+                ARG_set_column_command, ARG_set_row_command, ARG_write_ram_command, 
+                ARG_set_vertical_scroll, ARG_backlight_pin, ARG_brightness_command, 
+                ARG_brightness, ARG_auto_brightness, ARG_single_byte_bounds, ARG_data_as_commands, 
+                ARG_auto_refresh, ARG_native_frames_per_second, ARG_backlight_on_high, 
+                ARG_SH1107_addressing };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_display_bus, MP_ARG_REQUIRED | MP_ARG_OBJ },
         { MP_QSTR_init_sequence, MP_ARG_REQUIRED | MP_ARG_OBJ },
@@ -140,7 +148,7 @@ STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_a
         { MP_QSTR_auto_refresh, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = true} },
         { MP_QSTR_native_frames_per_second, MP_ARG_INT | MP_ARG_KW_ONLY, {.u_int = 60} },
         { MP_QSTR_backlight_on_high, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = true} },
-        { MP_QSTR_column_and_page_addressing, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = false} },
+        { MP_QSTR_SH1107_addressing, MP_ARG_BOOL | MP_ARG_KW_ONLY, {.u_bool = false} }
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
@@ -183,7 +191,7 @@ STATIC mp_obj_t displayio_display_make_new(const mp_obj_type_t *type, size_t n_a
         args[ARG_auto_refresh].u_bool,
         args[ARG_native_frames_per_second].u_int,
         args[ARG_backlight_on_high].u_bool,
-        args[ARG_column_and_page_addressing].u_bool
+        args[ARG_SH1107_addressing].u_bool
         );
 
     return self;
diff --git a/shared-bindings/displayio/Display.h b/shared-bindings/displayio/Display.h
index d6274a2b2..c1704eaad 100644
--- a/shared-bindings/displayio/Display.h
+++ b/shared-bindings/displayio/Display.h
@@ -46,7 +46,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
     uint8_t* init_sequence, uint16_t init_sequence_len, const mcu_pin_obj_t* backlight_pin, uint16_t brightness_command,
     mp_float_t brightness, bool auto_brightness,
     bool single_byte_bounds, bool data_as_commands, bool auto_refresh, uint16_t native_frames_per_second, 
-    bool backlight_on_high, bool column_and_page_addressing);
+    bool backlight_on_high, bool SH1107_addressing);
 
 bool common_hal_displayio_display_show(displayio_display_obj_t* self,
                                        displayio_group_t* root_group);
diff --git a/shared-module/displayio/Display.c b/shared-module/displayio/Display.c
index b6979c0b2..afc4b7564 100644
--- a/shared-module/displayio/Display.c
+++ b/shared-module/displayio/Display.c
@@ -49,7 +49,8 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
         uint8_t* init_sequence, uint16_t init_sequence_len, const mcu_pin_obj_t* backlight_pin,
         uint16_t brightness_command, mp_float_t brightness, bool auto_brightness,
         bool single_byte_bounds, bool data_as_commands, bool auto_refresh, uint16_t native_frames_per_second, 
-        bool backlight_on_high, bool column_and_page_addressing) {
+        bool backlight_on_high, bool SH1107_addressing) {
+
     // Turn off auto-refresh as we init.
     self->auto_refresh = false;
     uint16_t ram_width = 0x100;
@@ -69,7 +70,7 @@ void common_hal_displayio_display_construct(displayio_display_obj_t* self,
     self->first_manual_refresh = !auto_refresh;
     self->data_as_commands = data_as_commands;
     self->backlight_on_high = backlight_on_high;
-    self->column_and_page_addressing = column_and_page_addressing;
+    self->SH1107_addressing = SH1107_addressing;
 
     self->native_frames_per_second = native_frames_per_second;
     self->native_ms_per_frame = 1000 / native_frames_per_second;
@@ -242,11 +243,23 @@ STATIC bool _refresh_area(displayio_display_obj_t* self, const displayio_area_t*
     if (!displayio_display_core_clip_area(&self->core, area, &clipped)) {
         return true;
     }
-    uint16_t subrectangles = 1;
     uint16_t rows_per_buffer = displayio_area_height(&clipped);
     uint8_t pixels_per_word = (sizeof(uint32_t) * 8) / self->core.colorspace.depth;
     uint16_t pixels_per_buffer = displayio_area_size(&clipped);
-    if (displayio_area_size(&clipped) > buffer_size * pixels_per_word) {
+    
+    uint16_t subrectangles = 1;
+    // for SH1107 and other boundary constrained controllers
+    //      write one single row at a time
+    if (self->SH1107_addressing) {
+        subrectangles = rows_per_buffer;  // vertical (column mode) write each separately (height times)
+    }
+
+    // Skip this recalculation of subrectangles if SH1107 (each subrectangle is a single row)
+    // [mdroberts1243] I am worried/confused about the pixels_in_byte_share_row calculation though
+    //       since it makes sense to be on a byte boundary (actually a page boundary too)
+    //       seems to work as is though.
+    if ((displayio_area_size(&clipped) > buffer_size * pixels_per_word)
+                    && (!self->SH1107_addressing)) {
         rows_per_buffer = buffer_size * pixels_per_word / displayio_area_width(&clipped);
         if (rows_per_buffer == 0) {
             rows_per_buffer = 1;
@@ -283,14 +296,23 @@ STATIC bool _refresh_area(displayio_display_obj_t* self, const displayio_area_t*
             .x2 = clipped.x2,
             .y2 = clipped.y1 + rows_per_buffer * (j + 1)
         };
-        if (remaining_rows < rows_per_buffer) {
+        if (self->SH1107_addressing) {
+                // one row only for SH1107 in vertical (column) mode
+                subrectangle.y1 = clipped.y1 + j;
+                subrectangle.y2 = clipped.y1 + (j + 1);
+            };
+        if ((remaining_rows < rows_per_buffer) && (!self->SH1107_addressing)) {
             subrectangle.y2 = subrectangle.y1 + remaining_rows;
         }
-        remaining_rows -= rows_per_buffer;
+        if (self->SH1107_addressing) {
+            remaining_rows -= 1;
+        } else {
+            remaining_rows -= rows_per_buffer;
+        }
 
         displayio_display_core_set_region_to_update(&self->core, self->set_column_command, 
               self->set_row_command, NO_COMMAND, NO_COMMAND, self->data_as_commands, false, 
-              &subrectangle, self->column_and_page_addressing);
+              &subrectangle, self->SH1107_addressing);
 
         uint16_t subrectangle_size_bytes;
         if (self->core.colorspace.depth >= 8) {
diff --git a/shared-module/displayio/Display.h b/shared-module/displayio/Display.h
index 86c2a9360..cc9cd54c4 100644
--- a/shared-module/displayio/Display.h
+++ b/shared-module/displayio/Display.h
@@ -61,7 +61,7 @@ typedef struct {
     bool updating_backlight;
     bool backlight_on_high;
     // new quirk for sh1107
-    bool column_and_page_addressing;
+    bool SH1107_addressing;
 } displayio_display_obj_t;
 
 void displayio_display_background(displayio_display_obj_t* self);
diff --git a/shared-module/displayio/EPaperDisplay.c b/shared-module/displayio/EPaperDisplay.c
index 397ceb4fd..3391a56b4 100644
--- a/shared-module/displayio/EPaperDisplay.c
+++ b/shared-module/displayio/EPaperDisplay.c
@@ -238,6 +238,7 @@ bool displayio_epaperdisplay_refresh_area(displayio_epaperdisplay_obj_t* self, c
     for (uint8_t pass = 0; pass < passes; pass++) {
         uint16_t remaining_rows = displayio_area_height(&clipped);
 
+        // added false parameter at end for SH1107_addressing quirk
         if (self->set_row_window_command != NO_COMMAND) {
             displayio_display_core_set_region_to_update(&self->core, self->set_column_window_command, 
             self->set_row_window_command, self->set_current_column_command, self->set_current_row_command, 
diff --git a/shared-module/displayio/display_core.c b/shared-module/displayio/display_core.c
index 19e45e6b6..4e36e4dd1 100644
--- a/shared-module/displayio/display_core.c
+++ b/shared-module/displayio/display_core.c
@@ -41,7 +41,7 @@
 #include <string.h>
 
 #define DISPLAYIO_CORE_DEBUG(...) (void)0
-// #define DISPLAYIO_CORE_DEBUG(...) mp_printf(&mp_plat_print __VA_OPT__(,) __VA_ARGS__)
+//#define DISPLAYIO_CORE_DEBUG(...) mp_printf(&mp_plat_print __VA_OPT__(,) __VA_ARGS__)
 
 void displayio_display_core_construct(displayio_display_core_t* self,
         mp_obj_t bus, uint16_t width, uint16_t height, uint16_t ram_width, uint16_t ram_height, int16_t colstart, int16_t rowstart, uint16_t rotation,
@@ -57,7 +57,7 @@ void displayio_display_core_construct(displayio_display_core_t* self,
     self->colstart = colstart;
     self->rowstart = rowstart;
     self->last_refresh = 0;
-
+    
     // (framebufferdisplay already validated its 'bus' is a buffer-protocol object)
     if (bus) {
         if (MP_OBJ_IS_TYPE(bus, &displayio_parallelbus_type)) {
@@ -211,7 +211,7 @@ void displayio_display_core_end_transaction(displayio_display_core_t* self) {
 void displayio_display_core_set_region_to_update(displayio_display_core_t* self, uint8_t column_command, 
               uint8_t row_command, uint16_t set_current_column_command, uint16_t set_current_row_command, 
               bool data_as_commands, bool always_toggle_chip_select, 
-              displayio_area_t* area, bool column_and_page_addressing) {
+              displayio_area_t* area, bool SH1107_addressing) {
     uint16_t x1 = area->x1;
     uint16_t x2 = area->x2;
     uint16_t y1 = area->y1;
@@ -256,12 +256,13 @@ void displayio_display_core_set_region_to_update(displayio_display_core_t* self,
         data[data_length++] = x2 >> 8;
         data[data_length++] = x2 & 0xff;
     }
-    // Quirk for SH1107 "column_and_page_addressing"
-    //     Column lower command = 0x00, Column upper command = 0x10
-    if (column_and_page_addressing) {
-        data[0] = 0x00 | (x1 & 0x0F);
-        data[1] = 0x10 | (x1 >> 4);
-        data_length = 2;
+    // Quirk for SH1107 "SH1107_addressing"
+    //     Note... column is y!  page is x!
+    //     Page address command = 0xB0
+    if (SH1107_addressing) {
+        // set the page to our x value
+        data[0] = 0xB0 | (x1 & 0x07);
+        data_length = 1;
     }
     self->send(self->bus, data_type, chip_select, data, data_length);
     displayio_display_core_end_transaction(self);
@@ -294,11 +295,13 @@ void displayio_display_core_set_region_to_update(displayio_display_core_t* self,
         data[data_length++] = y2 >> 8;
         data[data_length++] = y2 & 0xff;
     }
-    // Quirk for SH1107 "column_and_page_addressing"
-    //      Page address command = 0xB0
-    if (column_and_page_addressing) {
-        data[0] = 0xB0 | (y1 & 0x07);
-        data_length = 1;
+    // Quirk for SH1107 "SH1107_addressing"
+    //     Note... column is y!  page is x!
+    //     Column lower command = 0x00, Column upper command = 0x10
+    if (SH1107_addressing) {
+        data[0] = y1 & 0x0F; // 0x00 to 0x0F
+        data[1] = (y1 >> 4 & 0x0F) | 0x10; // 0x10 to 0x17
+        data_length = 2;
     }
     self->send(self->bus, data_type, chip_select, data, data_length);
 
@@ -319,7 +322,7 @@ void displayio_display_core_start_refresh(displayio_display_core_t* self) {
 
 void displayio_display_core_finish_refresh(displayio_display_core_t* self) {
     if (self->current_group != NULL) {
-        DISPLAYIO_CORE_DEBUG("displayiocore group_finish_refresh\n");
+//       DISPLAYIO_CORE_DEBUG("displayiocore group_finish_refresh\n");
         displayio_group_finish_refresh(self->current_group);
     }
     self->full_refresh = false;
diff --git a/shared-module/displayio/display_core.h b/shared-module/displayio/display_core.h
index 1c880be44..ad9998a2f 100644
--- a/shared-module/displayio/display_core.h
+++ b/shared-module/displayio/display_core.h
@@ -77,7 +77,7 @@ void displayio_display_core_end_transaction(displayio_display_core_t* self);
 void displayio_display_core_set_region_to_update(displayio_display_core_t* self, uint8_t column_command, 
           uint8_t row_command, uint16_t set_current_column_command, uint16_t set_current_row_command, 
           bool data_as_commands, bool always_toggle_chip_select, 
-          displayio_area_t* area, bool column_and_page_addressing);
+          displayio_area_t* area, bool SH1107_addressing);
 
 void release_display_core(displayio_display_core_t* self);