Patchwork Fix AMD 690 HIGH_TABLES

Hi,

Myles, this is the patch which fixes HIGH_TABLES with UMA.
Zheng, I think you wanted to test if it fixes HIGH_TABLES for you as well.

On 11.06.2009 02:10, Carl-Daniel Hailfinger wrote:
> On 05.06.2009 12:51, Carl-Daniel Hailfinger wrote:
>   
>> Zheng, can you please test this with HIGH_TABLES enabled (and a new FILO)?
>> A full log would be appreciated. Thanks.
>>
>> This hopefully will fix AMD 690 HIGH_TABLES or at least give me an
>> insight into why HIGH_TABLES fails.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>>   
>>     
>
> I just tested the patch and it works perfectly with a recent FILO.
>
> This patch is not ready to be applied (it depends on AMD 690 code), but
> it illustrates what we need to to to get HIGH_TABLES working on the
> chipsets (usually with UMA graphics) which have problems with that setting.
>
> Basically, HIGH_TABLES are placed in UMA memory for some chipsets and
> that causes them to be overwritten by graphics drivers before they can
> be used.
>
> The preferred way to solve this would be a variable top_low_normal_memk
> which would be marking the upper end of non-UMA non-MMIO non-SMM memory
> below 4G.
>   

Rediffed against current svn.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

On Wed, Oct 14, 2009 at 6:40 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006@gmx.net> wrote:

> Hi,
>
> Myles, this is the patch which fixes HIGH_TABLES with UMA.
>
Thanks.  I've commented in line.


> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>
> Index: LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/northbridge.c
> ===================================================================
> --- LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/northbridge.c
> (Revision 4776)
> +++ LinuxBIOSv2-asus_m2a-vm/src/northbridge/amd/amdk8/northbridge.c
> (Arbeitskopie)
> @@ -839,6 +839,7 @@
>  #if CONFIG_HAVE_HIGH_TABLES==1
>  #define HIGH_TABLES_SIZE 64    // maximum size of high tables in KB
>
 extern uint64_t high_tables_base, high_tables_size;
>
#if CONFIG_SOUTHBRIDGE_AMD_RS690  // I guess this could be board-specific
too, since that's where these variables are defined.

> +extern uint64_t uma_memory_base, uma_memory_size;
>
#endif

>  #endif
>
>  static void amdk8_domain_set_resources(device_t dev)
> @@ -1003,7 +1004,9 @@
>                 }
>
>
> -//             printk_debug("node %d : mmio_basek=%08x, basek=%08x,
> limitk=%08x\n", i, mmio_basek, basek, limitk); //yhlu
> +               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);
>
This will always be true, right?  mmio_basek is the PCI region, which should
always be above the last few MB of RAM.


>
>                /* See if I need to split the region to accomodate pci
> memory space */
>                if ( (basek < 4*1024*1024 ) && (limitk > mmio_basek) ) {
> @@ -1017,7 +1020,7 @@
>  #if CONFIG_HAVE_HIGH_TABLES==1
>                                         if (i==0 && high_tables_base==0) {
>                                        /* Leave some space for ACPI, PIRQ
> and MP tables */
> -                                               high_tables_base =
> (mmio_basek - HIGH_TABLES_SIZE) * 1024;
> +                                               high_tables_base =
> ((uma_memory_base >> 10) - HIGH_TABLES_SIZE) * 1024;
>
No real need to shift it then multiply it again.


>                                                high_tables_size =
> HIGH_TABLES_SIZE * 1024;
>                                                 printk_debug(" split: %dK
> table at =%08llx\n", HIGH_TABLES_SIZE,
>
> high_tables_base);
> @@ -1042,6 +1045,9 @@
>                                 sizek -= (4*1024*1024 - mmio_basek);
>                        }
>                }
> +
> +               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 sizek == 0, it was split at mmio_basek without a hole.
>                 * Don't create an empty ram_resource.
>                 */
> @@ -1053,7 +1059,7 @@
>                              i, mmio_basek, basek, limitk);
>                if (i==0 && high_tables_base==0) {
>                /* Leave some space for ACPI, PIRQ and MP tables */
>
#if

> -                       high_tables_base = (limitk - HIGH_TABLES_SIZE) *
> 1024;
>
#else

> +                       high_tables_base = ((uma_memory_base >> 10) -
> HIGH_TABLES_SIZE) * 1024;
>
#endif

>                        high_tables_size = HIGH_TABLES_SIZE * 1024;
>                }
>  #endif
>
>
> --
> Developer quote of the week:
> "We are juggling too many chainsaws and flaming arrows and tigers."
>
>

On Wed, Oct 14, 2009 at 7:49 PM, Myles Watson <mylesgw@gmail.com> wrote:

>
> #if CONFIG_SOUTHBRIDGE_AMD_RS690  // I guess this could be board-specific
> too, since that's where these variables are defined.
>
>> +extern uint64_t uma_memory_base, uma_memory_size;
>>
>
> I think the nicest thing to do would be to move the definitions of
uma_memory_* into the southbridge code.  It doesn't really belong in the
mainboard.  We could also protect it with CONFIG_GFXUMA.  I'd like to use
CONFIG_VIDEO_MB, but it looks like the code assigns the uma_memory_size
based on the size of main memory.  I guess since it wouldn't brick the
boards we could just use the defines and set it to 128M since that's for 1G
and up.  Are many people running these boards with 256 MB of RAM?

Thanks,
Myles