[coreboot] [PATCH 7/7] ASUS M2V support (v2): Add m2v mainboard directory and files

Tobias Diedrich ranma+coreboot at tdiedrich.de
Wed Nov 3 18:03:44 CET 2010


Rudolf Marek wrote:
> On 29.10.2010 14:14, Tobias Diedrich wrote:
>> This adds the m2v directory to src/mainboards/asus and adjusts the Kconfig.
>> Note:
>>
>> I added pci irq routing setup based on pirq tables:
>> pci_fixup_irqs() in irq_tables.c
>>
>> I didn't see any existing functionality that will just take the pirq
>> information and use that to setup pci interrupts.
>> For example, in src/southbridge/via/vt8237r/vt8237r_lpc.c there is
>> some epia specific setup, which may really belong into the
>> corresponding mainboard directory...
>
> Hmm the legacy PIC routing may not work. In linux it could. I never tested that.

I currently working on cleaning up the mainboard part, but I tested
legacy pic routing (which is why I added this function), acpi w/o
apic, acpi with apic and mptables with apic, each in both XP and Linux.

>> +	/* Write SB IOAPIC. */
>> +	current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
>> +				VT8237R_APIC_ID, IO_APIC_ADDR, 0);
>> +
>> +	/* Write NB IOAPIC. */
>> +	current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current,
>> +				K8T890_APIC_ID, K8T890_APIC_BASE, gsi_base);
>
> I never used the VIA system on multicore CPU, dunno what to do if we have 
> in fact more cpus... The IDs should be shifted then.

The IDs are currently hardcoded:

src/southbridge/via/k8t890/k8t890.h:
#define K8T890_APIC_ID          0x3
#define K8T890_APIC_BASE        0xfecc0000

src/southbridge/via/vt8237r/vt8237r.h
#define VT8237R_APIC_ID                  0x2

Which is good for up to dual-core, should be shifted up for
quad-core... Maybe we should change this to 0x10 and 0x11? (up to 16 cores)
Not sure how many bits are available for the ID by default, but I
saw that there is an enable bit for extended ID numbers somewhere.

