Skip to content

ports/rp2: Provide direct memory access to UART FIFOs. #13378

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickovs
Copy link
Contributor

@nickovs nickovs commented Jan 7, 2024

This PR is similar in intent to #7650 but for UARTs, providing a way to easily get direct access to the FIFO register of a given UART so that the UART object can be passed directly to the read or write argument on a DMA controller.

The change introduces a new MICROPY_PY_MACHINE_UART_FIFO_BUFFER configuration in mpconfigport.h because parts of the machine.UART code are implemented in the extmod directory rather than the port directory.

Below is a simple example of how this can be used. (For the purpose of the example, pins 16 and 17 are connected together.)

>>> from machine import UART
>>> from rp2 import DMA
>>> u = UART(0, baudrate=1_000_000, tx=16, rx=17)
>>> recv_buf = bytearray(256)
>>> send_dma = DMA()
>>> send_conf = send_dma.pack_ctrl(size=0, inc_write=False, treq_sel=20)
>>> recv_dma = DMA()
>>> recv_conf = recv_dma.pack_ctrl(size=0, inc_read=False, treq_sel=21)
>>> recv_dma.config(read=u, write=recv_buf, ctrl=recv_conf, count=len(recv_buf), trigger=True)
>>> send_dma.config(read=b"Hello there!", write=u, ctrl=send_conf, count=12)
>>> recv_buf[:12]
bytearray(b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00')
>>> send_dma.active(1)
False
>>> recv_buf[:12]
bytearray(b'Hello there!')

Signed-off-by: Nicko van Someren <nicko@nicko.org>
Copy link

github-actions bot commented Jan 7, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:   +16 +0.005% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@nickovs
Copy link
Contributor Author

nickovs commented Jan 7, 2024

@dpgeorge This works just fine for TX. Currently on the RX side there is a race condition, for which I'd appreciate your suggestions.

Incoming data will raise an interrupt but in general the DMA controller will process the bytes much faster than the CPU can respond, so for small amounts of data this is not an issue. The problem comes when more data arrives after the interrupt handler has started; in this case the interrupt handler and the DMA controller will both try to read data from the FIFO, with some bytes going to each. It looks like we need a way to suppress the RX interrupt handler.

One possible (fairly clean) solution to this would be to add a .close() method to the UART class that allows the user to continue to reference a UART but disables processing through the software path, so that the DMA controller can have exclusive access. I would like to hear your thoughts before proceeding.

Copy link

codecov bot commented Jan 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9feb068) 98.40% compared to head (89b6f89) 98.36%.
Report is 42 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13378      +/-   ##
==========================================
- Coverage   98.40%   98.36%   -0.05%     
==========================================
  Files         159      159              
  Lines       21088    21088              
==========================================
- Hits        20752    20743       -9     
- Misses        336      345       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nickovs
Copy link
Contributor Author

nickovs commented Jan 7, 2024

One possible (fairly clean) solution to this would be to add a .close() method to the UART class

An alternative would be to add a .active() method, as used by a bunch of other devices, and just have it enable or disable the UART interrupt handler.

@robert-hh
Copy link
Contributor

Having the DMA to transfer the data is just one element of the game. Information about how much data has been transferred is equally important, especially for receive.
That said I would not consider DMA for UART as a huge benefit over the IRQ based solution. For SPI the speed gain is more valuable.

@nickovs
Copy link
Contributor Author

nickovs commented Jan 7, 2024

@robert-hh The amount of data received can trivially be read by looking at the value in the attribute dma.count since this gets decremented for each character transferred.

@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

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.

4 participants