2009-10-24 15:08:13

by Krzysztof Helt

[permalink] [raw]
Subject: [PATCH] longhaul: select Longhaul version 2 for capable CPUs

From: Krzysztof Helt <[email protected]>

There is a typo in the longhaul detection code so only Longhaul v1 or Longhaul v3
is selected. The Longhaul v2 is not selected even for CPUs which are capable of.

Tested on PCChips Giga Pro board. Frequency changes work and the Longhaul v2
detects that the board is not capable of changing CPU voltage.

Signed-off-by: Krzysztof Helt <[email protected]>

---
diff --git a/arch/x86/kernel/cpu/cpufreq/longhaul.c b/arch/x86/kernel/cpu/cpufreq/longhaul.c
index ce2ed3e..cabd2fa 100644
--- a/arch/x86/kernel/cpu/cpufreq/longhaul.c
+++ b/arch/x86/kernel/cpu/cpufreq/longhaul.c
@@ -813,7 +813,7 @@ static int __init longhaul_cpu_init(struct cpufreq_policy *policy)
memcpy(eblcr, samuel2_eblcr, sizeof(samuel2_eblcr));
break;
case 1 ... 15:
- longhaul_version = TYPE_LONGHAUL_V1;
+ longhaul_version = TYPE_LONGHAUL_V2;
if (c->x86_mask < 8) {
cpu_model = CPU_SAMUEL2;
cpuname = "C3 'Samuel 2' [C5B]";


---------------------------------------------------------------
Zobacz jak pracuje sie na wysokosciach.
Kliknij >>> http://link.interia.pl/f2384


2009-10-25 03:28:15

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] longhaul: select Longhaul version 2 for capable CPUs

On Sat, Oct 24, 2009 at 05:25:38PM +0200, Krzysztof Helt wrote:
> From: Krzysztof Helt <[email protected]>
>
> There is a typo in the longhaul detection code so only Longhaul v1 or Longhaul v3
> is selected. The Longhaul v2 is not selected even for CPUs which are capable of.
>
> Tested on PCChips Giga Pro board. Frequency changes work and the Longhaul v2
> detects that the board is not capable of changing CPU voltage.
>
> Signed-off-by: Krzysztof Helt <[email protected]>

It seems we deliberately changed this two years ago, though the changelog
is a bit sparse on details..

commit 07844252ffd81ec192a62014bada1016c9703765
Author: Rafal Bilski <[email protected]>
Date: Sun Apr 22 12:26:04 2007 +0200

[CPUFREQ] Longhaul - Revert Longhaul ver. 2

There is something wrong with this code. It needs more
testing. It is better to disable it for now because support
for some machines will be broken.

Signed-off-by: Rafal Bilski <[email protected]>
Signed-off-by: Dave Jones <[email protected]>


In hindsight, changing it to report V1 instead of V2 was the wrong thing
to do, and we should have done something like -ENODEV with
a printk explaining why.

I've not got any old VIA CPUs/boards to test with any more, but I'm inclined
to apply your change, but we'll have to keep an eye out for any strange
bugs on affected systems. Currently this driver should do nothing, as the
longhaul v1 registers that don't exist on longhaul V2 CPUs. With this change,
we're going to be actually doing scaling again, which may introduce instability
on some machines, as iirc, we never did get this driver 100% stable due
to a lot of really crappy motherboards.

Perhaps we should printk a warning related to this.
(We should definitly mention it in the Kconfig too, which I thought we already had)

Dave

2009-10-25 16:53:31

by Rafał Bilski

[permalink] [raw]
Subject: Re: [PATCH] longhaul: select Longhaul version 2 for capable CPUs


