AMD DBM690T PowerNow table problems

[Carl-Daniel Hailfinger]

Hi,

I get this message during boot on my socket AM2 system (Asus M2A-VM)
with the AMD DBM690T target:

powernow-k8: Found 1 AMD Athlon(tm) 64 Processor 3000+ processors (1 cpu
cores) (version
2.20.00)

powernow-k8: invalid freq entries 1800000 kHz vs. 800000
kHz

powernow-k8: invalid freq entries 1000000 kHz vs. 800000
kHz

powernow-k8: invalid
powernow_table

(CPUFreq not
supported)

The following files are attached:
- coreboot acpidump
- SSDT from proprietary BIOS
- _PSS created by genpowernow.c (from Rudolf) for my CPU

I appreciate any insights.

[Rudolf Marek]

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

Hi,

Here we go.

Its seems that the generated table has wrong conversion for frequency and its
FID representation. In other words the table is OK but the first field is not
consistent with FID and real freq.

invalid freq entries 1000000 kHz vs. 800000

This means that:

Package (0x06)
                {
                    0x00000320,. <--- here should be 0x3e8
                    0x0000F230,.
                    0x00000064,.
                    0x00000007,.
                    0xE8202F0A,.
                    0x0000030A
                },.

Package (0x06)
                {
                    0x00000320,.
                    0x0000BB8C,.
                    0x00000064,.
                    0x00000007,.
                    0xE8202C82,.
                    0x00000482
                }

FID is 0A and 02 which means it should be: set to 0x3e8 and 0x320.

I guess it is because your CPU is revF and not revG.

fid_multiplier = ((cpuid1.edx & 0x40) >> 6) * 100;

This line will cause that your CPU has fid_multiplier 0 instead of 100x.
I believe that the multiplier should be always 100. Because revF CPU hav LSB in
FID always 0.

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

iEYEARECAAYFAklf4OMACgkQ3J9wPJqZRNUYlgCcDpdFgUSR/9niTmZscuIxSV96
fBwAn1eweTn8g0YuHfRfJU6xex2tPIE+
=gOpD
-----END PGP SIGNATURE-----

[Rudolf Marek]

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

Whoops sorry the 1000 Mhz should be for second entry. First should have 0x708
for 1800Mhz.

It just states 800Mhz (0x320) because the formula is:

frq = FID * 0 + 800

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

iEYEARECAAYFAklf4mwACgkQ3J9wPJqZRNWqggCfR7Y9l0gP83qARvVNnx8SIBRY
g2UAoKNT6XJVV/w3khB4hyV0PbM405l4
=jG9b
-----END PGP SIGNATURE-----

[Marc Jones]

On Sat, Jan 3, 2009 at 3:04 PM, Rudolf Marek  wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> Here we go.
>
> Its seems that the generated table has wrong conversion for frequency and its
> FID representation. In other words the table is OK but the first field is not
> consistent with FID and real freq.
>
> invalid freq entries 1000000 kHz vs. 800000
>
> This means that:
>            Package (0x06)
>            {
>                0x00000320,. <--- here should be 0x3e8
>                0x0000F230,.
>                0x00000064,.
>                0x00000007,.
>                0xE8202F0A,.
>                0x0000030A
>            },.
>
>            Package (0x06)
>            {
>                0x00000320,.
>                0x0000BB8C,.
>                0x00000064,.
>                0x00000007,.
>                0xE8202C82,.
>                0x00000482
>            }
>
>
> FID is 0A and 02 which means it should be: set to 0x3e8 and 0x320.
>
> I guess it is because your CPU is revF and not revG.
>
> fid_multiplier = ((cpuid1.edx & 0x40) >> 6) * 100;
>
> This line will cause that your CPU has fid_multiplier 0 instead of 100x.
> I believe that the multiplier should be always 100. Because revF CPU hav LSB in
> FID always 0.
>
> Rudolf

I agree with this analysis. Patch attached.

Marc

[Bao, Zheng]

If fid_multiplier is 0, it doesn't seem to have any sense. So I agree.

Acked-by: zheng bao

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Marc Jones
Sent: Tuesday, January 06, 2009 9:26 AM
To: Rudolf Marek; Bao, Zheng
Cc: Carl-Daniel Hailfinger; Coreboot
Subject: Re: [coreboot] AMD DBM690T PowerNow table problems

On Sat, Jan 3, 2009 at 3:04 PM, Rudolf Marek 
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> Here we go.
>
> Its seems that the generated table has wrong conversion for frequency

and its
> FID representation. In other words the table is OK but the first field

