Re: sound skipping regression introduced in 2.6.30-rc8

[Takashi Iwai]

At Mon, 15 Jun 2009 10:30:01 +0200,
I wrote:
> 
> 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.
> > > > 
> > > > 
> > > > PCM: hw_ptr skipping! (pos=4096, delta=7169, period=1024, jdelta=0/149)
> > > 
> > > 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 <perex@xxxxxxxx>
> > >      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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/