[PATCH] K8M890 support

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello all,

Following patch adds K8M890 support. It initializes the AGP and graphics UMA.
The V-link setup and HT bridge is redone, because VT8237A has it in another
device. So far following combination of chipsets should work:

K8T890CE + VT8237R
K8M890 + VT8237R

I will work on "VT8237A" as next, but trying to get from VIA VT8237S datasheets.

As for the patch, some cosmetics may be needed. What I need is to check that I
did not break anything - it works for me and PCI dump looks OK too. The new part
is untested, but for example lowering total size of mem works. Other additions
are quite simple to check. Comments / suggestions welcome.

Oh and lets state: Signed-off-by: Rudolf Marek

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

iD8DBQFH4ajw3J9wPJqZRNURAnbJAKC2mZqanQCL5s1ne6YUhfQvsMyhTACbBIad
f2xVqzZJ9g27T5eLeAUw95U=
=227K
-----END PGP SIGNATURE-----

[Carl-Daniel Hailfinger]

On 20.03.2008 00:59, Rudolf Marek wrote:
> Following patch adds K8M890 support. It initializes the AGP and
> graphics UMA.
> The V-link setup and HT bridge is redone, because VT8237A has it in
> another
> device. So far following combination of chipsets should work:
>
> K8T890CE + VT8237R
> K8M890 + VT8237R
>
> I will work on "VT8237A" as next, but trying to get from VIA VT8237S
> datasheets.
>
> As for the patch, some cosmetics may be needed. What I need is to
> check that I
> did not break anything - it works for me and PCI dump looks OK too.
> The new part
> is untested, but for example lowering total size of mem works. Other
> additions
> are quite simple to check. Comments / suggestions welcome.
>
> Oh and lets state: Signed-off-by: Rudolf Marek

Great work!

foo
> Index: src/southbridge/via/vt8237r/vt8237r_bridge.c
>
> --- src/southbridge/via/vt8237r/vt8237r_bridge.c	(revision 3148)
> +++ src/southbridge/via/vt8237r/vt8237r_bridge.c	(working copy)
> @@ -1,56 +0,0 @@

This file is removed completely.
> Index: src/southbridge/via/k8t890/k8t890_traf_ctrl.c
>
> --- src/southbridge/via/k8t890/k8t890_traf_ctrl.c	(revision 3169)
> +++ src/southbridge/via/k8t890/k8t890_traf_ctrl.c	(working copy)
> @@ -68,15 +68,15 @@
>    res->flags = IORESOURCE_MEM;
>  }
>
> -static void traf_ctrl_enable(struct device *dev)
> +static void traf_ctrl_enable_generic(struct device *dev)
>  {
>    volatile u32 *apic;
>    u32 data;
> /* Enable D3F1-D3F3, no device2 redirect, enable just one device behind
> /* no device2 redirect, enable just one device behind
> bridge device 2 and device 3).
> */
> pci_write_config8(dev, 0x60, 0x88);
>
> pci_write_config8(dev, 0x60, 0x08);
>
> /* Will enable MMCONFIG later. */
> pci_write_config8(dev, 0x64, 0x23);
>
> @@ -104,16 +104,41 @@
>    apic[4] = (data & 0xF0FFFF) | (K8T890_APIC_ID << 24);
>  }
>
> -static const struct device_operations traf_ctrl_ops = {
> +static void traf_ctrl_enable_kt890(struct device *dev)

KT890 or K8T890? The same problem also happens elsewhere.
> +{
> u8 reg;
> traf_ctrl_enable_generic(dev);
> /* Enable D3F1-D3F3 */
> reg = pci_read_config8(dev, 0x60);
> pci_write_config8(dev, 0x60, 0x80 | reg);
>
> +}
>
> +static const struct device_operations traf_ctrl_ops_m = {
>    .read_resources		= apic_mmconfig_read_resources,
>    .set_resources		= mmconfig_set_resources,
>    .enable_resources	= pci_dev_enable_resources,
> .enable			= traf_ctrl_enable,
> .enable			= traf_ctrl_enable_generic,

*_generic
> .ops_pci		= 0,
>  };
>
> -static const struct pci_driver northbridge_driver __pci_driver = {
> .ops	= &traf_ctrl_ops,
>
> +static const struct device_operations traf_ctrl_ops_t = {
> .read_resources		= apic_mmconfig_read_resources,
> .set_resources		= mmconfig_set_resources,
> .enable_resources	= pci_dev_enable_resources,
> .enable			= traf_ctrl_enable_kt890,

*_kt890
I'm not really happy about the generic/kt890 naming. Sure, it is
efficient, but also a bit confusing. Maybe a simple wrapper for
k8m890 would look better.
> .ops_pci		= 0,
>
> +};
>
> +static const struct pci_driver northbridge_driver_t __pci_driver = {
> .ops	= &traf_ctrl_ops_t,
> .vendor	= PCI_VENDOR_ID_VIA,
> .device	= PCI_DEVICE_ID_VIA_K8T890CE_5,
>
> };
>
> +static const struct pci_driver northbridge_driver_m __pci_driver = {
> .ops	= &traf_ctrl_ops_m,
> .vendor	= PCI_VENDOR_ID_VIA,
> .device	= PCI_DEVICE_ID_VIA_K8M890CE_5,
>
> +};
> Index: src/southbridge/via/k8t890/k8t890_dram.c
> ===================================================================
> --- src/southbridge/via/k8t890/k8t890_dram.c	(revision 3169)
> +++ src/southbridge/via/k8t890/k8t890_dram.c	(working copy)
> @@ -23,6 +23,8 @@
>  #include <console/console.h>
>  #include <cpu/x86/msr.h>
>  #include <cpu/amd/mtrr.h>
> +#include <bitops.h>
> +#include "k8t890.h"
>
> static void dram_enable(struct device *dev)
>  {
> @@ -59,10 +61,73 @@
>    reg = pci_read_config16(dev, 0x88);
>    reg &= 0xf800;
> pci_write_config16(dev, 0x88, (msr.lo >> 24) | reg);
> /* The Address Next to the Last Valid DRAM Address */
> pci_write_config16(dev, 0x88, (msr.lo >> 24) | reg);

The line above is a pure whitespace change (it adds a space at the
beginning of the line).
> }
>
> -static const struct device_operations dram_ops = {
> +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 internal VGA is to be used here:
> * enable the internal GFX bit 7 0xa1
> * 6:4 X fbuffer size will be  2^(X+2) or 100 = 64MB, 101 = 128MB 
> * 3:0 BASE [31:28]
> * 0xa0 7:1 BASE [27:21] bit0 enable CPU access
> */

This comment seems to belong elsewhere (inside the function?) and
I have to admit I don't understand it, maybe due to missing context.
> +static void dram_init_fb(struct device *dev)
> +{
> u8 tmp;
> uint64_t proposed_base;
> unsigned int fbsize = (K8M890_FBSIZEMB * 1024 * 1024);
> resmax = NULL;
> search_global_resources(
>            IORESOURCE_MEM | IORESOURCE_CACHEABLE, IORESOURCE_MEM | IORESOURCE_CACHEABLE,
>            get_memres, (void *) &fbsize);
> /* no space for FB */
> if (!resmax)
> return;

Maybe spit out an error message here.
> proposed_base = resmax->base + resmax->size - fbsize;
> resmax->size -= fbsize;
> printk_debug("VIA FB proposed base: %llx\n", proposed_base);
> /* enable UMA but no FB */
> pci_write_config8(dev, 0xa1, 0x80);
> /* 27:21 goes to 7:1, 0 is enable CPU access */
> tmp = (proposed_base >> 20) | 0x1;
> pci_write_config8(dev, 0xa0, tmp);
> /* 31:28 goes to 3:0 */
> tmp = ((proposed_base >> 28) & 0xf);
> tmp = ((log2(K8M890_FBSIZEMB) - 2) << 4);
> tmp |= 0x80;
> pci_write_config8(dev, 0xa1, tmp);
> /* TODO K8 needs some UMA fine tuning too maybe call some generic routine here? */
>
> +}
>
> +static const struct device_operations dram_ops_t = {
>    .read_resources		= pci_dev_read_resources,
>    .set_resources		= pci_dev_set_resources,
>    .enable_resources	= pci_dev_enable_resources,
> @@ -70,8 +135,23 @@
>    .ops_pci		= 0,
>  };
>
> -static const struct pci_driver northbridge_driver __pci_driver = {
> .ops	= &dram_ops,
>
> +static const struct device_operations dram_ops_m = {
> .read_resources		= pci_dev_read_resources,
> .set_resources		= pci_dev_set_resources,
> .enable_resources	= pci_dev_enable_resources,
> .enable			= dram_enable,
> .init			= dram_init_fb,
> .ops_pci		= 0,
>
> +};
>
> +static const struct pci_driver northbridge_driver_t __pci_driver = {
> .ops	= &dram_ops_t,
> .vendor	= PCI_VENDOR_ID_VIA,
> .device	= PCI_DEVICE_ID_VIA_K8T890CE_3,
>
> };
>
> +static const struct pci_driver northbridge_driver_m __pci_driver = {
> .ops	= &dram_ops_m,
> .vendor	= PCI_VENDOR_ID_VIA,
> .device	= PCI_DEVICE_ID_VIA_K8M890CE_3,
>
> +};
> Index: src/southbridge/via/k8t890/k8t890_ctrl.c
> ===================================================================
> --- src/southbridge/via/k8t890/k8t890_ctrl.c	(revision 3169)
> +++ src/southbridge/via/k8t890/k8t890_ctrl.c	(working copy)
> @@ -23,6 +23,73 @@
>  #include <device/pci_ids.h>
>  #include <console/console.h>
>
> +/* Supports K8M890/KT890 and VT8237R PCI1/Vlink which setup is not in separate
> device (0:11.7) but here

Could you try to rephrase the sentence?
> */
>
> +static void vt8237r_cfg(struct device *dev, struct device *devsb)
> +{
> u8 regm, regm2, regm3;
> device_t devfun3;
> /* Magic init for the VT8237R, it is new 0:11.7 device for newer VT8237A/S VT8251 Chips.

Same here.
>       This is not well documented :/ */
> devfun3 = dev_find_device(PCI_VENDOR_ID_VIA,
> 			   PCI_DEVICE_ID_VIA_K8T890CE_3, 0);
> if (!devfun3)
> 	devfun3 = dev_find_device(PCI_VENDOR_ID_VIA,
> 			   PCI_DEVICE_ID_VIA_K8M890CE_3, 0);
> pci_write_config8(dev, 0x70, 0xc2);
> /* PCI Control */
> pci_write_config8(dev, 0x72, 0xee);
> pci_write_config8(dev, 0x73, 0x01);
> pci_write_config8(dev, 0x74, 0x24);
> pci_write_config8(dev, 0x75, 0x0f);
> pci_write_config8(dev, 0x76, 0x50);
> pci_write_config8(dev, 0x77, 0x08);
> pci_write_config8(dev, 0x78, 0x01);
> /* APIC on HT */
> pci_write_config8(dev, 0x7c, 0x7f);
> pci_write_config8(dev, 0x7f, 0x02);
> /* WARNING: Need to copy some registers from NB (D0F3) to SB (D0F7). */
> regm = pci_read_config8(devfun3, 0x88);	/* Shadow mem CTRL */
> pci_write_config8(dev, 0x57, regm);
> regm = pci_read_config8(devfun3, 0x80);	/* Shadow page C */
> pci_write_config8(dev, 0x61, regm);
> regm = pci_read_config8(devfun3, 0x81);	/* Shadow page D */
> pci_write_config8(dev, 0x62, regm);
> regm = pci_read_config8(devfun3, 0x86);	/* SMM and APIC decoding */
> pci_write_config8(dev, 0xe6, regm);
> regm3 = pci_read_config8(devfun3, 0x82);/* Shadow page E */
> /*
> * All access bits for 0xE0000-0xEFFFF encode as just 2 bits!
> * So the NB reg is quite inconsistent, we expect there only 0xff or 0x00,
> * and write them to 0x63 7-6 but! VIA 8237A has the mirror at 0x64!
> */
> if (regm3 == 0xff)
> regm3 = 0xc0;
> else
> regm3 = 0x0;
> /* Shadow page F + memhole copy */
> regm = pci_read_config8(devfun3, 0x83);
> pci_write_config8(dev, 0x63, regm3 | (regm & 0x3F));
>
> +}
>
> /**
> Setup the V-Link for VT8237R, 8X mode.
> Index: src/southbridge/via/k8t890/k8t890.h
>
> --- src/southbridge/via/k8t890/k8t890.h	(revision 3169)
> +++ src/southbridge/via/k8t890/k8t890.h	(working copy)
> @@ -32,4 +32,7 @@
>  #define K8T890_MMCONFIG_MBAR	0x61
>  #define K8T890_MULTIPLE_FN_EN	0x4f
>
> +/* the FB size in MB min is 8MB max is 512MB */

Maybe a comma/parenthesis here?

/* FB size in MB (min is 8MB max is 512MB) */
> +#define K8M890_FBSIZEMB		64
>
> #endif

With the changes above, the patch is
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel