| Message ID | 1462866346-11878-1-git-send-email-shengjiu.wang@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > --- > 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 >
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 > >
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;
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> --- 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(-)