8000 rp2/rp2_pio: Add fifo_join support for PIO. by tjvr · Pull Request #7093 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

rp2/rp2_pio: Add fifo_join support for PIO. #7093

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

rp2/rp2_pio: Add fifo_join support for PIO. #7093

wants to merge 1 commit into from

Conversation

tjvr
Copy link
Contributor
@tjvr tjvr commented Apr 2, 2021 •
edited
Loading

This PR adds support for joining the 4-word RX and TX FIFOs on the RP2040's PIOs into a single 8-word FIFO.

I've added JOIN_{RX,TX,NONE} constants to the PIO object, and set the appropriate bits in shiftctrl, as @dpgeorge suggested.

Resolves #6854.

I tested this with the following program:

from machine import Pin
from rp2 import PIO, StateMachine, asm_pio

@asm_pio(
    fifo_join=PIO.JOIN_TX,
)
def prog():
    nop()

sm = StateMachine(0, prog)

for i in range(8):
    sm.put(i)
    print(i)

The program pushes words into an inactive StateMachine until it blocks because the machine's TX FIFO is full.

Without fifo_join, it prints 0-3 and then blocks. With fifo_join, it prints 0-7 and then exits.

With fifo_join=PIO.JOIN_RX, it blocks without printing anything, since the TX buffer has zero size and always reports being full.

@dpgeorge
Copy link
Member

Thanks for this patch, it's nice and simple and adds good functionality!

IMO the new constants would be better named as JOIN_NONE, JOIN_TX and JOIN_RX. My reasoning:

  • In MicroPython it's generally better to make things smaller: it saves ROM/flash, RAM, processing time, transmission time, typing time (and maybe other things). Every little bit counts.
  • In this case there's not going to be any name clash using a shorter name, the PIO namespace is not very large and its scope will not increase.
  • In this case there's no loss in readability because the word FIFO is already there in the keyword argument, eg fifo_join=PIO.JOIN_RX.
  • It matches the SHIFT_LEFT and SHIFT_RIGHT constants (they aren't SHIFTDIR_SHIFT_LEFT).

@kevindawson kevindawson mentioned this pull request Apr 14, 2021
Closed
@tjvr
Copy link
Contributor Author
tjvr commented Apr 16, 2021

Fair enough, some very thorough reasoning!

@tjvr tjvr force-pushed the rp2-pio-fifo-join branch 2 times, most recently from b9e5fc0 to e7c778f Compare April 16, 2021 09:38
@tjvr
rp2/rp2_pio: Add fifo_join support for PIO.
641e253 
The PIO state machines on the RP2040 have 4 word deep TX and RX FIFOs.
If you only need one direction, you can "merge" them into either a
single 8 word deep TX or RX FIFO.

We simply add constants to the PIO object, and set the appropriate bits
in `shiftctrl`.

Resolves micropython#6854.

Signed-off-by: Tim Radvan <tim@tjvr.org>
@tjvr tjvr force-pushed the rp2-pio-fifo-join branch from e7c778f to 641e253 Compare April 16, 2021 09:42
@tjvr
Copy link
Contributor Author
tjvr commented Apr 16, 2021

Updated 👍

@tjvr tjvr mentioned this pull request Apr 16, 2021
Closed
@dpgeorge
Copy link
Member

Thanks for updating. Merged in f842a40 with a very minor change to place the new constants just after the existing SHIFT constants (for logical grouping).

@dpgeorge 905F dpgeorge closed this Apr 16, 2021
tjvr added a commit to tjvr/micropython that referenced this pull request Apr 17, 2021
@tjvr
docs/rp2: Document fifo_join argument.
04af773 
This was added in micropython#7093.
@kevindawson kevindawson mentioned this pull request May 2, 2021
Closed
@megazhuk megazhuk mentioned this pull request Nov 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
port-rp2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rp2 PIO: Allow merging Tx/Rx FIFOs together
2 participants
@tjvr @dpgeorge
0