LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Sibi Sankar <sibis@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Andy Gross <agross@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Doug Anderson <dianders@chromium.org>,
	linux-kernel-owner@vger.kernel.org,
	Kevin Hilman <khilman@kernel.org>
Subject: Re: [PATCH 1/2] PM / Domains: Add GENPD_FLAG_SUSPEND_ON flag
Date: Thu, 13 Aug 2020 14:34:13 +0200
Message-ID: <CAPDyKFrH9WTg4O5L+e1AijNvsagLYZ9QVTeoD0x0SQgYd3hkBg@mail.gmail.com> (raw)
In-Reply-To: <1ba3e4d703dd0a52547d63fa014451eb@codeaurora.org>

On Wed, 12 Aug 2020 at 19:03, Sibi Sankar <sibis@codeaurora.org> wrote:
>
> Uffe,
> Thanks for taking time to review the
> series!
>
> On 2020-08-12 15:15, Ulf Hansson wrote:
> > On Tue, 11 Aug 2020 at 21:03, Sibi Sankar <sibis@codeaurora.org> wrote:
> >>
> >> This is for power domains which needs to stay powered on for suspend
> >> but can be powered on/off as part of runtime PM. This flag is aimed at
> >> power domains coupled to remote processors which enter suspend states
> >> independent to that of the application processor. Such power domains
> >> are turned off only on remote processor crash/shutdown.
> >
> > As Kevin also requested, please elaborate more on the use case.
> >
> > Why exactly must the PM domain stay powered on during system suspend?
> > Is there a wakeup configured that needs to be managed - or is there a
> > co-processor/FW behaviour that needs to be obeyed to?
>
> Yes this is a co-processor behavior that
> needs to be obeyed. Specifically application
> processor notifies the Always on Subsystem
> (AOSS) that a particular co-processor is up
> using the power domains exposed by AOSS QMP
> driver. AOSS uses this information to wait
> for the co-processors to suspend before
> starting its sleep sequence. The application
> processor powers off these power domains only
> if the co-processor has crashed or powered
> off.

Thanks for clarifying!

Although, can you please elaborate a bit more on the actual use case?
What are the typical co-processor and what drivers are involved in
managing it?

As you may know, runtime PM becomes disabled during system suspend of
a device. Which means, if the driver tries to power off the
coprocessor (via calling pm_runtime_put() for example), somewhere in
the system suspend phase of the corresponding device, its attached PM
domain stays powered on when managed by genpd.

Then in the suspend_noirq phase, genpd tries to power off the PM
domain, unless there are wakeups to consider.

Taking the above into account, wouldn't that mean that you potentially
may end up keeping the PM domain powered on, even if it actually can
be powered off in the suspend_noirq phase by genpd?

Kind regards
Uffe

> >
> >>
> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >> ---
> >>  drivers/base/power/domain.c | 3 ++-
> >>  include/linux/pm_domain.h   | 5 +++++
> >>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> >> index 2cb5e04cf86cd..ba78ac4a450d4 100644
> >> --- a/drivers/base/power/domain.c
> >> +++ b/drivers/base/power/domain.c
> >> @@ -129,6 +129,7 @@ static const struct genpd_lock_ops genpd_spin_ops
> >> = {
> >>  #define genpd_is_active_wakeup(genpd)  (genpd->flags &
> >> GENPD_FLAG_ACTIVE_WAKEUP)
> >>  #define genpd_is_cpu_domain(genpd)     (genpd->flags &
> >> GENPD_FLAG_CPU_DOMAIN)
> >>  #define genpd_is_rpm_always_on(genpd)  (genpd->flags &
> >> GENPD_FLAG_RPM_ALWAYS_ON)
> >> +#define genpd_is_suspend_on(genpd)     (genpd->flags &
> >> GENPD_FLAG_SUSPEND_ON)
> >>
> >>  static inline bool irq_safe_dev_in_no_sleep_domain(struct device
> >> *dev,
> >>                 const struct generic_pm_domain *genpd)
> >> @@ -949,7 +950,7 @@ static void genpd_sync_power_off(struct
> >> generic_pm_domain *genpd, bool use_lock,
> >>  {
> >>         struct gpd_link *link;
> >>
> >> -       if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))
> >> +       if (!genpd_status_on(genpd) || genpd_is_always_on(genpd) ||
> >> genpd_is_suspend_on(genpd))
> >>                 return;
> >>
> >>         if (genpd->suspended_count != genpd->device_count
> >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> >> index ee11502a575b0..3002a2d68936a 100644
> >> --- a/include/linux/pm_domain.h
> >> +++ b/include/linux/pm_domain.h
> >> @@ -55,6 +55,10 @@
> >>   *
> >>   * GENPD_FLAG_RPM_ALWAYS_ON:   Instructs genpd to always keep the PM
> >> domain
> >>   *                             powered on except for system suspend.
> >> + *
> >> + * GENPD_FLAG_SUSPEND_ON:      Instructs genpd to keep the PM domain
> >> powered
> >> + *                             on during suspend and runtime PM
> >> controlled
> >> + *                             otherwise.
> >>   */
> >>  #define GENPD_FLAG_PM_CLK       (1U << 0)
> >>  #define GENPD_FLAG_IRQ_SAFE     (1U << 1)
> >> @@ -62,6 +66,7 @@
> >>  #define GENPD_FLAG_ACTIVE_WAKEUP (1U << 3)
> >>  #define GENPD_FLAG_CPU_DOMAIN   (1U << 4)
> >>  #define GENPD_FLAG_RPM_ALWAYS_ON (1U << 5)
> >> +#define GENPD_FLAG_SUSPEND_ON   (1U << 6)
> >>
> >>  enum gpd_status {
> >>         GPD_STATE_ACTIVE = 0,   /* PM domain is active */
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum,
> >> a Linux Foundation Collaborative Project
> >>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11 19:02 Sibi Sankar
2020-08-11 19:02 ` [PATCH 2/2] soc: qcom: aoss: Use " Sibi Sankar
2020-08-11 21:16   ` Doug Anderson
2020-08-11 21:17 ` [PATCH 1/2] PM / Domains: Add " Doug Anderson
2020-08-11 21:38 ` Stephen Boyd
2020-08-12 13:36   ` Sibi Sankar
2020-08-12  0:19 ` Kevin Hilman
2020-08-12 16:12   ` Sibi Sankar
2020-08-12  9:45 ` Ulf Hansson
2020-08-12 17:02   ` Sibi Sankar
2020-08-13 12:34     ` Ulf Hansson [this message]
2020-08-13 17:26       ` Sibi Sankar
2020-08-17  8:44         ` Ulf Hansson
2020-08-17 16:49           ` Sibi Sankar
2020-08-18  8:31             ` Ulf Hansson
2020-08-18  9:03               ` Sibi Sankar

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=CAPDyKFrH9WTg4O5L+e1AijNvsagLYZ9QVTeoD0x0SQgYd3hkBg@mail.gmail.com \
    --to=ulf.hansson@linaro.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=khilman@kernel.org \
    --cc=len.brown@intel.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel-owner@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=rnayak@codeaurora.org \
    --cc=sibis@codeaurora.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git