On Sun, Apr 26, 2009 at 4:12 AM, Jaroslav Kysela perex@perex.cz wrote:
On Sat, 25 Apr 2009, Jon Smirl wrote:
This appears to be a bug in this code...
if (delta < 0) { delta += runtime->period_size * runtime->periods;
it was adding, buffer_size. But buffer_size is not correct when the periods don't evenly fit into the buffer
Could you show real values what you're getting here with your driver?
root@phyCORE:~ aplay phone.wav mpc5200-psc-ac97 f0002200.sound: psc_dma_startup(substream=c792d700) mpc5200-psc-ac97 f0002200.sound: psc_dma_pcm_open(substream=c792d700) Playing WAVE 'phone.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo mpc5200-psc-ac97 f0002200.sound: psc_ac97_hw_analog_params(substream=c792d700) p_size=5513 p_bytes=44104 periods=3 buffer_size=22050 buffer_bytes=176400 channels=2 rate=44100 format=11 boundary 22050 boundary 1 1445068800 stac9766: ac97_analog_prepare rate 44100 rate is bb80 5622 mpc5200-psc-ac97 f0002200.sound: psc_dma_trigger(substream=c792d700, cmd=1) stream_id=0 psc_dma_pcm_pointer pos 5513 hw_ptr_interrupt 5513 new_hw_ptr 5513 delta 0 buffer_size 22050 old_hw_ptr 0 new_hw_ptr 5513 rate 44100 HZ 300 jdelta 50 jiffies -79179 runtime->hw_ptr_jiffies -79229 Saved jiffies a -79179 psc_dma_pcm_pointer pos 11026 hw_ptr_interrupt 11026 new_hw_ptr 11026 delta 0 buffer_size 22050 old_hw_ptr 5513 new_hw_ptr 11026 rate 44100 HZ 300 jdelta 38 jiffies -79141 runtime->hw_ptr_jiffies -79179 Saved jiffies a -79141 psc_dma_pcm_pointer pos 16539 hw_ptr_interrupt 16539 new_hw_ptr 16539 delta 0 buffer_size 22050 old_hw_ptr 11026 new_hw_ptr 16539 rate 44100 HZ 300 jdelta 37 jiffies -79104 runtime->hw_ptr_jiffies -79141 Saved jiffies a -79104 psc_dma_pcm_pointer pos 0 hw_ptr_interrupt 22052 new_hw_ptr 0 delta -22052 buffer_size 22050 ALSA sound/core/pcm_lib.c:241: PCM: Unexpected hw_pointer value (stream=0, pos=0, intr_ptr=22052) old_hw_ptr 16539 new_hw_ptr 22052 rate 44100 HZ 300 jdelta 38 jiffies -79066 runtime->hw_ptr_jiffies -79104 Saved jiffies a -79066 psc_dma_pcm_pointer pos 5513 hw_ptr_interrupt 27565 new_hw_ptr 27563 delta -2 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 49613 rate 44100 HZ 300 jdelta 37 jiffies -79029 runtime->hw_ptr_jiffies -79066 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=5513, delta=27561, period=5513, jdelta=37/187/0) Saved jiffies a -79029 psc_dma_pcm_pointer pos 11026 hw_ptr_interrupt 27565 new_hw_ptr 33076 delta 5511 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 33076 rate 44100 HZ 300 jdelta 38 jiffies -78991 runtime->hw_ptr_jiffies -79029 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=11026, delta=11024, period=5513, jdelta=38/74/0) Saved jiffies a -78991 psc_dma_pcm_pointer pos 16539 hw_ptr_interrupt 27565 new_hw_ptr 38589 delta 11024 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 38589 rate 44100 HZ 300 jdelta 37 jiffies -78954 runtime->hw_ptr_jiffies -78991 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=16539, delta=16537, period=5513, jdelta=37/112/0) Saved jiffies a -78954 psc_dma_pcm_pointer pos 0 hw_ptr_interrupt 27565 new_hw_ptr 22050 delta -5515 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 44100 rate 44100 HZ 300 jdelta 38 jiffies -78916 runtime->hw_ptr_jiffies -78954 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=0, delta=22048, period=5513, jdelta=38/149/0) Saved jiffies a -78916 psc_dma_pcm_pointer pos 5513 hw_ptr_interrupt 27565 new_hw_ptr 27563 delta -2 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 49613 rate 44100 HZ 300 jdelta 37 jiffies -78879 runtime->hw_ptr_jiffies -78916 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=5513, delta=27561, period=5513, jdelta=37/187/0) Saved jiffies a -78879 psc_dma_pcm_pointer pos 11026 hw_ptr_interrupt 27565 new_hw_ptr 33076 delta 5511 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 33076 rate 44100 HZ 300 jdelta 38 jiffies -78841 runtime->hw_ptr_jiffies -78879 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=11026, delta=11024, period=5513, jdelta=38/74/0) Saved jiffies a -78841 psc_dma_pcm_pointer pos 16539 hw_ptr_interrupt 27565 new_hw_ptr 38589 delta 11024 buffer_size 22050 old_hw_ptr 22052 new_hw_ptr 38589 rate 44100 HZ 300 jdelta 37 jiffies -78804 runtime->hw_ptr_jiffies -78841 ALSA sound/core/pcm_lib.c:269: PCM: hw_ptr skipping! [Q] (pos=16539, delta=16537, period=5513, jdelta=37/112/0) Saved jiffies a -78804 Aborted by signal Interrupt... mpc5200-psc-ac97 f0002200.sound: psc_dma_trigger(substream=c792d700, cmd=0) stream_id=0 mpc5200-psc-ac97 f0002200.sound: psc_dma_shutdown(substream=c792d700) mpc5200-psc-ac97 f0002200.sound: psc_dma_pcm_close(substream=c792d700) root@phyCORE:~ mpc5200-psc-ac97: timeout on ac97 write
The buffer_size is always a wrap point. For example:
buffer_size = 4096 period_size = 1536
hw_ptr_interrupt => 1536, 3072, 4608
For example, if we receive update_hw_interrupt for second period late when pos (.pointer callback) = 100 and hw_ptr_interrupt = 3072 (hw_base is 0 at the moment):
new_hw_ptr = 100 hw_ptr_interrupt = 3072 delta = -2972
delta += buffer_size -> delta = 1124 - seems correct
Apparently, your .pointer callback returns wrong values.
The .pointer computation code is in sound/soc/fsl/mpc5200_psc_i2s.c I was adding AC97 support to that driver. But the i2s is broken too.
psc_ac97_hw_analog_params(substream=c792d700) p_size=5513 p_bytes=44104 periods=3 buffer_size=22050 buffer_bytes=176400 channels=2 rate=44100 format=11
How does this add up? period size is 5513 and buffer_size is 22050. 5513 * 4 = 22052 which doesn't fit. That's why periods is equal to 3.
if (delta < 0) { hw_ptr_error(substream, "Unexpected hw_pointer value " "(stream=%i, pos=%ld, intr_ptr=%ld)\n", substream->stream, (long)pos, (long)hw_ptr_interrupt); /* rebase to interrupt position */ hw_base = new_hw_ptr = hw_ptr_interrupt; /* align hw_base to buffer_size */ hw_base -= hw_base % runtime->buffer_size; delta = 0; } else { hw_base += runtime->period_size * runtime->periods;
same here
hw_base is always aligned to buffer_size (start of ring buffer)
delta = jiffies - s->jiffies; /* s->jiffies recorded at DMA interrrupt at end of buffer */ delta = delta * runtime->rate / HZ;
Using jiffies to correct stream position seems correct to me.
Jaroslav
Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.