[PATCH] Implement DIMM loading for DDR on K8

[Rudolf Marek]

Hello,

Attached patch implements the memory speed reductions (and 2T/1T clock logic) 
for DDR1 memory (939 sockets). The details can be found in BKDG chapter 4.1.3.3.

The patch looks at certain DDR configurations (dual rank/single rank) and lowers 
the clocks to 2T or frequency as guide suggest. It sets the DualDIMMen bit which 
I believe should be set for non-dual channel configs.

Dual DIMM Enable (DualDimmEn)—Bit 9. When this bit is set, the A copy of the 
memory address bus is enabled, regardless of the MC0_EN (Function 2, Offset 94h) 
value, and the B copy of the memory bus is disabled if 939 package with 128-bit 
bus is not used. This bit should be set if unbuffered DIMMs are used, and two 
DIMM sockets are connected to the A copy of the memory address bus, as in SODIMM 
or 939 package configurations. See “Register Differences in Revisions of the AMD 
AthlonTM 64.

The patch does not implement support for three dimm configurations supported 
from revE.

On the other hand it should improve greatly memory stability across the 939 
platform.

Signed-off-by: Rudolf Marek

Thanks,
Rudolf

[Stefan Reinauer]

* Rudolf Marek  [101211 22:37]:
> Hello,
>
> Attached patch implements the memory speed reductions (and 2T/1T
> clock logic) for DDR1 memory (939 sockets). The details can be found
> in BKDG chapter 4.1.3.3.
>
> The patch looks at certain DDR configurations (dual rank/single
> rank) and lowers the clocks to 2T or frequency as guide suggest. It
> sets the DualDIMMen bit which I believe should be set for non-dual
> channel configs.
>
> Dual DIMM Enable (DualDimmEn)—Bit 9. When this bit is set, the A
> copy of the memory address bus is enabled, regardless of the MC0_EN
> (Function 2, Offset 94h) value, and the B copy of the memory bus is
> disabled if 939 package with 128-bit bus is not used. This bit
> should be set if unbuffered DIMMs are used, and two DIMM sockets are
> connected to the A copy of the memory address bus, as in SODIMM or
> 939 package configurations. See “Register Differences in Revisions
> of the AMD AthlonTM 64.
>
> The patch does not implement support for three dimm configurations
> supported from revE.
>
> On the other hand it should improve greatly memory stability across
> the 939 platform.
>
> Signed-off-by: Rudolf Marek
>
> Thanks,
> Rudolf

Wow, nice finding. Assuming it's abuild tested and tested on real
hardware:
Acked-by: Stefan Reinauer

Stefan

[Rudolf Marek]

Hi,

Yes it is tested on real HW. Attaching refreshed patch because

#if CONFIG_ ... SOCKET939 == 1

#endif

Is undefined, so I changed that to

#if defined(....)

(After abuild test ;)

Thanks,
Rudolf

[Stefan Reinauer]

* Rudolf Marek  [101212 01:10]:
> Hi,
>
> Yes it is tested on real HW. Attaching refreshed patch because
>
> #if CONFIG_ ... SOCKET939 == 1
>
> #endif
>
> Is undefined, so I changed that to
>
> #if defined(....)

that's not good enough i think. It might be 0, then defined would hit
even though it should not.

Stefan

[Peter Stuge]

Rudolf Marek wrote:
> +/*
> Following table comes directly from BKDG (unbuffered DIMM support)
> [Y][X] Y = ch0_0, ch1_0, ch0_1, ch1_1 1=present 0=empty
>  X uses same layout but 1 means double rank 0 is single rank/empty
>
> +*/

Maybe clarify that ch{0_0,1_0,0_1,1_1} maps to MEMCS_{1L,1H,2L,2H} in
the PDF. Maybe also mention that preE is table 45, and revE table 46.
> static const unsigned char dimm_loading_config_preE[16][16] = {
> [0x8] = {[0x0] = DDR400,[0x8] = DDR400},
> [0x2] = {[0x0] = DDR333,[0x2] = DDR400},
> [0xa] = {[0x0] = DDR400_2T,[0x2] = DDR400_2T,
> 	 [0x8] = DDR400_2T,[0xa] = DDR333_2T},
> [0xc] = {[0x0] = DDR400,[0xc] = DDR400},
> [0x3] = {[0x0] = DDR333,[0x3] = DDR400},
> [0xf] = {[0x0] = DDR400_2T,[0x3] = DDR400_2T,
> 	 [0xc] = DDR400_2T,[0xf] = DDR333_2T},
> };

