Furquan Shaikh submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
soc/amd/hda: Move HDA PCI device from DSDT to SSDT

This change adds support in common block HDA driver to add a PCI
device for HDA in SSDT and removes the HDA device from DSDT for
Stoneyridge and Picasso.

_INI method is still retained in stoneyridge since I am unsure why it
was added. In order to support the _INI method, HDA driver makes a
callback hda_soc_ssdt_quirks() to allow SoCs to add any quirks
required for the HDA device. This callback is implemented by
Stoneyridge to provide the _INI method which retains the same
functionality for HDA device.

This makes it easier to ensure that we don't accidentally
make the DSDT and SSDT entries inconsistent w.r.t. ACPI name and
scope.

BUG=b:153858769,b:155132752
TEST=Verified that audio still works fine on Trembyle.

Change-Id: I89dc46b92fdcb785bd37e18f0456935c0e57eff5
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40785
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/mainboard/amd/gardenia/acpi/gpe.asl
M src/mainboard/amd/padmelon/acpi/gpe.asl
M src/soc/amd/common/block/hda/hda.c
A src/soc/amd/common/block/include/amdblocks/hda.h
M src/soc/amd/picasso/acpi/northbridge.asl
M src/soc/amd/stoneyridge/acpi/northbridge.asl
M src/soc/amd/stoneyridge/northbridge.c
7 files changed, 83 insertions(+), 41 deletions(-)

diff --git a/src/mainboard/amd/gardenia/acpi/gpe.asl b/src/mainboard/amd/gardenia/acpi/gpe.asl
index a4aed9a..7756729 100644
--- a/src/mainboard/amd/gardenia/acpi/gpe.asl
+++ b/src/mainboard/amd/gardenia/acpi/gpe.asl
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* This file is part of the coreboot project. */

+External (\_SB.PCI0.AZHD, DeviceObj)
+
Scope(\_GPE) { /* Start Scope GPE */

/* General event 3 */
diff --git a/src/mainboard/amd/padmelon/acpi/gpe.asl b/src/mainboard/amd/padmelon/acpi/gpe.asl
index 3fd653e..545ba14 100644
--- a/src/mainboard/amd/padmelon/acpi/gpe.asl
+++ b/src/mainboard/amd/padmelon/acpi/gpe.asl
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* This file is part of the coreboot project. */

+External (\_SB.PCI0.AZHD, DeviceObj)
+
Scope(\_GPE) { /* Start Scope GPE */

/* General event 3 */
diff --git a/src/soc/amd/common/block/hda/hda.c b/src/soc/amd/common/block/hda/hda.c
index ea42b84..2028f09 100644
--- a/src/soc/amd/common/block/hda/hda.c
+++ b/src/soc/amd/common/block/hda/hda.c
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* This file is part of the coreboot project. */

+#include <arch/acpigen.h>
+#include <amdblocks/hda.h>
#include <device/device.h>
#include <device/pci.h>
#include <device/pci_ids.h>
@@ -19,12 +21,23 @@
return "AZHD";
}

+__weak void hda_soc_ssdt_quirks(const struct device *dev)
+{
+}
+
+static void hda_fill_ssdt(const struct device *dev)
+{
+ acpi_device_write_pci_dev(dev);
+ hda_soc_ssdt_quirks(dev);
+}
+
static struct device_operations hda_audio_ops = {
.read_resources = pci_dev_read_resources,
.set_resources = pci_dev_set_resources,
.enable_resources = pci_dev_enable_resources,
.ops_pci = &pci_dev_ops_pci,
.acpi_name = hda_acpi_name,
+ .acpi_fill_ssdt = hda_fill_ssdt,
};

static const struct pci_driver hdaaudio_driver __pci_driver = {
diff --git a/src/soc/amd/common/block/include/amdblocks/hda.h b/src/soc/amd/common/block/include/amdblocks/hda.h
new file mode 100644
index 0000000..b59a7b0
--- /dev/null
+++ b/src/soc/amd/common/block/include/amdblocks/hda.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef __AMDBLOCKS_HDA_H__
+#define __AMDBLOCKS_HDA_H__
+
+#include <device/device.h>
+
+/* SoC callback to add any quirks to HDA device node in SSDT. */
+void hda_soc_ssdt_quirks(const struct device *dev);
+
+#endif /* __AMDBLOCKS_HDA_H__ */
diff --git a/src/soc/amd/picasso/acpi/northbridge.asl b/src/soc/amd/picasso/acpi/northbridge.asl
index 386556c..67ae6f2 100644
--- a/src/soc/amd/picasso/acpi/northbridge.asl
+++ b/src/soc/amd/picasso/acpi/northbridge.asl
@@ -76,7 +76,3 @@
Return (PS8) /* PIC Mode */
} /* end _PRT */
} /* end PBR8 */
-
-Device(AZHD) { /* 0:9.2 - HD Audio */
- Name(_ADR, 0x00090002)
-} /* end AZHD */
diff --git a/src/soc/amd/stoneyridge/acpi/northbridge.asl b/src/soc/amd/stoneyridge/acpi/northbridge.asl
index c807601..91e43aa 100644
--- a/src/soc/amd/stoneyridge/acpi/northbridge.asl
+++ b/src/soc/amd/stoneyridge/acpi/northbridge.asl
@@ -81,40 +81,3 @@
Return (PS8) /* PIC Mode */
} /* end _PRT */
} /* end PBR8 */
-
-Device(AZHD) { /* 0:9.2 - HD Audio */
- Name(_ADR, 0x00090002)
- OperationRegion(AZPD, PCI_Config, 0x00, 0x100)
- Field(AZPD, AnyAcc, NoLock, Preserve) {
- offset (0x42),
- NSDI, 1,
- NSDO, 1,
- NSEN, 1,
- offset (0x44),
- IPCR, 4,
- offset (0x54),
- PWST, 2,
- , 6,
- PMEB, 1,
- , 6,
- PMST, 1,
- offset (0x62),
- MMCR, 1,
- offset (0x64),
- MMLA, 32,
- offset (0x68),
- MMHA, 32,
- offset (0x6c),
- MMDT, 16,
- }
-
- Method (_INI, 0, NotSerialized)
- {
- If (LEqual (OSVR, 0x03))
- {
- Store (Zero, NSEN)
- Store (One, NSDO)
- Store (One, NSDI)
- }
- }
-} /* end AZHD */
diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c
index db71509..2aa16b6 100644
--- a/src/soc/amd/stoneyridge/northbridge.c
+++ b/src/soc/amd/stoneyridge/northbridge.c
@@ -1,7 +1,9 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/* This file is part of the coreboot project. */

