Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Kuninori Morimoto]

Hi Mark, ALSA ML

We noticed that DMA seems transfered +1 time when Ctrl-C happen.
But, is this correct ? is this our driver bug ?
our buffer_size is 8192, and DMA transfers 2048 in 1 time.

1. Ctrl-C happen

   wait_for_avail() of snd_pcm_lib_write1() returns -ERESTARTSYS immediately
   no transfer() are called any more.

2. DMA transfer interrupt happen

   This is normal from driver point of view
   It calls snd_pcm_period_elapsed() and try to transfer next 2048 byte

3. snd_pcm_common_ioctl1() :: SNDRV_PCM_IOCTL_DRAIN is called

   it goes to SNDRV_PCM_STATE_DRAINING status in snd_pcm_drain()
   and it wait until all drained.

4. 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 2048 this time.

5. 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 4096 this time.

6. 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 6144 this time.

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   

2) / 4) / 5) / 6) / 7) continue DMA transfer.
but, 1) already quited from snd_pcm_lib_write1() I guess.
So, 2) and 7) uses same buffer ?
What/Where is wrong ?

Best regards
---
Kuninori Morimoto

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Clemens Ladisch]

Kuninori Morimoto wrote:
> 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.


Regards,
Clemens

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Kuninori Morimoto]

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" ?

-------------------
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);
-------------------

Best regards
---
Kuninori Morimoto

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Jaroslav Kysela]

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

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Kuninori Morimoto]

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

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Jaroslav Kysela]

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

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

Re: Question about SNDRV_PCM_STATE_DRAINING and DMA transfer

[Kuninori Morimoto]

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