public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] VFP context save/restore support for OMAP3
@ 2009-11-19 17:06 Tero Kristo
  2009-11-19 17:06 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
  0 siblings, 1 reply; 11+ messages in thread
From: Tero Kristo @ 2009-11-19 17:06 UTC (permalink / raw)
  To: linux-omap

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

These two patches add support for VFP context save and restore. These patches
are a collaboration from several people (listed in CC), I have mainly
split the original patch in two parts (one generic ARM, one for OMAP3 PM),
and optimized the code a bit. No sign-offs added from anybody except myself
at the moment because I have modified the patches quite heavily.

I think patch #1 should actually go to linux-arm list, however I thought to
get some comments about the patches on omap list first.

These will apply on top of PM branch.

-Tero



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-19 17:06 [PATCH 0/2] VFP context save/restore support for OMAP3 Tero Kristo
@ 2009-11-19 17:06 ` Tero Kristo
  2009-11-19 17:06   ` [PATCH 2/2] OMAP3: Implemented VFP restore/save context Tero Kristo
  2009-11-23 17:39   ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tony Lindgren
  0 siblings, 2 replies; 11+ messages in thread
From: Tero Kristo @ 2009-11-19 17:06 UTC (permalink / raw)
  To: linux-omap

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>
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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] OMAP3: Implemented VFP restore/save context
  2009-11-19 17:06 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
@ 2009-11-19 17:06   ` Tero Kristo
  2009-11-23 22:22     ` Kevin Hilman
  2009-11-23 17:39   ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tony Lindgren
  1 sibling, 1 reply; 11+ messages in thread
From: Tero Kristo @ 2009-11-19 17:06 UTC (permalink / raw)
  To: linux-omap

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-19 17:06 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
  2009-11-19 17:06   ` [PATCH 2/2] OMAP3: Implemented VFP restore/save context Tero Kristo
@ 2009-11-23 17:39   ` Tony Lindgren
  1 sibling, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2009-11-23 17:39 UTC (permalink / raw)
  To: Tero Kristo; +Cc: linux-omap

* Tero Kristo <tero.kristo@nokia.com> [091119 07:12]:
> 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.

This should go via the linux-arm-kernel@lists.infradead.org list.
We should probably merge them both via LAKML as they logically belong
toghether. Can you please resend, and also Cc linux-omap list?

For both, you can add Acked-by: Tony Lindgren <tony@atomide.com>
if you want to.

 
> 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/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
> 
> --
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] OMAP3: Implemented VFP restore/save context
  2009-11-19 17:06   ` [PATCH 2/2] OMAP3: Implemented VFP restore/save context Tero Kristo
@ 2009-11-23 22:22     ` Kevin Hilman
  0 siblings, 0 replies; 11+ messages in thread
From: Kevin Hilman @ 2009-11-23 22:22 UTC (permalink / raw)
  To: Tero Kristo; +Cc: linux-omap

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 10:37 [PATCH 0/2] VFP save/restore for OMAP3 Tero Kristo
@ 2009-11-24 10:37 ` Tero Kristo
  2009-11-24 11:48   ` Russell King - ARM Linux
  0 siblings, 1 reply; 11+ messages in thread
From: Tero Kristo @ 2009-11-24 10:37 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-omap

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 10:37 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
@ 2009-11-24 11:48   ` Russell King - ARM Linux
  2009-11-24 13:20     ` Catalin Marinas
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux @ 2009-11-24 11:48 UTC (permalink / raw)
  To: Tero Kristo; +Cc: linux-arm-kernel, linux-omap

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?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 11:48   ` Russell King - ARM Linux
@ 2009-11-24 13:20     ` Catalin Marinas
  2009-11-24 14:05       ` Tero.Kristo
  2009-11-24 15:19       ` Russell King - ARM Linux
  0 siblings, 2 replies; 11+ messages in thread
From: Catalin Marinas @ 2009-11-24 13:20 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Tero Kristo, Dave Estes, linux-omap, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 13:20     ` Catalin Marinas
@ 2009-11-24 14:05       ` Tero.Kristo
  2009-11-24 15:19       ` Russell King - ARM Linux
  1 sibling, 0 replies; 11+ messages in thread
From: Tero.Kristo @ 2009-11-24 14:05 UTC (permalink / raw)
  To: catalin.marinas, linux; +Cc: linux-omap, linux-arm-kernel, cestes

 

>-----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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 13:20     ` Catalin Marinas
  2009-11-24 14:05       ` Tero.Kristo
@ 2009-11-24 15:19       ` Russell King - ARM Linux
  2009-11-27 10:06         ` Tero.Kristo
  1 sibling, 1 reply; 11+ messages in thread
From: Russell King - ARM Linux @ 2009-11-24 15:19 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Tero Kristo, linux-omap, linux-arm-kernel, Dave Estes

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
  2009-11-24 15:19       ` Russell King - ARM Linux
@ 2009-11-27 10:06         ` Tero.Kristo
  0 siblings, 0 replies; 11+ messages in thread
From: Tero.Kristo @ 2009-11-27 10:06 UTC (permalink / raw)
  To: linux, catalin.marinas; +Cc: linux-omap, linux-arm-kernel, cestes

 

>-----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.
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-11-27 10:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-19 17:06 [PATCH 0/2] VFP context save/restore support for OMAP3 Tero Kristo
2009-11-19 17:06 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
2009-11-19 17:06   ` [PATCH 2/2] OMAP3: Implemented VFP restore/save context Tero Kristo
2009-11-23 22:22     ` Kevin Hilman
2009-11-23 17:39   ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tony Lindgren
  -- strict thread matches above, loose matches on Subject: below --
2009-11-24 10:37 [PATCH 0/2] VFP save/restore for OMAP3 Tero Kristo
2009-11-24 10:37 ` [PATCH 1/2] ARM: Implemented support for VFP PM context saving Tero Kristo
2009-11-24 11:48   ` Russell King - ARM Linux
2009-11-24 13:20     ` Catalin Marinas
2009-11-24 14:05       ` Tero.Kristo
2009-11-24 15:19       ` Russell King - ARM Linux
2009-11-27 10:06         ` Tero.Kristo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox