[LinuxBIOS] [PATCH] Fix the AMD K8 DDR init

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sat Sep 29 03:10:44 CEST 2007


Hi,

[you managed to trigger a bug in my Seamonkey mail client with your
mail. The mail contents started scrolling upward automatically and
reproducibly. Nice.]

On 29.09.2007 02:10, Rudolf Marek wrote:
> Hello,
> 
> This patch changes the "if else" style of parameter matching to table and also
> changes the rdpreamble parameter, which will cause that more then one DIMM will
> work for 939 motherboard.
> 
> What about the 2T support?
> 
> 
> Please take a look to a patch and will prepare some more final version.
> 
> Rudolf

No comments about the actual conversion, but some hints about coding style.

Carl-Daniel

> 
> 
> ------------------------------------------------------------------------
> 
> Index: src/northbridge/amd/amdk8/raminit.c
> ===================================================================
> --- src/northbridge/amd/amdk8/raminit.c	(revision 2776)
> +++ src/northbridge/amd/amdk8/raminit.c	(working copy)
> @@ -1177,6 +1177,7 @@
>  	int i;
>  	int registered;
>  	int unbuffered;
> +	int has_dualch = is_opteron(ctrl);
>  	uint32_t dcl;
>  	unbuffered = 0;
>  	registered = 0;
> @@ -1201,17 +1202,22 @@
>  	if (unbuffered && registered) {
>  		die("Mixed buffered and registered dimms not supported");
>  	}
> -#if 1
> -	// yhlu debug: Athlon64 939 can do dual channel, but it uses unbuffered DIMMs
> -	if (unbuffered && is_opteron(ctrl)) {
> -		die("Unbuffered Dimms not supported on Opteron");
> -	}
> -#endif
>  
>  	dcl = pci_read_config32(ctrl->f2, DRAM_CONFIG_LOW);
>  	dcl &= ~DCL_UnBufDimm;
>  	if (unbuffered) {
> -		dcl |= DCL_UnBufDimm;
> +		if ((has_dualch) && (!is_cpu_pre_d0())) {
> +			dcl |= DCL_UnBufDimm; // | DCL_DualDIMMen;
> +			
> +			// | DCL_En2T; SET if you have non-equal DDR mem types
> +			
> +			if ((cpuid_eax(1) & 0x30) == 0x30) {
> +				/* CS[7:4] is copy of CS[3:0], should be set for 939 socket */
> +				dcl |= DCL_UpperCSMap;
> +			}
> +		} else {
> +			dcl |= DCL_UnBufDimm;
> +		}
>  	}
>  	pci_write_config32(ctrl->f2, DRAM_CONFIG_LOW, dcl);
>  #if 0
> @@ -1333,6 +1339,9 @@
>  	uint32_t dch_memclk;
>  	uint16_t dch_tref4k, dch_tref8k;
>  	uint8_t	 dtl_twr;
> +	uint8_t	 dtl_twtr;
> +	uint8_t  dtl_trwt[3][3]; /* first index is CAS_LAT 2/2.5/3 and 128/registered64/64 */
> + 	uint8_t	 rdpreamble[4]; /* 0 is for registered, 1 for 1-2 DIMMS, 2 and 3 for 3 or 4 unreg dimm slots */
>  	char name[9];
>  };
>  
> @@ -1349,6 +1358,9 @@
>  			.dch_tref4k = DTH_TREF_100MHZ_4K,
>  			.dch_tref8k = DTH_TREF_100MHZ_8K,
>  			.dtl_twr    = 2,
> +			.dtl_twtr   = 1,
> +			.dtl_trwt   = { { 2, 2, 3 }, { 3, 3, 4 }, { 3, 3, 4 }},
> +			.rdpreamble = { ((9 << 1) + 0), ((9 << 1) + 0), ((9 << 1) + 0), ((9 << 1) + 0) }
>  		},
>  		{
>  			.name	    = "133Mhz\r\n",
> @@ -1360,6 +1372,9 @@
>  			.dch_tref4k = DTH_TREF_133MHZ_4K,
>  			.dch_tref8k = DTH_TREF_133MHZ_8K,
>  			.dtl_twr    = 2,
> +			.dtl_twtr   = 1,
> +			.dtl_trwt   = { { 2, 2, 3 }, { 3, 3, 4 }, { 3, 3, 4 }},
> +			.rdpreamble = { ((8 << 1) + 0), ((7 << 1) + 0), ((7 << 1) + 1), ((7 << 1) + 0) }
>  		},
>  		{
>  			.name	    = "166Mhz\r\n",
> @@ -1371,6 +1386,9 @@
>  			.dch_tref4k = DTH_TREF_166MHZ_4K,
>  			.dch_tref8k = DTH_TREF_166MHZ_8K,
>  			.dtl_twr    = 3,
> +			.dtl_twtr   = 1,
> +			.dtl_trwt   = { { 3, 2, 3 }, { 3, 3, 4 }, { 4, 3, 4 }},
> +			.rdpreamble = { ((7 << 1) + 1), ((6 << 1) + 0), ((6 << 1) + 1), ((6 << 1) + 0) }
>  		},
>  		{
>  			.name	    = "200Mhz\r\n",
> @@ -1382,6 +1400,9 @@
>  			.dch_tref4k = DTH_TREF_200MHZ_4K,
>  			.dch_tref8k = DTH_TREF_200MHZ_8K,
>  			.dtl_twr    = 3,
> +			.dtl_twtr   = 2,
> +			.dtl_trwt   = { { 0, 2, 3 }, { 3, 3, 4 }, { 3, 3, 4 }},
> +			.rdpreamble = { ((7 << 1) + 0), ((5 << 1) + 0), ((5 << 1) + 1), ((5 << 1) + 1) }
>  		},
>  		{
>  			.cycle_time = 0x00,
> @@ -1397,9 +1418,9 @@
>  		die("min_cycle_time to low");
>  	}
>  	print_spew(param->name);
> -#ifdef DRAM_MIN_CYCLE_TIME
> +//#ifdef DRAM_MIN_CYCLE_TIME
>  	print_debug(param->name);
> -#endif
> +//#endif

