2008-06-26 11:05:24 AD1836 driver question (snd_ad1836_playback_copy)
Norbert van Bolhuis (NETHERLANDS)
Message: 57925
I have a serious doubt regarding the AD1836 driver, maybe it's a bug, maybe not.
I hope somebody can answer this.
The problem regards this piece of code:
1281 #ifdef CONFIG_SND_BLACKFIN_AD1836_TDM
1282 static int snd_ad1836_playback_copy(struct snd_pcm_substream *substream, int channel,
1283 snd_pcm_uframes_t pos, void *src, snd_pcm_uframes_t count)
1284 {
.
.
1295 if (index > 0 && index <=2 && !(chip->tx_status & (1<<index))) {
1296 sub_info->data_count += count;
1297 return 0;
1298 }
1299
The if statement starting on line 1295 says not to copy any playback
data if DMA_TX did not start yet. Why is that ?
DMA_TX starts <b class="moz-txt-star">*after*</b> snd_ad1836_playback_copy is called
Normally the order in which ALSA driver functions are called is:
-1- snd_ad1836_playback_open
-2- snd_ad1836_hw_params
-3- snd_ad1836_playback_prepare
-4- playback_copy
-5- snd_ad1836_playback_trigger (cmd=start)
-6- <IRQ domain> snd_ad1836_playback_pointer
Because of the if statement on line 1295 no playback data is copied at
step -4-. This means DMA exchanges nonsense playback data at step -5-.
QuoteReplyEditDelete
2008-06-30 05:31:47 回复： AD1836 driver question (snd_ad1836_playback_copy)
Cliff Cai (CHINA)
Message: 58103
AD1836 support to play up to 3 stereo PCM streams at the same time.The DMA is always running on the ring buffer after the NO.0 stream started.So I think it's reasonable to fill the buffer(slot) after the NO.1/NO.2 stream starts.Of course,there will be at least one period of data lost,but I think we have no other chioce,since we only employ one DMA engine to implement playing 3 separate PCM stream.
Thanks
Cliff
QuoteReplyEditDelete
2008-06-30 09:21:52 Re: 回复： AD1836 driver question (snd_ad1836_playback_copy)
Norbert van Bolhuis (NETHERLANDS)
Message: 58118
I think I understand what you're saying. I don't see the problem though.
In short: snd_ad1836_playback_copy has a substream parameter. The ALSA
core will call snd_ad1836_playback_copy only if data should be copied
for this particular substream (when DMA started or is about to start), isn't it ?
Suppose DMA_TX starts for stereo substream #0, #1 and #2.
Now snd_ad1836_playback_copy is called regularly for all 3 substreams,
this is because the snd_ad1836_playback_copy function is called by
the ALSA core with a substream parameter indicating for what substream
data should be copied.
After a while DMA_TX stops for substream #0 and #1.
Now DMA_TX is still running, snd_ad1836_playback_copy is called regularly
and it shouldn't copy data for substream #0 and #1 (since the're stopped).
This is no problem since snd_ad1836_playback_copy is called (by ALSA core)
only for substream #2.
Therefore I'd say checking whether DMA runs for a particular substream
isn't necessary.
I'd change the if statement in:
1295 if (index > 0 && index <=2) {
QuoteReplyEditDelete
2008-07-01 00:00:05 回复： Re: 回复： AD1836 driver question (snd_ad1836_playback_copy)
Cliff Cai (CHINA)
Message: 58144
I think your change will cause no data be filled to the relevant buffer slot for #1,#2 substream.
supposing snd_ad1836_playback_copy is called for #1 substream at the first time,the data then will be fed to DAC immediately,because the DMA has been started by #0 substream. While at this time maybe snd_ad1836_playback_trigger (cmd=start) hasn't been called yet.I think it 's not a reasonable sequence and this is why the if statement is here.
But actually there is no audiable difference from using driver with or without the if statement.
Best Regards
Cliff
QuoteReplyEditDelete
2008-07-01 12:07:37 Re: 回复： Re: 回复： AD1836 driver question (snd_ad1836_playback_copy)
Norbert van Bolhuis (NETHERLANDS)
Message: 58161
thx (again) for your response.
Your example exactly shows when the "problem" occurs:
stream #0 is running (DMA started) and snd_ad1836_playback_copy for stream #0
is called frequently.
stream #1 is about to run. snd_ad1836_playback_copy for stream #1 is called,
but snd_ad1836_playback_trigger (cmd=start) for stream #1 hasn't been called yet.
I guess in this case you should copy data to the DMA buffer since alsa-core will soon call
snd_ad1836_playback_trigger (cmd=start) for stream #1. Besides, it won't copy to a location
which is immediately transferred to the DAC. As you know the user data to be copied runs
ahead of the DMA operations, therefore it is copied to a location in the DMA buffer which will
be exchanged with DMA in a few periods or so.
Also, I think you can trust alsa-core, it won't call snd_ad1836_playback_copy for stream x
without starting or having started DMA for stream #1.
On the other hand, I don't know alsa-core too well. Maybe there are cases in which
the copy function is called while DMA is not near running.
anyway, the reason why this if startement is the way it is, is clear to me now.
It only affects (max) 1 period as you already mentioned.
Did you actually test the changed if statement with an AD1836 (and the
result was no audible difference) ?
QuoteReplyEditDelete
2008-07-01 23:20:21 回复： Re: 回复： Re: 回复： AD1836 driver question (snd_ad1836_playback_copy)
Cliff Cai (CHINA)
Message: 58173
I said "at least one peroid"
Actually the "copy" callback will be called before "trigger" at the beginning.
It's easy to see this by inserting printk in the code.
I've tried to play a very short ring tone and a mp3 file in #1 substream,it's sure that there is no audiable difference.
Best Regards
Cliff