is not
> consistent with FID and real freq.
>
> invalid freq entries 1000000 kHz vs. 800000
>
> This means that:
>            Package (0x06)
>            {
>                0x00000320,. <--- here should be 0x3e8
>                0x0000F230,.
>                0x00000064,.
>                0x00000007,.
>                0xE8202F0A,.
>                0x0000030A
>            },.
>
>            Package (0x06)
>            {
>                0x00000320,.
>                0x0000BB8C,.
>                0x00000064,.
>                0x00000007,.
>                0xE8202C82,.
>                0x00000482
>            }
>
>
> FID is 0A and 02 which means it should be: set to 0x3e8 and 0x320.
>
> I guess it is because your CPU is revF and not revG.
>
> fid_multiplier = ((cpuid1.edx & 0x40) >> 6) * 100;
>
> This line will cause that your CPU has fid_multiplier 0 instead of

100x.
> I believe that the multiplier should be always 100. Because revF CPU

hav LSB in
> FID always 0.
>
> Rudolf

I agree with this analysis. Patch attached.

Marc

[Marc Jones]

On Mon, Jan 5, 2009 at 6:36 PM, Bao, Zheng  wrote:
> If fid_multiplier is 0, it doesn't seem to have any sense. So I agree.
>
> Acked-by: zheng bao

I also fixed this in acpi_tables.c Pistachio.

r 3847

Marc

[Carl-Daniel Hailfinger]

On 06.01.2009 17:47, Marc Jones wrote:
> On Mon, Jan 5, 2009 at 6:36 PM, Bao, Zheng  wrote:
> > If fid_multiplier is 0, it doesn't seem to have any sense. So I agree.
> >
> > Acked-by: zheng bao
>
> I also fixed this in acpi_tables.c Pistachio.
>
> r 3847

Thanks!
The Linux kernel powernow-k8 driver doesn't complain anymore, but now
the machine will lock up or reboot once I put some load on it.
Blacklisting the powernow-k8 driver avoids the issue, but that makes
frequency scaling impossible.

AFAICS the standard cpu frequency governor is ondemand and it scales the
frequency down from 1800 MHz to 1000 Mhz. That seems to work fine, but
scaling up again will cause the lockups/reboots. By the way, coreboot
reports a BIST failure after such reboots. Not nice.

Rudolf's PowerNow tables had entries for 1000/1600/1800 MHz. The
proprietary BIOS and coreboot only have entries for 1000/1800 MHz.
According to the docs, scaling from 1000 MHz to 1800 MHz can't be done
in a single step. Is that maybe the reason for the problems I'm seeing?

Regards,
Carl-Daniel

[Marc Jones]

On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
 wrote:
> On 06.01.2009 17:47, Marc Jones wrote:
> > On Mon, Jan 5, 2009 at 6:36 PM, Bao, Zheng  wrote:
> > > If fid_multiplier is 0, it doesn't seem to have any sense. So I agree.
> > >
> > > Acked-by: zheng bao
> >
> > I also fixed this in acpi_tables.c Pistachio.
> >
> > r 3847
>
> Thanks!
> The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> the machine will lock up or reboot once I put some load on it.
> Blacklisting the powernow-k8 driver avoids the issue, but that makes
> frequency scaling impossible.
>
> AFAICS the standard cpu frequency governor is ondemand and it scales the
> frequency down from 1800 MHz to 1000 Mhz. That seems to work fine, but
> scaling up again will cause the lockups/reboots. By the way, coreboot
> reports a BIST failure after such reboots. Not nice.
>
> Rudolf's PowerNow tables had entries for 1000/1600/1800 MHz. The
> proprietary BIOS and coreboot only have entries for 1000/1800 MHz.
> According to the docs, scaling from 1000 MHz to 1800 MHz can't be done
> in a single step. Is that maybe the reason for the problems I'm seeing?

Can you dump the current tables or can you not get that far before a
crash? The entries are end points and the driver takes care of the
stepping that needs to be done to get to those points. We need to
check that the table generated the correct vid setting for the fid. It
is well described in the bkdg section 10.6.

Marc

[Carl-Daniel Hailfinger]

On 08.01.2009 17:52, Marc Jones wrote:
> On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
>  wrote:
> > On 06.01.2009 17:47, Marc Jones wrote:
> > > On Mon, Jan 5, 2009 at 6:36 PM, Bao, Zheng  wrote:
> > > > If fid_multiplier is 0, it doesn't seem to have any sense. So I agree.
> > > >
> > > > Acked-by: zheng bao
> > >
> > > I also fixed this in acpi_tables.c Pistachio.
> > >
> > > r 3847
> >
> > Thanks!
> > The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> > the machine will lock up or reboot once I put some load on it.
> > Blacklisting the powernow-k8 driver avoids the issue, but that makes
> > frequency scaling impossible.
> >
> > AFAICS the standard cpu frequency governor is ondemand and it scales the
> > frequency down from 1800 MHz to 1000 Mhz. That seems to work fine, but
> > scaling up again will cause the lockups/reboots. By the way, coreboot
> > reports a BIST failure after such reboots. Not nice.
> >
> > Rudolf's PowerNow tables had entries for 1000/1600/1800 MHz. The
> > proprietary BIOS and coreboot only have entries for 1000/1800 MHz.
> > According to the docs, scaling from 1000 MHz to 1800 MHz can't be done
> > in a single step. Is that maybe the reason for the problems I'm seeing?
>
> Can you dump the current tables or can you not get that far before a
> crash? The entries are end points and the driver takes care of the
> stepping that needs to be done to get to those points. We need to
> check that the table generated the correct vid setting for the fid. It
> is well described in the bkdg section 10.6.

