Patchwork consolidate K8M890 VGA code

login
register
about
Submitter Rudolf Marek
Date 2009-10-28 21:53:41
Message ID <4AE8BD65.7050209@assembler.cz>
Download mbox | patch
Permalink /patch/507/
State Superseded
Headers show

Comments

Rudolf Marek - 2009-10-28 21:53:41
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello all,

Following patch changes the K8M890 VGA handling. It reverts the framebuffer size
to option based (similar what Uwe did) and also it uses GFXUMA to handle the
high_tables_start offset from memory top.

It adds HAVE_MOTHERBOARD_RESOURCES to kconfig because we do have the resources ;)

Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrooN8ACgkQ3J9wPJqZRNUGeACeM7QCbwiHj6QpTDaCojxC8NjK
Qy0AnAl61XJfK/UB1NQOd4gB5HYJ8pJV
=f8uT
-----END PGP SIGNATURE-----
Myles Watson - 2009-10-28 22:44:50
fbbits = ((log2(uma_memory_size >> 20) - 2) << 4);
Why not use CONFIG_VIDEO_MB here?  If it is going to be fixed at compile
time, it seems like you can get rid of uma_memory_size.

Thanks,
Myles
Myles Watson - 2009-10-28 23:00:11
On Wed, Oct 28, 2009 at 4:44 PM, Myles Watson <mylesgw@gmail.com> wrote:

>     fbbits = ((log2(uma_memory_size >> 20) - 2) << 4);
> Why not use CONFIG_VIDEO_MB here?  If it is going to be fixed at compile
> time, it seems like you can get rid of uma_memory_size.
>
Either way:

Acked-by: Myles Watson <mylesgw@gmail.com>

Thanks,
Myles
Luc Verhaegen - 2009-10-28 23:13:54
On Wed, Oct 28, 2009 at 10:53:41PM +0100, Rudolf Marek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hello all,
> 
> Following patch changes the K8M890 VGA handling. It reverts the framebuffer size
> to option based (similar what Uwe did) and also it uses GFXUMA to handle the
> high_tables_start offset from memory top.
> 
> It adds HAVE_MOTHERBOARD_RESOURCES to kconfig because we do have the resources ;)
> 
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>
> 
> Rudolf

Wtf?

Do you really want to get rid of dynamically being able to set the FB 
size?

Why?

Don't you think that this is a feature that regular users of probably 
the only fully implemented coreboot motherboards might actually want to 
touch?

Why is no time being spent on removing the other options that actually 
harm booting this device? Why does this feel like more pointless 
pedanticity?

Luc Verhaegen.
Luc Verhaegen - 2009-10-28 23:18:08
On Wed, Oct 28, 2009 at 05:00:11PM -0600, Myles Watson wrote:
> On Wed, Oct 28, 2009 at 4:44 PM, Myles Watson <mylesgw@gmail.com> wrote:
> 
> >     fbbits = ((log2(uma_memory_size >> 20) - 2) << 4);
> > Why not use CONFIG_VIDEO_MB here?  If it is going to be fixed at compile
> > time, it seems like you can get rid of uma_memory_size.
> >
> Either way:
> 
> Acked-by: Myles Watson <mylesgw@gmail.com>
> 
> Thanks,
> Myles

NACK.

WHY?

Luc Verhaegen.

Patch

Index: src/southbridge/via/k8t890/Kconfig
===================================================================
--- src/southbridge/via/k8t890/Kconfig	(revision 4874)
+++ src/southbridge/via/k8t890/Kconfig	(working copy)
@@ -1,3 +1,35 @@ 
 config SOUTHBRIDGE_VIA_K8T890
 	bool
 
