Re: [coreboot] [patch] i82810 WIP for fixing VGA and 512MB

[Joseph Smith]

On Mon, 17 Nov 2008 06:13:16 +0200, "Elia Yehuda" 
wrote:
> On Mon, Nov 17, 2008 at 5:30 AM, Joseph Smith

wrote:
> > On Mon, 17 Nov 2008 04:25:34 +0200, "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
> >
> > Sorry I'm going to have to nack your northbridge.patch.
> >
> > Your VGA memory has to be subtracted from tomk in kb,like this:
> >
> > #ifdef CONFIG_VIDEO_MB
> >                /* check for VGA reserved memory
> >                 * possible CONFIG_VIDEO_MB values are 512(kb) and 1(mb)
> >                 */
> >                if (CONFIG_VIDEO_MB == 512) {
> >                        tomk -= 512;
> >                } else if (CONFIG_VIDEO_MB == 1) {
> >                        tomk -= 1024;
> >                } else {
> >                           /* assume no vga if incorrect value */
> >                        tomk == tomk;
> > #endif
>
> the place ive added this is when tomk is still set as MBs. Ofcourse i can
> move
> this a few lines down after tomk is converted to KBs. Ive just put it
> there
> to be
> before the printk_debug("Setting RAM size to %d MB\n", tomk) so we can

see
> how much MBs are subtracted from tomk.

You will have to do that to make the 512 work. Or, what is the point in
showing it in MB's, just for user aesthetics? It would be much more
accurate to show it in kb's correct? Especially if your only subtracting
512kb. I would recomend changing this to kb's in the first place, that is
what tomk stands for (top of memory in kb's).

tomk += ((unsigned long)(translate_i82810_to_mb[drp_value]) * 1024);
>

---------------------------------------------------------------------------------
> /* set System Management RAM Control Register / Graphics Mode Select
>
> */
>     value = pci_read_config8(ctrl->d0, SMRAM);
>     /* Set size for Onboard-VGA framebuffer. */
>     switch (CONFIG_VIDEO_MB) {
>     case 512: /* 512K of memory */
>         val = 0x2;
>         break;
>     case 1: /* 1M of memory */
>         val = 0x3;
>         break;
>     default: /* No VGA memory */
>         /* Preserve bits except for GMS */
>         value &= 0x3f;
>         pci_write_config16(ctrl->d0, SMRAM, value);
>         val = 0x0;
>     }
>         value |= val << 6;
>     /* set AB segment Enabled as SMM RAM */
>     value |= 0x0C;
>     pci_write_config8(ctrl->d0, SMRAM, value);

Yup that looks good, of course I haven't a chance to compaire it to the
datasheet. So make sure you test it. This should go in raminit.c where you
set the SMRAM register.