On Thu, Oct 10, 2013 at 01:50:54PM +0800, Qiao Zhou wrote:
- In dma driver, once terminate_all in invoked, grab the lock, disable the
tasklet, pause/stop the dmaengine remove all the descriptors from the lists. This ensures that dmaengine doesnt trigger anything new. And if it does we dont call into client
what lock do you refer to? is it "snd_pcm_stream_lock" or a new one in dma driver?
We are ref dma driver so it would be a dma driver lock.
- If we get an interrupt or tasklet invoked after this, then it is the
resposiblity of dma driver to clear interrupt and return
- While you have invoked the terminate_all you might get a callback, in that
case the substream is still valid (you are still in TRIGGER_STOP). There should be no harm in calling period_elapsed, but it would be good if we detect that and return from here.
- My only worry is that during callback we drop the locks held, so callback can
be running on different CPU while you process the terminate all. This is very racy and possibly the issue being seen in this thread. This gets complicated by that fact that xrun would invoke the stop thus terminate_all.
The timing is very racy. we have two platforms, of which the only difference is that one is 2 * a9 cpu, and the other is 4 * a7 cpu. all other components and peripherals are the same. The result is we can't reproduce the panic issue after more than 4 days stress test on 2-cpu platform, but can reproduce the issue in ~10 hours level on the 4-cpu platform.
The only reason I see if dma driver is NOT buggy is that you dma driver already invoked callback and on different CPU you decided to terminate the audio and call terminate_all
On the other hand that last part could get tricky as the dmaengine_terminate_all() might be call from within the callback.
It's tricky indeed in case xrun happens. we should avoid possible deadlock.
I think we'll eventually need to versions of dmaengine_terminate_all(). A sync version which makes sure that the tasklet has finished and a non-sync version that only makes sure that no new callbacks are started. I think the sync version should be the default with an optional async version which must be used, if it can run from within the callback. So we'd call the async version in the pcm_trigger callback and the sync version in the pcm_close callback.
Yes this can be done. We can name this disable_callback cmd. The cmd will tell dma driver to disable all callback on the channel. This can be invoked from the TRIGEGR_STOP and then terminate_all in the free
Which dma driver are you guys using in this? I will send a patch for the core and pcm layer. Someone need to test on actual hardware with driver fix :)
I'm using the mmp_tdma driver under /drivers/dma/, and I can test the patch on our 4-cpu platform. thanks.
Ok, let me check the driver and also see what needs to be done for this
-- ~Vinod