[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

This patch is for AMD boards which can do the P state generation. This just
removes the ugly binary DSDT patching and all other related stuff. Stick to
infrastructure in previous patch.

This was just compile tested. I think it should work. Please test.

Signed-off-by: Rudolf Marek

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmWxJ0ACgkQ3J9wPJqZRNWqygCeKf09gJbRIBWn4we3XKd/3k3T
mMMAoOBs7K8q4Xuc4YqnUu3QZF7yHwwr
=jbxj
-----END PGP SIGNATURE-----

[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Paul Menzel]

Dear Marek,

Am Samstag, den 14.02.2009, 14:18 +0100 schrieb Rudolf Marek:
> This patch is for AMD boards which can do the P state generation. This just
> removes the ugly binary DSDT patching and all other related stuff. Stick to
> infrastructure in previous patch.
>
> This was just compile tested. I think it should work. Please test.
>
> Signed-off-by: Rudolf Marek

Is there a file (cpu/amd/model_fxx_powernow.h) missing in the patch?

Thanks,

Paul

[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Paul Menzel]

Dear everyone,

Am Samstag, den 14.02.2009, 19:01 +0100 schrieb Paul Menzel:
> Am Samstag, den 14.02.2009, 14:18 +0100 schrieb Rudolf Marek:
> > This patch is for AMD boards which can do the P state generation. This just
> > removes the ugly binary DSDT patching and all other related stuff. Stick to
> > infrastructure in previous patch.
> >
> > This was just compile tested. I think it should work. Please test.
> >
> > Signed-off-by: Rudolf Marek
>
> Is there a file (cpu/amd/model_fxx_powernow.h) missing in the patch?

If it says [PATCH 2/2], I should look at 1/2 first. Sorry for the noise.

Thanks,

Paul

[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Carl-Daniel Hailfinger]

On 14.02.2009 14:18, Rudolf Marek wrote:
> Hello,
>
> This patch is for AMD boards which can do the P state generation. This
> just
> removes the ugly binary DSDT patching and all other related stuff.
> Stick to
> infrastructure in previous patch.
>
> This was just compile tested. I think it should work. Please test.
>
> Signed-off-by: Rudolf Marek
>
> Rudolf

The following differences exist (shortened):
--- old
+++ ruik
-    Scope (_PR)
+    Scope (_PR)
     {
-        Processor (CPU0, 0x00, 0x00000808, 0x06)
+        Processor (_PR.CPU0, 0x00, 0x00000808, 0x06)
         {
             Name (_PCT, Package (0x02)
             {

And your code shows one processor, the original code shows four
processors (I have one single-core CPU).

It seems that your code also fixes the message "powernow-k8: ph2 null
fid transition 0xa", but I am not sure whether this is an artifact of my
testing setup.

I tested switching from maximum powersave to maximum performance in a
tight loop (0.1 seconds delay between state changes) and it survived 262
changes in each direction. After that, I stopped the test because it
seemed to work.

If you are sure that your code can handle multicore/multiprocessor
constellations and if you can explain that additional backslash before
_PR, the patch is

Acked-by: Carl-Daniel Hailfinger

Attached are acpidumps and bootlogs without and with your patch.

Regards,
Carl-Daniel

[PATCH 2/2] AMD boards to use new PowerNow infrastructure

[Myles Watson]

> The following differences exist (shortened):
> --- old
> +++ ruik
> Scope (_PR)
> Scope (_PR)
> {
>    Processor (CPU0, 0x00, 0x00000808, 0x06)
>    Processor (\_PR.CPU0, 0x00, 0x00000808, 0x06)
>    {
>        Name (_PCT, Package (0x02)
>        {

...
> If you are sure that your code can handle multicore/multiprocessor
> constellations and if you can explain that additional backslash before
> _PR, the patch is

My understanding is that the backslash makes the scoping explicit.  It puts
it in the "root".  So as long as _PR was not inside another scope, _PR and
_PR should be equivalent.  Similarly, _PR.CPU0 is the same as CPU0 defined
in _PR.

The scoping seems to make a difference in the size of the generated AML
code, but besides that shouldn't matter too much.

Thanks,
Myles

[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Rudolf Marek]

Hi,

The scope question was answered by Myles already.
> And your code shows one processor, the original code shows four
> processors (I have one single-core CPU).

I choose to generate the code just for cores which exists.
> It seems that your code also fixes the message "powernow-k8: ph2 null
> fid transition 0xa", but I am not sure whether this is an artifact of my
> testing setup.

It imho says that it is already there. Quite harmless messages.
> I tested switching from maximum powersave to maximum performance in a
> tight loop (0.1 seconds delay between state changes) and it survived 262
> changes in each direction. After that, I stopped the test because it
> seemed to work.

Well this change just tables not the chipset setup, but it is fine that it works.
> If you are sure that your code can handle multicore/multiprocessor
> constellations and if you can explain that additional backslash before
> _PR, the patch is
>
> Acked-by: Carl-Daniel Hailfinger

Committed revision 3955.

Ok thanks,

Rudolf
> Attached are acpidumps and bootlogs without and with your patch.
>
> Regards,
> Carl-Daniel

[Myles Watson]

On Thu, Feb 19, 2009 at 1:39 AM, Rudolf Marek  wrote:
> > And your code shows one processor, the original code shows four
> > processors (I have one single-core CPU).
>
> I choose to generate the code just for cores which exists.

I have two dual-core CPUs, and code is only generated for two of them.
 I see the for loop with write_pstates_for_core in powernow_acpi.c.
Should there be another loop for packages?  I guess I'm not sure which
two cores are being found by your algorithm.

Thanks,
Myles

[Rudolf Marek]

Myles Watson napsal(a):
> On Thu, Feb 19, 2009 at 1:39 AM, Rudolf Marek  wrote:
> > > And your code shows one processor, the original code shows four
> > > processors (I have one single-core CPU).
> >
> > I choose to generate the code just for cores which exists.
>
> I have two dual-core CPUs, and code is only generated for two of them.
>  I see the for loop with write_pstates_for_core in powernow_acpi.c.
> Should there be another loop for packages?

Yes but it needs to read new data from the AP CPU (the MSRs and CPUIDs)
Dont know if we have funcs like cpuid_on_cpu and msr_on_cpu.
> I guess I'm not sure which
> two cores are being found by your algorithm.

Just BSP processor. It needs to be thought how to do that for more CPUs, perhaps 
using funcs like above.

Rudolf

[Myles Watson]

On Mon, Mar 2, 2009 at 3:50 PM, Rudolf Marek  wrote:
> Myles Watson napsal(a):
> > On Thu, Feb 19, 2009 at 1:39 AM, Rudolf Marek 
> > wrote:
> > > > And your code shows one processor, the original code shows four
> > > > processors (I have one single-core CPU).
> > >
> > > I choose to generate the code just for cores which exists.
> >
> > I have two dual-core CPUs, and code is only generated for two of them.
> >  I see the for loop with write_pstates_for_core in powernow_acpi.c.
> > Should there be another loop for packages?
>
> Yes but it needs to read new data from the AP CPU (the MSRs and CPUIDs)
> Dont know if we have funcs like cpuid_on_cpu and msr_on_cpu.
> > I guess I'm not sure which
> > two cores are being found by your algorithm.
>
> Just BSP processor. It needs to be thought how to do that for more CPUs,
> perhaps using funcs like above.

I guess the alternative is to force users to use matched CPUs.  Do we
already force that for microcode patches?

Thanks,
Myles

[PATCH 2/2] AMD boards to use new Power Now infrastructure

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> I guess the alternative is to force users to use matched CPUs.  Do we
> already force that for microcode patches?

Dont know haven't looked into this yet. For now you could invoke the routines 2
times more I guess.

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkm1fhIACgkQ3J9wPJqZRNVf7ACfb+e964upDVGt1WZLbSNG/mhz
bnwAoI0dSPDxviZb/zeXyprDt5Xn5P1a
=13lA
-----END PGP SIGNATURE-----

[Myles Watson]

On Mon, Mar 9, 2009 at 1:37 PM, Rudolf Marek  wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> > I guess the alternative is to force users to use matched CPUs.  Do we
> > already force that for microcode patches?
>
> Dont know haven't looked into this yet. For now you could invoke the routines 2
> times more I guess.

Yes.  For now that's what I'm doing.

Thanks,
Myles