This matches the PDF.
> static const unsigned char dimm_loading_config_revE[16][16] = {
> [0x8] = {[0x0] = DDR400, [0x8] = DDR400},
> [0x2] = {[0x0] = DDR333, [0x2] = DDR400},
> [0x4] = {[0x0] = DDR400, [0x4] = DDR400},
> [0x1] = {[0x0] = DDR333, [0x1] = DDR400},
> [0xa] = {[0x0] = DDR400_2T, [0x2] = DDR400_2T,
> 	 [0x8] = DDR400_2T, [0xa] = DDR333_2T},
> [0x5] = {[0x0] = DDR400_2T, [0x1] = DDR400_2T,
> 	 [0x4] = DDR400_2T, [0x5] = DDR333_2T},
> [0xc] = {[0x0] = DDR400, [0xc] = DDR400, [0x4] = DDR400, [0x8] = DDR400},
> [0x3] = {[0x0] = DDR333, [0x1] = DDR333, [0x2] = DDR333, [0x3] = DDR400},
> [0xe] = {[0x0] = DDR400_2T, [0x4] = DDR400_2T, [0x2] = DDR400_2T,
> 	 [0x6] = DDR400_2T, [0x8] = DDR400_2T, [0xc] = DDR400_2T,
> 	 [0xa] = DDR333_2T, [0xe] = DDR333_2T},
> [0xb] = {[0x0] = DDR333, [0x1] = DDR400_2T, [0x2] = DDR333_2T,
> 	 [0x3] = DDR400_2T, [0x8] = DDR333_2T, [0x9] = DDR400_2T,
> 	 [0xa] = DDR333_2T, [0xb] = DDR333_2T},
> [0xd] = {[0x0] = DDR400_2T, [0x8] = DDR400_2T, [0x1] = DDR400_2T,
> 	 [0x9] = DDR333_2T, [0x4] = DDR400_2T, [0xc] = DDR400_2T,
> 	 [0x5] = DDR333_2T, [0xd] = DDR333_2T},
> [0x7] = {[0x0] = DDR333_2T, [0x2] = DDR400_2T, [0x1] = DDR333_2T,

7,0 above could be DDR333, ie use 1T.
> 	 [0x3] = DDR400_2T, [0x4] = DDR333_2T, [0x6] = DDR400_2T,
> 	 [0x5] = DDR333_2T, [0x7] = DDR333_2T},
> [0xf] = {[0x0] = DDR400_2T, [0x1] = DDR400_2T, [0x4] = DDR400_2T,
> 	 [0x5] = DDR333_2T, [0x2] = DDR400_2T, [0x3] = DDR400_2T,
> 	 [0x6] = DDR400_2T, [0x7] = DDR333_2T, [0x8] = DDR400_2T,
> 	 [0x9] = DDR400_2T, [0xc] = DDR400_2T, [0xd] = DDR333_2T,
> 	 [0xa] = DDR333_2T, [0xb] = DDR333_2T, [0xe] = DDR333_2T,
> 	 [0xf] = DDR333_2T},
> };

The 26094 BKDF PDF refers to "the AMD Athlon™ 64 939 Processor
Motherboard Design Guide, order# 30474" in some of the memory
configuration cases.

It seems that 30474 isn't publically available, maybe someone has
access to it and could check if we should take special considerations
for those cases in coreboot, maybe to choose a lower clock speed to
guarantee reliability?

Acked-by: Peter Stuge

[Rudolf Marek]

Hi all,

I fixed the comments and fixed the #if sections. Thanks for input commited as 
6176 (and as you noticed all my pending patches too ;)

Thanks,
Rudolf