Patchwork [commit] r6127 - in trunk/src: mainboard/asus/p2b northbridge/intel/i440bx/acpi southbridge/intel/i82371eb southbridge/intel/i82371eb/acpi

login
register
about
Submitter Tobias Diedrich
Date 2010-12-01 18:10:57
Message ID <20101201181057.GX32428@yumi.tdiedrich.de>
Download mbox | patch
Permalink /patch/2374/
State Accepted
Headers show

Comments

Tobias Diedrich - 2010-12-01 18:10:57
Tobias Diedrich wrote:
> Definitively a iasl problem, it can't even disassemble it's own
> output back to something equivalent to the input file.
> It seems to be generating Bytecode for the Add where it shouldn't.

Here is a solution using the SSDT.

Unfortunately iasl does not resolve simple arithmetic at compile
time, so we can not use Add(DEFAULT_PMBASE, PCNTRL) in the
Processor statement.
This patch instead dynamically generates the processor statement.
I can't use the speedstep generate_cpu_entries() directly since the
cpu doesn't support speedstep.
For now the code is in the southbridge directory, but maybe it
should go into cpu/intel/ somewhere.
IIRC notebook cpus of the era can already have speedstep, so it
would probably be possible to pair the i82371eb with a
speedstep-capable cpu...
Also, I don't know if multiprocessor boards (abit bp6?) would need
to be handled differently.

Abuild-tested.

Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>

---
Peter Stuge - 2010-12-01 20:42:01
Tobias Diedrich wrote:
> Here is a solution using the SSDT.
> 
> Unfortunately iasl does not resolve simple arithmetic at compile
> time, so we can not use Add(DEFAULT_PMBASE, PCNTRL) in the
> Processor statement.
> This patch instead dynamically generates the processor statement.
> I can't use the speedstep generate_cpu_entries() directly since the
> cpu doesn't support speedstep.
> For now the code is in the southbridge directory, but maybe it
> should go into cpu/intel/ somewhere.
> IIRC notebook cpus of the era can already have speedstep, so it
> would probably be possible to pair the i82371eb with a
> speedstep-capable cpu...
> Also, I don't know if multiprocessor boards (abit bp6?) would need
> to be handled differently.
> 
> Abuild-tested.
> 
> Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>

Not lovely to generate something that is so simple, but sure, if it
works.

Acked-by: Peter Stuge <peter@stuge.se>
Tobias Diedrich - 2010-12-08 21:47:20
Peter Stuge wrote:
> Tobias Diedrich wrote:
> > Here is a solution using the SSDT.
> > 
> > Unfortunately iasl does not resolve simple arithmetic at compile
> > time, so we can not use Add(DEFAULT_PMBASE, PCNTRL) in the
> > Processor statement.
> > This patch instead dynamically generates the processor statement.
> > I can't use the speedstep generate_cpu_entries() directly since the
> > cpu doesn't support speedstep.
> > For now the code is in the southbridge directory, but maybe it
> > should go into cpu/intel/ somewhere.
> > IIRC notebook cpus of the era can already have speedstep, so it
> > would probably be possible to pair the i82371eb with a
> > speedstep-capable cpu...
> > Also, I don't know if multiprocessor boards (abit bp6?) would need
> > to be handled differently.
> > 
> > Abuild-tested.
> > 
> > Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
> 
> Not lovely to generate something that is so simple, but sure, if it
> works.
> 
> Acked-by: Peter Stuge <peter@stuge.se>

Thanks, committed as r6153.
Damn, three added empty files sneaked in from the other parts of the
quilt patch set, removed by r6154.

Patch

Index: src/mainboard/asus/p2b/dsdt.asl
===================================================================
--- src/mainboard/asus/p2b/dsdt.asl.orig	2010-12-01 17:50:26.000000000 +0100
+++ src/mainboard/asus/p2b/dsdt.asl	2010-12-01 18:29:22.000000000 +0100
@@ -21,15 +21,6 @@ 
 
 DefinitionBlock ("DSDT.aml", "DSDT", 2, "CORE  ", "COREBOOT", 1)
 {
-	/* Define the main processor.*/
-	Scope (\_PR)
-	{
-		/* Looks like the P_CNT field can't be a name or method (except
-		 * builtins like Add()) and has to be hardcoded or generated
-		 * into SSDT */
-		Processor (CPU0, 0x01, Add(DEFAULT_PMBASE, PCNTRL), 0x06) {}
-	}
-
 	/* For now only define 2 power states:
 	 *  - S0 which is fully on
 	 *  - S5 which is soft off
Index: src/southbridge/intel/i82371eb/acpi_tables.c
===================================================================
--- src/southbridge/intel/i82371eb/acpi_tables.c.orig	2010-12-01 17:50:26.000000000 +0100
+++ src/southbridge/intel/i82371eb/acpi_tables.c	2010-12-01 18:30:47.000000000 +0100
@@ -26,9 +26,46 @@ 
 #include <arch/smp/mpspec.h>
 #include <device/device.h>
 #include <device/pci_ids.h>
+#include "i82371eb.h"
 
 extern const unsigned char AmlCode[];
 
+static int determine_total_number_of_cores(void)
+{
+	device_t cpu;
+	int count = 0;
+	for(cpu = all_devices; cpu; cpu = cpu->next) {
+		if ((cpu->path.type != DEVICE_PATH_APIC) ||
+			(cpu->bus->dev->path.type != DEVICE_PATH_APIC_CLUSTER)) {
+			continue;
+		}
+		if (!cpu->enabled) {
+			continue;
+		}
+		count++;
+	}
+	return count;
+}
+
+void generate_cpu_entries(void)
+{
+	int len;
+	int len_pr;
+	int cpu, pcontrol_blk=DEFAULT_PMBASE+PCNTRL, plen=6;
+	int numcpus = determine_total_number_of_cores();
+	printk(BIOS_DEBUG, "Found %d CPU(s).\n", numcpus);
+
+	/* without the outer scope, furhter ssdt addition will end up
+	 * within the processor statement */
+	len = acpigen_write_scope("\\_PR");
+	for (cpu=0; cpu < numcpus; cpu++) {
+		len_pr = acpigen_write_processor(cpu, pcontrol_blk, plen);
+		acpigen_patch_len(len_pr - 1);
+		len += len_pr;
+	}
+	acpigen_patch_len(len - 1);
+}
+
 unsigned long __attribute__((weak)) acpi_fill_slit(unsigned long current)
 {
 	// Not implemented
@@ -57,6 +94,10 @@ 
 						 const char *oem_table_id)
 {
 	acpigen_write_mainboard_resources("\\_SB.PCI0.MBRS", "_CRS");
+	/* generate_cpu_entries() generates weird bytecode and has to come
+	 * last or else the following entries will end up inside the
+	 * processor scope */
+	generate_cpu_entries();
 	return (unsigned long) acpigen_get_current();
 }