linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>
Cc: Dmitry Osipenko <digetx@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Roja Rani Yarubandi <rojay@codeaurora.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Stephan Gerhold <stephan@gerhold.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Rajendra Nayak <rnayak@codeaurora.org>
Subject: Re: [PATCH v2 4/4] PM: domains: Drop/restore performance state votes for devices at system PM
Date: Thu, 3 Jun 2021 12:20:57 +0200	[thread overview]
Message-ID: <CAPDyKFp2dKFQpLMgazXumCxf=FHQ9bdadXUkGsjiAwniF8p2dw@mail.gmail.com> (raw)
In-Reply-To: <20210603093438.138705-5-ulf.hansson@linaro.org>

+ Mark Brown, Dmitry Baryshkov

On Thu, 3 Jun 2021 at 11:34, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Recent changes in genpd drops and restore performance state votes for
> devices during runtime PM.
>
> For the similar reasons, but to avoid the same kind of boilerplate code in
> device PM callbacks for system sleep in subsystems/drivers, let's drop and
> restore performance states votes in genpd for the attached devices during
> system sleep.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

After a second thought, it looks like we maybe should defer to apply
this final patch of the series. At least until we figured out how to
address the below issue:

So, I noticed that we have things like "regulator-fixed-domain", that
uses "required-opps" to enable/disable a regulator through the
dev_pm_set_performance_state() interface. We likely don't want to drop
the performance state internally in genpd when genpd_suspend_noirq()
gets called, for the corresponding struct device for the regulator.

I guess if genpd should drop performance states like $subject patch
suggest, we need some kind of additional coordination, that allows a
subsystem/driver to inform genpd when it should avoid it. Or something
along those lines.

Kind regards
Uffe

