On Fri, Jul 10, 2015 at 08:10:47PM +0200, Lars-Peter Clausen wrote: > On 07/10/2015 07:23 PM, Vinod Koul wrote: > >On Fri, Jul 10, 2015 at 07:06:55PM +0200, Lars-Peter Clausen wrote: > >>On 07/10/2015 07:01 PM, Vinod Koul wrote: > >>[...] > >>>>>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? > >> > >>No, this has nothing to do with cyclic DMA. The flag needs to be set > >>otherwise drivers with descriptor pools will consider the descriptor > >>to be in use, even if we are done using it and hence the it will run > >>out of descriptors eventually. > > > >> > >>> > >>>>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 > >> > >>It's not just sound. The recommendation has always been set this > >>flag unless you know why you are not setting it. So pretty much > >>every DMAengine client driver sets it. > >Actually there has never been a recommendation for slave cases. It came from async > >API and slave users should never have used it, untill now. > > That doesn't reflect reality. The fast majority slave API users set > the flag and some DMAengine drivers expect it to be set. That is simply wrong and we need to fix that > > > > >Now to solve the mess, either we can revert this patch and find a new one > >for actual reuse of descriptors. Alternate would be fix users > > > >I am leaning towards former then follow up by fixing the users who dont want > >this. Since this is going to be a tree wide update it will need one merge > >cycle to complete. Once done we can bring back the usage required for actual > >descriptor reuse case. This will lead only one broken driver from Robert. Is > >the client for your driver merged? > > Just blindly removing all the existing users wont work, you'd have > to carefully review each case and see if anything potentially breaks > by removing it. This will probably take longer than one cycle. > Repurposing a flag that is so wildly used with completely different > semantics in such a short timespan will cause all kinds of issues. NO the idea is *not* to blindly remove BUT fix the users to do the right thing... -- Vinod > > This should really be a new flag with clearly defined semantics. > There is nothing gained from reusing DMA_CTRL_ACK it only makes it a > lot more difficult. > > - Lars > > -- > 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 -- -- 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
