Angel Pons submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
soc/intel/broadwell/pch: Replace ACPI device NVS

The same functionality can be provided through a runtime-generated SSDT.
The remaining parts of device NVS are removed in a follow-up.

Since the SSDTs are only loaded after the DSDT (if loaded at all), using
SSDT-provided objects outside method bodies is not possible: the objects
are not yet in OSPM's ACPI namespace, which causes in ACPI errors. Owing
to this, the operation regions used by the _PS0 and _PS3 methods need to
be moved into the SSDT, as they depend on the SSDT-provided BAR1 values.

Tested on out-of-tree Compal LA-A992P, generated SSDT disassembles with
no errors and contains expected values. Linux does not complain either.

Change-Id: I89fb658fbb10a8769ebea2e6535c45cd7c212d06
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52520
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
---
M src/mainboard/google/jecht/dsdt.asl
M src/mainboard/intel/wtm2/dsdt.asl
D src/soc/intel/broadwell/acpi/device_nvs.asl
M src/soc/intel/broadwell/acpi/platform.asl
M src/soc/intel/broadwell/include/soc/pch.h
M src/soc/intel/broadwell/pch/Makefile.inc
A src/soc/intel/broadwell/pch/acpi.c
M src/soc/intel/broadwell/pch/acpi/adsp.asl
M src/soc/intel/broadwell/pch/acpi/serialio.asl
M src/soc/intel/broadwell/pch/lpc.c
10 files changed, 164 insertions(+), 163 deletions(-)

diff --git a/src/mainboard/google/jecht/dsdt.asl b/src/mainboard/google/jecht/dsdt.asl
index 220caee..3f7b9ae 100644
--- a/src/mainboard/google/jecht/dsdt.asl
+++ b/src/mainboard/google/jecht/dsdt.asl
@@ -19,7 +19,6 @@

// global NVS and variables
#include <soc/intel/broadwell/pch/acpi/globalnvs.asl>
- #include <soc/intel/broadwell/acpi/device_nvs.asl>

// CPU
#include <cpu/intel/common/acpi/cpu.asl>
diff --git a/src/mainboard/intel/wtm2/dsdt.asl b/src/mainboard/intel/wtm2/dsdt.asl
index a968c2d..bb32b47 100644
--- a/src/mainboard/intel/wtm2/dsdt.asl
+++ b/src/mainboard/intel/wtm2/dsdt.asl
@@ -19,7 +19,6 @@

// global NVS and variables
#include <soc/intel/broadwell/pch/acpi/globalnvs.asl>
- #include <soc/intel/broadwell/acpi/device_nvs.asl>

// CPU
#include <cpu/intel/common/acpi/cpu.asl>
diff --git a/src/soc/intel/broadwell/acpi/device_nvs.asl b/src/soc/intel/broadwell/acpi/device_nvs.asl
deleted file mode 100644
index fb95df8..0000000
--- a/src/soc/intel/broadwell/acpi/device_nvs.asl
+++ /dev/null
@@ -1,40 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-Field (DNVS, ByteAcc, NoLock, Preserve)
-{
- /* Device enables in ACPI mode */
-
- S0EN, 8, // DMA Enable
- S1EN, 8, // I2C0 Enable
- S2EN, 8, // I2C1 Enable
- S3EN, 8, // SPI0 Enable
- S4EN, 8, // SPI1 Enable
- S5EN, 8, // UART0 Enable
- S6EN, 8, // UART1 Enable
- S7EN, 8, // SDIO Enable
- S8EN, 8, // ADSP Enable
-
- /* BAR 0 */
-
- S0B0, 32, // DMA BAR0
- S1B0, 32, // I2C0 BAR0
- S2B0, 32, // I2C1 BAR0
- S3B0, 32, // SPI0 BAR0
- S4B0, 32, // SPI1 BAR0
- S5B0, 32, // UART0 BAR0
- S6B0, 32, // UART1 BAR0
- S7B0, 32, // SDIO BAR0
- S8B0, 32, // ADSP BAR0
-
- /* BAR 1 */
-
- S0B1, 32, // DMA BAR1
- S1B1, 32, // I2C0 BAR1
- S2B1, 32, // I2C1 BAR1
- S3B1, 32, // SPI0 BAR1
- S4B1, 32, // SPI1 BAR1
- S5B1, 32, // UART0 BAR1
- S6B1, 32, // UART1 BAR1
- S7B1, 32, // SDIO BAR1
- S8B1, 32, // ADSP BAR1
-}
diff --git a/src/soc/intel/broadwell/acpi/platform.asl b/src/soc/intel/broadwell/acpi/platform.asl
index fe254ff..880b206 100644
--- a/src/soc/intel/broadwell/acpi/platform.asl
+++ b/src/soc/intel/broadwell/acpi/platform.asl
@@ -1,6 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */

-#include <soc/intel/broadwell/acpi/device_nvs.asl>
#include <southbridge/intel/common/acpi/platform.asl>

/*
diff --git a/src/soc/intel/broadwell/include/soc/pch.h b/src/soc/intel/broadwell/include/soc/pch.h
index cf27499..8089f5b 100644
--- a/src/soc/intel/broadwell/include/soc/pch.h
+++ b/src/soc/intel/broadwell/include/soc/pch.h
@@ -3,6 +3,8 @@
#ifndef _BROADWELL_PCH_H_
#define _BROADWELL_PCH_H_

+#include <acpi/acpi.h>
+
/* Haswell ULT Pch (LynxPoint-LP) */
#define PCH_LPT_LP_SAMPLE 0x9c41
#define PCH_LPT_LP_PREMIUM 0x9c43
@@ -30,6 +32,8 @@
u32 pch_read_soft_strap(int id);
void pch_disable_devfn(struct device *dev);

+void acpi_create_serialio_ssdt(acpi_header_t *ssdt);
+
void broadwell_pch_finalize(void);

#endif
diff --git a/src/soc/intel/broadwell/pch/Makefile.inc b/src/soc/intel/broadwell/pch/Makefile.inc
index 1afa92b..4e8384f 100644
--- a/src/soc/intel/broadwell/pch/Makefile.inc
+++ b/src/soc/intel/broadwell/pch/Makefile.inc
@@ -1,5 +1,6 @@
bootblock-y += bootblock.c

