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.
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.
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
