From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Dmitry Osipenko <digetx@gmail.com>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Linux PM <linux-pm@vger.kernel.org>,
Kevin Hilman <khilman@kernel.org>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Rajendra Nayak <rnayak@codeaurora.org>,
Dong Aisheng <aisheng.dong@nxp.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] PM: domains: Prevent power off for parent unless child is in deepest state
Date: Tue, 1 Mar 2022 15:56:31 +0100 [thread overview]
Message-ID: <CAJZ5v0guqv=A_kBQSiq03zuiR-6VPy7gthppTPJX5hiHRjC2Sg@mail.gmail.com> (raw)
In-Reply-To: <CAPDyKFrBxVry=yh8m+OSaq+xnzDmVqhFXQ0QN1F9WR3KB=2MGg@mail.gmail.com>
On Mon, Feb 28, 2022 at 9:56 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 18 Feb 2022 at 00:11, Dmitry Osipenko <digetx@gmail.com> wrote:
> >
> > 17.02.2022 15:49, Ulf Hansson пишет:
> > > A PM domain managed by genpd may support multiple idlestates (power-off
> > > states). During genpd_power_off() a genpd governor may be asked to select
> > > one of the idlestates based upon the dev PM QoS constraints, for example.
> > >
> > > However, there is a problem with the behaviour around this in genpd. More
> > > precisely, a parent-domain is allowed to be powered off, no matter of what
> > > idlestate that has been selected for the child-domain.
> > >
> > > For the stm32mp1 platform from STMicro, this behaviour doesn't play well.
> > > Instead, the parent-domain must not be powered off, unless the deepest
> > > idlestate has been selected for the child-domain. As the current behaviour
> > > in genpd is quite questionable anyway, let's simply change it into what is
> > > needed by the stm32mp1 platform.
> > >
> > > If it surprisingly turns out that other platforms may need a different
> > > behaviour from genpd, then we will have to revisit this to find a way to
> > > make it configurable.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >
> > > Changes in v2:
> > > - Clarified commit message - based upon discussions with Dmitry.
> > > - Updated a comment in the code, suggested by Dmitry.
> > >
> > > ---
> > > drivers/base/power/domain.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > > index 5db704f02e71..c87588c21700 100644
> > > --- a/drivers/base/power/domain.c
> > > +++ b/drivers/base/power/domain.c
> > > @@ -636,6 +636,18 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> > > atomic_read(&genpd->sd_count) > 0)
> > > return -EBUSY;
> > >
> > > + /*
> > > + * The children must be in their deepest (powered-off) states to allow
> > > + * the parent to be powered off. Note that, there's no need for
> > > + * additional locking, as powering on a child, requires the parent's
> > > + * lock to be acquired first.
> > > + */
> > > + list_for_each_entry(link, &genpd->parent_links, parent_node) {
> > > + struct generic_pm_domain *child = link->child;
> > > + if (child->state_idx < child->state_count - 1)
> > > + return -EBUSY;
> > > + }
> > > +
> > > list_for_each_entry(pdd, &genpd->dev_list, list_node) {
> > > enum pm_qos_flags_status stat;
> > >
> > > @@ -1073,6 +1085,13 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
> > > || atomic_read(&genpd->sd_count) > 0)
> > > return;
> > >
> > > + /* Check that the children are in their deepest (powered-off) state. */
> > > + list_for_each_entry(link, &genpd->parent_links, parent_node) {
> > > + struct generic_pm_domain *child = link->child;
> > > + if (child->state_idx < child->state_count - 1)
> > > + return;
> > > + }
> > > +
> > > /* Choose the deepest state when suspending */
> > > genpd->state_idx = genpd->state_count - 1;
> > > if (_genpd_power_off(genpd, false))
> >
> > Thank you, looks good. Although, this should be v3.
> >
> > Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>
> Thanks Dmitry! I think v2 should be correct. At least I haven't sent a
> v2 before. :-)
>
> Rafael, I think this is ready to go, can please pick it up?
Applied as 5.18 material, thanks!
next prev parent reply other threads:[~2022-03-01 14:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 12:49 [PATCH v2] PM: domains: Prevent power off for parent unless child is in deepest state Ulf Hansson
2022-02-17 23:11 ` Dmitry Osipenko
2022-02-28 8:56 ` Ulf Hansson
2022-03-01 14:56 ` Rafael J. Wysocki [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-02-04 10:16 Ulf Hansson
2022-02-15 21:49 ` Dmitry Osipenko
2022-02-16 9:45 ` Ulf Hansson
2022-02-17 10:02 ` Dmitry Osipenko
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='CAJZ5v0guqv=A_kBQSiq03zuiR-6VPy7gthppTPJX5hiHRjC2Sg@mail.gmail.com' \
--to=rafael@kernel.org \
--cc=aisheng.dong@nxp.com \
--cc=alexandre.torgue@foss.st.com \
--cc=digetx@gmail.com \
--cc=geert+renesas@glider.be \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rnayak@codeaurora.org \
--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).