>> +++ src/mainboard/asus/m2v/dsdt.asl	2010-10-29 14:07:37.000000000 +0200
>> @@ -0,0 +1,967 @@
>> +/*
>> + * This file is part of the coreboot project.
>> + *
>> + * Copyright (C) 2004 Nick Barker<Nick.Barker9 at btinternet.com>
>> + * Copyright (C) 2007 Rudolf Marek<r.marek at assembler.cz>
>> + * Copyright (C) 2010 Tobias Diedrich<ranma+coreboot at tdiedrich.de>
>
> Where you have taken parts of it? Some parts feel like AMD 7xx code? 
> Maybe we miss copyright here?

Portions (e.g. "Remember the OS' IRQ routing choice" are from
src/mainboard/ibase/mb899/acpi/), I'll add "Copyright (C) 2007-2009
coresystems GmbH"

>> + *
>> + * 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; version 2 of the License.
>> + *
>> + * 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
>> + */
>> +
>> +/*
>> + * ISA portions taken from QEMU acpi-dsdt.dsl.
>> + */
>> +
>> +DefinitionBlock ("DSDT.aml", "DSDT", 2, "CORE  ", "COREBOOT", 1)
>> +{
>> +	/* Data to be patched by the BIOS during POST */
>> +	/* FIXME the patching is not done yet! */
>> +	/* Memory related values */
>> +	Name(LOMH, 0x0)	/* Start of unused memory in C0000-E0000 range */
>> +	Name(PBAD, 0x0)	/* Address of BIOS area (If TOM2 != 0, Addr>>  16) */
>> +	Name(PBLN, 0x0)	/* Length of BIOS area */
>
> We dont do patching we do apcigen stuff instead into SSDT.

That's copied from gigabyte/ma78gm/dsdt.asl, I'll remove it, it
isn't used anyway.  As I already said elsewhere, I haven't cleaned
up the mainboard bits yet, especially the acpi stuff.

>> +
>> +	Name(PCBA, 0xE0000000)	/* Base address of PCIe config space */
>
> This cannot be hardcoded.

Ok, then I'll add it to the SSDT generation code and use an
external.

>
>> +	Name(HPBA, 0xFED00000)	/* Base address of HPET table */
>> +
>> +	/* global variables */
>> +	Name(APIC, 0)		// 0=>8259, 1=>IOAPIC
>> +	Name(LINX, 0)
>> +	Name(OSYS, 0x0000)
>> +
>> +	/* very generic stuff */
>> +
>> +	/* Port 80 POST */
>
>> +
>> +	OperationRegion (POST, SystemIO, 0x80, 1)
>> +	Field (POST, ByteAcc, Lock, Preserve)
>> +	{
>> +		DBG0, 8
>> +	}
>> +
>> +	Method (DEBG, 1, NotSerialized)
>> +	{
>> +		Store (Arg0, DBG0)
>> +	}
>> +
>
> I dont think you need this. My philosophy for ACPI code is: Put there 
> only what is needed even no extra bit more.

I thought the POST port might come in handy for debugging, but haven't used it
so far.

>> +	/* _PR CPU0 is dynamically supplied by SSDT */
>> +
>> +	/* For now only define 2 power states:
>> +	 *  - S0 which is fully on
>> +	 *  - S5 which is soft off
>> +	 * Any others would involve declaring the wake up methods.
>> +	 *
>> +	 * Package contents:
>> +	 * ofs len desc
>> +	 * 0   1   Value for PM1a_CNT.SLP_TYP register to enter this system state.
>> +	 * 1   1   Value for PM1b_CNT.SLP_TYP register to enter this system state. To enter any
>> +	 *         given state, OSPM must write the PM1a_CNT.SLP_TYP register before the
>> +	 *         PM1b_CNT.SLP_TYP register.
>> +	 * 2   2   Reserved
>> +	 */
>> +	Name (\_S0, Package () { 0x00, 0x00, 0x00, 0x00 })
>> +	Name (\_S5, Package () { 0x02, 0x02, 0x00, 0x00 })
>> +
>> +	/*
>> +	 * Prepare to sleep
>> +	 *
>> +	 * Arg0 – An Integer containing the value of the sleeping state (1 for S1, 2 for S2, etc.)
>> +	 * Return Value: None
>> +	 */
>> +	Method (_PTS, 1, NotSerialized)
>> +	{
>> +		// FIXME: Not implemented
>> +	}
>> +
>> +	/*
>> +	 * Transition to state
>> +	 *
>> +	 * Arg0 – An Integer containing the value of the sleeping state (1 for S1, 2 for S2, etc.)
>> +	 * Return Value: None
>> +	 */
>> +	Method (_TTS, 1, NotSerialized)
>> +	{
>> +		// FIXME: Not implemented
>> +	}
>> +
>
> get rif of those please

I thought they were required methods, and put them in when I was
trying to get XP to boot with ACPI on. I'll remove them if they are
unneeded.

I probably will have to add them back when I look at suspend to ram support
though...

>> +			Device (PCIE)
>> +			{
>> +				Name (_HID, EisaId ("PNP0C02"))
>> +				Method (_CRS, 0, NotSerialized)
>> +				{
>> +					Name (TMP, ResourceTemplate () {
>> +						Memory32Fixed(ReadOnly,
>> +							0xE0000000,
>
> Sorry this cannot be hardcoded. I dont think you need this at all.

Win Vista/7 might need it for MMCONF.  I'll remove the hardcoding.
Linux is contempt with 0xe0000000-0xefffffff resered in either e820
or acpi, but one must be there for MMCONF / pcie extended config
space.

>> +			Name(CRES, ResourceTemplate() {
>> +				WordBusNumber(ResourceProducer, MinFixed, MaxFixed, PosDecode,
>> +					0x0000,             // Granularity
>> +					0x0000,             // Range Minimum
>> +					0x00FF,             // Range Maximum
>> +					0x0000,             // Translation Offset
>> +					0x0100,             // Length
>> +					,,
>> +				)
>> +				IO(Decode16, 0x0CF8, 0x0CF8, 1,	8)
>> +
>> +				WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
>> +					0x0000,			/* address granularity */
>> +					0x0000,			/* range minimum */
>> +					0x0CF7,			/* range maximum */
>> +					0x0000,			/* translation */
>> +					0x0CF8			/* length */
>> +				)
>> +
>> +				WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
>> +					0x0000,			/* address granularity */
>> +					0x0D00,			/* range minimum */
>> +					0xFFFF,			/* range maximum */
>> +					0x0000,			/* translation */
>> +					0xF300			/* length */
>> +				)
>> +
>> +				Memory32Fixed(READWRITE, 0, 0xA0000, BSMM)
>> +				Memory32Fixed(READONLY, 0x000A0000, 0x00020000, VGAM) 	/* VGA memory space */
>> +				Memory32Fixed(READONLY, 0x000C0000, 0x00020000, EMM1)	/* Assume C0000-E0000 empty */
>> +				Memory32Fixed(READONLY, 0x000E0000, 0x00020000, RDBS)   /* BIOS ROM area */
>> +				/* DRAM Memory from 1MB to TopMem */
>> +				Memory32Fixed(READWRITE, 0x00100000, 0x00000000, DMLO)  /* 1MB to TopMem */
>> +				Memory32Fixed(ReadOnly, 0xE0000000, 0x10000000, MCFG)   /* MMCONFIG area */
>> +				Memory32Fixed(READONLY, 0xF0000000, 0x10000000, MMIO)   /* PCI mapping space */
>> +
>> +#if 0
>> +				/* BIOS space just below 4GB */
>> +				DWORDMemory(
>> +					ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>> +					0x00,			/* Granularity */
>> +					0x00000000,		/* Min */
>> +					0x00000000,		/* Max */
>> +					0x00000000,		/* Translation */
>> +					0x00000001,		/* Max-Min, RLEN */
>> +					,,
>> +					PCBM
>> +				)
>> +
>> +				/* BIOS space just below 16EB */
>> +				QWORDMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>> +					0x00000000,		/* Granularity */
>> +					0x00000000,		/* Min */
>> +					0x00000000,		/* Max */
>> +					0x00000000,		/* Translation */
>> +					0x00000001,		/* Max-Min, RLEN */
>> +					,,
>> +					PEBM
>> +				)
>> +#endif
>> +
>> +				/* DRAM memory from 4GB to TopMem2 */
>> +				QWORDMemory(ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
>> +					0x00000000,		/* Granularity */
>> +					0x00000000,		/* Min */
>> +					0x00000000,		/* Max */
>> +					0x00000000,		/* Translation */
>> +					0x00000001,		/* Max-Min, RLEN */
>> +					,,
>> +					DMHI
>> +				)
>> +
>> +			}) /* End Name(_SB.PCI0.CRES) */
>> +
>> +			External(TOM1) /* 32bit top of memory from SSDT */
>> +			External(TOM2) /* 64bit top of memory from SSDT */
>
>
> Maybe windows will accept only IO resorce without any memory resource. It 
> is PITA. Please try to investigate what we should relly tell here:

Declaring main memory there turned out to be a problem actually.
AFAICS the asus dsdt doesn't declare main memory in ACPI, E820 or
the efi equivalent is used for main memory map according to the ACPI standard
if I read it right.

What needs to be there:
Unused IO port and memory ranges which can be assigned to PCI/PCIE
devices.

If we don't want to declare all legacy ports, using IO 0x400-0xcf8
and 0xd00-0xffff, as well as memory TOM-0xdfffffff and
0xf0000000-first apic? should be ok.

> Name(CRES, ResourceTemplate() {
>                 IO(Decode16, 0x0CF8, 0x0CF8, 1, 8)
>
>                 WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode,  
> EntireRange,
>                     0x0000,         /* address granularity */
>                     0x0000,         /* range minimum */
>                     0x0CF7,         /* range maximum */
>                     0x0000,         /* translation */
>                     0x0CF8          /* length */
>                 )
>
>                 WORDIO(ResourceProducer, MinFixed, MaxFixed, PosDecode,  
> EntireRange,
>                     0x0000,         /* address granularity */
>                     0x0D00,         /* range minimum */
>                     0xFFFF,         /* range maximum */
>                     0x0000,         /* translation */
>                     0xF300          /* length */
>                      )
>             }) /* End Name(_SB.PCI0.CRES) */
>
>             Method(_CRS, 0) {
>                 /* DBGO("\\_SB\\PCI0\\_CRS\n") */
>                 Return(CRES) /* note to change the Name buffer */
>             }  /* end of Method(_SB.PCI0._CRS) */
>         } /* End Device(PCI0)  */
>
>
> If this works for XP it would be nice...

Memory resources are needed too for PCI BARs.

>> +	Scope (\_SB)
>> +	{
>> +		OperationRegion (PCI0.SBRG.PIX0, PCI_Config, 0x55, 0x04)
>> +		OperationRegion (PCI0.SBRG.PIX1, PCI_Config, 0x50, 0x02)
>> +		OperationRegion (PCI0.SBRG.PIX2, PCI_Config, 0x44, 0x02)
>> +		OperationRegion (PCI0.SBRG.PIX3, PCI_Config, 0x67, 0x03)
>> +		OperationRegion (PCI0.SBRG.PIX4, PCI_Config, 0x6C, 0x04)
>> +		OperationRegion (PCI0.SBRG.PIEF, PCI_Config, 0x46, 0x01)
>> +		Field (PCI0.SBRG.PIX0, ByteAcc, NoLock, Preserve)
>> +		{
>> +			    , 4,
>> +			PIRA, 4,
>> +			PIRB, 4,
>> +			PIRC, 4,
>> +			    , 4,
>> +			PIRD, 4,
>> +			    , 4
>> +		}
>> +		Field (PCI0.SBRG.PIX1, ByteAcc, NoLock, Preserve)
>> +		{
>> +			    , 1,
>> +			EP3C, 1,
>> +			EN3C, 1,
>> +			    , 6,
>> +			KBFG, 1
>> +		}
>> +		Field (PCI0.SBRG.PIX2, ByteAcc, NoLock, Preserve)
>> +		{
>> +			PIRE, 4,
>> +			PIRF, 4,
>> +			PIRG, 4,
>> +			PIRH, 4,
>> +		}
>> +		Field (PCI0.SBRG.PIX3, ByteAcc, NoLock, Preserve)
>> +		{
>> +			ENIR, 1,
>> +			IRSD, 1,
>> +			Offset (0x02),
>> +			IRBA, 8
>> +		}
>> +		Field (PCI0.SBRG.PIX4, ByteAcc, NoLock, Preserve)
>> +		{
>> +			PS0E, 1,
>> +			PS1E, 1,
>> +			ROME, 1,
>> +			APCE, 1,
>> +			LPMS, 2,
>> +			MSEN, 1,
>> +			IXEN, 1,
>> +			LPMD, 2,
>> +			MDEN, 1,
>> +			GMEN, 1,
>> +			LPLP, 2,
>> +			LPEN, 1,
>> +			FDEN, 1,
>> +			LPCA, 3,
>> +			CAEN, 1,
>> +			LPCB, 3,
>> +			CBEN, 1,
>> +			LPSB, 2,
>> +			SBEN, 1,
>> +			FDSE, 1,
>> +			Offset (0x04)
>
>
> This code comes from where? You wrote it?
> This looks suspicius. We cannot copy anything from orig bios. I think you 
> dont need legacy IRQ anyway windows should work with APIC only. Please 
> try to keep the ACPI stuff as simple as possible.

That was copied from asus dsdt, good catch. I'll rewrite it from
scratch. (Even though it an barely be called code, it's mostly IO
space defines).

I'd like to have legacy IRQ as well as APIC working.
I think working legacy support is required for ACPI compliance.

>> +/* The content of IT8712F_CONFIG_REG_LDN (index 0x07) must be set to the
>> +   LDN the register belongs to, before you can access the register. */
>> +static void it8712f_sio_write(uint8_t ldn, uint8_t index, uint8_t value)
>> +{
>> +	outb(IT8712F_CONFIG_REG_LDN, SIO_BASE);
>> +	outb(ldn, SIO_DATA);
>> +	outb(index, SIO_BASE);
>> +	outb(value, SIO_DATA);
>> +}
>
>
> I think we have some generic function like pnp_enter_ext_func_mode
> etc please check it.

Ok, I'll have a look.

>> +	/*
>> +	 * it8712f gpio config
>> +	 *
>> +	 * Most importantly this switches pin 91 from
>> +	 * PCIRSTIN# to VIN7.
>> +	 * Note that only PCIRST3# and PCIRST5# are affected
>> +	 * by PCIRSTIN#, the PCIRST1#, PCIRST2#, PCIRST4# are always
>> +	 * direct buffers of #LRESET (low pin count bus reset).
>> +	 * If this is not done All PCIRST are in reset state and the
>> +	 * pcie slots don't initialize.
>> +	 *
>> +	 * pci reset handling:
>> +	 * pin 91: VIN7 (alternate PCIRSTIN#)
>> +	 * pin 48: PCIRST5# / gpio port 5 bit 0
>> +	 * pin 84: PCIRST4# / gpio port 1 bit 0
>> +	 * pin 31: PCIRST1# / gpio port 1 bit 4
>> +	 * pin 33: PCIRST2# / gpio port 1 bit 2
>> +	 * pin 34: PCIRST3# / gpio port 1 bit 1
>> +	 *
>> +	 * PCIRST[0-5]# are connected as follows:
>> +	 * pcirst1# ->  pci bus
>> +	 * pcirst2# ->  ide bus
>> +	 * pcirst3# ->  pcie devices
>> +	 * pcirst4# ->  pcie graphics
>> +	 * pcirst5# ->  maybe n/c (untested)
>
> nice how did you found out?

I configured it for software control, wiggled the bits and saw what
happened. :)

> Btw we usually have sio setup in romstage.c But maybe it makes more sense 
> here if it is not critical.
>
> Maybe resets and voltage setup should be really in romstage.c (so it is 
> called before memory etc is setup).

The pcie resets are not critical, so I'm thinking romstage should
only contain what's strictly necessary at that point.
I tried finding the memory slot voltage setting, but I didn't find
any noticable gpio differences. :/
TODO: Measure the memory slot voltage.

>> +
>> +static void m2v_bus_init(void)
>> +{
>> +	u8 tmp;
>> +
>> +	pci_cf8_conf1.write8(NULL, 0, PCI_DEVFN(0, 0), K8T890_MULTIPLE_FN_EN, 0x01);
>> +	/*
>> +	 * Northbridge pcie bridge 3.3 is not connected to anything, hide it.
>> +	 */
>> +	tmp = pci_cf8_conf1.read8(NULL, 0, PCI_DEVFN(0x0, 5), 0xf0);
>> +	tmp&= ~0x10; /* hide pcie bridge 0:3.3 */
>> +	tmp&= ~0x40; /* hide scratch register function 0:0.6 */
>> +	pci_cf8_conf1.write8(NULL, 0, PCI_DEVFN(0x0, 5), 0xf0, tmp);
>> +	/* Enable southbridge bridges 13.0 and 13.1 */
>> +	pci_cf8_conf1.write8(NULL, 0, PCI_DEVFN(0x11, 7), 0X4F, 0x43);
>
>
> Hmm this most likely shoudl be done with the help of devicetree.cb

I don't see how this can be done with devicetree.cb.

>> +	/*
>> +	 * Mark APIC memory as reserved to get closer to ASUS E820 map
>> +	 */
>> +	lb_add_memory_range(mem, LB_MEM_RESERVED, IO_APIC_ADDR,     0x1000);
>> +	lb_add_memory_range(mem, LB_MEM_RESERVED, K8T890_APIC_BASE, 0x1000);
>> +	/*
>> +	 * Mark BIOS ROM space as reserved
>> +	 */
>> +	lb_add_memory_range(mem, LB_MEM_RESERVED, 0xffc00000, 0x400000);
>
> Dont think this is neccessary.

I think all ranges that have mapped devices and are unavailable for
PCI bars should be marked as reserved in E820 for correctness.
Should probably be done in the chipset code and not in the mainboard
code though.

>> +	return 0;
>> +}
>> +
>> +struct chip_operations mainboard_ops = {
>> +	CHIP_NAME("ASUS M2V")
>> +	.enable_dev = m2v_enable,
>> +};
>> Index: src/mainboard/asus/m2v/mptable.c
>> ===================================================================
>> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
>> +++ src/mainboard/asus/m2v/mptable.c	2010-10-29 14:07:37.000000000 +0200
>
> THis was recently fixed are you using fixed version?

Well, I was the one who fixed it. ;)

>> +#define SB_VFSMAF 0
>
> I think for you would work normal way (without this ldstop_sb. I had 
> troubles with integrated VGA on K8M890.

It's there because I started with a copy of m2v-mx_se/romstage.c.
I'll remove it.

-- 
Tobias						PGP: http://8ef7ddba.uguu.de




More information about the coreboot mailing list