Re: sound skipping regression introduced in 2.6.30-rc8

[Takashi Iwai <tiwai@suse.de>]

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@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?
> > > 
> > > 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@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.


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);
 }