Fix BusDisplay transaction handling#10571
Fix BusDisplay transaction handling#10571JamesWhitlock wants to merge 1 commit intoadafruit:mainfrom
Conversation
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
Thanks for the PR! I'm not sure this is the way we want to do it though. (More inline)
| 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); | ||
| } |
There was a problem hiding this comment.
I'm worried all of these deletes are going to break something due to changing chip select issues. I wonder if we should split out the bus locking from chip select.
There was a problem hiding this comment.
Makes sense, I was unaware that some display drivers are fussy about when chip selects are toggled. I can change it back into individual bus transactions for now, and spin in any cmd sequences. This would eliminate the possibility of regressions.
There was a problem hiding this comment.
That sounds like a good plan. Thanks!
|
I think I addressed this as part of #10887. Thanks for bringing it up. |
3 participants
Fixes #10569
I've tested on a m5stack_cores3 with no apparent regressions on display handling. This strangely does not resolve #10536 so some other issue must also be present, however I can see on an oscilloscope that the SPI bus is now being shared correctly between busdisplay and sdcardio. One thing to note is running spi = board.SPI(); spi.try_lock() will now prevent the screen from being updated until the lock is released - this is a limitation of the the hardware.
I have also applied the same/similar fixes to EPaperDisplay - however I do not have hardware to verify this.