All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Vinod Koul <vinod.koul@intel.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	"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 16:32:32 +0100	[thread overview]
Message-ID: <CAPDyKFqnmhps5nhj-exZ0rnmkwdeD+iuz3CH7btjvxoJaZtVDg@mail.gmail.com> (raw)
In-Reply-To: <20170213122742.GL2843@localhost>

[...]

>> 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?
>
> The primary ask for that has been to enable runtime_pm for drivers. It's not
> a new ask, but we somehow haven't gotten around to do it.

Okay, I see.

>
>> > 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.
>
> Yes, but do bear in mind that some cases do need atomic prepare. The primary
> cases for DMA had that in mind and also submitting next transaction from the
> callback (tasklet) context, so that won't go away.
>
> It would help in other cases where clients know that they will not be in
> atomic context so we provide additional non-atomic "allocation" followed by
> prepare, so that drivers can split the work among these and people can do
> runtime_pm and other things..

That for sharing the details.

It seems like some dma expert really need to be heavily involved if we
ever are going to complete this work. :-)

[...]

>>
>> 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.
>
> Yes this is an interesting point. Yes till now people have been doing above
> to workaround this problem, but hey this is not a unique to dmaengine. Any
> subsystem which provides services to others has this issue, so the solution
> much be driver or pm framework and not unique to dmaengine.

I definitely agree, these problems aren't unique to the dmaengine
subsystem. Exactly how/where to manage them, that I guess, is the key
question.

However, I can't resist from finding the device links useful, as those
really do address and solve our issues from a runtime/system PM point
of view.

>
>> 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.
>
> Well setting those links is not practical at initialization time. Most
> modern dma controllers feature a SW mux, with multiple clients connecting
> and requesting, would we link all of them? Most of times dmaengine driver
> wont know about those..

Okay, I see!

Kind regards
Uffe

WARNING: multiple messages have this Message-ID (diff)
From: ulf.hansson@linaro.org (Ulf Hansson)
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 16:32:32 +0100	[thread overview]
Message-ID: <CAPDyKFqnmhps5nhj-exZ0rnmkwdeD+iuz3CH7btjvxoJaZtVDg@mail.gmail.com> (raw)
In-Reply-To: <20170213122742.GL2843@localhost>

[...]

>> 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?
>
> The primary ask for that has been to enable runtime_pm for drivers. It's not
> a new ask, but we somehow haven't gotten around to do it.

Okay, I see.

>
>> > 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.
>
> Yes, but do bear in mind that some cases do need atomic prepare. The primary
> cases for DMA had that in mind and also submitting next transaction from the
> callback (tasklet) context, so that won't go away.
>
> It would help in other cases where clients know that they will not be in
> atomic context so we provide additional non-atomic "allocation" followed by
> prepare, so that drivers can split the work among these and people can do
> runtime_pm and other things..

That for sharing the details.

It seems like some dma expert really need to be heavily involved if we
ever are going to complete this work. :-)

[...]

>>
>> 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.
>
> Yes this is an interesting point. Yes till now people have been doing above
> to workaround this problem, but hey this is not a unique to dmaengine. Any
> subsystem which provides services to others has this issue, so the solution
> much be driver or pm framework and not unique to dmaengine.

I definitely agree, these problems aren't unique to the dmaengine
subsystem. Exactly how/where to manage them, that I guess, is the key
question.

However, I can't resist from finding the device links useful, as those
really do address and solve our issues from a runtime/system PM point
of view.

>
>> 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.
>
> Well setting those links is not practical at initialization time. Most
> modern dma controllers feature a SW mux, with multiple clients connecting
> and requesting, would we link all of them? Most of times dmaengine driver
> wont know about those..

Okay, I see!

Kind regards
Uffe

  reply	other threads:[~2017-02-13 15:32 UTC|newest]

Thread overview: 68+ 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
     [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-09 14:22       ` [PATCH v8 1/3] dmaengine: Add new device_{set, release}_slave callbacks Marek Szyprowski
2017-02-09 14:22       ` 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
     [not found]   ` <CGME20170209142308eucas1p24d52db3d52e19228e8f423c3dc8b085b@eucas1p2.samsung.com>
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
     [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-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-10 13:57             ` Ulf Hansson
2017-02-13  2:03             ` Vinod Koul
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 11:11                 ` Ulf Hansson
2017-02-13 12:15                 ` Marek Szyprowski
2017-02-13 12:15                   ` Marek Szyprowski
2017-02-13 12:15                   ` Marek Szyprowski
2017-02-13 12:32                   ` Vinod Koul
2017-02-13 12:32                     ` Vinod Koul
2017-02-13 12:32                     ` Vinod Koul
2017-02-13 12:27                 ` Vinod Koul
2017-02-13 12:27                   ` Vinod Koul
2017-02-13 12:27                   ` Vinod Koul
2017-02-13 15:32                   ` Ulf Hansson [this message]
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  7:50                         ` Marek Szyprowski
2017-02-14  8:24                       ` Ulf Hansson
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 12:01                 ` Marek Szyprowski
2017-02-13 11:45             ` 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=CAPDyKFqnmhps5nhj-exZ0rnmkwdeD+iuz3CH7btjvxoJaZtVDg@mail.gmail.com \
    --to=ulf.hansson@linaro.org \
    --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=vinod.koul@intel.com \
    /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.