+
+config SOUTHBRIDGE_VIA_K8T890_VGA_EN
+	bool "Enable onboard K8M890 graphics"
+	default y
+	depends on SOUTHBRIDGE_VIA_K8T890
+	select VGA
+	select GFXUMA
+
+choice
+	prompt "Framebuffer size"
+	default K8M890_VIDEO_MB_32MB
+	depends on SOUTHBRIDGE_VIA_K8T890_VGA_EN
+
+config K8M890_VIDEO_MB_32MB
+	bool "32MB"
+config K8M890_VIDEO_MB_64MB
+	bool "64MB"
+config K8M890_VIDEO_MB_128MB
+	bool "128MB"
+config K8M890_VIDEO_MB_256MB
+	bool "256MB"
+
+endchoice
+
+config VIDEO_MB
+	int
+	default 0   if K8M890_VIDEO_MB_OFF
+	default 32  if K8M890_VIDEO_MB_32MB
+	default 64  if K8M890_VIDEO_MB_64MB
+	default 128  if K8M890_VIDEO_MB_128MB
+	default 256 if K8M890_VIDEO_MB_256MB
+	depends on SOUTHBRIDGE_VIA_K8T890_VGA_EN
Index: src/southbridge/via/k8t890/k8t890_dram.c
===================================================================
--- src/southbridge/via/k8t890/k8t890_dram.c	(revision 4874)
+++ src/southbridge/via/k8t890/k8t890_dram.c	(working copy)
@@ -67,47 +67,25 @@ 
 
 }
 
+#if CONFIG_GFXUMA
+extern uint64_t uma_memory_base, uma_memory_size;
+#endif
+
 static void dram_enable_k8m890(struct device *dev)
 {
-	dram_enable(dev);
-
+#if CONFIG_GFXUMA
+	msr_t msr;
+	uma_memory_size = (CONFIG_VIDEO_MB << 20);
+	msr = rdmsr(TOP_MEM);
+	uma_memory_base = msr.lo - uma_memory_size;
+	printk_info("K8M890: UMA base is  %llx\n", uma_memory_base);
 	/* enable VGA, so the bridges gets VGA_EN and resources are set */
 	pci_write_config8(dev, 0xa1, 0x80);
-}
-
-static struct resource *resmax;
-
-static void get_memres(void *gp, struct device *dev, struct resource *res)
-{
-	unsigned int *fbsize = (unsigned int *) gp;
-	uint64_t proposed_base = res->base + res->size - *fbsize;
-
-	printk_debug("get_memres: res->base=%llx res->size=%llx %d %d %d\n",
-			res->base, res->size, (res->size > *fbsize), 
-			(!(proposed_base & (*fbsize - 1))),
-			(proposed_base < ((uint64_t) 0xffffffff)));
-
-	/* if we fit and also align OK, and must be below 4GB */
-	if ((res->size > *fbsize) && (!(proposed_base & (*fbsize - 1))) && 
-		(proposed_base < ((uint64_t) 0xffffffff) )) {
-		resmax = res;
-	}
-#if CONFIG_WRITE_HIGH_TABLES==1
-/* in arch/i386/boot/tables.c */
-extern uint64_t high_tables_base, high_tables_size;
-
-	if ((high_tables_base) && ((high_tables_base > proposed_base) &&
-			(high_tables_base < (res->base + res->size)))) {
-		high_tables_base = proposed_base - high_tables_size;
-		printk_debug("Moving the high_tables_base pointer to "
-				"new base %llx\n", high_tables_base);
-	}
 #endif
+	dram_enable(dev);
+
 }
 
