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 <[EMAIL PROTECTED]> > > > 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(); > -- http://www.hailfinger.org/ -- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
