ALSA: Enable PCM hw_ptr_jiffies check only in xrun_debug mode
The PCM hw_ptr jiffies check results sometimes in problems when a hardware doesn't give smooth hw_ptr updates. So far, au88x0 and some other drivers appear not working due to this strict check. However, this check is a nice debug tool, and the capability should be still kept.
Hence, we disable this check now as default unless the user enables it by setting the xrun_debug mode to the specific stream via a proc file.
causes sound to skip while listing to MP3s using mplayer on intel8x0 sound cards. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> ALSA: Enable PCM hw_ptr_jiffies check only in xrun_debug mode
> The PCM hw_ptr jiffies check results sometimes in problems when a > hardware doesn't give smooth hw_ptr updates. So far, au88x0 and some > other drivers appear not working due to this strict check. > However, this check is a nice debug tool, and the capability should be > still kept.
> Hence, we disable this check now as default unless the user enables it > by setting the xrun_debug mode to the specific stream via a proc file.
> causes sound to skip while listing to MP3s using mplayer on > intel8x0 sound cards.
Could you set CONFIG_SND_PCM_XRUN_DEBUG=y and do # echo 1 > /proc/asound/card0/pcm0p/xrun_debug then check your app? Does it work better, and get any kernel messages?
Basically it's rather a job of the lowlevel driver (intel8x0) to fix / filter out the bogus values.
I though we have already some code to correct it in 2.6.30, but it seems not enough...
Also, it'd be helpful if you can check whether the problem exists in the current sound git tree (for-linus branch), too. git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> ALSA: Enable PCM hw_ptr_jiffies check only in xrun_debug mode
> The PCM hw_ptr jiffies check results sometimes in problems when a > hardware doesn't give smooth hw_ptr updates. So far, au88x0 and some > other drivers appear not working due to this strict check. > However, this check is a nice debug tool, and the capability should be > still kept.
> Hence, we disable this check now as default unless the user enables it > by setting the xrun_debug mode to the specific stream via a proc file.
> > ALSA: Enable PCM hw_ptr_jiffies check only in xrun_debug mode
> > The PCM hw_ptr jiffies check results sometimes in problems when a > > hardware doesn't give smooth hw_ptr updates. So far, au88x0 and some > > other drivers appear not working due to this strict check. > > However, this check is a nice debug tool, and the capability should be > > still kept.
> > Hence, we disable this check now as default unless the user enables it > > by setting the xrun_debug mode to the specific stream via a proc file.
>> I'm also affected by the issue described above.
> Which hardware?
It's a Samsung X20 notebook that has some "SoundMAX" hardware. I don't know how to describe it better. Does the ALSA driver report, which ac97 chip is connected?
I'm using the snd_intel8x0 kernel module.
>> Can somebody point out, how I can set this xrun_debug thing or can >> somebody provide a patch that reverts the change?
> Did you read my reply to David?
I will try, what is described there and I will report back if I get any debug output.
> Takashi Iwai schrieb: > >> I'm also affected by the issue described above.
> > Which hardware?
> It's a Samsung X20 notebook that has some "SoundMAX" hardware. > I don't know how to describe it better. Does the ALSA driver report, > which ac97 chip is connected?
> I'm using the snd_intel8x0 kernel module.
OK, then really a same issue. I think the problem is independent of the codec ("SoundMax" is a name of Analog-Device codec chips) but rather the controller, which you see in lspci.
> >> Can somebody point out, how I can set this xrun_debug thing or can > >> somebody provide a patch that reverts the change?
> > Did you read my reply to David?
> I will try, what is described there and I will report back if I get any > debug output.
> Could you set CONFIG_SND_PCM_XRUN_DEBUG=y and do > # echo 1 > /proc/asound/card0/pcm0p/xrun_debug > then check your app? Does it work better, and get any kernel messages?
Yes, the skipping goes away and I get hw_ptr skipping kernel messages:
> Also, it'd be helpful if you can check whether the problem exists in > the current sound git tree (for-linus branch), too. > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
I'm building this to test right now, will report when I try it out. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> Also, it'd be helpful if you can check whether the problem exists in > the current sound git tree (for-linus branch), too. > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
Same behavior as 2.6.30, but debugging messages are formatted differently :-)
[ 1109.062182] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=73728) [ 1125.126123] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=861184) [ 1131.398094] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1178624) [ 1135.579406] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1395712) [ 1145.115367] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1869824) [ 1152.688670] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=2249728) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> > Also, it'd be helpful if you can check whether the problem exists in > > the current sound git tree (for-linus branch), too. > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> Same behavior as 2.6.30, but debugging messages are formatted differently > :-)
> [ 1109.062182] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=73728) > [ 1125.126123] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=861184) > [ 1131.398094] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1178624) > [ 1135.579406] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1395712) > [ 1145.115367] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1869824) > [ 1152.688670] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=2249728)
[ Just another data point. ]
I'm experiencing similar problem with the sound in recent -next kernels.
Though the only debug data that I'm getting with xrun_debug == 1 is:
> On Thursday 11 June 2009 14:02:20 David Miller wrote: > > From: Takashi Iwai <ti...@suse.de> > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > Also, it'd be helpful if you can check whether the problem exists in > > > the current sound git tree (for-linus branch), too. > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > Same behavior as 2.6.30, but debugging messages are formatted differently > > :-)
> > [ 1109.062182] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=73728) > > [ 1125.126123] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=861184) > > [ 1131.398094] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1178624) > > [ 1135.579406] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1395712) > > [ 1145.115367] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1869824) > > [ 1152.688670] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=2249728)
> [ Just another data point. ]
> I'm experiencing similar problem with the sound in recent -next kernels.
Which driver?
> Though the only debug data that I'm getting with xrun_debug == 1 is:
> with xrun_debug == 3: > XRUN: pcmC0D0p:0 > Pid: 4718, comm: audacious Not tainted 2.6.30-next-20090611-07944-gc1b019d-dirty #21 > Call Trace:
The semantics of xrun_debug proc file was a bit changed. Now it's bit flags, and 4 corresponds to the additional jiffies check (1 is to show the debug message, 2 to show stack trace).
Try to set 5 to xrun_debug. Do you get other messages?
> > Could you set CONFIG_SND_PCM_XRUN_DEBUG=y and do > > # echo 1 > /proc/asound/card0/pcm0p/xrun_debug > > then check your app? Does it work better, and get any kernel messages?
> Yes, the skipping goes away and I get hw_ptr skipping kernel messages:
> > Also, it'd be helpful if you can check whether the problem exists in > > the current sound git tree (for-linus branch), too. > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> Same behavior as 2.6.30, but debugging messages are formatted differently > :-)
> [ 1109.062182] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=73728) > [ 1125.126123] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=861184) > [ 1131.398094] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1178624) > [ 1135.579406] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1395712) > [ 1145.115367] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=1869824) > [ 1152.688670] PCM: Lost interrupts? (stream=0, delta=16384, intr_ptr=2249728)
I guess this is because of the change of xrun_debug semantics. Try to set 5 to xrun_debug instead of 1. Then you'll get the similar results like 2.6.30, I guess. (Setting to 4 will run with the sanity check but without debug prints, BTW.)
I'll investigate the problem, but probably a bit later as now is a long weekend in Germany...
> At Thu, 11 Jun 2009 15:32:33 +0200, > Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 11 June 2009 14:02:20 David Miller wrote: > > > From: Takashi Iwai <ti...@suse.de> > > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > > Also, it'd be helpful if you can check whether the problem exists in > > > > the current sound git tree (for-linus branch), too. > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > > Same behavior as 2.6.30, but debugging messages are formatted differently > > > :-)
> The semantics of xrun_debug proc file was a bit changed. Now it's > bit flags, and 4 corresponds to the additional jiffies check (1 is to > show the debug message, 2 to show stack trace).
> Try to set 5 to xrun_debug. Do you get other messages?
> On Thursday 11 June 2009 15:56:14 Takashi Iwai wrote: > > At Thu, 11 Jun 2009 15:32:33 +0200, > > Bartlomiej Zolnierkiewicz wrote:
> > > On Thursday 11 June 2009 14:02:20 David Miller wrote: > > > > From: Takashi Iwai <ti...@suse.de> > > > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > > > Also, it'd be helpful if you can check whether the problem exists in > > > > > the current sound git tree (for-linus branch), too. > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > > > Same behavior as 2.6.30, but debugging messages are formatted differently > > > > :-)
> > The semantics of xrun_debug proc file was a bit changed. Now it's > > bit flags, and 4 corresponds to the additional jiffies check (1 is to > > show the debug message, 2 to show stack trace).
> > Try to set 5 to xrun_debug. Do you get other messages?
> [ They seem to be the same as reported by davem against 2.6.30. ]
Yes.
> and the skipping is gone.
Does the patch below fix the problem? If not, how about to revert the commit below?
commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 Author: Jaroslav Kysela <pe...@perex.cz> Date: Mon Apr 13 21:31:25 2009 +0200 [ALSA] intel8x0: do not use zero value from PICB register
> At Thu, 11 Jun 2009 16:14:07 +0200, > Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 11 June 2009 15:56:14 Takashi Iwai wrote: > > > At Thu, 11 Jun 2009 15:32:33 +0200, > > > Bartlomiej Zolnierkiewicz wrote:
> > > > On Thursday 11 June 2009 14:02:20 David Miller wrote: > > > > > From: Takashi Iwai <ti...@suse.de> > > > > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > > > > Also, it'd be helpful if you can check whether the problem exists in > > > > > > the current sound git tree (for-linus branch), too. > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > > > > Same behavior as 2.6.30, but debugging messages are formatted differently > > > > > :-)
> > > The semantics of xrun_debug proc file was a bit changed. Now it's > > > bit flags, and 4 corresponds to the additional jiffies check (1 is to > > > show the debug message, 2 to show stack trace).
> > > Try to set 5 to xrun_debug. Do you get other messages?
> > [ They seem to be the same as reported by davem against 2.6.30. ]
> Yes.
> > and the skipping is gone.
> Does the patch below fix the problem?
Unfortunately, it did not fix the issue.
> If not, how about to revert the commit below?
> commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > Author: Jaroslav Kysela <pe...@perex.cz> > Date: Mon Apr 13 21:31:25 2009 +0200 > [ALSA] intel8x0: do not use zero value from PICB register
> On Thursday 11 June 2009 16:23:04 Takashi Iwai wrote: > > At Thu, 11 Jun 2009 16:14:07 +0200, > > Bartlomiej Zolnierkiewicz wrote:
> > > On Thursday 11 June 2009 15:56:14 Takashi Iwai wrote: > > > > At Thu, 11 Jun 2009 15:32:33 +0200, > > > > Bartlomiej Zolnierkiewicz wrote:
> > > > > On Thursday 11 June 2009 14:02:20 David Miller wrote: > > > > > > From: Takashi Iwai <ti...@suse.de> > > > > > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > > > > > Also, it'd be helpful if you can check whether the problem exists in > > > > > > > the current sound git tree (for-linus branch), too. > > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > > > > > Same behavior as 2.6.30, but debugging messages are formatted differently > > > > > > :-)
> > > > The semantics of xrun_debug proc file was a bit changed. Now it's > > > > bit flags, and 4 corresponds to the additional jiffies check (1 is to > > > > show the debug message, 2 to show stack trace).
> > > > Try to set 5 to xrun_debug. Do you get other messages?
> > > [ They seem to be the same as reported by davem against 2.6.30. ]
> > Yes.
> > > and the skipping is gone.
> > Does the patch below fix the problem?
> Unfortunately, it did not fix the issue.
> > If not, how about to revert the commit below?
> > commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > > Author: Jaroslav Kysela <pe...@perex.cz> > > Date: Mon Apr 13 21:31:25 2009 +0200 > > [ALSA] intel8x0: do not use zero value from PICB register
> Same here.
Hmm... How about the patch below, then? This should show other debug prints from intel8x0 if my guess is right.
> At Thu, 11 Jun 2009 17:40:00 +0200, > Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 11 June 2009 16:23:04 Takashi Iwai wrote: > > > At Thu, 11 Jun 2009 16:14:07 +0200, > > > Bartlomiej Zolnierkiewicz wrote:
> > > > On Thursday 11 June 2009 15:56:14 Takashi Iwai wrote: > > > > > At Thu, 11 Jun 2009 15:32:33 +0200, > > > > > Bartlomiej Zolnierkiewicz wrote:
> > > > > > On Thursday 11 June 2009 14:02:20 David Miller wrote: > > > > > > > From: Takashi Iwai <ti...@suse.de> > > > > > > > Date: Wed, 10 Jun 2009 21:37:14 +0200
> > > > > > > > Also, it'd be helpful if you can check whether the problem exists in > > > > > > > > the current sound git tree (for-linus branch), too. > > > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git for-linus
> > > > > > > Same behavior as 2.6.30, but debugging messages are formatted differently > > > > > > > :-)
> > > > > The semantics of xrun_debug proc file was a bit changed. Now it's > > > > > bit flags, and 4 corresponds to the additional jiffies check (1 is to > > > > > show the debug message, 2 to show stack trace).
> > > > > Try to set 5 to xrun_debug. Do you get other messages?
> > > > [ They seem to be the same as reported by davem against 2.6.30. ]
> > > Yes.
> > > > and the skipping is gone.
> > > Does the patch below fix the problem?
> > Unfortunately, it did not fix the issue.
> > > If not, how about to revert the commit below?
> > > commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > > > Author: Jaroslav Kysela <pe...@perex.cz> > > > Date: Mon Apr 13 21:31:25 2009 +0200 > > > [ALSA] intel8x0: do not use zero value from PICB register
> > Same here.
> Hmm... How about the patch below, then?
Still not fixed.
> This should show other debug prints from intel8x0 if my guess is right.
There is a lot of them.. :)
XXX intel8x0: invalid update 4060 (jdelta=3) XXX intel8x0: invalid update 4040 (jdelta=3) XXX intel8x0: invalid update 4024 (jdelta=3) XXX intel8x0: invalid update 3992 (jdelta=2) XXX intel8x0: invalid update 3956 (jdelta=3) XXX intel8x0: invalid update 4044 (jdelta=2) XXX intel8x0: invalid update 4028 (jdelta=3) XXX intel8x0: invalid update 4020 (jdelta=3) XXX intel8x0: invalid update 3984 (jdelta=3) XXX intel8x0: invalid update 4052 (jdelta=2) ... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
>>>> Can somebody point out, how I can set this xrun_debug thing or can >>>> somebody provide a patch that reverts the change? >>> Did you read my reply to David? >> I will try, what is described there and I will report back if I get any >> debug output.
> Thanks, that'll be be helpful.
Below is the output. Note, that audacious 2.x was not only skipping a little bit. In fact, the clock that was showing the time of audio played was running twice as fast or even 4 times as fast. The sound was played at the right samplerate but in fast it seems, that much of the audio data was skipped and I only heard a series of short fragments of what should have been the song I'm listening to.
> >>>> Can somebody point out, how I can set this xrun_debug thing or can > >>>> somebody provide a patch that reverts the change? > >>> Did you read my reply to David? > >> I will try, what is described there and I will report back if I get any > >> debug output.
> > Thanks, that'll be be helpful.
> Below is the output. Note, that audacious 2.x was not only skipping a > little bit. In fact, the clock that was showing the time of audio played > was running twice as fast or even 4 times as fast. The sound was played > at the right samplerate but in fast it seems, that much of the audio > data was skipped and I only heard a series of short fragments of what > should have been the song I'm listening to.
OK, it must be the same problem indeed as David and Bartlomiej see.
Although Bartlomiej wrote that reverting the commit below didn't help, I still suspect it comes from there. commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 Author: Jaroslav Kysela <pe...@perex.cz> Date: Mon Apr 13 21:31:25 2009 +0200 [ALSA] intel8x0: do not use zero value from PICB register
The below is a revised patch to fix the possible regression from this. Could you guys give it a try, or check reverting the above?
thanks,
Takashi
--- diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 173bebf..2446ed6 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -1077,15 +1077,18 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs ptr1 <<= ichdev->pos_shift; ptr = ichdev->fragsize1 - ptr1; ptr += position; - ichdev->last_pos = ptr; - ichdev->last_pos_jiffies = jiffies; } else { ptr1 = jiffies - ichdev->last_pos_jiffies; if (ptr1) ptr1 -= 1; ptr = ichdev->last_pos + ptr1 * ichdev->jiffy_to_bytes; ptr %= ichdev->size; + if (ptr < position || + ptr > position + substream->runtime->period_size) + ptr = position; } + ichdev->last_pos = ptr; + ichdev->last_pos_jiffies = jiffies; spin_unlock(&chip->reg_lock); if (ptr >= ichdev->size) return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> At Thu, 11 Jun 2009 23:38:17 +0200, > Sven Köhler wrote:
> > >>>> Can somebody point out, how I can set this xrun_debug thing or can > > >>>> somebody provide a patch that reverts the change? > > >>> Did you read my reply to David? > > >> I will try, what is described there and I will report back if I get any > > >> debug output.
> > > Thanks, that'll be be helpful.
> > Below is the output. Note, that audacious 2.x was not only skipping a > > little bit. In fact, the clock that was showing the time of audio played > > was running twice as fast or even 4 times as fast. The sound was played > > at the right samplerate but in fast it seems, that much of the audio > > data was skipped and I only heard a series of short fragments of what > > should have been the song I'm listening to.
> OK, it must be the same problem indeed as David and Bartlomiej see.
> Although Bartlomiej wrote that reverting the commit below didn't help, > I still suspect it comes from there. > commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > Author: Jaroslav Kysela <pe...@perex.cz> > Date: Mon Apr 13 21:31:25 2009 +0200 > [ALSA] intel8x0: do not use zero value from PICB register
> The below is a revised patch to fix the possible regression from this. > Could you guys give it a try, or check reverting the above?
The issue is still present with the revised patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> At Fri, 12 Jun 2009 13:44:25 +0200, > Bartlomiej Zolnierkiewicz wrote:
> > On Friday 12 June 2009 03:52:48 Takashi Iwai wrote: > > > At Thu, 11 Jun 2009 23:38:17 +0200, > > > Sven Köhler wrote:
> > > > >>>> Can somebody point out, how I can set this xrun_debug thing or can > > > > >>>> somebody provide a patch that reverts the change? > > > > >>> Did you read my reply to David? > > > > >> I will try, what is described there and I will report back if I get any > > > > >> debug output.
> > > > > Thanks, that'll be be helpful.
> > > > Below is the output. Note, that audacious 2.x was not only skipping a > > > > little bit. In fact, the clock that was showing the time of audio played > > > > was running twice as fast or even 4 times as fast. The sound was played > > > > at the right samplerate but in fast it seems, that much of the audio > > > > data was skipped and I only heard a series of short fragments of what > > > > should have been the song I'm listening to.
> > > OK, it must be the same problem indeed as David and Bartlomiej see.
> > > Although Bartlomiej wrote that reverting the commit below didn't help, > > > I still suspect it comes from there. > > > commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > > > Author: Jaroslav Kysela <pe...@perex.cz> > > > Date: Mon Apr 13 21:31:25 2009 +0200 > > > [ALSA] intel8x0: do not use zero value from PICB register
> > > The below is a revised patch to fix the possible regression from this. > > > Could you guys give it a try, or check reverting the above?
> > The issue is still present with the revised patch.
> I think we should go back to basic. The patch below cuts off the > position compensation but falls back to the last value, which is a > safer option. How about this?
If that patch also doesn't work, we need to track the position update sequence in more details. The patch below will dump the each position update (CAUTION: it can be long logs!). Please apply it instead of the previous one, run with xrun_debug=5 on 2.6.31 (or xrun_debug=1 on 2.6.30), and give the outputs around a skip occurs.
thanks,
Takashi
--- diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 173bebf..0a8a055 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -1059,6 +1059,7 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs size_t ptr1, ptr; int civ, timeout = 10; unsigned int position; + unsigned long jdelta;
spin_lock(&chip->reg_lock); do { @@ -1073,19 +1074,17 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb)) break; } while (timeout--); + jdelta = jiffies - ichdev->last_pos_jiffies; + printk(KERN_DEBUG "XXX intel8x0: ptr1=%d/%d, base=%d, jdelta=%ld\n", + (int)ptr1, position, ichdev->fragsize1, jdelta); + ptr = ichdev->last_pos; if (ptr1 != 0) { ptr1 <<= ichdev->pos_shift; ptr = ichdev->fragsize1 - ptr1; ptr += position; - ichdev->last_pos = ptr; - ichdev->last_pos_jiffies = jiffies; - } else { - ptr1 = jiffies - ichdev->last_pos_jiffies; - if (ptr1) - ptr1 -= 1; - ptr = ichdev->last_pos + ptr1 * ichdev->jiffy_to_bytes; - ptr %= ichdev->size; } + ichdev->last_pos = ptr; + ichdev->last_pos_jiffies = jiffies; spin_unlock(&chip->reg_lock); if (ptr >= ichdev->size) return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> On Friday 12 June 2009 03:52:48 Takashi Iwai wrote: > > At Thu, 11 Jun 2009 23:38:17 +0200, > > Sven Köhler wrote:
> > > >>>> Can somebody point out, how I can set this xrun_debug thing or can > > > >>>> somebody provide a patch that reverts the change? > > > >>> Did you read my reply to David? > > > >> I will try, what is described there and I will report back if I get any > > > >> debug output.
> > > > Thanks, that'll be be helpful.
> > > Below is the output. Note, that audacious 2.x was not only skipping a > > > little bit. In fact, the clock that was showing the time of audio played > > > was running twice as fast or even 4 times as fast. The sound was played > > > at the right samplerate but in fast it seems, that much of the audio > > > data was skipped and I only heard a series of short fragments of what > > > should have been the song I'm listening to.
> > OK, it must be the same problem indeed as David and Bartlomiej see.
> > Although Bartlomiej wrote that reverting the commit below didn't help, > > I still suspect it comes from there. > > commit da2436a23c038055b1da6fe30b6ea2886b1e07b0 > > Author: Jaroslav Kysela <pe...@perex.cz> > > Date: Mon Apr 13 21:31:25 2009 +0200 > > [ALSA] intel8x0: do not use zero value from PICB register
> > The below is a revised patch to fix the possible regression from this. > > Could you guys give it a try, or check reverting the above?
> The issue is still present with the revised patch.
I think we should go back to basic. The patch below cuts off the position compensation but falls back to the last value, which is a safer option. How about this?
thanks,
Takashi
--- diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 173bebf..1c02705 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -356,8 +356,6 @@ struct ichdev { unsigned int position; unsigned int pos_shift; unsigned int last_pos; - unsigned long last_pos_jiffies; - unsigned int jiffy_to_bytes; int frags; int lvi; int lvi_frag; @@ -844,7 +842,6 @@ static int snd_intel8x0_pcm_trigger(struct snd_pcm_substream *substream, int cmd case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: val = ICH_IOCE | ICH_STARTBM; ichdev->last_pos = ichdev->position; - ichdev->last_pos_jiffies = jiffies; break; case SNDRV_PCM_TRIGGER_SUSPEND: ichdev->suspended = 1; @@ -1048,7 +1045,6 @@ static int snd_intel8x0_pcm_prepare(struct snd_pcm_substream *substream) ichdev->pos_shift = (runtime->sample_bits > 16) ? 2 : 1; } snd_intel8x0_setup_periods(chip, ichdev); - ichdev->jiffy_to_bytes = (runtime->rate * 4 * ichdev->pos_shift) / HZ; return 0; }
@@ -1073,19 +1069,13 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb)) break; } while (timeout--); + ptr = ichdev->last_pos; if (ptr1 != 0) { ptr1 <<= ichdev->pos_shift; ptr = ichdev->fragsize1 - ptr1; ptr += position; - ichdev->last_pos = ptr; - ichdev->last_pos_jiffies = jiffies; - } else { - ptr1 = jiffies - ichdev->last_pos_jiffies; - if (ptr1) - ptr1 -= 1; - ptr = ichdev->last_pos + ptr1 * ichdev->jiffy_to_bytes; - ptr %= ichdev->size; } + ichdev->last_pos = ptr; spin_unlock(&chip->reg_lock); if (ptr >= ichdev->size) return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> I think we should go back to basic. The patch below cuts off the > position compensation but falls back to the last value, which is a > safer option. How about this?
I'll give this a try in a few moments. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> If that patch also doesn't work, we need to track the position update > sequence in more details. > The patch below will dump the each position update (CAUTION: it can be > long logs!). Please apply it instead of the previous one, run with > xrun_debug=5 on 2.6.31 (or xrun_debug=1 on 2.6.30), and give the > outputs around a skip occurs.
there is a "PCM: hw_ptr skipping! ..." message near the end. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> > If that patch also doesn't work, we need to track the position update > > sequence in more details. > > The patch below will dump the each position update (CAUTION: it can be > > long logs!). Please apply it instead of the previous one, run with > > xrun_debug=5 on 2.6.31 (or xrun_debug=1 on 2.6.30), and give the > > outputs around a skip occurs.
> there is a "PCM: hw_ptr skipping! ..." message near the end.
Thanks! I see the problem now. It's a race in the update of the current buffer position. The counter was reset to the full fragment size, but its index still points the previous position. So, the driver was confused and put the position back.
The below is a revised patch. It has an additional sanity check. Please give it a try.
Takashi
--- diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 173bebf..474e40a 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -356,8 +356,6 @@ struct ichdev { unsigned int position; unsigned int pos_shift; unsigned int last_pos; - unsigned long last_pos_jiffies; - unsigned int jiffy_to_bytes; int frags; int lvi; int lvi_frag; @@ -844,7 +842,6 @@ static int snd_intel8x0_pcm_trigger(struct snd_pcm_substream *substream, int cmd case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: val = ICH_IOCE | ICH_STARTBM; ichdev->last_pos = ichdev->position; - ichdev->last_pos_jiffies = jiffies; break; case SNDRV_PCM_TRIGGER_SUSPEND: ichdev->suspended = 1; @@ -1048,7 +1045,6 @@ static int snd_intel8x0_pcm_prepare(struct snd_pcm_substream *substream) ichdev->pos_shift = (runtime->sample_bits > 16) ? 2 : 1; } snd_intel8x0_setup_periods(chip, ichdev); - ichdev->jiffy_to_bytes = (runtime->rate * 4 * ichdev->pos_shift) / HZ; return 0; }
@@ -1073,19 +1069,19 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb)) break; } while (timeout--); + ptr = ichdev->last_pos; if (ptr1 != 0) { ptr1 <<= ichdev->pos_shift; ptr = ichdev->fragsize1 - ptr1; ptr += position; - ichdev->last_pos = ptr; - ichdev->last_pos_jiffies = jiffies; - } else { - ptr1 = jiffies - ichdev->last_pos_jiffies; - if (ptr1) - ptr1 -= 1; - ptr = ichdev->last_pos + ptr1 * ichdev->jiffy_to_bytes; - ptr %= ichdev->size; + if (ptr < ichdev->last_pos) { + /* invalid position; likely ptr1 went back to + * fragsize1 while the base position wasn't updated yet + */ + ptr = ichdev->last_pos; + } } + ichdev->last_pos = ptr; spin_unlock(&chip->reg_lock); if (ptr >= ichdev->size) return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
> > > If that patch also doesn't work, we need to track the position update > > > sequence in more details. > > > The patch below will dump the each position update (CAUTION: it can be > > > long logs!). Please apply it instead of the previous one, run with > > > xrun_debug=5 on 2.6.31 (or xrun_debug=1 on 2.6.30), and give the > > > outputs around a skip occurs.
> > there is a "PCM: hw_ptr skipping! ..." message near the end.
> Thanks! I see the problem now. It's a race in the update of the > current buffer position. The counter was reset to the full fragment > size, but its index still points the previous position. So, the > driver was confused and put the position back.
> The below is a revised patch. It has an additional sanity check. > Please give it a try.
Damn, that was buggy again. The fixed version is below.
Takashi
--- diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c index 173bebf..8aa5687 100644 --- a/sound/pci/intel8x0.c +++ b/sound/pci/intel8x0.c @@ -356,8 +356,6 @@ struct ichdev { unsigned int position; unsigned int pos_shift; unsigned int last_pos; - unsigned long last_pos_jiffies; - unsigned int jiffy_to_bytes; int frags; int lvi; int lvi_frag; @@ -844,7 +842,6 @@ static int snd_intel8x0_pcm_trigger(struct snd_pcm_substream *substream, int cmd case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: val = ICH_IOCE | ICH_STARTBM; ichdev->last_pos = ichdev->position; - ichdev->last_pos_jiffies = jiffies; break; case SNDRV_PCM_TRIGGER_SUSPEND: ichdev->suspended = 1; @@ -1048,7 +1045,6 @@ static int snd_intel8x0_pcm_prepare(struct snd_pcm_substream *substream) ichdev->pos_shift = (runtime->sample_bits > 16) ? 2 : 1; } snd_intel8x0_setup_periods(chip, ichdev); - ichdev->jiffy_to_bytes = (runtime->rate * 4 * ichdev->pos_shift) / HZ; return 0; }
@@ -1073,19 +1069,23 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs ptr1 == igetword(chip, ichdev->reg_offset + ichdev->roff_picb)) break; } while (timeout--); + ptr = ichdev->last_pos; if (ptr1 != 0) { ptr1 <<= ichdev->pos_shift; ptr = ichdev->fragsize1 - ptr1; ptr += position; - ichdev->last_pos = ptr; - ichdev->last_pos_jiffies = jiffies; - } else { - ptr1 = jiffies - ichdev->last_pos_jiffies; - if (ptr1) - ptr1 -= 1; - ptr = ichdev->last_pos + ptr1 * ichdev->jiffy_to_bytes; - ptr %= ichdev->size; + if (ptr < ichdev->last_pos) { + unsigned int pos_base, last_base; + pos_base = position / ichdev->fragsize1; + last_base = ichdev->last_pos / ichdev->fragsize1; + /* another sanity check; ptr1 can go back to full + * before the base position is updated + */ + if (pos_base == last_base) + ptr = ichdev->last_pos; + } } + ichdev->last_pos = ptr; spin_unlock(&chip->reg_lock); if (ptr >= ichdev->size) return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
