On Sun, Apr 26, 2009 at 4:12 AM, Jaroslav Kysela <perex@xxxxxxxx> 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@xxxxxxxx>
> Linux Kernel Sound Maintainer
> ALSA Project, Red Hat, Inc.
>
>
--
Jon Smirl
jonsmirl@xxxxxxxxx
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