Please remove the ifdef completely and also remove the print_spew above
the ifdef. No need to print stuff twice. OTOH, you could remove the
print_debug instead. Your decision.

>  	return param;
>  }
>  
> @@ -1425,6 +1446,10 @@
>  
>  
>  	value = pci_read_config32(ctrl->f3, NORTHBRIDGE_CAP);
> +	print_debug("NB CAP REG:");
> +	print_debug_hex32(value);
> +	print_debug("\n");
> +
>  	min_cycle_time = min_cycle_times[(value >> NBCAP_MEMCLK_SHIFT) & NBCAP_MEMCLK_MASK];
>  	bios_cycle_time = min_cycle_times[
>  		read_option(CMOS_VSTART_max_mem_clock, CMOS_VLEN_max_mem_clock, 0)];
> @@ -1877,75 +1902,47 @@
>  static void set_Twtr(const struct mem_controller *ctrl, const struct mem_param *param)
>  {
>  	uint32_t dth;
> -	unsigned clocks;
> -	clocks = 1; /* AMD says hard code this */
> +
>  	dth = pci_read_config32(ctrl->f2, DRAM_TIMING_HIGH);
>  	dth &= ~(DTH_TWTR_MASK << DTH_TWTR_SHIFT);
> -	dth |= ((clocks - DTH_TWTR_BASE) << DTH_TWTR_SHIFT);
> +	dth |= ((param->dtl_twtr - DTH_TWTR_BASE) << DTH_TWTR_SHIFT);
>  	pci_write_config32(ctrl->f2, DRAM_TIMING_HIGH, dth);
>  }
>  
>  static void set_Trwt(const struct mem_controller *ctrl, const struct mem_param *param)
>  {
>  	uint32_t dth, dtl;
> -	unsigned divisor;
>  	unsigned latency;
>  	unsigned clocks;
> +	int lat, mtype;
>  
>  	clocks = 0;
>  	dtl = pci_read_config32(ctrl->f2, DRAM_TIMING_LOW);
>  	latency = (dtl >> DTL_TCL_SHIFT) & DTL_TCL_MASK;
> -	divisor = param->divisor;
>  
>  	if (is_opteron(ctrl)) {
> -		if (latency == DTL_CL_2) {
> -			if (divisor == ((6 << 0) + 0)) {
> -				/* 166Mhz */
> -				clocks = 3;
> -			}
> -			else if (divisor > ((6 << 0)+0)) {
> -				/* 100Mhz && 133Mhz */
> -				clocks = 2;
> -			}
> -		}
> -		else if (latency == DTL_CL_2_5) {
> -			clocks = 3;
> -		}
> -		else if (latency == DTL_CL_3) {
> -			if (divisor == ((6 << 0)+0)) {
> -				/* 166Mhz */
> -				clocks = 4;
> -			}
> -			else if (divisor > ((6 << 0)+0)) {
> -				/* 100Mhz && 133Mhz */
> -				clocks = 3;
> -			}
> -		}
> +		mtype = 0; /* dual channel */
> +	} else if (is_registered(ctrl)) {
> +		mtype = 1; /* registered 64bit interface */
> +	} else {
> +		mtype = 2; /* unbuffered 64bit interface */
>  	}
> -	else /* Athlon64 */ {
> -		if (is_registered(ctrl)) {
> -			if (latency == DTL_CL_2) {
> -				clocks = 2;
> -			}
> -			else if (latency == DTL_CL_2_5) {
> -				clocks = 3;
> -			}
> -			else if (latency == DTL_CL_3) {
> -				clocks = 3;
> -			}
> -		}
> -		else /* Unbuffered */{
> -			if (latency == DTL_CL_2) {
> -				clocks = 3;
> -			}
> -			else if (latency == DTL_CL_2_5) {
> -				clocks = 4;
> -			}
> -			else if (latency == DTL_CL_3) {
> -				clocks = 4;
> -			}
> -		}
> +
> +	switch (latency) {
> +		case DTL_CL_2:
> +			lat = 0;
> +			break;
> +		case DTL_CL_2_5:
> +			lat = 1;
> +			break;
> +		case DTL_CL_3:
> +			lat = 2;
> +			break;
> +		default:
> +			die("Unknown LAT for Trwt");
>  	}
> +
> +	clocks = param->dtl_trwt[lat][mtype];
>  	if ((clocks < DTH_TRWT_MIN) || (clocks > DTH_TRWT_MAX)) {
>  		die("Unknown Trwt\r\n");
>  	}
> @@ -1977,85 +1974,44 @@
>  static void set_read_preamble(const struct mem_controller *ctrl, const struct mem_param *param)
>  {
>  	uint32_t dch;
> -	unsigned divisor;
>  	unsigned rdpreamble;
> -	divisor = param->divisor;
> -	dch = pci_read_config32(ctrl->f2, DRAM_CONFIG_HIGH);
> -	dch &= ~(DCH_RDPREAMBLE_MASK << DCH_RDPREAMBLE_SHIFT);
> -	rdpreamble = 0;
> -	if (is_registered(ctrl)) {
> -		if (divisor == ((10 << 1)+0)) {
> -			/* 100Mhz, 9ns */
> -			rdpreamble = ((9 << 1)+ 0);
> +	int slots, i;
> +
> +	slots = 0;
> +
> +	for(i = 0; i < 4; i++) {
> +		if (ctrl->channel0[i]) {
> +			slots += 1;
>  		}
> -		else if (divisor == ((7 << 1)+1)) {
> -			/* 133Mhz, 8ns */
> -			rdpreamble = ((8 << 1)+0);
> -		}
> -		else if (divisor == ((6 << 1)+0)) {
> -			/* 166Mhz, 7.5ns */
> -			rdpreamble = ((7 << 1)+1);
> -		}
> -		else if (divisor == ((5 << 1)+0)) {
> -			/* 200Mhz,  7ns */
> -			rdpreamble = ((7 << 1)+0);
> -		}
>  	}
> -	else {
> -		int slots;
> -		int i;
> -		slots = 0;
> -		for(i = 0; i < 4; i++) {
> -			if (ctrl->channel0[i]) {
> -				slots += 1;
> -			}
> -		}
> -		if (divisor == ((10 << 1)+0)) {
> -			/* 100Mhz */
> -			if (slots <= 2) {
> -				/* 9ns */
> -				rdpreamble = ((9 << 1)+0);
> -			} else {
> -				/* 14ns */
> -				rdpreamble = ((14 << 1)+0);
> -			}
> -		}
> -		else if (divisor == ((7 << 1)+1)) {
> -			/* 133Mhz */
> -			if (slots <= 2) {
> -				/* 7ns */
> -				rdpreamble = ((7 << 1)+0);
> -			} else {
> -				/* 11 ns */
> -				rdpreamble = ((11 << 1)+0);
> -			}
> -		}
> -		else if (divisor == ((6 << 1)+0)) {
> -			/* 166Mhz */
> -			if (slots <= 2) {
> -				/* 6ns */
> -				rdpreamble = ((7 << 1)+0);
> -			} else {
> -				/* 9ns */
> -				rdpreamble = ((9 << 1)+0);
> -			}
> -		}
> -		else if (divisor == ((5 << 1)+0)) {
> -			/* 200Mhz */
> -			if (slots <= 2) {
> -				/* 5ns */
> -				rdpreamble = ((5 << 1)+0);
> -			} else {
> -				/* 7ns */
> -				rdpreamble = ((7 << 1)+0);
> -			}
> -		}
> +
> +	/* map to index to param.rdpreamble array */
> +	if (is_registered(ctrl)) {
> +		i = 0;
> +	} else if (slots < 3) {
> +		i = 1;
> +	} else if (slots == 3) {
> +		i = 2;
> +	} else if (slots == 4) {
> +		i = 3;
> +	} else {
> +		die("Unknown rdpreamble for this nr of slots");
>  	}
> +
> +	dch = pci_read_config32(ctrl->f2, DRAM_CONFIG_HIGH);
> +	dch &= ~(DCH_RDPREAMBLE_MASK << DCH_RDPREAMBLE_SHIFT);
> +	rdpreamble = param->rdpreamble[i];
> +
>  	if ((rdpreamble < DCH_RDPREAMBLE_MIN) || (rdpreamble > DCH_RDPREAMBLE_MAX)) {
>  		die("Unknown rdpreamble");
>  	}
> +
>  	dch |= (rdpreamble - DCH_RDPREAMBLE_BASE) << DCH_RDPREAMBLE_SHIFT;
>  	pci_write_config32(ctrl->f2, DRAM_CONFIG_HIGH, dch);
> +
> +	print_err("RDPREAMBLE: \n");
> +	print_err_hex16(rdpreamble);
> +	print_err("\n");
>  }
>  
>  static void set_max_async_latency(const struct mem_controller *ctrl, const struct mem_param *param)
> Index: src/northbridge/amd/amdk8/amdk8.h
> ===================================================================
> --- src/northbridge/amd/amdk8/amdk8.h	(revision 2776)
> +++ src/northbridge/amd/amdk8/amdk8.h	(working copy)
> @@ -134,7 +134,9 @@
>  #define	 DCL_D_DRV	   (1<<1)
>  #define	 DCL_QFC_EN	   (1<<2)
>  #define	 DCL_DisDqsHys	   (1<<3)
> +#define  DCL_Burst2Opt     (1<<5)

tabs vs. spaces.

>  #define	 DCL_DramInit	   (1<<8)
> +#define  DCL_DualDIMMen    (1<<9)

same here.

>  #define	 DCL_DramEnable	   (1<<10)
>  #define	 DCL_MemClrStatus  (1<<11)
>  #define	 DCL_ESR	   (1<<12)
> @@ -147,7 +149,8 @@
>  #define	 DCL_DisInRcvrs    (1<<24)
>  #define	 DCL_BypMax_SHIFT  25
>  #define	 DCL_En2T          (1<<28)
> -
> +#define  DCL_UpperCSMap    (1<<29)

same here.

> +	
>  #define DRAM_CONFIG_HIGH   0x94
>  #define	 DCH_ASYNC_LAT_SHIFT  0
>  #define	 DCH_ASYNC_LAT_MASK   0xf
> 




More information about the coreboot mailing list