mbox series

[00/10] OPP: Support multiple power-domains per device

Message ID cover.1530252803.git.viresh.kumar@linaro.org
Headers show
Series OPP: Support multiple power-domains per device | expand

Message

Viresh Kumar June 29, 2018, 6:19 a.m. UTC
Hi,

This series improves the OPP core (and a bit of genpd core as well) to
support multiple phandles in the "required-opps" property, which are
only used for multiple power-domains per device for now.

We still don't propagate the changes to master domains for the
sub-domains, but this patchset is an important stepping stone for that
to happen.

Tested on Hikey960 after faking some power domains for CPUs.

--
viresh

Viresh Kumar (10):
  OPP: Parse OPP table's DT properties from _of_init_opp_table()
  OPP: Identify and mark genpd OPP tables
  OPP: Separate out custom OPP handler specific code
  OPP: Populate required opp tables from "required-opps" property
  OPP: Populate OPPs from "required-opps" property
  OPP: Add dev_pm_opp_{set|put}_required_device() helper
  PM / Domains: Add genpd_opp_to_performance_state()
  OPP: Configure all required OPPs
  OPP: Rename and relocate of_genpd_opp_to_performance_state()
  OPP: Remove of_dev_pm_opp_find_required_opp()

 drivers/base/power/domain.c |  82 ++++----
 drivers/opp/core.c          | 228 ++++++++++++++-------
 drivers/opp/of.c            | 382 ++++++++++++++++++++++++++++++------
 drivers/opp/opp.h           |  18 ++
 include/linux/pm_domain.h   |   7 +-
 include/linux/pm_opp.h      |  16 +-
 6 files changed, 557 insertions(+), 176 deletions(-)

-- 
2.18.0.rc1.242.g61856ae69a2c

Comments

Rafael J. Wysocki June 29, 2018, 9:06 a.m. UTC | #1
On Friday, June 29, 2018 8:19:30 AM CEST Viresh Kumar wrote:
> Hi,

> 

> This series improves the OPP core (and a bit of genpd core as well) to

> support multiple phandles in the "required-opps" property, which are

> only used for multiple power-domains per device for now.

> 

> We still don't propagate the changes to master domains for the

> sub-domains, but this patchset is an important stepping stone for that

> to happen.

> 

> Tested on Hikey960 after faking some power domains for CPUs.


I'm assuming that this work will go in via OPP.

Thanks,
Rafael
Viresh Kumar July 16, 2018, 4:32 a.m. UTC | #2
On 29-06-18, 11:49, Viresh Kumar wrote:
> Hi,

> 

> This series improves the OPP core (and a bit of genpd core as well) to

> support multiple phandles in the "required-opps" property, which are

> only used for multiple power-domains per device for now.

> 

> We still don't propagate the changes to master domains for the

> sub-domains, but this patchset is an important stepping stone for that

> to happen.

> 

> Tested on Hikey960 after faking some power domains for CPUs.


Ulf, can you please help review this stuff ? 

-- 
viresh
Ulf Hansson July 16, 2018, 9:04 a.m. UTC | #3
On 16 July 2018 at 06:32, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-06-18, 11:49, Viresh Kumar wrote:

>> Hi,

>>

>> This series improves the OPP core (and a bit of genpd core as well) to

>> support multiple phandles in the "required-opps" property, which are

>> only used for multiple power-domains per device for now.

>>

>> We still don't propagate the changes to master domains for the

>> sub-domains, but this patchset is an important stepping stone for that

>> to happen.

>>

>> Tested on Hikey960 after faking some power domains for CPUs.

>

> Ulf, can you please help review this stuff ?


I am in holiday mode, so I needs some more time before I can review
this. Apologize for the inconvenience.

Kind regards
Uffe
Ulf Hansson Sept. 10, 2018, 2:18 p.m. UTC | #4
On 29 June 2018 at 08:19, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Multiple generic power domains for a device are supported with the help

> of virtual devices, which are created for each device-genpd pair. These


What "device-genpd" pair are you referring to?

> are the device structures which are attached to the power domain and are

> required by the OPP core to set the performance state of the genpd.

>

> The helpers added by this commit are required to be called once for each

> genpd of a device. These are required only if multiple domains are

> present for a device, otherwise the actual device structure will be used

> instead by the OPP core.

>

> This commit also updates the genpd core to automatically call the new

> helper to set the required devices with the OPP layer, whenever the

> virtual devices are created for multiple genpd. The prototype of

> __genpd_dev_pm_attach() is slightly updated for this.

>

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---

>  drivers/base/power/domain.c | 31 ++++++++++++++---

>  drivers/opp/core.c          | 69 +++++++++++++++++++++++++++++++++++++

>  drivers/opp/of.c            | 12 +++++++

>  drivers/opp/opp.h           |  2 ++

>  include/linux/pm_opp.h      |  8 +++++

>  5 files changed, 118 insertions(+), 4 deletions(-)

>

> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c

> index c298de8a8308..e9c85c96580c 100644

> --- a/drivers/base/power/domain.c

> +++ b/drivers/base/power/domain.c

> @@ -2219,8 +2219,14 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)

>         genpd_queue_power_off_work(pd);

>

>         /* Unregister the device if it was created by genpd. */

> -       if (dev->bus == &genpd_bus_type)

> +       if (dev->bus == &genpd_bus_type) {

> +               struct opp_table *opp_table = dev_get_drvdata(dev);


This feels wrong, to me. Genpd is not a driver, but rather a
subsystem, hence I think it's a bad idea to "abuse" the driver data
pointer like this.

Instead, in genpd we store device specific data, through the
dev->power.subsys_data.

Seems like a better option is to extend the subsys_data, to also
include data needed to be shared for opp/performance states.

In that way, we might not even need to call into the opp library
(dev_pm_opp_set_required_device()) API, but rather the opp library can
itself check what is in the subsys_data for the device that is being
targeted. Would that work?

[...]

Kind regards
Uffe