[PATCH] k8 uma and high tables

[Stefan Reinauer]

See patch. Carl Daniel sent something very similar before... This one is
the same but adds some preprocessing sugar to it so it does the right
thing both for UMA and non-UMA cases

[Carl-Daniel Hailfinger]

On 24.10.2009 03:14, Stefan Reinauer wrote:
> See patch. Carl Daniel sent something very similar before... This one is
> the same but adds some preprocessing sugar to it so it does the right
> thing both for UMA and non-UMA cases

Yay! One less patch to carry around.
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 24.10.2009 03:14, Stefan Reinauer wrote:
> > See patch. Carl Daniel sent something very similar before... This one is
> > the same but adds some preprocessing sugar to it so it does the right
> > thing both for UMA and non-UMA cases
>
> Yay! One less patch to carry around.
> Acked-by: Carl-Daniel Hailfinger
>
> Regards,
> Carl-Daniel

Thanks.. r4835

[Myles Watson]

> +#if CONFIG_GFXUMA == 1
>           printk_debug("node %d : uma_memory_base/1024=0x%08x,
>
> mmio_basek=0x%08x, basek=0x%08x, limitk=0x%08x\n", i, uma_memory_base >> 10,
> mmio_basek, basek, limitk);
>           if ((uma_memory_base >> 10) < mmio_basek)
>                   printk_alert("node %d: UMA memory starts below
>
> mmio_basek\n", i);

Isn't this always true?  It seems like if uma_memory_base > (mmio_basek -
uma_memory_size) it's an error.

As a side note, I think that calculation should be done here, and I don't
think it should depend on how much RAM is installed, but should be user
configurable.

