[PATCH]: AMD FAM10 MCT: Channel B only.

[Bao, Zheng]

Without this patch, if we only got a DIMM in Channel B, memory can not
be
set up correctly. Now it can. Please test it.

Moving "mct_AfterGetCLT(pMCTstat, pDCTstat, dct);" out of the "if" is
the
key point. 
Changing the Get_DIMMAddress_D(pDCTstat, i) to
Get_DIMMAddress_D(pDCTstat, dct + i)
doesnt seem to take any effect. But I believe this is what it should be.

Zheng

Signed-off-by: Zheng Bao

Index: src/northbridge/amd/amdmct/mct/mct_d.c
===================================================================
--- src/northbridge/amd/amdmct/mct/mct_d.c	(revision 4561)
+++ src/northbridge/amd/amdmct/mct/mct_d.c	(working copy)
@@ -982,8 +982,8 @@
    	if ( mctGet_NVbits(NV_MCTUSRTMGMODE) == 2)
    		pDCTstat->Speed = mctGet_NVbits(NV_MemCkVal) +
1;

-		mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
    }
+	mct_AfterGetCLT(pMCTstat, pDCTstat, dct);

/* Gather all DIMM mini-max values for cycle timing data */
    Rows = 0;
@@ -1001,7 +1001,7 @@
    for ( i = 0; i< MAX_DIMMS_SUPPORTED; i++) {
    	LDIMM = i >> 1;
    	if (pDCTstat->DIMMValid & (1 << i)) {
-			smbaddr = Get_DIMMAddress_D(pDCTstat, i);
+			smbaddr = Get_DIMMAddress_D(pDCTstat, dct + i);
    		byte = mctRead_SPD(smbaddr, SPD_ROWSZ);
    		if (Rows < byte)
    			Rows = byte;	/* keep track of largest
row sz */

[Stefan Reinauer]

On 8/24/09 5:41 AM, Bao, Zheng wrote:
> Without this patch, if we only got a DIMM in Channel B, memory can not
> be
> set up correctly. Now it can. Please test it.
>
> Moving "mct_AfterGetCLT(pMCTstat, pDCTstat, dct);" out of the "if" is
> the
> key point. 
> Changing the Get_DIMMAddress_D(pDCTstat, i) to
> Get_DIMMAddress_D(pDCTstat, dct + i)
> doesnt seem to take any effect. But I believe this is what it should be.
>
> Zheng
>
> Signed-off-by: Zheng Bao

Awesome finding!

Acked-by: Stefan Reinauer
> Index: src/northbridge/amd/amdmct/mct/mct_d.c
>
> --- src/northbridge/amd/amdmct/mct/mct_d.c	(revision 4561)
> +++ src/northbridge/amd/amdmct/mct/mct_d.c	(working copy)
> @@ -982,8 +982,8 @@
>    	if ( mctGet_NVbits(NV_MCTUSRTMGMODE) == 2)
>    		pDCTstat->Speed = mctGet_NVbits(NV_MemCkVal) +
> 1;
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
> }
>
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
>
> /* Gather all DIMM mini-max values for cycle timing data */
> Rows = 0;
>
> @@ -1001,7 +1001,7 @@
>    for ( i = 0; i< MAX_DIMMS_SUPPORTED; i++) {
>    	LDIMM = i >> 1;
>    	if (pDCTstat->DIMMValid & (1 << i)) {
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, i);
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, dct + i);
> byte = mctRead_SPD(smbaddr, SPD_ROWSZ);
> if (Rows < byte)
> 	Rows = byte;	/* keep track of largest
>
> row sz */

[Bao, Zheng]

Have you guys tried 2 different types of DIMM on Channel A/B respectively? By "different", I mean "very different, totally different". Maybe one is Single-Rank, while the other is Dual-Rank.

Zheng

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Monday, August 24, 2009 6:01 PM
To: Bao, Zheng
Cc: Coreboot
Subject: Re: [coreboot] [PATCH]: AMD FAM10 MCT: Channel B only.

On 8/24/09 5:41 AM, Bao, Zheng wrote:
> Without this patch, if we only got a DIMM in Channel B, memory can not
> be
> set up correctly. Now it can. Please test it.
>
> Moving "mct_AfterGetCLT(pMCTstat, pDCTstat, dct);" out of the "if" is
> the
> key point. 
> Changing the Get_DIMMAddress_D(pDCTstat, i) to
> Get_DIMMAddress_D(pDCTstat, dct + i)
> doesnt seem to take any effect. But I believe this is what it should be.
>
> Zheng
>
> Signed-off-by: Zheng Bao

Awesome finding!

Acked-by: Stefan Reinauer
> Index: src/northbridge/amd/amdmct/mct/mct_d.c
>
> --- src/northbridge/amd/amdmct/mct/mct_d.c	(revision 4561)
> +++ src/northbridge/amd/amdmct/mct/mct_d.c	(working copy)
> @@ -982,8 +982,8 @@
>    	if ( mctGet_NVbits(NV_MCTUSRTMGMODE) == 2)
>    		pDCTstat->Speed = mctGet_NVbits(NV_MemCkVal) +
> 1;
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
> }
>
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
>
> /* Gather all DIMM mini-max values for cycle timing data */
> Rows = 0;
>
> @@ -1001,7 +1001,7 @@
>    for ( i = 0; i< MAX_DIMMS_SUPPORTED; i++) {
>    	LDIMM = i >> 1;
>    	if (pDCTstat->DIMMValid & (1 << i)) {
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, i);
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, dct + i);
> byte = mctRead_SPD(smbaddr, SPD_ROWSZ);
> if (Rows < byte)
> 	Rows = byte;	/* keep track of largest
>
> row sz */