+ramstage-y += acpi.c
ramstage-y += adsp.c
romstage-y += early_pch.c
ramstage-$(CONFIG_ELOG) += elog.c
diff --git a/src/soc/intel/broadwell/pch/acpi.c b/src/soc/intel/broadwell/pch/acpi.c
new file mode 100644
index 0000000..85726b0
--- /dev/null
+++ b/src/soc/intel/broadwell/pch/acpi.c
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <acpi/acpi.h>
+#include <acpi/acpi_gnvs.h>
+#include <acpi/acpigen.h>
+#include <soc/device_nvs.h>
+#include <soc/pch.h>
+#include <types.h>
+#include <version.h>
+
+static void acpi_write_serialio_psx_methods(const char *const name, const uint32_t bar1)
+{
+ const char *const spcs = "SPCS";
+ const unsigned int spcs_bits = 32;
+ const unsigned long offset = bar1 + 0x84;
+ const uint8_t flags = FIELD_DWORDACC | FIELD_NOLOCK | FIELD_PRESERVE;
+ const struct opregion op_reg = OPREGION("SPRT", SYSTEMMEMORY, offset, spcs_bits / 8);
+ const struct fieldlist field = FIELDLIST_NAMESTR(spcs, spcs_bits);
+
+ acpigen_write_scope(name);
+ {
+ acpigen_write_opregion(&op_reg);
+ acpigen_write_field(op_reg.name, &field, 1, flags);
+
+ acpigen_write_method_serialized("_PS0", 0);
+ {
+ /* SPCS &= 0xfffffffc */
+ acpigen_emit_byte(AND_OP);
+ acpigen_emit_namestring(spcs);
+ acpigen_write_dword(0xfffffffc);
+ acpigen_emit_namestring(spcs);
+
+ /* Do a posting read after writing */
+ acpigen_write_store();
+ acpigen_emit_namestring(spcs);
+ acpigen_emit_byte(LOCAL0_OP);
+ }
+ acpigen_pop_len();
+
+ acpigen_write_method_serialized("_PS3", 0);
+ {
+ /* SPCS |= 3 */
+ acpigen_emit_byte(OR_OP);
+ acpigen_emit_namestring(spcs);
+ acpigen_write_byte(3);
+ acpigen_emit_namestring(spcs);
+
+ /* Do a posting read after writing */
+ acpigen_write_store();
+ acpigen_emit_namestring(spcs);
+ acpigen_emit_byte(LOCAL0_OP);
+ }
+ acpigen_pop_len();
+ }
+ acpigen_pop_len();
+}
+
+static void acpi_create_serialio_ssdt_entry(int sio_index, struct device_nvs *dev_nvs)
+{
+ const char idx = '0' + sio_index;
+ const char sxen[5] = { 'S', idx, 'E', 'N', '\0' };
+ acpigen_write_name_byte(sxen, dev_nvs->enable[sio_index]);
+
+ const char sxb0[5] = { 'S', idx, 'B', '0', '\0' };
+ acpigen_write_name_dword(sxb0, dev_nvs->bar0[sio_index]);
+
+ const char sxb1[5] = { 'S', idx, 'B', '1', '\0' };
+ acpigen_write_name_dword(sxb1, dev_nvs->bar1[sio_index]);
+}
+
+void acpi_create_serialio_ssdt(acpi_header_t *ssdt)
+{
+ unsigned long current = (unsigned long)ssdt + sizeof(acpi_header_t);
+ struct device_nvs *dev_nvs = acpi_get_device_nvs();
+ if (!dev_nvs)
+ return;
+
+ /* Fill the SSDT header */
+ memset((void *)ssdt, 0, sizeof(acpi_header_t));
+ memcpy(&ssdt->signature, "SSDT", 4);
+ ssdt->revision = get_acpi_table_revision(SSDT);
+ memcpy(&ssdt->oem_id, OEM_ID, 6);
+ memcpy(&ssdt->oem_table_id, "SERIALIO", 8);
+ ssdt->oem_revision = 43;
+ memcpy(&ssdt->asl_compiler_id, ASLC, 4);
+ ssdt->asl_compiler_revision = asl_revision;
+ ssdt->length = sizeof(acpi_header_t);
+ acpigen_set_current((char *)current);
+
+ /* Fill the SSDT with an entry for each SerialIO device */
+ for (int id = 0; id < 9; id++)
+ acpi_create_serialio_ssdt_entry(id, dev_nvs);
+
+ acpigen_write_scope("\\_SB.PCI0");
+ {
+ acpi_write_serialio_psx_methods("I2C0", dev_nvs->bar1[SIO_NVS_I2C0]);
+ acpi_write_serialio_psx_methods("I2C1", dev_nvs->bar1[SIO_NVS_I2C1]);
+ acpi_write_serialio_psx_methods("SPI0", dev_nvs->bar1[SIO_NVS_SPI0]);
+ acpi_write_serialio_psx_methods("SPI1", dev_nvs->bar1[SIO_NVS_SPI1]);
+ acpi_write_serialio_psx_methods("UAR0", dev_nvs->bar1[SIO_NVS_UART0]);
+ acpi_write_serialio_psx_methods("UAR1", dev_nvs->bar1[SIO_NVS_UART1]);
+ }
+ acpigen_pop_len();
+
+ /* (Re)calculate length and checksum. */
+ current = (unsigned long)acpigen_get_current();
+ ssdt->length = current - (unsigned long)ssdt;
+ ssdt->checksum = acpi_checksum((void *)ssdt, ssdt->length);
+}
diff --git a/src/soc/intel/broadwell/pch/acpi/adsp.asl b/src/soc/intel/broadwell/pch/acpi/adsp.asl
index 51dd38c..43c0354 100644
--- a/src/soc/intel/broadwell/pch/acpi/adsp.asl
+++ b/src/soc/intel/broadwell/pch/acpi/adsp.asl
@@ -1,5 +1,11 @@
/* SPDX-License-Identifier: GPL-2.0-only */

+// This is defined in SSDT2 which is generated at boot based
+// on whether or not the device is enabled in ACPI mode.
+External (\S8EN)
+External (\S8B0)
+External (\S8B1)
+
Device (ADSP)
{
Method (_HID, 0, Serialized)
diff --git a/src/soc/intel/broadwell/pch/acpi/serialio.asl b/src/soc/intel/broadwell/pch/acpi/serialio.asl
index 183bca6..85c7cf5 100644
--- a/src/soc/intel/broadwell/pch/acpi/serialio.asl
+++ b/src/soc/intel/broadwell/pch/acpi/serialio.asl
@@ -5,27 +5,34 @@
// Serial IO Device BAR0 and BAR1 is 4KB
#define SIO_BAR_LEN 0x1000

-// Put SerialIO device in D0 state
-// Arg0 - Ref to offset 0x84 of device's PCI config space
-Method (LPD0, 1, Serialized)
-{
- Arg0 = DeRefOf (Arg0) & 0xFFFFFFFC
- Local0 = DeRefOf (Arg0) // Read back after writing
+// This is defined in SSDT2 which is generated at boot based
+// on whether or not the device is enabled in ACPI mode.
+External (\S0EN)
+External (\S1EN)
+External (\S2EN)
+External (\S3EN)
+External (\S4EN)
+External (\S5EN)
+External (\S6EN)
+External (\S7EN)

- // Use Local0 to avoid iasl warning: Method Local is set but never used
- Local0 &= Ones
-}
+External (\S0B0)
+External (\S1B0)
+External (\S2B0)
+External (\S3B0)
+External (\S4B0)
+External (\S5B0)
+External (\S6B0)
+External (\S7B0)

-// Put SerialIO device in D3 state
-// Arg0 - Ref to offset 0x84 of device's PCI config space
-Method (LPD3, 1, Serialized)
-{
- Arg0 = DeRefOf (Arg0) | 0x3
- Local0 = DeRefOf (Arg0) // Read back after writing
-
- // Use Local0 to avoid iasl warning: Method Local is set but never used
- Local0 &= Ones
-}
+External (\S0B1)
+External (\S1B1)
+External (\S2B1)
+External (\S3B1)
+External (\S4B1)
+External (\S5B1)
+External (\S6B1)
+External (\S7B1)

// Serial IO Resource Consumption for BAR1
Device (SIOR)
@@ -196,7 +203,7 @@
}

