[coreboot] [patch][v2] AMD Fam10 memory controller updates

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Apr 9 02:25:11 CEST 2008


On 09.04.2008 00:28, Marc Jones wrote:

> Bring Fam10 memory controller init up to date with the latest AMD BKDG
> recomendations.
>
> Changes include the following:
> fix > 4GB dqs tests
> fix channel interleaving
> fix memory hoisting across nodes (memory hole was always on node0)
> ecc memory scrub updates
> debug print changes
>
> Signed-off-by: Marc Jones (marc.jones at amd.com)

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
with some comments, though.

04:49 < carldani> mct_d.c, line 605. Should this be devx and not dev?
04:51 < carldani> marcj: mct_d.c, function HTMemMapInit_D(), the first big for (Node = 0; Node < MAX_NODES_SUPPORTED; Node++) loop
04:54 < marcj> ok, had to read through. The memmap is setup on node0 and copied to the other nodes later. so dev, not devx.
04:55 < carldani> marcj: the DRAM Hole Address Register is set via devx in each node, but the Node number <-> DRAM Base mapping and the Node number <-> DstNode mapping is set in Node 0
04:55 < marcj> yes
04:55 < carldani> ah ok thanks
04:56 < marcj> the hole address is only set on the node with the hole
04:57 < marcj> the entire map is the same for all nodes so it gets setup all in the first node. Otherwise you would have part of it in each node and would be a pain to copy iy to every node.
04:57 < marcj> and that was the bug, the hole was always being set on the first node.
05:04 < carldani> marcj: could you add the last two lines you wrote to the changelog? that would help immensely.
05:05 < marcj> sure

Adding that explanation to the code as comments would be even greater.

>
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mct_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mct_d.c	2008-04-07 16:14:23.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mct_d.c	2008-04-08 14:38:27.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -279,6 +279,9 @@
>  	print_t("mctAutoInitMCT_D: DQSTiming_D\n");
>  	DQSTiming_D(pMCTstat, pDCTstatA);	/* Get Receiver Enable and DQS signal timing*/
>  
> +	print_t("mctAutoInitMCT_D: UMAMemTyping_D\n");
> +	UMAMemTyping_D(pMCTstat, pDCTstatA);	/* Fix up for UMA sizing */
> +
>  	print_t("mctAutoInitMCT_D: :OtherTiming\n");
>  	mct_OtherTiming(pMCTstat, pDCTstatA);
>  
> @@ -348,10 +351,11 @@
>  	/* FIXME: BOZO- DQS training every time*/
>  	nv_DQSTrainCTL = 1;

I assume the assignment above will be replaced with a calculated value
in later code revisions. You confirmed that on IRC.

