RE: [LinuxBIOS] E7501 Raminit rewrite

[Lu, Yinghai]

Good, I suggest use
spd_ddr.h
spd_ddr2.h
spd_ddr3.h
spd_fbdimm.h
....

YH

-----Original Message-----
From: Steven J. Magnani [mailto:[e-mail]] 
Sent: Tuesday, November 01, 2005 10:09 AM
To: Lu, Yinghai; [e-mail]
Subject: RE: [LinuxBIOS] E7501 Raminit rewrite

Sorry it's taken so long to respond to this.

I propose keeping SPD_xxx for byte addresses that are common to DDR and
DDR2, and using SPD_DDR_xxx and SPD_DDR2_xxx for addresses that are
defined differently or are unique to DDR and DDR2. Since in many cases,
JEDEC kept the same byte address description for DDR2 but changed the
interpretation of the bits, I also propose qualifying all of the
field/bit-definitions with DDR_ or DDR2_, even if the interpretation
happens to be the same. The only exception would be the memory type
values, which have to be common to all memory types.

The reason for keeping SPD_xxx for byte addresses common to DDR and DDR2
is that code that supports both (i.e. E7520) might get confusing to read
if we didn't do this. For example, that code reads parameters like the
number of columns only once, so calling the byte address
SPD_DDR_NUM_COLUMNS or SPD_DDR2_NUM_COLUMNS

Does this sound like a reasonable compromise? The spd.h file would
become something like this:

#ifndef __SPD_H_DEFINED
#define __SPD_H_DEFINED

// Byte numbers
    // (Essentially) common to DDR & DDR2
#define SPD_MEMORY_TYPE                          2
#define SPD_NUM_ROWS                             3
#define SPD_NUM_COLUMNS                          4
#define SPD_MODULE_VOLTAGE                       8
#define SPD_MIN_CYCLE_TIME_AT_CAS_MAX            9
#define SPD_DIMM_CONFIG_TYPE                    11
#define SPD_REFRESH                             12
#define SPD_PRIMARY_DRAM_WIDTH                  13
#define SPD_SUPPORTED_BURST_LENGTHS             16
#define SPD_NUM_BANKS_PER_DRAM                  17
#define SPD_ACCEPTABLE_CAS_LATENCIES            18
#define SPD_MODULE_ATTRIBUTES                   21
#define SPD_MIN_ROW_PRECHARGE_TIME              27
#define SPD_MIN_ROW_ACTIVE_DELAY                28
#define SPD_MIN_RAS_TO_CAS_DELAY                29
#define SPD_MIN_ACTIVE_TO_PRECHARGE_DELAY       30
#define SPD_ADDRESS_CMD_HOLD                    33
#define SPD_MIN_ACTIVE_TO_ACTIVE_TIME           41
#define SPD_MIN_AUTO_REFRESH_CMD_PERIOD         42

// DDR-specific
#define SPD_DDR_NUM_DIMM_BANKS                   5
#define SPD_DDR_MODULE_DATA_WIDTH_LSB            6
#define SPD_DDR_MODULE_DATA_WIDTH_MSB            7
#define SPD_DDR_MIN_CYCLE_TIME_AT_CAS_REDUCED_05    23
#define SPD_DDR_MIN_CYCLE_TIME_AT_CAS_REDUCED_10    25

// DDR2-specific
#define SPD_DDR2_NUM_DIMM_RANKS                  5
#define SPD_DDR2_MODULE_DATA_WIDTH               6
#define SPD_DDR2_MIN_CYCLE_TIME_AT_CAS_REDUCED_1 23
#define SPD_DDR2_MIN_CYCLE_TIME_AT_CAS_REDUCED_2 25

/****************************************************/
// SPD_MEMORY_TYPE values
#define MEMORY_TYPE_SDRAM_DDR   7
#define MEMORY_TYPE_DDR2_SDRAM  8

// SPD_MODULE_VOLTAGE values
#define DDR_VOLTAGE_SSTL2_5     4
#define DDR2_VOLTAGE_SSTL2_5    4

// SPD_DIMM_CONFIG_TYPE values
#define DDR_ERROR_SCHEME_NONE       0
#define DDR_ERROR_SCHEME_PARITY     1
#define DDR_ERROR_SCHEME_ECC        2
#define DDR2_DATA_PARITY        (1<<0)
#define DDR2_DATA_ECC           (1<<1)
#define DDR2_ADDRESS_PARITY     (1<<2)

// SPD_ACCEPTABLE_CAS_LATENCIES values
#define DDR_CAS_LATENCY_1_0     0x01
#define DDR_CAS_LATENCY_1_5     0x02
#define DDR_CAS_LATENCY_2_0     0x04
#define DDR_CAS_LATENCY_2_5     0x08
#define DDR_CAS_LATENCY_3_0     0x10
#define DDR_CAS_LATENCY_3_5     0x20
#define DDR_CAS_LATENCY_4_0     0x40
#define DDR2_CAS_LATENCY_2      (1<<2)
#define DDR2_CAS_LATENCY_3      (1<<3)
#define DDR2_CAS_LATENCY_4      (1<<4)
#define DDR2_CAS_LATENCY_5      (1<<5)
#define DDR2_CAS_LATENCY_6      (1<<6)

// SPD_SUPPORTED_BURST_LENGTHS values
#define DDR_BURST_LENGTH_1      1
#define DDR_BURST_LENGTH_2      2
#define DDR_BURST_LENGTH_4      4
#define DDR_BURST_LENGTH_8      8
#define DDR_BURST_LENGTH_PAGE   (1<<7)
#define DDR2_BURST_LENGTH_4     4
#define DDR2_BURST_LENGTH_8     8

// SPD_MODULE_ATTRIBUTES values
#define DDR_MODULE_BUFFERED     1
#define DDR_MODULE_REGISTERED   2

#endif  // __SPD_H_DEFINED

It's a sticky problem, since undoubtedly there will be more changes when
the next DRAM technology is standardized, and (theoretically, at least)
there can be differences in definitions even between revisions of the
standard for a particular memory type. Hopefully a scheme like this will
work for awhile, even if it isn't perfect.

------------------------------------------------------------------------
 Steven J. Magnani               "I claim this network for MARS!
 www.digidescorp.com              Earthling, return my space modulator!"

#include <standard.disclaimer>

-----Original Message-----
From: Lu, Yinghai [mailto:[e-mail]] 
Sent: Wednesday, September 14, 2005 1:11 PM
To: Steven J. Magnani; [e-mail]
Subject: RE: [LinuxBIOS] E7501 Raminit rewrite

Every define have SPD_DDR_ prefix,
or with ddr_spd.h and SPD_ prefix....

YH

-----Original Message-----
From: Steven J. Magnani [mailto:[e-mail]] 
Sent: Wednesday, September 14, 2005 11:06 AM
To: Lu, Yinghai; [e-mail]
Subject: RE: [LinuxBIOS] E7501 Raminit rewrite

Can you be more specific about the changes you'd like to see?

Steve

-----Original Message-----
From: Lu, Yinghai [mailto:[e-mail]] 
Sent: Tuesday, September 13, 2005 10:27 AM
To: Steven J. Magnani; [e-mail]
Subject: RE: [LinuxBIOS] E7501 Raminit rewrite

It's good to have spd.h there.

But the name should be changed for support DDR2...etc.

YH

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Steven J. Magnani
Sent: Tuesday, September 13, 2005 8:08 AM
To: [e-mail]
Subject: [LinuxBIOS] E7501 Raminit rewrite

This one ends up being a little more than a "patch".
I ended up reorganizing the E7501 raminit code so I could follow it, and
fixed some bugs along the way.

Significant logical changes:

* Added support for "fast" (64-clock) refresh
* Added code to support remap window for 3 - 4 GB systems
* Fixed premature configuration of true row boundaries that resulted in
some sections of DRAM not receiving JEDEC commands (see
http://openbios.org/pipermail/linuxbios/2005-June/011752.html).
* Redefined RCOMP_MMIO so that RCOMP registers can be configured on
systems where A20M# is asserted.
* Disabled subsystem (vendor) ID configuration
* #ifdef'd out suspicious looking code (see
http://openbios.org/pipermail/linuxbios/2005-June/011759.html)
* Added optional run-time checking of dual-channel compatibility of
installed DIMMs 
* Move JEDEC SPD and SDRAM definitions into reusable #include files

Probably, the rewritten code should be reviewed as if it were new. My
hope is that it is now easy enough to follow that this shouldn't be too
bad.

The original code is, of course, in Subversion. Since I reordered the
functions as part of the rewrite, a 'diff' of of the new code against
the original won't tell you much. I've attached a reordered copy of the
Subversion code that won't compile, but should be a little easier to
diff against the new code so you can see what's changed.

I will hold off on committing this for awhile to give people time to
look at it. I know that radical changes are a pain to review, but IMHO
the improvement in maintainability is significant.

New files that are part of this 'patch', but not included here (I
committed these to Subversion): src/include/spd.h
src/include/sdram_mode.h src/include/northbridge/intel/e7501/e7501.h

Thanks,
Steve

------------------------------------------------------------------------
 Steven J. Magnani               "I claim this network for MARS!
 www.digidescorp.com              Earthling, return my space modulator!"

#include <standard.disclaimer>