On Mon, Mar 16, 2015 at 11:02:37AM +0530, Vinod Koul wrote: > On Fri, Mar 13, 2015 at 02:21:50PM +0100, Maxime Ripard wrote: > > Hi, > > > > On Fri, Mar 13, 2015 at 01:48:49PM +0530, Vinod Koul wrote: > > > On Mon, Mar 09, 2015 at 11:18:53PM +0100, Robert Jarzmik wrote: > > > > Hi Vinod, > > > > > > > > I've been writing the dmaengine slave driver for pxa architecture lately. I have > > > > only one remaining concern with descriptors reusing, and I need your input. > > > > > > > > My usecase is as follows, from a driver point of view : > > > > - tx = dmaengine_prep_slave_sg(dma_chan, sg, sglen, DMA_PREP_CONTINUE); > > > > - dma_map_sg(dev, sg, sglen, DMA_DEV_TO_MEM) > > > > - dmaengine_submit(tx) > > > > - dma_async_issue_pending(dma_chan); > > > > - ... wait for tx->callback() > > > > => here, because of DMA_PREP_CONTINUE, the hardware descriptors are not > > > > freed, as what I need to reuse > > > > > > > > - dma_sync_sg_for_cpu(dev, sg, sglen, ) > > > > - copy video buffer somewhere (or not if frame interleaving) > > > > > > > > - dmaengine_submit(tx) > > > > - dma_async_issue_pending(dma_chan) > > > > => the video buffer is captured again, so far so good > > > > - ... wait for tx->callback() > > > > > > > > And here comes my question : how do I free the hardware descriptors for tx now ? > > > > As I chose to not free them upon tx completion, is there an API call to do it ? > > > The problem with this is that we are assuming the > > > DMA_PREP_CONTINUE/DMA_PREP_ACK etc is valid for slave. These were defined > > > for async_tx. I have added Dan on how they handled it there > > > > > > But, I think we should not use these for slave transactions. > > > > I think we should really stop treating async_tx as a special case. > > It would be good and simpler but there are quite a bit differenace which > cause this assumption to stand out. For me the prominent among them is > buffer mapping. For slave-dma we expect client to map the buffers, but > async api relies on dmaengine to do the mapping. So, for a single user, we keep such undocumented and obscure cases? Wouldn't it make sense to fix that user instead, to have a clear and consistent behaviour? And that wouldn't introduce more code, just move it around. > > It just creates a combination of confusion (like seen above), > > brokenness (DMA_PRIVATE), and duplicated code (dma_find_channel vs > > dma_request_channel). > > > > If we need some features for async_tx, let's had it to the framework, > > and make it available to all the cases. This will make things better > > for everyone. > Bringing additional features is ok, but brining needs to justify why it > can't be done with current API. My point wasn't really that we wanted new features, but rather that existing features should be applicable to anybody. We might very well encounter cases (just like the one that started this thread) that need the exact same feature for the slave devices. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature
