From c37e3a73955c9b46a801bfa05c834453af9486fe Mon Sep 17 00:00:00 2001 From: James Whitlock Date: Sun, 17 Aug 2025 22:33:14 +0100 Subject: [PATCH] Fix display bus transaction handling. Fixes #10569 --- shared-module/busdisplay/BusDisplay.c | 12 ++++---- shared-module/displayio/bus_core.c | 18 +++++------ shared-module/displayio/bus_core.h | 2 +- shared-module/epaperdisplay/EPaperDisplay.c | 33 ++++++++++++++------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/shared-module/busdisplay/BusDisplay.c b/shared-module/busdisplay/BusDisplay.c index ac57f3bf3e019..993839596e512 100644 --- a/shared-module/busdisplay/BusDisplay.c +++ b/shared-module/busdisplay/BusDisplay.c @@ -273,7 +273,10 @@ 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 (!displayio_display_bus_set_region_to_update(&self->bus, &self->core, &subrectangle)) { + // This is a failure to acquire the bus, skip the rest of the refresh. + return false; + } uint16_t subrectangle_size_bytes; if (self->core.colorspace.depth >= 8) { @@ -287,12 +290,11 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer); - // Can't acquire display bus; skip the rest of the data. - if (!displayio_display_bus_is_free(&self->bus)) { + // If we can't acquire display bus; skip the rest of the data. + if (!displayio_display_bus_begin_transaction(&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); @@ -310,7 +312,7 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are static void _refresh_display(busdisplay_busdisplay_obj_t *self) { if (!displayio_display_bus_is_free(&self->bus)) { - // A refresh on this bus is already in progress. Try next display. + // Don't bother with the refresh if the bus is already busy return; } displayio_display_core_start_refresh(&self->core); diff --git a/shared-module/displayio/bus_core.c b/shared-module/displayio/bus_core.c index e01b9d9eef685..506e8bcfb3be3 100644 --- a/shared-module/displayio/bus_core.c +++ b/shared-module/displayio/bus_core.c @@ -101,7 +101,8 @@ void displayio_display_bus_end_transaction(displayio_display_bus_t *self) { self->end_transaction(self->bus); } -void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area) { +// Determine if this is run from a background task or not. Should we skip sending the data or wait for the bus? +bool displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area) { uint16_t x1 = area->x1 + self->colstart; uint16_t x2 = area->x2 + self->colstart; uint16_t y1 = area->y1 + self->rowstart; @@ -127,8 +128,12 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d chip_select = CHIP_SELECT_TOGGLE_EVERY_BYTE; } + if (!displayio_display_bus_begin_transaction(self)) { + // Can't acquire display bus; skip and report failure. + return false; + } + // Set column. - displayio_display_bus_begin_transaction(self); uint8_t data[5]; data[0] = self->column_command; uint8_t data_length = 1; @@ -163,20 +168,16 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d } self->send(self->bus, data_type, chip_select, data, data_length); - displayio_display_bus_end_transaction(self); if (self->set_current_column_command != NO_COMMAND) { uint8_t command = self->set_current_column_command; - displayio_display_bus_begin_transaction(self); self->send(self->bus, DISPLAY_COMMAND, chip_select, &command, 1); // Only send the first half of data because it is the first coordinate. self->send(self->bus, DISPLAY_DATA, chip_select, data, data_length / 2); - displayio_display_bus_end_transaction(self); } // Set row. - displayio_display_bus_begin_transaction(self); data[0] = self->row_command; data_length = 1; if (!self->data_as_commands) { @@ -207,16 +208,15 @@ void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, d } self->send(self->bus, data_type, chip_select, data, data_length); - displayio_display_bus_end_transaction(self); if (self->set_current_row_command != NO_COMMAND) { uint8_t command = self->set_current_row_command; - displayio_display_bus_begin_transaction(self); self->send(self->bus, DISPLAY_COMMAND, chip_select, &command, 1); // Only send the first half of data because it is the first coordinate. self->send(self->bus, DISPLAY_DATA, chip_select, data, data_length / 2); - displayio_display_bus_end_transaction(self); } + displayio_display_bus_end_transaction(self); + return true; } void displayio_display_bus_collect_ptrs(displayio_display_bus_t *self) { diff --git a/shared-module/displayio/bus_core.h b/shared-module/displayio/bus_core.h index 838454c92e6d8..5d8deba22e0b0 100644 --- a/shared-module/displayio/bus_core.h +++ b/shared-module/displayio/bus_core.h @@ -47,7 +47,7 @@ bool displayio_display_bus_is_free(displayio_display_bus_t *self); bool displayio_display_bus_begin_transaction(displayio_display_bus_t *self); 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); +bool displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area); void release_display_bus(displayio_display_bus_t *self); diff --git a/shared-module/epaperdisplay/EPaperDisplay.c b/shared-module/epaperdisplay/EPaperDisplay.c index e1f0a07f467ee..444dc90538132 100644 --- a/shared-module/epaperdisplay/EPaperDisplay.c +++ b/shared-module/epaperdisplay/EPaperDisplay.c @@ -162,7 +162,9 @@ static void send_command_sequence(epaperdisplay_epaperdisplay_obj_t *self, data_size = ((data_size & ~DELAY) << 8) + *(cmd + 2); data = cmd + 3; } - displayio_display_bus_begin_transaction(&self->bus); + while (!displayio_display_bus_begin_transaction(&self->bus)) { + RUN_BACKGROUND_TASKS; + } self->bus.send(self->bus.bus, DISPLAY_COMMAND, self->chip_select, cmd, 1); self->bus.send(self->bus.bus, DISPLAY_DATA, self->chip_select, data, data_size); displayio_display_bus_end_transaction(&self->bus); @@ -310,14 +312,21 @@ static bool epaperdisplay_epaperdisplay_refresh_area(epaperdisplay_epaperdisplay uint16_t remaining_rows = displayio_area_height(&clipped); if (self->bus.row_command != NO_COMMAND) { - displayio_display_bus_set_region_to_update(&self->bus, &self->core, &clipped); + if (!displayio_display_bus_set_region_to_update(&self->bus, &self->core, &clipped)) { + // This is a failure to acquire the bus, skip the rest of the refresh. + return false; + } } uint8_t write_command = self->write_black_ram_command; if (pass == 1) { write_command = self->write_color_ram_command; } - displayio_display_bus_begin_transaction(&self->bus); + + if (!displayio_display_bus_begin_transaction(&self->bus)) { + // We can't acquire the bus skip the rest of the refresh. + return false; + } self->bus.send(self->bus.bus, DISPLAY_COMMAND, self->chip_select, &write_command, 1); displayio_display_bus_end_transaction(&self->bus); @@ -394,24 +403,26 @@ static bool _clean_area(epaperdisplay_epaperdisplay_obj_t *self) { memset(buffer, 0x77, width / 2); uint8_t write_command = self->write_black_ram_command; - displayio_display_bus_begin_transaction(&self->bus); + + if (!displayio_display_bus_begin_transaction(&self->bus)) { + // Can't acquire display bus; skip the rest of the data. Try next display. + return false; + } self->bus.send(self->bus.bus, DISPLAY_COMMAND, self->chip_select, &write_command, 1); - displayio_display_bus_end_transaction(&self->bus); for (uint16_t j = 0; j < height; j++) { - if (!displayio_display_bus_begin_transaction(&self->bus)) { - // Can't acquire display bus; skip the rest of the data. Try next display. - return false; - } self->bus.send(self->bus.bus, DISPLAY_DATA, self->chip_select, buffer, width / 2); - displayio_display_bus_end_transaction(&self->bus); // TODO(tannewt): Make refresh displays faster so we don't starve other // background tasks. #if CIRCUITPY_TINYUSB - usb_background(); + displayio_display_bus_end_transaction(&self->bus); + do { + usb_background(); + } while (!displayio_display_bus_begin_transaction(&self->bus)); #endif } + displayio_display_bus_end_transaction(&self->bus); return true; }