>  
> +	print_t("DQSTiming_D: mct_BeforeDQSTrain_D:\n");
> +	mct_BeforeDQSTrain_D(pMCTstat, pDCTstatA);;
> +	phyAssistedMemFnceTraining(pMCTstat, pDCTstatA);
> +
>  	if (nv_DQSTrainCTL) {

With the current code, this is always true. (see above)

> -		print_t("DQSTiming_D: mct_BeforeDQSTrain_D:\n");
> -		mct_BeforeDQSTrain_D(pMCTstat, pDCTstatA);;
> -		phyAssistedMemFnceTraining(pMCTstat, pDCTstatA);
>  		mctHookBeforeAnyTraining();
>  
>  		print_t("DQSTiming_D: TrainReceiverEn_D FirstPass:\n");
> @@ -512,7 +516,7 @@
>  	u32 val;
>  	u32 base;
>  	u32 limit;
> -	u32 dev;
> +	u32 dev, devx;
>  	struct DCTStatStruc *pDCTstat;
>  
>  	_MemHoleRemap = mctGet_NVbits(NV_MemHole);
> @@ -531,6 +535,8 @@
>  
>  
>  	for (Node = 0; Node < MAX_NODES_SUPPORTED; Node++) {
> +		pDCTstat = pDCTstatA + Node;
> +		devx = pDCTstat->dev_map;
>  		DramSelBaseAddr = 0;
>  		pDCTstat = pDCTstatA + Node;
>  		if (!pDCTstat->GangedMode) {
> @@ -569,7 +575,7 @@
>  					val <<= 8; /* shl 16, rol 24 */
>  					val |= DramHoleBase << 24;
>  					val |= 1  << DramHoleValid;
> -					Set_NB32(dev, 0xF0, val); /*Dram Hole Address Register*/
> +					Set_NB32(devx, 0xF0, val); /*Dram Hole Address Register*/
>  					pDCTstat->DCTSysLimit += HoleSize;
>  					base = pDCTstat->DCTSysBase;
>  					limit = pDCTstat->DCTSysLimit;
> @@ -598,21 +604,31 @@
>  			pMCTstat->SysLimit = limit;
>  		}
>  		Set_NB32(dev, 0x40 + (Node << 3), base); /* [Node] + Dram Base 0 */
> -		val = limit & 0xffff0000;
> -		val |= Node;				/* set DstNode*/
> +		if ((mctSetNodeBoundary_D()) && (limit > 0x00400000)) {

Hm. Could it be that we have to check (limit >= 0x00400000) instead?

> +			limit++;
> +			limit &= 0xFFC00000;
> +			limit--;

I see the formula, but I don't understand its purpose.

> +		}
> +		val = limit & 0xFFFF0000;
> +		val |= Node;
>  		Set_NB32(dev, 0x44 + (Node << 3), val);	/* set DstNode */
>  
>  		limit = pDCTstat->DCTSysLimit;
>  		if (limit) {
> -			NextBase = (limit & 0xffff0000) + 0x10000;
> +			NextBase = (limit & 0xFFFF0000) + 0x10000;
> +			if ((mctSetNodeBoundary_D()) && (NextBase > 0x00400000)) {
> +				NextBase++;
> +				NextBase &= 0xFFC00000;
> +				NextBase--;

Same here.

> +			}
>  		}
>  	}
>  
>  	/* Copy dram map from Node 0 to Node 1-7 */
>  	for (Node = 1; Node < MAX_NODES_SUPPORTED; Node++) {
> -		pDCTstat = pDCTstatA + Node;
>  		u32 reg;
> -		u32 devx = pDCTstat->dev_map;
> +		pDCTstat = pDCTstatA + Node;
> +		devx = pDCTstat->dev_map;
>  
>  		if (pDCTstat->NodePresent) {
>  		printk_debug(" Copy dram map from Node 0 to Node %02x \n", Node);
> @@ -1016,7 +1032,7 @@
>  			if ((val == 0) || (val == 0xFF)) {
>  				pDCTstat->ErrStatus |= 1<<SB_NoTrcTrfc;
>  				pDCTstat->ErrCode = SC_VarianceErr;
> -				val = Get_DefTrc_k_D(pDCTstat->DIMMAutoSpeed);
> +				val = Get_DefTrc_k_D(pDCTstat->Speed);
>  			} else {
>  				byte = mctRead_SPD(smbaddr, SPD_TRCRFC);
>  				if (byte & 0xF0) {
> @@ -1054,7 +1070,7 @@
>  
>  	/* Convert  DRAM CycleTiming values and store into DCT structure */
>  	DDR2_1066 = 0;
> -	byte = pDCTstat->DIMMAutoSpeed;
> +	byte = pDCTstat->Speed;
>  	if (byte == 5)
>  		DDR2_1066 = 1;
>  	Tk40 = Get_40Tk_D(byte);
> @@ -1175,12 +1191,10 @@
>  	dword = Trtp * 10;
>  	pDCTstat->DIMMTrtp = dword;
>  	val = pDCTstat->Speed;
> -	if (val <= 2) {
> -		val = 2;	/* Calculate by 7.75ns / Speed in ns to get clock # */
> -	} else if (val == 4) {	/* Note a speed of 3 will be a Trtp of 3 */
> -		val = 3;
> -	} else if (val == 5){
> -		val = 2;
> +	if (val <= 2) {				/* Calculate by 7.75ns / Speed in ns to get clock # */
> +		val = 2;				/* for DDR400/DDR533 */
> +	} else {					/* Note a speed of 3 will be a Trtp of 3 */
> +		val = 3;				/* for DDR667/DDR800/DDR1066 */

Are the added tabs before the comments intentional?

>  	}
>  	pDCTstat->Trtp = val;
>  
> @@ -1810,7 +1824,7 @@
>  	/* SetCKETriState */
>  	SetODTTriState(pMCTstat, pDCTstat, dct);
>  
> -	if ( pDCTstat->Status & 1<<SB_128bitmode) {
> +	if ( pDCTstat->Status & (1 << SB_128bitmode)) {

Space after (

>  		SetCSTriState(pMCTstat, pDCTstat, 1); /* force dct1) */
>  		SetODTTriState(pMCTstat, pDCTstat, 1); /* force dct1) */
>  	}
> @@ -2631,7 +2645,7 @@
>  	if (!pDCTstat->GangedMode) {
>  		dev = pDCTstat->dev_dct;
>  		pDCTstat->NodeSysLimit += pDCTstat->DCTSysLimit;
> -		/* if DCT0 and DCT1 exist both, set DctSelBaseAddr[47:27] */
> +		/* if DCT0 and DCT1 exist both, set DctSelBaseAddr[47:27] to the top of DCT0 */

Maybe use "...both exist...", but I'm not a native speaker.

>  		if (dct == 0) {
>  			if (pDCTstat->DIMMValidDCT[1] > 0) {
>  				dword = pDCTstat->DCTSysLimit + 1;
> @@ -2641,8 +2655,7 @@
>  					pMCTstat->HoleBase = (DramHoleBase & 0xFFFFF800) << 8;
>  					val = pMCTstat->HoleBase;
>  					val >>= 16;
> -					val &= ~(0xFF);
> -					val |= (((~val) & 0xFF) + 1);
> +					val = (((~val) & 0xFF) + 1);
>  					val <<= 8;
>  					dword += val;
>  				}
> @@ -2653,6 +2666,7 @@
>  				val |= 3;  /* Set F2x110[DctSelHiRngEn], F2x110[DctSelHi] */
>  				Set_NB32(dev, reg, val);
>  				print_tx("AfterStitch DCT0 and DCT1: DRAM Controller Select Low Register = ", val);
> +				print_tx("AfterStitch DCT0 and DCT1: DRAM Controller Select High Register = ", dword);
>  
>  				reg = 0x114;
>  				val = dword;
> @@ -2664,7 +2678,7 @@
>  			if (pDCTstat->DIMMValidDCT[0] == 0) {
>  				dword = pDCTstat->NodeSysBase;
>  				dword >>= 8;
> -				if (dword >= DramHoleBase) {
> +				if ((dword >= DramHoleBase) && _MemHoleRemap) {
>  					pMCTstat->HoleBase = (DramHoleBase & 0xFFFFF800) << 8;
>  					val = pMCTstat->HoleBase;
>  					val >>= 8;
> @@ -2680,6 +2694,7 @@
>  				val |= 3;	/* Set F2x110[DctSelHiRngEn], F2x110[DctSelHi] */
>  				Set_NB32(dev, reg, val);
>  				print_tx("AfterStitch DCT1 only: DRAM Controller Select Low Register = ", val);
> +				print_tx("AfterStitch DCT1 only: DRAM Controller Select High Register = ", dword);
>  			}
>  		}
>  	} else {
> @@ -2872,16 +2887,25 @@
>  static void Get_Twrrd(struct MCTStatStruc *pMCTstat,
>  			struct DCTStatStruc *pDCTstat, u8 dct)
>  {
> -	u8 byte, bytex;
> +	u8 byte, bytex, val;
>  	u32 index_reg = 0x98 + 0x100 * dct;
>  	u32 dev = pDCTstat->dev_dct;
>  
>  	/* On any given byte lane, the largest WrDatGrossDlyByte delay of
>  	   any DIMM minus the DqsRcvEnGrossDelay delay of any other DIMM is
>  	   equal to the Critical Gross Delay Difference (CGDD) for Twrrd.*/
> -	pDCTstat->Twrrd = 0;
> +
> +	/* WrDatGrossDlyByte only use one set register when DDR400~DDR667
> +	   DDR800 have two set register for DIMM0 and DIMM1 */
> +	if (pDCTstat->Speed > 3) {
> +		val = Get_WrDatGross_Diff(pDCTstat, dct, dev, index_reg);
> +	} else {
> +		val = Get_WrDatGross_MaxMin(pDCTstat, dct, dev, index_reg, 1);	/* WrDatGrossDlyByte byte 0,1,2,3 for DIMM0 */
> +		pDCTstat->WrDatGrossH = (u8) val; /* low byte = max value */
> +	}
> +
>  	Get_DqsRcvEnGross_Diff(pDCTstat, dev, index_reg);
> -	Get_WrDatGross_Diff(pDCTstat, dct, dev, index_reg);
> +
>  	bytex = pDCTstat->DqsRcvEnGrossL;
>  	byte = pDCTstat->WrDatGrossH;
>  	if (byte > bytex) {
> @@ -2946,12 +2970,16 @@
>  	u8 i;
>  	u32 val;
>  	u8 byte;
> +	u8 ecc_reg = 0;
>  
>  	Smallest_0 = 0xFF;
>  	Smallest_1 = 0xFF;
>  	Largest_0 = 0;
>  	Largest_1 = 0;
>  
> +	if (index == 0x12)
> +		ecc_reg = 1;
> +
>  	for (i=0; i < 8; i+=2) {
>  		if ( pDCTstat->DIMMValid & (1 << i)) {
>  			val = Get_NB32_index_wait(dev, index_reg, index);
> @@ -2960,11 +2988,13 @@
>  				Smallest_0 = byte;
>  			if (byte > Largest_0)
>  				Largest_0 = byte;
> -			byte = (val >> 16) & 0xFF;
> -			if (byte < Smallest_1)
> -				Smallest_1 = byte;
> -			if (byte > Largest_1)
> -				Largest_1 = byte;
> +			if (!(ecc_reg)) {
> +				byte = (val >> 16) & 0xFF;
> +				if (byte < Smallest_1)
> +					Smallest_1 = byte;
> +				if (byte > Largest_1)
> +					Largest_1 = byte;
> +			}
>  		}
>  		index += 3;
>  	}	/* while ++i */
> @@ -2973,8 +3003,9 @@
>  	   two DIMMs is less than half of a MEMCLK */
>  	if ((Largest_0 - Smallest_0) > 31)
>  		return 1;
> -	if ((Largest_1 - Smallest_1) > 31)
> -		return 1;
> +	if (!(ecc_reg))
> +		if ((Largest_1 - Smallest_1) > 31)
> +			return 1;
>  	return 0;
>  }
>  
> @@ -3072,10 +3103,14 @@
>  	u8 byte;
>  	u32 val;
>  	u16 word;
> +	u8 ecc_reg = 0;
>  
>  	Smallest = 7;
>  	Largest = 0;
>  
> +	if (index == 0x12)
> +		ecc_reg = 1;
> +
>  	for (i=0; i < 8; i+=2) {
>  		if ( pDCTstat->DIMMValid & (1 << i)) {
>  			val = Get_NB32_index_wait(dev, index_reg, index);
> @@ -3085,11 +3120,13 @@
>  				Smallest = byte;
>  			if (byte > Largest)
>  				Largest = byte;
> -			byte = (val >> (16 + 5)) & 0xFF;
> -			if (byte < Smallest)
> -				Smallest = byte;
> -			if (byte > Largest)
> -				Largest = byte;
> +			if (!(ecc_reg)) {
> +				byte = (val >> (16 + 5)) & 0xFF;
> +				if (byte < Smallest)
> +					Smallest = byte;
> +				if (byte > Largest)
> +					Largest = byte;
> +			}
>  		}
>  	index += 3;
>  	}	/* while ++i */
> @@ -3353,25 +3390,32 @@
>  	u32 index_reg = 0x98 + 0x100 * dct;
>  	u8 cs;
>  	u32 index;
> -	u16 word;
> -
> -	/* Tri-state unused ODTs when motherboard termination is available */
> +	u8 odt;
> +	u8 max_dimms;
>  
>  	// FIXME: skip for Ax
>  
> -	dev = pDCTstat->dev_dct;
> -	word = 0;
> +	/* Tri-state unused ODTs when motherboard termination is available */
> +	max_dimms = (u8) mctGet_NVbits(NV_MAX_DIMMS);
> +	odt = 0x0F;	/* tristate all the pins then clear the used ones. */
> +
>  	for (cs = 0; cs < 8; cs += 2) {
> -		if (!(pDCTstat->CSPresent & (1 << cs))) {
> -			if (!(pDCTstat->CSPresent & (1 << (cs + 1))))
> -				word |= (1 << (cs >> 1));
> +		if (pDCTstat->CSPresent & (1 << cs)) {
> +			odt &= ~(1 << (cs / 2));
> +
> +			/* if quad-rank capable platform clear adtitional pins */
> +			if (max_dimms != MAX_CS_SUPPORTED) {
> +				if (pDCTstat->CSPresent & (1 << (cs + 1)))
> +					odt &= ~(4 << (cs / 2));
> +			}
>  		}
>  	}
>  
>  	index  = 0x0C;
>  	val = Get_NB32_index_wait(dev, index_reg, index);
> -	val |= (word << 8);
> +	val |= (odt << 8);
>  	Set_NB32_index_wait(dev, index_reg, index, val);
> +
>  }
>  
>  
> @@ -3414,8 +3458,14 @@
>  	}
>  
>  	/* Override/Exception */
> -	if ((pDCTstat->Speed == 2) && (pDCTstat->MAdimms[dct] == 4))
> -		dword &= 0xF18FFF18;
> +	if (!pDCTstat->GangedMode) {
> +		i = 0; // use i for the dct setting required
> +		if (pDCTstat->MAdimms[0] < 4)
> +			i = 1;
> +		if (((pDCTstat->Speed == 2) || (pDCTstat->Speed == 3)) && (pDCTstat->MAdimms[i] == 4))
> +			dword &= 0xF18FFF18;
> +			index_reg = 0x98;	/* force dct = 0 */
> +	}
>  
>  	Set_NB32_index_wait(dev, index_reg, 0x0a, dword);
>  }
> @@ -3733,12 +3783,26 @@
>  					struct DCTStatStruc *pDCTstat, u8 dct)
>  {
>  	u8 Receiver;
> -	u32 val;
>  	u32 dev = pDCTstat->dev_dct;
>  	u32 reg_off = 0x100 * dct;
>  	u32 addr;
> +	u32 lo, hi;
> +	u8 wrap32dis = 0;
>  	u8 valid = 0;
>  
> +	/* FIXME: Skip reset DLL for B3 */
> +
> +	addr = HWCR;
> +	_RDMSR(addr, &lo, &hi);
> +	if(lo & (1<<17)) {		/* save the old value */
> +		wrap32dis = 1;
> +	}
> +	lo |= (1<<17);			/* HWCR.wrap32dis */
> +	lo &= ~(1<<15);			/* SSEDIS */
> +	/* Setting wrap32dis allows 64-bit memory references in 32bit mode */
> +	_WRMSR(addr, lo, hi);
> +
> +
>  	pDCTstat->Channel = dct;
>  	Receiver = mct_InitReceiver_D(pDCTstat, dct);
>  	/* there are four receiver pairs, loosely associated with chipselects.*/
> @@ -3747,23 +3811,24 @@
>  			addr = mct_GetRcvrSysAddr_D(pMCTstat, pDCTstat, dct, Receiver, &valid);
>  			if (valid) {
>  				mct_Read1LTestPattern_D(pMCTstat, pDCTstat, addr);	/* cache fills */
> -				Set_NB32(dev, 0x98 + reg_off, 0x0D00000C);
> -				val = Get_NB32(dev, 0x9C + reg_off);
> -				val |= 1 << 15;
> -				Set_NB32(dev, 0x9C + reg_off, val);
> -				Set_NB32(dev, 0x98 + reg_off, 0x4D0F0F0C);
> -				mct_Wait_10ns(60); /* wait >= 300ns */
> -
> -				Set_NB32(dev, 0x98 + reg_off, 0x0D00000C);
> -				val = Get_NB32(dev, 0x9C + reg_off);
> -				val &= ~(1 << 15);
> -				Set_NB32(dev, 0x9C + reg_off, val);
> -				Set_NB32(dev, 0x98 + reg_off, 0x4D0F0F0C);
> -				mct_Wait_10ns(400); /* wait >= 2us */
> +
> +				/* Write 0000_8000h to register F2x[1, 0]9C_xD080F0C */

Should there really be a space after the comma?

> +				Set_NB32_index_wait(dev, 0x98 + reg_off, 0x4D080F0C, 0x00008000);
> +				mct_Wait(80); /* wait >= 300ns */
> +
> +				/* Write 0000_0000h to register F2x[1, 0]9C_xD080F0C */

Dito.

> +				Set_NB32_index_wait(dev, 0x98 + reg_off, 0x4D080F0C, 0x00000000);
> +				mct_Wait(800); /* wait >= 2us */
>  				break;
>  			}
>  		}
>  	}
> +	if(!wrap32dis) {
> +		addr = HWCR;
> +		_RDMSR(addr, &lo, &hi);
> +		lo &= ~(1<<17);		/* restore HWCR.wrap32dis */
> +		_WRMSR(addr, lo, hi);
> +	}
>  }
>  
>  
> @@ -3784,7 +3849,7 @@
>  		// FIXME Skip for Cx
>  		dev = pDCTstat->dev_nbmisc;
>  		val = Get_NB32(dev, 0x8C);	// NB Configuration Hi
> -		val |= 36-32;			// DisDatMask
> +		val |= 1 << (36-32);		// DisDatMask
>  		Set_NB32(dev, 0x8C, val);
>  	}
>  }
> @@ -3818,8 +3883,9 @@
>  	u32 reg_off = 0x100 * dct;
>  	u32 dev = pDCTstat->dev_dct;
>  
> +	/* FIXME: Add B3 */
>  	if (pDCTstat->LogicalCPUID & AMD_DR_B2) {
> -		mct_Wait_10ns(5000);	/* Wait 50 us*/
> +		mct_Wait(10000);	/* Wait 50 us*/

The comment seems to contradict the code.

>  		val = Get_NB32(dev, 0x110);
>  		if ( val & (1 << DramEnabled)) {
>  			/* If 50 us expires while DramEnable =0 then do the following */
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mct_d.h
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mct_d.h	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mct_d.h	2008-04-07 21:42:39.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -699,6 +699,7 @@
>  u8 mct_Get_Start_RcvrEnDly_1Pass(u8 Pass);
>  u8 mct_Average_RcvrEnDly_Pass(struct DCTStatStruc *pDCTstat, u8 RcvrEnDly, u8 RcvrEnDlyLimit, u8 Channel, u8 Receiver, u8 Pass);
>  void CPUMemTyping_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstatA);
> +void UMAMemTyping_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstatA);
>  u32 mctGetLogicalCPUID(u32 Node);
>  u8 ECCInit_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstatA);
>  void TrainReceiverEn_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstatA, u8 Pass);
> @@ -730,7 +731,7 @@
>  u8 mct_SaveRcvEnDly_D_1Pass(struct DCTStatStruc *pDCTstat, u8 pass);
>  static void mct_AdjustScrub_D(struct DCTStatStruc *pDCTstat, u16 *scrub_request);
>  static u8 mct_InitReceiver_D(struct DCTStatStruc *pDCTstat, u8 dct);
> -static void mct_Wait_10ns (u32 cycles);
> +static void mct_Wait(u32 cycles);
>  u8 mct_RcvrRankEnabled_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, u8 Channel, u8 ChipSel);
>  u32 mct_GetRcvrSysAddr_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, u8 channel, u8 receiver, u8 *valid);
>  void mct_Read1LTestPattern_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstat, u32 addr);
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mctchi_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mctchi_d.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mctchi_d.c	2008-04-07 21:42:57.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -41,11 +41,10 @@
>  	/* call back to wrapper not needed ManualChannelInterleave_D(); */
>  	/* call back - DctSelIntLvAddr = mctGet_NVbits(NV_ChannelIntlv);*/	/* override interleave */
>  	// FIXME: Check for Cx
> -	DctSelIntLvAddr = 5;	/* use default: Enable channel interleave */
> -	enabled = 1;		/* with Hash*: exclusive OR of address bits[20:16, 6]. */
> +	DctSelIntLvAddr = mctGet_NVbits(NV_ChannelIntlv); /* typ=5: Hash*: exclusive OR of address bits[20:16, 6]. */
>  	beforeInterleaveChannels_D(pDCTstatA, &enabled);
>  
> -	if (enabled) {
> +	if (DctSelIntLvAddr & 1) {
>  		DctSelIntLvAddr >>= 1;
>  		HoleSize = 0;
>  		if ((pMCTstat->GStatus & (1 << GSB_SoftHole)) ||
> @@ -84,7 +83,7 @@
>  				dct0_size += DramBase;
>  				dct0_size += dct1_size;
>  				if (dct0_size >= HoleBase)	/* if DctSelBaseAddr > HoleBase */
> -					dct0_size += HoleBase;
> +					dct0_size += HoleSize;
>  				DctSelBase = dct0_size;
>  
>  				if (dct1_size == 0)
> @@ -100,7 +99,7 @@
>  				val |= DctSelHi;
>  				val |= (DctSelIntLvAddr << 6) & 0xFF;
>  				Set_NB32(pDCTstat->dev_dct, 0x110, val);
> -				print_tx("InterleaveChannels: DRAM Controller Select Low Register = ", val);
> +				print_tx("InterleaveChannels: F2x110 DRAM Controller Select Low Register = ", val);
>  
>  				if (HoleValid) {
>  					tmp = DramBase;
> @@ -112,10 +111,10 @@
>  					}
>  					tmp += HoleSize;
>  					val = Get_NB32(pDCTstat->dev_map, 0xF0);	/* DramHoleOffset */
> -					val &= 0x7F;
> -					val |= (tmp & 0xFF);
> +					val &= 0xFFFF007F;
> +					val |= (tmp & ~0xFFFF007F);
>  					Set_NB32(pDCTstat->dev_map, 0xF0, val);
> -print_tx("InterleaveChannels:0xF0 = ", val);
> +					print_tx("InterleaveChannels: F1xF0 DRAM Hole Address Register = ", val);
>  
>  				}
>  			}
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mctdqs_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mctdqs_d.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mctdqs_d.c	2008-04-07 21:43:15.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -1130,6 +1130,9 @@
>  			val += (1 << (15-8));	/* add 32K */
>  	}
>  
> +	/* Add a node seed */
> +	val += (((1 * pDCTstat->Node_ID) << 20) >> 8);	/* Add 1MB per node to avoid aliases */
> +
>  	/* HW remap disabled? */
>  	if (!(pDCTstat->Status & (1 << SB_HWHole))) {
>  		if (!(pDCTstat->Status & (1 << SB_SWNodeHole))) {
> @@ -1167,6 +1170,13 @@
>  			*valid = 1;
>  		}
>  	}
> +	print_debug_dqs("mct_GetMCTSysAddr_D: receiver ", receiver, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: Channel ", Channel, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: base_addr ", val, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: valid ", *valid, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: status ", pDCTstat->Status, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: HoleBase ", pDCTstat->DCTHoleBase, 2);
> +	print_debug_dqs("mct_GetMCTSysAddr_D: Cachetop ", pMCTstat->Sub4GCacheTop, 2);
>  
>  exitGetAddr:
>  	return val;
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mctecc_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mctecc_d.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mctecc_d.c	2008-04-08 15:17:01.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -88,6 +88,7 @@
>  	u32 dev;
>  	u32 reg;
>  	u32 val;
> +	u16 nvbits;
>  
>  	mctHookBeforeECC();
>  
> @@ -100,14 +101,15 @@
>  	OB_ChipKill = mctGet_NVbits(NV_ChipKill); 	/* ECC Chip-kill mode */
>  
>  	OF_ScrubCTL = 0;		/* Scrub CTL for Dcache, L2, and dram */
> -	val = mctGet_NVbits(NV_DCBKScrub);
> -	mct_AdjustScrub_D(pDCTstatA, val);
> -	OF_ScrubCTL |= val << 16;
> -	val = mctGet_NVbits(NV_L2BKScrub);
> -	OF_ScrubCTL |= val << 8;
> +	nvbits = mctGet_NVbits(NV_DCBKScrub);
> +	mct_AdjustScrub_D(pDCTstatA, &nvbits);
> +	OF_ScrubCTL |= (u32) nvbits << 16;
>  
> -	val = mctGet_NVbits(NV_DramBKScrub);
> -	OF_ScrubCTL |= val;
> +	nvbits = mctGet_NVbits(NV_L2BKScrub);
> +	OF_ScrubCTL |= (u32) nvbits << 8;
> +
> +	nvbits = mctGet_NVbits(NV_DramBKScrub);
> +	OF_ScrubCTL |= nvbits;
>  
>  	AllECC = 1;
>  	MemClrECC = 0;
> @@ -190,7 +192,20 @@
>  					Set_NB32(dev, 0x5C, val); /* Dram Scrub Addr Low */
>  					val = curBase>>24;
>  					Set_NB32(dev, 0x60, val); /* Dram Scrub Addr High */
> -					Set_NB32(dev, 0x58, OF_ScrubCTL);	/*Scrub Control */    /*set dram background scrubbing to setup value */
> +					Set_NB32(dev, 0x58, OF_ScrubCTL);	/*Scrub Control */
> +
> +					/* Divisor should not be set deeper than
> +					 * divide by 16 when Dcache scrubber or
> +					 * L2 scrubber is enabled.
> +					 */
> +					if ((OF_ScrubCTL & (0x1F << 16)) || (OF_ScrubCTL & (0x1F << 8))) {
> +						val = Get_NB32(dev, 0x84);
> +						if ((val & 0xE0000000) > 0x80000000) {	/* Get F3x84h[31:29]ClkDivisor for C1 */
> +							val &= 0x1FFFFFFF;	/* If ClkDivisor is deeper than divide-by-16 */
> +							val |= 0x80000000;	/* set it to divide-by-16 */
> +							Set_NB32(dev, 0x84, val);
> +						}
> +					}
>  				}	/* this node has ECC enabled dram */
>  			}	/*Node has Dram */
>  		}	/*if Node present */
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mctmtr_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mctmtr_d.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mctmtr_d.c	2008-04-07 21:43:46.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -61,15 +61,7 @@
>  		Bottom40bIO = val;
>  	}
>  
> -	val = mctGet_NVbits(NV_BottomUMA);
> -	if(val == 0)
> -		val++;
> -
> -	val <<= (24-8);
> -	if(val >  Bottom32bIO)
> -		val = Bottom32bIO;
> -
> -	Cache32bTOP = val;
> +	Cache32bTOP = Bottom32bIO;
>  
>  	/*======================================================================
>  	 Set default values for CPU registers
> @@ -118,6 +110,8 @@
>  	if(Bottom40bIO) {
>  		hi = Bottom40bIO >> 24;
>  		lo = Bottom40bIO << 8;
> +		if (mctSetNodeBoundary_D())
> +			lo &= 0xC0000000;
>  		addr += 3;		/* TOM2 */
>  		_WRMSR(addr, lo, hi);
>  	}
> @@ -210,4 +204,54 @@
>  	*pMtrrAddr = addr;
>  }
>  
> +void UMAMemTyping_D(struct MCTStatStruc *pMCTstat, struct DCTStatStruc *pDCTstatA)
> +{
> +/* UMA memory size may need splitting the MTRR configuration into two
> +  Before training use NB_BottomIO or the physical memory size to set the MTRRs.
> +  After training, add UMAMemTyping function reconfigure the MTRRs based on

Suggested wording: "...function to reconfigure..."

> +  NV_BottomUMA (for UMA systems only).
> +  This two-step process allows all memory to be cached for training
> +*/
> +	u32 Bottom32bIO, Cache32bTOP;
> +	u32 val;
> +	u32 addr;
> +	u32 lo, hi;
> +
> +	/*======================================================================
> +	 * Adjust temp top of memory down to accomodate UMA memory start
> +	 *======================================================================*/
> +	/* Bottom32bIO=sub 4GB top of memory, right justified 8 bits
> +	 * (defines dram versus IO space type)
> +	 * Cache32bTOP=sub 4GB top of WB cacheable memory, right justified 8 bits */
>  
> +	Bottom32bIO = pMCTstat->Sub4GCacheTop >> 8;
> +
> +	val = mctGet_NVbits(NV_BottomUMA);
> +	if (val == 0)
> +		val++;
> +
> +	val <<= (24-8);
> +	if (val < Bottom32bIO) {
> +		Cache32bTOP = val;
> +		pMCTstat->Sub4GCacheTop = val;
> +
> +	/*======================================================================
> +	 * Clear variable MTRR values
> +	 *======================================================================*/
> +		addr = 0x200;
> +		lo = 0;
> +		hi = lo;
> +		while( addr < 0x20C) {
> +			_WRMSR(addr, lo, hi);		/* prog. MTRR with current region Mask */
> +			addr++;						/* next MTRR pair addr */
> +		}
> +
> +		/*======================================================================
> +		 * Set variable MTRR values
> +		 *======================================================================*/
> +		print_tx("\t UMAMemTyping_D: Cache32bTOP:", Cache32bTOP);
> +		SetMTRRrangeWB_D(0, &Cache32bTOP, &addr);
> +		if(addr == -1)		/* ran out of MTRRs?*/
> +			pMCTstat->GStatus |= 1<<GSB_MTRRshort;
> +	}
> +}
> Index: coreboot-v2/src/northbridge/amd/amdmct/mct/mctsrc.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/mct/mctsrc.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/mct/mctsrc.c	2008-04-07 21:43:58.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -42,10 +42,7 @@
>  				u8 Channel, u8 Receiver,
>  				u32 dev, u32 index_reg,
>  				u8 Addl_Index, u8 Pass);
> -static void CalcMaxLatency_D(struct DCTStatStruc *pDCTstat,
> -				u8 DQSRcvrEnDly, u8 Channel);
>  static void mct_SetMaxLatency_D(struct DCTStatStruc *pDCTstat, u8 Channel, u8 DQSRcvEnDly);
> -static void mct_SetDQSRcvEn_D(struct DCTStatStruc *pDCTstat, u32 val);
>  static void fenceDynTraining_D(struct MCTStatStruc *pMCTstat,
>  			struct DCTStatStruc *pDCTstat, u8 dct);
>  static void mct_DisableDQSRcvEn_D(struct DCTStatStruc *pDCTstat);
> @@ -766,9 +763,8 @@
>  	u8 *test_buf;
>  	u8 i;
>  	u8 result;
> -	u8 *addr_lo_buf;
> +	u8 value;
>  
> -	SetUpperFSbase(addr);	// needed?
>  
>  	if(Pass == FirstPass) {
>  		if(pattern==1) {
> @@ -780,44 +776,27 @@
>  		test_buf = (u8 *)TestPattern2_D;
>  	}
>  
> -	addr_lo_buf = (u8 *) (addr << 8);
> -	result = DQS_FAIL;
> +	SetUpperFSbase(addr);
> +	addr <<= 8;
>  
>  	if((pDCTstat->Status & (1<<SB_128bitmode)) && channel ) {
> -		addr_lo_buf += 8;	/* second channel */
> +		addr += 8;	/* second channel */
>  		test_buf += 8;
>  	}
>  
> -
> -#if DQS_TRAIN_DEBUG > 4
> -	print_debug("\t\t\t\t\t\tQW0 :   test_buf  = ");
> -	print_debug_hex32((unsigned)test_buf);
> -	print_debug(": ");
> +	print_debug_dqs_pair("\t\t\t\t\t\t  test_buf = ", (u32)test_buf, "  |  addr_lo = ", addr,  4);
>  	for (i=0; i<8; i++) {
> -		print_debug_hex8(test_buf[i]); print_debug(" ");
> -	}
> -	print_debug("\n");
> -
> -	print_debug("\t\t\t\t\t\tQW0 : addr_lo_buf = ");
> -	print_debug_hex32((unsigned)addr_lo_buf);
> -	print_debug(": ");
> -	for (i=0; i<8; i++) {
> -		print_debug_hex8(addr_lo_buf[i]); print_debug(" ");
> -	}
> -	print_debug("\n");
> -#endif
> +		value = read32_fs(addr);
> +		print_debug_dqs_pair("\t\t\t\t\t\t\t\t ", test_buf[i], "  |  ", value, 4);
>  
> -	/* prevent speculative execution of following instructions */
> -	_EXECFENCE;
> -
> -	for (i=0; i<8; i++) {
> -		if(addr_lo_buf[i] == test_buf[i]) {
> +		if (value == test_buf[i]) {
>  			pDCTstat->DqsRcvEn_Pass |= (1<<i);
>  		} else {
>  			pDCTstat->DqsRcvEn_Pass &= ~(1<<i);
>  		}
>  	}
>  
> +	result = DQS_FAIL;
>  
>  	if (Pass == FirstPass) {
>  		/* if first pass, at least one byte lane pass
> @@ -1062,7 +1041,7 @@
>  	Set_NB32_index_wait(dev, index_reg, 0x08, val);
>  
>  	/* Wait 200 MEMCLKs. */
> -	mct_Wait_10ns (20000);		/* wait 200us */
> +	mct_Wait(50000);		/* wait 200us */
>  
>  	/* Clear F2x[1,0]9C_x08[PhyFenceTrEn]=0. */
>  	val = Get_NB32_index_wait(dev, index_reg, 0x08);
> @@ -1101,21 +1080,20 @@
>  }
>  
>  
> -static void mct_Wait_10ns (u32 cycles)
> +static void mct_Wait(u32 cycles)
>  {
> -	u32 saved, i;
> +	u32 saved;
>  	u32 hi, lo, msr;
>  
> -	/* cycles = number of 10ns cycles(or longer) to delay */
> -	/* FIXME: Need to calibrate to CPU/NCLK speed? */
> +	/* Wait # of 50ns cycles
> +	   This seems like a hack to me...  */
>  
> -	msr = 0x10;				/* TSC */
> -	for (i = 0; i < cycles; i++) {
> -		_RDMSR(msr, &lo, &hi);
> -		saved = lo;
> +	cycles <<= 3;		/* x8 (number of 1.25ns ticks) */
>  
> -		do {
> -			_RDMSR(msr, &lo, &hi);
> -		} while (lo - saved < 8);	/* 8 x 1.25 ns as NCLK is  at 1.25ns */
> -	}
> +	msr = 0x10;			/* TSC */
> +	_RDMSR(msr, &lo, &hi);
> +	saved = lo;
> +	do {
> +		_RDMSR(msr, &lo, &hi);
> +	} while (lo - saved < cycles );
>  }
> Index: coreboot-v2/src/northbridge/amd/amdmct/wrappers/mcti_d.c
> ===================================================================
> --- coreboot-v2.orig/src/northbridge/amd/amdmct/wrappers/mcti_d.c	2008-04-07 15:56:37.000000000 -0600
> +++ coreboot-v2/src/northbridge/amd/amdmct/wrappers/mcti_d.c	2008-04-07 16:14:36.000000000 -0600
> @@ -1,7 +1,7 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -129,11 +129,11 @@
>  		//val = 1;	/* enable */
>  		break;
>  	case NV_BottomIO:
> -		val = 0xC0;	/* address bits [31:24] */
> +		val = 0xE0;	/* address bits [31:24] */
>  		break;
>  	case NV_BottomUMA:
>  #if (UMA_SUPPORT == 0)
> -		val = 0xC0;	/* address bits [31:24] */
> +		val = 0xE0;	/* address bits [31:24] */
>  #elif (UMA_SUPPORT == 1)
>  		val = 0xB0;	/* address bits [31:24] */
>  #endif
> @@ -205,7 +205,7 @@
>  		val = 1;		/* Enabled */
>  		//val = 0;	/* Disabled */
>  	case NV_ChannelIntlv:
> -		val = 5;	/* Disabled */ /* Not currently checked in mctchi_d.c */
> +		val = 5;	/* Not currently checked in mctchi_d.c */
>  	/* Bit 0 =     0 - Disable
>  	 *             1 - Enable
>  	 * Bits[2:1] = 00b - Address bits 6
> @@ -336,3 +336,8 @@
>  {
>  	return mctGetLogicalCPUID(node);
>  }
> +
> +u8 mctSetNodeBoundary_D(void)
> +{
> +	return 0;
> +}

Overall, I have to admit that I'm not familiar enough with the
memory controller to be able to perform a perfect review. However,
the patch seems to add comments where appropriate and the changes
look solid.

Regards,
Carl-Daniel





More information about the coreboot mailing list