From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: DMA engine API issue (was: [PATCH/RFC 0/5] R-Car Gen2 DMAC hardware descriptor list support)
Date: Mon, 4 Aug 2014 18:54:58 +0100 [thread overview]
Message-ID: <20140804175458.GH30282@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <2568292.I2Yo4lJlu6@avalon>
On Mon, Aug 04, 2014 at 07:00:36PM +0200, Laurent Pinchart wrote:
> Hi Russell,
>
> On Friday 01 August 2014 15:30:20 Russell King - ARM Linux wrote:
> > This sequence must occur in a timely manner as some DMA engine
> > implementations hold a lock between the prepare and submit callbacks (Dan
> > explicitly permits this as part of the API.)
>
> That really triggers a red alarm in the part of my brain that deals with API
> design, but I suppose it would be too difficult to change that.
Mine to, but there's not a lot which can be done about it without
changing a lot of users.
> > > The DMA_PRIVATE capability flag seems to play a role here, but it's far
> > > from being clear how that mechanism is supposed to work. This should be
> > > documented as well, and any light you could shed on this dark corner of
> > > the API would help.
> >
> > Why do you think that DMA_PRIVATE has a bearing in the callbacks? It
> > doesn't.
>
> Not on callbacks, but on how pending descriptors are pushed to the hardware.
> The flag is explicitly checked in dma_issue_pending_all().
Right. So, let me put a question to you - what do you think is the
effect of the check in dma_issue_pending_all()?
I'll give you a hint - disregard the comment at the top of the function,
because that's out of date.
> > DMA_PRIVATE is all about channel allocation as I explained yesterday, and
> > whether the channel is available for async_tx usage.
> >
> > A channel marked DMA_PRIVATE is not available for async_tx usage at
> > any moment. A channel without DMA_PRIVATE is available for async_tx
> > usage until it is allocated for the slave API - at which point the
> > generic DMA engine code will mark the channel with DMA_PRIVATE,
> > thereby taking it away from async_tx API usage. When the slave API
> > frees the channel, DMA_PRIVATE will be cleared, making the channel
> > available for async_tx usage.
> >
> > So, basically, DMA_PRIVATE set -> async_tx usage not allowed.
> > DMA_PRIVATE clear -> async_tx usage permitted. It really is that
> > simple.
>
> DMA_PRIVATE is a dma_device flag, not a dma_chan flag. As soon as one channel
> is allocated by __dma_request_channel() the whole device is marked with
> DMA_PRIVATE, making all channels private. What am I missing ?
I can't answer that - I don't know why the previous authors decided to
make it a DMA-device wide property - presumably there are DMA controllers
where this matters.
However, one thing to realise is that a dma_device is a virtual concept -
it is a set of channels which share a common set of properties. It is not
a physical device. It is entirely reasonable for a set of channels on a
physical device to be shared between two different dma_device instances
and handed out by the driver code as needed.
> > > Similarly, the DMA engine API is split in functions with different
> > > prefixes (mostly dmaengine_*, dma_async_*, dma_*, and various
> > > unprefixed niceties such as async_tx_ack or txd_lock. If there's a
> > > rationale for that (beyond just historical reasons) it should also
> > > be documented, otherwise a cleanup would help all the confused DMA
> > > engine users (myself included).
> >
> > dmaengine_* are generally the interface functions to the DMA engine code,
> > which have been recently introduced to avoid the multiple levels of
> > pointer indirection having to be typed in every driver.
> >
> > dma_async_* are the DMA engine interface functions for the async_tx API.
> >
> > dma_* predate the dmaengine_* naming, and are probably too generic, so
> > should probably end up being renamed to dmaengine_*.
>
> Thank you for the confirmation. I'll see if I can cook up a patch. It will
> likely be pretty large and broad though, but I guess there's no way around it.
>
> > txd_* are all about the DMA engine descriptors.
> >
> > async_tx_* are the higher level async_tx API functions.
>
> Thank you for the information. How about the dma_async_* functions, should
> they be renamed to dmaengine_* as well ? Or are some of them part of the
> async_tx_* API ?
Well, these:
dma_async_device_register
dma_async_device_unregister
dma_async_tx_descriptor_init
are more DMA engine core <-> DMA engine driver interface functions than
user functions. The remainder of the dma_async_* functions are internal
to the async_tx API.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2014-08-04 17:54 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1406032431-3807-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
[not found] ` <87d2cwvmxa.wl%kuninori.morimoto.gx@gmail.com>
[not found] ` <3361994.F0HgAeJkR9@avalon>
2014-07-23 11:07 ` DMA engine API issue (was: [PATCH/RFC 0/5] R-Car Gen2 DMAC hardware descriptor list support) Laurent Pinchart
2014-07-24 0:46 ` Kuninori Morimoto
2014-07-24 1:35 ` Kuninori Morimoto
2014-07-24 4:53 ` Vinod Koul
2014-07-24 4:52 ` Vinod Koul
2014-08-01 8:51 ` Laurent Pinchart
2014-08-01 14:30 ` Russell King - ARM Linux
2014-08-01 17:09 ` Vinod Koul
2014-08-04 13:47 ` Geert Uytterhoeven
2014-08-04 17:00 ` Laurent Pinchart
2014-08-04 17:54 ` Russell King - ARM Linux [this message]
2014-08-05 23:19 ` Laurent Pinchart
2014-08-06 7:17 ` Geert Uytterhoeven
2014-08-06 11:04 ` Arnd Bergmann
2014-08-01 17:07 ` Vinod Koul
2014-08-04 16:50 ` Laurent Pinchart
2014-08-04 18:03 ` DMA engine API issue Lars-Peter Clausen
2014-08-04 18:32 ` Russell King - ARM Linux
2014-08-04 23:12 ` Laurent Pinchart
2014-08-05 16:56 ` DMA engine API issue (was: [PATCH/RFC 0/5] R-Car Gen2 DMAC hardware descriptor list support) Vinod Koul
2014-07-24 12:29 ` [alsa-devel] DMA engine API issue Lars-Peter Clausen
2014-07-24 12:51 ` DMA engine API issue (was: [PATCH/RFC 0/5] R-Car Gen2 DMAC hardware descriptor list support) Russell King - ARM Linux
2014-08-01 9:24 ` Laurent Pinchart
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=20140804175458.GH30282@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--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).