Re: Bad PCM stream after a suspend/resume cycle

From: Mirza Krak <mirza.krak@gmail.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: linux-rockchip@lists.infradead.org, alsa-devel@alsa-project.org,
	"Heiko Stübner" <heiko@sntech.de>
Subject: Re: Bad PCM stream after a suspend/resume cycle
Date: Thu, 18 Jan 2018 09:49:53 +0100	[thread overview]
Message-ID: <CALw8SCV4-6G24B1=kykv65+C7X7OfqY9LKGbZj5Di1s7SFxdVw@mail.gmail.com> (raw)
In-Reply-To: <s5h7esgwmno.wl-tiwai@suse.de>

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.

-- 
Med Vänliga Hälsningar / Best Regards

Mirza Krak
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel