[PATCH 0/2] VFP save/restore for OMAP3

[PATCH 0/2] VFP save/restore for OMAP3

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Hi,

The following two patches are needed by OMAP3 to save/restore VFP context
during off-mode. Patch 1 adds generic support inside ARM VFP code, and the
second one adds the necessary hooks into OMAP3 power management code.

--Tero

[PATCH 0/2] VFP save/restore for OMAP3

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Hi,

The following two patches are needed by OMAP3 to save/restore VFP context
during off-mode. Patch 1 adds generic support inside ARM VFP code, and the
second one adds the necessary hooks into OMAP3 power management code.

--Tero

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

In some ARM architectures, like OMAP3, the VFP context can be lost during
dynamic sleep cycle. For this purpose, there is now a function
vfp_pm_save_context() that should be called before the VFP is assumed to
lose context. Next VFP trap will then restore context automatically.

We need to have the last_VFP_context[cpu] cleared after the save in idle,
else the restore would fail to restore when it sees that the last_VFP_context
is same as the current threads vfp_state. This happens when the same
process/thread traps an exception post idle.

Main work for this patch was done by Peter and Rajendra. Some cleanup and
optimization by Tero.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
---
 arch/arm/vfp/vfpmodule.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2d7423a..80a08bd 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -329,6 +329,31 @@ static void vfp_enable(void *unused)
 #ifdef CONFIG_PM
 #include <linux/sysdev.h>
 
+void vfp_pm_save_context(void)
+{
+	struct thread_info *thread = current_thread_info();
+	u32 fpexc = fmrx(FPEXC);
+	__u32 cpu = thread->cpu;
+
+	if (last_VFP_context[cpu]) {
+		if (!(fpexc & FPEXC_EN)) {
+			/* enable vfp now to save context */
+			vfp_enable(NULL);
+			fmxr(FPEXC, fmrx(FPEXC) | FPEXC_EN);
+		}
+		vfp_save_state(last_VFP_context[cpu], fpexc);
+
+		/* Disable vfp. The next inst traps an exception and restores*/
+		fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
+
+		/*
+		 * This is needed else the restore might fail if it sees
+		 * last_VFP_context if same as the current threads vfp_state.
+		 */
+		last_VFP_context[cpu] = NULL;
+	}
+}
+
 static int vfp_pm_suspend(struct sys_device *dev, pm_message_t state)
 {
 	struct thread_info *ti = current_thread_info();
-- 
1.5.4.3

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

In some ARM architectures, like OMAP3, the VFP context can be lost during
dynamic sleep cycle. For this purpose, there is now a function
vfp_pm_save_context() that should be called before the VFP is assumed to
lose context. Next VFP trap will then restore context automatically.

We need to have the last_VFP_context[cpu] cleared after the save in idle,
else the restore would fail to restore when it sees that the last_VFP_context
is same as the current threads vfp_state. This happens when the same
process/thread traps an exception post idle.

Main work for this patch was done by Peter and Rajendra. Some cleanup and
optimization by Tero.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
---
 arch/arm/vfp/vfpmodule.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2d7423a..80a08bd 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -329,6 +329,31 @@ static void vfp_enable(void *unused)
 #ifdef CONFIG_PM
 #include <linux/sysdev.h>
 
+void vfp_pm_save_context(void)
+{
+	struct thread_info *thread = current_thread_info();
+	u32 fpexc = fmrx(FPEXC);
+	__u32 cpu = thread->cpu;
+
+	if (last_VFP_context[cpu]) {
+		if (!(fpexc & FPEXC_EN)) {
+			/* enable vfp now to save context */
+			vfp_enable(NULL);
+			fmxr(FPEXC, fmrx(FPEXC) | FPEXC_EN);
+		}
+		vfp_save_state(last_VFP_context[cpu], fpexc);
+
+		/* Disable vfp. The next inst traps an exception and restores*/
+		fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
+
+		/*
+		 * This is needed else the restore might fail if it sees
+		 * last_VFP_context if same as the current threads vfp_state.
+		 */
+		last_VFP_context[cpu] = NULL;
+	}
+}
+
 static int vfp_pm_suspend(struct sys_device *dev, pm_message_t state)
 {
 	struct thread_info *ti = current_thread_info();
-- 
1.5.4.3

[PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

VFP save context is called before MPU/NEON off. Restore is not needed as
the next VFP trap will restore context automatically. Uses the support
routine implemented in arch/arm/vfp/vfpmodule.c.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
---
 arch/arm/mach-omap2/pm.h     |    1 +
 arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 4f22107..dd5bbaf 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
 extern u32 voltage_off_while_idle;
 
 extern void *omap3_secure_ram_storage;
+extern void vfp_pm_save_context(void);
 extern void omap3_pm_off_mode_enable(int);
 extern void omap_sram_idle(void);
 extern int omap3_can_sleep(void);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index b26ae5b..4b01303 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
 	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
 }
 
+static inline void omap3_save_neon_context(void)
+{
+#ifdef CONFIG_VFP
+	vfp_pm_save_context();
+#endif
+}
+
+static inline void omap3_restore_neon_context(void)
+{
+	return;
+}
+
 /* Function to restore the table entry that was modified for enabling MMU */
 static void restore_table_entry(void)
 {
@@ -365,6 +377,7 @@ void omap_sram_idle(void)
 	/* save_state = 3 => L1, L2 and logic lost */
 	int save_state = 0;
 	int mpu_next_state = PWRDM_POWER_ON;
+	int neon_next_state = PWRDM_POWER_ON;
 	int per_next_state = PWRDM_POWER_ON;
 	int core_next_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
@@ -398,8 +411,12 @@ void omap_sram_idle(void)
 	pwrdm_pre_transition();
 
 	/* NEON control */
-	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
+	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+		neon_next_state = mpu_next_state;
+		if (neon_next_state == PWRDM_POWER_OFF)
+			omap3_save_neon_context();
+	}
 
 	/* PER */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
@@ -537,6 +554,8 @@ void omap_sram_idle(void)
 		omap3_disable_io_chain();
 	}
 
+	if (neon_next_state == PWRDM_POWER_OFF)
+		omap3_restore_neon_context();
 
 	pwrdm_post_transition();
 }
