| Message ID | 1370587152-4630-3-git-send-email-nicolas.pitre@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Nico, On Fri, Jun 07, 2013 at 07:39:12AM +0100, Nicolas Pitre wrote: > From: Nicolas Pitre <nico@linaro.org> > > This is simplistic for the moment as the expected residency is used to > prevent the shutting down of L2 and the cluster if the residency for > the last man is lower than 5 ms. To make this right, the shutdown > latency for each CPU would need to be recorded and taken into account. > > On a suspend, the firmware mailbox address has to be set prior entering > low power mode. > > Signed-off-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/mach-vexpress/tc2_pm.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c > index a3ea524372..7901528566 100644 > --- a/arch/arm/mach-vexpress/tc2_pm.c > +++ b/arch/arm/mach-vexpress/tc2_pm.c > @@ -79,7 +79,7 @@ static int tc2_pm_power_up(unsigned int cpu, unsigned int cluster) > return 0; > } > > -static void tc2_pm_power_down(void) > +static void tc2_pm_down(u64 residency) > { > unsigned int mpidr, cpu, cluster; > bool last_man = false, skip_wfi = false; > @@ -100,7 +100,8 @@ static void tc2_pm_power_down(void) > vexpress_spc_set_cpu_wakeup_irq(cpu, cluster, 1); > if (!tc2_pm_use_count[0][cluster] && > !tc2_pm_use_count[1][cluster] && > - !tc2_pm_use_count[2][cluster]) { > + !tc2_pm_use_count[2][cluster] && > + (!residency || residency > 5000)) { I would remove the line above altogether. It is ok to have parameter (we should still define what that residency actually is), I do not think it makes sense to add policy at this stage, I have a patchset to queue for that. Thanks, Lorenzo
On Fri, 7 Jun 2013, Lorenzo Pieralisi wrote: > Hi Nico, > > On Fri, Jun 07, 2013 at 07:39:12AM +0100, Nicolas Pitre wrote: > > From: Nicolas Pitre <nico@linaro.org> > > > > This is simplistic for the moment as the expected residency is used to > > prevent the shutting down of L2 and the cluster if the residency for > > the last man is lower than 5 ms. To make this right, the shutdown > > latency for each CPU would need to be recorded and taken into account. > > > > On a suspend, the firmware mailbox address has to be set prior entering > > low power mode. > > > > Signed-off-by: Nicolas Pitre <nico@linaro.org> > > --- > > arch/arm/mach-vexpress/tc2_pm.c | 23 +++++++++++++++++++++-- > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c > > index a3ea524372..7901528566 100644 > > --- a/arch/arm/mach-vexpress/tc2_pm.c > > +++ b/arch/arm/mach-vexpress/tc2_pm.c > > @@ -79,7 +79,7 @@ static int tc2_pm_power_up(unsigned int cpu, unsigned int cluster) > > return 0; > > } > > > > -static void tc2_pm_power_down(void) > > +static void tc2_pm_down(u64 residency) > > { > > unsigned int mpidr, cpu, cluster; > > bool last_man = false, skip_wfi = false; > > @@ -100,7 +100,8 @@ static void tc2_pm_power_down(void) > > vexpress_spc_set_cpu_wakeup_irq(cpu, cluster, 1); > > if (!tc2_pm_use_count[0][cluster] && > > !tc2_pm_use_count[1][cluster] && > > - !tc2_pm_use_count[2][cluster]) { > > + !tc2_pm_use_count[2][cluster] && > > + (!residency || residency > 5000)) { > > I would remove the line above altogether. It is ok to have parameter (we > should still define what that residency actually is), I do not think it > makes sense to add policy at this stage, I have a patchset to queue for > that. OK, let's discuss that separately. I'll simply let the whole cluster go down in all cases initially. That's what we have right now anyway as the cpuidle driver is not passing anything interesting at the moment other than 0. Nicolas
On Mon, Jun 10, 2013 at 04:56:12AM +0100, Nicolas Pitre wrote: > On Fri, 7 Jun 2013, Lorenzo Pieralisi wrote: > > > Hi Nico, > > > > On Fri, Jun 07, 2013 at 07:39:12AM +0100, Nicolas Pitre wrote: > > > From: Nicolas Pitre <nico@linaro.org> > > > > > > This is simplistic for the moment as the expected residency is used to > > > prevent the shutting down of L2 and the cluster if the residency for > > > the last man is lower than 5 ms. To make this right, the shutdown > > > latency for each CPU would need to be recorded and taken into account. > > > > > > On a suspend, the firmware mailbox address has to be set prior entering > > > low power mode. > > > > > > Signed-off-by: Nicolas Pitre <nico@linaro.org> > > > --- > > > arch/arm/mach-vexpress/tc2_pm.c | 23 +++++++++++++++++++++-- > > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c > > > index a3ea524372..7901528566 100644 > > > --- a/arch/arm/mach-vexpress/tc2_pm.c > > > +++ b/arch/arm/mach-vexpress/tc2_pm.c > > > @@ -79,7 +79,7 @@ static int tc2_pm_power_up(unsigned int cpu, unsigned int cluster) > > > return 0; > > > } > > > > > > -static void tc2_pm_power_down(void) > > > +static void tc2_pm_down(u64 residency) > > > { > > > unsigned int mpidr, cpu, cluster; > > > bool last_man = false, skip_wfi = false; > > > @@ -100,7 +100,8 @@ static void tc2_pm_power_down(void) > > > vexpress_spc_set_cpu_wakeup_irq(cpu, cluster, 1); > > > if (!tc2_pm_use_count[0][cluster] && > > > !tc2_pm_use_count[1][cluster] && > > > - !tc2_pm_use_count[2][cluster]) { > > > + !tc2_pm_use_count[2][cluster] && > > > + (!residency || residency > 5000)) { > > > > I would remove the line above altogether. It is ok to have parameter (we > > should still define what that residency actually is), I do not think it > > makes sense to add policy at this stage, I have a patchset to queue for > > that. > > OK, let's discuss that separately. I'll simply let the whole cluster go > down in all cases initially. That's what we have right now anyway as > the cpuidle driver is not passing anything interesting at the moment > other than 0. Agreed, thanks. Lorenzo
diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c index a3ea524372..7901528566 100644 --- a/arch/arm/mach-vexpress/tc2_pm.c +++ b/arch/arm/mach-vexpress/tc2_pm.c @@ -79,7 +79,7 @@ static int tc2_pm_power_up(unsigned int cpu, unsigned int cluster) return 0; } -static void tc2_pm_power_down(void) +static void tc2_pm_down(u64 residency) { unsigned int mpidr, cpu, cluster; bool last_man = false, skip_wfi = false; @@ -100,7 +100,8 @@ static void tc2_pm_power_down(void) vexpress_spc_set_cpu_wakeup_irq(cpu, cluster, 1); if (!tc2_pm_use_count[0][cluster] && !tc2_pm_use_count[1][cluster] && - !tc2_pm_use_count[2][cluster]) { + !tc2_pm_use_count[2][cluster] && + (!residency || residency > 5000)) { vexpress_spc_powerdown_enable(cluster, 1); vexpress_spc_set_global_wakeup_intr(1); last_man = true; @@ -161,6 +162,23 @@ static void tc2_pm_power_down(void) /* Not dead at this point? Let our caller cope. */ } +static void tc2_pm_power_down(void) +{ + tc2_pm_down(0); +} + +static void tc2_pm_suspend(u64 residency) +{ + unsigned int mpidr, cpu, cluster; + + mpidr = read_cpuid_mpidr(); + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1); + vexpress_spc_write_resume_reg(cluster, cpu, + virt_to_phys(mcpm_entry_point)); + tc2_pm_down(residency); +} + static void tc2_pm_powered_up(void) { unsigned int mpidr, cpu, cluster; @@ -196,6 +214,7 @@ static void tc2_pm_powered_up(void) static const struct mcpm_platform_ops tc2_pm_power_ops = { .power_up = tc2_pm_power_up, .power_down = tc2_pm_power_down, + .suspend = tc2_pm_suspend, .powered_up = tc2_pm_powered_up, };