Re: Bad PCM stream after a suspend/resume cycle

[Takashi Iwai <tiwai@suse.de>]

On Thu, 18 Jan 2018 09:49:53 +0100,
Mirza Krak wrote:
> 
> On 17 January 2018 at 16:05, Takashi Iwai <tiwai@suse.de> wrote:
> > On Wed, 17 Jan 2018 15:08:27 +0100,
> > Mirza Krak wrote:
> >>
> >> On 12 January 2018 at 19:04, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Fri, 12 Jan 2018 14:36:45 +0100,
> >> > Mirza Krak wrote:
> >> >>
> >>
> >> < snip >
> >>
> >> >
> >> > The EBADFD is already indicating a fatal error, and something is wrong
> >> > in the driver.  Basically the driver should suspend the stream via
> >> > snd_pcm_suspend*() call in the PM resume ops.  Most likely your driver
> >> > misses that.
> >> >
> >> > That said, it's specific to your using driver, and you'd need to take
> >> > a look at that code there.
> >>
> >> Thank you for you patience with me.
> >>
> >> I have looked further in to my hardware drivers and I can not really
> >> see any faults here.
> >>
> >> But I did some further testing and applying the following diff on
> >> aplay "resolves" the problem (v1.1.4 of alsa-utils):
> >>
> >> diff --git a/aplay/aplay.c b/aplay/aplay.c
> >> index f793c82..040cec1 100644
> >> --- a/aplay/aplay.c
> >> +++ b/aplay/aplay.c
> >> @@ -2020,6 +2020,8 @@ static ssize_t pcm_write(u_char *data, size_t count)
> >>                 if (test_position)
> >>                         do_test_position();
> >>                 if (r == -EAGAIN || (r >= 0 && (size_t)r < count)) {
> >> +                       if (snd_pcm_state(handle) == SND_PCM_STATE_SUSPENDED)
> >> +                               suspend();
> >>                         if (!test_nowait)
> >>                                 snd_pcm_wait(handle, 100);
> >>                 } else if (r == -EPIPE) {
> >>
> >> Which means that there is no error in regard to suspending the stream
> >> as it properly reports this when checked.
> >>
> >> My problem is that this condition:
> >>
> >>     (r >= 0 && (size_t)r < count)
> >>
> >> Is always true after a suspend/resume cycle. Which normally does not
> >> result in a "resume" call from aplay nor from snd_pcm_recover, which
> >> means that it will try to write data to a suspended stream which
> >> results in -EBADFD due to this (from alsa-lib):
> >>
> >> snd_pcm_sframes_t snd_pcm_writei(snd_pcm_t *pcm, const void *buffer,
> >> snd_pcm_uframes_t size)
> >> {
> >>     < snip >
> >>
> >>     if (bad_pcm_state(pcm, P_STATE_RUNNABLE))
> >>         return -EBADFD;
> >>     return _snd_pcm_writei(pcm, buffer, size);
> >> }
> >>
> >> Because SUSPENDED is not part of the P_STATE_RUNNABLE, which maybe it should be?
> >
> > OK, that's a bug in alsa-lib, then.  The sanity check is good, but it
> > returns the inconsistent error code that doesn't match with the PCM
> > state.
> >
> > Could you try the patch below?
> 
> Yeah, the patch resolved my issues and the stream is resumed/reset
> after suspend properly. Tested with aplay and with custom application
> that uses snd_pcm_recover to handle return values from write.
> 
> You can add my:
> 
> Tested-by: Mirza Krak <mirza.krak@gmail.com>
> 
> If you like.

Good to hear, I pushed the fix to git tree now.


thanks,

Takashi