Ok, so I've been working on porting the SPD code, and this patch is what I've got so far. Here are some of the outstanding issues/changes:
* SPD register code (tested using PCI/SMBus Dumps) I ported the v1 assembly and made some modifications when the numbers didn't really add up on my machine (particularly the DRB registers) and added some minor functionality (CAS2/3 detection) that may or may not work, although based on the results from the dataset I used, it should work (at least for my current setup).
* PMCR - When should it be updated? Looking at the assembly, it seems as though it's ok to just set the final value before the RAM refresh code instead of waiting until afterwards, but I don't know for sure, so I left the original code alone. Judging from some of the recent 440bx discussions, it may not make much of a difference anyway.
* sdram_enable delays I changed all the RAM timing delays (tRP, tRC, tMRD) to 1usec, since the timings are on the order of hundreds of nanoseconds (according to SPD values) and the smallest resolution timer available seems to be udelay() anyway. It should work for any SDRAM, and shaves a few milliseconds off previous code.
Incidentally, I noticed that the v1 assembly sets the clock idle value to 32 in the PGPOL register until SDRAM refresh is enabled, but it seems like it should eventually be set back to zero (which is what the patch does). Does it matter?
Anyway, enough verbosity... take a look and see what you think. Unfortunately, besides testing the SPD code on memory dumps, I haven't tested the image.
Alfred
On Mon, 2007-04-30 at 10:47 -0400, Alfred Wanga wrote:
Ok, so I've been working on porting the SPD code, and this patch is what I've got so far. Here are some of the outstanding issues/changes:
...
Anyway, enough verbosity... take a look and see what you think. Unfortunately, besides testing the SPD code on memory dumps, I haven't tested the image.
Alfred
I've applied this patch after applying Uwe's patch (v2_tyan_s1846_progress.patch)
His patch modified the following files: Index: src/southbridge/intel/i82371eb/Config.lb Index: src/southbridge/intel/i82371eb/i82371eb.c Index: src/southbridge/intel/i82371eb/i82371eb.h Index: src/southbridge/intel/i82371eb/i82371eb_smbus.c Index: src/mainboard/tyan/s1846/Config.lb Index: src/mainboard/tyan/s1846/auto.c Index: targets/tyan/s1846/Config.lb
But left raminint.c untouched.
I've only viewed the error messages. I'll research them a little later.
Uwe, from what I've read concerning the i440BX memory initialization docs, SPD is *required* to initialize memory. This patch is needed isn't it? Or have we verified writing to memory is successful? I'm puzzled here. (or is memory init and activating SPD two seperate things here?)
-- Roger http://www.eskimo.com/~roger/index.html Key fingerprint = 8977 A252 2623 F567 70CD 1261 640F C963 1005 1D61
Mon Apr 30 12:12:00 PDT 2007
On Mon, Apr 30, 2007 at 12:19:46PM -0700, roger wrote:
(or is memory init and activating SPD two seperate things here?)
SPD is short for Serial Presence Detect, it's the protocol/method used to probe what type of RAM module is installed in a RAM slot.
SPD works by reading parameters from an EEPROM on the RAM module.
Generic memory controller initialization must use SPD, but it's quite possible to hardcode init for a specific RAM module if you already know the parameters.
//Peter
On Mon, Apr 30, 2007 at 12:19:46PM -0700, roger wrote:
Uwe, from what I've read concerning the i440BX memory initialization docs, SPD is *required* to initialize memory.
Yes, you get the information about timings etc. of the RAM via SPD, see Peters mail.
Or have we verified writing to memory is successful?
Well, yes, at least I think so :) However, we only know for sure when we booted a payload successfully (which will hopefully be soon).
Next up: investigate what the reason for the elfboot error is.
raminit.c:407.18: raminit.c:796.20: generic_sdram.c:49.40: auto.c:97.25: too few registers
Don't worry. This is a romcc problem/limitation, and we cannot do too much to fix it. We'll switch to using Cache as RAM eventually.
Uwe.
On Tue, 2007-05-01 at 01:12 +0200, Uwe Hermann wrote:
On Mon, Apr 30, 2007 at 12:19:46PM -0700, roger wrote:
Or have we verified writing to memory is successful?
Well, yes, at least I think so :) <-- lol.
However, we only know for sure when we booted a payload successfully (which will hopefully be soon). Next up: investigate what the reason for the elfboot error is.
Right. After spending a day hacking with my superior lower level of intellect then everybody else here, i was ready to hack elfboot.c to dump parts of the memory it was actually reading, rather then just the address values.
raminit.c:407.18: raminit.c:796.20: generic_sdram.c:49.40: auto.c:97.25: too few registers
Don't worry. This is a romcc problem/limitation, and we cannot do too much to fix it. We'll switch to using Cache as RAM eventually.
After awhile of staring at the code, it started pointing as a more internal coding error.
-- Roger http://www.eskimo.com/~roger/index.html Key fingerprint = 8977 A252 2623 F567 70CD 1261 640F C963 1005 1D61
Mon Apr 30 16:44:26 PDT 2007
Hi,
thanks for your patch. However, please sign-off all patches you submit, otherwise we cannot commit them.
http://linuxbios.org/Development_Guidelines#Sign-off_Procedure
Please re-send this patch with a sign-off.
On Mon, Apr 30, 2007 at 10:47:42AM -0400, Alfred Wanga wrote:
- PMCR - When should it be updated?
Looking at the assembly, it seems as though it's ok to just set the final value before the RAM refresh code instead of waiting until afterwards, but I don't know for sure, so I left the original code alone.
Yeah, I'm not sure either. Will look into that later...
- sdram_enable delays
I changed all the RAM timing delays (tRP, tRC, tMRD) to 1usec, since the timings are on the order of hundreds of nanoseconds (according to SPD values) and the smallest resolution timer available seems to be udelay() anyway. It should work for any SDRAM, and shaves a few milliseconds off previous code.
Yep, when everything is working fine (or maybe even before) we'll lower the delays. I set them quite high to make sure that it's definately not a too short delay which is causing problems...
If you want you can submit an extra patch with just the delay-fixes. I'll test that on my hardware and commit if it still works with shorter delays.
Anyway, enough verbosity... take a look and see what you think. Unfortunately, besides testing the SPD code on memory dumps, I haven't tested the image.
It doesn't compile, but that's not a problem in your code, but rather a limitation of romcc ("too few registers").
I'll try to move the 440BX code over to Cache as RAM, that'll be needed for LinuxBIOSv3 anyways. Expect a patch soonish...
After a few quick tests (after ripping out lots of code and debug statements so that romcc compiles the code) it didn't seem to work on my board. That may have lots of reasons (after all, I had to remove lots of code and replace it with hardcoded values), but I'll look into it in more detail.
Anyway, on the long run I don't want to merge this code as is (not a pure translation of the v1 assembler code), but rather create a generic framework for the RAM init on 440BX-ish chipsets.
It should be possible to share most of the code for, e.g. - 440BX/ZX/FX/... - 430TX/... - and maybe more such (probably quite similar) chipsets.
Thus, no spd_set_pgpol() etc., but more generic code (as far as possible).
But please re-send your patch with a sign-off, I think we can merge at least some parts of it.
Index: src/northbridge/intel/i440bx/raminit.c
--- src/northbridge/intel/i440bx/raminit.c (revision 2621) +++ src/northbridge/intel/i440bx/raminit.c (working copy) @@ -377,7 +377,348 @@ DIMM-independant configuration functions. -----------------------------------------------------------------------------*/
+/* Performs Bit Scan Right (MSB) Function (for SPD code) */ +static inline uint8_t bsr(uint16_t val) +{ +#if ASSEMBLY
- __asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val));
No assembler, please. Apart from the very early stuff which _has_ to be written in assembler, LinuxBIOS should be written completely in C.
/* 2. Precharge all. Wait tRP. */ PRINT_DEBUG("RAM Enable 2: Precharge all\r\n");
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0);
- mdelay(10);
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000);
Why 0x2000 here?
Uwe.
Apologies everyone... I meant to tweak it after the feedback, but never could seem to get around to it. Here is the original patch again.
Signed-off-by: Alfred Wanga awanga@gmail.com
Uwe Hermann wrote:
Hi,
thanks for your patch. However, please sign-off all patches you submit, otherwise we cannot commit them.
http://linuxbios.org/Development_Guidelines#Sign-off_Procedure
Please re-send this patch with a sign-off.
On Mon, Apr 30, 2007 at 10:47:42AM -0400, Alfred Wanga wrote:
- PMCR - When should it be updated?
Looking at the assembly, it seems as though it's ok to just set the final value before the RAM refresh code instead of waiting until afterwards, but I don't know for sure, so I left the original code alone.
Yeah, I'm not sure either. Will look into that later...
- sdram_enable delays
I changed all the RAM timing delays (tRP, tRC, tMRD) to 1usec, since the timings are on the order of hundreds of nanoseconds (according to SPD values) and the smallest resolution timer available seems to be udelay() anyway. It should work for any SDRAM, and shaves a few milliseconds off previous code.
Yep, when everything is working fine (or maybe even before) we'll lower the delays. I set them quite high to make sure that it's definately not a too short delay which is causing problems...
If you want you can submit an extra patch with just the delay-fixes. I'll test that on my hardware and commit if it still works with shorter delays.
Anyway, enough verbosity... take a look and see what you think. Unfortunately, besides testing the SPD code on memory dumps, I haven't tested the image.
It doesn't compile, but that's not a problem in your code, but rather a limitation of romcc ("too few registers").
I'll try to move the 440BX code over to Cache as RAM, that'll be needed for LinuxBIOSv3 anyways. Expect a patch soonish...
After a few quick tests (after ripping out lots of code and debug statements so that romcc compiles the code) it didn't seem to work on my board. That may have lots of reasons (after all, I had to remove lots of code and replace it with hardcoded values), but I'll look into it in more detail.
Anyway, on the long run I don't want to merge this code as is (not a pure translation of the v1 assembler code), but rather create a generic framework for the RAM init on 440BX-ish chipsets.
It should be possible to share most of the code for, e.g.
- 440BX/ZX/FX/...
- 430TX/...
- and maybe more such (probably quite similar) chipsets.
Thus, no spd_set_pgpol() etc., but more generic code (as far as possible).
But please re-send your patch with a sign-off, I think we can merge at least some parts of it.
Index: src/northbridge/intel/i440bx/raminit.c
--- src/northbridge/intel/i440bx/raminit.c (revision 2621) +++ src/northbridge/intel/i440bx/raminit.c (working copy) @@ -377,7 +377,348 @@ DIMM-independant configuration functions. -----------------------------------------------------------------------------*/
+/* Performs Bit Scan Right (MSB) Function (for SPD code) */ +static inline uint8_t bsr(uint16_t val) +{ +#if ASSEMBLY
- __asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val));
No assembler, please. Apart from the very early stuff which _has_ to be written in assembler, LinuxBIOS should be written completely in C.
/* 2. Precharge all. Wait tRP. */ PRINT_DEBUG("RAM Enable 2: Precharge all\r\n");
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0);
- mdelay(10);
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000);
Why 0x2000 here?
Uwe.
Index: src/northbridge/intel/i440bx/raminit.c =================================================================== --- src/northbridge/intel/i440bx/raminit.c (revision 2621) +++ src/northbridge/intel/i440bx/raminit.c (working copy) @@ -377,7 +377,348 @@ DIMM-independant configuration functions. -----------------------------------------------------------------------------*/
+/* Performs Bit Scan Right (MSB) Function (for SPD code) */ +static inline uint8_t bsr(uint16_t val) +{ +#if ASSEMBLY + __asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val)); + + return (uint8_t)val; +#else + uint8_t i; + + for (i = 15; i > 0; i--) + { + if (val & 0x8000) break; + val <<= 1; + } + return i; +#endif +} + + /** + * Set the DRAM Row Boundary Registers for all DIMMs. + * + * @param Memory controller + */ +static void spd_set_drb(const struct mem_controller *ctrl) +{ + uint8_t i, size = 0, addr = 0; + uint16_t tmp; + + for( i = 0; i < DIMM_SOCKETS; i++ ) + { + size = (smbus_read_byte(ctrl->channel0[i], SPD_NUM_ROWS) & 0xf); + size += (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_COLUMNS) & 0xf); + + /* Skip calculations if SPD returns undefined data */ + if (size) + { + tmp = smbus_read_byte(ctrl->channel0[i], + SPD_NUM_BANKS_PER_SDRAM) & 0xff; + size += bsr(tmp); + + /* Get the module data width and convert it + * to a power of two */ + tmp = (smbus_read_byte(ctrl->channel0[i], + SPD_MODULE_DATA_WIDTH_MSB) << 8) | + (smbus_read_byte(ctrl->channel0[i], + SPD_MODULE_DATA_WIDTH_LSB) & 0xff); + size += bsr(tmp); + + /* Now we have ram size as a power of two (less 1) */ + /* 2^23 = 8MBit: seems like it should be MBits not MB */ + size -= (23 - 1); /* Make it multiples of 8MBits */ + } + addr += (size << 1); + + PRINT_DEBUG("DRB"); + PRINT_DEBUG_HEX8(i<<1); + PRINT_DEBUG(": "); + PRINT_DEBUG_HEX8(addr); + PRINT_DEBUG("\r\n"); + + /* Ignore the dimm if it is over 2GB */ + if (size >= 8) + pci_write_config8(ctrl->d0, DRB+(i<<1), 0); + else + pci_write_config8(ctrl->d0, DRB+(i<<1), addr); + + + /* side two */ + if (smbus_read_byte(ctrl->channel0[i], SPD_NUM_DIMM_BANKS) > 1) + { + +#if 0 /* Asymmetric code doesn't seem to work */ + size = (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_ROWS) & 0xf0) >> 4; + size += (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_COLUMNS) & 0xf0) >> 4; + + if (size) + { + tmp = smbus_read_byte(ctrl->channel0[i], + SPD_NUM_BANKS_PER_SDRAM) & 0xff; + size += bsr(tmp); + + /* Get the module data width and convert it + * to a power of two */ + tmp = (smbus_read_byte(ctrl->channel0[i], + SPD_MODULE_DATA_WIDTH_MSB) << 8) | + (smbus_read_byte(ctrl->channel0[i], + SPD_MODULE_DATA_WIDTH_LSB) & 0x1f); + size += bsr(tmp); + + /* Make it multiples of 8MBits */ + size -= (23 - 1); + } +#endif + addr += (size << 1); + + PRINT_DEBUG("DRB"); + PRINT_DEBUG_HEX8((i<<1)+1); + PRINT_DEBUG(": "); + PRINT_DEBUG_HEX8(addr); + PRINT_DEBUG("\r\n"); + + /* Ignore the dimm if it is over 2GB (possible?) */ + if (size >= 8) + pci_write_config8(ctrl->d0, DRB+(i<<1)+1, 0); + else + pci_write_config8(ctrl->d0, DRB+(i<<1)+1, addr); + } + + } +} + +/** + * Set the DRAM Control Register. + * + * @param Memory controller + */ +static void spd_set_dramc(const struct mem_controller *ctrl) +{ + uint8_t i, memid; + uint8_t dram_reg = 0; + + /* Auto detect if RAM is registered or not. */ + /* The DRAMC register also controls the refresh rate but we can't + * set that here because we must leave refresh disabled. + */ + + /* Find the first dimm and assume the rest are the same (dangerous?) */ + for (i = 0; i < DIMM_SOCKETS; i++) + { + /* This was changed from SPD_MODULE_ATTRIBUTES because + * it can be equal to zero for an existing DIMM */ + memid = smbus_read_byte(ctrl->channel0[i], SPD_MEMORY_TYPE); + + if (memid == SPD_MEMORY_TYPE_SDRAM) break; + } + + if (memid != SPD_MEMORY_TYPE_SDRAM) + { + /* no SDRAM memory! */ + die("No memory found!"); + } + + memid = smbus_read_byte(ctrl->channel0[i], SPD_MODULE_ATTRIBUTES); + + if (memid & 0x12) + dram_reg |= 0x10; /* registered DRAM */ + else + dram_reg |= 0x08; /* unregistered DRAM */ + + /* Write DRAM control register (without refresh) */ + PRINT_DEBUG("DRAMC: "); + PRINT_DEBUG_HEX8(dram_reg); + PRINT_DEBUG("\r\n"); + pci_write_config8(ctrl->d0, DRAMC, dram_reg); +} + +/** + * Set the SDRAM Row Page Size Register. + * + * @param Memory controller + */ +static void spd_set_rps(const struct mem_controller *ctrl) +{ + uint16_t page, reg = 0; + uint8_t i, cnt = 0; + + /* The RPS register holds the size of a "page" of DRAM on each DIMM */ + /* default all page sizes to 2KB */ + for (i = 0; i < DIMM_SOCKETS; i++ ) + { + page = (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_COLUMNS) & 0xf); + + /* FIXME: do something with page sizes greater than 8KB!! */ + /* This workaround seems to generate valid values */ + if (page > 8) page = 8; + + if (page <= 8) + { + page <<= cnt; + reg |= page & 0xf; + + /* Only handling the symmetric case */ + if ( smbus_read_byte(ctrl->channel0[i], + SPD_NUM_DIMM_BANKS) > 1) + reg |= ((page & 0xf) << 4); + } + cnt += 4; + } + + /* Next block is for Ron's attempt to get registered to work. + * We have just verified that we have to have this code. It appears that + * the registered SDRAMs do indeed set the RPS wrong. Sheesh. + * + * We have verified that for registered DRAM the values are + * 1/2 the size they should be. So we test for registered + * and then double the sizes if needed. + */ + + if (pci_read_config8(ctrl->d0, DRAMC) & 0x10) + { + /* BIOS makes weird page size for registered! + * what we have found is you need to set the EVEN banks to + * twice the size. Fortunately there is a very easy way to + * do this. First, read the WORD value of the RPS register + * and double the size of the EVEN banks we only need to add 1 + * because the size is log2 + */ + reg += 0x1111; + } + + /* now write that final value into RPS register */ + PRINT_DEBUG("RPS: "); + PRINT_DEBUG_HEX16(reg); + PRINT_DEBUG("\r\n"); + pci_write_config16(ctrl->d0, RPS, reg); +} + +/** + * Set the Paging Policy Register. + * + * @param Memory controller + */ +static void spd_set_pgpol(const struct mem_controller *ctrl) +{ + uint16_t policy = 0; + uint8_t i, cnt = 0; + + /* The PGPOL register stores the number of logical banks per DIMM, + * and number of clocks the DRAM controller waits in the idle + * state. + */ + for (i = 0; i < DIMM_SOCKETS; i++ ) + { + if (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_BANKS_PER_SDRAM) >= 4) + { + policy |= (0x1 << cnt); + + /* for now only handle the symmtrical case */ + if (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_DIMM_BANKS) > 1) + policy |= (0x2 << cnt); + } + cnt += 2; + } + + /* 32 clocks DRAM idle time (change back to 0 after MRS?) */ + policy = (policy << 8) | 0x7; + + PRINT_DEBUG("PGPOL: "); + PRINT_DEBUG_HEX16(policy); + PRINT_DEBUG("\r\n"); + pci_write_config16(ctrl->d0, PGPOL, policy); +} + +/** + * Set the SDRAM Control Register. + * + * @param Memory controller + */ +static void spd_set_sdramc(const struct mem_controller *ctrl) +{ + uint8_t i; + uint16_t reg = 0x0100; + + for (i = 0; i < DIMM_SOCKETS; i++ ) + { + /* Default settings are OK if only CAS=3 is supported */ + if (smbus_read_byte(ctrl->channel0[i], + SPD_ACCEPTABLE_CAS_LATENCIES) & 0x2) + { + PRINT_DEBUG("DIMM bank "); + PRINT_DEBUG_HEX8(i); + PRINT_DEBUG(" supports CAS=2\r\n"); + pci_write_config8(ctrl->d0, SDRAMC, reg | 0x4); + break; + } + } +} + +/** + * Set the NBX Configuration Register. + * + * @param Memory controller + */ +static inline void spd_set_nbxcfg(const struct mem_controller *ctrl) +{ + uint8_t i, cnt = 24; + uint32_t reg; + + /* Say all dimms have no ECC support */ + reg = pci_read_config32(ctrl->d0, NBXCFG) | 0xff000000; + + for (i = 0; i < DIMM_SOCKETS; i++) + { + /* Module error correction type: + * 0 == None, 1 == Parity, 2 == ECC */ + if (smbus_read_byte(ctrl->channel0[i], + SPD_DIMM_CONFIG_TYPE) & 0x2) + { + reg ^= (0x1 << cnt); + + /* TODO: Set Data Integrity Mode for ECC Mode */ + /* Is this right? What about other EC modes? */ + /* reg |= 0x10; */ + + if (smbus_read_byte(ctrl->channel0[i], + SPD_NUM_DIMM_BANKS) > 1) + { + /* only the symmetric case is possible */ + reg ^= (0x2 << cnt); + } + } + cnt += 2; + } + + /* set correct DRAM bus speed */ + for (i = 0; i < DIMM_SOCKETS; i++) + { + /* set to 66MHz if at least one DIMM is 66MHz only */ + if (smbus_read_byte(ctrl->channel0[i], + SPD_INTEL_SPEC_FOR_FREQUENCY) == 0x66) + { + /* TODO: need to handle?: 66Mhz->CL2 / 100MHz->CL3 */ + reg |= 0x2000; + break; + } + } + + PRINT_DEBUG("NBXCFG: "); + PRINT_DEBUG_HEX32(reg); + PRINT_DEBUG("\r\n"); + pci_write_config32(ctrl->d0, NBXCFG, reg); +} + +/** * TODO. * * @param Memory controller @@ -387,6 +728,9 @@ int i, value; uint8_t reg;
+ /* Set DRAM idle time to 0 after MRS (needed?) */ + pci_write_config8(ctrl->d0, PGPOL, 0); + reg = pci_read_config8(ctrl->d0, DRAMC);
for (i = 0; i < DIMM_SOCKETS; i++) { @@ -446,32 +790,25 @@ */ static void sdram_set_spd_registers(const struct mem_controller *ctrl) { - /* TODO: Don't hardcode the values here, get info via SPD. */ + PRINT_DEBUG("Reading SPD Registers...\r\n");
- /* TODO: Set DRB0-DRB7. */ - pci_write_config8(ctrl->d0, DRB0, 0x08); - pci_write_config8(ctrl->d0, DRB1, 0x08); - pci_write_config8(ctrl->d0, DRB2, 0x08); - pci_write_config8(ctrl->d0, DRB3, 0x08); - pci_write_config8(ctrl->d0, DRB4, 0x08); - pci_write_config8(ctrl->d0, DRB5, 0x08); - pci_write_config8(ctrl->d0, DRB6, 0x08); - pci_write_config8(ctrl->d0, DRB7, 0x08); + /* Set DRB0-DRB7. */ + spd_set_drb(ctrl);
- /* TODO: Set DRAMC. Don't enable refresh for now. */ - pci_write_config8(ctrl->d0, DRAMC, 0x08); + /* Set DRAMC. Don't enable refresh for now. */ + spd_set_dramc(ctrl);
- /* TODO: Set RPS. */ - pci_write_config16(ctrl->d0, RPS, 0x0001); + /* Set RPS. */ + spd_set_rps(ctrl);
- /* TODO: Set SDRAMC. */ - // pci_write_config16(ctrl->d0, SDRAMC, 0x0000); + /* Set SDRAMC. */ + spd_set_sdramc(ctrl);
- /* TODO: Set PGPOL. */ - pci_write_config16(ctrl->d0, PGPOL, 0x0107); + /* Set PGPOL. */ + spd_set_pgpol(ctrl);
- /* TODO: Set NBXCFG. */ - // pci_write_config32(ctrl->d0, NBXCFG, 0x0100220c); + /* Set NBXCFG. */ + spd_set_nbxcfg(ctrl);
/* TODO: Set PMCR? */ // pci_write_config8(ctrl->d0, PMCR, 0x14); @@ -495,45 +832,51 @@ int i;
/* TODO: Use a delay here? Needed? */ - mdelay(200); + /* Wait at least 1msec after device deselect (when is that?) */ + mdelay(1);
- /* TODO: How long should the delays be? Fix later. */ + /* RAM Timings (tRP, tRC, tMRD) from SPD are on the order or + * nanoseconds (max=255ns) so 1usec should be more than enough + * time for any given SDRAM. */
/* 1. Apply NOP. */ PRINT_DEBUG("RAM Enable 1: Apply NOP\r\n"); do_ram_command(ctrl, RAM_COMMAND_NOP, 0); - mdelay(10); + udelay(200); /* minimum pause of 200usec after the NOP */
/* 2. Precharge all. Wait tRP. */ PRINT_DEBUG("RAM Enable 2: Precharge all\r\n"); - do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0); - mdelay(10); + do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000); + udelay(1);
/* 3. Perform 8 refresh cycles. Wait tRC each time. */ PRINT_DEBUG("RAM Enable 3: CBR\r\n"); for (i = 0; i < 8; i++) { do_ram_command(ctrl, RAM_COMMAND_CBR, 0); - mdelay(10); + udelay(1); }
/* 4. Mode register set. Wait two memory cycles. */ PRINT_DEBUG("RAM Enable 4: Mode register set\r\n"); - do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0); - // TODO: Is 0x1d0 correct? - // do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0000); - mdelay(10); - mdelay(10); + /* Compute MRS Opcode (Burst length = 4, Interleaved) */ + i = 0x2a; + /* If CAS = 3, modify opcode */ + if (pci_read_config8(ctrl->d0, SDRAMC) & 0x4 == 0) + i |= 0x10; + i <<= 3; + do_ram_command(ctrl, RAM_COMMAND_MRS, i); //0x1d0 + udelay(1);
/* 5. Normal operation. */ PRINT_DEBUG("RAM Enable 5: Normal operation\r\n"); do_ram_command(ctrl, RAM_COMMAND_NORMAL, 0); - mdelay(10); + udelay(1);
/* 6. Finally enable refresh. */ PRINT_DEBUG("RAM Enable 6: Enable refresh\r\n"); // pci_write_config8(ctrl->d0, PMCR, 0x10); spd_enable_refresh(ctrl); - mdelay(10); + udelay(1);
PRINT_DEBUG("Northbridge following SDRAM init:\r\n"); DUMPNORTH();
What's the status of the code below?
Carl-Daniel
On 30.05.2007 04:36, Alfred Wanga wrote:
Apologies everyone... I meant to tweak it after the feedback, but never could seem to get around to it. Here is the original patch again.
Signed-off-by: Alfred Wanga awanga@gmail.com
Uwe Hermann wrote:
Hi,
thanks for your patch. However, please sign-off all patches you submit, otherwise we cannot commit them.
http://linuxbios.org/Development_Guidelines#Sign-off_Procedure
Please re-send this patch with a sign-off.
On Mon, Apr 30, 2007 at 10:47:42AM -0400, Alfred Wanga wrote:
- PMCR - When should it be updated?
Looking at the assembly, it seems as though it's ok to just set the final value before the RAM refresh code instead of waiting until afterwards, but I don't know for sure, so I left the original code alone.
Yeah, I'm not sure either. Will look into that later...
- sdram_enable delays
I changed all the RAM timing delays (tRP, tRC, tMRD) to 1usec, since the timings are on the order of hundreds of nanoseconds (according to SPD values) and the smallest resolution timer available seems to be udelay() anyway. It should work for any SDRAM, and shaves a few milliseconds off previous code.
Yep, when everything is working fine (or maybe even before) we'll lower the delays. I set them quite high to make sure that it's definately not a too short delay which is causing problems...
If you want you can submit an extra patch with just the delay-fixes. I'll test that on my hardware and commit if it still works with shorter delays.
Anyway, enough verbosity... take a look and see what you think. Unfortunately, besides testing the SPD code on memory dumps, I haven't tested the image.
It doesn't compile, but that's not a problem in your code, but rather a limitation of romcc ("too few registers").
I'll try to move the 440BX code over to Cache as RAM, that'll be needed for LinuxBIOSv3 anyways. Expect a patch soonish...
After a few quick tests (after ripping out lots of code and debug statements so that romcc compiles the code) it didn't seem to work on my board. That may have lots of reasons (after all, I had to remove lots of code and replace it with hardcoded values), but I'll look into it in more detail.
Anyway, on the long run I don't want to merge this code as is (not a pure translation of the v1 assembler code), but rather create a generic framework for the RAM init on 440BX-ish chipsets.
It should be possible to share most of the code for, e.g.
- 440BX/ZX/FX/...
- 430TX/...
- and maybe more such (probably quite similar) chipsets.
Thus, no spd_set_pgpol() etc., but more generic code (as far as possible).
But please re-send your patch with a sign-off, I think we can merge at least some parts of it.
Index: src/northbridge/intel/i440bx/raminit.c
--- src/northbridge/intel/i440bx/raminit.c (revision 2621) +++ src/northbridge/intel/i440bx/raminit.c (working copy) @@ -377,7 +377,348 @@ DIMM-independant configuration functions. -----------------------------------------------------------------------------*/
+/* Performs Bit Scan Right (MSB) Function (for SPD code) */ +static inline uint8_t bsr(uint16_t val) +{ +#if ASSEMBLY
- __asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val));
No assembler, please. Apart from the very early stuff which _has_ to be written in assembler, LinuxBIOS should be written completely in C.
/* 2. Precharge all. Wait tRP. */ PRINT_DEBUG("RAM Enable 2: Precharge all\r\n");
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0);
- mdelay(10);
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000);
Why 0x2000 here?
Uwe.
Index: src/northbridge/intel/i440bx/raminit.c
--- src/northbridge/intel/i440bx/raminit.c (revision 2621) +++ src/northbridge/intel/i440bx/raminit.c (working copy) @@ -377,7 +377,348 @@ DIMM-independant configuration functions. -----------------------------------------------------------------------------*/
+/* Performs Bit Scan Right (MSB) Function (for SPD code) */ +static inline uint8_t bsr(uint16_t val) +{ +#if ASSEMBLY
- __asm__ __volatile__ ("bsr %1, %%ax;" : "=a"(val) : "a"(val));
- return (uint8_t)val;
+#else
- uint8_t i;
- for (i = 15; i > 0; i--)
- {
if (val & 0x8000) break;val <<= 1;- }
- return i;
+#endif +}
/**
- Set the DRAM Row Boundary Registers for all DIMMs.
- @param Memory controller
- */
+static void spd_set_drb(const struct mem_controller *ctrl) +{
- uint8_t i, size = 0, addr = 0;
- uint16_t tmp;
- for( i = 0; i < DIMM_SOCKETS; i++ )
- {
size = (smbus_read_byte(ctrl->channel0[i], SPD_NUM_ROWS) & 0xf);size += (smbus_read_byte(ctrl->channel0[i],SPD_NUM_COLUMNS) & 0xf);/* Skip calculations if SPD returns undefined data */if (size){tmp = smbus_read_byte(ctrl->channel0[i],SPD_NUM_BANKS_PER_SDRAM) & 0xff;size += bsr(tmp);/* Get the module data width and convert it* to a power of two */tmp = (smbus_read_byte(ctrl->channel0[i],SPD_MODULE_DATA_WIDTH_MSB) << 8) |(smbus_read_byte(ctrl->channel0[i],SPD_MODULE_DATA_WIDTH_LSB) & 0xff);size += bsr(tmp);/* Now we have ram size as a power of two (less 1) *//* 2^23 = 8MBit: seems like it should be MBits not MB */size -= (23 - 1); /* Make it multiples of 8MBits */}addr += (size << 1);PRINT_DEBUG("DRB");PRINT_DEBUG_HEX8(i<<1);PRINT_DEBUG(": ");PRINT_DEBUG_HEX8(addr);PRINT_DEBUG("\r\n");/* Ignore the dimm if it is over 2GB */if (size >= 8)pci_write_config8(ctrl->d0, DRB+(i<<1), 0);elsepci_write_config8(ctrl->d0, DRB+(i<<1), addr);/* side two */if (smbus_read_byte(ctrl->channel0[i], SPD_NUM_DIMM_BANKS) > 1){+#if 0 /* Asymmetric code doesn't seem to work */
size = (smbus_read_byte(ctrl->channel0[i],SPD_NUM_ROWS) & 0xf0) >> 4;size += (smbus_read_byte(ctrl->channel0[i],SPD_NUM_COLUMNS) & 0xf0) >> 4;if (size){tmp = smbus_read_byte(ctrl->channel0[i],SPD_NUM_BANKS_PER_SDRAM) & 0xff;size += bsr(tmp);/* Get the module data width and convert it* to a power of two */tmp = (smbus_read_byte(ctrl->channel0[i],SPD_MODULE_DATA_WIDTH_MSB) << 8) |(smbus_read_byte(ctrl->channel0[i],SPD_MODULE_DATA_WIDTH_LSB) & 0x1f);size += bsr(tmp);/* Make it multiples of 8MBits */size -= (23 - 1);}+#endif
addr += (size << 1);PRINT_DEBUG("DRB");PRINT_DEBUG_HEX8((i<<1)+1);PRINT_DEBUG(": ");PRINT_DEBUG_HEX8(addr);PRINT_DEBUG("\r\n");/* Ignore the dimm if it is over 2GB (possible?) */if (size >= 8)pci_write_config8(ctrl->d0, DRB+(i<<1)+1, 0);elsepci_write_config8(ctrl->d0, DRB+(i<<1)+1, addr);}- }
+}
+/**
- Set the DRAM Control Register.
- @param Memory controller
- */
+static void spd_set_dramc(const struct mem_controller *ctrl) +{
- uint8_t i, memid;
- uint8_t dram_reg = 0;
- /* Auto detect if RAM is registered or not. */
- /* The DRAMC register also controls the refresh rate but we can't
* set that here because we must leave refresh disabled.*/- /* Find the first dimm and assume the rest are the same (dangerous?) */
- for (i = 0; i < DIMM_SOCKETS; i++)
- {
/* This was changed from SPD_MODULE_ATTRIBUTES because* it can be equal to zero for an existing DIMM */memid = smbus_read_byte(ctrl->channel0[i], SPD_MEMORY_TYPE);if (memid == SPD_MEMORY_TYPE_SDRAM) break;- }
- if (memid != SPD_MEMORY_TYPE_SDRAM)
- {
/* no SDRAM memory! */die("No memory found!");- }
- memid = smbus_read_byte(ctrl->channel0[i], SPD_MODULE_ATTRIBUTES);
- if (memid & 0x12)
dram_reg |= 0x10; /* registered DRAM */- else
dram_reg |= 0x08; /* unregistered DRAM */- /* Write DRAM control register (without refresh) */
- PRINT_DEBUG("DRAMC: ");
- PRINT_DEBUG_HEX8(dram_reg);
- PRINT_DEBUG("\r\n");
- pci_write_config8(ctrl->d0, DRAMC, dram_reg);
+}
+/**
- Set the SDRAM Row Page Size Register.
- @param Memory controller
- */
+static void spd_set_rps(const struct mem_controller *ctrl) +{
- uint16_t page, reg = 0;
- uint8_t i, cnt = 0;
- /* The RPS register holds the size of a "page" of DRAM on each DIMM */
- /* default all page sizes to 2KB */
- for (i = 0; i < DIMM_SOCKETS; i++ )
- {
page = (smbus_read_byte(ctrl->channel0[i],SPD_NUM_COLUMNS) & 0xf);/* FIXME: do something with page sizes greater than 8KB!! *//* This workaround seems to generate valid values */if (page > 8) page = 8;if (page <= 8){page <<= cnt;reg |= page & 0xf;/* Only handling the symmetric case */if ( smbus_read_byte(ctrl->channel0[i],SPD_NUM_DIMM_BANKS) > 1)reg |= ((page & 0xf) << 4);}cnt += 4;- }
- /* Next block is for Ron's attempt to get registered to work.
* We have just verified that we have to have this code. It appears that* the registered SDRAMs do indeed set the RPS wrong. Sheesh.** We have verified that for registered DRAM the values are* 1/2 the size they should be. So we test for registered* and then double the sizes if needed.*/- if (pci_read_config8(ctrl->d0, DRAMC) & 0x10)
- {
/* BIOS makes weird page size for registered!* what we have found is you need to set the EVEN banks to* twice the size. Fortunately there is a very easy way to* do this. First, read the WORD value of the RPS register* and double the size of the EVEN banks we only need to add 1* because the size is log2*/reg += 0x1111;- }
- /* now write that final value into RPS register */
- PRINT_DEBUG("RPS: ");
- PRINT_DEBUG_HEX16(reg);
- PRINT_DEBUG("\r\n");
- pci_write_config16(ctrl->d0, RPS, reg);
+}
+/**
- Set the Paging Policy Register.
- @param Memory controller
- */
+static void spd_set_pgpol(const struct mem_controller *ctrl) +{
- uint16_t policy = 0;
- uint8_t i, cnt = 0;
- /* The PGPOL register stores the number of logical banks per DIMM,
* and number of clocks the DRAM controller waits in the idle* state.*/- for (i = 0; i < DIMM_SOCKETS; i++ )
- {
if (smbus_read_byte(ctrl->channel0[i],SPD_NUM_BANKS_PER_SDRAM) >= 4){policy |= (0x1 << cnt);/* for now only handle the symmtrical case */if (smbus_read_byte(ctrl->channel0[i],SPD_NUM_DIMM_BANKS) > 1)policy |= (0x2 << cnt);}cnt += 2;- }
- /* 32 clocks DRAM idle time (change back to 0 after MRS?) */
- policy = (policy << 8) | 0x7;
- PRINT_DEBUG("PGPOL: ");
- PRINT_DEBUG_HEX16(policy);
- PRINT_DEBUG("\r\n");
- pci_write_config16(ctrl->d0, PGPOL, policy);
+}
+/**
- Set the SDRAM Control Register.
- @param Memory controller
- */
+static void spd_set_sdramc(const struct mem_controller *ctrl) +{
- uint8_t i;
- uint16_t reg = 0x0100;
- for (i = 0; i < DIMM_SOCKETS; i++ )
- {
/* Default settings are OK if only CAS=3 is supported */if (smbus_read_byte(ctrl->channel0[i],SPD_ACCEPTABLE_CAS_LATENCIES) & 0x2){PRINT_DEBUG("DIMM bank ");PRINT_DEBUG_HEX8(i);PRINT_DEBUG(" supports CAS=2\r\n");pci_write_config8(ctrl->d0, SDRAMC, reg | 0x4);break;}- }
+}
+/**
- Set the NBX Configuration Register.
- @param Memory controller
- */
+static inline void spd_set_nbxcfg(const struct mem_controller *ctrl) +{
- uint8_t i, cnt = 24;
- uint32_t reg;
- /* Say all dimms have no ECC support */
- reg = pci_read_config32(ctrl->d0, NBXCFG) | 0xff000000;
- for (i = 0; i < DIMM_SOCKETS; i++)
- {
/* Module error correction type:* 0 == None, 1 == Parity, 2 == ECC */if (smbus_read_byte(ctrl->channel0[i],SPD_DIMM_CONFIG_TYPE) & 0x2){reg ^= (0x1 << cnt);/* TODO: Set Data Integrity Mode for ECC Mode *//* Is this right? What about other EC modes? *//* reg |= 0x10; */if (smbus_read_byte(ctrl->channel0[i],SPD_NUM_DIMM_BANKS) > 1){/* only the symmetric case is possible */reg ^= (0x2 << cnt);}}cnt += 2;- }
- /* set correct DRAM bus speed */
- for (i = 0; i < DIMM_SOCKETS; i++)
- {
/* set to 66MHz if at least one DIMM is 66MHz only */if (smbus_read_byte(ctrl->channel0[i],SPD_INTEL_SPEC_FOR_FREQUENCY) == 0x66){/* TODO: need to handle?: 66Mhz->CL2 / 100MHz->CL3 */reg |= 0x2000;break;}- }
- PRINT_DEBUG("NBXCFG: ");
- PRINT_DEBUG_HEX32(reg);
- PRINT_DEBUG("\r\n");
- pci_write_config32(ctrl->d0, NBXCFG, reg);
+}
+/**
- TODO.
- @param Memory controller
@@ -387,6 +728,9 @@ int i, value; uint8_t reg;
/* Set DRAM idle time to 0 after MRS (needed?) */
pci_write_config8(ctrl->d0, PGPOL, 0);
reg = pci_read_config8(ctrl->d0, DRAMC);
for (i = 0; i < DIMM_SOCKETS; i++) {
@@ -446,32 +790,25 @@ */ static void sdram_set_spd_registers(const struct mem_controller *ctrl) {
- /* TODO: Don't hardcode the values here, get info via SPD. */
- PRINT_DEBUG("Reading SPD Registers...\r\n");
- /* TODO: Set DRB0-DRB7. */
- pci_write_config8(ctrl->d0, DRB0, 0x08);
- pci_write_config8(ctrl->d0, DRB1, 0x08);
- pci_write_config8(ctrl->d0, DRB2, 0x08);
- pci_write_config8(ctrl->d0, DRB3, 0x08);
- pci_write_config8(ctrl->d0, DRB4, 0x08);
- pci_write_config8(ctrl->d0, DRB5, 0x08);
- pci_write_config8(ctrl->d0, DRB6, 0x08);
- pci_write_config8(ctrl->d0, DRB7, 0x08);
- /* Set DRB0-DRB7. */
- spd_set_drb(ctrl);
- /* TODO: Set DRAMC. Don't enable refresh for now. */
- pci_write_config8(ctrl->d0, DRAMC, 0x08);
- /* Set DRAMC. Don't enable refresh for now. */
- spd_set_dramc(ctrl);
- /* TODO: Set RPS. */
- pci_write_config16(ctrl->d0, RPS, 0x0001);
- /* Set RPS. */
- spd_set_rps(ctrl);
- /* TODO: Set SDRAMC. */
- // pci_write_config16(ctrl->d0, SDRAMC, 0x0000);
- /* Set SDRAMC. */
- spd_set_sdramc(ctrl);
- /* TODO: Set PGPOL. */
- pci_write_config16(ctrl->d0, PGPOL, 0x0107);
- /* Set PGPOL. */
- spd_set_pgpol(ctrl);
- /* TODO: Set NBXCFG. */
- // pci_write_config32(ctrl->d0, NBXCFG, 0x0100220c);
/* Set NBXCFG. */
spd_set_nbxcfg(ctrl);
/* TODO: Set PMCR? */ // pci_write_config8(ctrl->d0, PMCR, 0x14);
@@ -495,45 +832,51 @@ int i;
/* TODO: Use a delay here? Needed? */
- mdelay(200);
- /* Wait at least 1msec after device deselect (when is that?) */
- mdelay(1);
- /* TODO: How long should the delays be? Fix later. */
/* RAM Timings (tRP, tRC, tMRD) from SPD are on the order or
* nanoseconds (max=255ns) so 1usec should be more than enough* time for any given SDRAM. *//* 1. Apply NOP. */ PRINT_DEBUG("RAM Enable 1: Apply NOP\r\n"); do_ram_command(ctrl, RAM_COMMAND_NOP, 0);
- mdelay(10);
udelay(200); /* minimum pause of 200usec after the NOP */
/* 2. Precharge all. Wait tRP. */ PRINT_DEBUG("RAM Enable 2: Precharge all\r\n");
- do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0);
- mdelay(10);
do_ram_command(ctrl, RAM_COMMAND_PRECHARGE, 0x2000);
udelay(1);
/* 3. Perform 8 refresh cycles. Wait tRC each time. */ PRINT_DEBUG("RAM Enable 3: CBR\r\n"); for (i = 0; i < 8; i++) { do_ram_command(ctrl, RAM_COMMAND_CBR, 0);
mdelay(10);
udelay(1);}
/* 4. Mode register set. Wait two memory cycles. */ PRINT_DEBUG("RAM Enable 4: Mode register set\r\n");
- do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0);
- // TODO: Is 0x1d0 correct?
- // do_ram_command(ctrl, RAM_COMMAND_MRS, 0x1d0000);
- mdelay(10);
- mdelay(10);
/* Compute MRS Opcode (Burst length = 4, Interleaved) */
i = 0x2a;
/* If CAS = 3, modify opcode */
if (pci_read_config8(ctrl->d0, SDRAMC) & 0x4 == 0)
i |= 0x10;i <<= 3;
do_ram_command(ctrl, RAM_COMMAND_MRS, i); //0x1d0
udelay(1);
/* 5. Normal operation. */ PRINT_DEBUG("RAM Enable 5: Normal operation\r\n"); do_ram_command(ctrl, RAM_COMMAND_NORMAL, 0);
- mdelay(10);
udelay(1);
/* 6. Finally enable refresh. */ PRINT_DEBUG("RAM Enable 6: Enable refresh\r\n"); // pci_write_config8(ctrl->d0, PMCR, 0x10); spd_enable_refresh(ctrl);
- mdelay(10);
udelay(1);
PRINT_DEBUG("Northbridge following SDRAM init:\r\n"); DUMPNORTH();
Interesting... seems this patch is creating a cpu run when compiling. Within ASM code maybe?
Roger http://www.eskimo.com/~roger/index.html Key fingerprint = 8977 A252 2623 F567 70CD 1261 640F C963 1005 1D61
Mon Apr 30 15:18:19 PDT 2007
-
Alfred Wanga -
Carl-Daniel Hailfinger -
Peter Stuge -
roger -
Uwe Hermann