audiocore-i2s-driver #3

Merged

matthias merged 8 commits from audiocore-i2s-driver into main 2024-07-29 18:48:19 +00:00

Conversation 3 Commits 8 Files Changed 22 +919 -10

matthias commented 2024-06-01 12:31:35 +00:00

Owner

Copy Link

No description provided.

matthias self-assigned this 2024-06-01 12:31:35 +00:00

matthias added 5 commits 2024-06-01 12:31:36 +00:00

Update .gitignore ... 7fcbbf2bed

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

Add audiocore module and I2S audio driver ... 6c687d2841

This is the skeleton of the code that will run on core1 to perform the
audio decoding and output. In this initial commit, the module
infrastructure, starting code on core1, and the I2S audio driver with
DMA are implemented.

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

Add manual test script ... bd641775d3

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

Add clang-format ... 18c4f54b8b

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

Add flake8 CI checks ... 3c3ac33e36

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

matthias force-pushed audiocore-i2s-driver from 3c3ac33e36 to 170861f9e9 2024-06-01 12:33:32 +00:00 Compare

matthias added 1 commit 2024-06-01 14:24:05 +00:00

Add unit test infrastructure 954c85d5d2

matthias force-pushed audiocore-i2s-driver from 954c85d5d2 to 872b3f1a61 2024-06-01 14:26:59 +00:00 Compare

matthias force-pushed audiocore-i2s-driver from 872b3f1a61 to 51f2b8922f 2024-06-01 14:31:02 +00:00 Compare

matthias added 1 commit 2024-06-01 15:28:04 +00:00

audiocore: Add unit tests for audiocore ... 91e08b6c8e

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

matthias added 1 commit 2024-06-01 15:41:52 +00:00

audiocore: Comment python API, some cleanup ... c2074e136a

Signed-off-by: Matthias Blankertz <matthias@blankertz.org>

matthias changed title from WIP: audiocore-i2s-driver to audiocore-i2s-driver 2024-06-01 15:42:00 +00:00

matthias requested review from stefank 2024-06-01 15:42:09 +00:00

matthias force-pushed audiocore-i2s-driver from c2074e136a to ee5ea90e0a 2024-06-02 09:58:27 +00:00 Compare

stefank changed title from audiocore-i2s-driver to WIP: audiocore-i2s-driver 2024-06-09 18:49:55 +00:00

stefank changed title from WIP: audiocore-i2s-driver to audiocore-i2s-driver 2024-06-09 19:01:15 +00:00

stefank commented 2024-06-09 19:42:31 +00:00

Member

Copy Link

bool i2s_init(int out_pin, int sideset_base, int samplerate)
Is it clear to the user that setting sideset_base means initializing gpio sideset_base and sideset_base+1?

We should also note somewhere that our assumption is that already an underfull DMA transfer is counted as an underrun (i2s.c, l. 33ff.). I am thinking ahead a bit and assume that we will write a bit of technical documentation at some point :)

Question regarding void dma_isr(void):
Currently we dma_channel_acknowledge_irq1 the interrupt first, then call spin_lock_blocking(), which disables interrupts.
Is this safe? Or in other words: are we sure we don't use nested interrupts? Imho we should disable interrupts first, then acknowledge the source.
The rp2040 datasheet ch. 2.3.2 states that the NVIC supports interrupt nesting, but it doesn't say anything about en- or disabling nesting. Neither does the sdk docu.

`bool i2s_init(int out_pin, int sideset_base, int samplerate)` Is it clear to the user that setting `sideset_base` means initializing gpio `sideset_base` and `sideset_base+1`? We should also note somewhere that our assumption is that already an underfull DMA transfer is counted as an underrun (i2s.c, l. 33ff.). I am thinking ahead a bit and assume that we will write a bit of technical documentation at some point :) Question regarding `void dma_isr(void)`: Currently we `dma_channel_acknowledge_irq1` the interrupt first, then call `spin_lock_blocking()`, which disables interrupts. Is this safe? Or in other words: are we sure we don't use nested interrupts? Imho we should disable interrupts first, then acknowledge the source. The rp2040 datasheet ch. 2.3.2 states that the NVIC supports interrupt nesting, but it doesn't say anything about en- or disabling nesting. Neither does the sdk docu.

matthias commented 2024-06-12 18:51:48 +00:00

Author

Owner

Copy Link

bool i2s_init(int out_pin, int sideset_base, int samplerate)
Is it clear to the user that setting sideset_base means initializing gpio sideset_base and sideset_base+1?

Not directly, but documented for the external (python) API in module.c:93.

We should also note somewhere that our assumption is that already an underfull DMA transfer is counted as an underrun (i2s.c, l. 33ff.). I am thinking ahead a bit and assume that we will write a bit of technical documentation at some point :)

In the future this buffer will not be filled by the python user, but by the MP3 decoder which makes it less public.

Question regarding void dma_isr(void):
Currently we dma_channel_acknowledge_irq1 the interrupt first, then call spin_lock_blocking(), which disables interrupts.
Is this safe? Or in other words: are we sure we don't use nested interrupts? Imho we should disable interrupts first, then acknowledge the source.
The rp2040 datasheet ch. 2.3.2 states that the NVIC supports interrupt nesting, but it doesn't say anything about en- or disabling nesting. Neither does the sdk docu.

As I understand it, only interrupts with different priorities nest (a higher priority IRQ can interrupt a lower priority handler). By default all interrupts are the same priority, so interrupt nesting should not play a role if not explicitly activated. And I don't know if it is needed in this project.

> `bool i2s_init(int out_pin, int sideset_base, int samplerate)` > Is it clear to the user that setting `sideset_base` means initializing gpio `sideset_base` and `sideset_base+1`? > Not directly, but documented for the external (python) API in module.c:93. > We should also note somewhere that our assumption is that already an underfull DMA transfer is counted as an underrun (i2s.c, l. 33ff.). I am thinking ahead a bit and assume that we will write a bit of technical documentation at some point :) In the future this buffer will not be filled by the python user, but by the MP3 decoder which makes it less public. > > Question regarding `void dma_isr(void)`: > Currently we `dma_channel_acknowledge_irq1` the interrupt first, then call `spin_lock_blocking()`, which disables interrupts. > Is this safe? Or in other words: are we sure we don't use nested interrupts? Imho we should disable interrupts first, then acknowledge the source. > The rp2040 datasheet ch. 2.3.2 states that the NVIC supports interrupt nesting, but it doesn't say anything about en- or disabling nesting. Neither does the sdk docu. As I understand it, only interrupts with different priorities nest (a higher priority IRQ can interrupt a lower priority handler). By default all interrupts are the same priority, so interrupt nesting should not play a role if not explicitly activated. And I don't know if it is needed in this project.

stefank approved these changes 2024-06-19 18:07:36 +00:00

matthias merged commit 6f366ea81c into main 2024-07-29 18:48:19 +00:00

matthias deleted branch audiocore-i2s-driver 2024-07-29 18:48:19 +00:00

matthias referenced this issue from a commit 2024-07-29 18:48:19 +00:00

Merge pull request 'audiocore-i2s-driver' (#3) from audiocore-i2s-driver into main

Sign in to join this conversation.

Reviewers

No Reviewers

stefank

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

flomue matthias stefank

No Assignees matthias

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TonBERRY/tonberry-pico#3