From: Lina Iyer <lina.iyer@linaro.org>
To: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
linux-pm@vger.kernel.org, Len Brown <len.brown@intel.com>,
Pavel Machek <pavel@ucw.cz>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Axel Haslam <ahaslam+renesas@baylibre.com>,
Krzysztof Kozlowski <k.kozlowski.k@gmail.com>,
Russell King <linux@arm.linux.org.uk>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V4] PM / Domains: Remove intermediate states from the power off sequence
Date: Thu, 9 Jul 2015 08:05:32 -0600 [thread overview]
Message-ID: <20150709140532.GA2049@linaro.org> (raw)
In-Reply-To: <7htwtee84d.fsf@deeprootsystems.com>
On Wed, Jul 08 2015 at 13:56 -0600, Kevin Hilman wrote:
>Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> Genpd's ->runtime_suspend() (assigned to pm_genpd_runtime_suspend())
>> doesn't immediately walk the hierarchy of ->runtime_suspend() callbacks.
>> Instead, pm_genpd_runtime_suspend() calls pm_genpd_poweroff() which
>> postpones that until *all* the devices in the genpd are runtime suspended.
>>
>> When pm_genpd_poweroff() discovers that the last device in the genpd is
>> about to be runtime suspended, it calls __pm_genpd_save_device() for *all*
>> the devices in the genpd sequentially. Furthermore,
>> __pm_genpd_save_device() invokes the ->start() callback, walks the
>> hierarchy of the ->runtime_suspend() callbacks and invokes the ->stop()
>> callback. This causes a "thundering herd" problem.
>>
>> Let's address this issue by having pm_genpd_runtime_suspend() immediately
>> walk the hierarchy of the ->runtime_suspend() callbacks, instead of
>> postponing that to the power off sequence via pm_genpd_poweroff(). If the
>> selected ->runtime_suspend() callback doesn't return an error code, call
>> pm_genpd_poweroff() to see if it's feasible to also power off the PM
>> domain.
>>
>> Adopting this change enables us to simplify parts of the code in genpd,
>> for example the locking mechanism. Additionally, it gives some positive
>> side effects, as described below.
>>
>> i)
>> One device's ->runtime_resume() latency is no longer affected by other
>> devices' latencies in a genpd.
>>
>> The complexity genpd has to support the option to abort the power off
>> sequence suffers from latency issues. More precisely, a device that is
>> requested to be runtime resumed, may end up waiting for
>> __pm_genpd_save_device() to complete its operations for *another* device.
>> That's because pm_genpd_poweroff() can't confirm an abort request while it
>> waits for __pm_genpd_save_device() to return.
>>
>> As this patch removes the intermediate states in pm_genpd_poweroff() while
>> powering off the PM domain, we no longer need the ability to abort that
>> sequence.
>>
>> ii)
>> Make pm_runtime[_status]_suspended() reliable when used with genpd.
>>
>> Until the last device in a genpd becomes idle, pm_genpd_runtime_suspend()
>> will return 0 without actually walking the hierarchy of the
>> ->runtime_suspend() callbacks. However, by returning 0 the runtime PM core
>> considers the device as runtime_suspended, so
>> pm_runtime[_status]_suspended() will return true, even though the device
>> isn't (yet) runtime suspended.
>>
>> After this patch, since pm_genpd_runtime_suspend() immediately walks the
>> hierarchy of the ->runtime_suspend() callbacks,
>> pm_runtime[_status]_suspended() will accurately reflect the status of the
>> device.
>>
>> iii)
>> Enable fine-grained PM through runtime PM callbacks in drivers/subsystems.
>>
>> There are currently cases were drivers/subsystems implements runtime PM
>> callbacks to deploy fine-grained PM (e.g. gate clocks, move pinctrl to
>> power-save state, etc.). While using the genpd, pm_genpd_runtime_suspend()
>> postpones invoking these callbacks until *all* the devices in the genpd
>> are runtime suspended. In essence, one runtime resumed device prevents
>> fine-grained PM for other devices within the same genpd.
>>
>> After this patch, since pm_genpd_runtime_suspend() immediately walks the
>> hierarchy of the ->runtime_suspend() callbacks, fine-grained PM is enabled
>> throughout all the levels of runtime PM callbacks.
>>
>> iiii)
>> Enable fine-grained PM for IRQ safe devices
>>
>> Per the definition for an IRQ safe device, its runtime PM callbacks must
>> be able to execute in atomic context. In the path while genpd walks the
>> hierarchy of the ->runtime_suspend() callbacks for the device, it uses a
>> mutex. Therefore, genpd prevents that path to be executed for IRQ safe
>> devices.
>>
>> As this patch changes pm_genpd_runtime_suspend() to immediately walk the
>> hierarchy of the ->runtime_suspend() callbacks and without needing to use
>> a mutex, fine-grained PM is enabled throughout all the levels of runtime
>> PM callbacks for IRQ safe devices.
>>
>> Unfortunately this patch also comes with a drawback, as described in the
>> summary below.
>>
>> Driver's/subsystem's runtime PM callbacks may be invoked even when the
>> genpd hasn't actually powered off the PM domain, potentially introducing
>> unnecessary latency.
>>
>> However, in most cases, saving/restoring register contexts for devices are
>> typically fast operations or can be optimized in device specific ways
>> (e.g. shadow copies of register contents in memory, device-specific checks
>> to see if context has been lost before restoring context, etc.).
>>
>> Still, in some cases the driver/subsystem may suffer from latency if
>> runtime PM is used in a very fine-grained manner (e.g. for each IO request
>> or xfer). To prevent that extra overhead, the driver/subsystem may deploy
>> the runtime PM autosuspend feature.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
>Reviewed-by: Kevin Hilman <khilman@linaro.org>
>
>I believe Geert and Lina have both tested earlier versions, but would be
>nice to see their Tested-by for this one.
>
I tested verssion 4. Did not see any issues with genpd suspend/resume.
Tested-by: Lina Iyer <lina.iyer@linaro.org>
Thanks,
Lina
WARNING: multiple messages have this Message-ID (diff)
From: lina.iyer@linaro.org (Lina Iyer)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V4] PM / Domains: Remove intermediate states from the power off sequence
Date: Thu, 9 Jul 2015 08:05:32 -0600 [thread overview]
Message-ID: <20150709140532.GA2049@linaro.org> (raw)
In-Reply-To: <7htwtee84d.fsf@deeprootsystems.com>
On Wed, Jul 08 2015 at 13:56 -0600, Kevin Hilman wrote:
>Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> Genpd's ->runtime_suspend() (assigned to pm_genpd_runtime_suspend())
>> doesn't immediately walk the hierarchy of ->runtime_suspend() callbacks.
>> Instead, pm_genpd_runtime_suspend() calls pm_genpd_poweroff() which
>> postpones that until *all* the devices in the genpd are runtime suspended.
>>
>> When pm_genpd_poweroff() discovers that the last device in the genpd is
>> about to be runtime suspended, it calls __pm_genpd_save_device() for *all*
>> the devices in the genpd sequentially. Furthermore,
>> __pm_genpd_save_device() invokes the ->start() callback, walks the
>> hierarchy of the ->runtime_suspend() callbacks and invokes the ->stop()
>> callback. This causes a "thundering herd" problem.
>>
>> Let's address this issue by having pm_genpd_runtime_suspend() immediately
>> walk the hierarchy of the ->runtime_suspend() callbacks, instead of
>> postponing that to the power off sequence via pm_genpd_poweroff(). If the
>> selected ->runtime_suspend() callback doesn't return an error code, call
>> pm_genpd_poweroff() to see if it's feasible to also power off the PM
>> domain.
>>
>> Adopting this change enables us to simplify parts of the code in genpd,
>> for example the locking mechanism. Additionally, it gives some positive
>> side effects, as described below.
>>
>> i)
>> One device's ->runtime_resume() latency is no longer affected by other
>> devices' latencies in a genpd.
>>
>> The complexity genpd has to support the option to abort the power off
>> sequence suffers from latency issues. More precisely, a device that is
>> requested to be runtime resumed, may end up waiting for
>> __pm_genpd_save_device() to complete its operations for *another* device.
>> That's because pm_genpd_poweroff() can't confirm an abort request while it
>> waits for __pm_genpd_save_device() to return.
>>
>> As this patch removes the intermediate states in pm_genpd_poweroff() while
>> powering off the PM domain, we no longer need the ability to abort that
>> sequence.
>>
>> ii)
>> Make pm_runtime[_status]_suspended() reliable when used with genpd.
>>
>> Until the last device in a genpd becomes idle, pm_genpd_runtime_suspend()
>> will return 0 without actually walking the hierarchy of the
>> ->runtime_suspend() callbacks. However, by returning 0 the runtime PM core
>> considers the device as runtime_suspended, so
>> pm_runtime[_status]_suspended() will return true, even though the device
>> isn't (yet) runtime suspended.
>>
>> After this patch, since pm_genpd_runtime_suspend() immediately walks the
>> hierarchy of the ->runtime_suspend() callbacks,
>> pm_runtime[_status]_suspended() will accurately reflect the status of the
>> device.
>>
>> iii)
>> Enable fine-grained PM through runtime PM callbacks in drivers/subsystems.
>>
>> There are currently cases were drivers/subsystems implements runtime PM
>> callbacks to deploy fine-grained PM (e.g. gate clocks, move pinctrl to
>> power-save state, etc.). While using the genpd, pm_genpd_runtime_suspend()
>> postpones invoking these callbacks until *all* the devices in the genpd
>> are runtime suspended. In essence, one runtime resumed device prevents
>> fine-grained PM for other devices within the same genpd.
>>
>> After this patch, since pm_genpd_runtime_suspend() immediately walks the
>> hierarchy of the ->runtime_suspend() callbacks, fine-grained PM is enabled
>> throughout all the levels of runtime PM callbacks.
>>
>> iiii)
>> Enable fine-grained PM for IRQ safe devices
>>
>> Per the definition for an IRQ safe device, its runtime PM callbacks must
>> be able to execute in atomic context. In the path while genpd walks the
>> hierarchy of the ->runtime_suspend() callbacks for the device, it uses a
>> mutex. Therefore, genpd prevents that path to be executed for IRQ safe
>> devices.
>>
>> As this patch changes pm_genpd_runtime_suspend() to immediately walk the
>> hierarchy of the ->runtime_suspend() callbacks and without needing to use
>> a mutex, fine-grained PM is enabled throughout all the levels of runtime
>> PM callbacks for IRQ safe devices.
>>
>> Unfortunately this patch also comes with a drawback, as described in the
>> summary below.
>>
>> Driver's/subsystem's runtime PM callbacks may be invoked even when the
>> genpd hasn't actually powered off the PM domain, potentially introducing
>> unnecessary latency.
>>
>> However, in most cases, saving/restoring register contexts for devices are
>> typically fast operations or can be optimized in device specific ways
>> (e.g. shadow copies of register contents in memory, device-specific checks
>> to see if context has been lost before restoring context, etc.).
>>
>> Still, in some cases the driver/subsystem may suffer from latency if
>> runtime PM is used in a very fine-grained manner (e.g. for each IO request
>> or xfer). To prevent that extra overhead, the driver/subsystem may deploy
>> the runtime PM autosuspend feature.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
>Reviewed-by: Kevin Hilman <khilman@linaro.org>
>
>I believe Geert and Lina have both tested earlier versions, but would be
>nice to see their Tested-by for this one.
>
I tested verssion 4. Did not see any issues with genpd suspend/resume.
Tested-by: Lina Iyer <lina.iyer@linaro.org>
Thanks,
Lina
next prev parent reply other threads:[~2015-07-09 14:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-18 13:17 [PATCH V4] PM / Domains: Remove intermediate states from the power off sequence Ulf Hansson
2015-06-18 13:17 ` Ulf Hansson
2015-07-08 19:56 ` Kevin Hilman
2015-07-08 19:56 ` Kevin Hilman
2015-07-09 7:05 ` Geert Uytterhoeven
2015-07-09 7:05 ` Geert Uytterhoeven
2015-07-09 14:05 ` Lina Iyer [this message]
2015-07-09 14:05 ` Lina Iyer
2015-07-10 1:26 ` Rafael J. Wysocki
2015-07-10 1:26 ` Rafael J. Wysocki
2015-12-22 12:49 ` Daniel Kurtz
2015-12-22 12:49 ` Daniel Kurtz
2015-12-23 11:40 ` Ulf Hansson
2015-12-23 11:40 ` 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=20150709140532.GA2049@linaro.org \
--to=lina.iyer@linaro.org \
--cc=ahaslam+renesas@baylibre.com \
--cc=dmitry.torokhov@gmail.com \
--cc=geert+renesas@glider.be \
--cc=k.kozlowski.k@gmail.com \
--cc=khilman@kernel.org \
--cc=len.brown@intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=pavel@ucw.cz \
--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.