I admit that following code instruction by instruction for checking if an interrupt trigger could bring to problems is tricky. The use of a double buffer (next1_s and next2_s) is obscure at first. Maybe it could be simpler to disable and enable interrupts, without worrying about next pending audio stream. --- audio.c --- typedef struct { const uint16_t *samples; size_t size; uint16_t *s; } AudioStream; static AudioStream curr_s; // current stream in playing static AudioStream *curr; // used mainly as atomic flags void Timer_ISR(void) { uint16_t sample; bool dac_refresh = false; if (curr) { // we are playing a stream if (curr->s == NULL) { curr->s = curr->samples; } sample = *(curr->s); dac_refresh = true; if (++curr->s - curr->samples == curr->size) { curr = NULL; // last sample processed } } if (dac_refresh) DAC_VAL = sample; } void play(const uint16_t *samples, size_t size) { disableInterrupts(); // start of critical section curr_s.samples = samples; curr_s.size = size; curr_s.s = NULL; curr = &curr_s; enableInterrupts(); // end of critical section } --- audio.c --- In this solution, I don't need the init member in AudioStream structure: I use s member to understand if it is initialized or not. Here I don't need volatile for any variables managed in play(): after entering critical section, all the variables can't be changed asyncronously (interrupts are disabled), so their access can be normally optimized. [It is possible to disable only the Timer interrupt and not all the interrupts.] The only problem is to avoid optimizer to move instructions from inside the critical section to outside (before disableInterrupts() or after enableInterrupts()). This is one thing I never taked care until David explanation. One solution to avoid reordering is to embed a memory barrier in disableInterrupts() and enableInterrupts(). The memory barrier guarantees that the memory accesses before and after it (inside critical section in our case) aren't optimized (reordered), even if curr_s and curr aren't volatile (David, is this right?). If the code is for Atmel AVR architecture, I think it is better to use macros from <util/atomic.h>: #include <util/atomic.h> ... void play(const uint16_t *samples, size_t size) { ATOMIC_BLOC(ATOMIC_FORCEON) { curr_s.samples = samples; curr_s.size = size; curr_s.s = NULL; curr = &curr_s; } } The macro ATOMIC_BLOCK automatically disable interrupts at the beginning (using a memory barrier) and enable interrupt at the end (using again a memory barrier). Incidentally, I noticed a strange thing in <util/atomic.h> and <interrupt.h>: --- <interrupt.h> --- ... #define sei() __asm__ __volatile__ ("sei" ::: "memory") ... --- <interrupt.h> --- --- <util/atomic.h> --- ... static __inline__ void __iSeiParam(const uint8_t *__s) { sei(); __asm__ volatile ("" ::: "memory"); (void)__s; } ... --- <util/atomic.h> --- Why adding a memory barrier after sei() in __iSeiParam() if sei() macro alreay has a memory barrier? Anyway, reading <util/atomic.h> is very instructive, because many tricks of C language and gcc compiler are used there (for example, cleanup attribute of a variable). If the code is for Cortex-Mx architectures, macros proposed by Tim or David can be used. I don't know if it is necessary to add DSB and ISB hw memory barriers after "cpsid i", to be sure pending interrupts won't really trigger after disabling interrupts instruction.
[Second solution: disabling interrupts] Re: Help with interrupt software routine and non-atomic operations
Started by ●May 2, 2016
Reply by ●May 2, 20162016-05-02