[alsa-devel] Timer instability

[Takashi Iwai]

At Fri, 20 Feb 2009 02:22:20 +0100,
Lennart Poettering wrote:
> 
> On Thu, 19.02.09 07:52, Takashi Iwai (tiwai at suse.de) wrote:
> 
> > 
> > At Thu, 19 Feb 2009 03:46:11 +0100,
> > Lennart Poettering wrote:
> > > 
> > > Heya!
> > > 
> > > As you might now PulseAudio schedules audio by system timers, not by
> > > sound card interrupts. Unfortunately there are are couple of issues
> > > with the reliability of the timing/buffering information we get from
> > > ALSA. 
> > > 
> > > For example, on an ens1371 I have tracked down the following issue: I
> > > use the full buffer that the sound card provides (64K, i.e. 371ms). I
> > > sleep for about 350 ms, then fill up again, and sleep 350ms, and so
> > > and so on. Everytime I wake up I check snd_pcm_avail() and write
> > > exactly as much as this call tells me to. I also listen to POLLOUT
> > > events on the fd, however I call snd_pcm_sw_params_set_period_event(,
> > > 0) for it to make sure I actually don't get any real events on that.
> > > 
> > > After a couple of minutes of running this loop I can reliably
> > > reproduce that _avail() tells me I should fill the buffer up, I do so,
> > > query it again and it shows the buffer is filled up. I then go to
> > > sleep but immediately get POLLIN (although I shouldnt, given that I
> > > called snd_pcm_sw_params_set_period_event()), wake up again, call
> > > snd_pcm_avail() and according to it the buffer ran completely
> > > empty. The poll() took less than 1 ms time, and the last thing I did
> > > before going to sleep was checking that the buffer was full. So
> > > certainly ALSA is lying to me here... The buffer couldn't have run
> > > empty in less than 1 ms!
> > > 
> > > The same with real numbers as one example:
> > > 
> > >   1. ... we are woken up
> > >   2. _avail() returns 15496 samples (i.e. 350ms to fill up)
> > >   3. we gather and write 15496 samples
> > >   4. _avail() returns 48 samples, which we consider "filled up enough" to go to sleep
> > >   5. we call poll()
> > >   6. after < 1ms we are woken up by a POLLIN (which we actually thought we had explicitly disabled)
> > >   7. _avail() returns 16448 samples (i.e. 372ms to fill up -- larger than the actual buffer of 16384 samples!)  
> > >   
> > > This smells a lot like some kind of overflow to me, given that in every
> > > case where I could reproduce this it was possible to 'fix' the issue
> > > by substracting the buffer size from the _avail() after the
> > > poll(). After that substraction the value started to make sense
> > > again. (i.e. in the example above it then becomes 64 samples, which is
> > > reasonable after sleep less than 1ms I guess).
> > > 
> > > The stop threshold is set to the boundary here btw.
> > > 
> > > If necessary I could extract a test case for this, but my hope that
> > > you guys might have an idea what goes on without a test case, given
> > > that this smells so "overflowy" ;-)
> > > 
> > > And of course, how come ALSA triggers POLLIN if I asked it not to?
> > 
> > Are you using *_period_event() stuff?
> 
> Yes, I am. But ignore that part for now. I have now commented the use
> of that call. Now I certainly get more POLLOUTs as expected, but the
> real problem stays: after a few minutes _avail() will suddenly jump
> from next to zero to more then the hwbuf size in less than 1ms without
> any further inteference and with a buffer size of 350ms! There is
> something really wrong with the behaviour of _avail().
> 
> I can reproduce this only on ens1371 for now.

The ens1371 driver itself is damn simple.  The pointer callback just
returns the read value.  So, it implies that it's basically a hardware
issue.

Does the patch below have any influence?

> > Unless you use dmix, this is the only place where strange POLLIN may
> > come in.  There are many new hacks there...
> 
> There is no plug in used but "hw".

Yes but *_period_event() is implemented in "hw".


thanks,

Takashi

---
diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c
index 18f4d1e..3a7739b 100644
--- a/sound/pci/ens1370.c
+++ b/sound/pci/ens1370.c
@@ -991,15 +991,36 @@ static int snd_ensoniq_capture_prepare(struct snd_pcm_substream *substream)
 	return 0;
 }
 
+#define MAX_PTR_TIMEOUT		50
+
+static unsigned int get_cur_ptr(struct ensoniq *ensoniq, unsigned int map,
+				unsigned int reg)
+{
+	unsigned int optr, ptr;
+	int t;
+
+	outl(map, ES_REG(ensoniq, MEM_PAGE));
+	inl(ES_REG(ensoniq, MEM_PAGE));
+	optr = inl(reg);
+	for (t = 0; t < MAX_PTR_TIMEOUT; t++) {
+		ptr = inl(reg);
+		if (ptr == optr)
+			return ptr;
+		optr = ptr;
+	}
+	return ptr;
+}
+
 static snd_pcm_uframes_t snd_ensoniq_playback1_pointer(struct snd_pcm_substream *substream)
 {
 	struct ensoniq *ensoniq = snd_pcm_substream_chip(substream);
 	size_t ptr;
 
 	spin_lock(&ensoniq->reg_lock);
-	if (inl(ES_REG(ensoniq, CONTROL)) & ES_DAC1_EN) {
-		outl(ES_MEM_PAGEO(ES_PAGE_DAC), ES_REG(ensoniq, MEM_PAGE));
-		ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, DAC1_SIZE)));
+	if (ensoniq->ctrl & ES_DAC1_EN) {
+		ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_DAC),
+				  ES_REG(ensoniq, DAC1_SIZE));
+		ptr = ES_REG_FCURR_COUNTI(ptr);
 		ptr = bytes_to_frames(substream->runtime, ptr);
 	} else {
 		ptr = 0;
@@ -1014,9 +1035,10 @@ static snd_pcm_uframes_t snd_ensoniq_playback2_pointer(struct snd_pcm_substream
 	size_t ptr;
 
 	spin_lock(&ensoniq->reg_lock);
-	if (inl(ES_REG(ensoniq, CONTROL)) & ES_DAC2_EN) {
-		outl(ES_MEM_PAGEO(ES_PAGE_DAC), ES_REG(ensoniq, MEM_PAGE));
-		ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, DAC2_SIZE)));
+	if (ensoniq->ctrl & ES_DAC2_EN) {
+		ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_DAC),
+				  ES_REG(ensoniq, DAC2_SIZE));
+		ptr = ES_REG_FCURR_COUNTI(ptr);
 		ptr = bytes_to_frames(substream->runtime, ptr);
 	} else {
 		ptr = 0;
@@ -1031,9 +1053,10 @@ static snd_pcm_uframes_t snd_ensoniq_capture_pointer(struct snd_pcm_substream *s
 	size_t ptr;
 
 	spin_lock(&ensoniq->reg_lock);
-	if (inl(ES_REG(ensoniq, CONTROL)) & ES_ADC_EN) {
-		outl(ES_MEM_PAGEO(ES_PAGE_ADC), ES_REG(ensoniq, MEM_PAGE));
-		ptr = ES_REG_FCURR_COUNTI(inl(ES_REG(ensoniq, ADC_SIZE)));
+	if (ensoniq->ctrl & ES_ADC_EN) {
+		ptr = get_cur_ptr(ensoniq, ES_MEM_PAGEO(ES_PAGE_ADC),
+				  ES_REG(ensoniq, ADC_SIZE));
+		ptr = ES_REG_FCURR_COUNTI(ptr);
 		ptr = bytes_to_frames(substream->runtime, ptr);
 	} else {
 		ptr = 0;