> ---
>
> Changes in v2:
>         - Rebased.
>         - A few cosmetic changes.
>
> ---
>  drivers/base/power/domain.c | 9 +++++++++
>  include/linux/pm_domain.h   | 1 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e5d97174c254..a33e5b341f3f 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1171,6 +1171,7 @@ static int genpd_prepare(struct device *dev)
>   */
>  static int genpd_finish_suspend(struct device *dev, bool poweroff)
>  {
> +       struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev);
>         struct generic_pm_domain *genpd;
>         int ret = 0;
>
> @@ -1201,6 +1202,7 @@ static int genpd_finish_suspend(struct device *dev, bool poweroff)
>         }
>
>         genpd_lock(genpd);
> +       gpd_data->pm_pstate = genpd_drop_performance_state(dev);
>         genpd->suspended_count++;
>         genpd_sync_power_off(genpd, true, 0);
>         genpd_unlock(genpd);
> @@ -1245,6 +1247,7 @@ static int genpd_resume_noirq(struct device *dev)
>         genpd_lock(genpd);
>         genpd_sync_power_on(genpd, true, 0);
>         genpd->suspended_count--;
> +       genpd_restore_performance_state(dev, dev_gpd_data(dev)->pm_pstate);
>         genpd_unlock(genpd);
>
>         if (genpd->dev_ops.stop && genpd->dev_ops.start &&
> @@ -1364,6 +1367,7 @@ static int genpd_restore_noirq(struct device *dev)
>         }
>
>         genpd_sync_power_on(genpd, true, 0);
> +       genpd_restore_performance_state(dev, dev_gpd_data(dev)->pm_pstate);
>         genpd_unlock(genpd);
>
>         if (genpd->dev_ops.stop && genpd->dev_ops.start &&
> @@ -1409,23 +1413,28 @@ static void genpd_complete(struct device *dev)
>  static void genpd_switch_state(struct device *dev, bool suspend)
>  {
>         struct generic_pm_domain *genpd;
> +       struct generic_pm_domain_data *gpd_data;
>         bool use_lock;
>
>         genpd = dev_to_genpd_safe(dev);
>         if (!genpd)
>                 return;
>
> +       gpd_data = dev_gpd_data(dev);
> +
>         use_lock = genpd_is_irq_safe(genpd);
>
>         if (use_lock)
>                 genpd_lock(genpd);
>
>         if (suspend) {
> +               gpd_data->pm_pstate = genpd_drop_performance_state(dev);
>                 genpd->suspended_count++;
>                 genpd_sync_power_off(genpd, use_lock, 0);
>         } else {
>                 genpd_sync_power_on(genpd, use_lock, 0);
>                 genpd->suspended_count--;
> +               genpd_restore_performance_state(dev, gpd_data->pm_pstate);
>         }
>
>         if (use_lock)
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 21a0577305ef..f6e9dc28621c 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -199,6 +199,7 @@ struct generic_pm_domain_data {
>         int cpu;
>         unsigned int performance_state;
>         unsigned int rpm_pstate;
> +       unsigned int pm_pstate;
>         ktime_t next_wakeup;
>         void *data;
>  };
> --
> 2.25.1
>

  reply	other threads:[~2021-06-03 10:22 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03  9:34 [PATCH v2 0/4] PM: domains: Avoid boilerplate code for DVFS in subsystem/drivers Ulf Hansson
2021-06-03  9:34 ` [PATCH v2 1/4] PM: domains: Split code in dev_pm_genpd_set_performance_state() Ulf Hansson
2021-06-03  9:34 ` [PATCH v2 2/4] PM: domains: Return early if perf state is already set for the device Ulf Hansson
2021-06-03  9:34 ` [PATCH v2 3/4] PM: domains: Drop/restore performance state votes for devices at runtime PM Ulf Hansson
2021-06-03  9:55   ` Viresh Kumar
2021-06-03 10:31     ` Ulf Hansson
2021-06-03 11:17       ` Ulf Hansson
2021-06-04  3:53         ` Viresh Kumar
2021-06-04  7:45           ` Ulf Hansson
2021-06-07  4:47             ` Viresh Kumar
2021-06-09 12:25               ` Ulf Hansson
2021-06-03 19:02   ` Dmitry Osipenko
2021-06-03 19:08     ` Dmitry Osipenko
2021-06-04  7:20       ` Ulf Hansson
2021-06-03  9:34 ` [PATCH v2 4/4] PM: domains: Drop/restore performance state votes for devices at system PM Ulf Hansson
2021-06-03 10:20   ` Ulf Hansson [this message]
2021-06-03 11:15     ` Mark Brown
2021-06-03 13:48       ` Ulf Hansson
2021-06-08 12:53     ` Stephan Gerhold
2021-06-08 14:08       ` Ulf Hansson
2021-06-08 14:20         ` Mark Brown
2021-06-08 14:39           ` Ulf Hansson
2021-06-08 15:37         ` Stephan Gerhold
2021-06-03 11:12 ` [PATCH v2 0/4] PM: domains: Avoid boilerplate code for DVFS in subsystem/drivers Stephan Gerhold
2021-06-03 15:27   ` Ulf Hansson
2021-06-03 17:14     ` Stephan Gerhold
2021-06-04  7:18       ` Ulf Hansson
2021-06-04  8:23         ` Stephan Gerhold
2021-06-04 10:57           ` Ulf Hansson
2021-06-04 11:50             ` Stephan Gerhold
2021-06-11 16:42               ` Rafael J. Wysocki

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='CAPDyKFp2dKFQpLMgazXumCxf=FHQ9bdadXUkGsjiAwniF8p2dw@mail.gmail.com' \
    --to=ulf.hansson@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=broonie@kernel.org \
    --cc=digetx@gmail.com \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=jonathanh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=rnayak@codeaurora.org \
    --cc=rojay@codeaurora.org \
    --cc=sboyd@kernel.org \
    --cc=stephan@gerhold.net \
    --cc=thierry.reding@gmail.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@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).