Skip to content

Commit 6471489

Browse files
Copilotppsx
andcommitted
Fix: guard all QSPI code with #if CIRCUITPY_QSPIBUS to eliminate non-QSPI build impact
Three issues found and fixed: 1. bus_core.c: displayio_display_bus_send_region_commands() was compiled unconditionally, adding ~110 lines of dead code to every non-QSPI display build. Now guarded with #if CIRCUITPY_QSPIBUS. 2. bus_core.h: Same guard for the declaration. 3. BusDisplay.c: The previous commit moved set_region_to_update() after fill_area() even for non-QSPI buses, changing the call ordering. Now the non-QSPI else-block reproduces the exact original ordering: set_region_to_update → size_calc → fill_area → is_free → send_pixels. Verified: when CIRCUITPY_QSPIBUS=0, all C statements in the affected files are identical to the original codebase. Co-authored-by: ppsx <7107135+ppsx@users.noreply.github.com>
1 parent b96f5d3 commit 6471489

3 files changed

Lines changed: 28 additions & 15 deletions

File tree

shared-module/busdisplay/BusDisplay.c

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -325,33 +325,42 @@ static bool _refresh_area(busdisplay_busdisplay_obj_t *self, const displayio_are
325325
}
326326
remaining_rows -= rows_per_buffer;
327327

328-
uint16_t subrectangle_size_bytes;
329-
if (self->core.colorspace.depth >= 8) {
330-
subrectangle_size_bytes = displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
331-
} else {
332-
subrectangle_size_bytes = displayio_area_size(&subrectangle) / (8 / self->core.colorspace.depth);
333-
}
328+
#if CIRCUITPY_QSPIBUS
329+
if (is_qspi_bus) {
330+
// QSPI path: fill_area first (overlaps with previous DMA),
331+
// then single-transaction set_region + RAMWR + pixels.
332+
// depth is always 16 here (guarded by is_qspi_bus check above).
333+
uint16_t subrectangle_size_bytes = displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
334334

335-
memset(mask, 0, mask_length * sizeof(mask[0]));
336-
memset(buffer, 0, buffer_size * sizeof(buffer[0]));
335+
memset(mask, 0, mask_length * sizeof(mask[0]));
336+
memset(buffer, 0, buffer_size * sizeof(buffer[0]));
337337

338-
displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
338+
displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
339339

340-
#if CIRCUITPY_QSPIBUS
341-
if (is_qspi_bus) {
342-
// QSPI single-transaction path: set_region + RAMWR + pixels
343-
// in one begin/end, eliminating per-subrectangle transaction
344-
// overhead. begin_transaction waits for any prior async DMA
345-
// to finish, so fill_area above overlaps with previous DMA.
340+
// begin_transaction waits for any prior async DMA to finish,
341+
// so fill_area above overlaps with previous DMA.
346342
displayio_display_bus_begin_transaction(&self->bus);
347343
displayio_display_bus_send_region_commands(&self->bus, &self->core, &subrectangle);
348344
_send_pixels(self, (uint8_t *)buffer, subrectangle_size_bytes);
349345
displayio_display_bus_end_transaction(&self->bus);
350346
} else
351347
#endif
352348
{
349+
// Non-QSPI path: original ordering preserved exactly.
353350
displayio_display_bus_set_region_to_update(&self->bus, &self->core, &subrectangle);
354351

352+
uint16_t subrectangle_size_bytes;
353+
if (self->core.colorspace.depth >= 8) {
354+
subrectangle_size_bytes = displayio_area_size(&subrectangle) * (self->core.colorspace.depth / 8);
355+
} else {
356+
subrectangle_size_bytes = displayio_area_size(&subrectangle) / (8 / self->core.colorspace.depth);
357+
}
358+
359+
memset(mask, 0, mask_length * sizeof(mask[0]));
360+
memset(buffer, 0, buffer_size * sizeof(buffer[0]));
361+
362+
displayio_display_core_fill_area(&self->core, &subrectangle, mask, buffer);
363+
355364
// Can't acquire display bus; skip the rest of the data.
356365
if (!displayio_display_bus_is_free(&self->bus)) {
357366
return false;

shared-module/displayio/bus_core.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ void displayio_display_bus_end_transaction(displayio_display_bus_t *self) {
101101
self->end_transaction(self->bus);
102102
}
103103

104+
#if CIRCUITPY_QSPIBUS
104105
// Send column/row window commands. Caller must already be in a transaction.
105106
void displayio_display_bus_send_region_commands(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area) {
106107
uint16_t x1 = area->x1 + self->colstart;
@@ -210,6 +211,7 @@ void displayio_display_bus_send_region_commands(displayio_display_bus_t *self, d
210211
self->send(self->bus, DISPLAY_DATA, chip_select, data, data_length / 2);
211212
}
212213
}
214+
#endif
213215

214216
void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area) {
215217
uint16_t x1 = area->x1 + self->colstart;

shared-module/displayio/bus_core.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,11 @@ void displayio_display_bus_end_transaction(displayio_display_bus_t *self);
4949

5050
void displayio_display_bus_set_region_to_update(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area);
5151

52+
#if CIRCUITPY_QSPIBUS
5253
// Send column/row window commands within an already-open transaction.
5354
// Caller must have called displayio_display_bus_begin_transaction() first.
5455
void displayio_display_bus_send_region_commands(displayio_display_bus_t *self, displayio_display_core_t *display, displayio_area_t *area);
56+
#endif
5557

5658
void release_display_bus(displayio_display_bus_t *self);
5759

0 commit comments

Comments
 (0)