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

[Takashi Iwai <tiwai@suse.de>]

On Wed, 11 May 2016 08:24:55 +0200,
Shengjiu Wang wrote:
> 
> Hi
> 
> > On Wed, 11 May 2016 04:28:41 +0200,
> > Shengjiu Wang wrote:
> > >
> > > 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.
> > 
> > Thanks, this is exactly what I wanted to see in the changelog!
> > It's rather a regression, and it should be clearly mentioned.
> > 
> > Now about your fix: the problem is not about setting the correct
> > state at suspending.  It is suspended, so setting the right state
> > there is the correct behavior.  However, the counterpart, the resume,
> > is the culprit of this bug.  It misses the restore of the shadow
> > state.
> > 
> > Does the patch below work instead?
> > 
> Yes, it is workable.

Great, now I pushed the fix to git tree.
Thanks for your quick test!


Takashi