From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH] dma: pl330: Add support for DMA_PAUSE command Date: Mon, 19 May 2014 07:57:47 +0200 Message-ID: <53799D5B.2060001@metafoo.de> References: <1400037830-21211-1-git-send-email-tushar.behera@linaro.org> <5373607D.2000200@metafoo.de> <537513A8.701@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Tushar Behera Cc: Jassi Brar , Linux Kernel Mailing List , dmaengine@vger.kernel.org, Vinod Koul , Dan Williams , Linux-ALSA , Takashi Iwai List-Id: alsa-devel@alsa-project.org On 05/19/2014 05:10 AM, Tushar Behera wrote: > On 16 May 2014 00:51, Lars-Peter Clausen wrote: >> On 05/15/2014 02:01 PM, Tushar Behera wrote: >>> >>> On 14 May 2014 17:54, Lars-Peter Clausen wrote: >>>> >>>> On 05/14/2014 02:07 PM, Tushar Behera wrote: >>>>> >>>>> >>>>> On 14 May 2014 17:29, Jassi Brar wrote: >>>>>> >>>>>> >>>>>> On Wed, May 14, 2014 at 8:53 AM, Tushar Behera >>>>>> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> While playing back audio, pmc_dmaengine requests the DMA channel to >>>>>>> stop DMA transmission through DMA_PAUSE command. >>>>>>> >>>>>>> Currently PL330 driver doesn't support DMA pause command, leaving >>>>>>> the DMA state inconsistent when the system resumes. Instead, it would >>>>>>> be better to terminate the DMA transfer during suspend and restart >>>>>>> again during resume. >>>>>>> >>>>>>> Tested with audio playback across a suspend-resume cycle. >>>>>>> >>>>>> What is pmc_dmaengine? How does DMA_PAUSE help, when there is no >>>>>> DMA_RESUME? >>>>>> >>>>> >>>>> Sorry, it is a typo. >>>>> >>>>> sound/core/pcm_dmaengine.c:snd_dmaengine_pcm_trigger() --> >>>>> dmaengine_pause() is called during system suspend. >>>> >>>> >>>> >>>> It is only called if the DMA driver has support for pausing and resuming >>>> DMA >>>> transfers. Or at least that is the intention. >>>> >>>> - Lars >>> >>> >>> During suspend, snd_dmaengine_pcm_trigger():SNDRV_PCM_TRIGGER_SUSPEND >>> is called which unconditionally calls dmaengine_pause(). Should we >>> update snd_dmaengine_pcm_trigger() to check for DMA pause/resume >>> support and call dmaengine_pause() or dmaengine_terminate_all() >>> accordingly? >> >> >> As far as I understand it we do not have to do anything for TRIGGER_SUSPEND >> if we do not set the SNDRV_PCM_INFO_RESUME flag. It looks like >> TRIGGER_SUSPEND is called unconditionally during suspend. But since the >> error code is ignored it should be fine if we just call dmaengine_pause() >> and that return -ENOSYS or similar. >> >> Are you seeing an actual issue that you are trying to fix with your patch? >> >> - Lars >> > > Without this patch applied, if audio is playing back while suspend is > triggered, it doesn't playback after resume. Stopping the stream and > replaying works. > Ok, I think your second suggestion was correct. Call dmaengine_terminate_all() instead of damengine_pause() in snd_dmaengine_pcm_trigger() if the dmaengine driver does not support pausing the transfer. - Lars