I managed to boot fine with the proprietary BIOS, blacklisted the
k8-powernow driver and rebooted with coreboot.

Attached are the following files:
_PSS with coreboot (crash)
_PSS with proprietary BIOS (ok)

If I combine the _PSS from the proprietary BIOS with the other tables
from coreboot, the machine does not crash. I'll try to narrow it down
further.

Regards,
Carl-Daniel

[Paul Menzel]

Dear Carl-Daniel,

there were no attachments in the message I received.

Thanks,

Paul

[Carl-Daniel Hailfinger]

On 09.01.2009 10:05, Carl-Daniel Hailfinger wrote:
> On 08.01.2009 17:52, Marc Jones wrote:
> > On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
> >  wrote:
> > > The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> > > the machine will lock up or reboot once I put some load on it.
> > > Blacklisting the powernow-k8 driver avoids the issue, but that makes
> > > frequency scaling impossible.
> >
> > Can you dump the current tables or can you not get that far before a
> > crash? The entries are end points and the driver takes care of the
> > stepping that needs to be done to get to those points. We need to
> > check that the table generated the correct vid setting for the fid. It
> > is well described in the bkdg section 10.6.
>
> I managed to boot fine with the proprietary BIOS, blacklisted the
> k8-powernow driver and rebooted with coreboot.
>
> If I combine the _PSS from the proprietary BIOS with the other tables
> from coreboot, the machine does not crash. I'll try to narrow it down
> further.

I finally found the bug.

The DBM90T code sets bit 10 in _PSS as part of the control value, but
bit 10 is part of NewVID. That means the resulting VID is wrong and
causes the processor to crash.
The Pistachio code has the same bug.

This patch fixes the wrong setting and changes control from a magic and
incorrect unexplained value (0xE8202C00) to a combination of explained
values and shifts which has the right value (0xE8202800).

It is tested on my machine and it survived 200 changes from minimum to
maximum frequency every 100 ms under heavy load and under no load.

In the long term we want to consolidate all AMD FIDVID code into one
generic library file.

Signed-off-by: Carl-Daniel Hailfinger

--- src/mainboard/amd/pistachio/acpi_tables.c	(Revision 3850)
+++ src/mainboard/amd/pistachio/acpi_tables.c	(Arbeitskopie)
@@ -400,10 +400,10 @@
    	Pstate_num++;
    }

-	/* Print Pstate feq,vid,volt,power */
+	/* Print Pstate freq,vid,volt,power */

