From: maxime.ripard@free-electrons.com (Maxime Ripard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] Documentation: dmaengine: Add a documentation for the dma controller API
Date: Sat, 2 Aug 2014 21:05:15 +0200 [thread overview]
Message-ID: <20140802190515.GN3952@lukather> (raw)
In-Reply-To: <20140802152921.GC30282@n2100.arm.linux.org.uk>
On Sat, Aug 02, 2014 at 04:29:21PM +0100, Russell King - ARM Linux wrote:
> On Sat, Aug 02, 2014 at 05:11:44PM +0200, Maxime Ripard wrote:
> > On Fri, Aug 01, 2014 at 03:53:26PM +0100, Russell King - ARM Linux wrote:
> > > > > > - That might just be my experience, but judging from previous
> > > > > > commits, DMA_PRIVATE is completely obscure, and we just set it
> > > > > > because it was making it work, without knowing what it was
> > > > > > supposed to do.
> > > > >
> > > > > DMA_PRIVATE means that the channel is unavailable for async-tx operations
> > > > > - in other words, it's slave-only. Setting it before registration results
> > > > > in the private-use count being incremented, disabling the DMA_PRIVATE
> > > > > manipulation in the channel request/release APIs (requested channels are
> > > > > unavailable for async-tx operations, which is why that flag is also set
> > > > > there.)
> > > > >
> > > > > To put it another way, if a DMA engine driver only provides slave DMA
> > > > > support, it must set DMA_PRIVATE to mark the channel unavailable for
> > > > > async-tx operations. If a DMA engine drivers channels can also be used
> > > > > for async-tx operations, then it should leave the flag clear.
> > > >
> > > > What about channels that can be both used for slave operations and
> > > > async-tx (like memcpy) ?
> > >
> > > Then you don't set DMA_PRIVATE when the DMA engine driver registers with
> > > the core. That then allows the DMA engine core to manage the flag,
> > > setting it when the channel is allocated for slave usage.
> >
> > Then I guess we currently have a bug related to this.
> >
> > During the development of the A31 driver that recently got merged,
> > during two subsequent channel allocation, the second would fail if
> > DMA_PRIVATE wasn't set.
> >
> > I think it was on in dmaengine_get, but I'll have to get back to you
> > on that whenever I'm back from my holydays.
>
> I think you mean dma_chan_get(). dmaengine_get() is used to obtain
> references to the async_tx memcpy/crypto channels, and should not be used
> by a driver making use of the slave capabilities.
Like I said, I don't remember where the actual issue was lying, but
ignoring DMA_PRIVATE prevented to allocate two channels at the same
time in my case.
When I'll have access again to my board, I'll try to dig more into it.
> There two systems are slightly competitive between each other. Slave
> stuff generally want to get hold of specific channels, whereas the
> async_tx stuff can generally do with any channel which provides the
> appropriate capabilities, and can switch between channels if it needs
> to perform a series of operations only supported by separate channels.
>
> > You can also add vchan scheduling or descriptor allocations. The atmel
> > guys seem to have hit a performance issue with dma_pool_alloc, so they
> > re-developped a descriptor allocation mechanism in their driver. I
> > don't have much more details on this, but if that was to be true, that
> > would definitely be something that should be in the framework.
>
> It is entirely legal for a dmaengine driver to maintain its own list of
> "free" transaction descriptors (that's actually what many async-tx
> drivers do) and is partly why there's the alloc_chan_resources and
> free_chan_resources callbacks.
>
> However, they tend to be fairly large structures, and don't always
> match what the hardware wants, so using them to describe the individual
> scatterlist elements of a transaction is not always a good idea.
I was not really claiming it was illegal, but rather that it could be
useful for other drivers too.
> > vchan mapping to physical channels also seem to be quite common in the
> > drivers. That would be a nice addition too.
>
> It's intentionally not in the vchan interface because that mapping is
> device specific.
>
> For example, there are some DMA engine implementations where certain
> physical DMA channels should not be used because they have slightly
> different properties (for example, they can lock the CPU off the bus
> if they're permanently transferring data, such as in a memcpy or
> device-to-memory transfer where the device always has data available.)
>
> In any case, I'm not interested in seeing vchan turning into another
> "layer" - it must remain a library. Layers are bad news. :)
Yeah, I was assuming there would be such constraints. Still, handling
the trivial case where you can map any vchan to any free physical
channels is very painful, while at least a couple of drivers implement
the same dumb logic.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140802/035803e7/attachment.sig>
next prev parent reply other threads:[~2014-08-02 19:05 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-30 16:03 [PATCH] Documentation: dmaengine: Add a documentation for the dma controller API Maxime Ripard
2014-07-30 16:06 ` Vinod Koul
2014-07-31 7:44 ` Maxime Ripard
2014-07-31 11:56 ` Vinod Koul
2014-07-31 16:23 ` Maxime Ripard
2014-08-01 17:13 ` Vinod Koul
2014-08-02 14:49 ` Maxime Ripard
2014-08-02 15:17 ` Russell King - ARM Linux
2014-08-02 19:06 ` Maxime Ripard
2014-08-05 16:25 ` Vinod Koul
2014-07-31 12:44 ` Lars-Peter Clausen
2014-07-31 16:13 ` Maxime Ripard
2014-07-31 16:54 ` Lars-Peter Clausen
2014-07-31 17:37 ` Maxime Ripard
2014-08-01 8:00 ` Lars-Peter Clausen
2014-08-01 8:57 ` Maxime Ripard
2014-08-01 17:15 ` Vinod Koul
2014-08-01 18:09 ` Lars-Peter Clausen
2014-08-02 15:13 ` Maxime Ripard
2014-08-04 7:16 ` Lars-Peter Clausen
2014-07-31 13:22 ` Russell King - ARM Linux
2014-07-31 16:41 ` Maxime Ripard
2014-08-01 14:53 ` Russell King - ARM Linux
2014-08-02 15:11 ` Maxime Ripard
2014-08-02 15:29 ` Russell King - ARM Linux
2014-08-02 19:05 ` Maxime Ripard [this message]
2014-08-01 17:22 ` Vinod Koul
2014-08-05 8:16 ` Geert Uytterhoeven
2014-08-14 8:53 ` Ludovic Desroches
2014-08-14 8:57 ` Russell King - ARM Linux
2014-08-19 13:45 ` Vinod Koul
2014-08-19 14:44 ` Russell King - ARM Linux
2014-08-19 14:57 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140802190515.GN3952@lukather \
--to=maxime.ripard@free-electrons.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).