Re: sound skipping regression introduced in 2.6.30-rc8

From: Takashi Iwai
Date: Thu Jun 11 2009 - 12:08:00 EST


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 <tiwai@xxxxxxx>
> > > > > > 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?
> > >
> > > snd_intel8x0
> > >
> > > > > Though the only debug data that I'm getting with xrun_debug == 1 is:
> > > > >
> > > > > XRUN: pcmC0D0p:0
> > > > > XRUN: pcmC0D0p:0
> > > > > XRUN: pcmC0D0p:0
> > > > > XRUN: pcmC0D0p:0
> > > > > XRUN: pcmC0D0p:0
> > > > >
> > > > > 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?
> > >
> > > With xrun_debug == 5 I get:
> > >
> > > PCM: hw_ptr skipping! (pos=14336, delta=15893, period=1024, jdelta=3/90)
> > > PCM: hw_ptr skipping! (pos=11264, delta=15893, period=1024, jdelta=3/90)
> > > PCM: hw_ptr skipping! (pos=7168, delta=15888, period=1024, jdelta=3/90)
> > > PCM: hw_ptr skipping! (pos=8192, delta=15915, period=1024, jdelta=3/90)
> > > PCM: hw_ptr skipping! (pos=15360, delta=15888, period=1024, jdelta=3/90)
> > > XRUN: pcmC0D0p:0
> > > PCM: hw_ptr skipping! (pos=6144, delta=15890, period=1024, jdelta=3/90)
> > > PCM: hw_ptr skipping! (pos=3072, delta=15890, period=1024, jdelta=3/90)
> > >
> > > [ 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 <perex@xxxxxxxx>
> > 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.


thanks,

Takashi

---
diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
index 173bebf..1b292ae 100644
--- a/sound/pci/intel8x0.c
+++ b/sound/pci/intel8x0.c
@@ -1048,7 +1048,8 @@ 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;
+ ichdev->jiffy_to_bytes =
+ runtime->rate * (2 * runtime->sample_bits / 8) / HZ;
return 0;
}

@@ -1056,9 +1057,11 @@ static snd_pcm_uframes_t snd_intel8x0_pcm_pointer(struct snd_pcm_substream *subs
{
struct intel8x0 *chip = snd_pcm_substream_chip(substream);
struct ichdev *ichdev = get_ichdev(substream);
+ struct snd_pcm_runtime *runtime = substream->runtime;
size_t ptr1, ptr;
int civ, timeout = 10;
- unsigned int position;
+ unsigned int position, threshold;
+ unsigned long jdelta;

spin_lock(&chip->reg_lock);
do {
@@ -1073,22 +1076,29 @@ 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;
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;
- }
+ threshold = (jdelta + (HZ / 100)) *
+ runtime->rate * (2 * runtime->sample_bits / 8) / HZ;
+ if (ptr <= threshold) {
+ ptr += position;
+ if (ptr >= ichdev->size)
+ ptr = 0;
+ goto updated;
+ } else
+ printk(KERN_DEBUG "XXX intel8x0: invalid update "
+ "%d (jdelta=%ld)\n", (int)ptr, jdelta);
+ }
+ if (jdelta)
+ jdelta--;
+ ptr = ichdev->last_pos + jdelta * ichdev->jiffy_to_bytes;
+ ptr %= ichdev->size;
+ updated:
+ ichdev->last_pos = ptr;
+ ichdev->last_pos_jiffies = jiffies;
spin_unlock(&chip->reg_lock);
- if (ptr >= ichdev->size)
- return 0;
return bytes_to_frames(substream->runtime, ptr);
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/