> On Sat, Oct 24, 2009 at 05:25:38PM +0200, Krzysztof Helt wrote:
> > From: Krzysztof Helt <[email protected]>
> >
> > There is a typo in the longhaul detection code so only Longhaul v1 or Longhaul v3
> > is selected. The Longhaul v2 is not selected even for CPUs which are capable of.
> >
> > Tested on PCChips Giga Pro board. Frequency changes work and the Longhaul v2
> > detects that the board is not capable of changing CPU voltage.
>
Most likely processor isn't capable of changing voltage. It is hardcoded
in most processors.
> >
> > Signed-off-by: Krzysztof Helt <[email protected]>
>
> It seems we deliberately changed this two years ago, though the changelog
> is a bit sparse on details..
>
> commit 07844252ffd81ec192a62014bada1016c9703765
> Author: Rafal Bilski <[email protected]>
> Date: Sun Apr 22 12:26:04 2007 +0200
>
> [CPUFREQ] Longhaul - Revert Longhaul ver. 2
>
> There is something wrong with this code. It needs more
> testing. It is better to disable it for now because support
> for some machines will be broken.
>
> Signed-off-by: Rafal Bilski <[email protected]>
> Signed-off-by: Dave Jones <[email protected]>
>
>
> In hindsight, changing it to report V1 instead of V2 was the wrong thing
> to do, and we should have done something like -ENODEV with
> a printk explaining why.
>
> I've not got any old VIA CPUs/boards to test with any more, but I'm inclined
> to apply your change, but we'll have to keep an eye out for any strange
> bugs on affected systems. Currently this driver should do nothing, as the
> longhaul v1 registers that don't exist on longhaul V2 CPUs.
Yes this register do exists on ver. 2 CPU's. It was removed later from
"powersaver" line of
processors.
> With this change,
> we're going to be actually doing scaling again, which may introduce instability
> on some machines, as iirc, we never did get this driver 100% stable due
> to a lot of really crappy motherboards.
>
> Perhaps we should printk a warning related to this.
> (We should definitly mention it in the Kconfig too, which I thought we already had)
>
> Dave
It's my fault. Commit 52a2638bff063acb28ba3355891c49cc240cc98b:
> Longhaul: add auto enabled "revid_errata" option
>
> VIA C3 Ezra-T has RevisionID equal to 1, but it needs RevisionKey to be 0
> or CPU will ignore new frequency and will continue to work at old
> frequency. New "revid_errata" option will force RevisionKey to be set to
> 0, whatever RevisionID is.
>
> Additionaly "Longhaul" will not silently ignore unsuccessful transition.
> It will try to check if "revid_errata" or "disable_acpi_c3" options need to
> be enabled for this processor/system.
>
> Same for Longhaul ver. 2 support. It will be disabled if none of above
> options will work.
It was permanent fix for badly programmed ver. 2 registers. What this
commit
failed to do is to restore ver. 2 support. Probably I used wrong branch
to create it.
One with voltage scaling enabled.
> Best case scenario (with patch apllied and v2 enabled):
> longhaul: VIA C3 'Ezra' [C5C] CPU detected. Longhaul v2 supported.
> longhaul: Using northbridge support.
> longhaul: VRM 8.5
> longhaul: Max VID=1.350 Min VID=1.050, 13 possible voltage scales
> longhaul: f: 300000 kHz, index: 0, vid: 1050 mV
> [...]
> longhaul: Voltage scaling enabled.
> Worst case scenario:
> longhaul: VIA C3 'Ezra-T' [C5M] CPU detected. Powersaver supported.
> longhaul: Using northbridge support.
> longhaul: Using ACPI support.
> longhaul: VRM 8.5
> longhaul: Claims to support voltage scaling but min & max are both 1.250. Voltage scaling disabled
> longhaul: Failed to set requested frequency!
> longhaul: Enabling "Ignore Revision ID" option.
> longhaul: Failed to set requested frequency!
> longhaul: Disabling ACPI C3 support.
> longhaul: Disabling "Ignore Revision ID" option.
> longhaul: Failed to set requested frequency!
> longhaul: Enabling "Ignore Revision ID" option.
Sorry.
Rafał Bilski


----------------------------------------------------------------------
Bezp?atne konto i limit do 100 tys. Otwierasz?
>> http://link.interia.pl/f23bb

2009-10-25 18:28:19

by Krzysztof Helt

[permalink] [raw]
Subject: [PATCH] powernow-k6: set transition latency value so ondemand governor can be used

From: Krzysztof Helt <[email protected]>

Set the transition latency to value smaller than CPUFREQ_ETERNAL so
governors other than "performance" work (like the "ondemand" one).

The value is found in "AMD PowerNow! Technology Platform Design Guide for
Embedded Processors" dated December 2000 (AMD doc #24267A). There is the
answer to one of FAQs on page 40 which states that suggested complete transition
period is 200 us.

Tested on K6-2+ CPU with K6-3 core (model 13, stepping 4).

Signed-off-by: Krzysztof Helt <[email protected]>
---


diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k6.c b/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
index f10dea4..afc6a27 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
@@ -164,7 +164,7 @@ static int powernow_k6_cpu_init(struct cpufreq_policy *policy)
}

