[2/2] ARM: vexpress/TC2: implement PM suspend method

Message ID 1370587152-4630-3-git-send-email-nicolas.pitre@linaro.org
State New
Headers show

Commit Message

Nicolas Pitre June 7, 2013, 6:39 a.m.
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(-)

Comments

Lorenzo Pieralisi June 7, 2013, 10:56 a.m. | #1
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
Nicolas Pitre June 10, 2013, 3:56 a.m. | #2
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
Lorenzo Pieralisi June 10, 2013, 4:03 p.m. | #3
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

Patch

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,
 };