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: Tue, 31 May 2016 09:47:31 +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 18A192654C3 for ; Tue, 31 May 2016 09:47:32 +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, 31 May 2016 09:30:49 +0200, Shengjiu Wang wrote: > > Hi > > > 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? > > > No, the driver is in dma folder, path is /drivers/dma/. We use the > /drivers/dma/imx-sdma.c But the driver in community is old. We have > updated the dma driver to support virtual-dma, just like the > drivers/dma/omap-dma.c. Then it's even less interesting to us. The stuff should be upstreamed at first :) > The c->desc should be released in terminate_all() function, if it not > Released, the issue_pending() will not go to start dma. > > I can't find a good way to fix this issue in dma driver. Well it's a clearly driver bug. Per API definition, there is no guarantee that RESUME is performed always after SUSPEND, but it can be DROP. We may add some coverage in the alsa-lib like my patch, but it doesn't guarantee that all user-space behave in that way. So, hardening the kernel behavior is inevitable. In anyway, still waiting for your test result of my patch. thanks, Takashi