rp2/i2s: Add I2S protocol support.

[miketeachman]

This PR adds I2S protocol support for the RP2 port:

• I2S API consistent with STM32 and ESP32 ports
• I2S configurations supported:
  • master transmit and master receive
  • 16-bit and 32-bit sample sizes
  • mono and stereo formats
  • sampling frequency
  • 3 modes of operation:
    • blocking
    • non-blocking with callback
    • uasyncio
  • internal ring buffer size
• MicroPython documentation
• tested on the following development boards:
  • Raspberry Pi Pico
• I2S examples repo to support this PR:
  • https://github.com/miketeachman/micropython-i2s-examples
• rp2 build metric changes wrt master branch: text(+4552), data(0), bss(+8)

code reviews appreciated !

[odewdney]

Hi, I have an number of observations..
It looks like you are claiming the dma irq in main, when other systems the user might use might want the irqs - should they be setup when the user first creates the i2s object?
It looks like there could be a race condition between the irq handler and the user blocking read/write code ( and other ) with the head/tail pointer.
Would a scatter/gather list of dma buffers, with the second chained dma controller updating the addr/len of the first, and buffers moving between user owned and dma owned, allow for less race conditions, tighter byte copying loop in/out of the buffers, and allow of easier configuration of the buffer sizes in the constructor - supplying number of buffers and buffer size. A downside would be slight addition of latency of one buffer.

[miketeachman]

@odewdney Thanks for taking a look at this code.

It looks like you are claiming the dma irq in main, when other systems the user might use might want the irqs - should they be setup when the user first creates the i2s object?

That is definitely a better approach. I'll investigate making that change.

It looks like there could be a race condition between the irq handler and the user blocking read/write code ( and other ) with the head/tail pointer.

I took another look at the ring buffer implementation and can't pick out any race conditions. The ring buffer is a SPSC implementation which is designed to be inherently thread safe. Is it possible to give an example where a race condition exists?

[odewdney]

Yes, there shouldnt be a race condition there as long as the user doesnt mix blocking and non-blocking code :)
I was wondering if there was a way to avoid the byte-by-byte handling ( on a 32 bit processor ), the read operation seems to check for buffer to be multiple of sample size, but the write doesnt. and can you control of the buffers are 32-bit aligned.

[miketeachman]

The copy routines could definitely be improved. I considered making the sample copy routines more efficient, but some timing profiling showed that these routines don't significantly impact performance. Also, the byte oriented copies to/from the ring buffer simplify the implementation. I decided that any optimization is not warranted at this time. Audio processing, even at CD quality rates is not overly processing intensive for a 133MHz processor. If video was being processed these copy routines would need to be improved. By far, the biggest bottleneck for RP2 audio applications is the slow speed of SD card access. Any optimization efforts should first be directed towards this area. e.g. create a C-based SD card interface (machine_sdcard.c) for the RP2 port.

[miketeachman]

new commit to manage DMA IRQs on a per-I2S-object basis, as discussed here.

[dpgeorge]

Thanks for this. Great to see it matching the API of the existing stm32/esp32 implementations!

I don't have the time to test this on hardware, and I'll assume it works as designed.

@miketeachman do you consider this ready to merge?

[miketeachman]

It's had 3 weeks of scrutiny so I'd say merge it. I've done a lot of dev testing using the same I2S examples as the STM32 and ESP32 and I haven't found any issues. I'll keep an eye on reported issues and fix bugs as they arise.

[dpgeorge]

Merged in b6dbbbe.

Great work thank you!