I'm sorry, this post should be as an answer to another thread.
Reply by pozz●May 2, 20162016-05-02
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.