At Sat, 14 Aug 2010 09:53:38 +0200 (CEST), Jaroslav Kysela wrote:

On Fri, 13 Aug 2010, Takashi Iwai wrote:

...

...

http://www.alsa-project.org/main/index.php/XRUN_Debug is a good start to see if the virtual machine does receive interrupts in the right time.

Actually, there is no real interrupt, but just the virtual one, thus it can be never accurate. So, rather the question is how the driver is tolerant for sloppy IRQ updates.

Usually, when the period size is large and the buffer have many periods (not two), the delayed update works more stably.

I know. The current position checking in pcm_lib.c should handle delayed updates, but it does not handle well the multiple elapsed() calls merged to one time (double interrupt acknowledges). In some cases, the code things that buffer_size boundary were crossed, thus the hw_ptr position update fails.

I wanted to force the driver to be responsible to call the elapsed() calls at the right time. But it's true that the interrupt controller and other operating system contrainsts and behaviour might influence the interrupt processing which is more visible in the virtual environment.

Thinking more about it, we need probably another quick check against different timing source to check properly for the buffer_size crossing.

What about this patch (the hw_base gets updates in in_interrupt only when delta for next jiffies is bigger than (buffer jiffies / 2)). It should handle all double acks from the interrupt callbacks:

Looks good. We need some tests, though. Maybe we can put some randomness in dummy driver...

thanks,

Takashi

diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 85f1c6b..dfd9b76 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -278,6 +278,7 @@ struct snd_pcm_runtime { snd_pcm_uframes_t hw_ptr_base; /* Position at buffer restart */ snd_pcm_uframes_t hw_ptr_interrupt; /* Position at interrupt time */ unsigned long hw_ptr_jiffies; /* Time when hw_ptr is updated */

• unsigned long hw_ptr_buffer_jiffies; /* buffer time in jiffies */ snd_pcm_sframes_t delay; /* extra delay; typically FIFO size */

  /* -- HW params -- */

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index e23e0e7..a1707cc 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -334,11 +334,15 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream, /* delta = "expected next hw_ptr" for in_interrupt != 0 */ delta = runtime->hw_ptr_interrupt + runtime->period_size; if (delta > new_hw_ptr) {

• 	hw_base += runtime->buffer_size;
  

• 	if (hw_base >= runtime->boundary)
  

• 		hw_base = 0;
  

• 	new_hw_ptr = hw_base + pos;
  

• 	goto __delta;
  

• 	/* check for double acknowledged interrupts */
  

• 	hdelta = jiffies - runtime->hw_ptr_jiffies;
  

• 	if (hdelta > runtime->hw_ptr_buffer_jiffies/2) {
  

• 		hw_base += runtime->buffer_size;
  

• 		if (hw_base >= runtime->boundary)
  

• 			hw_base = 0;
  

• 		new_hw_ptr = hw_base + pos;
  

• 		goto __delta;
  

• 	}
  

  } } /* new_hw_ptr might be lower than old_hw_ptr in case when */

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index a3b2a64..b5dae26 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -864,6 +864,8 @@ static void snd_pcm_post_start(struct snd_pcm_substream *substream, int state) struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_trigger_tstamp(substream); runtime->hw_ptr_jiffies = jiffies;

• runtime->hw_ptr_buffer_jiffies = (runtime->buffer_size * HZ) /

• 					    runtime->rate;
  

  runtime->status->state = state; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && runtime->silence_size > 0)

---

Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.