Re: [LinuxBIOS] i440bx RAM initialization code (SPD)

[Carl-Daniel Hailfinger]

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
>
> 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();