Skip to content
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 RP2040 displayio.ParallelBus Support #4130

Merged
merged 14 commits into from
Mar 16, 2021

Conversation

gamblor21
Copy link
Member

Adding ParallelBus support for the RP2040.

Modified the standard bus to accept frequency to tune the PIO speed. Modified other ports to ignore this parameter which is optional.

Tested on the Raspberry Pi Pico and the 3.5" TFT (product 2050). Tested the modified build on a feather M4 but without a display attached (just to ensure the parameter did not fail)

@tannewt tannewt self-requested a review February 5, 2021 19:55
@tannewt tannewt added this to the 6.2.0 milestone Feb 5, 2021
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks good overall. Just a few minor comments.

ports/raspberrypi/common-hal/displayio/ParallelBus.c Outdated Show resolved Hide resolved
ports/raspberrypi/common-hal/displayio/ParallelBus.c Outdated Show resolved Hide resolved
ports/raspberrypi/common-hal/displayio/ParallelBus.c Outdated Show resolved Hide resolved
ports/raspberrypi/common-hal/rp2pio/StateMachine.c Outdated Show resolved Hide resolved
shared-bindings/displayio/ParallelBus.c Outdated Show resolved Hide resolved
@tannewt
Copy link
Member

tannewt commented Feb 23, 2021

#4251 includes the new init code. Sorry for blocking this!

@jepler
Copy link
Member

jepler commented Feb 25, 2021

#4251 has been merged so maybe that un-sticks this PR?

@gamblor21
Copy link
Member Author

#4251 includes the new init code. Sorry for blocking this!

Hope to get to it this weekend, shouldn't take long (once I figure out how to pull in that other PR to my branch)

@gamblor21
Copy link
Member Author

gamblor21 commented Mar 7, 2021

Tested on the Feather RP2040 and works.

@jepler
Copy link
Member

jepler commented Mar 7, 2021

I think this is really close, but there was one of @tannewt's suggestions that looks like it didn't get incorporated yet.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing that may be a 2x speedup :-) Thanks!

ports/raspberrypi/common-hal/displayio/ParallelBus.c Outdated Show resolved Hide resolved
jepler
jepler previously approved these changes Mar 12, 2021
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@gamblor21
Copy link
Member Author

Thank you!

Looks like a change committed after my last rebase changed the state machine construction. I'll have to fix that and the build is failing.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you!

@gamblor21 gamblor21 merged commit 1be5ca7 into adafruit:main Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants