[alsa-devel] Problem bringing up new alsa driver
My new ALSA driver is playing the first three samples in a loop. It's not progressing forward in the wav file. It's behaving like I'm not calling snd_pcm_period_elapsed() but I have a printk() in it so I'm sure it is being called. Later samples are not getting copied into the DMA buffer,
Can anyone give me a clue as to what to look for?
root@phyCORE-MPC5200B-tiny:~ aplay phone.wav snd_pcm_playback_open snd_pcm_open snd_pcm_open_file snd_pcm_open_substream mpc5200-psc-ac97 f0002200.sound: psc_dma_startup(substream=c78bd800) mpc5200-psc-ac97 f0002200.sound: psc_dma_pcm_open(substream=c78bd800) Playing WAVE 'phone.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo mpc5200-psc-ac97 f0002200.sound: psc_ac97_hw_analog_params(substream=c78bd800) p_size=5513 p_bytes=22052 periods=3 buffer_size=22050 buffer_bytes=88200 channels=2 snd_pcm_prepare stac9766: ac97_analog_prepare rate 44100 mpc5200-psc-ac97 f0002200.sound: psc_dma_trigger(substream=c78bd800, cmd=1) stream_id=0 snd_pcm_hwsync snd_pcm_update_hw_ptr snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 5513 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 11026 snd_pcm_period_elapsed snd_pcm_update_hw_ptr_interrupt 0 Aborted by signal Interrupt... mpc5200-psc-ac97 f0002200.sound: psc_dma_trigger(substream=c78bd800, cmd=0) stream_id=0 mpc5200-psc-ac97 f0002200.sound: psc_dma_shutdown(substream=c78bd800) mpc5200-psc-ac97 f0002200.sound: psc_dma_pcm_close(substream=c78bd800) root@phyCORE-MPC5200B-tiny:
2009/4/23 Jon Smirl jonsmirl@gmail.com:
My new ALSA driver is playing the first three samples in a loop. It's not progressing forward in the wav file. It's behaving like I'm not calling snd_pcm_period_elapsed() but I have a printk() in it so I'm sure it is being called. Later samples are not getting copied into the DMA buffer,
Can anyone give me a clue as to what to look for?
root@phyCORE-MPC5200B-tiny:~ aplay phone.wav snd_pcm_playback_open snd_pcm_open snd_pcm_open_file snd_pcm_open_substream mpc5200-psc-ac97 f0002200.sound: psc_dma_startup(substream=c78bd800) mpc5200-psc-ac97 f0002200.sound: psc_dma_pcm_open(substream=c78bd800) Playing WAVE 'phone.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo mpc5200-psc-ac97 f0002200.sound: psc_ac97_hw_analog_params(substream=c78bd800) p_size=5513 p_bytes=22052 periods=3 buffer_size=22050 buffer_bytes=88200
Your buffer is 4x the period size, but periods=3. Off by one error somewhere?
Lee
These new patches have broken audio support on the mpc5200....
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation. I'm still checking to see if I can get that data out of the Bestcomm DMA engine.
jonsmirl@terra:/home/linus/sound/core$ git log pcm_lib.c commit bbf6ad1399e9516b0a95de3ad58ffbaed670e4cc Author: Jaroslav Kysela perex@perex.cz Date: Fri Apr 10 12:28:58 2009 +0200
[ALSA] pcm-midlevel: Add more strict buffer position checks based on jiffies
Some drivers like Intel8x0 or Intel HDA are broken for some hardware variants. This patch adds more strict buffer position checks based on jiffies when internal hw_ptr is updated. Enable xrun_debug to see mangling of wrong positions.
As a side effect, the hw_ptr interrupt update routine might do slightly better job when many interrupts are lost.
Signed-off-by: Jaroslav Kysela perex@perex.cz
commit 8b22d943c34b616eefbd6d2f8f197a53b1f29fd0 Author: Takashi Iwai tiwai@suse.de Date: Fri Mar 20 16:26:15 2009 +0100
ALSA: pcm - Safer boundary checks
Make the boundary checks a bit safer. These caese are rare or theoretically won't happen, but nothing bad to keep the checks safer...
Signed-off-by: Takashi Iwai tiwai@suse.de
commit ded652f7024bc2d7b6118b561a44187af30841b0 Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:08:49 2009 +0100
ALSA: pcm - Fix delta calculation at boundary overlap
When the hw_ptr_interrupt reaches the boundary, it must check whether the hw_base was already lapped and corret the delta value appropriately.
Also, rebasing the hw_ptr needs a correction because buffer_size isn't always aligned to period_size.
Signed-off-by: Takashi Iwai tiwai@suse.de
commit 5f513e1197f27e9a0bcfec0feaac59f976f4a37e Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:01:47 2009 +0100
ALSA: pcm - Reset invalid position even without debug option
Always reset the invalind hw_ptr position returned by the pointer callback. The behavior should be consitent independently from the debug option.
Also, add the printk_ratelimit() check to avoid flooding debug prints.
Signed-off-by: Takashi Iwai tiwai@suse.de
At Sat, 25 Apr 2009 17:28:55 -0400, Jon Smirl wrote:
These new patches have broken audio support on the mpc5200....
Then the lowlevel callbacks are likely wrong :)
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation.
Well, nothing new by these patches. The original PCM core implementation has been designed in that way. These changes may check some bogus value more strictly.
If the hardware doesn't support it, it needs report at least some "sane" value; i.e. it must not be over the real position, but not below the previous pointer. In the worst case, the driver just needs to report the same position at the previous period boundary until the next snd_pcm_period_elapsed().
Takashi
I'm still checking to see if I can get that data out of the Bestcomm DMA engine.
jonsmirl@terra:/home/linus/sound/core$ git log pcm_lib.c commit bbf6ad1399e9516b0a95de3ad58ffbaed670e4cc Author: Jaroslav Kysela perex@perex.cz Date: Fri Apr 10 12:28:58 2009 +0200
[ALSA] pcm-midlevel: Add more strict buffer position checks based on jiffies Some drivers like Intel8x0 or Intel HDA are broken for somehardware variants. This patch adds more strict buffer position checks based on jiffies when internal hw_ptr is updated. Enable xrun_debug to see mangling of wrong positions.
As a side effect, the hw_ptr interrupt update routine might doslightly better job when many interrupts are lost.
Signed-off-by: Jaroslav Kysela <perex@perex.cz>commit 8b22d943c34b616eefbd6d2f8f197a53b1f29fd0 Author: Takashi Iwai tiwai@suse.de Date: Fri Mar 20 16:26:15 2009 +0100
ALSA: pcm - Safer boundary checks Make the boundary checks a bit safer. These caese are rare or theoretically won't happen, but nothing bad to keep the checks safer... Signed-off-by: Takashi Iwai <tiwai@suse.de>commit ded652f7024bc2d7b6118b561a44187af30841b0 Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:08:49 2009 +0100
ALSA: pcm - Fix delta calculation at boundary overlap When the hw_ptr_interrupt reaches the boundary, it must check whether the hw_base was already lapped and corret the delta value appropriately. Also, rebasing the hw_ptr needs a correction because buffer_size isn't always aligned to period_size. Signed-off-by: Takashi Iwai <tiwai@suse.de>commit 5f513e1197f27e9a0bcfec0feaac59f976f4a37e Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:01:47 2009 +0100
ALSA: pcm - Reset invalid position even without debug option Always reset the invalind hw_ptr position returned by the pointer callback. The behavior should be consitent independently from the debug option. Also, add the printk_ratelimit() check to avoid flooding debug prints. Signed-off-by: Takashi Iwai <tiwai@suse.de>-- Jon Smirl jonsmirl@gmail.com _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sun, Apr 26, 2009 at 7:14 AM, Takashi Iwai tiwai@suse.de wrote:
At Sat, 25 Apr 2009 17:28:55 -0400, Jon Smirl wrote:
These new patches have broken audio support on the mpc5200....
Then the lowlevel callbacks are likely wrong :)
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation.
Well, nothing new by these patches. The original PCM core implementation has been designed in that way. These changes may check some bogus value more strictly.
If the hardware doesn't support it, it needs report at least some "sane" value; i.e. it must not be over the real position, but not below the previous pointer.
In the worst case, the driver just needs to report the same position at the previous period boundary until the next snd_pcm_period_elapsed().
That's doesn't work anymore. The PCM code is using jiffies to estimate where the pointer needs to be. I used the same jiffies to fake up a hardware position.
Takashi
I'm still checking to see if I can get that data out of the Bestcomm DMA engine.
jonsmirl@terra:/home/linus/sound/core$ git log pcm_lib.c commit bbf6ad1399e9516b0a95de3ad58ffbaed670e4cc Author: Jaroslav Kysela perex@perex.cz Date: Fri Apr 10 12:28:58 2009 +0200
[ALSA] pcm-midlevel: Add more strict buffer position checks based on jiffies
Some drivers like Intel8x0 or Intel HDA are broken for some hardware variants. This patch adds more strict buffer position checks based on jiffies when internal hw_ptr is updated. Enable xrun_debug to see mangling of wrong positions.
As a side effect, the hw_ptr interrupt update routine might do slightly better job when many interrupts are lost.
Signed-off-by: Jaroslav Kysela perex@perex.cz
commit 8b22d943c34b616eefbd6d2f8f197a53b1f29fd0 Author: Takashi Iwai tiwai@suse.de Date: Fri Mar 20 16:26:15 2009 +0100
ALSA: pcm - Safer boundary checks
Make the boundary checks a bit safer. These caese are rare or theoretically won't happen, but nothing bad to keep the checks safer...
Signed-off-by: Takashi Iwai tiwai@suse.de
commit ded652f7024bc2d7b6118b561a44187af30841b0 Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:08:49 2009 +0100
ALSA: pcm - Fix delta calculation at boundary overlap
When the hw_ptr_interrupt reaches the boundary, it must check whether the hw_base was already lapped and corret the delta value appropriately.
Also, rebasing the hw_ptr needs a correction because buffer_size isn't always aligned to period_size.
Signed-off-by: Takashi Iwai tiwai@suse.de
commit 5f513e1197f27e9a0bcfec0feaac59f976f4a37e Author: Takashi Iwai tiwai@suse.de Date: Thu Mar 19 10:01:47 2009 +0100
ALSA: pcm - Reset invalid position even without debug option
Always reset the invalind hw_ptr position returned by the pointer callback. The behavior should be consitent independently from the debug option.
Also, add the printk_ratelimit() check to avoid flooding debug prints.
Signed-off-by: Takashi Iwai tiwai@suse.de
-- Jon Smirl jonsmirl@gmail.com _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Sun, 26 Apr 2009 11:42:14 -0400, Jon Smirl wrote:
On Sun, Apr 26, 2009 at 7:14 AM, Takashi Iwai tiwai@suse.de wrote:
At Sat, 25 Apr 2009 17:28:55 -0400, Jon Smirl wrote:
These new patches have broken audio support on the mpc5200....
Then the lowlevel callbacks are likely wrong :)
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation.
Well, nothing new by these patches. The original PCM core implementation has been designed in that way. These changes may check some bogus value more strictly.
If the hardware doesn't support it, it needs report at least some "sane" value; i.e. it must not be over the real position, but not below the previous pointer.
In the worst case, the driver just needs to report the same position at the previous period boundary until the next snd_pcm_period_elapsed().
That's doesn't work anymore. The PCM code is using jiffies to estimate where the pointer needs to be. I used the same jiffies to fake up a hardware position.
Hrm, then it's a breakage. The quantum position must be still supported. Jaroslav, could you check that? It's your new code...
As an easy workaround, we can give a pcm info flag to indicate that the driver doesn't provide the continuous DMA position.
thanks,
Takashi
On Sun, 26 Apr 2009, Takashi Iwai wrote:
At Sun, 26 Apr 2009 11:42:14 -0400, Jon Smirl wrote:
On Sun, Apr 26, 2009 at 7:14 AM, Takashi Iwai tiwai@suse.de wrote:
At Sat, 25 Apr 2009 17:28:55 -0400, Jon Smirl wrote:
These new patches have broken audio support on the mpc5200....
Then the lowlevel callbacks are likely wrong :)
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation.
Well, nothing new by these patches. The original PCM core implementation has been designed in that way. These changes may check some bogus value more strictly.
If the hardware doesn't support it, it needs report at least some "sane" value; i.e. it must not be over the real position, but not below the previous pointer.
In the worst case, the driver just needs to report the same position at the previous period boundary until the next snd_pcm_period_elapsed().
That's doesn't work anymore. The PCM code is using jiffies to estimate where the pointer needs to be. I used the same jiffies to fake up a hardware position.
Hrm, then it's a breakage. The quantum position must be still supported. Jaroslav, could you check that? It's your new code...
My code checks only if samples delta is over expected jiffies delta, thus this case should not be evaluated as a wrong ring buffer position. It just problem with John's broken lowlevel driver.
But adding finer resolution for pointer callback using jiffies or other timing source might make sense when hardware does not transfer whole buffer quickly (just small FIFO is on path). I'm not only sure, if we should add this code to the pcm midlevel code, because it's relatively easy to implement this in the lowlevel driver.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Sun, Apr 26, 2009 at 1:43 PM, Jaroslav Kysela perex@perex.cz wrote:
On Sun, 26 Apr 2009, Takashi Iwai wrote:
At Sun, 26 Apr 2009 11:42:14 -0400, Jon Smirl wrote:
On Sun, Apr 26, 2009 at 7:14 AM, Takashi Iwai tiwai@suse.de wrote:
At Sat, 25 Apr 2009 17:28:55 -0400, Jon Smirl wrote:
These new patches have broken audio support on the mpc5200....
Then the lowlevel callbacks are likely wrong :)
They assume that the hardware is capable of telling where the DMA engine is in the middle of an operation.
Well, nothing new by these patches. The original PCM core implementation has been designed in that way. These changes may check some bogus value more strictly.
If the hardware doesn't support it, it needs report at least some "sane" value; i.e. it must not be over the real position, but not below the previous pointer.
In the worst case, the driver just needs to report the same position at the previous period boundary until the next snd_pcm_period_elapsed().
That's doesn't work anymore. The PCM code is using jiffies to estimate where the pointer needs to be. I used the same jiffies to fake up a hardware position.
Hrm, then it's a breakage. The quantum position must be still supported. Jaroslav, could you check that? It's your new code...
My code checks only if samples delta is over expected jiffies delta, thus this case should not be evaluated as a wrong ring buffer position. It just problem with John's broken lowlevel driver.
But adding finer resolution for pointer callback using jiffies or other timing source might make sense when hardware does not transfer whole buffer quickly (just small FIFO is on path). I'm not only sure, if we should add this code to the pcm midlevel code, because it's relatively easy to implement this in the lowlevel driver.
I set snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); I don't get errors from ALSA when playing with this set.
I then tried removing the DMA position estimating code.
delta = jiffies - s->jiffies; delta = delta * runtime->rate / HZ; frames = bytes_to_frames(substream->runtime, count); return frames + delta;
If I take this out I get these errors... ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=11026, delta=5513, period=5513, jdelta=33/37/0)
Once I hit one of those, the attempt at pointer fix up makes things worse... ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=16539, delta=11026, period=5513, jdelta=38/75/0) ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=0, delta=16539, period=5513, jdelta=37/112/0) ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=5513, delta=22052, period=5513, jdelta=38/150/0)
Jaroslav
Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Sun, 26 Apr 2009, Jon Smirl wrote:
If I take this out I get these errors... ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=11026, delta=5513, period=5513, jdelta=33/37/0)
It means that the period DMA operation is too quick in your case and does not correspond to the timing specified by rate. It might be that the FIFO on path is large or rate set to the codec is not correct. What's HZ value on your system and FIFO size? There is HZ/100 margin in the check now.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Sun, Apr 26, 2009 at 2:31 PM, Jaroslav Kysela perex@perex.cz wrote:
On Sun, 26 Apr 2009, Jon Smirl wrote:
If I take this out I get these errors... ALSA sound/core/pcm_lib.c:264: PCM: hw_ptr skipping! [Q] (pos=11026, delta=5513, period=5513, jdelta=33/37/0)
It means that the period DMA operation is too quick in your case and does not correspond to the timing specified by rate. It might be that the FIFO on path is large or rate set to the codec is not correct. What's HZ value on your system and FIFO size? There is HZ/100 margin in the check now.
FIFO is 512 bytes, HZ is 300.
Jaroslav
Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
participants (4)
-
Jaroslav Kysela -
Jon Smirl -
Lee Revell -
Takashi Iwai