Then we can unify GFXUMA and VGA_MEM_MB -> UMA_MEM_MB.  Then uma_memory_base
can go away too, since it will be mmio_base - VGA_MEM_MB.
> +#else
>  //             printk_debug("node %d : mmio_basek=%08x, basek=%08x,
> limitk=%08x\n", i, mmio_basek, basek, limitk); //yhlu
> +#endif
>            /* See if I need to split the region to accomodate pci
>
> memory space */
>                if ( (basek < 4*1024*1024 ) && (limitk > mmio_basek) ) {
> @@ -1015,7 +1024,11 @@
>  #if CONFIG_WRITE_HIGH_TABLES==1
>                                        if (i==0 && high_tables_base==0) {
>                                        /* Leave some space for ACPI, PIRQ
> and MP tables */
> +#if CONFIG_GFXUMA == 1
>                                           high_tables_base =
>
> ((uma_memory_base >> 10) - HIGH_TABLES_SIZE) *

1024;
>

high_tables_base = uma_memory_base - (HIGH_TABLES_SIZE * 1024);
> +#else
>                                                high_tables_base =
> (mmio_basek - HIGH_TABLES_SIZE) * 1024;
> @@ -1051,7 +1064,11 @@
>                             i, mmio_basek, basek, limitk);
>                if (i==0 && high_tables_base==0)
>                /* Leave some space for ACPI, PIRQ and MP tables */
> +#if CONFIG_GFXUMA == 1
>                   high_tables_base = ((uma_memory_base >> 10) -
>
> HIGH_TABLES_SIZE) * 1024;

same thing.

Thanks,
Myles

[Stefan Reinauer]

Myles Watson wrote:
> +#if CONFIG_GFXUMA == 1
> +               printk_debug("node %d :
> uma_memory_base/1024=0x%08x, mmio_basek=0x%08x, basek=0x%08x,
> limitk=0x%08x\n", i, uma_memory_base >> 10, mmio_basek, basek,
> limitk);
> +               if ((uma_memory_base >> 10) < mmio_basek)
> +                       printk_alert("node %d: UMA memory starts
> below mmio_basek\n", i);
>
> Isn't this always true?  It seems like if uma_memory_base >
> (mmio_basek - uma_memory_size) it's an error.

Can this happen? I have no idea,.. Carl-Daniel came up with the equation
and it worked great for me on the KT690
> As a side note, I think that calculation should be done here, and I
> don't think it should depend on how much RAM is installed, but should
> be user configurable.

I think it depends on the installed RAM because there is a hole below 4G
for PCI devices etc.
> Then we can unify GFXUMA and VGA_MEM_MB -> UMA_MEM_MB.  Then
> uma_memory_base can go away too, since it will be mmio_base - VGA_MEM_MB.

unless uma memory can live above 4G on some chipsets...?
> +#else
>  //             printk_debug("node %d : mmio_basek=%08x,
> basek=%08x, limitk=%08x\n", i, mmio_basek, basek, limitk); //yhlu
> +#endif
>
>                /* See if I need to split the region to accomodate
> pci memory space */
>                if ( (basek < 4*1024*1024 ) && (limitk >
> mmio_basek) ) {
> @@ -1015,7 +1024,11 @@
>  #if CONFIG_WRITE_HIGH_TABLES==1
>                                        if (i==0 &&
> high_tables_base==0) {
>                                        /* Leave some space for
> ACPI, PIRQ and MP tables */
> +#if CONFIG_GFXUMA == 1
> +                                               high_tables_base =
> ((uma_memory_base >> 10) - HIGH_TABLES_SIZE) * 
>
> 1024;
>
> high_tables_base = uma_memory_base - (HIGH_TABLES_SIZE * 1024);

d'uh, yes. Can you commit?
> +#else
>                                                high_tables_base =
> (mmio_basek - HIGH_TABLES_SIZE) * 1024;
>
>
>
> @@ -1051,7 +1064,11 @@
>                             i, mmio_basek, basek, limitk);
>                if (i==0 && high_tables_base==0)
>                /* Leave some space for ACPI, PIRQ and MP tables */
> +#if CONFIG_GFXUMA == 1
> +                       high_tables_base = ((uma_memory_base >>
> 10) - HIGH_TABLES_SIZE) * 1024;
>
> same thing.

Thanks for catching this.

[Myles Watson]

> -----Original Message-----
> From: Stefan Reinauer [mailto:[e-mail]]
> Sent: Saturday, October 24, 2009 7:20 AM
> To: Myles Watson
> Cc: coreboot_mailing list
> Subject: Re: [coreboot] [PATCH] k8 uma and high tables
>
> Myles Watson wrote:
> > +#if CONFIG_GFXUMA == 1
> > +               printk_debug("node %d :
> > uma_memory_base/1024=0x%08x, mmio_basek=0x%08x, basek=0x%08x,
> > limitk=0x%08x\n", i, uma_memory_base >> 10, mmio_basek, basek,
> > limitk);
> > +               if ((uma_memory_base >> 10) < mmio_basek)
> > +                       printk_alert("node %d: UMA memory starts
> > below mmio_basek\n", i);
> >
> > Isn't this always true?  It seems like if uma_memory_base >
> > (mmio_basek - uma_memory_size) it's an error.
>
> Can this happen? I have no idea,.. Carl-Daniel came up with the equation
> and it worked great for me on the KT690

So you don't see that message?  It seems like you'd have to have more than
4G of RAM and have UMA live above 4G for that to happen.
> > As a side note, I think that calculation should be done here, and I
> > don't think it should depend on how much RAM is installed, but should
> > be user configurable.
>
> I think it depends on the installed RAM because there is a hole below 4G
> for PCI devices etc.
> > Then we can unify GFXUMA and VGA_MEM_MB -> UMA_MEM_MB.  Then
> > uma_memory_base can go away too, since it will be mmio_base -
>
> VGA_MEM_MB.
> unless uma memory can live above 4G on some chipsets...?

I don't think we have to worry about that with K8.
> > +                                               high_tables_base =
> > ((uma_memory_base >> 10) - HIGH_TABLES_SIZE) *
> >
> > 1024;
> >
> > high_tables_base = uma_memory_base - (HIGH_TABLES_SIZE * 1024);
>
> d'uh, yes. Can you commit?

Rev 4389.

Thanks,
Myles