Re: [coreboot] AMD DBM690T PowerNow table problems

[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) |