[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=amd.com at coreboot.org [mailto:coreboot-bounces+maggie.li=amd.com at coreboot.org] 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
>>> <c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
>
> --- 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) |
>
>