From: Vinod Koul <vinod.koul@intel.com>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
dmaengine@vger.kernel.org,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Krzysztof Kozlowski <krzk@kernel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Lars-Peter Clausen <lars@metafoo.de>,
Arnd Bergmann <arnd@arndb.de>, Inki Dae <inki.dae@samsung.com>
Subject: Re: [PATCH v8 3/3] dmaengine: pl330: Don't require irq-safe runtime PM
Date: Mon, 13 Feb 2017 18:02:35 +0530 [thread overview]
Message-ID: <20170213123235.GM2843@localhost> (raw)
In-Reply-To: <d6f67b70-92f2-7e9a-7082-ecca3d6b392b@samsung.com>
On Mon, Feb 13, 2017 at 01:15:27PM +0100, Marek Szyprowski wrote:
> >Although, I don't know of other examples, besides the runtime PM use
> >case, where non-atomic channel prepare/unprepare would make sense. Do
> >you?
>
> Changing GFP_ATOMIC to GFP_KERNEL in some calls in the DMA engine drivers
> would be also a nice present for the memory management subsystem if there
> is no real reason to drain atomic pools.
The reason for the calls being atomic is that they will be invoked from
atomic context. All prepare callbacks, submit, issue_pending are in that context.
You have to be mindful that we can prepare and issue next txn from dmaengine
callback which is a tasklet.
> >>As I said earlier, if we want to solve that problem a better idea is to
> >>actually split the prepare as we discussed in [1]
> >>
> >>This way we can get a non atomic descriptor allocate/prepare and release.
> >>Yes we need to redesign the APIs to solve this, but if you guys are up for
> >>it, I think we can do it and avoid any further round abouts :)
> >Adding/re-designing dma APIs is a viable option to solve the runtime PM case.
> >
> >Changes would be needed for all related dma client drivers as well,
> >although if that's what we need to do - let's do it.
> >
> >[...]
> >
> >>>So besides solving the irq-safe issue for dma driver, using the
> >>>device-links has additionally two advantages. I already mentioned the
> >>>-EPROBE_DEFER issue above.
> >>>
> >>>The second thing, is the runtime/system PM relations we get for free
> >>>by using the links. In other words, the dma driver/core don't need to
> >>>care about dealing with pm_runtime_get|put() as that would be managed
> >>>by the dma client driver.
> >>Yeah sorry took me a while to figure that out :), If we do a different API
> >>then dmaengine core can call pm_runtime_get|put() from non-atomic context.
> >Yes, it can and this works from runtime PM point of view. But the
> >following issues would remain unsolved.
> >
> >1)
> >Dependencies between dma drivers and dma client drivers during system
> >PM. For example, a dma client driver needs the dma controller to be
> >operational (remain system resumed), until the dma client driver
> >itself becomes system suspended.
> >
> >The *only* currently available solution for this, is to try to system
> >suspend the dma controller later than the dma client, via using the
> >*late or the *noirq system PM callbacks. This works for most cases,
> >but it becomes a problem when the dma client also needs to be system
> >suspended at the *late or the *noirq phase. Clearly this solution that
> >doesn't scale.
> >
> >Using device links explicitly solves this problem as it allows to
> >specify this dependency between devices.
>
> Frankly, then creating device links has to be added to EVERY subsystem,
> which involves getting access to the resources provided by the other
> device. More or less this will apply to all kernel frameworks, which
> provide kind of ABC_get_XYZ(dev, ...) functions (like clk_get, phy_get,
> dma_chan_get, ...). Sounds like a topic for another loooong discussion.
Yeah, that was my view too :-)
> >2)
> >We won't avoid dma clients from getting -EPROBE_DEFER when requesting
> >their dma channels in their ->probe() routines. This would be
> >possible, if we can set up the device links at device initialization.
>
> The question is which core (DMA engine?, kernel device subsystem?) and
> how to find all clients before they call dma_chan_get().
Thanks
--
~Vinod
next prev parent reply other threads:[~2017-02-13 12:32 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20170209142307eucas1p2592bbad82dbbffc56bbd993f5a890981@eucas1p2.samsung.com>
2017-02-09 14:22 ` [PATCH v8 0/3] DMA Engine: switch PL330 driver to non-irq-safe runtime PM Marek Szyprowski
[not found] ` <CGME20170209142307eucas1p180323d005f524760913b8d04ac966423@eucas1p1.samsung.com>
2017-02-09 14:22 ` [PATCH v8 1/3] dmaengine: Add new device_{set, release}_slave callbacks Marek Szyprowski
2017-02-10 4:34 ` [PATCH v8 1/3] dmaengine: Add new device_{set,release}_slave callbacks Vinod Koul
2017-02-10 12:07 ` Marek Szyprowski
2017-02-13 1:42 ` Vinod Koul
2017-02-13 11:48 ` Marek Szyprowski
[not found] ` <CGME20170209142308eucas1p24d52db3d52e19228e8f423c3dc8b085b@eucas1p2.samsung.com>
2017-02-09 14:22 ` [PATCH v8 2/3] dmaengine: pl330: remove pdata based initialization Marek Szyprowski
2017-03-22 8:22 ` Marek Szyprowski
2017-03-27 4:34 ` Vinod Koul
[not found] ` <CGME20170209142309eucas1p2b1277d96139eafc0d1dcc14145600476@eucas1p2.samsung.com>
2017-02-09 14:22 ` [PATCH v8 3/3] dmaengine: pl330: Don't require irq-safe runtime PM Marek Szyprowski
2017-02-10 4:50 ` Vinod Koul
2017-02-10 11:51 ` Marek Szyprowski
2017-02-10 13:57 ` Ulf Hansson
2017-02-13 2:03 ` Vinod Koul
2017-02-13 11:11 ` Ulf Hansson
2017-02-13 12:15 ` Marek Szyprowski
2017-02-13 12:32 ` Vinod Koul [this message]
2017-02-13 12:27 ` Vinod Koul
2017-02-13 15:32 ` Ulf Hansson
2017-02-13 15:47 ` Vinod Koul
2017-02-14 7:50 ` Marek Szyprowski
2017-02-14 8:24 ` Ulf Hansson
2017-02-13 12:01 ` Marek Szyprowski
2017-02-13 11:45 ` Marek Szyprowski
2017-02-13 15:09 ` Ulf Hansson
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=20170213123235.GM2843@localhost \
--to=vinod.koul@intel.com \
--cc=arnd@arndb.de \
--cc=b.zolnierkie@samsung.com \
--cc=dmaengine@vger.kernel.org \
--cc=inki.dae@samsung.com \
--cc=krzk@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=rjw@rjwysocki.net \
--cc=ulf.hansson@linaro.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).