Rework background display task to allow reads from SPI SD card during display. Clarify code. Handle multiple displays better.

2 files changed, 51 insertions, 33 deletions

diff --git a/shared-module/displayio/Display.c b/shared-module/displayio/Display.c
index 2f2ba0665..0e76a868f 100644
--- a/shared-module/displayio/Display.c
+++ b/shared-module/displayio/Display.c
@@ -202,14 +202,16 @@ bool common_hal_displayio_display_set_brightness(displayio_display_obj_t* self,
     return ok;
 }
 
-// This routine is meant to be called as a background task. If it cannot acquire the display bus,
-// it will return false immediately to indicate it didn't do anything.
-bool displayio_display_start_region_update(displayio_display_obj_t* self, uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) {
+bool displayio_display_begin_transaction(displayio_display_obj_t* self) {
+    return self->begin_transaction(self->bus);
+}
+
+void displayio_display_end_transaction(displayio_display_obj_t* self) {
+    self->end_transaction(self->bus);
+}
+
+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.
-    if (!self->begin_transaction(self->bus)) {
-        // Could not acquire the bus; give up.
-        return false;
-    }
     uint16_t data[2];
     self->send(self->bus, true, &self->set_column_command, 1);
     data[0] = __builtin_bswap16(x0 + self->colstart);
@@ -220,11 +222,6 @@ bool displayio_display_start_region_update(displayio_display_obj_t* self, uint16
     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);
-    return true;
-}
-
-void displayio_display_finish_region_update(displayio_display_obj_t* self) {
-    self->end_transaction(self->bus);
 }
 
 bool displayio_display_frame_queued(displayio_display_obj_t* self) {
@@ -244,9 +241,8 @@ void displayio_display_finish_refresh(displayio_display_obj_t* self) {
     self->last_refresh = ticks_ms;
 }
 
-bool displayio_display_send_pixels(displayio_display_obj_t* self, uint32_t* pixels, uint32_t length) {
+void displayio_display_send_pixels(displayio_display_obj_t* self, uint32_t* pixels, uint32_t length) {
     self->send(self->bus, false, (uint8_t*) pixels, length * 4);
-    return true;
 }
 
 void displayio_display_update_backlight(displayio_display_obj_t* self) {
diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c
index 58a6ce471..546a46b1e 100644
--- a/shared-module/displayio/__init__.c
+++ b/shared-module/displayio/__init__.c
@@ -22,7 +22,8 @@ static inline void swap(uint16_t* a, uint16_t* b) {
     *b = temp;
 }
 
-bool refreshing_displays = false;
+// Check for recursive calls to displayio_refresh_displays.
+bool refresh_displays_in_progress = false;
 
 void displayio_refresh_displays(void) {
     if (mp_hal_is_interrupted()) {
@@ -35,20 +36,25 @@ void displayio_refresh_displays(void) {
         return;
     }
 
-    if (refreshing_displays) {
+    if (refresh_displays_in_progress) {
+        // Don't allow recursive calls to this routine.
         return;
     }
-    refreshing_displays = true;
+
+    refresh_displays_in_progress = true;
+
     for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) {
         if (displays[i].display.base.type == NULL || displays[i].display.base.type == &mp_type_NoneType) {
+            // Skip null display.
             continue;
         }
         displayio_display_obj_t* display = &displays[i].display;
         displayio_display_update_backlight(display);
 
+        // Time to refresh at specified frame rate?
         if (!displayio_display_frame_queued(display)) {
-            refreshing_displays = false;
-            return;
+            // Too soon. Try next display.
+            continue;
         }
         if (displayio_display_refresh_queued(display)) {
             // We compute the pixels. r and c are row and column to match the display memory
@@ -60,11 +66,13 @@ void displayio_refresh_displays(void) {
             if (display->transpose_xy) {
                 swap(&c1, &r1);
             }
-            // Someone else is holding the bus used to talk to the display,
-            // so skip update for now.
-            if (!displayio_display_start_region_update(display, c0, r0, c1, r1)) {
-                return;
+
+            if (!displayio_display_begin_transaction(display)) {
+                // Can't acquire display bus; skip updating this display. Try next display.
+                continue;
             }
+            displayio_display_set_region_to_update(display, c0, r0, c1, r1);
+            displayio_display_end_transaction(display);
 
             uint16_t x0 = 0;
             uint16_t x1 = display->width - 1;
@@ -98,6 +106,8 @@ void displayio_refresh_displays(void) {
             size_t index = 0;
             uint16_t buffer_size = 256;
             uint32_t buffer[buffer_size / 2];
+            bool skip_this_display = false;
+
             for (uint16_t y = starty; y0 <= y && y <= y1; y += dy) {
                 for (uint16_t x = startx; x0 <= x && x <= x1; x += dx) {
                     uint16_t* pixel = &(((uint16_t*)buffer)[index]);
@@ -114,11 +124,14 @@ void displayio_refresh_displays(void) {
                     index += 1;
                     // The buffer is full, send it.
                     if (index >= buffer_size) {
-                        if (!displayio_display_send_pixels(display, buffer, buffer_size / 2) || reload_requested) {
-                            displayio_display_finish_region_update(display);
-                            refreshing_displays = false;
-                            return;
+                        if (!displayio_display_begin_transaction(display)) {
+                            // Can't acquire display bus; skip the rest of the data. Try next display.
+                            index = 0;
+                            skip_this_display = true;
+                            break;
                         }
+                        displayio_display_send_pixels(display, buffer, buffer_size / 2);
+                        displayio_display_end_transaction(display);
                         // TODO(tannewt): Make refresh displays faster so we don't starve other
                         // background tasks.
                         usb_background();
@@ -126,17 +139,26 @@ void displayio_refresh_displays(void) {
                     }
                 }
             }
+
+            if (skip_this_display) {
+                // Go on to next display.
+                continue;
+            }
             // Send the remaining data.
-            if (index && !displayio_display_send_pixels(display, buffer, index * 2)) {
-                displayio_display_finish_region_update(display);
-                refreshing_displays = false;
-                return;
+            if (index) {
+                if (!displayio_display_begin_transaction(display)) {
+                    // Can't get display bus. Skip the rest of the data. Try next display.
+                    continue;
+                }
+                displayio_display_send_pixels(display, buffer, index * 2);
             }
-            displayio_display_finish_region_update(display);
+            displayio_display_end_transaction(display);
         }
         displayio_display_finish_refresh(display);
     }
-    refreshing_displays = false;
+
+    // All done.
+    refresh_displays_in_progress = false;
 }
 
 void common_hal_displayio_release_displays(void) {