[coreboot] [PATCH] K8M890 support
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Thu Mar 20 01:38:55 CET 2008
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 <r.marek at assembler.cz>
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 <c-d.hailfinger.devel.2006 at gmx.net>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list