On 21.02.2008 01:09, Marc Jones wrote:
> This patch assumes the vsa/idt patch is in place.
Comments inline.
> Removed CONFIG_VIDE0_MB in Kconfig and set the Geode
> graphics memory size through the Geode northbridge pci dts.
> This is a better way to handle a cpu/mainboard specific value.
>
> Signed-off-by: Marc Jones <[EMAIL PROTECTED]>
>
> Index: coreboot-v3/northbridge/amd/geodelx/geodelx.c
> ===================================================================
> --- coreboot-v3.orig/northbridge/amd/geodelx/geodelx.c 2008-02-20
> 10:07:27.000000000 -0700
> +++ coreboot-v3/northbridge/amd/geodelx/geodelx.c 2008-02-20
> 12:12:07.000000000 -0700
> @@ -1,7 +1,7 @@
> /*
> * This file is part of the coreboot project.
> *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -25,12 +25,14 @@
> #include <device/pci_ids.h>
> #include <msr.h>
> #include <amd_geodelx.h>
> +#include <statictree.h>
> +#include "geodelink.h"
>
> /* Function prototypes */
> extern void chipsetinit(void);
> -extern u32 get_systop(void);
> extern void northbridge_init_early(void);
> -
> +extern void graphics_init(struct northbridge_amd_geodelx_pci_config *nb_pci);
> +extern int sizeram(void);
Do we want sizeram() as a function on all subarches? If yes, we have
to either call it something like sizeram_4G() or we have to change the
return type to u64.
>
> /**
> * Currently not set up.
> @@ -41,6 +43,37 @@
> {
> }
>
> +
> +/**
> + * TODO.
> + *
> + * @return TODO.
> + */
> +u32 get_systop(struct northbridge_amd_geodelx_pci_config *nb_pci)
> +{
> + struct gliutable *gl = NULL;
> + u32 systop;
> + struct msr msr;
> + int i;
> +
> + for (i = 0; gliu0table[i].desc_name != GL_END; i++) {
> + if (gliu0table[i].desc_type == R_SYSMEM) {
> + gl = &gliu0table[i];
> + break;
> + }
> + }
> + if (gl) {
> + msr = rdmsr(gl->desc_name);
> + systop = ((msr.hi & 0xFF) << 24) | ((msr.lo & 0xFFF00000) >> 8);
> + systop += 4 * 1024; /* 4K */
> + } else {
> + systop =
> + ((sizeram() - nb_pci->geode_video_mb) * 1024) - SMM_SIZE -
> 1024;
> + }
> +
> + return systop;
> +}
> +
> /**
> * Initialize the northbridge PCI device.
> * Right now this a no op. We leave it here as a hook for later use.
> @@ -116,6 +149,8 @@
> {
> int idx;
> struct device *mc_dev;
> + struct northbridge_amd_geodelx_pci_config *nb_pci =
> + (struct northbridge_amd_geodelx_pci_config *)dev->device_configuration;
>
> printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);
>
> @@ -127,7 +162,7 @@
> ram_resource(dev, idx++, 0, 640);
> /* 1 MB .. (Systop - 1 MB) (converted to KB) */
You subtract only 1 MB here.
> ram_resource(dev, idx++, 1024,
> - (get_systop() - (1 * 1024 * 1024)) / 1024);
> + (get_systop(nb_pci) - (1 * 1024 * 1024)) / 1024);
> }
>
> phase4_assign_resources(&dev->link[0]);
> @@ -147,6 +182,9 @@
> */
> static void geodelx_pci_domain_phase2(struct device *dev)
> {
> + struct northbridge_amd_geodelx_pci_config *nb_pci =
> + (struct northbridge_amd_geodelx_pci_config *)dev->device_configuration;
> +
> void do_vsmbios(void);
>
> printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);
> @@ -161,8 +199,7 @@
> printk(BIOS_SPEW, "After VSA:\n");
> /* print_conf(); */
>
> -#warning graphics_init is disabled.
> - /* graphics_init(); */
> + graphics_init(nb_pci);
> pci_set_method(dev);
> }
>
> Index: coreboot-v3/northbridge/amd/geodelx/geodelxinit.c
> ===================================================================
> --- coreboot-v3.orig/northbridge/amd/geodelx/geodelxinit.c 2008-02-19
> 10:46:51.000000000 -0700
> +++ coreboot-v3/northbridge/amd/geodelx/geodelxinit.c 2008-02-20
> 10:59:05.000000000 -0700
> @@ -1,7 +1,7 @@
> /*
> * This file is part of the coreboot project.
> *
> - * Copyright (C) 2007 Advanced Micro Devices, Inc.
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -23,63 +23,8 @@
> #include <msr.h>
> #include <cpu.h>
> #include <amd_geodelx.h>
> +#include "geodelink.h"
>
> -/* Function prototypes */
> -
> -struct gliutable {
> - unsigned long desc_name;
> - unsigned short desc_type;
> - unsigned long hi, lo;
> -};
> -
> -static struct gliutable gliu0table[] = {
> - /* 0-7FFFF to MC */
> - {.desc_name = MSR_GLIU0_BASE1,.desc_type = BM,.hi = MSR_MC + 0x0,
> - .lo = 0x0FFF80},
> - /* 80000-9FFFF to MC */
> - {.desc_name = MSR_GLIU0_BASE2,.desc_type = BM,.hi = MSR_MC + 0x0,
> - .lo = (0x80 << 20) + 0x0FFFE0},
> - /* C0000-FFFFF split to MC and PCI (sub decode) A0000-BFFFF
> - * handled by SoftVideo.
> - */
> - {.desc_name = MSR_GLIU0_SHADOW,.desc_type = SC_SHADOW,
> - .hi = MSR_MC + 0x0,.lo = 0x03},
> - /* Catch and fix dynamically. */
> - {.desc_name = MSR_GLIU0_SYSMEM,.desc_type = R_SYSMEM,
> - .hi = MSR_MC,.lo = 0x0},
> - /* Catch and fix dynamically. */
> - {.desc_name = MSR_GLIU0_SMM,.desc_type = BMO_SMM,
> - .hi = MSR_MC,.lo = 0x0},
> - {.desc_name = GLIU0_GLD_MSR_COH,.desc_type = OTHER,
> - .hi = 0x0,.lo = GL0_CPU},
> - {.desc_name = GL_END,.desc_type = GL_END,.hi = 0x0,.lo = 0x0},
> -};
> -
> -static struct gliutable gliu1table[] = {
> - /* 0-7FFFF to MC */
> - {.desc_name = MSR_GLIU1_BASE1,.desc_type = BM,.hi = MSR_GL0 + 0x0,
> - .lo = 0x0FFF80},
> - /* 80000-9FFFF to MC */
> - {.desc_name = MSR_GLIU1_BASE2,.desc_type = BM,.hi = MSR_GL0 + 0x0,
> - .lo = (0x80 << 20) + 0x0FFFE0},
> - /* C0000-Fffff split to MC and PCI (sub decode) */
> - {.desc_name = MSR_GLIU1_SHADOW,.desc_type = SC_SHADOW,
> - .hi = MSR_GL0 + 0x0,.lo = 0x03},
> - /* Catch and fix dynamically. */
> - {.desc_name = MSR_GLIU1_SYSMEM,.desc_type = R_SYSMEM,
> - .hi = MSR_GL0,.lo = 0x0},
> - /* Catch and fix dynamically. */
> - {.desc_name = MSR_GLIU1_SMM,.desc_type = BM_SMM,
> - .hi = MSR_GL0,.lo = 0x0},
> - {.desc_name = GLIU1_GLD_MSR_COH,.desc_type = OTHER,
> - .hi = 0x0,.lo = GL1_GLIU0},
> - /* FooGlue FPU 0xF0 */
> - {.desc_name = MSR_GLIU1_FPU_TRAP,.desc_type = SCIO,
> - .hi = (GL1_GLCP << 29) + 0x0,.lo = 0x033000F0},
> - {.desc_name = GL_END,.desc_type = GL_END,.hi = 0x0,.lo = 0x0},
> -};
> -
> -static struct gliutable *gliutables[] = { gliu0table, gliu1table, 0 };
>
> static struct msrinit clock_gating_default[] = {
> {GLIU0_GLD_MSR_PM, {.hi = 0x00,.lo = 0x0005}},
> @@ -763,10 +708,6 @@
> printk(BIOS_DEBUG, "L2 cache enabled\n");
> }
>
> -#ifndef CONFIG_VIDEO_MB
> -#warning "CONFIG_VIDEO_MB was not defined"
> -#define CONFIG_VIDEO_MB 8
> -#endif
>
> /**
> * Set up all LX cache registers, L1, L2, and x86.
> @@ -790,36 +731,6 @@
> }
>
> /**
> - * TODO.
> - *
> - * @return TODO.
> - */
> -u32 get_systop(void)
> -{
> - struct gliutable *gl = NULL;
> - u32 systop;
> - struct msr msr;
> - int i;
> -
> - for (i = 0; gliu0table[i].desc_name != GL_END; i++) {
> - if (gliu0table[i].desc_type == R_SYSMEM) {
> - gl = &gliu0table[i];
> - break;
> - }
> - }
> - if (gl) {
> - msr = rdmsr(gl->desc_name);
> - systop = ((msr.hi & 0xFF) << 24) | ((msr.lo & 0xFFF00000) >> 8);
> - systop += 4 * 1024; /* 4K */
> - } else {
> - systop =
> - ((sizeram() - CONFIG_VIDEO_MB) * 1024) - SMM_SIZE - 1024;
> - }
> -
> - return systop;
> -}
> -
> -/**
> * Do all the Nasty Bits that have to happen.
> *
> * These can be done once memory is up, but before much else is done.
> Index: coreboot-v3/northbridge/amd/geodelx/grphinit.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ coreboot-v3/northbridge/amd/geodelx/grphinit.c 2008-02-20
> 10:59:05.000000000 -0700
> @@ -0,0 +1,62 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2007-2008 Advanced Micro Devices, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <io.h>
> +#include <amd_geodelx.h>
> +#include <console.h>
> +#include <statictree.h>
> +
> + /*
> + * This function mirrors the Graphics_Init routine in GeodeROM.
> + */
> +void graphics_init(struct northbridge_amd_geodelx_pci_config *nb_pci)
> +{
> + u16 wClassIndex, wData, res;
> +
> + /* SoftVG initialization */
That's scary. Do we really need VSA support even if there is a native
geodelx framebuffer/X driver?
> + printk(BIOS_DEBUG, "Graphics init...\n");
> +
> + /* Call SoftVG with the main configuration parameters. */
> + /* NOTE: SoftVG expects the memory size to be given in 2MB blocks */
> +
> + wClassIndex = (VRC_VG << 8) + VG_CONFIG;
> +
> + /*
> + * Graphics Driver Enabled (13) 0, NO (lets BIOS
> controls the GP)
> + * External Monochrome Card Support(12) 0, NO
> + * Controller Priority Select(11) 1, Primary
> + * Display Select(10:8) 0x0, CRT
> + * Graphics Memory Size(7:1) CONFIG_VIDEO_MB >> 1,
> + * defined in mainboard/../dts
> + * PLL Reference Clock Bypass(0) 0, Default
> + */
> +
> + /* Video RAM has to be given in 2MB chunks
> + * the value is read @ 7:1 (value in 7:0 looks like /2)
Typo?
+ * the value is read @ 7:1 (value in 7:0 looks like *2)
Besides that, being limited to multiples of 2MB for VRAM seems not to mix
well with subtracting only 1MB from systop.
> + * so we can add the real value in megabytes.
> + */
> +
> + wData = VG_CFG_DRIVER | VG_CFG_PRIORITY |
> + VG_CFG_DSCRT | (nb_pci->geode_video_mb & VG_MEM_MASK);
> + vr_write(wClassIndex, wData);
> +
> + res = vr_read(wClassIndex);
> + printk(BIOS_DEBUG, "VRC_VG value: 0x%04x\n", res);
> +}
> Index: coreboot-v3/northbridge/amd/geodelx/geodelink.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ coreboot-v3/northbridge/amd/geodelx/geodelink.h 2008-02-20
> 10:59:05.000000000 -0700
> @@ -0,0 +1,78 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef GEODELINK_H
> +#define GEODELINK_H
> +
> +struct gliutable {
> + unsigned long desc_name;
> + unsigned short desc_type;
> + unsigned long hi, lo;
> +};
> +
> +static struct gliutable gliu0table[] = {
> + /* 0-7FFFF to MC */
> + {.desc_name = MSR_GLIU0_BASE1,.desc_type = BM,.hi = MSR_MC + 0x0,
> + .lo = 0x0FFF80},
> + /* 80000-9FFFF to MC */
> + {.desc_name = MSR_GLIU0_BASE2,.desc_type = BM,.hi = MSR_MC + 0x0,
> + .lo = (0x80 << 20) + 0x0FFFE0},
> + /* C0000-FFFFF split to MC and PCI (sub decode) A0000-BFFFF
> + * handled by SoftVideo.
> + */
> + {.desc_name = MSR_GLIU0_SHADOW,.desc_type = SC_SHADOW,
> + .hi = MSR_MC + 0x0,.lo = 0x03},
> + /* Catch and fix dynamically. */
> + {.desc_name = MSR_GLIU0_SYSMEM,.desc_type = R_SYSMEM,
> + .hi = MSR_MC,.lo = 0x0},
> + /* Catch and fix dynamically. */
> + {.desc_name = MSR_GLIU0_SMM,.desc_type = BMO_SMM,
> + .hi = MSR_MC,.lo = 0x0},
> + {.desc_name = GLIU0_GLD_MSR_COH,.desc_type = OTHER,
> + .hi = 0x0,.lo = GL0_CPU},
> + {.desc_name = GL_END,.desc_type = GL_END,.hi = 0x0,.lo = 0x0},
> +};
> +
> +static struct gliutable gliu1table[] = {
> + /* 0-7FFFF to MC */
> + {.desc_name = MSR_GLIU1_BASE1,.desc_type = BM,.hi = MSR_GL0 + 0x0,
> + .lo = 0x0FFF80},
> + /* 80000-9FFFF to MC */
> + {.desc_name = MSR_GLIU1_BASE2,.desc_type = BM,.hi = MSR_GL0 + 0x0,
> + .lo = (0x80 << 20) + 0x0FFFE0},
> + /* C0000-Fffff split to MC and PCI (sub decode) */
> + {.desc_name = MSR_GLIU1_SHADOW,.desc_type = SC_SHADOW,
> + .hi = MSR_GL0 + 0x0,.lo = 0x03},
> + /* Catch and fix dynamically. */
> + {.desc_name = MSR_GLIU1_SYSMEM,.desc_type = R_SYSMEM,
> + .hi = MSR_GL0,.lo = 0x0},
> + /* Catch and fix dynamically. */
> + {.desc_name = MSR_GLIU1_SMM,.desc_type = BM_SMM,
> + .hi = MSR_GL0,.lo = 0x0},
> + {.desc_name = GLIU1_GLD_MSR_COH,.desc_type = OTHER,
> + .hi = 0x0,.lo = GL1_GLIU0},
> + /* FooGlue FPU 0xF0 */
> + {.desc_name = MSR_GLIU1_FPU_TRAP,.desc_type = SCIO,
> + .hi = (GL1_GLCP << 29) + 0x0,.lo = 0x033000F0},
> + {.desc_name = GL_END,.desc_type = GL_END,.hi = 0x0,.lo = 0x0},
> +};
It's debatable whether gliu0table and gliu1table should really be in a
header file as opposed to a normal .c code file. This max cause additional
warnings from gcc.
If any of the tables are constant, please mark them as const.
> +
> +static struct gliutable *gliutables[] = { gliu0table, gliu1table, 0 };
> +
> +#endif /* GEODELINK_H */
> Index: coreboot-v3/include/arch/x86/amd_geodelx.h
> ===================================================================
> --- coreboot-v3.orig/include/arch/x86/amd_geodelx.h 2008-02-19
> 10:46:52.000000000 -0700
> +++ coreboot-v3/include/arch/x86/amd_geodelx.h 2008-02-20
> 10:59:05.000000000 -0700
> @@ -1254,7 +1254,7 @@
> * @param class_index The register index
> * @return the 16-bit word of data
> */
> -static inline u16 vr_ead(u16 class_index)
> +static inline u16 vr_read(u16 class_index)
> {
> u16 data;
> outl(((u32) VR_UNLOCK << 16) | class_index, VRC_INDEX);
> Index: coreboot-v3/mainboard/pcengines/alix1c/dts
> ===================================================================
> --- coreboot-v3.orig/mainboard/pcengines/alix1c/dts 2008-02-19
> 10:46:52.000000000 -0700
> +++ coreboot-v3/mainboard/pcengines/alix1c/dts 2008-02-20
> 10:59:05.000000000 -0700
> @@ -29,6 +29,8 @@
> /config/("northbridge/amd/geodelx/domain");
> [EMAIL PROTECTED],0 {
> /config/("northbridge/amd/geodelx/pci");
> + /* Video RAM has to be in 2MB chunks. */
> + geode_video_mb = "8";
> };
> [EMAIL PROTECTED],0 {
> /config/("southbridge/amd/cs5536/dts");
> Index: coreboot-v3/northbridge/amd/geodelx/pci
> ===================================================================
> --- coreboot-v3.orig/northbridge/amd/geodelx/pci 2008-02-19
> 10:46:52.000000000 -0700
> +++ coreboot-v3/northbridge/amd/geodelx/pci 2008-02-20 10:59:05.000000000
> -0700
> @@ -2,6 +2,7 @@
> * This file is part of the coreboot project.
> *
> * Copyright (C) 2008 Ronald G. Minnich <[EMAIL PROTECTED]>
> + * Copyright (C) 2008 Advanced Micro Devices, Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -20,5 +21,8 @@
>
> {
> device_operations = "geodelx_north_pci";
> +
> + /* Video RAM has to be in 2MB chunks. */
> + geode_video_mb = "0";
> };
>
> Index: coreboot-v3/northbridge/amd/geodelx/Makefile
> ===================================================================
> --- coreboot-v3.orig/northbridge/amd/geodelx/Makefile 2008-02-20
> 11:34:34.000000000 -0700
> +++ coreboot-v3/northbridge/amd/geodelx/Makefile 2008-02-20
> 11:35:28.000000000 -0700
> @@ -23,6 +23,7 @@
>
> STAGE2_CHIPSET_OBJ += $(obj)/northbridge/amd/geodelx/geodelx.o \
> $(obj)/northbridge/amd/geodelx/vsmsetup.o \
> - $(obj)/util/x86emu/vm86_gdt.o
> + $(obj)/util/x86emu/vm86_gdt.o \
> + $(obj)/northbridge/amd/geodelx/grphinit.o
>
> endif
With my comments addressed, the patch is
Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
--
coreboot mailing list
coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