-- 
1.5.4.3

[PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

VFP save context is called before MPU/NEON off. Restore is not needed as
the next VFP trap will restore context automatically. Uses the support
routine implemented in arch/arm/vfp/vfpmodule.c.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
---
 arch/arm/mach-omap2/pm.h     |    1 +
 arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 4f22107..dd5bbaf 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
 extern u32 voltage_off_while_idle;
 
 extern void *omap3_secure_ram_storage;
+extern void vfp_pm_save_context(void);
 extern void omap3_pm_off_mode_enable(int);
 extern void omap_sram_idle(void);
 extern int omap3_can_sleep(void);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index b26ae5b..4b01303 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
 	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
 }
 
+static inline void omap3_save_neon_context(void)
+{
+#ifdef CONFIG_VFP
+	vfp_pm_save_context();
+#endif
+}
+
+static inline void omap3_restore_neon_context(void)
+{
+	return;
+}
+
 /* Function to restore the table entry that was modified for enabling MMU */
 static void restore_table_entry(void)
 {
@@ -365,6 +377,7 @@ void omap_sram_idle(void)
 	/* save_state = 3 => L1, L2 and logic lost */
 	int save_state = 0;
 	int mpu_next_state = PWRDM_POWER_ON;
+	int neon_next_state = PWRDM_POWER_ON;
 	int per_next_state = PWRDM_POWER_ON;
 	int core_next_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
@@ -398,8 +411,12 @@ void omap_sram_idle(void)
 	pwrdm_pre_transition();
 
 	/* NEON control */
-	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
+	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+		neon_next_state = mpu_next_state;
+		if (neon_next_state == PWRDM_POWER_OFF)
+			omap3_save_neon_context();
+	}
 
 	/* PER */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
@@ -537,6 +554,8 @@ void omap_sram_idle(void)
 		omap3_disable_io_chain();
 	}
 
+	if (neon_next_state == PWRDM_POWER_OFF)
+		omap3_restore_neon_context();
 
 	pwrdm_post_transition();
 }
-- 
1.5.4.3

RE: [PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Sripathy, Vishwanath]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tero Kristo
> Sent: Tuesday, November 24, 2009 4:07 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: linux-omap@vger.kernel.org
> Subject: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
> 
> From: Tero Kristo <tero.kristo@nokia.com>
> 
> VFP save context is called before MPU/NEON off. Restore is not needed as
> the next VFP trap will restore context automatically. Uses the support
> routine implemented in arch/arm/vfp/vfpmodule.c.
> 
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Richard Woodruff <r-woodruff2@ti.com>
> Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
> ---
>  arch/arm/mach-omap2/pm.h     |    1 +
>  arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 4f22107..dd5bbaf 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
>  extern u32 voltage_off_while_idle;
> 
>  extern void *omap3_secure_ram_storage;
> +extern void vfp_pm_save_context(void);
>  extern void omap3_pm_off_mode_enable(int);
>  extern void omap_sram_idle(void);
>  extern int omap3_can_sleep(void);
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index b26ae5b..4b01303 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
>  	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
>  }
> 
> +static inline void omap3_save_neon_context(void)
> +{
> +#ifdef CONFIG_VFP
> +	vfp_pm_save_context();
> +#endif
> +}
> +
> +static inline void omap3_restore_neon_context(void)
> +{
> +	return;
> +}
> +
>  /* Function to restore the table entry that was modified for enabling MMU */
>  static void restore_table_entry(void)
>  {
> @@ -365,6 +377,7 @@ void omap_sram_idle(void)
>  	/* save_state = 3 => L1, L2 and logic lost */
>  	int save_state = 0;
>  	int mpu_next_state = PWRDM_POWER_ON;
> +	int neon_next_state = PWRDM_POWER_ON;
>  	int per_next_state = PWRDM_POWER_ON;
>  	int core_next_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
> @@ -398,8 +411,12 @@ void omap_sram_idle(void)
>  	pwrdm_pre_transition();
> 
>  	/* NEON control */
> -	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> +	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> +		neon_next_state = mpu_next_state;
> +		if (neon_next_state == PWRDM_POWER_OFF)
> +			omap3_save_neon_context();
> +	}
> 
>  	/* PER */
>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> @@ -537,6 +554,8 @@ void omap_sram_idle(void)
>  		omap3_disable_io_chain();
>  	}
> 
> +	if (neon_next_state == PWRDM_POWER_OFF)
> +		omap3_restore_neon_context();
> 
Why do call restore here when restore is done automatically upon context switch?

