[alsa-devel] 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