-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for the Pimoroni Tufty 2040 #53
Conversation
boards/pimoroni-tufty2040/Cargo.toml
Outdated
|
||
[features] | ||
# This is the set of features we enable by default | ||
default = ["boot2", "rt", "critical-section-impl", "rom-func-cache"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tufty2040 is new enough that it should be safe to always enable rom-v2-intrinsics
pins.lcd_rd.into_push_pull_output_in_state(PinState::High); | ||
|
||
/* | ||
Example of using GPIO data lines: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think anyone will use these pins as GPIO?
If so, it would be better to document this use-case elsewhere, not in an example.
I think everyone will use your PIO+DMA driver and never touch the display pins directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair question - it was useful to me whilst getting everything working, but given just the built-in peripherals I don't know that anyone would choose to avoid the PIO unit & DMA channel.
Do you want me to consign the GpioDataLines
implementation to history? (If I leave the DisplayDataLines
trait, it's easy enough to implement out-of-tree.)
.program st7789_parallel | ||
.side_set 1 | ||
|
||
out pins, 32 side 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's only 8 LCD data pins, why specify 32 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judging by the datasheet, the shift register is always filled by 32-bits and since we're using DMA to write a byte at a time, we want to clear the whole thing.
Also improve the description of the demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay getting back to re-reviewing this.
LGTM, thanks for the PR!
* Add support for Tufty 2040 * Rename tufty_blinky to tufty_demo Also improve the description of the demo. * Enable rom-intrinsics-v2 by default on tufty2040
This includes pin numbering and bindings to the ST7789 display.
There is one pretty painful issue: DMA is generally not used for
embedded_graphics
commands, since most commands are generated via an iterator. (This is particularly noticeable at startup, as the st7789 library turns the display on without leaving time for us to clear the screen buffer, which takes a while without DMA.)However, I don't feel that solving that issue is under the remit of
rp2040-hal
. Ideallyst7789
could do with hooks to allow DMA for long copy operations and itsinit()
function needs to be broken up.