>  	pwrdm_post_transition();
>  }
> --
> 1.5.4.3
> 
> --
> 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

[PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Sripathy, Vishwanath]

> -----Original Message-----
> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
> owner at vger.kernel.org] On Behalf Of Tero Kristo
> Sent: Tuesday, November 24, 2009 4:07 PM
> To: linux-arm-kernel at lists.infradead.org
> Cc: linux-omap at vger.kernel.org
> Subject: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
> 
> From: Tero Kristo <tero.kristo@nokia.com>
> 
> VFP save context is called before MPU/NEON off. Restore is not needed as
> the next VFP trap will restore context automatically. Uses the support
> routine implemented in arch/arm/vfp/vfpmodule.c.
> 
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Richard Woodruff <r-woodruff2@ti.com>
> Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
> ---
>  arch/arm/mach-omap2/pm.h     |    1 +
>  arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 4f22107..dd5bbaf 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
>  extern u32 voltage_off_while_idle;
> 
>  extern void *omap3_secure_ram_storage;
> +extern void vfp_pm_save_context(void);
>  extern void omap3_pm_off_mode_enable(int);
>  extern void omap_sram_idle(void);
>  extern int omap3_can_sleep(void);
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index b26ae5b..4b01303 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
>  	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
>  }
> 
> +static inline void omap3_save_neon_context(void)
> +{
> +#ifdef CONFIG_VFP
> +	vfp_pm_save_context();
> +#endif
> +}
> +
> +static inline void omap3_restore_neon_context(void)
> +{
> +	return;
> +}
> +
>  /* Function to restore the table entry that was modified for enabling MMU */
>  static void restore_table_entry(void)
>  {
> @@ -365,6 +377,7 @@ void omap_sram_idle(void)
>  	/* save_state = 3 => L1, L2 and logic lost */
>  	int save_state = 0;
>  	int mpu_next_state = PWRDM_POWER_ON;
> +	int neon_next_state = PWRDM_POWER_ON;
>  	int per_next_state = PWRDM_POWER_ON;
>  	int core_next_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
> @@ -398,8 +411,12 @@ void omap_sram_idle(void)
>  	pwrdm_pre_transition();
> 
>  	/* NEON control */
> -	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> +	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> +		neon_next_state = mpu_next_state;
> +		if (neon_next_state == PWRDM_POWER_OFF)
> +			omap3_save_neon_context();
> +	}
> 
>  	/* PER */
>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> @@ -537,6 +554,8 @@ void omap_sram_idle(void)
>  		omap3_disable_io_chain();
>  	}
> 
> +	if (neon_next_state == PWRDM_POWER_OFF)
> +		omap3_restore_neon_context();
> 
Why do call restore here when restore is done automatically upon context switch?

