Re: [PATCH] pcm: Don't store the state for SND_PCM_STATE_SUSPENDED

[Shengjiu Wang <shengjiu.wang@nxp.com>]

Hi

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, May 10, 2016 4:22 PM
> To: Shengjiu Wang
> Cc: perex@perex.cz; alsa-devel@alsa-project.org
> Subject: Re: [PATCH] pcm: Don't store the state for
> SND_PCM_STATE_SUSPENDED
> 
> On Tue, 10 May 2016 09:45:46 +0200,
> Shengjiu Wang wrote:
> >
> > The resume function don't update the dmix->state, if store SUSPENDED
> > state in snd_pcm_dmix_state, the write function after resume will
> > return error -ESTRPIPE, because the snd_pcm_write_areas() will check
> > the state of the pcm device.
> > This patch remove the store SND_PCM_STATE_SUSPENDED state operation
> > for dmix,dshare,dsnoop.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> 
> What's the exact problem you're seeing on surface?  Could illustrate
> how the bug is triggered and how to reproduce easily?  It'll make
> easier to understand what you're trying to fix.
> 
> 
> thanks,
> 
> Takashi

The aplay endlessly print " Suspended. Trying resume. Done." if suspend
and resume in the middle of playback. Which is caused by this patch.

commit 8985742d91dbdd74b2f605374207473393454fff
Author: Takashi Iwai <tiwai@suse.de>
Date:   Fri Oct 30 17:13:50 2015 +0100

    pcm: dmix: Handle slave PCM xrun and unexpected states properly 

This patch store the SUSPENDED state to dmix->state, but after resume
the dmix->state still is SUSPENDED, next write function will check the
state, if state is SUSPENDED, it will return -ESTRPIPE, then the aplay
will print another " Suspended. Trying resume. Done."  Then repeat this
behavior again and again.

Best regards
Wang shengjiu
> 
> > ---
> >  src/pcm/pcm_dmix.c   | 2 +-
> >  src/pcm/pcm_dshare.c | 2 +-
> >  src/pcm/pcm_dsnoop.c | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
> > index 007d356..66bb288 100644
> > --- a/src/pcm/pcm_dmix.c
> > +++ b/src/pcm/pcm_dmix.c
> > @@ -451,9 +451,9 @@ static snd_pcm_state_t
> snd_pcm_dmix_state(snd_pcm_t *pcm)
> >  	state = snd_pcm_state(dmix->spcm);
> >  	switch (state) {
> >  	case SND_PCM_STATE_XRUN:
> > -	case SND_PCM_STATE_SUSPENDED:
> >  	case SND_PCM_STATE_DISCONNECTED:
> >  		dmix->state = state;
> > +	case SND_PCM_STATE_SUSPENDED:
> >  		return state;
> >  	default:
> >  		break;
> > diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
> > index adb3587..a133c72 100644
> > --- a/src/pcm/pcm_dshare.c
> > +++ b/src/pcm/pcm_dshare.c
> > @@ -241,9 +241,9 @@ static snd_pcm_state_t
> snd_pcm_dshare_state(snd_pcm_t *pcm)
> >  	state = snd_pcm_state(dshare->spcm);
> >  	switch (state) {
> >  	case SND_PCM_STATE_XRUN:
> > -	case SND_PCM_STATE_SUSPENDED:
> >  	case SND_PCM_STATE_DISCONNECTED:
> >  		dshare->state = state;
> > +	case SND_PCM_STATE_SUSPENDED:
> >  		return state;
> >  	default:
> >  		break;
> > diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
> > index 8ff0ba5..d56dd97 100644
> > --- a/src/pcm/pcm_dsnoop.c
> > +++ b/src/pcm/pcm_dsnoop.c
> > @@ -205,9 +205,9 @@ static snd_pcm_state_t
> snd_pcm_dsnoop_state(snd_pcm_t *pcm)
> >  	state = snd_pcm_state(dsnoop->spcm);
> >  	switch (state) {
> >  	case SND_PCM_STATE_XRUN:
> > -	case SND_PCM_STATE_SUSPENDED:
> >  	case SND_PCM_STATE_DISCONNECTED:
> >  		dsnoop->state = state;
> > +	case SND_PCM_STATE_SUSPENDED:
> >  		return state;
> >  	default:
> >  		break;
> > --
> > 1.9.1
> >