RE: [PATCH 1/2] ARM: Implemented support for VFP PM context saving
-Original Message- From: Russell King - ARM Linux [mailto:li...@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-ker...@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 +, 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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ARM: Implemented support for VFP PM context saving
From: Tero Kristo tero.kri...@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.kri...@nokia.com
Acked-by: Tony Lindgren t...@atomide.com
Cc: Vishwanath Sripathy vishwanath...@ti.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Richard Woodruff r-woodru...@ti.com
Cc: Peter 'p2' De Schrijver peter.de-schrij...@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 majord...@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
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? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@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
On Tue, 2009-11-24 at 11:48 +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@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
-Original Message- From: ext Catalin Marinas [mailto:catalin.mari...@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-ker...@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 +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@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
On Tue, Nov 24, 2009 at 01:20:26PM +, 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. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@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
* Tero Kristo tero.kri...@nokia.com [091119 07:12]:
From: Tero Kristo tero.kri...@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-ker...@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 t...@atomide.com
if you want to.
Signed-off-by: Tero Kristo tero.kri...@nokia.com
Cc: Vishwanath Sripathy vishwanath...@ti.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Richard Woodruff r-woodru...@ti.com
Cc: Peter 'p2' De Schrijver peter.de-schrij...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ARM: Implemented support for VFP PM context saving
From: Tero Kristo tero.kri...@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.kri...@nokia.com
Cc: Vishwanath Sripathy vishwanath...@ti.com
Cc: Rajendra Nayak rna...@ti.com
Cc: Richard Woodruff r-woodru...@ti.com
Cc: Peter 'p2' De Schrijver peter.de-schrij...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