>  	pwrdm_post_transition();
>  }
> --
> 1.5.4.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Sripathy, Vishwanath [mailto:vishwanath.bs@ti.com] 
>Sent: 24 November, 2009 13:34
>To: Kristo Tero (Nokia-D/Tampere); linux-arm-kernel@lists.infradead.org
>Cc: linux-omap@vger.kernel.org
>Subject: RE: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
>
>
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Tero Kristo
>> Sent: Tuesday, November 24, 2009 4:07 PM
>> To: linux-arm-kernel@lists.infradead.org
>> Cc: linux-omap@vger.kernel.org
>> Subject: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
>> 
>> From: Tero Kristo <tero.kristo@nokia.com>
>> 
>> VFP save context is called before MPU/NEON off. Restore is 
>not needed as
>> the next VFP trap will restore context automatically. Uses 
>the support
>> routine implemented in arch/arm/vfp/vfpmodule.c.
>> 
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> Acked-by: Tony Lindgren <tony@atomide.com>
>> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
>> Cc: Rajendra Nayak <rnayak@ti.com>
>> Cc: Richard Woodruff <r-woodruff2@ti.com>
>> Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm.h     |    1 +
>>  arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
>>  2 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>> index 4f22107..dd5bbaf 100644
>> --- a/arch/arm/mach-omap2/pm.h
>> +++ b/arch/arm/mach-omap2/pm.h
>> @@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
>>  extern u32 voltage_off_while_idle;
>> 
>>  extern void *omap3_secure_ram_storage;
>> +extern void vfp_pm_save_context(void);
>>  extern void omap3_pm_off_mode_enable(int);
>>  extern void omap_sram_idle(void);
>>  extern int omap3_can_sleep(void);
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index b26ae5b..4b01303 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
>>  	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : 
>"r" (val));
>>  }
>> 
>> +static inline void omap3_save_neon_context(void)
>> +{
>> +#ifdef CONFIG_VFP
>> +	vfp_pm_save_context();
>> +#endif
>> +}
>> +
>> +static inline void omap3_restore_neon_context(void)
>> +{
>> +	return;
>> +}
>> +
>>  /* Function to restore the table entry that was modified 
>for enabling MMU */
>>  static void restore_table_entry(void)
>>  {
>> @@ -365,6 +377,7 @@ void omap_sram_idle(void)
>>  	/* save_state = 3 => L1, L2 and logic lost */
>>  	int save_state = 0;
>>  	int mpu_next_state = PWRDM_POWER_ON;
>> +	int neon_next_state = PWRDM_POWER_ON;
>>  	int per_next_state = PWRDM_POWER_ON;
>>  	int core_next_state = PWRDM_POWER_ON;
>>  	int core_prev_state, per_prev_state;
>> @@ -398,8 +411,12 @@ void omap_sram_idle(void)
>>  	pwrdm_pre_transition();
>> 
>>  	/* NEON control */
>> -	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>> +	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
>>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
>> +		neon_next_state = mpu_next_state;
>> +		if (neon_next_state == PWRDM_POWER_OFF)
>> +			omap3_save_neon_context();
>> +	}
>> 
>>  	/* PER */
>>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>> @@ -537,6 +554,8 @@ void omap_sram_idle(void)
>>  		omap3_disable_io_chain();
>>  	}
>> 
>> +	if (neon_next_state == PWRDM_POWER_OFF)
>> +		omap3_restore_neon_context();
>> 
>Why do call restore here when restore is done automatically 
>upon context switch?

That is just a stub if we would need to add something there at some point. restore_neon_context() currently does nothing.


>
>>  	pwrdm_post_transition();
>>  }
>> --
>> 1.5.4.3
>> 
>> --
>> 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
>

[PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Tero.Kristo at nokia.com]

 

>-----Original Message-----
>From: ext Sripathy, Vishwanath [mailto:vishwanath.bs at ti.com] 
>Sent: 24 November, 2009 13:34
>To: Kristo Tero (Nokia-D/Tampere); linux-arm-kernel at lists.infradead.org
>Cc: linux-omap at vger.kernel.org
>Subject: RE: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
>
>
>
>> -----Original Message-----
>> From: linux-omap-owner at vger.kernel.org [mailto:linux-omap-
>> owner at vger.kernel.org] On Behalf Of Tero Kristo
>> Sent: Tuesday, November 24, 2009 4:07 PM
>> To: linux-arm-kernel at lists.infradead.org
>> Cc: linux-omap at vger.kernel.org
>> Subject: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
>> 
>> From: Tero Kristo <tero.kristo@nokia.com>
>> 
>> VFP save context is called before MPU/NEON off. Restore is 
>not needed as
>> the next VFP trap will restore context automatically. Uses 
>the support
>> routine implemented in arch/arm/vfp/vfpmodule.c.
>> 
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> Acked-by: Tony Lindgren <tony@atomide.com>
>> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
>> Cc: Rajendra Nayak <rnayak@ti.com>
>> Cc: Richard Woodruff <r-woodruff2@ti.com>
>> Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm.h     |    1 +
>>  arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
>>  2 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>> index 4f22107..dd5bbaf 100644
>> --- a/arch/arm/mach-omap2/pm.h
>> +++ b/arch/arm/mach-omap2/pm.h
>> @@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
>>  extern u32 voltage_off_while_idle;
>> 
>>  extern void *omap3_secure_ram_storage;
>> +extern void vfp_pm_save_context(void);
>>  extern void omap3_pm_off_mode_enable(int);
>>  extern void omap_sram_idle(void);
>>  extern int omap3_can_sleep(void);
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index b26ae5b..4b01303 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
>>  	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : 
>"r" (val));
>>  }
>> 
>> +static inline void omap3_save_neon_context(void)
>> +{
>> +#ifdef CONFIG_VFP
>> +	vfp_pm_save_context();
>> +#endif
>> +}
>> +
>> +static inline void omap3_restore_neon_context(void)
>> +{
>> +	return;
>> +}
>> +
>>  /* Function to restore the table entry that was modified 
>for enabling MMU */
>>  static void restore_table_entry(void)
>>  {
>> @@ -365,6 +377,7 @@ void omap_sram_idle(void)
>>  	/* save_state = 3 => L1, L2 and logic lost */
>>  	int save_state = 0;
>>  	int mpu_next_state = PWRDM_POWER_ON;
>> +	int neon_next_state = PWRDM_POWER_ON;
>>  	int per_next_state = PWRDM_POWER_ON;
>>  	int core_next_state = PWRDM_POWER_ON;
>>  	int core_prev_state, per_prev_state;
>> @@ -398,8 +411,12 @@ void omap_sram_idle(void)
>>  	pwrdm_pre_transition();
>> 
>>  	/* NEON control */
>> -	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
>> +	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
>>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
>> +		neon_next_state = mpu_next_state;
>> +		if (neon_next_state == PWRDM_POWER_OFF)
>> +			omap3_save_neon_context();
>> +	}
>> 
>>  	/* PER */
>>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>> @@ -537,6 +554,8 @@ void omap_sram_idle(void)
>>  		omap3_disable_io_chain();
>>  	}
>> 
>> +	if (neon_next_state == PWRDM_POWER_OFF)
>> +		omap3_restore_neon_context();
>> 
>Why do call restore here when restore is done automatically 
>upon context switch?

That is just a stub if we would need to add something there at some point. restore_neon_context() currently does nothing.


>
>>  	pwrdm_post_transition();
>>  }
>> --
>> 1.5.4.3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe 
>linux-omap" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
> In some ARM architectures, like OMAP3, the VFP context can be lost during
> dynamic sleep cycle. For this purpose, there is now a function
> vfp_pm_save_context() that should be called before the VFP is assumed to
> lose context. Next VFP trap will then restore context automatically.
> 
> We need to have the last_VFP_context[cpu] cleared after the save in idle,
> else the restore would fail to restore when it sees that the last_VFP_context
> is same as the current threads vfp_state. This happens when the same
> process/thread traps an exception post idle.
> 
> Main work for this patch was done by Peter and Rajendra. Some cleanup and
> optimization by Tero.

Why not re-use vfp_pm_suspend() ?  Haven't you shown that vfp_pm_suspend
may be buggy since it doesn't save in the VFP-disabled case?

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
> In some ARM architectures, like OMAP3, the VFP context can be lost during
> dynamic sleep cycle. For this purpose, there is now a function
> vfp_pm_save_context() that should be called before the VFP is assumed to
> lose context. Next VFP trap will then restore context automatically.
> 
> We need to have the last_VFP_context[cpu] cleared after the save in idle,
> else the restore would fail to restore when it sees that the last_VFP_context
> is same as the current threads vfp_state. This happens when the same
> process/thread traps an exception post idle.
> 
> Main work for this patch was done by Peter and Rajendra. Some cleanup and
> optimization by Tero.

Why not re-use vfp_pm_suspend() ?  Haven't you shown that vfp_pm_suspend
may be buggy since it doesn't save in the VFP-disabled case?

Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Catalin Marinas]

On Tue, 2009-11-24 at 11:48 +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
> > In some ARM architectures, like OMAP3, the VFP context can be lost during
> > dynamic sleep cycle. For this purpose, there is now a function
> > vfp_pm_save_context() that should be called before the VFP is assumed to
> > lose context. Next VFP trap will then restore context automatically.
> >
> > We need to have the last_VFP_context[cpu] cleared after the save in idle,
> > else the restore would fail to restore when it sees that the last_VFP_context
> > is same as the current threads vfp_state. This happens when the same
> > process/thread traps an exception post idle.
> >
> > Main work for this patch was done by Peter and Rajendra. Some cleanup and
> > optimization by Tero.
> 
> Why not re-use vfp_pm_suspend() ?  Haven't you shown that vfp_pm_suspend
> may be buggy since it doesn't save in the VFP-disabled case?

BTW, the two patches below were mentioned to me some time ago but I
haven't got the time to look at them:

[ARM] vfp: Fix bug in vfp_pm_suspend
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff

[ARM] vfp: Add additional vfp interfaces
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

-- 
Catalin

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Catalin Marinas]

On Tue, 2009-11-24 at 11:48 +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
> > In some ARM architectures, like OMAP3, the VFP context can be lost during
> > dynamic sleep cycle. For this purpose, there is now a function
> > vfp_pm_save_context() that should be called before the VFP is assumed to
> > lose context. Next VFP trap will then restore context automatically.
> >
> > We need to have the last_VFP_context[cpu] cleared after the save in idle,
> > else the restore would fail to restore when it sees that the last_VFP_context
> > is same as the current threads vfp_state. This happens when the same
> > process/thread traps an exception post idle.
> >
> > Main work for this patch was done by Peter and Rajendra. Some cleanup and
> > optimization by Tero.
> 
> Why not re-use vfp_pm_suspend() ?  Haven't you shown that vfp_pm_suspend
> may be buggy since it doesn't save in the VFP-disabled case?

BTW, the two patches below were mentioned to me some time ago but I
haven't got the time to look at them:

[ARM] vfp: Fix bug in vfp_pm_suspend
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff

[ARM] vfp: Add additional vfp interfaces
https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

-- 
Catalin

RE: [PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Catalin Marinas [mailto:catalin.marinas@arm.com] 
>Sent: 24 November, 2009 15:20
>To: Russell King - ARM Linux
>Cc: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org; 
>linux-arm-kernel@lists.infradead.org; Dave Estes
>Subject: Re: [PATCH 1/2] ARM: Implemented support for VFP PM 
>context saving
>
>On Tue, 2009-11-24 at 11:48 +0000, Russell King - ARM Linux wrote:
>> On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
>> > In some ARM architectures, like OMAP3, the VFP context can 
>be lost during
>> > dynamic sleep cycle. For this purpose, there is now a function
>> > vfp_pm_save_context() that should be called before the VFP 
>is assumed to
>> > lose context. Next VFP trap will then restore context 
>automatically.
>> >
>> > We need to have the last_VFP_context[cpu] cleared after 
>the save in idle,
>> > else the restore would fail to restore when it sees that 
>the last_VFP_context
>> > is same as the current threads vfp_state. This happens 
>when the same
>> > process/thread traps an exception post idle.
>> >
>> > Main work for this patch was done by Peter and Rajendra. 
>Some cleanup and
>> > optimization by Tero.
>> 
>> Why not re-use vfp_pm_suspend() ?  Haven't you shown that 
>vfp_pm_suspend
>> may be buggy since it doesn't save in the VFP-disabled case?
>BTW, the two patches below were mentioned to me some time ago but I
>haven't got the time to look at them:
>
>[ARM] vfp: Fix bug in vfp_pm_suspend
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff
>
>[ARM] vfp: Add additional vfp interfaces
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

These look pretty much like the same thing we have attempted on the omap3 patches, I could try these out and check if they work for omap3 also. They fix the potentially broken suspend also.

-Tero

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero.Kristo at nokia.com]

 

>-----Original Message-----
>From: ext Catalin Marinas [mailto:catalin.marinas at arm.com] 
>Sent: 24 November, 2009 15:20
>To: Russell King - ARM Linux
>Cc: Kristo Tero (Nokia-D/Tampere); linux-omap at vger.kernel.org; 
>linux-arm-kernel at lists.infradead.org; Dave Estes
>Subject: Re: [PATCH 1/2] ARM: Implemented support for VFP PM 
>context saving
>
>On Tue, 2009-11-24 at 11:48 +0000, Russell King - ARM Linux wrote:
>> On Tue, Nov 24, 2009 at 12:37:03PM +0200, Tero Kristo wrote:
>> > In some ARM architectures, like OMAP3, the VFP context can 
>be lost during
>> > dynamic sleep cycle. For this purpose, there is now a function
>> > vfp_pm_save_context() that should be called before the VFP 
>is assumed to
>> > lose context. Next VFP trap will then restore context 
>automatically.
>> >
>> > We need to have the last_VFP_context[cpu] cleared after 
>the save in idle,
>> > else the restore would fail to restore when it sees that 
>the last_VFP_context
>> > is same as the current threads vfp_state. This happens 
>when the same
>> > process/thread traps an exception post idle.
>> >
>> > Main work for this patch was done by Peter and Rajendra. 
>Some cleanup and
>> > optimization by Tero.
>> 
>> Why not re-use vfp_pm_suspend() ?  Haven't you shown that 
>vfp_pm_suspend
>> may be buggy since it doesn't save in the VFP-disabled case?
>BTW, the two patches below were mentioned to me some time ago but I
>haven't got the time to look at them:
>
>[ARM] vfp: Fix bug in vfp_pm_suspend
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff
>
>[ARM] vfp: Add additional vfp interfaces
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

These look pretty much like the same thing we have attempted on the omap3 patches, I could try these out and check if they work for omap3 also. They fix the potentially broken suspend also.

-Tero

Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 01:20:26PM +0000, Catalin Marinas wrote:
> BTW, the two patches below were mentioned to me some time ago but I
> haven't got the time to look at them:
> 
> [ARM] vfp: Fix bug in vfp_pm_suspend
> https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff

This one is bad - it gets the current CPU by directly referencing ti->cpu.
Too bad if you have kernel preemption enabled; the value obtained that
way is effectively meaningless.  The only way to get a meaningful value
is via 'get_cpu()' and after you've done with using it, 'put_cpu()'.
That ensures you can't be preempted onto a different CPU mid-operation.

It's safe in vfp_notifier because we're called in an already atomic context.

> [ARM] vfp: Add additional vfp interfaces
> https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

I don't like what's in this one.  Lack of explaination in the commit log
doesn't help.

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 01:20:26PM +0000, Catalin Marinas wrote:
> BTW, the two patches below were mentioned to me some time ago but I
> haven't got the time to look at them:
> 
> [ARM] vfp: Fix bug in vfp_pm_suspend
> https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff

This one is bad - it gets the current CPU by directly referencing ti->cpu.
Too bad if you have kernel preemption enabled; the value obtained that
way is effectively meaningless.  The only way to get a meaningful value
is via 'get_cpu()' and after you've done with using it, 'put_cpu()'.
That ensures you can't be preempted onto a different CPU mid-operation.

It's safe in vfp_notifier because we're called in an already atomic context.

> [ARM] vfp: Add additional vfp interfaces
> https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=commit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668

I don't like what's in this one.  Lack of explaination in the commit log
doesn't help.

RE: [PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero.Kristo]

 

>-----Original Message-----
>From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] 
>Sent: 24 November, 2009 17:19
>To: Catalin Marinas
>Cc: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org; 
>linux-arm-kernel@lists.infradead.org; Dave Estes
>Subject: Re: [PATCH 1/2] ARM: Implemented support for VFP PM 
>context saving
>
>On Tue, Nov 24, 2009 at 01:20:26PM +0000, Catalin Marinas wrote:
>> BTW, the two patches below were mentioned to me some time ago but I
>> haven't got the time to look at them:
>> 
>> [ARM] vfp: Fix bug in vfp_pm_suspend
>> 
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff
>
>This one is bad - it gets the current CPU by directly 
>referencing ti->cpu.
>Too bad if you have kernel preemption enabled; the value obtained that
>way is effectively meaningless.  The only way to get a meaningful value
>is via 'get_cpu()' and after you've done with using it, 'put_cpu()'.
>That ensures you can't be preempted onto a different CPU mid-operation.
>
>It's safe in vfp_notifier because we're called in an already 
>atomic context.

I investigated this issue a bit more, and indeed there is a potential bug in the vfp_pm_suspend(). Most of the time it works fine as apparently shell process has VFP enabled at least on my system, and it saves the state. The issue is different with dynamic idle, we are calling the code from init thread which does not need VFP for anything, and thus VFP is always disabled if we try to call vfp_pm_suspend(). For OMAP3, I found a way to fix the dynamic idle part to work properly by just simply calling vfp_sync_state() from idle. This functionality is supposed to be used by ptrace, but I guess it could be used for this also?

A proper fix for suspend is bit more difficult, as I don't know too well how SMP is supposed to work in this case. I guess there is a separate VFP co-processor available for each ARM core, but vfp_pm_suspend() is only called once for the whole system?

>
>> [ARM] vfp: Add additional vfp interfaces
>> 
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668
>
>I don't like what's in this one.  Lack of explaination in the 
>commit log
>doesn't help.
>

[PATCH 1/2] ARM: Implemented support for VFP PM context saving

[Tero.Kristo at nokia.com]

 

>-----Original Message-----
>From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk] 
>Sent: 24 November, 2009 17:19
>To: Catalin Marinas
>Cc: Kristo Tero (Nokia-D/Tampere); linux-omap at vger.kernel.org; 
>linux-arm-kernel at lists.infradead.org; Dave Estes
>Subject: Re: [PATCH 1/2] ARM: Implemented support for VFP PM 
>context saving
>
>On Tue, Nov 24, 2009 at 01:20:26PM +0000, Catalin Marinas wrote:
>> BTW, the two patches below were mentioned to me some time ago but I
>> haven't got the time to look at them:
>> 
>> [ARM] vfp: Fix bug in vfp_pm_suspend
>> 
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=88984c9b2d69c222ee1e2afc948ca73f597d40ff
>
>This one is bad - it gets the current CPU by directly 
>referencing ti->cpu.
>Too bad if you have kernel preemption enabled; the value obtained that
>way is effectively meaningless.  The only way to get a meaningful value
>is via 'get_cpu()' and after you've done with using it, 'put_cpu()'.
>That ensures you can't be preempted onto a different CPU mid-operation.
>
>It's safe in vfp_notifier because we're called in an already 
>atomic context.

I investigated this issue a bit more, and indeed there is a potential bug in the vfp_pm_suspend(). Most of the time it works fine as apparently shell process has VFP enabled at least on my system, and it saves the state. The issue is different with dynamic idle, we are calling the code from init thread which does not need VFP for anything, and thus VFP is always disabled if we try to call vfp_pm_suspend(). For OMAP3, I found a way to fix the dynamic idle part to work properly by just simply calling vfp_sync_state() from idle. This functionality is supposed to be used by ptrace, but I guess it could be used for this also?

A proper fix for suspend is bit more difficult, as I don't know too well how SMP is supposed to work in this case. I guess there is a separate VFP co-processor available for each ARM core, but vfp_pm_suspend() is only called once for the whole system?

>
>> [ARM] vfp: Add additional vfp interfaces
>> 
>https://www.codeaurora.org/gitweb/quic/le/?p=kernel/msm.git;a=c
>ommit;h=393e4bfaaf79377d29cd6bb2228f87601aeca668
>
>I don't like what's in this one.  Lack of explaination in the 
>commit log
>doesn't help.
>