[Bao, Zheng]

Committed, r4569.
And the extra semicolon is removed.

-----Original Message-----
From: Stefan Reinauer [mailto:[e-mail]] 
Sent: Monday, August 24, 2009 6:01 PM
To: Bao, Zheng
Cc: Coreboot
Subject: Re: [coreboot] [PATCH]: AMD FAM10 MCT: Channel B only.

On 8/24/09 5:41 AM, Bao, Zheng wrote:
> Without this patch, if we only got a DIMM in Channel B, memory can not
> be
> set up correctly. Now it can. Please test it.
>
> Moving "mct_AfterGetCLT(pMCTstat, pDCTstat, dct);" out of the "if" is
> the
> key point. 
> Changing the Get_DIMMAddress_D(pDCTstat, i) to
> Get_DIMMAddress_D(pDCTstat, dct + i)
> doesnt seem to take any effect. But I believe this is what it should be.
>
> Zheng
>
> Signed-off-by: Zheng Bao

Awesome finding!

Acked-by: Stefan Reinauer
> Index: src/northbridge/amd/amdmct/mct/mct_d.c
>
> --- src/northbridge/amd/amdmct/mct/mct_d.c	(revision 4561)
> +++ src/northbridge/amd/amdmct/mct/mct_d.c	(working copy)
> @@ -982,8 +982,8 @@
>    	if ( mctGet_NVbits(NV_MCTUSRTMGMODE) == 2)
>    		pDCTstat->Speed = mctGet_NVbits(NV_MemCkVal) +
> 1;
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
> }
>
> mct_AfterGetCLT(pMCTstat, pDCTstat, dct);
>
> /* Gather all DIMM mini-max values for cycle timing data */
> Rows = 0;
>
> @@ -1001,7 +1001,7 @@
>    for ( i = 0; i< MAX_DIMMS_SUPPORTED; i++) {
>    	LDIMM = i >> 1;
>    	if (pDCTstat->DIMMValid & (1 << i)) {
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, i);
> 	smbaddr = Get_DIMMAddress_D(pDCTstat, dct + i);
> byte = mctRead_SPD(smbaddr, SPD_ROWSZ);
> if (Rows < byte)
> 	Rows = byte;	/* keep track of largest
>
> row sz */