[coreboot] [ patch] v3 Geode graphics init.

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Feb 21 02:12:01 CET 2008


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 <marc.jones at amd.com>
>
> 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");
>  		pci at 1,0 {
>  			/config/("northbridge/amd/geodelx/pci");
> +			/* Video RAM has to be in 2MB chunks. */
> +			geode_video_mb = "8";
>  		};
>  		pci at 15,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 <rminnich at gmail.com>
> + * 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 <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list