| Message ID | 1342189185-5306-5-git-send-email-t-kristo@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 13 July 2012 07:49 PM, Tero Kristo wrote: > mpu / core powerdomain usecounts are now statically increased > by 1 during MPU activity. This allows the domains to reflect > actual usage, and will allow the usecount to reach 0 just before > all CPUs are ready to idle. Proper powerdomain usecounts are > propageted to voltagedomain level also, and will allow vc > callbacks to be triggered at right point of time. > > Signed-off-by: Tero Kristo<t-kristo@ti.com> > Cc: Paul Walmsley<paul@pwsan.com> > Cc: Kevin Hilman<khilman@ti.com> > --- > arch/arm/mach-omap2/pm34xx.c | 3 ++ > arch/arm/mach-omap2/pm44xx.c | 3 ++ > arch/arm/mach-omap2/powerdomain.c | 64 +++++++++++++++++++++++++++++++++++++ > arch/arm/mach-omap2/powerdomain.h | 3 ++ > 4 files changed, 73 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c > index 3a595e8..7c7b173 100644 > --- a/arch/arm/mach-omap2/pm34xx.c > +++ b/arch/arm/mach-omap2/pm34xx.c > @@ -758,6 +758,9 @@ int __init omap3_pm_init(void) > omap_pm_suspend = omap3_pm_suspend; > #endif > > + /* Notify pwrdm usecounters about active CPU */ > + pwrdm_cpu_wakeup(); > + These internally increment/decrement usecount for MPU and CORE but the name pwrdm_cpu_wakeup/idle seems somewhat misleading. But I don't know either what would be a better name, so.. Reviewed-by: Rajendra Nayak <rnayak@ti.com>
Tero Kristo <t-kristo@ti.com> writes: > mpu / core powerdomain usecounts are now statically increased > by 1 during MPU activity. This allows the domains to reflect > actual usage, and will allow the usecount to reach 0 just before > all CPUs are ready to idle. Proper powerdomain usecounts are > propageted to voltagedomain level also, and will allow vc > callbacks to be triggered at right point of time. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > Cc: Paul Walmsley <paul@pwsan.com> > Cc: Kevin Hilman <khilman@ti.com> IMO, the idea is fine, but I'm not crazy about the implementation in powerdomain.c, which is meant for pwrdm generic code. In particular, I'm not crazy about the pwrdm lookups in powerdomain.c. Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c Also, the changelog should be a bit more specific about why CORE powerdomain is also handled here when most of the code only talks about the CPU. Kevin
On Fri, 2012-07-27 at 12:36 -0700, Kevin Hilman wrote: > Tero Kristo <t-kristo@ti.com> writes: > > > mpu / core powerdomain usecounts are now statically increased > > by 1 during MPU activity. This allows the domains to reflect > > actual usage, and will allow the usecount to reach 0 just before > > all CPUs are ready to idle. Proper powerdomain usecounts are > > propageted to voltagedomain level also, and will allow vc > > callbacks to be triggered at right point of time. > > > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > > Cc: Paul Walmsley <paul@pwsan.com> > > Cc: Kevin Hilman <khilman@ti.com> > > IMO, the idea is fine, but I'm not crazy about the implementation in > powerdomain.c, which is meant for pwrdm generic code. In particular, > I'm not crazy about the pwrdm lookups in powerdomain.c. > > Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why > not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c I think this was how the patch was in some earlier rev but I thought I'd try to be more clever with this. :) I can revert the implementation back to this. > Also, the changelog should be a bit more specific about why CORE > powerdomain is also handled here when most of the code only talks about > the CPU. Yea, I'll add some beef to this also. -Tero
Hi Tero, On Mon, Jul 30, 2012 at 10:40 AM, Tero Kristo <t-kristo@ti.com> wrote: > On Fri, 2012-07-27 at 12:36 -0700, Kevin Hilman wrote: >> Tero Kristo <t-kristo@ti.com> writes: >> >> > mpu / core powerdomain usecounts are now statically increased >> > by 1 during MPU activity. This allows the domains to reflect >> > actual usage, and will allow the usecount to reach 0 just before >> > all CPUs are ready to idle. Proper powerdomain usecounts are >> > propageted to voltagedomain level also, and will allow vc >> > callbacks to be triggered at right point of time. >> > >> > Signed-off-by: Tero Kristo <t-kristo@ti.com> >> > Cc: Paul Walmsley <paul@pwsan.com> >> > Cc: Kevin Hilman <khilman@ti.com> >> >> IMO, the idea is fine, but I'm not crazy about the implementation in >> powerdomain.c, which is meant for pwrdm generic code. In particular, >> I'm not crazy about the pwrdm lookups in powerdomain.c. >> >> Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why >> not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c > > I think this was how the patch was in some earlier rev but I thought I'd > try to be more clever with this. :) I can revert the implementation back > to this. Furthermore after the changes in pre/post transitions [1], some more checks will be needed to identify the transitions on the mpu and core power domains. [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e055548953355b6e69c56f9e54388845b29b4e97 Regards, Jean > >> Also, the changelog should be a bit more specific about why CORE >> powerdomain is also handled here when most of the code only talks about >> the CPU. > > Yea, I'll add some beef to this also. > > -Tero > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Kevin Hilman <khilman@ti.com> writes: > Tero Kristo <t-kristo@ti.com> writes: > >> mpu / core powerdomain usecounts are now statically increased >> by 1 during MPU activity. This allows the domains to reflect >> actual usage, and will allow the usecount to reach 0 just before >> all CPUs are ready to idle. Proper powerdomain usecounts are >> propageted to voltagedomain level also, and will allow vc >> callbacks to be triggered at right point of time. >> >> Signed-off-by: Tero Kristo <t-kristo@ti.com> >> Cc: Paul Walmsley <paul@pwsan.com> >> Cc: Kevin Hilman <khilman@ti.com> > > IMO, the idea is fine, but I'm not crazy about the implementation in > powerdomain.c, which is meant for pwrdm generic code. In particular, > I'm not crazy about the pwrdm lookups in powerdomain.c. > > Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why > not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c > > Also, the changelog should be a bit more specific about why CORE > powerdomain is also handled here when most of the code only talks about > the CPU. After playing with this some more, and doing a test hack of my idea above, along with the per-pwrdm pre/post changes now in mainline, I thinks this series should go one step further. Basically, we should be able to get rid of the calls to prdm_pre|post_transition that are outside the powerdomain code. With the addition of pwrdm_clkdm_enable|disable, it seems that the pre|post transition calls should be called directly from pwrdm_clkdm_enable|disable when the usecount transitions to/from zero. Kevin
On Mon, 2012-08-06 at 12:14 +0200, Jean Pihet wrote: > Hi Tero, > > On Mon, Jul 30, 2012 at 10:40 AM, Tero Kristo <t-kristo@ti.com> wrote: > > On Fri, 2012-07-27 at 12:36 -0700, Kevin Hilman wrote: > >> Tero Kristo <t-kristo@ti.com> writes: > >> > >> > mpu / core powerdomain usecounts are now statically increased > >> > by 1 during MPU activity. This allows the domains to reflect > >> > actual usage, and will allow the usecount to reach 0 just before > >> > all CPUs are ready to idle. Proper powerdomain usecounts are > >> > propageted to voltagedomain level also, and will allow vc > >> > callbacks to be triggered at right point of time. > >> > > >> > Signed-off-by: Tero Kristo <t-kristo@ti.com> > >> > Cc: Paul Walmsley <paul@pwsan.com> > >> > Cc: Kevin Hilman <khilman@ti.com> > >> > >> IMO, the idea is fine, but I'm not crazy about the implementation in > >> powerdomain.c, which is meant for pwrdm generic code. In particular, > >> I'm not crazy about the pwrdm lookups in powerdomain.c. > >> > >> Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why > >> not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c > > > > I think this was how the patch was in some earlier rev but I thought I'd > > try to be more clever with this. :) I can revert the implementation back > > to this. > Furthermore after the changes in pre/post transitions [1], some more > checks will be needed to identify the transitions on the mpu and core > power domains. > > [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e055548953355b6e69c56f9e54388845b29b4e97 > > Regards, > Jean Hi Kevin, What is the latest status regarding this one, seeing the patch mentioned got reverted due to problems? Should I do some changes for this or not? I can look at moving the code away from the generic powerdomain.c at least, but is there anything else? -Tero > > > > >> Also, the changelog should be a bit more specific about why CORE > >> powerdomain is also handled here when most of the code only talks about > >> the CPU. > > > > Yea, I'll add some beef to this also. > > > > -Tero > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo <t-kristo@ti.com> writes: > On Mon, 2012-08-06 at 12:14 +0200, Jean Pihet wrote: >> Hi Tero, >> >> On Mon, Jul 30, 2012 at 10:40 AM, Tero Kristo <t-kristo@ti.com> wrote: >> > On Fri, 2012-07-27 at 12:36 -0700, Kevin Hilman wrote: >> >> Tero Kristo <t-kristo@ti.com> writes: >> >> >> >> > mpu / core powerdomain usecounts are now statically increased >> >> > by 1 during MPU activity. This allows the domains to reflect >> >> > actual usage, and will allow the usecount to reach 0 just before >> >> > all CPUs are ready to idle. Proper powerdomain usecounts are >> >> > propageted to voltagedomain level also, and will allow vc >> >> > callbacks to be triggered at right point of time. >> >> > >> >> > Signed-off-by: Tero Kristo <t-kristo@ti.com> >> >> > Cc: Paul Walmsley <paul@pwsan.com> >> >> > Cc: Kevin Hilman <khilman@ti.com> >> >> >> >> IMO, the idea is fine, but I'm not crazy about the implementation in >> >> powerdomain.c, which is meant for pwrdm generic code. In particular, >> >> I'm not crazy about the pwrdm lookups in powerdomain.c. >> >> >> >> Since pm<soc>.c already has references to mpu_pwrdm and core_pwrdm, why >> >> not just add the pwrdm_clkdm_enable/disable calls directly in pm<soc>.c >> > >> > I think this was how the patch was in some earlier rev but I thought I'd >> > try to be more clever with this. :) I can revert the implementation back >> > to this. >> Furthermore after the changes in pre/post transitions [1], some more >> checks will be needed to identify the transitions on the mpu and core >> power domains. >> >> [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e055548953355b6e69c56f9e54388845b29b4e97 >> >> Regards, >> Jean > > Hi Kevin, > > What is the latest status regarding this one, seeing the patch mentioned > got reverted due to problems? Should I do some changes for this or not? No, using latest minline should be fine. > I can look at moving the code away from the generic powerdomain.c at > least, but is there anything else? Nothing else I can think of (but my vacation has purged my brain of most of the details here, so I may be forgetting something.) Kevin
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index 3a595e8..7c7b173 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -758,6 +758,9 @@ int __init omap3_pm_init(void) omap_pm_suspend = omap3_pm_suspend; #endif + /* Notify pwrdm usecounters about active CPU */ + pwrdm_cpu_wakeup(); + arm_pm_idle = omap3_pm_idle; omap3_idle_init(); diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c index ea24174..45eef2d 100644 --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm44xx.c @@ -206,6 +206,9 @@ int __init omap4_pm_init(void) omap_pm_suspend = omap4_pm_suspend; #endif + /* Notify pwrdm usecounters about active CPU */ + pwrdm_cpu_wakeup(); + /* Overwrite the default cpu_do_idle() */ arm_pm_idle = omap_default_idle; diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 3b4b15d..39a45a9 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -47,6 +47,7 @@ enum { static LIST_HEAD(pwrdm_list); static struct pwrdm_ops *arch_pwrdm; +static struct powerdomain *mpu_pwrdm, *core_pwrdm; /* Private functions */ @@ -1023,11 +1024,15 @@ void pwrdm_clkdm_disable(struct powerdomain *pwrdm) int pwrdm_pre_transition(void) { pwrdm_for_each(_pwrdm_pre_transition_cb, NULL); + /* Decrease mpu / core usecounts to indicate we are entering idle */ + pwrdm_cpu_idle(); return 0; } int pwrdm_post_transition(void) { + /* Increase mpu / core usecounts to indicate we are leaving idle */ + pwrdm_cpu_wakeup(); pwrdm_for_each(_pwrdm_post_transition_cb, NULL); return 0; } @@ -1107,3 +1112,62 @@ bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm) return 0; } + +/** + * pwrdm_get_idle_cycle_pwrdms - init pwrdms needed by idle cycle + * + * MPU and CORE powerdomain states are changed automatically by + * hardware during suspend / cpuidle. Thus, the usecounts for + * these domains will be manually changed during pwrdm_pre_transition + * pwrdm_post_transition callbacks. The callbacks will need pointers + * to the MPU and CORE powerdomains, so this init function is used + * to get those once needed. + */ +static void pwrdm_get_idle_cycle_pwrdms(void) +{ + mpu_pwrdm = pwrdm_lookup("mpu_pwrdm"); + if (!mpu_pwrdm) + pr_err_once("%s: failed to get mpu_pwrdm\n", __func__); + + core_pwrdm = pwrdm_lookup("core_pwrdm"); + if (!core_pwrdm) + pr_err_once("%s: failed to get core_pwrdm\n", __func__); +} + +/** + * pwrdm_cpu_wakeup - notify pwrdm usecounters about active CPU + * + * This function must be called just after a CPU has become active. + * Some powerdomains have static dependencies with MPU idle cycle, + * namely mpu_pwrdm and core_pwrdm. These powerdomains will get + * their usecounts increased / decreased each sleep cycle so that + * they reach 0 just before all CPUs have reached idle, and wake-up + * right after it. This allows the dependent voltage domains to + * follow idle cycle properly and trigger their callbacks for + * sleep / wakeup, which in turn will control e.g. auto retention + * feature. + */ +void pwrdm_cpu_wakeup(void) +{ + if (!mpu_pwrdm || !core_pwrdm) + pwrdm_get_idle_cycle_pwrdms(); + + pwrdm_clkdm_enable(mpu_pwrdm); + pwrdm_clkdm_enable(core_pwrdm); +} + +/** + * pwrdm_cpu_idle - notify pwrdm usecounters about idling CPU + * + * This function must be called just before CPU is about to idle. + * Similar to pwrdm_cpu_wakeup, this is used to make sure the idle + * cycle dependent powerdomains follow the sleep cycle properly. + */ +void pwrdm_cpu_idle(void) +{ + if (!mpu_pwrdm || !core_pwrdm) + pwrdm_get_idle_cycle_pwrdms(); + + pwrdm_clkdm_disable(mpu_pwrdm); + pwrdm_clkdm_disable(core_pwrdm); +} diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index 705b983..ecf7d3d 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -220,6 +220,9 @@ int pwrdm_post_transition(void); void pwrdm_clkdm_enable(struct powerdomain *pwrdm); void pwrdm_clkdm_disable(struct powerdomain *pwrdm); +void pwrdm_cpu_wakeup(void); +void pwrdm_cpu_idle(void); + int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm); int pwrdm_get_context_loss_count(struct powerdomain *pwrdm); bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
mpu / core powerdomain usecounts are now statically increased by 1 during MPU activity. This allows the domains to reflect actual usage, and will allow the usecount to reach 0 just before all CPUs are ready to idle. Proper powerdomain usecounts are propageted to voltagedomain level also, and will allow vc callbacks to be triggered at right point of time. Signed-off-by: Tero Kristo <t-kristo@ti.com> Cc: Paul Walmsley <paul@pwsan.com> Cc: Kevin Hilman <khilman@ti.com> --- arch/arm/mach-omap2/pm34xx.c | 3 ++ arch/arm/mach-omap2/pm44xx.c | 3 ++ arch/arm/mach-omap2/powerdomain.c | 64 +++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/powerdomain.h | 3 ++ 4 files changed, 73 insertions(+), 0 deletions(-)