On Mon, Nov 17, 2008 at 02:58:55PM +0100, Stefan Reinauer wrote:
Elia Yehuda wrote:
Those 2 patches are one step towards having a working Onboard-VGA and 512MB (the max for i810). The raminit.c patch fixes some misconfigurations and probes the DIMMs correctly (all dual-sided are now recognized properly), and also set buffer_strength to handle 2 DIMMs although atm this doesn't work. The i82810/northbridge.c patch takes care of allocating Onboard-VGA memory.
Signed-off-by: Elia Yehuda <z4ziggy@gmail.com mailto:z4ziggy@gmail.com>
Good work!
Yep, indeed. But please split up the patch in multiple ones, each fixing one issue. This way they can be tested and reviewed more easily. I suggest at least one patch for supporting multiple DIMMs and one for VGA stuff, maybe more...
I'll be able to test the patches this evening...
- val = pci_read_config8(ctrl->d0, DRAMT);
I suggest using PCI_DEV(0,0,0) instead of the indirection ctrl->d0 all over raminit.c.
That indirection was invented for AMD K8 where in an SMP environment there are several memory controllers with several DIMMs attached to each. But the i810 definitely only supports a single memory controller, and the code can be kept a lot simpler.
Yep, this is probably true for various NBs in v2, e.g. 440BX, i810, i830, and maybe more. That should be an extra patch though.
Also, your code only treats single channel configurations. Is the i810 capable of dual channel operation?
- /* TODO: This needs to be set according to the DRAM tech
- /* TODO: This needs to be calculated according to the DRAM tech
Don't think this can really be calculated. Looking at your findings below, you should probably use a table for this and look up the correct values from the table.
* (x8, x16, or x32). Argh, Intel provides no docs on this! * Currently, it needs to be pulled from the output of * lspci -xxx Rx92
* here are some common results:* value: tom: config:* 0x3356 256MB 1 x 256MB DIMM(s), dual sided* 0x0001 512MB 2 x 256MB DIMM(s), dual sided* 0x77da 128MB 1 x 128MB DIMM(s), single sided* 0x55c6 128MB 2 x 128MB DIMM(s), single sided* 0x3356 128MB 1 x 128MB DIMM(s), dual sided* 0x0001 256MB 2 x 128MB DIMM(s), dual sided
I assume these are gathered from actual hardware on your board? Or did you find them in the datasheet somewhere?
- /* setting Vendor/Device ids - there must be a better way of doing
* this...*/- /* set vendor id */
- val = pci_read_config16(ctrl->d0, 0);
- pci_write_config16(ctrl->d0, 0x2c, val);
- /* set device id */
- val = pci_read_config16(ctrl->d0, 2);
- pci_write_config16(ctrl->d0, 0x2e, val);
Yes, during pci_set_subsystem_ids in northbridge.c:
Yep, this should definately be moved.
Uwe.