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

AudioPlaySdWav: buffer alignment and irq priority #294

Closed

Conversation

tsandmann
Copy link
Contributor

Currently there are two problems using AudioPlaySdWav together with the latest version of the SD library:

  • for DMA data transfers from the SDHC, the buffer supplied to File::read() has to be 4-byte aligned (see check alignment for DMA buffers SD#17). This PR enforces a 4-byte alignment of AudioPlaySdWav::buffer to fix this.
  • AudioPlaySdWav::begin() disables interrupts completely while calling SD.open(). As this includes the DMA transfer complete interrupt, SD.open() never returns (because dmaDone is not updated). As a workaround, this PR only disables interrupts with a lower priority than SDHC_IRQ.

@manitou48
Copy link

the same problem with IRQ and SD.open() occurs in play_sd_raw.cpp

@tsandmann
Copy link
Contributor Author

Pull request updated to cover AudioPlaySdRaw as well.

@FrankBoesing
Copy link
Contributor

Any Idea why it disables the interrupts?

@manitou48
Copy link

I'm not sure why interrupts are disabled. my hack was to remove the disable_irq() before SD.open(), and that seemed to work for my testing.

https://forum.pjrc.com/threads/54711-Teensy-4-0-First-Beta-Test?p=207700&viewfull=1#post207700

@FrankBoesing
Copy link
Contributor

Hm, Paul?
Is it to prevent other Sd.open(), called by interrupts ?

@FrankBoesing
Copy link
Contributor

In this case it is probably better to prevent this in open()

@tsandmann
Copy link
Contributor Author

Any Idea why it disables the interrupts?

AudioStream::update() is called asynchronously from a software interrupt. Its overloads may use read() / write() on a file handle (as e.g. AudioPlaySdWav::update() does) and the filesystem cache of the SD card library is shared between files, but not thread- / interrupt-save IIRC. As (at least) SD.open() also uses this cache, software interrupts have to be disabled during SD.open().

It might be sufficient to just disable IRQ_SOFTWARE instead of all interrupts of lower priority than IRQ_SDHC, if no other ISR uses SD related stuff, but I didn't double check this.

@manitou48
Copy link

This bug arose when the SD lib (SDHC portion) was rewritten to use DMA and DMA interrupt. But the SD logic in NXP_SDHC.cpp just spins on dmaDone for completion. dmaDone is set in the DMA ISR. So another fix would be to change the SD lib to not use DMA ISR, but to spin on the DMA status or SDHC status flag to check for read/write completion. Then no changes would be needed in Audio lib. The previous version of SD lib polled the SDHC status for completion.

@tsandmann
Copy link
Contributor Author

No longer necessary, it's fixed with 6369a6a.

@tsandmann tsandmann closed this Nov 18, 2020
@tsandmann tsandmann deleted the bugfixes/dma-align_irq branch November 21, 2020 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants