mbox series

[v13,0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)

Message ID 20190327143548.25305-1-ulf.hansson@linaro.org (mailing list archive)
Headers show
Series PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM) | expand

Message

Ulf Hansson March 27, 2019, 2:35 p.m. UTC
Changes in v13:
 - Use WRITE|READ_ONCE when reading/writing the "next_hrtimer" variable in the
struct cpuidle_device. Also reset the same variable after resumed from idle to
avoid it from containing a stale value.
 - Added acks from Daniel Lezcano.
 - The entire v13 series, including the PSCI/ARM changes are available in a
git branch [3].

Changes in v12:
 - Drop the patches for restructuring tick_nohz_get_sleep_length(). Instead
replace them all with a new timer/cpuidle patch, according to suggestions by
Rafael.
 - The entire v12 series, including the PSCI/ARM changes are available in a
git branch [2].

Changes in v11:
 - This version contains only the infrastructure changes that is needed for
deployment. The PSCI/ARM changes have also been updated and tested, but I will
post them separately. Still, to provide completeness, I have published a branch
containing everything to a git tree [1], feel free to have a look and test.
 - The v10 series contained a patch, "timer: Export next wakeup time of a CPU",
which has been replaced by a couple of new patches, whom reworks the existing
tick_nohz_get_sleep_length() function, to provide the next timer expiration
instead of the duration.
 - More changelogs are available per patch.

Changes in v10:
 - Quite significant changes have been to the PSCI driver deployment. According
   to an agreement with Lorenzo, the hierarchical CPU layout for PSCI should be
   orthogonal to whether the PSCI FW supports OSI or not. This has been taken
   care of in this version.
 - Drop the generic attach/detach helpers of CPUs to genpd, instead make that
   related code internal to PSCI, for now.
 - Fix "BUG: sleeping for invalid context" for hotplug, as reported by Raju.
 - Addressed various comments from version 8 and 9.
 - Clarified changelogs and re-wrote the cover-letter to better explain the
   motivations behind these changes.

Background:

For ARM64/ARM based platforms CPUs are often arranged in a hierarchical manner.
From a CPU idle state perspective, this means some states may be shared among a
group of CPUs (aka CPU cluster).

To deal with idle management of a group of CPUs, sometimes the kernel needs to
be involved to manage the last-man standing algorithm, simply because it can't
rely solely on power management FWs to deal with this. Depending on the
platform, of course.

There are a couple of typical scenarios for when the kernel needs to be in
control, dealing with synchronization of when the last CPU in a cluster is about
to enter a deep idle state.

1)
The kernel needs to carry out so called last-man activities before the
CPU cluster can enter a deep idle state. This may for example involve to
configure external logics for wakeups, as the GIC may no longer be functional
once a deep cluster idle state have been entered. Likewise, these operations
may need to be restored, when the first CPU wakes up.

2)
Other more generic I/O devices, such as an MMC controller for example, may be a
part of the same power domain as the CPU cluster, due to a shared power-rail.
For these scenarios, when the MMC controller is in use dealing with an MMC
request, a deeper idle state of the CPU cluster may needs to be temporarily
disabled. This is needed to retain the MMC controller in a functional state,
else it may loose its register-context in the middle of serving a request.

In this series, we are extending the generic PM domain (aka genpd) to be used
for also CPU devices. Hence the goal is to re-use much of its current code to
help us manage the last-man standing synchronization. Moreover, as we already
use genpd to model power domains for generic I/O devices, both 1) and 2) can be
address with its help.

Moreover, to address these problems for ARM64 DT based platforms, we are
deploying support for genpd and runtime PM to the PSCI FW driver - and finally
we make some updates to two ARM64 DTBs, as to deploy the new PSCI CPU topology
layout.

The series has been tested on a Qcom 410c dragonboard and on a Hisilicon Hikey
board. The first one uses PSCI OS-initiated mode, while the second uses the PSCI
Platform-Coordinated mode.

Kind regards
Ulf Hansson

[1]
git.linaro.org/people/ulf.hansson/linux-pm.git next_v11
[2]
git.linaro.org/people/ulf.hansson/linux-pm.git next_v12
[2]
git.linaro.org/people/ulf.hansson/linux-pm.git next_v13


Ulf Hansson (4):
  PM / Domains: Add a generic data pointer to the genpd_power_state
    struct
  PM / Domains: Add support for CPU devices to genpd
  cpuidle: Export the next timer/tick expiration for a CPU
  PM / Domains: Add genpd governor for CPUs

 drivers/base/power/domain.c          | 77 ++++++++++++++++++++++++++--
 drivers/base/power/domain_governor.c | 65 ++++++++++++++++++++++-
 drivers/cpuidle/cpuidle.c            | 19 ++++++-
 include/linux/cpuidle.h              |  1 +
 include/linux/pm_domain.h            | 20 +++++++-
 include/linux/tick.h                 |  7 ++-
 kernel/time/tick-sched.c             | 12 +++++
 7 files changed, 193 insertions(+), 8 deletions(-)

Comments

Ulf Hansson April 9, 2019, 11:57 a.m. UTC | #1
On Wed, 27 Mar 2019 at 15:35, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Changes in v13:
>  - Use WRITE|READ_ONCE when reading/writing the "next_hrtimer" variable in the
> struct cpuidle_device. Also reset the same variable after resumed from idle to
> avoid it from containing a stale value.
>  - Added acks from Daniel Lezcano.
>  - The entire v13 series, including the PSCI/ARM changes are available in a
> git branch [3].

Rafael, I have address the received comments on this version $subject
series, is there anything that is holding you back from queuing it?

If you like I can send a pull-request, to simplify for you.

Moreover, there is another series which I have pinged you about
earlier, which I would appreciate to queue via your tree. It's acked
by PSCI maintainers and is ready to go. "[PATCH v2 0/5] drivers:
firmware: psci: Some cleanup and refactoring".

Kind regards
Uffe

>
> Changes in v12:
>  - Drop the patches for restructuring tick_nohz_get_sleep_length(). Instead
> replace them all with a new timer/cpuidle patch, according to suggestions by
> Rafael.
>  - The entire v12 series, including the PSCI/ARM changes are available in a
> git branch [2].
>
> Changes in v11:
>  - This version contains only the infrastructure changes that is needed for
> deployment. The PSCI/ARM changes have also been updated and tested, but I will
> post them separately. Still, to provide completeness, I have published a branch
> containing everything to a git tree [1], feel free to have a look and test.
>  - The v10 series contained a patch, "timer: Export next wakeup time of a CPU",
> which has been replaced by a couple of new patches, whom reworks the existing
> tick_nohz_get_sleep_length() function, to provide the next timer expiration
> instead of the duration.
>  - More changelogs are available per patch.
>
> Changes in v10:
>  - Quite significant changes have been to the PSCI driver deployment. According
>    to an agreement with Lorenzo, the hierarchical CPU layout for PSCI should be
>    orthogonal to whether the PSCI FW supports OSI or not. This has been taken
>    care of in this version.
>  - Drop the generic attach/detach helpers of CPUs to genpd, instead make that
>    related code internal to PSCI, for now.
>  - Fix "BUG: sleeping for invalid context" for hotplug, as reported by Raju.
>  - Addressed various comments from version 8 and 9.
>  - Clarified changelogs and re-wrote the cover-letter to better explain the
>    motivations behind these changes.
>
> Background:
>
> For ARM64/ARM based platforms CPUs are often arranged in a hierarchical manner.
> From a CPU idle state perspective, this means some states may be shared among a
> group of CPUs (aka CPU cluster).
>
> To deal with idle management of a group of CPUs, sometimes the kernel needs to
> be involved to manage the last-man standing algorithm, simply because it can't
> rely solely on power management FWs to deal with this. Depending on the
> platform, of course.
>
> There are a couple of typical scenarios for when the kernel needs to be in
> control, dealing with synchronization of when the last CPU in a cluster is about
> to enter a deep idle state.
>
> 1)
> The kernel needs to carry out so called last-man activities before the
> CPU cluster can enter a deep idle state. This may for example involve to
> configure external logics for wakeups, as the GIC may no longer be functional
> once a deep cluster idle state have been entered. Likewise, these operations
> may need to be restored, when the first CPU wakes up.
>
> 2)
> Other more generic I/O devices, such as an MMC controller for example, may be a
> part of the same power domain as the CPU cluster, due to a shared power-rail.
> For these scenarios, when the MMC controller is in use dealing with an MMC
> request, a deeper idle state of the CPU cluster may needs to be temporarily
> disabled. This is needed to retain the MMC controller in a functional state,
> else it may loose its register-context in the middle of serving a request.
>
> In this series, we are extending the generic PM domain (aka genpd) to be used
> for also CPU devices. Hence the goal is to re-use much of its current code to
> help us manage the last-man standing synchronization. Moreover, as we already
> use genpd to model power domains for generic I/O devices, both 1) and 2) can be
> address with its help.
>
> Moreover, to address these problems for ARM64 DT based platforms, we are
> deploying support for genpd and runtime PM to the PSCI FW driver - and finally
> we make some updates to two ARM64 DTBs, as to deploy the new PSCI CPU topology
> layout.
>
> The series has been tested on a Qcom 410c dragonboard and on a Hisilicon Hikey
> board. The first one uses PSCI OS-initiated mode, while the second uses the PSCI
> Platform-Coordinated mode.
>
> Kind regards
> Ulf Hansson
>
> [1]
> git.linaro.org/people/ulf.hansson/linux-pm.git next_v11
> [2]
> git.linaro.org/people/ulf.hansson/linux-pm.git next_v12
> [2]
> git.linaro.org/people/ulf.hansson/linux-pm.git next_v13
>
>
> Ulf Hansson (4):
>   PM / Domains: Add a generic data pointer to the genpd_power_state
>     struct
>   PM / Domains: Add support for CPU devices to genpd
>   cpuidle: Export the next timer/tick expiration for a CPU
>   PM / Domains: Add genpd governor for CPUs
>
>  drivers/base/power/domain.c          | 77 ++++++++++++++++++++++++++--
>  drivers/base/power/domain_governor.c | 65 ++++++++++++++++++++++-
>  drivers/cpuidle/cpuidle.c            | 19 ++++++-
>  include/linux/cpuidle.h              |  1 +
>  include/linux/pm_domain.h            | 20 +++++++-
>  include/linux/tick.h                 |  7 ++-
>  kernel/time/tick-sched.c             | 12 +++++
>  7 files changed, 193 insertions(+), 8 deletions(-)
>
> --
> 2.17.1
>
Rafael J. Wysocki April 9, 2019, 3:01 p.m. UTC | #2
On Tue, Apr 9, 2019 at 1:57 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Wed, 27 Mar 2019 at 15:35, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > Changes in v13:
> >  - Use WRITE|READ_ONCE when reading/writing the "next_hrtimer" variable in the
> > struct cpuidle_device. Also reset the same variable after resumed from idle to
> > avoid it from containing a stale value.
> >  - Added acks from Daniel Lezcano.
> >  - The entire v13 series, including the PSCI/ARM changes are available in a
> > git branch [3].
>
> Rafael, I have address the received comments on this version $subject
> series, is there anything that is holding you back from queuing it?
>
> If you like I can send a pull-request, to simplify for you.

