From: Axel Haslam <redacted>
Some architectures may have intermediate power
levels between on and off. each state in between
may have its own set of procedures to put the
power domain into that state.
This patch adds the ability to declare multiple
states for a given generic power domain, the idea
is that the deepest state will be entered which
does not violate any of the device or sub-domain
latency constraints.
Changes since v3:
* remove old power on/off function at the end of the
series so that compilation will not break in between.
Changes since v2:
* remove state argument and macros from save/restore
callbacks.
* added init_state for platforms to pass the
initial state when the genpd is initially off.
* convert current genpd users for the structure changes.
* compile tested for shmobile_defconfig, exynos_defconfig,
s3c6400_defconfig u8500_defconfig.
Changes since v1:
* split the changes so that the actual logic that
selects the target state is a separate patch.
* move the cached logic out of the state function and add
it back to default_power_down_ok.
* use static arrays instead of dynamic for state declaration
since its not expected to have lots of intermediate states.
That way we don't have to mess with allocations.
* rename default_power_down_ok_state to power_down_ok_for_state
Axel Haslam (8):
PM / Domains: structure changes for multiple states
PM / Domains: select deepest state
ARM: s3c64xx: pm: Convert to multiple states
ARM: exynos: pm: Convert to multiple states
ARM: r8a7779: pm: Convert to multiple states
ARM: rmobile: pm: Convert to multiple states
ARM: ux500: pm: Convert to multiple states
PM / Domains: remove old power on/off callbacks.
arch/arm/mach-exynos/pm_domains.c | 6 ++-
arch/arm/mach-s3c64xx/pm.c | 57 ++++++++++++++++++--------
arch/arm/mach-shmobile/pm-r8a7779.c | 6 ++-
arch/arm/mach-shmobile/pm-rmobile.c | 6 ++-
arch/arm/mach-ux500/pm_domains.c | 8 +++-
drivers/base/power/domain.c | 43 ++++++++++++++------
drivers/base/power/domain_governor.c | 77 ++++++++++++++++++++++++------------
include/linux/pm_domain.h | 21 ++++++++--
8 files changed, 160 insertions(+), 64 deletions(-)
--
1.9.1
From: Axel Haslam <redacted>
Add the structure changes to be able to declare
multiple states. When trying to set a power domain
to off, genpd will be able to choose from an array
of states declared by the platform.
The power on and off callbacks and latencies are
now tied to a particular state.
States should be declared in ascending order from
shallowest to deepest, deepest meaning the state
which takes longer to enter and exit.
if the genpd is initially off, the user should set the
.init_state field when registering the genpd,
so that genpd knows which callbacks to call to set the
domain to on.
An genpd with multiple states would look something like:
struct generic_pm_domain pd1 = {
.name = "PD1",
.states = {
[0] = {
.name= "D0",
.power_off = D0_power_off_cb,
.power_on = D0_power_on_cb,
.power_off_latency_ns = D0_POWER_OFF_LATENCY,
.power_on_latency_ns = D0_POWER_ON_LATENCY,
},
[1] = {
.name= "D1",
.power_off = D1_power_off_cb,
.power_on = D1_power_on_cb,
.power_off_latency_ns = D1_POWER_OFF_LATENCY,
.power_on_latency_ns = D1_POWER_ON_LATENCY,
},
[2] = {
.name= "D2",
.power_off = D2_power_off_cb,
.power_on = D2_power_on_cb,
.power_off_latency_ns = D2_POWER_OFF_LATENCY,
.power_on_latency_ns = D2_POWER_ON_LATENCY,
},
},
.state_count = 3,
.init_state = 2,
[...]
};
Signed-off-by: Axel Haslam <redacted>
---
drivers/base/power/domain.c | 43 +++++++++++++++++++++++++++---------
drivers/base/power/domain_governor.c | 8 ++++---
include/linux/pm_domain.h | 17 ++++++++++++++
3 files changed, 54 insertions(+), 14 deletions(-)
@@ -17,6 +17,8 @@#include<linux/notifier.h>#include<linux/cpuidle.h>+#define GENPD_MAX_NSTATES 10+/* Defines used for the flags field in the struct generic_pm_domain */#define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */
@@ -46,6 +48,16 @@ struct gpd_cpuidle_data {structcpuidle_state*idle_state;};+structgeneric_pm_domain;++structgenpd_power_state{+char*name;+s64power_off_latency_ns;+s64power_on_latency_ns;+int(*power_off)(structgeneric_pm_domain*domain);+int(*power_on)(structgeneric_pm_domain*domain);+};+structgeneric_pm_domain{structdev_pm_domaindomain;/* PM domain operations */structlist_headgpd_list_node;/* Node in the global PM domains list */
@@ -80,6 +92,11 @@ struct generic_pm_domain {void(*detach_dev)(structgeneric_pm_domain*domain,structdevice*dev);unsignedintflags;/* Bit field of configs for genpd */++structgenpd_power_statestates[GENPD_MAX_NSTATES];+inttarget_state;/* state that genpd will go to when off */+intinit_state;/* initial genpd state */+intstate_count;/* number of states */};staticinlinestructgeneric_pm_domain*pd_to_genpd(structdev_pm_domain*pd)
From: Axel Haslam <redacted>
now that the structures of genpd can support
multiple state definitions, add the logic in
the governor to select the deepest possible
state when powering down.
For this, create the new function power_down_ok_for_state
which will test if a particular state will not violate
the devices and sub-domains constraints.
default_power_down_ok is modified to try each
state starting from the deepest until a valid
state is found or there are no more states to test.
the resulting target state will be valid until
there are latency or constraint changes,
thus, we can avoid looping every power_down,
and use the cached results instead.
Signed-off-by: Axel Haslam <redacted>
---
drivers/base/power/domain_governor.c | 71 ++++++++++++++++++++++++------------
1 file changed, 48 insertions(+), 23 deletions(-)
@@ -222,6 +200,53 @@ static bool default_power_down_ok(struct dev_pm_domain *pd)returntrue;}+/*+*Loopintoallthepossiblelowpowerstates+*andfindthedeepeststatethatdoesnotviolateany+*constraints.Ifdeviceconstraintsdidnotchange,+*wecanreturnthecachedvalueandusethetarget_state+*calculatedpreviously.+*/+staticbooldefault_power_down_ok(structdev_pm_domain*pd)+{+structgeneric_pm_domain*genpd=pd_to_genpd(pd);+intlast_state_idx=genpd->state_count-1;+structgpd_link*link;+boolretval=false;+inti;++/*+*iftherewasnochangeonmax_off_time,wecanreturnthe+*cachedvalueandwedontneedtofindanewtarget_state+*/+if(!genpd->max_off_time_changed)+returngenpd->cached_power_down_ok;++/*+*Wehavetoinvalidatethecachedresultsforthemasters,so+*usetheobservationthatdefault_power_down_ok()isnot+*goingtobecalledforanymasteruntilthisinstance+*returns.+*/+list_for_each_entry(link,&genpd->slave_links,slave_node)+link->master->max_off_time_changed=true;++genpd->max_off_time_ns=-1;+genpd->max_off_time_changed=false;++/* find a state to power down to, starting from the deepest */+for(i=0;i<genpd->state_count;i++){+if(power_down_ok_for_state(pd,last_state_idx-i)){+genpd->target_state=last_state_idx-i;+retval=true;+break;+}+}++genpd->cached_power_down_ok=retval;+returnretval;+}+staticboolalways_on_power_down_ok(structdev_pm_domain*domain){returnfalse;
From: Axel Haslam <redacted>
The generic power domain framework added structure changes
to support multiple intermediate states when powering off a domain.
These changes are needed to prevent compilation breaks with
the new structures.
Signed-off-by: Axel Haslam <redacted>
---
arch/arm/mach-s3c64xx/pm.c | 57 +++++++++++++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 16 deletions(-)
From: Axel Haslam <redacted>
The generic power domain framework added structure changes
to support multiple intermediate states when powering off a domain.
These changes are needed to prevent compilation breaks with
the new structures.
Signed-off-by: Axel Haslam <redacted>
---
arch/arm/mach-exynos/pm_domains.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
From: Axel Haslam <redacted>
The generic power domain framework added structure changes
to support multiple intermediate states when powering off a domain.
These changes are needed to prevent compilation breaks with
the new structures.
Signed-off-by: Axel Haslam <redacted>
---
arch/arm/mach-shmobile/pm-r8a7779.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
From: Axel Haslam <redacted>
The generic power domain framework added structure changes
to support multiple intermediate states when powering off a domain.
These changes are needed to prevent compilation breaks with
the new structures.
Signed-off-by: Axel Haslam <redacted>
---
arch/arm/mach-shmobile/pm-rmobile.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
From: Axel Haslam <redacted>
The generic power domain framework added structure changes
to support multiple intermediate states when powering off a domain.
These changes are needed to prevent compilation breaks with
the new structures.
Signed-off-by: Axel Haslam <redacted>
---
arch/arm/mach-ux500/pm_domains.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Axel Haslam <redacted>
Now that all known users have been converted to use the state
version of the callbacks, we can remove the default callbacks
without breaking compilation.
Signed-off-by: Axel Haslam <redacted>
---
include/linux/pm_domain.h | 4 ----
1 file changed, 4 deletions(-)
@@ -78,10 +78,6 @@ struct generic_pm_domain {unsignedintsuspended_count;/* System suspend device counter */unsignedintprepared_count;/* Suspend counter of prepared devices */boolsuspend_power_off;/* Power status before system suspend */-int(*power_off)(structgeneric_pm_domain*domain);-s64power_off_latency_ns;-int(*power_on)(structgeneric_pm_domain*domain);-s64power_on_latency_ns;structgpd_dev_opsdev_ops;s64max_off_time_ns;/* Maximum allowed "suspended" time. */boolmax_off_time_changed;
On Wed, Apr 22, 2015 at 11:45 AM, [off-list ref] wrote:
From: Axel Haslam <redacted>
Add the structure changes to be able to declare
multiple states. When trying to set a power domain
to off, genpd will be able to choose from an array
of states declared by the platform.
The power on and off callbacks and latencies are
now tied to a particular state.
States should be declared in ascending order from
shallowest to deepest, deepest meaning the state
which takes longer to enter and exit.
if the genpd is initially off, the user should set the
.init_state field when registering the genpd,
so that genpd knows which callbacks to call to set the
domain to on.
The initial state may depend on various factors (hardware default,
boot loader, previously loaded kernel, ...). This is even true with the
current binary states.
@@ -154,23 +155,24 @@ static void genpd_recalc_cpu_exit_latency(struct generic_pm_domain *genpd) static int genpd_power_on(struct generic_pm_domain *genpd) {+ int target_state = genpd->target_state; ktime_t time_start; s64 elapsed_ns; int ret;- if (!genpd->power_on)+ if (!genpd->states[target_state].power_on) return 0;
While your series now looks compilable for all patch steps, it's not
bisectable: the right hook won't be called until the platform is
converted to support multiple states.
I think you can fix this by e.g. using -1 for the target_state on non-converted
platforms, and doing:
if (target_state < 0) {
if (!genpd->power_on)
return 0;
} else {
if (!genpd->states[target_state].power_on)
return 0;
}
and
ret = target_state < 0 ? genpd->power_on(genpd)
: genpd->states[target_state].power_on(genpd);
The checks for a negative target_state can be removed only after
all platforms have been converted.
@@ -17,6 +17,8 @@#include<linux/notifier.h>#include<linux/cpuidle.h>+#define GENPD_MAX_NSTATES 10+/* Defines used for the flags field in the struct generic_pm_domain */#define GENPD_FLAG_PM_CLK (1U << 0) /* PM domain uses PM clk */
@@ -46,6 +48,16 @@ struct gpd_cpuidle_data {structcpuidle_state*idle_state;};+structgeneric_pm_domain;++structgenpd_power_state{+char*name;+s64power_off_latency_ns;+s64power_on_latency_ns;+int(*power_off)(structgeneric_pm_domain*domain);+int(*power_on)(structgeneric_pm_domain*domain);+};+structgeneric_pm_domain{structdev_pm_domaindomain;/* PM domain operations */structlist_headgpd_list_node;/* Node in the global PM domains list */
@@ -80,6 +92,11 @@ struct generic_pm_domain {void(*detach_dev)(structgeneric_pm_domain*domain,structdevice*dev);unsignedintflags;/* Bit field of configs for genpd */++structgenpd_power_statestates[GENPD_MAX_NSTATES];
I think states should be a pointer to an array, not an array of 10 entries, to
avoid wasting memory on systems with a small number of states.
E.g. on r8a73a4, we have 24 PM Domains.
That would waste 24 * 9 * 32 = 6912 bytes of memory.
What about keeping the .power_o{ff,n}() callbacks in struct generic_pm_domain,
but adding a state index parameter to the callbacks?
That would save even more memory on systems where all callbacks are identical.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Axel Haslam <hidden> Date: 2015-04-22 13:23:05
Hi Geert,
quoted
if the genpd is initially off, the user should set the
.init_state field when registering the genpd,
so that genpd knows which callbacks to call to set the
domain to on.
The initial state may depend on various factors (hardware default,
boot loader, previously loaded kernel, ...). This is even true with the
current binary states.
This would mean that the inital state of the genpd cannot be
hardcoded right?
maybe there should be the option to add platform callback
to get the initial state from platform specific code or default
to the hardcoded one? this callback could do whatever it needs to return the on/off status and if off, the index of the current state.
@@ -154,23 +155,24 @@ static void genpd_recalc_cpu_exit_latency(struct generic_pm_domain *genpd) static int genpd_power_on(struct generic_pm_domain *genpd) {+ int target_state = genpd->target_state; ktime_t time_start; s64 elapsed_ns; int ret;- if (!genpd->power_on)+ if (!genpd->states[target_state].power_on) return 0;
While your series now looks compilable for all patch steps, it's not
bisectable: the right hook won't be called until the platform is
converted to support multiple states.
I think you can fix this by e.g. using -1 for the target_state on non-converted
platforms, and doing:
if (target_state < 0) {
if (!genpd->power_on)
return 0;
} else {
if (!genpd->states[target_state].power_on)
return 0;
}
and
ret = target_state < 0 ? genpd->power_on(genpd)
: genpd->states[target_state].power_on(genpd);
The checks for a negative target_state can be removed only after
all platforms have been converted.
Ok, on the next series i will make sure the old callbacks are used until they are converted.
I think states should be a pointer to an array, not an array of 10 entries, to
avoid wasting memory on systems with a small number of states.
Ok, I had it as pointer on array on the first series, i changed it
to make the code simpler, but i can go back to pointer to array
to save memory.
E.g. on r8a73a4, we have 24 PM Domains.
That would waste 24 * 9 * 32 = 6912 bytes of memory.
What about keeping the .power_o{ff,n}() callbacks in struct generic_pm_domain,
but adding a state index parameter to the callbacks?
That would save even more memory on systems where all callbacks are identical.
makes sense.
Thanks,
Axel
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Axel,
On Wed, Apr 22, 2015 at 3:23 PM, Axel Haslam [off-list ref] wrote:
quoted
quoted
if the genpd is initially off, the user should set the
.init_state field when registering the genpd,
so that genpd knows which callbacks to call to set the
domain to on.
The initial state may depend on various factors (hardware default,
boot loader, previously loaded kernel, ...). This is even true with the
current binary states.
This would mean that the inital state of the genpd cannot be
hardcoded right?
Indeed. And it should be filled in by the platform code before registration.
quoted
What about keeping the .power_o{ff,n}() callbacks in struct
generic_pm_domain,
but adding a state index parameter to the callbacks?
That would save even more memory on systems where all callbacks are
identical.
makes sense.
It will complicate compilability/bisectability, though.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Axel Haslam <hidden> Date: 2015-04-22 14:50:39
Hi Geert,
quoted
While your series now looks compilable for all patch steps, it's not
bisectable: the right hook won't be called until the platform is
converted to support multiple states.
I think you can fix this by e.g. using -1 for the target_state on
non-converted
platforms, and doing:
if (target_state < 0) {
if (!genpd->power_on)
return 0;
} else {
if (!genpd->states[target_state].power_on)
return 0;
}
and
ret = target_state < 0 ? genpd->power_on(genpd)
:
genpd->states[target_state].power_on(genpd);
The checks for a negative target_state can be removed only after
all platforms have been converted.
instead of adding checks for negative target state,
i think we can use a single state as a placeholder for the
old callbacks.
would this be an acceptable solution?
on init i can add something like:
+ if (genpd->power_off || genpd->power_on) {
+ pr_warn("Using old genpd callbacks\n");
+ genpd->state_count = 1;
+ genpd->states[0].name = "OFF";
+ genpd->states[0].power_off = genpd->power_off;
+ genpd->states[0].power_off_latency_ns =
+ genpd->power_off_latency_ns;
+ genpd->states[0].power_on = genpd->power_on;
+ genpd->states[0].power_on_latency_ns =
+ genpd->power_on_latency_ns;
+ }
+
this way, old callbacks are called in case the platform
has not been converted yet.
Regards
Axel
From: Axel Haslam <hidden> Date: 2015-04-22 16:26:26
Hi Geert,
On 22/04/2015 15:57, Geert Uytterhoeven wrote:
Hi Axel,
On Wed, Apr 22, 2015 at 3:23 PM, Axel Haslam [off-list ref] wrote:
quoted
quoted
quoted
if the genpd is initially off, the user should set the
.init_state field when registering the genpd,
so that genpd knows which callbacks to call to set the
domain to on.
The initial state may depend on various factors (hardware default,
boot loader, previously loaded kernel, ...). This is even true with the
current binary states.
This would mean that the inital state of the genpd cannot be
hardcoded right?
Indeed. And it should be filled in by the platform code before registration.
quoted
quoted
What about keeping the .power_o{ff,n}() callbacks in struct
generic_pm_domain,
but adding a state index parameter to the callbacks?
That would save even more memory on systems where all callbacks are
identical.
makes sense.
It will complicate compilability/bisectability, though.
since the genpd pointer is allready an argument to the .power_o{ff,n}() callbacks, it would be simpler if the platform could check genpd->target_state to know what state is powering on/off to, instead of adding a new argument.
That way i can remove the power on/off callbacks from the state array,
and there are no big issues with bisectability.
regards,
Axel
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds