Change in coreboot[master]: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44016 )
Change subject: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges ......................................................................
soc/amd/picasso: Split ops for internal and external PCIe GPP bridges
This change splits the device operations for internal and external PCIe GPP bridges so that the external bridges use `pciexp_scan_bridge()` instead of `pci_scan_bridge()`. `pciexp_scan_bridge()` is required for external GPP bridges to enable ASPM on downstream devices if supported.
BUG=b:162352484 TEST=Verified on Trembyle: $ lspci -s 1:00.0 -vvv | grep ASPM LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <4us, L1 <64u ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ice2aa3e4758adccf7b0b89d4222fc65a40761153 --- M src/soc/amd/picasso/pcie_gpp.c 1 file changed, 20 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/44016/1
diff --git a/src/soc/amd/picasso/pcie_gpp.c b/src/soc/amd/picasso/pcie_gpp.c index 73de803..96d33f2 100644 --- a/src/soc/amd/picasso/pcie_gpp.c +++ b/src/soc/amd/picasso/pcie_gpp.c @@ -3,6 +3,7 @@ #include <acpi/acpigen.h> #include <device/device.h> #include <device/pci.h> +#include <device/pciexp.h> #include <device/pci_ids.h> #include <soc/pci_devs.h> #include <stdio.h> @@ -36,7 +37,7 @@ return NULL; }
-static struct device_operations pcie_gpp_ops = { +static struct device_operations internal_pcie_gpp_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = pci_bus_enable_resources, @@ -47,14 +48,29 @@ };
static const unsigned short pci_device_ids[] = { - PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP, PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP_BUSA, PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP_BUSB, 0 };
-static const struct pci_driver pcie_gpp_driver __pci_driver = { - .ops = &pcie_gpp_ops, +static const struct pci_driver internal_pcie_gpp_driver __pci_driver = { + .ops = &internal_pcie_gpp_ops, .vendor = PCI_VENDOR_ID_AMD, .devices = pci_device_ids, }; + +static struct device_operations external_pcie_gpp_ops = { + .read_resources = pci_bus_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_bus_enable_resources, + .scan_bus = pciexp_scan_bridge, + .reset_bus = pci_bus_reset, + .acpi_name = pcie_gpp_acpi_name, + .acpi_fill_ssdt = acpi_device_write_pci_dev, +}; + +static const struct pci_driver external_pcie_gpp_driver __pci_driver = { + .ops = &external_pcie_gpp_ops, + .vendor = PCI_VENDOR_ID_AMD, + .device = PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP, +};
Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44016 )
Change subject: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44016/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44016/1//COMMIT_MSG@15 PS1, Line 15: TEST=Verified on Trembyle: can you check that Wifi with ax200 is still working as intended on Morphius? As I recall, this was an issue before.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44016 )
Change subject: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44016 )
Change subject: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44016/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44016/1//COMMIT_MSG@15 PS1, Line 15: TEST=Verified on Trembyle:
can you check that Wifi with ax200 is still working as intended on Morphius? As I recall, this was a […]
Done. WiFi works with AX200 and ASPM is enabled too.
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44016 )
Change subject: soc/amd/picasso: Split ops for internal and external PCIe GPP bridges ......................................................................
soc/amd/picasso: Split ops for internal and external PCIe GPP bridges
This change splits the device operations for internal and external PCIe GPP bridges so that the external bridges use `pciexp_scan_bridge()` instead of `pci_scan_bridge()`. `pciexp_scan_bridge()` is required for external GPP bridges to enable ASPM on downstream devices if supported.
BUG=b:162352484 TEST=Verified on Trembyle: $ lspci -s 1:00.0 -vvv | grep ASPM LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <4us, L1 <64u ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: Ice2aa3e4758adccf7b0b89d4222fc65a40761153 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44016 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Eric Peers epeers@google.com Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/soc/amd/picasso/pcie_gpp.c 1 file changed, 20 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Eric Peers: Looks good to me, but someone else must approve
diff --git a/src/soc/amd/picasso/pcie_gpp.c b/src/soc/amd/picasso/pcie_gpp.c index 73de803..96d33f2 100644 --- a/src/soc/amd/picasso/pcie_gpp.c +++ b/src/soc/amd/picasso/pcie_gpp.c @@ -3,6 +3,7 @@ #include <acpi/acpigen.h> #include <device/device.h> #include <device/pci.h> +#include <device/pciexp.h> #include <device/pci_ids.h> #include <soc/pci_devs.h> #include <stdio.h> @@ -36,7 +37,7 @@ return NULL; }
-static struct device_operations pcie_gpp_ops = { +static struct device_operations internal_pcie_gpp_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = pci_bus_enable_resources, @@ -47,14 +48,29 @@ };
static const unsigned short pci_device_ids[] = { - PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP, PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP_BUSA, PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP_BUSB, 0 };
-static const struct pci_driver pcie_gpp_driver __pci_driver = { - .ops = &pcie_gpp_ops, +static const struct pci_driver internal_pcie_gpp_driver __pci_driver = { + .ops = &internal_pcie_gpp_ops, .vendor = PCI_VENDOR_ID_AMD, .devices = pci_device_ids, }; + +static struct device_operations external_pcie_gpp_ops = { + .read_resources = pci_bus_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_bus_enable_resources, + .scan_bus = pciexp_scan_bridge, + .reset_bus = pci_bus_reset, + .acpi_name = pcie_gpp_acpi_name, + .acpi_fill_ssdt = acpi_device_write_pci_dev, +}; + +static const struct pci_driver external_pcie_gpp_driver __pci_driver = { + .ops = &external_pcie_gpp_ops, + .vendor = PCI_VENDOR_ID_AMD, + .device = PCI_DEVICE_ID_AMD_FAM17H_PCIE_GPP, +};
-
Aaron Durbin (Code Review) -
Eric Peers (Code Review)
-
Furquan Shaikh (Code Review)