I will get to this one shortly.  I've been waiting for possible
comments on patch [3/4], but since no one has commented so far, it
seems to be non-controversial. :-)

I'm going to add this material to the cpuidle branch rather than genpd.

> Moreover, there is another series which I have pinged you about
> earlier, which I would appreciate to queue via your tree. It's acked
> by PSCI maintainers and is ready to go. "[PATCH v2 0/5] drivers:
> firmware: psci: Some cleanup and refactoring".

If you want me to merge the PSCI series, please resent it with a
request to that end in the cover letter.  Also please let me know
whether or not it depends on the $subject series.
Ulf Hansson April 10, 2019, 8:25 a.m. UTC | #3
On Tue, 9 Apr 2019 at 17:01, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, Apr 9, 2019 at 1:57 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Wed, 27 Mar 2019 at 15:35, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > Changes in v13:
> > >  - Use WRITE|READ_ONCE when reading/writing the "next_hrtimer" variable in the
> > > struct cpuidle_device. Also reset the same variable after resumed from idle to
> > > avoid it from containing a stale value.
> > >  - Added acks from Daniel Lezcano.
> > >  - The entire v13 series, including the PSCI/ARM changes are available in a
> > > git branch [3].
> >
> > Rafael, I have address the received comments on this version $subject
> > series, is there anything that is holding you back from queuing it?
> >
> > If you like I can send a pull-request, to simplify for you.
>
> I will get to this one shortly.  I've been waiting for possible
> comments on patch [3/4], but since no one has commented so far, it
> seems to be non-controversial. :-)
>
> I'm going to add this material to the cpuidle branch rather than genpd.

Sound good to me, thanks!

>
> > Moreover, there is another series which I have pinged you about
> > earlier, which I would appreciate to queue via your tree. It's acked
> > by PSCI maintainers and is ready to go. "[PATCH v2 0/5] drivers:
> > firmware: psci: Some cleanup and refactoring".
>
> If you want me to merge the PSCI series, please resent it with a
> request to that end in the cover letter.  Also please let me know
> whether or not it depends on the $subject series.

No dependency, but I am using it as a base for a future series that I
will post as soon as you have queued it.

So, I have just resent the series, as per your request. If there is
anything else, just let me know.

Kind regards
Uffe