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
WARNING: multiple messages have this Message-ID (diff)
From: vinod.koul@intel.com (Vinod Koul)
To: linux-arm-kernel@lists.infradead.org
Subject: [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: 57+ 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
2017-02-09 14:22 ` Marek Szyprowski
2017-02-09 14:22 ` Marek Szyprowski
2017-02-09 14:22 ` [PATCH v8 1/3] dmaengine: Add new device_{set, release}_slave callbacks Marek Szyprowski
2017-02-09 14:22 ` [PATCH v8 1/3] dmaengine: Add new device_{set,release}_slave callbacks Marek Szyprowski
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 4:34 ` Vinod Koul
2017-02-10 12:07 ` Marek Szyprowski
2017-02-10 12:07 ` Marek Szyprowski
2017-02-13 1:42 ` Vinod Koul
2017-02-13 1:42 ` Vinod Koul
2017-02-13 11:48 ` Marek Szyprowski
2017-02-13 11:48 ` Marek Szyprowski
2017-02-09 14:22 ` [PATCH v8 2/3] dmaengine: pl330: remove pdata based initialization Marek Szyprowski
2017-02-09 14:22 ` Marek Szyprowski
2017-03-22 8:22 ` Marek Szyprowski
2017-03-22 8:22 ` Marek Szyprowski
2017-03-22 8:22 ` Marek Szyprowski
2017-03-27 4:34 ` Vinod Koul
2017-03-27 4:34 ` Vinod Koul
2017-03-27 4:34 ` Vinod Koul
2017-02-09 14:22 ` [PATCH v8 3/3] dmaengine: pl330: Don't require irq-safe runtime PM Marek Szyprowski
2017-02-09 14:22 ` Marek Szyprowski
2017-02-09 14:22 ` Marek Szyprowski
2017-02-10 4:50 ` Vinod Koul
2017-02-10 4:50 ` Vinod Koul
2017-02-10 11:51 ` Marek Szyprowski
2017-02-10 11:51 ` Marek Szyprowski
2017-02-10 13:57 ` Ulf Hansson
2017-02-10 13:57 ` Ulf Hansson
2017-02-13 2:03 ` Vinod Koul
2017-02-13 2:03 ` Vinod Koul
2017-02-13 11:11 ` Ulf Hansson
2017-02-13 11:11 ` Ulf Hansson
2017-02-13 12:15 ` Marek Szyprowski
2017-02-13 12:15 ` Marek Szyprowski
2017-02-13 12:32 ` Vinod Koul [this message]
2017-02-13 12:32 ` Vinod Koul
2017-02-13 12:27 ` Vinod Koul
2017-02-13 12:27 ` Vinod Koul
2017-02-13 15:32 ` Ulf Hansson
2017-02-13 15:32 ` Ulf Hansson
2017-02-13 15:47 ` Vinod Koul
2017-02-13 15:47 ` Vinod Koul
2017-02-13 15:47 ` Vinod Koul
2017-02-14 7:50 ` Marek Szyprowski
2017-02-14 7:50 ` Marek Szyprowski
2017-02-14 8:24 ` Ulf Hansson
2017-02-14 8:24 ` Ulf Hansson
2017-02-13 12:01 ` Marek Szyprowski
2017-02-13 12:01 ` Marek Szyprowski
2017-02-13 11:45 ` Marek Szyprowski
2017-02-13 11:45 ` Marek Szyprowski
2017-02-13 15:09 ` Ulf Hansson
2017-02-13 15:09 ` Ulf Hansson
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.