-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Feature: I2S callback on DMA read #4205
Conversation
Fix I2S base frequency
Less is more
The next rev is going to be bugfix, with new cool features like this for the follow-on. Thx! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple little things, please.
cores/esp8266/core_esp8266_i2s.c
Outdated
ETS_SLC_INTR_ENABLE(); | ||
} | ||
} | ||
|
||
void ICACHE_FLASH_ATTR i2s_set_callback(void (*callback) (void)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that 2.4.1 is out this can get into the merge queue. Only minor change I would request is to remove the ICACHE_FLASH_ATTR from the set-callback function. There's no need for this to be kept in precious IRAM.
@@ -51,6 +51,7 @@ bool i2s_write_lr(int16_t left, int16_t right);//combines both channels and call | |||
bool i2s_is_full();//returns true if DMA is full and can not take more bytes (overflow) | |||
bool i2s_is_empty();//returns true if DMA is empty (underflow) | |||
int16_t i2s_available();// returns the number of samples than can be written before blocking | |||
void i2s_set_callback(void (*callback) (void)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(continuing prior thought)
The actual callback function passed in, though, needs to be in IRAM. A comment in the code to that effect would be appreciated here, so it's obvious to people coming into it that it's an ISR and needs to be very special...
The requested changes are implemented, lets hope this gets merged soon :)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. Just updated, if it clears CI I can merge later today.
Thanks, squashed and committed. |
This will allow to fill the DMA buffer on demand using callback function (or at least set a flag in the callback and do stuff later outside of the interrupt routine). This will save a lot of overhead because the only way to do it now is to check in a loop if there's empty buffer waiting to be filled.