On Fri, Jul 10, 2015 at 06:05:22PM +0200, Robert Jarzmik wrote: > Lars-Peter Clausen <lars@xxxxxxxxxx> writes: > > > On 07/10/2015 01:40 PM, Jun Nie wrote: > >>>> Furthermore audio DMA uses cyclic DMA, so it doesn't even hit this code > >>>> path, so the patch description doesn't really add up to the changes. > > Actually I'd like to know _exactly_ which code is hit there, which dmaengine > user. I have found that fsl_asrc_dma.c uses the flag "DMA_CTRL_ACK", without > using anywhere the async_ack() method (I might be wrong, I don't know that > code). It is used in dmaengine_pcm_prepare_and_submit() in sound/core/pcm_dmaengine.c > > The question behind is : which driver has a regression, for what purpose is it > setting the DMA_CTRL_ACK bit ? That would help to know if I used a bit whose > purpose what already well defined, or if it's just a driver which sets > DMA_CTRL_ACK without really needing it. > > If we find that DMA_CTRL_ACK has a specific meaning, or a "dmaengine driver" > specific meaning (ie. not usable by dmaengine clients), then another flag will > probably be necessary. > > >>> Sorry, it does. Looked at the wrong line. Reverting b9855f03d560 is probably > >>> the right thing to do as it breaks the existing semantics in a very bad way > >>> causing descriptors to be not freed when they should be. > >> I guess my change does not meet video requirement for patch > >> b9855f03d560, but I do not know detail video usage case related to > >> that patch. You can either revert the patch or confirm change in this > >> email is OK for the video case. > > > To handle the video case something else is needed. The current dmaengine > > semantics are that once a descriptor is passed to dma_engine_submit() it is > > owned by the DMAengine driver and the client must never ever touch it > > again. > > > Commit b9855f03d560 overloaded DMA_CTRL_ACK to mean that the should be > > re-usable by the client. But this breaks existing users of DMA_CTRL_ACK since > > its a completely new and different meaning to the existing one. For marking a > > descriptor as reusable for the client a new flag should probably be > > introduced. Lars, IIRC the usage in sound/ was to ensure we dont clean up the descriptor once the dma driver completes it so that we keep on using the same descriptor in cyclic fashion, right? > Maybe yes. > The problem will be to list the existing users of DMA_CTRL_ACK (dmaengine slave > API side), and see what is their expectation from this bit. > > From the discussion we had with Vinod, there was no clear definition so far for > DMA_CTRL_ACK (still for dmaengine slave API). I might have overlooked something > here, so any input will be valuable. I should have realised sound/ uses it and check this bit. Worst case we need another flag -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