/* cpuinfo and default policy values */
- policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
+ policy->cpuinfo.transition_latency = 200000;
policy->cur = busfreq * max_multiplier;

result = cpufreq_frequency_table_cpuinfo(policy, clock_ratio);

----------------------------------------------------------------------
Szukasz pracy? Chcesz lepiej zarabia??
Sprawd? oferty na http://link.interia.pl/f23ba

2009-10-26 06:41:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] powernow-k6: set transition latency value so ondemand governor can be used


* Krzysztof Helt <[email protected]> wrote:

> From: Krzysztof Helt <[email protected]>
>
> Set the transition latency to value smaller than CPUFREQ_ETERNAL so
> governors other than "performance" work (like the "ondemand" one).
>
> The value is found in "AMD PowerNow! Technology Platform Design Guide for
> Embedded Processors" dated December 2000 (AMD doc #24267A). There is the
> answer to one of FAQs on page 40 which states that suggested complete transition
> period is 200 us.

Btw., would be nice to put this paragraph between '/* .. */' signs and
put it into the source code - seems highly relevant when someone comes
across that wonders what the magic number '200000' is.

Ingo

2009-10-31 09:45:06

by Krzysztof Helt

[permalink] [raw]
Subject: [PATCH] powernow-k6: set transition latency value so ondemand governor can be used

From: Krzysztof Helt <[email protected]>

Set the transition latency to value smaller than CPUFREQ_ETERNAL so
governors other than "performance" work (like the "ondemand" one).

The value is found in "AMD PowerNow! Technology Platform Design Guide for
Embedded Processors" dated December 2000 (AMD doc #24267A). There is the
answer to one of FAQs on page 40 which states that suggested complete
transition period is 200 us.

Tested on K6-2+ CPU with K6-3 core (model 13, stepping 4).

Signed-off-by: Krzysztof Helt <[email protected]>
---
This is the second version of the patch with added comment into the source code.

arch/x86/kernel/cpu/cpufreq/powernow-k6.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k6.c b/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
index f10dea4..15f84a6 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k6.c
@@ -163,8 +163,15 @@ static int powernow_k6_cpu_init(struct cpufreq_policy *policy)
clock_ratio[i].frequency = busfreq * f;
}

- /* cpuinfo and default policy values */
- policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
+ /*
+ * cpuinfo and default policy values
+ *
+ * The value is found in "AMD PowerNow! Technology Platform
+ * Design Guide for Embedded Processors" dated December 2000
+ * (AMD doc #24267A). There is the answer to one of FAQs on page 40
+ * which states that suggested complete transition period is 200 us.
+ */
+ policy->cpuinfo.transition_latency = 200000;
policy->cur = busfreq * max_multiplier;

result = cpufreq_frequency_table_cpuinfo(policy, clock_ratio);
--
1.6.4

2009-11-03 19:33:30

by Krzysztof Helt

[permalink] [raw]
Subject: Re: [PATCH] longhaul: select Longhaul version 2 for capable CPUs

On Sat, 24 Oct 2009 23:28:14 -0400
Dave Jones <[email protected]> wrote:

> On Sat, Oct 24, 2009 at 05:25:38PM +0200, Krzysztof Helt wrote:
> > From: Krzysztof Helt <[email protected]>
> >
> > There is a typo in the longhaul detection code so only Longhaul v1 or Longhaul v3
> > is selected. The Longhaul v2 is not selected even for CPUs which are capable of.
> >
> > Tested on PCChips Giga Pro board. Frequency changes work and the Longhaul v2
> > detects that the board is not capable of changing CPU voltage.
> >
> > Signed-off-by: Krzysztof Helt <[email protected]>
>
> It seems we deliberately changed this two years ago, though the changelog
> is a bit sparse on details..
>
> commit 07844252ffd81ec192a62014bada1016c9703765
> Author: Rafal Bilski <[email protected]>
> Date: Sun Apr 22 12:26:04 2007 +0200
>
> [CPUFREQ] Longhaul - Revert Longhaul ver. 2
>
> There is something wrong with this code. It needs more
> testing. It is better to disable it for now because support
> for some machines will be broken.
>
> Signed-off-by: Rafal Bilski <[email protected]>
> Signed-off-by: Dave Jones <[email protected]>
>
>

Are there any chances this patch will go into the linux-next?

Regards,
Krzysztof

------------------------------------------------------------------------------
Strrraszny konkurs z Scooby-Doo!
Wez udzial >> http://link.interia.pl/f2412