// Check if Serial IO DMA Controller is enabled
- If (\_SB.PCI0.SDMA._STA != 0) {
+ If (\S0EN != 0) {
Return (ConcatenateResTemplate (RBUF, DBUF))
} Else {
Return (RBUF)
@@ -211,22 +218,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S1B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (I2C1)
@@ -272,7 +263,7 @@
}

// Check if Serial IO DMA Controller is enabled
- If (\_SB.PCI0.SDMA._STA != 0) {
+ If (\S0EN != 0) {
Return (ConcatenateResTemplate (RBUF, DBUF))
} Else {
Return (RBUF)
@@ -287,22 +278,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S2B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (SPI0)
@@ -348,22 +323,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S3B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (SPI1)
@@ -406,7 +365,7 @@
}

// Check if Serial IO DMA Controller is enabled
- If (\_SB.PCI0.SDMA._STA != 0) {
+ If (\S0EN != 0) {
Return (ConcatenateResTemplate (RBUF, DBUF))
} Else {
Return (RBUF)
@@ -421,22 +380,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S4B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (UAR0)
@@ -479,7 +422,7 @@
}

// Check if Serial IO DMA Controller is enabled
- If (\_SB.PCI0.SDMA._STA != 0) {
+ If (\S0EN != 0) {
Return (ConcatenateResTemplate (RBUF, DBUF))
} Else {
Return (RBUF)
@@ -494,22 +437,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S5B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (UAR1)
@@ -555,22 +482,6 @@
Return (0xF)
}
}
-
- OperationRegion (SPRT, SystemMemory, \S6B1 + 0x84, 4)
- Field (SPRT, DWordAcc, NoLock, Preserve)
- {
- SPCS, 32
- }
-
- Method (_PS0, 0, Serialized)
- {
- ^^LPD0 (RefOf (SPCS))
- }
-
- Method (_PS3, 0, Serialized)
- {
- ^^LPD3 (RefOf (SPCS))
- }
}

Device (SDIO)
diff --git a/src/soc/intel/broadwell/pch/lpc.c b/src/soc/intel/broadwell/pch/lpc.c
index 4b4aa9f..f62394b 100644
--- a/src/soc/intel/broadwell/pch/lpc.c
+++ b/src/soc/intel/broadwell/pch/lpc.c
@@ -600,6 +600,16 @@
pch_lpc_add_io_resources(dev);
}

+static unsigned long acpi_write_serialio_ssdt(unsigned long current, struct acpi_rsdp *rsdp)
+{
+ printk(BIOS_DEBUG, "ACPI: * SSDT2\n");
+ acpi_header_t *ssdt = (acpi_header_t *)current;
+ acpi_create_serialio_ssdt(ssdt);
+ current += ssdt->length;
+ acpi_add_table(rsdp, ssdt);
+ return acpi_align_current(current);
+}
+
static unsigned long broadwell_write_acpi_tables(const struct device *device,
unsigned long current,
struct acpi_rsdp *rsdp)
@@ -610,7 +620,10 @@
PCH_DEV_UART1 : PCH_DEV_UART0,
ACPI_ACCESS_SIZE_DWORD_ACCESS);
}
- return acpi_write_hpet(device, current, rsdp);
+ current = acpi_write_hpet(device, current, rsdp);
+ current = acpi_align_current(current);
+ current = acpi_write_serialio_ssdt(current, rsdp);
+ return current;
}

static struct device_operations device_ops = {

To view, visit change 52520. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I89fb658fbb10a8769ebea2e6535c45cd7c212d06
Gerrit-Change-Number: 52520
Gerrit-PatchSet: 11
Gerrit-Owner: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged