Patchwork consolidate K8M890 VGA code - take2

login
register
about
Submitter Rudolf Marek
Date 2010-03-06 13:42:15
Message ID <4B925BB7.2090301@assembler.cz>
Download mbox | patch
Permalink /patch/1018/
State Accepted
Headers show

Comments

Rudolf Marek - 2010-03-06 13:42:15
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

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.

To satisfy the CMOS option users (Hi, libv! ;) I added also a possibility to do
that through CMOS.

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

Works here, maybe delete that old version from patchwork (from 28.10.2009 22:53)?

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

iEYEARECAAYFAkuSW7cACgkQ3J9wPJqZRNWa3wCgj5Fv/0CkqJXdtSWlvK9X2DW+
i7wAniOA+1D8GBoNjv3VYrbh2xyn+THB
=bTQ6
-----END PGP SIGNATURE-----
Rudolf Marek - 2010-03-24 12:30:06
Ack anyone?

Rudolf
Stefan Reinauer - 2010-03-24 12:58:14
On 3/6/10 2:42 PM, Rudolf Marek wrote:
> 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.
>
> To satisfy the CMOS option users (Hi, libv! ;) I added also a
> possibility to do
> that through CMOS.
>
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>
>
Can't test, but I like the general direction.

Acked-by: Stefan Reinauer <stepan@coresystems.de>
Myles Watson - 2010-03-24 13:01:38
2010/3/6 Rudolf Marek <r.marek@assembler.cz>

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> 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.
>
> To satisfy the CMOS option users (Hi, libv! ;) I added also a possibility
> to do
> that through CMOS.
>
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>
>

+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"
+config K8M890_VIDEO_MB_CMOS
+    bool "Use CMOS option"

Shouldn't there be a config K8M890_VIDEO_MB_OFF choice?

+endchoice
+
+config VIDEO_MB
+    int
+    default 0   if K8M890_VIDEO_MB_OFF

I didn't see any comments documenting the meaning of fbbits.  At first I
thought it  was log2 of the MB of memory....

+        uma_memory_size = 4 << (fbbits + 20);
+    } else {
+        uma_memory_size = (CONFIG_VIDEO_MB << 20);
     }

And this definition looks a little different, but maybe it's equivalent?

+    fbbits = ((log2(uma_memory_size >> 20) - 2) << 4);
+    printk_info("K8M890: Using a %dMB framebuffer.\n", (unsigned int)
(uma_memory_size >> 20));

It seems like clarity would be good, since it's a user option.

With a few minor things addressed:

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

Thanks,
Myles

Patch

Index: src/southbridge/via/k8t890/Kconfig
===================================================================
--- src/southbridge/via/k8t890/Kconfig	(revision 5169)
+++ src/southbridge/via/k8t890/Kconfig	(working copy)
@@ -1,3 +1,38 @@ 
 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"
+config K8M890_VIDEO_MB_CMOS
+	bool "Use CMOS option"
+
+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
+	default -1  if K8M890_VIDEO_MB_CMOS
+	depends on SOUTHBRIDGE_VIA_K8T890_VGA_EN
+
Index: src/southbridge/via/k8t890/k8t890_dram.c
===================================================================
--- src/southbridge/via/k8t890/k8t890_dram.c	(revision 5169)
+++ src/southbridge/via/k8t890/k8t890_dram.c	(working copy)
@@ -67,47 +67,44 @@ 
 
 }
 
+#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;
+	int ret;
+	unsigned int fbbits;
 
-	/* enable VGA, so the bridges gets VGA_EN and resources are set */
-	pci_write_config8(dev, 0xa1, 0x80);
-}
+	/* use CMOS */
+	if (CONFIG_VIDEO_MB == -1) {
+		ret = get_option(&fbbits, "videoram_size");
+		if (ret) {
+			printk_warning("Failed to get videoram size (error %d), using default.\n", ret);
+			fbbits = 5;
+	 	}
 
-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 ((fbbits < 1) || (fbbits > 7)) {
+			printk_warning("Invalid videoram size (%d), using default.\n",
+				       4 << fbbits);
+			fbbits = 5;
 	}
-#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);
+		uma_memory_size = 4 << (fbbits + 20);
+	} else {
+		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 size is %d (MB)\n", uma_memory_base, uma_memory_size / 1024 / 1024);
+	/* enable VGA, so the bridges gets VGA_EN and resources are set */
+	pci_write_config8(dev, 0xa1, 0x80);
 #endif
+	dram_enable(dev);
 }
 
-/*
- *
- */
 int
 k8m890_host_fb_size_get(void)
 {
@@ -125,57 +122,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/mainboard.c
===================================================================
--- src/mainboard/asus/m2v-mx_se/mainboard.c	(revision 5169)
+++ 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));