diff mbox

Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

Message ID 87lhp6n70k.wl%kuninori.morimoto.gx@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kuninori Morimoto Sept. 26, 2014, 8:03 a.m. UTC
Hi Clemens

Thank you for your explain

> > We noticed that DMA seems transfered +1 time when Ctrl-C happen.
> > But, is this correct ? is this our driver bug ?
> > [...]
> > 7. DMA transfer interrupt happen
> >
> >    It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte
> >    snd_pcm_playback_avail() in snd_pcm_update_state() return 8192 this time.
> >    then, it calls snd_pcm_drain_done()
> >
> > 9. snd_soc_dai_ops :: trigger called with SNDRV_PCM_TRIGGER_STOP
> >
> >    driver stops DMA transfer
> 
> There is no strong synchronization between snd_pcm_drain() and the rest
> of the system; snd_pcm_drain() just waits for an underrun to happen.
> 
> In other words, the last actual DMA transfer is likely to contain
> invalid (outdated) samples, but gets aborted immediately.

I wonder why we need drain ?
This "likely to contain invalid samples" will be solved if
we can skip "complete drain" ?

-------------------
-------------------

Best regards
---
Kuninori Morimoto

Comments

Jaroslav Kysela Sept. 26, 2014, 8:15 a.m. UTC | #1
Dne 26.9.2014 v 10:03 Kuninori Morimoto napsal(a):
> 
> Hi Clemens
> 
> Thank you for your explain
> 
>>> We noticed that DMA seems transfered +1 time when Ctrl-C happen.
>>> But, is this correct ? is this our driver bug ?
>>> [...]
>>> 7. DMA transfer interrupt happen
>>>
>>>    It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte
>>>    snd_pcm_playback_avail() in snd_pcm_update_state() return 8192 this time.
>>>    then, it calls snd_pcm_drain_done()
>>>
>>> 9. snd_soc_dai_ops :: trigger called with SNDRV_PCM_TRIGGER_STOP
>>>
>>>    driver stops DMA transfer
>>
>> There is no strong synchronization between snd_pcm_drain() and the rest
>> of the system; snd_pcm_drain() just waits for an underrun to happen.
>>
>> In other words, the last actual DMA transfer is likely to contain
>> invalid (outdated) samples, but gets aborted immediately.
> 
> I wonder why we need drain ?
> This "likely to contain invalid samples" will be solved if
> we can skip "complete drain" ?

The user app should do the flush instead the drain in this case...

					Jaroslav
Kuninori Morimoto Sept. 26, 2014, 8:45 a.m. UTC | #2
Hi Jaroslav

> >>> We noticed that DMA seems transfered +1 time when Ctrl-C happen.
> >>> But, is this correct ? is this our driver bug ?
> >>> [...]
> >>> 7. DMA transfer interrupt happen
> >>>
> >>>    It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte
> >>>    snd_pcm_playback_avail() in snd_pcm_update_state() return 8192 this time.
> >>>    then, it calls snd_pcm_drain_done()
> >>>
> >>> 9. snd_soc_dai_ops :: trigger called with SNDRV_PCM_TRIGGER_STOP
> >>>
> >>>    driver stops DMA transfer
> >>
> >> There is no strong synchronization between snd_pcm_drain() and the rest
> >> of the system; snd_pcm_drain() just waits for an underrun to happen.
> >>
> >> In other words, the last actual DMA transfer is likely to contain
> >> invalid (outdated) samples, but gets aborted immediately.
> > 
> > I wonder why we need drain ?
> > This "likely to contain invalid samples" will be solved if
> > we can skip "complete drain" ?
> 
> The user app should do the flush instead the drain in this case...

Thank you about this help.
Can you show me how to do this in app side ?
(Calling some alsa-lib function ?)

Best regards
---
Kuninori Morimoto
Jaroslav Kysela Sept. 26, 2014, 10:07 a.m. UTC | #3
Dne 26.9.2014 v 10:45 Kuninori Morimoto napsal(a):
> 
> Hi Jaroslav
> 
>>>>> We noticed that DMA seems transfered +1 time when Ctrl-C happen.
>>>>> But, is this correct ? is this our driver bug ?
>>>>> [...]
>>>>> 7. DMA transfer interrupt happen
>>>>>
>>>>>    It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte
>>>>>    snd_pcm_playback_avail() in snd_pcm_update_state() return 8192 this time.
>>>>>    then, it calls snd_pcm_drain_done()
>>>>>
>>>>> 9. snd_soc_dai_ops :: trigger called with SNDRV_PCM_TRIGGER_STOP
>>>>>
>>>>>    driver stops DMA transfer
>>>>
>>>> There is no strong synchronization between snd_pcm_drain() and the rest
>>>> of the system; snd_pcm_drain() just waits for an underrun to happen.
>>>>
>>>> In other words, the last actual DMA transfer is likely to contain
>>>> invalid (outdated) samples, but gets aborted immediately.
>>>
>>> I wonder why we need drain ?
>>> This "likely to contain invalid samples" will be solved if
>>> we can skip "complete drain" ?
>>
>> The user app should do the flush instead the drain in this case...
> 
> Thank you about this help.
> Can you show me how to do this in app side ?
> (Calling some alsa-lib function ?)

Yes, the function is snd_pcm_drop() .

				Jaroslav
Kuninori Morimoto Sept. 26, 2014, 10:31 a.m. UTC | #4
Hi Jaroslav

> >>>>> We noticed that DMA seems transfered +1 time when Ctrl-C happen.
> >>>>> But, is this correct ? is this our driver bug ?
> >>>>> [...]
> >>>>> 7. DMA transfer interrupt happen
> >>>>>
> >>>>>    It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte
> >>>>>    snd_pcm_playback_avail() in snd_pcm_update_state() return 8192 this time.
> >>>>>    then, it calls snd_pcm_drain_done()
> >>>>>
> >>>>> 9. snd_soc_dai_ops :: trigger called with SNDRV_PCM_TRIGGER_STOP
> >>>>>
> >>>>>    driver stops DMA transfer
> >>>>
> >>>> There is no strong synchronization between snd_pcm_drain() and the rest
> >>>> of the system; snd_pcm_drain() just waits for an underrun to happen.
> >>>>
> >>>> In other words, the last actual DMA transfer is likely to contain
> >>>> invalid (outdated) samples, but gets aborted immediately.
> >>>
> >>> I wonder why we need drain ?
> >>> This "likely to contain invalid samples" will be solved if
> >>> we can skip "complete drain" ?
> >>
> >> The user app should do the flush instead the drain in this case...
> > 
> > Thank you about this help.
> > Can you show me how to do this in app side ?
> > (Calling some alsa-lib function ?)
> 
> Yes, the function is snd_pcm_drop() .

Thank you !

Best regards
---
Kuninori Morimoto
diff mbox

Patch

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 6f3e10c..841a5e0 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -501,7 +501,7 @@  int snd_pcm_status(struct snd_pcm_substream *substream,
                   struct snd_pcm_status *status);
 int snd_pcm_start(struct snd_pcm_substream *substream);
 int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
-int snd_pcm_drain_done(struct snd_pcm_substream *substream);
+int snd_pcm_drain_abort(struct snd_pcm_substream *substream);
 #ifdef CONFIG_PM
 int snd_pcm_suspend(struct snd_pcm_substream *substream);
 int snd_pcm_suspend_all(struct snd_pcm *pcm);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 9acc77e..fbdbbde5 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -286,10 +286,8 @@  int snd_pcm_update_state(struct snd_pcm_substream *substream,
        if (avail > runtime->avail_max)
                runtime->avail_max = avail;
        if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
-               if (avail >= runtime->buffer_size) {
-                       snd_pcm_drain_done(substream);
-                       return -EPIPE;
-               }
+               snd_pcm_drain_abort(substream);
+               return -EPIPE;
        } else {
                if (avail >= runtime->stop_threshold) {
                        xrun(substream);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 8cd2f93..f3c48de 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -977,7 +977,7 @@  EXPORT_SYMBOL(snd_pcm_stop);
  *
  * Return: Zero if succesful, or a negative error code.
  */
-int snd_pcm_drain_done(struct snd_pcm_substream *substream)
+int snd_pcm_drain_abort(struct snd_pcm_substream *substream)
 {
        return snd_pcm_action_single(&snd_pcm_action_stop, substream,
                                     SNDRV_PCM_STATE_SETUP);