+#include <assert.h>
#include <amdblocks/biosram.h>
+#include <amdblocks/hda.h>
#include <device/pci_ops.h>
#include <arch/ioapic.h>
#include <arch/acpi.h>
@@ -501,3 +503,55 @@
params->iGpuVgaMode = 0;
params->GnbIoapicAddress = IO_APIC2_ADDR;
}
+
+void hda_soc_ssdt_quirks(const struct device *dev)
+{
+ const char *scope = acpi_device_path(dev);
+ static const struct fieldlist list[] = {
+ FIELDLIST_OFFSET(0x42),
+ FIELDLIST_NAMESTR("NSDI", 1),
+ FIELDLIST_NAMESTR("NSDO", 1),
+ FIELDLIST_NAMESTR("NSEN", 1),
+ };
+ struct opregion opreg = OPREGION("AZPD", PCI_CONFIG, 0x0, 0x100);
+
+ assert(scope);
+
+ acpigen_write_scope(scope);
+
+ /*
+ * OperationRegion(AZPD, PCI_Config, 0x00, 0x100)
+ * Field (AZPD, AnyAcc, NoLock, Preserve) {
+ * Offset (0x42),
+ * NSDI, 1,
+ * NSDO, 1,
+ * NSEN, 1,
+ * }
+ */
+ acpigen_write_opregion(&opreg);
+ acpigen_write_field(opreg.name, list, ARRAY_SIZE(list),
+ FIELD_ANYACC | FIELD_NOLOCK | FIELD_PRESERVE);
+
+ /*
+ * Method (_INI, 0, NotSerialized) {
+ * If (LEqual (OSVR, 0x03)) {
+ * Store (Zero, NSEN)
+ * Store (One, NSDO)
+ * Store (One, NSDI)
+ * }
+ * }
+ */
+ acpigen_write_method("_INI", 0);
+
+ acpigen_write_if_lequal_namestr_int("OSVR", 0x03);
+
+ acpigen_write_store_op_to_namestr(ONE_OP, "NSEN");
+ acpigen_write_store_op_to_namestr(ZERO_OP, "NSDO");
+ acpigen_write_store_op_to_namestr(ZERO_OP, "NSDI");
+
+ acpigen_pop_len(); /* If */
+
+ acpigen_pop_len(); /* Method _INI */
+
+ acpigen_pop_len(); /* Scope */
+}

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I89dc46b92fdcb785bd37e18f0456935c0e57eff5
Gerrit-Change-Number: 40785
Gerrit-PatchSet: 8
Gerrit-Owner: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged