Reply by pozz May 2, 20162016-05-02
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.