* [PATCH] speedstep-smi: enable interrupts when waiting
2013-12-12 0:39 ` [PATCH 3/3] powernow-k6: reorder frequencies Mikulas Patocka
@ 2013-12-12 0:55 ` Mikulas Patocka
0 siblings, 0 replies; 5+ messages in thread
From: Mikulas Patocka @ 2013-12-12 0:55 UTC (permalink / raw
To: Rafael J. Wysocki, Viresh Kumar; +Cc: cpufreq, linux-kernel
On Dell Latitude C600 laptop with Pentium 3 850MHz processor, the
speedstep-smi driver sometimes loads and sometimes doesn't load with
"change to state X failed" message.
I found out that we need to enable interrupts while waiting. When we
enable interrupts, the blockage that prevents frequency transition
resolves and the transition is possible. With disabled interrupts, the
blockage doesn't resolve (no matter how long do we wait).
This patch enables interrupts in the function speedstep_set_state that can
be called with disabled interrupts. However, this function is called with
disabled interrupts only from speedstep_get_freqs, so it shouldn't cause
any problem.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com
Cc: stable@kernel.org
---
drivers/cpufreq/speedstep-lib.c | 1 +
drivers/cpufreq/speedstep-smi.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
Index: linux-3.13-rc3/drivers/cpufreq/speedstep-smi.c
===================================================================
--- linux-3.13-rc3.orig/drivers/cpufreq/speedstep-smi.c 2013-12-12 01:32:37.769060946 +0100
+++ linux-3.13-rc3/drivers/cpufreq/speedstep-smi.c 2013-12-12 01:42:29.240216311 +0100
@@ -200,7 +200,19 @@ static void speedstep_set_state(unsigned
if (retry) {
pr_debug("retry %u, previous result %u, waiting...\n",
retry, result);
+ /*
+ * We need to enable interrupts, otherwise the blockage
+ * won't resolve.
+ *
+ * We disable preemption so that other processes don't
+ * run. If other processes were running, they could
+ * submit more DMA requests, making the blockage worse.
+ */
+ preempt_disable();
+ local_irq_enable();
mdelay(retry * 50);
+ local_irq_disable();
+ preempt_enable_no_resched();
}
retry++;
__asm__ __volatile__(
@@ -217,6 +229,7 @@ static void speedstep_set_state(unsigned
/* enable IRQs */
local_irq_restore(flags);
+ preempt_check_resched();
if (new_state == state)
pr_debug("change to %u MHz succeeded after %u tries "
Index: linux-3.13-rc3/drivers/cpufreq/speedstep-lib.c
===================================================================
--- linux-3.13-rc3.orig/drivers/cpufreq/speedstep-lib.c 2013-10-18 20:24:16.000000000 +0200
+++ linux-3.13-rc3/drivers/cpufreq/speedstep-lib.c 2013-12-12 01:42:41.435120321 +0100
@@ -464,6 +464,7 @@ unsigned int speedstep_get_freqs(enum sp
out:
local_irq_restore(flags);
+ preempt_check_resched();
return ret;
}
EXPORT_SYMBOL_GPL(speedstep_get_freqs);
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] speedstep-smi: enable interrupts when waiting
@ 2015-02-09 18:38 Mikulas Patocka
2015-02-11 3:29 ` Viresh Kumar
0 siblings, 1 reply; 5+ messages in thread
From: Mikulas Patocka @ 2015-02-09 18:38 UTC (permalink / raw
To: Rafael J. Wysocki, Viresh Kumar; +Cc: linux-pm
On Dell Latitude C600 laptop with Pentium 3 850MHz processor, the
speedstep-smi driver sometimes loads and sometimes doesn't load with
"change to state X failed" message.
The hardware sometimes refuses to change frequency and in this case, we
need to retry later. I found out that we need to enable interrupts while
waiting. When we enable interrupts, the hardware blockage that prevents
frequency transition resolves and the transition is possible. With
disabled interrupts, the blockage doesn't resolve (no matter how long do
we wait). The exact reasons for this hardware behavior are unknown.
This patch enables interrupts in the function speedstep_set_state that can
be called with disabled interrupts. However, this function is called with
disabled interrupts only from speedstep_get_freqs, so it shouldn't cause
any problem.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com
Cc: stable@vger.kernel.org
---
drivers/cpufreq/speedstep-lib.c | 3 +++
drivers/cpufreq/speedstep-smi.c | 12 ++++++++++++
2 files changed, 15 insertions(+)
Index: linux-3.14-rc1/drivers/cpufreq/speedstep-smi.c
===================================================================
--- linux-3.14-rc1.orig/drivers/cpufreq/speedstep-smi.c 2014-02-03 18:56:44.000000000 +0100
+++ linux-3.14-rc1/drivers/cpufreq/speedstep-smi.c 2014-02-03 19:06:21.000000000 +0100
@@ -156,6 +156,7 @@ static void speedstep_set_state(unsigned
return;
/* Disable IRQs */
+ preempt_disable();
local_irq_save(flags);
command = (smi_sig & 0xffffff00) | (smi_cmd & 0xff);
@@ -166,9 +167,19 @@ static void speedstep_set_state(unsigned
do {
if (retry) {
+ /*
+ * We need to enable interrupts, otherwise the blockage
+ * won't resolve.
+ *
+ * We disable preemption so that other processes don't
+ * run. If other processes were running, they could
+ * submit more DMA requests, making the blockage worse.
+ */
pr_debug("retry %u, previous result %u, waiting...\n",
retry, result);
+ local_irq_enable();
mdelay(retry * 50);
+ local_irq_disable();
}
retry++;
__asm__ __volatile__(
@@ -185,6 +196,7 @@ static void speedstep_set_state(unsigned
/* enable IRQs */
local_irq_restore(flags);
+ preempt_enable();
if (new_state == state)
pr_debug("change to %u MHz succeeded after %u tries "
Index: linux-3.14-rc1/drivers/cpufreq/speedstep-lib.c
===================================================================
--- linux-3.14-rc1.orig/drivers/cpufreq/speedstep-lib.c 2014-02-03 18:54:39.000000000 +0100
+++ linux-3.14-rc1/drivers/cpufreq/speedstep-lib.c 2014-02-03 19:06:21.000000000 +0100
@@ -400,6 +400,7 @@ unsigned int speedstep_get_freqs(enum sp
pr_debug("previous speed is %u\n", prev_speed);
+ preempt_disable();
local_irq_save(flags);
/* switch to low state */
@@ -464,6 +465,8 @@ unsigned int speedstep_get_freqs(enum sp
out:
local_irq_restore(flags);
+ preempt_enable();
+
return ret;
}
EXPORT_SYMBOL_GPL(speedstep_get_freqs);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] speedstep-smi: enable interrupts when waiting
2015-02-09 18:38 [PATCH] speedstep-smi: enable interrupts when waiting Mikulas Patocka
@ 2015-02-11 3:29 ` Viresh Kumar
2015-02-11 11:01 ` Mikulas Patocka
0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2015-02-11 3:29 UTC (permalink / raw
To: Mikulas Patocka; +Cc: Rafael J. Wysocki, linux-pm@vger.kernel.org
On 10 February 2015 at 02:38, Mikulas Patocka <mpatocka@redhat.com> wrote:
> On Dell Latitude C600 laptop with Pentium 3 850MHz processor, the
> speedstep-smi driver sometimes loads and sometimes doesn't load with
> "change to state X failed" message.
>
> The hardware sometimes refuses to change frequency and in this case, we
> need to retry later. I found out that we need to enable interrupts while
> waiting. When we enable interrupts, the hardware blockage that prevents
> frequency transition resolves and the transition is possible. With
> disabled interrupts, the blockage doesn't resolve (no matter how long do
> we wait). The exact reasons for this hardware behavior are unknown.
>
> This patch enables interrupts in the function speedstep_set_state that can
> be called with disabled interrupts. However, this function is called with
> disabled interrupts only from speedstep_get_freqs, so it shouldn't cause
> any problem.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com
> Cc: stable@vger.kernel.org
>
> ---
> drivers/cpufreq/speedstep-lib.c | 3 +++
> drivers/cpufreq/speedstep-smi.c | 12 ++++++++++++
> 2 files changed, 15 insertions(+)
It looks it was reviewed/Acked long time back, what happened
then ?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] speedstep-smi: enable interrupts when waiting
2015-02-11 3:29 ` Viresh Kumar
@ 2015-02-11 11:01 ` Mikulas Patocka
2015-02-11 14:18 ` Rafael J. Wysocki
0 siblings, 1 reply; 5+ messages in thread
From: Mikulas Patocka @ 2015-02-11 11:01 UTC (permalink / raw
To: Viresh Kumar; +Cc: Rafael J. Wysocki, linux-pm@vger.kernel.org
On Wed, 11 Feb 2015, Viresh Kumar wrote:
> On 10 February 2015 at 02:38, Mikulas Patocka <mpatocka@redhat.com> wrote:
> > On Dell Latitude C600 laptop with Pentium 3 850MHz processor, the
> > speedstep-smi driver sometimes loads and sometimes doesn't load with
> > "change to state X failed" message.
> >
> > The hardware sometimes refuses to change frequency and in this case, we
> > need to retry later. I found out that we need to enable interrupts while
> > waiting. When we enable interrupts, the hardware blockage that prevents
> > frequency transition resolves and the transition is possible. With
> > disabled interrupts, the blockage doesn't resolve (no matter how long do
> > we wait). The exact reasons for this hardware behavior are unknown.
> >
> > This patch enables interrupts in the function speedstep_set_state that can
> > be called with disabled interrupts. However, this function is called with
> > disabled interrupts only from speedstep_get_freqs, so it shouldn't cause
> > any problem.
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com
> > Cc: stable@vger.kernel.org
> >
> > ---
> > drivers/cpufreq/speedstep-lib.c | 3 +++
> > drivers/cpufreq/speedstep-smi.c | 12 ++++++++++++
> > 2 files changed, 15 insertions(+)
>
> It looks it was reviewed/Acked long time back, what happened
> then ?
I don't know. You acknowledged it, but didn't put it upstream.
Mikulas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] speedstep-smi: enable interrupts when waiting
2015-02-11 11:01 ` Mikulas Patocka
@ 2015-02-11 14:18 ` Rafael J. Wysocki
0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2015-02-11 14:18 UTC (permalink / raw
To: Mikulas Patocka; +Cc: Viresh Kumar, linux-pm@vger.kernel.org
On Wednesday, February 11, 2015 06:01:27 AM Mikulas Patocka wrote:
>
> On Wed, 11 Feb 2015, Viresh Kumar wrote:
>
> > On 10 February 2015 at 02:38, Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > On Dell Latitude C600 laptop with Pentium 3 850MHz processor, the
> > > speedstep-smi driver sometimes loads and sometimes doesn't load with
> > > "change to state X failed" message.
> > >
> > > The hardware sometimes refuses to change frequency and in this case, we
> > > need to retry later. I found out that we need to enable interrupts while
> > > waiting. When we enable interrupts, the hardware blockage that prevents
> > > frequency transition resolves and the transition is possible. With
> > > disabled interrupts, the blockage doesn't resolve (no matter how long do
> > > we wait). The exact reasons for this hardware behavior are unknown.
> > >
> > > This patch enables interrupts in the function speedstep_set_state that can
> > > be called with disabled interrupts. However, this function is called with
> > > disabled interrupts only from speedstep_get_freqs, so it shouldn't cause
> > > any problem.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com
> > > Cc: stable@vger.kernel.org
> > >
> > > ---
> > > drivers/cpufreq/speedstep-lib.c | 3 +++
> > > drivers/cpufreq/speedstep-smi.c | 12 ++++++++++++
> > > 2 files changed, 15 insertions(+)
> >
> > It looks it was reviewed/Acked long time back, what happened
> > then ?
>
> I don't know. You acknowledged it, but didn't put it upstream.
That's probably my fault then, because I didn't apply it.
OK, I'll queue it up for the next pull request with an ACK from Viresh.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-11 13:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-09 18:38 [PATCH] speedstep-smi: enable interrupts when waiting Mikulas Patocka
2015-02-11 3:29 ` Viresh Kumar
2015-02-11 11:01 ` Mikulas Patocka
2015-02-11 14:18 ` Rafael J. Wysocki
-- strict thread matches above, loose matches on Subject: below --
2013-12-12 0:37 [PATCH 0/3] powernow-k6: fixes for the frequency driver Mikulas Patocka
2013-12-12 0:38 ` [PATCH 1/3] powernow-k6: disable cache when changing frequency Mikulas Patocka
2013-12-12 0:38 ` [PATCH 2/3] powernow-k6: correctly initialize default parameters Mikulas Patocka
2013-12-12 0:39 ` [PATCH 3/3] powernow-k6: reorder frequencies Mikulas Patocka
2013-12-12 0:55 ` [PATCH] speedstep-smi: enable interrupts when waiting Mikulas Patocka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.