Re: [PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> VFP save context is called before MPU/NEON off. Restore is not needed as
> the next VFP trap will restore context automatically. Uses the support
> routine implemented in arch/arm/vfp/vfpmodule.c.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Richard Woodruff <r-woodruff2@ti.com>
> Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>

Looks good.  I'll queue in PM branch when patch 1/2 is reviewed/accepted on LAKML.

Kevin

> ---
>  arch/arm/mach-omap2/pm.h     |    1 +
>  arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 4f22107..dd5bbaf 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
>  extern u32 voltage_off_while_idle;
>  
>  extern void *omap3_secure_ram_storage;
> +extern void vfp_pm_save_context(void);
>  extern void omap3_pm_off_mode_enable(int);
>  extern void omap_sram_idle(void);
>  extern int omap3_can_sleep(void);
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index b26ae5b..4b01303 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
>  	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
>  }
>  
> +static inline void omap3_save_neon_context(void)
> +{
> +#ifdef CONFIG_VFP
> +	vfp_pm_save_context();
> +#endif
> +}
> +
> +static inline void omap3_restore_neon_context(void)
> +{
> +	return;
> +}
> +
>  /* Function to restore the table entry that was modified for enabling MMU */
>  static void restore_table_entry(void)
>  {
> @@ -365,6 +377,7 @@ void omap_sram_idle(void)
>  	/* save_state = 3 => L1, L2 and logic lost */
>  	int save_state = 0;
>  	int mpu_next_state = PWRDM_POWER_ON;
> +	int neon_next_state = PWRDM_POWER_ON;
>  	int per_next_state = PWRDM_POWER_ON;
>  	int core_next_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
> @@ -398,8 +411,12 @@ void omap_sram_idle(void)
>  	pwrdm_pre_transition();
>  
>  	/* NEON control */
> -	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
> +	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
>  		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
> +		neon_next_state = mpu_next_state;
> +		if (neon_next_state == PWRDM_POWER_OFF)
> +			omap3_save_neon_context();
> +	}
>  
>  	/* PER */
>  	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
> @@ -537,6 +554,8 @@ void omap_sram_idle(void)
>  		omap3_disable_io_chain();
>  	}
>  
> +	if (neon_next_state == PWRDM_POWER_OFF)
> +		omap3_restore_neon_context();
>  
>  	pwrdm_post_transition();
>  }
> -- 
> 1.5.4.3
>
> --
> 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

[PATCH 2/2] OMAP3: Implemented VFP restore/save context

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

VFP save context is called before MPU/NEON off. Restore is not needed as
the next VFP trap will restore context automatically. Uses the support
routine implemented in arch/arm/vfp/vfpmodule.c.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Peter 'p2' De Schrijver <peter.de-schrijver@nokia.com>
---
 arch/arm/mach-omap2/pm.h     |    1 +
 arch/arm/mach-omap2/pm34xx.c |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 4f22107..dd5bbaf 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -18,6 +18,7 @@ extern u32 sleep_while_idle;
 extern u32 voltage_off_while_idle;
 
 extern void *omap3_secure_ram_storage;
+extern void vfp_pm_save_context(void);
 extern void omap3_pm_off_mode_enable(int);
 extern void omap_sram_idle(void);
 extern int omap3_can_sleep(void);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index b26ae5b..4b01303 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -321,6 +321,18 @@ static void restore_control_register(u32 val)
 	__asm__ __volatile__ ("mcr p15, 0, %0, c1, c0, 0" : : "r" (val));
 }
 
+static inline void omap3_save_neon_context(void)
+{
+#ifdef CONFIG_VFP
+	vfp_pm_save_context();
+#endif
+}
+
+static inline void omap3_restore_neon_context(void)
+{
+	return;
+}
+
 /* Function to restore the table entry that was modified for enabling MMU */
 static void restore_table_entry(void)
 {
@@ -365,6 +377,7 @@ void omap_sram_idle(void)
 	/* save_state = 3 => L1, L2 and logic lost */
 	int save_state = 0;
 	int mpu_next_state = PWRDM_POWER_ON;
+	int neon_next_state = PWRDM_POWER_ON;
 	int per_next_state = PWRDM_POWER_ON;
 	int core_next_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
@@ -398,8 +411,12 @@ void omap_sram_idle(void)
 	pwrdm_pre_transition();
 
 	/* NEON control */
-	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
+	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON) {
 		pwrdm_set_next_pwrst(neon_pwrdm, mpu_next_state);
+		neon_next_state = mpu_next_state;
+		if (neon_next_state == PWRDM_POWER_OFF)
+			omap3_save_neon_context();
+	}
 
 	/* PER */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
@@ -537,6 +554,8 @@ void omap_sram_idle(void)
 		omap3_disable_io_chain();
 	}
 
+	if (neon_next_state == PWRDM_POWER_OFF)
+		omap3_restore_neon_context();
 
 	pwrdm_post_transition();
 }
-- 
1.5.4.3