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@xxxxxxxx> > Linux Kernel Sound Maintainer > ALSA Project, Red Hat, Inc. > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
