[coreboot] [PATCH] KT890 autosetup LDT speeds from AMD NB

Uwe Hermann uwe at hermann-uwe.de
Sat Mar 15 19:05:39 CET 2008


On Sat, Mar 15, 2008 at 02:12:18AM +0100, Rudolf Marek wrote:
> Following experimental patch will setup KT890 HT automatically. It will find the
> max width of the link and also it will take the frequency of HT "setuped"
> already by coreboot (and checks if t can run on it).
> 
> Uwe, this should fix your board.

Great, will test later.

Please repost with a Signed-off-by though, you forgot it for this patch.

 
> Index: src/southbridge/via/k8t890/k8t890_early_car.c
> ===================================================================
> --- src/southbridge/via/k8t890/k8t890_early_car.c	(revision 3148)
> +++ src/southbridge/via/k8t890/k8t890_early_car.c	(working copy)
> @@ -22,35 +22,69 @@
>   * generate PCI reset or LDTSTOP to apply.
>   */
>  
> -u8 k8t890_early_setup_car(u8 width, u8 speed)
> +
> +static ldtreg[3] = {0x86, 0xa6, 0xc6};

The values should be documented in a comment, though.


> +
> +static min(unsigned int a, unsigned int b) {
> +	return (a < b) ? a : b ;
> +}

Please make this

  #define MIN(a,b) ((a) < (b) ? (a) : (b))
  #define MAX(a,b) ((a) > (b) ? (a) : (b))

and add it to stdlib.h where similar stuff (e.g. ARRAY_SIZE) lives.
It's not k8t890-specific and useful elsewhere.


> +
> +u8 k8t890_early_setup_HT(void)

Please add a high-level comment of what's going on in this function.


>  {
> -	u8 awidth, aspeed;
> +	u8 awidth, afreq;
> +	u8 cldtwidth_in, cldtwidth_out, vldtwidth_in, vldtwidth_out, ldtnr, width;
> +	u8 cldtfreq; 
> +	u16 vldtcaps;
> +	/* check if connected non coherent, initcomplete */
> +	if (0x7 == pci_read_config8(PCI_DEV(0, 0x18, 0), 0x98)) {
> +		ldtnr = 0;
> +	} else if (0x7 == pci_read_config8(PCI_DEV(0, 0x18, 0), 0xb8)) {
> +		ldtnr = 1;
> +	} else if (0x7 == pci_read_config8(PCI_DEV(0, 0x18, 0), 0xd8)) {
> +		ldtnr = 2;
> +	}
>  
> -	print_debug("LDT width and speed for K8T890 was");
> +	print_debug("K8T890f found at LDT ");

K8T890f? Is the "f" intentional?


> +	print_debug_hex8(ldtnr);
> +
> +	/* get the maximum widths for both sides */
> +	cldtwidth_in = pci_read_config8(PCI_DEV(0, 0x18, 0), ldtreg[ldtnr]) & 0x7;
> +	cldtwidth_out = (pci_read_config8(PCI_DEV(0, 0x18, 0), ldtreg[ldtnr]) >> 4) & 0x7;
> +	vldtwidth_in = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x66) & 0x7;
> +	vldtwidth_out = (pci_read_config8(PCI_DEV(0, 0x0, 0), 0x66) >> 4) & 0x7;
> +
> +	width = min(min(min(cldtwidth_out, cldtwidth_in), vldtwidth_out), vldtwidth_in);
> +	print_debug(" Agreed on width: ");
> +	print_debug_hex8(width);
> +
>  	awidth = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x67);
> -	print_debug_hex8(awidth);
>  
> -	aspeed = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x6d);
> -	print_debug_hex8(aspeed);
> +	/* Update the desired HT LNK to match AMD NB max from VIA NB is 0x1 */
> +	width = (width == 0x01) ? 0x11 : 0x00;
>  
> -	if ((aspeed == speed) && (((width == 16) && (awidth == 0x11)) ||
> -				  ((width == 8) && (awidth == 0x00))))
> -		return 0;
> +	pci_write_config8(PCI_DEV(0, 0x0, 0), 0x67, width);
>  
> -	/* Update the desired HT LNK capabilities in NB too. */
> -	pci_write_config8(PCI_DEV(0, 0x0, 0), 0x67,
> -			  (width == 16) ? 0x11 : 0x00);
> -	pci_write_config8(PCI_DEV(0, 0x0, 0), 0x6d, speed);
> +	/* Get programmed HT freq at base 0x89 */
> +	cldtfreq = pci_read_config8(PCI_DEV(0, 0x18, 0), ldtreg[ldtnr] + 3) & 0xf;
> +	print_debug(" CPU programmed to HT freq: ");
> +	print_debug_hex8(cldtfreq);
>  
> -	print_debug(" and will after HT reset: ");
> +	print_debug(" VIA HT caps: ");
> +	vldtcaps = pci_read_config16(PCI_DEV(0, 0, 0), 0x6e);
> +	print_debug_hex16(vldtcaps);
>  
> -	awidth = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x67);
> -	print_debug_hex8(awidth);
> +	if (!(vldtcaps & (1 << cldtfreq ))) {
> +		die("Chipset does not support desired HT frequency\n");
> +	}
>  
> -	aspeed = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x6d);
> -	print_debug_hex8(aspeed);
> -
> +	afreq = pci_read_config8(PCI_DEV(0, 0x0, 0), 0x6d);
> +	pci_write_config8(PCI_DEV(0, 0x0, 0), 0x6d, cldtfreq);
>  	print_debug("\n");
>  
> +	/* no reset needed */
> +	if ((width == awidth) && (afreq == cldtfreq)) {
> +		return 0;
> +	}
> +
>  	return 1;
>  }
> Index: src/mainboard/asus/a8v-e_se/cache_as_ram_auto.c
> ===================================================================
> --- src/mainboard/asus/a8v-e_se/cache_as_ram_auto.c	(revision 3148)
> +++ src/mainboard/asus/a8v-e_se/cache_as_ram_auto.c	(working copy)
> @@ -284,20 +284,19 @@
>  	init_timer();
>  	ht_setup_chains_x(sysinfo); /* Init sblnk and sbbusn, nodes, sbdn. */
>  
> -	enable_fid_change();
> -	init_fidvid_bsp(bsp_apicid);
> -
>  	needs_reset = optimize_link_coherent_ht();
>  	needs_reset |= optimize_link_incoherent_ht(sysinfo);
> +	needs_reset |= k8t890_early_setup_HT();

Rename this to k8t890_early_setup_ht(), function names should be
all-lowercase.


>  
> -	/* FIXME: Assumes that 1000MHz LDT is selected. */
> -	needs_reset |= k8t890_early_setup_car(16, 0x6);
> -
>  	if (needs_reset) {
>  		print_debug("ht reset -\r\n");
>  		soft_reset();
>  	}
>  
> +	/* the HT settings needs to be OK, because link freq chnage may cause HT disconnect */
> +	enable_fid_change();
> +	init_fidvid_bsp(bsp_apicid);
> +
>  	/* Stop the APs so we can start them later in init. */
>  	allow_all_aps_stop(bsp_apicid);

Looks good otherwise, but I'll test on hardware later and report back.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the coreboot mailing list