for (index = 0; index < Pstate_num; index++) {
-		printk_info("Pstate_feq[%d] = %dMHz\t", index,
+		printk_info("Pstate_freq[%d] = %dMHz\t", index,
    		    Pstate_feq[index]);
    	printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
    	printk_info("Pstate_volt[%d] = %dmv\t", index,
@@ -414,8 +414,11 @@

/*
     * Modify the DSDT Table to put the actural _PSS package
-	 * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
-	 * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
+	 * corefeq-->Pstate_feq[index]
+	 * power-->Pstate_power[index]
+	 * transitionlatency-->0x64
+	 * busmasterlatency-->0x7,
+	 * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
     * status --> Pstate_vid[index]<<6 | Pstate_fid[index]
     * Get the _PSS control method Sig.
     */
@@ -461,7 +464,13 @@
    			transitionlatency = 0x64;
    			busmasterlatency = 0x7;
    			control =
-				    0xE8202C00 | (Pstate_vid[index] << 6) |
+				    (0x3 << 30) | /* IRT */
+				    (0x2 << 28) | /* RVO */
+				    (0x1 << 27) | /* ExtType */
+				    (0x2 << 20) | /* PLL_LOCK_TIME */
+				    (0x0 << 18) | /* MVS */
+				    (0x5 << 11) | /* VST */
+				    (Pstate_vid[index] << 6) |
    			    Pstate_fid[index];
    			status =
    			    (Pstate_vid[index] << 6) |
--- src/mainboard/amd/dbm690t/acpi_tables.c	(Revision 3850)
+++ src/mainboard/amd/dbm690t/acpi_tables.c	(Arbeitskopie)
@@ -260,7 +260,7 @@

/* Get the multipier of the fid frequency */
    /*
-	 * Fid multiplier is always 100 revF and revG.
+	 * Fid multiplier is always 100 revF and revG.
     */
    fid_multiplier = 100;

@@ -400,10 +400,10 @@
    	Pstate_num++;
    }

-	/* Print Pstate feq,vid,volt,power */
+	/* Print Pstate freq,vid,volt,power */

for (index = 0; index < Pstate_num; index++) {
-		printk_info("Pstate_feq[%d] = %dMHz\t", index,
+		printk_info("Pstate_freq[%d] = %dMHz\t", index,
    		    Pstate_feq[index]);
    	printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
    	printk_info("Pstate_volt[%d] = %dmv\t", index,
@@ -414,8 +414,11 @@

/*
     * Modify the DSDT Table to put the actural _PSS package
-	 * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
-	 * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
+	 * corefeq-->Pstate_feq[index]
+	 * power-->Pstate_power[index]
+	 * transitionlatency-->0x64
+	 * busmasterlatency-->0x7,
+	 * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
     * status --> Pstate_vid[index]<<6 | Pstate_fid[index]
     * Get the _PSS control method Sig.
     */
@@ -461,7 +464,13 @@
    			transitionlatency = 0x64;
    			busmasterlatency = 0x7;
    			control =
-				    0xE8202C00 | (Pstate_vid[index] << 6) |
+				    (0x3 << 30) | /* IRT */
+				    (0x2 << 28) | /* RVO */
+				    (0x1 << 27) | /* ExtType */
+				    (0x2 << 20) | /* PLL_LOCK_TIME */
+				    (0x0 << 18) | /* MVS */
+				    (0x5 << 11) | /* VST */
+				    (Pstate_vid[index] << 6) |
    			    Pstate_fid[index];
    			status =
    			    (Pstate_vid[index] << 6) |

[Li, Maggie]

Hi

I have tested it on my DBM690T board. It is ok.

Best regards
Maggie li

-----Original Message-----
From: coreboot-bounces+maggie.li=[e-mail] [mailto:coreboot-bounces+maggie.li=[e-mail]] On Behalf Of Carl-Daniel Hailfinger
Sent: Thursday, January 15, 2009 12:35 PM
To: Marc Jones
Cc: Rudolf Marek; Bao, Zheng; Coreboot
Subject: Re: [coreboot] AMD DBM690T PowerNow table problems

On 09.01.2009 10:05, Carl-Daniel Hailfinger wrote:
> On 08.01.2009 17:52, Marc Jones wrote:
> > On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
> >  wrote:
> > > The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> > > the machine will lock up or reboot once I put some load on it.
> > > Blacklisting the powernow-k8 driver avoids the issue, but that makes
> > > frequency scaling impossible.
> >
> > Can you dump the current tables or can you not get that far before a
> > crash? The entries are end points and the driver takes care of the
> > stepping that needs to be done to get to those points. We need to
> > check that the table generated the correct vid setting for the fid. It
> > is well described in the bkdg section 10.6.
>
> I managed to boot fine with the proprietary BIOS, blacklisted the
> k8-powernow driver and rebooted with coreboot.
>
> If I combine the _PSS from the proprietary BIOS with the other tables
> from coreboot, the machine does not crash. I'll try to narrow it down
> further.

I finally found the bug.

The DBM90T code sets bit 10 in _PSS as part of the control value, but
bit 10 is part of NewVID. That means the resulting VID is wrong and
causes the processor to crash.
The Pistachio code has the same bug.

This patch fixes the wrong setting and changes control from a magic and
incorrect unexplained value (0xE8202C00) to a combination of explained
values and shifts which has the right value (0xE8202800).

It is tested on my machine and it survived 200 changes from minimum to
maximum frequency every 100 ms under heavy load and under no load.

In the long term we want to consolidate all AMD FIDVID code into one
generic library file.

Signed-off-by: Carl-Daniel Hailfinger

--- src/mainboard/amd/pistachio/acpi_tables.c	(Revision 3850)
+++ src/mainboard/amd/pistachio/acpi_tables.c	(Arbeitskopie)
@@ -400,10 +400,10 @@
    	Pstate_num++;
    }

-	/* Print Pstate feq,vid,volt,power */
+	/* Print Pstate freq,vid,volt,power */

for (index = 0; index < Pstate_num; index++) {
-		printk_info("Pstate_feq[%d] = %dMHz\t", index,
+		printk_info("Pstate_freq[%d] = %dMHz\t", index,
    		    Pstate_feq[index]);
    	printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
    	printk_info("Pstate_volt[%d] = %dmv\t", index,
@@ -414,8 +414,11 @@

/*
     * Modify the DSDT Table to put the actural _PSS package
-	 * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
-	 * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
+	 * corefeq-->Pstate_feq[index]
+	 * power-->Pstate_power[index]
+	 * transitionlatency-->0x64
+	 * busmasterlatency-->0x7,
+	 * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
     * status --> Pstate_vid[index]<<6 | Pstate_fid[index]
     * Get the _PSS control method Sig.
     */
@@ -461,7 +464,13 @@
    			transitionlatency = 0x64;
    			busmasterlatency = 0x7;
    			control =
-				    0xE8202C00 | (Pstate_vid[index] << 6) |
+				    (0x3 << 30) | /* IRT */
+				    (0x2 << 28) | /* RVO */
+				    (0x1 << 27) | /* ExtType */
+				    (0x2 << 20) | /* PLL_LOCK_TIME */
+				    (0x0 << 18) | /* MVS */
+				    (0x5 << 11) | /* VST */
+				    (Pstate_vid[index] << 6) |
    			    Pstate_fid[index];
    			status =
    			    (Pstate_vid[index] << 6) |
--- src/mainboard/amd/dbm690t/acpi_tables.c	(Revision 3850)
+++ src/mainboard/amd/dbm690t/acpi_tables.c	(Arbeitskopie)
@@ -260,7 +260,7 @@

/* Get the multipier of the fid frequency */
    /*
-	 * Fid multiplier is always 100 revF and revG.
+	 * Fid multiplier is always 100 revF and revG.
     */
    fid_multiplier = 100;

@@ -400,10 +400,10 @@
    	Pstate_num++;
    }

-	/* Print Pstate feq,vid,volt,power */
+	/* Print Pstate freq,vid,volt,power */

for (index = 0; index < Pstate_num; index++) {
-		printk_info("Pstate_feq[%d] = %dMHz\t", index,
+		printk_info("Pstate_freq[%d] = %dMHz\t", index,
    		    Pstate_feq[index]);
    	printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
    	printk_info("Pstate_volt[%d] = %dmv\t", index,
@@ -414,8 +414,11 @@

/*
     * Modify the DSDT Table to put the actural _PSS package
-	 * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
-	 * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
+	 * corefeq-->Pstate_feq[index]
+	 * power-->Pstate_power[index]
+	 * transitionlatency-->0x64
+	 * busmasterlatency-->0x7,
+	 * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
     * status --> Pstate_vid[index]<<6 | Pstate_fid[index]
     * Get the _PSS control method Sig.
     */
@@ -461,7 +464,13 @@
    			transitionlatency = 0x64;
    			busmasterlatency = 0x7;
    			control =
-				    0xE8202C00 | (Pstate_vid[index] << 6) |
+				    (0x3 << 30) | /* IRT */
+				    (0x2 << 28) | /* RVO */
+				    (0x1 << 27) | /* ExtType */
+				    (0x2 << 20) | /* PLL_LOCK_TIME */
+				    (0x0 << 18) | /* MVS */
+				    (0x5 << 11) | /* VST */
+				    (Pstate_vid[index] << 6) |
    			    Pstate_fid[index];
    			status =
    			    (Pstate_vid[index] << 6) |

[Carl-Daniel Hailfinger]

Hi,

if anyone wants to ack this, I'll commit it.

Regards,
Carl-Daniel

On 15.01.2009 07:42, Li, Maggie wrote:
> Hi
>
> I have tested it on my DBM690T board. It is ok.
>
> Best regards
> Maggie li
>
> -----Original Message-----
> From: coreboot-bounces+maggie.li=[e-mail] [mailto:coreboot-bounces+maggie.li=[e-mail]] On Behalf Of Carl-Daniel Hailfinger
> Sent: Thursday, January 15, 2009 12:35 PM
> To: Marc Jones
> Cc: Rudolf Marek; Bao, Zheng; Coreboot
> Subject: Re: [coreboot] AMD DBM690T PowerNow table problems
>
> On 09.01.2009 10:05, Carl-Daniel Hailfinger wrote:
> > On 08.01.2009 17:52, Marc Jones wrote:
> > > On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
> > >  wrote:
> > > > The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> > > > the machine will lock up or reboot once I put some load on it.
> > > > Blacklisting the powernow-k8 driver avoids the issue, but that makes
> > > > frequency scaling impossible.
> > >
> > > Can you dump the current tables or can you not get that far before a
> > > crash? The entries are end points and the driver takes care of the
> > > stepping that needs to be done to get to those points. We need to
> > > check that the table generated the correct vid setting for the fid. It
> > > is well described in the bkdg section 10.6.
> >
> > I managed to boot fine with the proprietary BIOS, blacklisted the
> > k8-powernow driver and rebooted with coreboot.
> >
> > If I combine the _PSS from the proprietary BIOS with the other tables
> > from coreboot, the machine does not crash. I'll try to narrow it down
> > further.
>
> I finally found the bug.
>
> The DBM90T code sets bit 10 in _PSS as part of the control value, but
> bit 10 is part of NewVID. That means the resulting VID is wrong and
> causes the processor to crash.
> The Pistachio code has the same bug.
>
> This patch fixes the wrong setting and changes control from a magic and
> incorrect unexplained value (0xE8202C00) to a combination of explained
> values and shifts which has the right value (0xE8202800).
>
> It is tested on my machine and it survived 200 changes from minimum to
> maximum frequency every 100 ms under heavy load and under no load.
>
> In the long term we want to consolidate all AMD FIDVID code into one
> generic library file.
>
> Signed-off-by: Carl-Daniel Hailfinger
>
> --- src/mainboard/amd/pistachio/acpi_tables.c	(Revision 3850)
> +++ src/mainboard/amd/pistachio/acpi_tables.c	(Arbeitskopie)
> @@ -400,10 +400,10 @@
>    	Pstate_num++;
>    }
> /* Print Pstate feq,vid,volt,power */
>
> /* Print Pstate freq,vid,volt,power */
>
> for (index = 0; index < Pstate_num; index++) {
> printk_info("Pstate_feq[%d] = %dMHz\t", index,
> printk_info("Pstate_freq[%d] = %dMHz\t", index,
>     Pstate_feq[index]);
> printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
> printk_info("Pstate_volt[%d] = %dmv\t", index,
>
> @@ -414,8 +414,11 @@
>
> /*
>     * Modify the DSDT Table to put the actural _PSS package
> * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
> * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
> * corefeq-->Pstate_feq[index]
> * power-->Pstate_power[index]
> * transitionlatency-->0x64
> * busmasterlatency-->0x7,
> * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
> status --> Pstate_vid[index]<<6 | Pstate_fid[index]
> Get the _PSS control method Sig.
> */
>
> @@ -461,7 +464,13 @@
>    			transitionlatency = 0x64;
>    			busmasterlatency = 0x7;
>    			control =
> 		    0xE8202C00 | (Pstate_vid[index] << 6) |
> 		    (0x3 << 30) | /* IRT */
> 		    (0x2 << 28) | /* RVO */
> 		    (0x1 << 27) | /* ExtType */
> 		    (0x2 << 20) | /* PLL_LOCK_TIME */
> 		    (0x0 << 18) | /* MVS */
> 		    (0x5 << 11) | /* VST */
> 		    (Pstate_vid[index] << 6) |
> 	    Pstate_fid[index];
> 	status =
> 	    (Pstate_vid[index] << 6) |
>
> --- src/mainboard/amd/dbm690t/acpi_tables.c	(Revision 3850)
> +++ src/mainboard/amd/dbm690t/acpi_tables.c	(Arbeitskopie)
> @@ -260,7 +260,7 @@
>
> /* Get the multipier of the fid frequency */
>    /*
> * Fid multiplier is always 100 revF and revG.
> * Fid multiplier is always 100 revF and revG.
> */
>  fid_multiplier = 100;
>
> @@ -400,10 +400,10 @@
>    	Pstate_num++;
>    }
> /* Print Pstate feq,vid,volt,power */
>
> /* Print Pstate freq,vid,volt,power */
>
> for (index = 0; index < Pstate_num; index++) {
> printk_info("Pstate_feq[%d] = %dMHz\t", index,
> printk_info("Pstate_freq[%d] = %dMHz\t", index,
>     Pstate_feq[index]);
> printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
> printk_info("Pstate_volt[%d] = %dmv\t", index,
>
> @@ -414,8 +414,11 @@
>
> /*
>     * Modify the DSDT Table to put the actural _PSS package
> * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
> * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
> * corefeq-->Pstate_feq[index]
> * power-->Pstate_power[index]
> * transitionlatency-->0x64
> * busmasterlatency-->0x7,
> * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
> status --> Pstate_vid[index]<<6 | Pstate_fid[index]
> Get the _PSS control method Sig.
> */
>
> @@ -461,7 +464,13 @@
>    			transitionlatency = 0x64;
>    			busmasterlatency = 0x7;
>    			control =
> 		    0xE8202C00 | (Pstate_vid[index] << 6) |
> 		    (0x3 << 30) | /* IRT */
> 		    (0x2 << 28) | /* RVO */
> 		    (0x1 << 27) | /* ExtType */
> 		    (0x2 << 20) | /* PLL_LOCK_TIME */
> 		    (0x0 << 18) | /* MVS */
> 		    (0x5 << 11) | /* VST */
> 		    (Pstate_vid[index] << 6) |
> 	    Pstate_fid[index];
> 	status =
> 	    (Pstate_vid[index] << 6) |

[Li, Maggie]

Acked-by: Maggie li

Sorry I thought it should be acked by Marc. He knows more about this part than me.

Best regards
Maggie li

-----Original Message-----
From: Carl-Daniel Hailfinger [mailto:[e-mail]] 
Sent: Friday, January 16, 2009 1:32 PM
To: Li, Maggie
Cc: Marc Jones; Rudolf Marek; Bao, Zheng; Coreboot
Subject: Re: [coreboot] AMD DBM690T PowerNow table problems

Hi,

if anyone wants to ack this, I'll commit it.

Regards,
Carl-Daniel

On 15.01.2009 07:42, Li, Maggie wrote:
> Hi
>
> I have tested it on my DBM690T board. It is ok.
>
> Best regards
> Maggie li
>
> -----Original Message-----
> From: coreboot-bounces+maggie.li=[e-mail] [mailto:coreboot-bounces+maggie.li=[e-mail]] On Behalf Of Carl-Daniel Hailfinger
> Sent: Thursday, January 15, 2009 12:35 PM
> To: Marc Jones
> Cc: Rudolf Marek; Bao, Zheng; Coreboot
> Subject: Re: [coreboot] AMD DBM690T PowerNow table problems
>
> On 09.01.2009 10:05, Carl-Daniel Hailfinger wrote:
> > On 08.01.2009 17:52, Marc Jones wrote:
> > > On Wed, Jan 7, 2009 at 7:34 PM, Carl-Daniel Hailfinger
> > >  wrote:
> > > > The Linux kernel powernow-k8 driver doesn't complain anymore, but now
> > > > the machine will lock up or reboot once I put some load on it.
> > > > Blacklisting the powernow-k8 driver avoids the issue, but that makes
> > > > frequency scaling impossible.
> > >
> > > Can you dump the current tables or can you not get that far before a
> > > crash? The entries are end points and the driver takes care of the
> > > stepping that needs to be done to get to those points. We need to
> > > check that the table generated the correct vid setting for the fid. It
> > > is well described in the bkdg section 10.6.
> >
> > I managed to boot fine with the proprietary BIOS, blacklisted the
> > k8-powernow driver and rebooted with coreboot.
> >
> > If I combine the _PSS from the proprietary BIOS with the other tables
> > from coreboot, the machine does not crash. I'll try to narrow it down
> > further.
>
> I finally found the bug.
>
> The DBM90T code sets bit 10 in _PSS as part of the control value, but
> bit 10 is part of NewVID. That means the resulting VID is wrong and
> causes the processor to crash.
> The Pistachio code has the same bug.
>
> This patch fixes the wrong setting and changes control from a magic and
> incorrect unexplained value (0xE8202C00) to a combination of explained
> values and shifts which has the right value (0xE8202800).
>
> It is tested on my machine and it survived 200 changes from minimum to
> maximum frequency every 100 ms under heavy load and under no load.
>
> In the long term we want to consolidate all AMD FIDVID code into one
> generic library file.
>
> Signed-off-by: Carl-Daniel Hailfinger
>
> --- src/mainboard/amd/pistachio/acpi_tables.c	(Revision 3850)
> +++ src/mainboard/amd/pistachio/acpi_tables.c	(Arbeitskopie)
> @@ -400,10 +400,10 @@
>    	Pstate_num++;
>    }
> /* Print Pstate feq,vid,volt,power */
>
> /* Print Pstate freq,vid,volt,power */
>
> for (index = 0; index < Pstate_num; index++) {
> printk_info("Pstate_feq[%d] = %dMHz\t", index,
> printk_info("Pstate_freq[%d] = %dMHz\t", index,
>     Pstate_feq[index]);
> printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
> printk_info("Pstate_volt[%d] = %dmv\t", index,
>
> @@ -414,8 +414,11 @@
>
> /*
>     * Modify the DSDT Table to put the actural _PSS package
> * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
> * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
> * corefeq-->Pstate_feq[index]
> * power-->Pstate_power[index]
> * transitionlatency-->0x64
> * busmasterlatency-->0x7,
> * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
> status --> Pstate_vid[index]<<6 | Pstate_fid[index]
> Get the _PSS control method Sig.
> */
>
> @@ -461,7 +464,13 @@
>    			transitionlatency = 0x64;
>    			busmasterlatency = 0x7;
>    			control =
> 		    0xE8202C00 | (Pstate_vid[index] << 6) |
> 		    (0x3 << 30) | /* IRT */
> 		    (0x2 << 28) | /* RVO */
> 		    (0x1 << 27) | /* ExtType */
> 		    (0x2 << 20) | /* PLL_LOCK_TIME */
> 		    (0x0 << 18) | /* MVS */
> 		    (0x5 << 11) | /* VST */
> 		    (Pstate_vid[index] << 6) |
> 	    Pstate_fid[index];
> 	status =
> 	    (Pstate_vid[index] << 6) |
>
> --- src/mainboard/amd/dbm690t/acpi_tables.c	(Revision 3850)
> +++ src/mainboard/amd/dbm690t/acpi_tables.c	(Arbeitskopie)
> @@ -260,7 +260,7 @@
>
> /* Get the multipier of the fid frequency */
>    /*
> * Fid multiplier is always 100 revF and revG.
> * Fid multiplier is always 100 revF and revG.
> */
>  fid_multiplier = 100;
>
> @@ -400,10 +400,10 @@
>    	Pstate_num++;
>    }
> /* Print Pstate feq,vid,volt,power */
>
> /* Print Pstate freq,vid,volt,power */
>
> for (index = 0; index < Pstate_num; index++) {
> printk_info("Pstate_feq[%d] = %dMHz\t", index,
> printk_info("Pstate_freq[%d] = %dMHz\t", index,
>     Pstate_feq[index]);
> printk_info("Pstate_vid[%d] = %d\t", index, Pstate_vid[index]);
> printk_info("Pstate_volt[%d] = %dmv\t", index,
>
> @@ -414,8 +414,11 @@
>
> /*
>     * Modify the DSDT Table to put the actural _PSS package
> * corefeq-->Pstate_feq[index] power-->Pstate_power[index] transitionlatency-->0x64 busmasterlatency-->0x7,
> * control-->0xE8202C00| Pstate_vid[index]<<6 | Pstate_fid[index]
> * corefeq-->Pstate_feq[index]
> * power-->Pstate_power[index]
> * transitionlatency-->0x64
> * busmasterlatency-->0x7,
> * control--> 0xE8202800| Pstate_vid[index]<<6 | Pstate_fid[index]
> status --> Pstate_vid[index]<<6 | Pstate_fid[index]
> Get the _PSS control method Sig.
> */
>
> @@ -461,7 +464,13 @@
>    			transitionlatency = 0x64;
>    			busmasterlatency = 0x7;
>    			control =
> 		    0xE8202C00 | (Pstate_vid[index] << 6) |
> 		    (0x3 << 30) | /* IRT */
> 		    (0x2 << 28) | /* RVO */
> 		    (0x1 << 27) | /* ExtType */
> 		    (0x2 << 20) | /* PLL_LOCK_TIME */
> 		    (0x0 << 18) | /* MVS */
> 		    (0x5 << 11) | /* VST */
> 		    (Pstate_vid[index] << 6) |
> 	    Pstate_fid[index];
> 	status =
> 	    (Pstate_vid[index] << 6) |

[Carl-Daniel Hailfinger]

On 16.01.2009 06:39, Li, Maggie wrote:
> Acked-by: Maggie li

Thanks, committed in r3868.
> Sorry I thought it should be acked by Marc. He knows more about this part than me.

No need to be sorry. I appreciate that you tested and acked it.
> Best regards
> Maggie li

Regards,
Carl-Daniel
> -----Original Message-----
> From: Carl-Daniel Hailfinger [mailto:[e-mail]] 
> Sent: Friday, January 16, 2009 1:32 PM
> To: Li, Maggie
> Cc: Marc Jones; Rudolf Marek; Bao, Zheng; Coreboot
> Subject: Re: [coreboot] AMD DBM690T PowerNow table problems
>
> Hi,
>
> if anyone wants to ack this, I'll commit it.
>
> Regards,
> Carl-Daniel
>
> On 15.01.2009 07:42, Li, Maggie wrote:
> > Hi
> >
> > I have tested it on my DBM690T board. It is ok.
> >
> > Best regards
> > Maggie li
> >
> > -----Original Message-----
> > From: coreboot-bounces+maggie.li=[e-mail] [mailto:coreboot-bounces+maggie.li=[e-mail]] On Behalf Of Carl-Daniel Hailfinger
> > Sent: Thursday, January 15, 2009 12:35 PM
> > To: Marc Jones
> > Cc: Rudolf Marek; Bao, Zheng; Coreboot
> > Subject: Re: [coreboot] AMD DBM690T PowerNow table problems
> >
> > The DBM90T code sets bit 10 in _PSS as part of the control value, but
> > bit 10 is part of NewVID. That means the resulting VID is wrong and
> > causes the processor to crash.
> > The Pistachio code has the same bug.
> >
> > This patch fixes the wrong setting and changes control from a magic and
> > incorrect unexplained value (0xE8202C00) to a combination of explained
> > values and shifts which has the right value (0xE8202800).
> >
> > It is tested on my machine and it survived 200 changes from minimum to
> > maximum frequency every 100 ms under heavy load and under no load.
> >
> > In the long term we want to consolidate all AMD FIDVID code into one
> > generic library file.
> >
> > Signed-off-by: Carl-Daniel Hailfinger