From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] pcm: Don't store the state for SND_PCM_STATE_SUSPENDED Date: Sat, 28 May 2016 10:46:06 +0200 Message-ID: References: <1462866346-11878-1-git-send-email-shengjiu.wang@freescale.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 770AB2604A9 for ; Sat, 28 May 2016 10:46:07 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Shengjiu Wang Cc: "alsa-devel@alsa-project.org" List-Id: alsa-devel@alsa-project.org On Tue, 24 May 2016 12:18:18 +0200, Takashi Iwai wrote: > > On Tue, 24 May 2016 12:12:49 +0200, > Shengjiu Wang wrote: > > > > Hi > > > > > -----Original Message----- > > > From: Takashi Iwai [mailto:tiwai@suse.de] > > > Sent: Friday, May 20, 2016 10:32 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 Fri, 20 May 2016 12:46:37 +0200, > > > Takashi Iwai wrote: > > > > > > > > On Fri, 20 May 2016 11:41:25 +0200, > > > > Shengjiu Wang wrote: > > > > > > > > > > Hi Takashi > > > > > > > > > > I tested your patch, after suspend and resume, the playback is > > > stopped. > > > > > It is caused by the DMA. DMA is not started after resume. > > > > > > > > > > With your patch, DMA is not terminated but then is re-started. The > > > driver don't > > > > > support this behavior. > > > > > > > > If so, it's simply a driver bug. Blame the kernel driver instead. > > > > > > Which driver did you see the problem? We should fix it. > > > > But my thought is when suspended, the dmaengine_pause() is called, then > > dmaengine_resume() should be called in resume(). If there is no resume() > > Just call the prepare() and start(), it seems not reasonable. What do > > you think? > > There are several ways to fix the problem, but the point is that, from > the API POV, the direct state change from SUSPENDED to PREPARED is > valid. So, the kernel driver has to support such a state change, no > matter how. > > An easier way would be to add a check and some trigger in PCM core > side. OTOH, this would affect effectively all drivers, thus we'd need > a wider test coverage, too. > > Judging from your comment, the broken driver is ASoC one, right? Thinking of this again, I'm inclined to have a workaround for such buggy drivers. In the end, alsa-lib should work for older kernels, too. Does the patch below work on your device? Maybe better to clear the buffer beforehand for avoiding the unnecessary noise. But it can be done later. Takashi --- From: Takashi Iwai Subject: [PATCH] pcm: dmix: resume workaround for buggy driver The previous commit removed the whole handling of resume in dmix, but this seems causing another regression; some buggy drivers assume that the device-resume needs to be triggered before transitioning to PREPARED state. As an ugly workaround, in this patch, when the slave PCM supports resume, snd_pcm_direct_resume() does resume of the slave PCM but immediately drop the stream after that. In that way, the device is brought to the sane active state, then the apps can prepare and restart the stream properly. Signed-off-by: Takashi Iwai --- src/pcm/pcm_direct.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 53c49929cb1f..169758d18a29 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -837,6 +837,21 @@ int snd_pcm_direct_prepare(snd_pcm_t *pcm) int snd_pcm_direct_resume(snd_pcm_t *pcm) { + snd_pcm_direct_t *dmix = pcm->private_data; + + snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT); + /* some buggy drivers require the device resumed before prepared; + * when a device has RESUME flag and is in SUSPENDED state, resume + * here but immediately drop to bring it to a sane active state. + */ + if ((dmix->spcm->info & SND_PCM_INFO_RESUME) && + snd_pcm_state(dmix->spcm) == SND_PCM_STATE_SUSPENDED) { + snd_pcm_resume(dmix->spcm); + snd_pcm_drop(dmix->spcm); + snd_pcm_direct_timer_stop(dmix); + snd_pcm_direct_clear_timer_queue(dmix); + } + snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT); return -ENOSYS; } @@ -845,7 +860,7 @@ int snd_pcm_direct_resume(snd_pcm_t *pcm) /* copy the slave setting */ static void save_slave_setting(snd_pcm_direct_t *dmix, snd_pcm_t *spcm) { - spcm->info &= ~(SND_PCM_INFO_PAUSE | SND_PCM_INFO_RESUME); + spcm->info &= ~SND_PCM_INFO_PAUSE; COPY_SLAVE(access); COPY_SLAVE(format); @@ -874,6 +889,8 @@ static void save_slave_setting(snd_pcm_direct_t *dmix, snd_pcm_t *spcm) COPY_SLAVE(buffer_time); COPY_SLAVE(sample_bits); COPY_SLAVE(frame_bits); + + dmix->shmptr->s.info &= ~SND_PCM_INFO_RESUME; } #undef COPY_SLAVE -- 2.8.3