-/*
- *
- */
 int
 k8m890_host_fb_size_get(void)
 {
@@ -125,57 +103,29 @@ 
 
 static void dram_init_fb(struct device *dev)
 {
+#if CONFIG_GFXUMA
 	/* Important bits:
 	 * Enable the internal GFX bit 7 of reg 0xa1 plus in same reg:
 	 * bits 6:4 X fbuffer size will be  2^(X+2) or 100 = 64MB, 101 = 128MB
 	 * bits 3:0 BASE [31:28]
 	 * reg 0xa0 bits 7:1 BASE [27:21] bit0 enable CPU access
 	 */
-	u8 tmp;
-	uint64_t proposed_base;
 	unsigned int fbbits = 0;
-	unsigned int fbsize;
+	u8 tmp;
 	int ret;
 
+	fbbits = ((log2(uma_memory_size >> 20) - 2) << 4);
+	printk_info("K8M890: Using a %dMB framebuffer.\n", (unsigned int) (uma_memory_size >> 20));
 
-	ret = get_option(&fbbits, "videoram_size");
-	if (ret) {
-		printk_warning("Failed to get videoram size (error %d), using default.\n", ret);
-		fbbits = 5;
- 	}
-
-	if ((fbbits < 1) || (fbbits > 7)) {
-		printk_warning("Invalid videoram size (%d), using default.\n",
-			       4 << fbbits);
-		fbbits = 5;
-	}
-
-	fbsize = 4 << (fbbits + 20);
-
-	resmax = NULL;
-	search_global_resources(
-                IORESOURCE_MEM | IORESOURCE_CACHEABLE, IORESOURCE_MEM | IORESOURCE_CACHEABLE,
-                get_memres, (void *) &fbsize);
-
-	/* no space for FB */
-	if (!resmax) {
-		printk_err("VIA FB: no space for framebuffer in RAM\n");
-		return;
-	}
-
-	proposed_base = resmax->base + resmax->size - fbsize;
-	resmax->size -= fbsize;
-
-	printk_info("K8M890: Using a %dMB framebuffer.\n", 4 << fbbits);
-
 	/* Step 1: enable UMA but no FB */
 	pci_write_config8(dev, 0xa1, 0x80);
 
 	/* Step 2: enough is just the FB size, the CPU accessible address is not needed */
-	tmp = (fbbits << 4) | 0x80;
+	tmp = fbbits | 0x80;
 	pci_write_config8(dev, 0xa1, tmp);
 
 	/* TODO K8 needs some UMA fine tuning too maybe call some generic routine here? */
+#endif
 }
 
 static const struct device_operations dram_ops_t = {
Index: src/mainboard/asus/m2v-mx_se/Kconfig
===================================================================
--- src/mainboard/asus/m2v-mx_se/Kconfig	(revision 4874)
+++ src/mainboard/asus/m2v-mx_se/Kconfig	(working copy)
@@ -34,7 +34,7 @@ 
 	select HAVE_OPTION_TABLE
 	select HAVE_ACPI_TABLES
 	select BOARD_ROMSIZE_KB_512
-	select VGA
+	select HAVE_MAINBOARD_RESOURCES
 
 config MAINBOARD_DIR
 	string
Index: src/mainboard/asus/m2v-mx_se/Options.lb
===================================================================
--- src/mainboard/asus/m2v-mx_se/Options.lb	(revision 4874)
+++ src/mainboard/asus/m2v-mx_se/Options.lb	(working copy)
@@ -72,6 +72,8 @@ 
 uses CONFIG_GDB_STUB
 uses CONFIG_VGA
 uses CONFIG_CONSOLE_VGA
+uses CONFIG_VIDEO_MB
+uses CONFIG_GFXUMA
 uses CONFIG_PCI_ROM_RUN
 # bx_b001- uses K8_HW_MEM_HOLE_SIZEK
 uses CONFIG_K8_HT_FREQ_1G_SUPPORT
@@ -133,6 +135,8 @@ 
 default CONFIG_SB_HT_CHAIN_UNITID_OFFSET_ONLY = 0
 
 default CONFIG_VGA = 1
+default CONFIG_VIDEO_MB = 32
+default CONFIG_GFXUMA = 1
 default CONFIG_CONSOLE_VGA = 1		# Needed for VGA.
 default CONFIG_PCI_ROM_RUN = 0		# Needed for VGA.
 default CONFIG_USE_DCACHE_RAM = 1
Index: src/mainboard/asus/m2v-mx_se/mainboard.c
===================================================================
--- src/mainboard/asus/m2v-mx_se/mainboard.c	(revision 4874)
+++ src/mainboard/asus/m2v-mx_se/mainboard.c	(working copy)
@@ -22,6 +22,12 @@ 
 #include <device/pci_ids.h>
 #include <boot/tables.h>
 #include "chip.h"
+
+
+#if CONFIG_GFXUMA
+uint64_t uma_memory_base, uma_memory_size;
+#endif
+
 #include <../../../southbridge/via/k8t890/k8t890.h>
 
 int add_mainboard_resources(struct lb_memory *mem)
@@ -38,6 +44,11 @@ 
 					    res->size);
 	}
 
+#if (CONFIG_GFXUMA == 1)
+	lb_add_memory_range(mem, LB_MEM_RESERVED,
+		uma_memory_base, uma_memory_size);
+#endif
+
 #if CONFIG_HAVE_ACPI_RESUME == 1
 	lb_add_memory_range(mem, LB_MEM_RESERVED,
 		CONFIG_RAMBASE, ((CONFIG_RAMTOP) - CONFIG_RAMBASE));