v2: [RFC] Dynamic RAM detection for intel/i440bx

[Mats Erik Andersson]

Hello all,

I send a patch which is not intended for insertion as is,
but I have to draw the line somewhere in order to get your
comments.

The patch alters

coreboot-v2/src/northbridge/intel/i440bx/raminit.c

and it implements dynamic calculation of the registers
DRB#, RPS and PGPOL. It does this using alternate methods,
chosen by macro switches in the beginning of the file.

In order to successfully compile the new source, it is
unfortunately necessary to use Romcc with optimisation
"-mcpu=p2" to provide scratch registers for normal/fallback,
and also "-O2" to shrink the fallback image. A script "optim.sh",
that mends the makefiles, is appended to this message.

I have successfully (in the sense of complete image build) 
done this alteration for all relevant mainboards in the
source tree:

abit/be6-ii_v2_0
asus/p2b
asus/p2b-f
asus/p3b-f
a-trend/atc-6220
a-trend/atc-6240         (needs one PRINT_DEBUG less!)
azza/pt-6ibd
biostar/m6tba
compaq/deskpro_en_sff_p600
gigabyte/ga-6bxc
msi/ms6119
tyan/s1846

and for my own mainboard msi/ms6147 I can actually boot into
Filo with many different combinations and positions of SDRAM
devices. However, there are still some issues with timings,
which lead to the strange phenomenon that one particular
double sided 128 MB card in my possession, boots well enough
into Filo when positioned in DIMM1, but cannot enter the second
phase of Coreboot when used in DIMM0. Other 128 MB cards work
in both positions. These differences are probably due to my
incomplete understanding of the northbridge register MBSC, as
well as the buffer strength bits in NBXCFG.

Since ms6147 only has two DIMM-slots, I have not been able to
test the patch for use with USE_DIMM2 or USE_DIMM3 activated.
The function do_ram_command() has two outcommented work arounds
in order to activate the different RAM position, but a true dynamic
code has not yet been taken into consideration.

I now desire other people to test this code against their
mainboards, in order that we (or mainly I) could get indications
as to push forward to choose one of the methods the code
offers for its SPD-detection, and to provide me with more
information on SDRAM timings that do not work with the present
values in MBSC and NBXCFG. The values present now are _not_
the only values I have _inconclusively_ tried!

I welcome all feedback. The 48 first SPD-bytes of malfunctioning
SDRAM cards would be most valuable. Testers should take care
to add one or more read32() inside do_ram_command() in order
to perform a read in any additional memory bank. That read32()
bears the sole responsibility of awakening the SDRAM chips,
as this mechanism is independent of any configuration the
northbridge i440bx gets from any code for dynamic detection.

Best regards and with the best intents,

Mats Erik Andersson

[Tim ter Laak]

On Sun, 14 Sep 2008, Mats Erik Andersson wrote:
> Hello all,
>
> I send a patch which is not intended for insertion as is,
> but I have to draw the line somewhere in order to get your
> comments.

Thanks. I'll do some testing with this one.
> The patch alters
>
> coreboot-v2/src/northbridge/intel/i440bx/raminit.c
>
> and it implements dynamic calculation of the registers
> DRB#, RPS and PGPOL. It does this using alternate methods,
> chosen by macro switches in the beginning of the file.

I guess you'd like feedback on success/failure for both methods?
> These differences are probably due to my
> incomplete understanding of the northbridge register MBSC, as
> well as the buffer strength bits in NBXCFG.

The basics of buffer strength boil down to this: The Northbridge is the 
source of address and control signals; the DRAM chips on the DIMMs are 
loads on these signals. The bigger the load on a particular signal, the 
stronger the signal needs to be asserted by the northbridge, so as to make 
the signal strong enough within the required time.

Usually, the load on the data lines depends only on the number of DIMM 
"sides" that are present. The load on address and other control lines also 
depends on the number of chips each "side" consists of (which is related 
to the data width of the DRAM chips). When more chips need to be connected 
to the same signal, the load is higher and the buffer needs to be 
stronger.

Unfortunately, blindly setting the buffers to the biggest strength is not 
a good idea either, as this will result in signal "overshoot" when the 
actual load is rather low. This means the signal will rapidly fluctuate 
before stabilizing on its final value, which may take too long.

The exact implementation details are unknown to me though. I guess I'll 
put different combinations of DIMMs in the board and see how the stock 
BIOS sets up the MBSC.
> Since ms6147 only has two DIMM-slots, I have not been able to
> test the patch for use with USE_DIMM2 or USE_DIMM3 activated.

The AB-BM6 I'm working on has 3 slots, and I usually have all 3 populated. 
For my first test I didn't have USE_DIMM2 activated, but it still booted 
into Coreinfo with the full 384 MB (3 * 128) reported. Probably there's a 
lot of luck involved there.
> The function do_ram_command() has two outcommented work arounds
> in order to activate the different RAM position, but a true dynamic
> code has not yet been taken into consideration.

Having looked at the code in i82830/raminit.c (as suggested by 
Joseph Smith, I think), I would suggest we copy and adjust that. Hopefully 
romcc can find the registers for the loop and the address calculation...
It also contains some code for SPD-based setup of RAM parameters, did you 
have a look there as well for inspiration?

Finally, a somewhat related question for a wider audience:
For obvious reasons I wanted to change to Memtest as my payload. As it 
turns out, the size of that is almost 100 kB, which is bigger than the 
64kB available (256 kB flashchip, normal and fallback images with each 64 
kB for coreboot and 64 kb for payload). I'm going to look if it is 
possible to make an image with only normal or fallback, and give the extra 
space to the payload. If anyone has some pointers for me, they would be 
most appreciated.

Kind regards,
    Tim.

[Myles Watson]

> Finally, a somewhat related question for a wider audience:
> For obvious reasons I wanted to change to Memtest as my payload. As it
> turns out, the size of that is almost 100 kB, which is bigger than the
> 64kB available (256 kB flashchip, normal and fallback images with each 64
> kB for coreboot and 64 kb for payload). I'm going to look if it is
> possible to make an image with only normal or fallback, and give the extra
> space to the payload. If anyone has some pointers for me, they would be
> most appreciated.

Fallback only isn't too hard to do.  Look at any of the Config-lab.lb files
for boards that are supported in buildrom.

Thanks,
Myles

[Tim ter Laak]

On Mon, 15 Sep 2008, Myles Watson wrote:
> Fallback only isn't too hard to do.  Look at any of the Config-lab.lb files
> for boards that are supported in buildrom.

That looks simple enough. Actually, it reminded me I could compress 
memtest first, so I now have even another option.

Thanks!

Tim.

[Mats Erik Andersson]

Hello Tim and all those interested in i440bx,

the following function is fully functional code for implementing
the activation of all detected SDRAM modules, and both sides.
It has been tested with some combinations of 16 MB and 64 MB banks
and works as expected. It will be included in the original import
for mainboard/msi/ms6147, which I am about to post.

Best regards,

Mats Erik Andersson

------

static void do_ram_command(const struct mem_controller *ctrl,
    		   uint32_t command, uint32_t addr_offset)
{
    char i, dram_known;
    uint16_t reg;

/* Support for multiple DIMMs.
     * This requires that one read32() is performed
     * for each RAM bank/row.
     */

/* Configure the RAM command. */
    reg = pci_read_config16(ctrl->d0, SDRAMC);
    reg &= 0xff1f;		/* Clear bits 7-5. */
    reg |= (uint16_t) (command << 5);
    pci_write_config16(ctrl->d0, SDRAMC, reg);

/* RAM_COMMAND_NORMAL affects only the memory controller and
       doesn't need to be "sent" to the DIMMs. */
    if (command == RAM_COMMAND_NORMAL)
    	return;

/* RAM is populated from the bottom. This variable always
     * denotes the beginning of a newly populated block. */
    dram_known = 0;

for (i = 0; i < (DIMM_SOCKETS * 2); i++) {
    	/* Is this bank populated? */
    	if (pci_read_config8(ctrl->d0, DRB + i) > dram_known)
    	{
    		/* The DRB# registers count in 8 MB blocks.
    		 * 'dram_known contains the known lower bound of
    		 * the present block. Activate it. */
    		read32( (dram_known * 8 * 1024 * 1024) + addr_offset);
    		PRINT_DEBUG("RAM strobe 0x");
    		PRINT_DEBUG_HEX32(dram_known * 8 * 1024 * 1024 + addr_offset);
    		PRINT_DEBUG("\r\n");
    		/* DRB registers are cumulative, so grab the current value. */
    		dram_known = pci_read_config8(ctrl->d0, DRB + i);
    	};

}
}