~20% improvements in data transfer when drawing

Author: ppsx

ports/espressif/common-hal/qspibus/QSPIBus.c

@@ -194,12 +194,11 @@ static void qspibus_send_color_bytes(
         remaining -= chunk;
     }
 
-    // Keep Python/API semantics predictable: color transfer call returns only
-    // after queued DMA chunks have completed.
-    if (!qspibus_wait_all_transfers_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
-        qspibus_reset_transfer_state(self);
-        mp_raise_OSError_msg(MP_ERROR_TEXT("Operation timed out"));
-    }
+    // Let DMA complete asynchronously. The next begin_transaction() will
+    // wait for all in-flight transfers, allowing fill_area() computation
+    // to overlap with DMA.  The explicit wait is only needed for the
+    // Python write_data() API path where callers expect the transfer to
+    // be finished on return.
 }
 
 static bool qspibus_is_color_payload_command(uint8_t command) {
@@ -465,12 +464,19 @@ void common_hal_qspibus_qspibus_write_data(
         }
         return;
     }
-    if (!self->has_pending_command) {
+   if (!self->has_pending_command) {
         mp_raise_ValueError(MP_ERROR_TEXT("No pending command"));
     }
 
     if (qspibus_is_color_payload_command(self->pending_command)) {
         qspibus_send_color_bytes(self, self->pending_command, data, len);
+        // Python API: wait for DMA to finish so callers see the transfer as
+        // complete on return. The internal displayio path skips this wait
+        // to allow fill_area/DMA overlap.
+        if (!qspibus_wait_all_transfers_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
+            qspibus_reset_transfer_state(self);
+            mp_raise_OSError_msg(MP_ERROR_TEXT("Operation timed out"));
+        }
     } else {
         qspibus_send_command_bytes(self, self->pending_command, data, len);
     }
@@ -497,9 +503,20 @@ bool common_hal_qspibus_qspibus_bus_free(mp_obj_t obj) {
 
 bool common_hal_qspibus_qspibus_begin_transaction(mp_obj_t obj) {
     qspibus_qspibus_obj_t *self = MP_OBJ_TO_PTR(obj);
-    if (!common_hal_qspibus_qspibus_bus_free(obj)) {
+    if (!self->bus_initialized || self->in_transaction || self->has_pending_command) {
         return false;
     }
+    // Wait for any in-flight DMA to complete before starting a new
+    // transaction. This replaces the old non-blocking bus_free() check
+    // and enables fill_area()/DMA overlap: the CPU fills the next
+    // subrectangle while the previous DMA runs, and this wait only
+    // blocks when we actually need the bus for the next send.
+    if (self->transfer_in_progress) {
+        if (!qspibus_wait_all_transfers_done(self, pdMS_TO_TICKS(QSPI_COLOR_TIMEOUT_MS))) {
+            qspibus_reset_transfer_state(self);
+            return false;
+        }
+    }
     self->in_transaction = true;
     return true;
 }

shared-module/busdisplay/BusDisplay.c

@@ -217,7 +217,7 @@ static void _send_pixels(busdisplay_busdisplay_obj_t *self, uint8_t *pixels, uin
 }
 
 static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_area_t *area) {
-    uint16_t buffer_size = 128; // In uint32_ts
+    uint32_t buffer_size = 128; // In uint32_ts
 
     displayio_area_t clipped;
     // Clip the area to the display by overlapping the areas. If there is no overlap then we're done.
@@ -226,7 +226,7 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
     }
     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);
+    uint32_t pixels_per_buffer = displayio_area_size(&clipped);
 
     uint16_t subrectangles = 1;
     // for SH1107 and other boundary constrained controllers
@@ -265,7 +265,8 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
     // upper bound for ESP32-S3's 24KB main task stack.
     _Static_assert(CIRCUITPY_QSPI_DISPLAY_AREA_BUFFER_SIZE <= 2048,
         "CIRCUITPY_QSPI_DISPLAY_AREA_BUFFER_SIZE exceeds safe stack limit (8KB)");
-    if (mp_obj_is_type(self->bus.bus, &qspibus_qspibus_type) &&
+    bool is_qspi_bus = mp_obj_is_type(self->bus.bus, &qspibus_qspibus_type);
+    if (is_qspi_bus &&
         self->core.colorspace.depth == 16 &&
         !self->bus.data_as_commands &&
         !self->bus.SH1107_addressing &&
@@ -275,6 +276,10 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
         if (rows_per_buffer == 0) {
             rows_per_buffer = 1;
         }
+        // Clamp to actual display height.
+        if (rows_per_buffer > displayio_area_height(&clipped)) {
+            rows_per_buffer = displayio_area_height(&clipped);
+        }
         subrectangles = displayio_area_height(&clipped) / rows_per_buffer;
         if (displayio_area_height(&clipped) % rows_per_buffer != 0) {
             subrectangles++;
@@ -284,32 +289,31 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
         if (pixels_per_buffer % pixels_per_word) {
             buffer_size += 1;
         }
-    }
 
-    if (mp_obj_is_type(self->bus.bus, &qspibus_qspibus_type) &&
-        self->core.colorspace.depth == 16 &&
-        !self->bus.data_as_commands &&
-        displayio_area_height(&clipped) > 1 &&
-        rows_per_buffer < 2 &&
-        (2 * displayio_area_width(&clipped) + pixels_per_word - 1) / pixels_per_word <= CIRCUITPY_QSPI_DISPLAY_AREA_BUFFER_SIZE) {
-        rows_per_buffer = 2;
-        subrectangles = displayio_area_height(&clipped) / rows_per_buffer;
-        if (displayio_area_height(&clipped) % rows_per_buffer != 0) {
-            subrectangles++;
-        }
-        pixels_per_buffer = rows_per_buffer * displayio_area_width(&clipped);
-        buffer_size = pixels_per_buffer / pixels_per_word;
-        if (pixels_per_buffer % pixels_per_word) {
-            buffer_size += 1;
+        // Ensure at least 2 rows per buffer when possible.
+        if (rows_per_buffer < 2 &&
+            displayio_area_height(&clipped) > 1 &&
+            (uint32_t)((2 * displayio_area_width(&clipped) + pixels_per_word - 1) / pixels_per_word) <= (uint32_t)CIRCUITPY_QSPI_DISPLAY_AREA_BUFFER_SIZE) {
+            rows_per_buffer = 2;
+            subrectangles = displayio_area_height(&clipped) / rows_per_buffer;
+            if (displayio_area_height(&clipped) % rows_per_buffer != 0) {
+                subrectangles++;
+            }
+            pixels_per_buffer = rows_per_buffer * displayio_area_width(&clipped);
+            buffer_size = pixels_per_buffer / pixels_per_word;
+            if (pixels_per_buffer % pixels_per_word) {
+                buffer_size += 1;
+            }
         }
     }
     #endif
 
     // Allocated and shared as a uint32_t array so the compiler knows the
     // alignment everywhere.
-    uint32_t buffer[buffer_size];
     uint32_t mask_length = (pixels_per_buffer / 32) + 1;
+    uint32_t buffer[buffer_size];
     uint32_t mask[mask_length];
+
     uint16_t remaining_rows = displayio_area_height(&clipped);
 
     for (uint16_t j = 0; j < subrectangles; j++) {
@@ -324,28 +328,55 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
         }
         remaining_rows -= rows_per_buffer;
 
-        displayio_display_bus_set_region_to_update(&self->bus, &self->core, &subrectangle);
+        #if CIRCUITPY_QSPIBUS
+        if (is_qspi_bus) {
+            // QSPI path: fill_area first (overlaps with previous DMA),
+            // then single-transaction set_region + RAMWR + pixels.
+            // depth is always 16 here (guarded by is_qspi_bus check above).
+            uint32_t subrectangle_size_bytes = (uint32_t)displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
 
-        uint16_t subrectangle_size_bytes;
-        if (self->core.colorspace.depth >= 8) {
-            subrectangle_size_bytes = displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
-        } else {
-            subrectangle_size_bytes = displayio_area_size(&subrectangle) / (8 / self->core.colorspace.depth);
-        }
+            memset(mask, 0, mask_length * sizeof(mask[0]));
+            memset(buffer, 0, buffer_size * sizeof(buffer[0]));
+
+            displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
 
-        memset(mask, 0, mask_length * sizeof(mask[0]));
-        memset(buffer, 0, buffer_size * sizeof(buffer[0]));
+            // begin_transaction waits for any prior async DMA to finish,
+            // so fill_area above overlaps with previous DMA.
+            if (!displayio_display_bus_begin_transaction(&self->bus)) {
+                // Transaction failed (bus deinitialized, timeout, etc.).
+                // Bail out to prevent calling send() outside a transaction.
+                return false;
+            }
+            displayio_display_bus_send_region_commands(&self->bus, &self->core, &subrectangle);
+            _send_pixels(self, (uint8_t *)buffer, subrectangle_size_bytes);
+            displayio_display_bus_end_transaction(&self->bus);
+        } else
+        #endif
+        {
+            // Non-QSPI path: original ordering preserved exactly.
+            displayio_display_bus_set_region_to_update(&self->bus, &self->core, &subrectangle);
+
+            uint16_t subrectangle_size_bytes;
+            if (self->core.colorspace.depth >= 8) {
+                subrectangle_size_bytes = displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
+            } else {
+                subrectangle_size_bytes = displayio_area_size(&subrectangle) / (8 / self->core.colorspace.depth);
+            }
 
-        displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
+            memset(mask, 0, mask_length * sizeof(mask[0]));
+            memset(buffer, 0, buffer_size * sizeof(buffer[0]));
 
-        // Can't acquire display bus; skip the rest of the data.
-        if (!displayio_display_bus_is_free(&self->bus)) {
-            return false;
-        }
+            displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
 
-        displayio_display_bus_begin_transaction(&self->bus);
-        _send_pixels(self, (uint8_t *)buffer, subrectangle_size_bytes);
-        displayio_display_bus_end_transaction(&self->bus);
+            // Can't acquire display bus; skip the rest of the data.
+            if (!displayio_display_bus_is_free(&self->bus)) {
+                return false;
+            }
+
+            displayio_display_bus_begin_transaction(&self->bus);
+            _send_pixels(self, (uint8_t *)buffer, subrectangle_size_bytes);
+            displayio_display_bus_end_transaction(&self->bus);
+        }
 
         // Run background tasks so they can run during an explicit refresh.
         // Auto-refresh won't run background tasks here because it is a background task itself.
@@ -356,6 +387,21 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
         usb_background();
         #endif
     }
+
+    #if CIRCUITPY_QSPIBUS
+    if (is_qspi_bus) {
+        // Drain the last async DMA transfer before returning.
+        // Within the loop, begin_transaction() waits for the PREVIOUS
+        // subrectangle's DMA, enabling fill_area/DMA overlap.  But the
+        // LAST subrectangle's DMA is still in-flight when the loop ends.
+        // Without this drain, bus_free() returns false on the next
+        // refresh() call, causing it to be silently skipped.
+        if (displayio_display_bus_begin_transaction(&self->bus)) {
+            displayio_display_bus_end_transaction(&self->bus);
+        }
+    }
+    #endif
+
     return true;
 }
 

shared-module/displayio/TileGrid.c

@@ -512,6 +512,75 @@ bool displayio_tilegrid_fill_area(displayio_tilegrid_t *self,
         y_shift = temp_shift;
     }
 
+    // Fast path for the common case: single-tile, 16bpp bitmap with Palette,
+    // no transforms, full coverage.  Inlines bitmap access and palette cache
+    // lookup to eliminate per-pixel function-call overhead (~4-5x speedup).
+    if (full_coverage &&
+        colorspace->depth == 16 &&
+        self->width_in_tiles == 1 &&
+        self->height_in_tiles == 1 &&
+        self->absolute_transform->scale == 1 &&
+        self->absolute_transform->dx == 1 &&
+        self->absolute_transform->dy == 1 &&
+        !self->absolute_transform->transpose_xy &&
+        !self->flip_x && !self->flip_y &&
+        !self->transpose_xy &&
+        x_stride == 1 && y_stride == displayio_area_width(area) &&
+        x_shift == 0 && y_shift == 0 &&
+        mp_obj_is_type(self->bitmap, &displayio_bitmap_type) &&
+        mp_obj_is_type(self->pixel_shader, &displayio_palette_type)) {
+
+        displayio_bitmap_t *bmp = MP_OBJ_TO_PTR(self->bitmap);
+        displayio_palette_t *pal = MP_OBJ_TO_PTR(self->pixel_shader);
+
+        if (bmp->bits_per_value == 16 && !pal->dither) {
+            uint16_t *out = (uint16_t *)buffer;
+            bool all_opaque = true;
+
+            // Process all pixels with inlined bitmap access and palette cache.
+            uint32_t idx = 0;
+            for (int16_t y = start_y; y < end_y; ++y) {
+                // Direct row pointer into bitmap data (16bpp → uint16_t).
+                // stride is in uint32_t units, so multiply by 2 for uint16_t indexing.
+                const uint16_t *bmp_row = ((const uint16_t *)bmp->data) + y * (bmp->stride * 2);
+
+                for (int16_t x = start_x; x < end_x; ++x) {
+                    uint16_t px_index = bmp_row[x];
+
+                    // Inline palette cache check (avoids function call overhead).
+                    if (px_index < pal->color_count) {
+                        _displayio_color_t *color = &pal->colors[px_index];
+                        if (color->transparent) {
+                            all_opaque = false;
+                            idx++;
+                            continue;
+                        }
+                        if (color->cached_colorspace == colorspace &&
+                            color->cached_colorspace_grayscale_bit == colorspace->grayscale_bit &&
+                            color->cached_colorspace_grayscale == colorspace->grayscale) {
+                            out[idx] = (uint16_t)color->cached_color;
+                        } else {
+                            // Cache miss — do full conversion, then cache.
+                            displayio_input_pixel_t in_px = {.pixel = px_index};
+                            displayio_output_pixel_t out_px = {.opaque = true};
+                            displayio_palette_get_color(pal, colorspace, &in_px, &out_px);
+                            out[idx] = (uint16_t)out_px.pixel;
+                        }
+                    } else {
+                        // Out of palette range — transparent.
+                        all_opaque = false;
+                        idx++;
+                        continue;
+                    }
+                    mask[idx >> 5] |= 1u << (idx & 31);
+                    idx++;
+                }
+            }
+
+            return all_opaque;
+        }
+    }
+
     displayio_input_pixel_t input_pixel;
     displayio_output_pixel_t output_pixel;
 

shared-module/displayio/bus_core.h

@@ -49,6 +49,12 @@ void displayio_display_bus_end_transaction(displayio_display_bus_t *self);
 
 void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area);
 
+#if CIRCUITPY_QSPIBUS
+// Send column/row window commands within an already-open transaction.
+// Caller must have called displayio_display_bus_begin_transaction() first.
+void displayio_display_bus_send_region_commands(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area);
+#endif
+
 void release_display_bus(displayio_display_bus_t *self);
 
 void displayio_display_bus_collect_ptrs(displayio_display_bus_t *self);