Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/halvor/overridetree.cb M src/mainboard/google/volteer/variants/lindar/overridetree.cb M src/mainboard/google/volteer/variants/malefor/overridetree.cb M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 8 files changed, 59 insertions(+), 125 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 636d946..2717acd 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -377,13 +377,17 @@ device generic 0 on end end end - device ref tbt_pcie_rp0 on - probe DB_USB USB4_GEN2 - probe DB_USB USB4_GEN3 + chip drivers/intel/usb4/pcie + device ref tbt_pcie_rp0 on + probe DB_USB USB4_GEN2 + probe DB_USB USB4_GEN3 + end end - device ref tbt_pcie_rp1 on - probe DB_USB USB4_GEN2 - probe DB_USB USB4_GEN3 + chip drivers/intel/usb4/pcie + device ref tbt_pcie_rp1 on + probe DB_USB USB4_GEN2 + probe DB_USB USB4_GEN3 + end end device ref tbt_dma0 on probe DB_USB USB4_GEN2 diff --git a/src/mainboard/google/volteer/variants/halvor/overridetree.cb b/src/mainboard/google/volteer/variants/halvor/overridetree.cb index 2db3b96..68269b5 100644 --- a/src/mainboard/google/volteer/variants/halvor/overridetree.cb +++ b/src/mainboard/google/volteer/variants/halvor/overridetree.cb @@ -18,7 +18,9 @@ register "SaGv" = "SaGv_Disabled"
device domain 0 on - device pci 07.2 on end # TBT_PCIe2 + chip drivers/intel/usb4/pcie + device pci 07.2 on end # TBT_PCIe2 + end device pci 15.0 on chip drivers/i2c/generic register "hid" = ""10EC5682"" diff --git a/src/mainboard/google/volteer/variants/lindar/overridetree.cb b/src/mainboard/google/volteer/variants/lindar/overridetree.cb index 6f14622..9d9a376 100644 --- a/src/mainboard/google/volteer/variants/lindar/overridetree.cb +++ b/src/mainboard/google/volteer/variants/lindar/overridetree.cb @@ -27,10 +27,18 @@ [PchSerialIoIndexI2C5] = PchSerialIoPci, }" device domain 0 on - device pci 07.0 off end # TBT_PCIe0 0x9A23 - device pci 07.1 off end # TBT_PCIe1 0x9A25 - device pci 07.2 off end # TBT_PCIe2 0x9A27 - device pci 07.3 off end # TBT_PCIe3 0x9A29 + chip drivers/intel/usb4/pcie + device pci 07.0 off end # TBT_PCIe0 0x9A23 + end + chip drivers/intel/usb4/pcie + device pci 07.1 off end # TBT_PCIe1 0x9A25 + end + chip drivers/intel/usb4/pcie + device pci 07.2 off end # TBT_PCIe2 0x9A27 + end + chip drivers/intel/usb4/pcie + device pci 07.3 off end # TBT_PCIe3 0x9A29 + end device pci 15.0 on chip drivers/i2c/generic register "hid" = ""10EC5682"" diff --git a/src/mainboard/google/volteer/variants/malefor/overridetree.cb b/src/mainboard/google/volteer/variants/malefor/overridetree.cb index b184924..a4f3afe 100644 --- a/src/mainboard/google/volteer/variants/malefor/overridetree.cb +++ b/src/mainboard/google/volteer/variants/malefor/overridetree.cb @@ -34,10 +34,18 @@ }"
device domain 0 on - device pci 07.0 off end # TBT_PCIe0 0x9A23 - device pci 07.1 off end # TBT_PCIe1 0x9A25 - device pci 07.2 off end # TBT_PCIe2 0x9A27 - device pci 07.3 off end # TBT_PCIe3 0x9A29 + chip drivers/intel/usb4/pcie + device pci 07.0 off end # TBT_PCIe0 0x9A23 + end + chip drivers/intel/usb4/pcie + device pci 07.1 off end # TBT_PCIe1 0x9A25 + end + chip drivers/intel/usb4/pcie + device pci 07.2 off end # TBT_PCIe2 0x9A27 + end + chip drivers/intel/usb4/pcie + device pci 07.3 off end # TBT_PCIe3 0x9A29 + end device pci 15.0 on chip drivers/i2c/generic register "hid" = ""10EC5682"" diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 3b0df5d..250840f 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -17,6 +17,7 @@ select CACHE_MRC_SETTINGS select CPU_INTEL_COMMON select CPU_INTEL_FIRMWARE_INTERFACE_TABLE + select DRIVERS_INTEL_USB4_PCIE select FSP_COMPRESS_FSP_S_LZ4 select FSP_M_XIP select GENERIC_GPIO_LIB @@ -51,6 +52,7 @@ select SOC_INTEL_COMMON_BLOCK_SA select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_BLOCK_USB4 select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl index da2e8fe..d00f794 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl @@ -82,30 +82,3 @@ { Return (Package() { 0x6D, 4 }) } - -Method (_DSD, 0) -{ - Return( - Package() - { - /* Thunderbolt GUID for IMR_VALID at ../drivers/acpi/property.c */ - ToUUID("C44D002F-69F9-4E7D-A904-A7BAABDF43F7"), - Package () - { - Package (2) { "IMR_VALID", 1 } - }, - - /* Thunderbolt GUID for WAKE_SUPPORTED at ../drivers/acpi/property.c */ - ToUUID("6C501103-C189-4296-BA72-9BF5A26EBE5D"), - Package () - { - Package (2) { "WAKE_SUPPORTED", 1 } - } - } - ) -} - -Method (_DSM, 4, Serialized) -{ - Return (Buffer() { 0 }) -} diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl index abca5d1..3f495be 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl @@ -216,85 +216,6 @@ } }
-Method (_DSD, 0) { - If ((TUID == 0) || (TUID == 1)) { - Return ( Package() { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, - Package (2) { "usb4-port-number", TUID }, - } - }) - } ElseIf (TUID == 2) { - Return ( Package () { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, - Package (2) { "usb4-port-number", 0 }, - } - }) - } Else { /* TUID == 3 */ - Return ( Package () { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, - Package (2) { "usb4-port-number", 1 }, - } - }) - } -} - Method (_S0W, 0x0, NotSerialized) { Return (0x4) diff --git a/src/soc/intel/tigerlake/chipset.cb b/src/soc/intel/tigerlake/chipset.cb index 0d2b7a2..43c99a5 100644 --- a/src/soc/intel/tigerlake/chipset.cb +++ b/src/soc/intel/tigerlake/chipset.cb @@ -5,10 +5,26 @@ device pci 04.0 alias dptf off end device pci 05.0 alias ipu off end device pci 06.0 alias peg off end - device pci 07.0 alias tbt_pcie_rp0 off end - device pci 07.1 alias tbt_pcie_rp1 off end - device pci 07.2 alias tbt_pcie_rp2 off end - device pci 07.3 alias tbt_pcie_rp3 off end + chip drivers/intel/usb4/pcie + register "port_id" = "0" + use tbt_dma0 as usb4_port + device pci 07.0 alias tbt_pcie_rp0 off end + end + chip drivers/intel/usb4/pcie + register "port_id" = "1" + use tbt_dma0 as usb4_port + device pci 07.1 alias tbt_pcie_rp1 off end + end + chip drivers/intel/usb4/pcie + register "port_id" = "0" + use tbt_dma1 as usb4_port + device pci 07.2 alias tbt_pcie_rp2 off end + end + chip drivers/intel/usb4/pcie + register "port_id" = "1" + use tbt_dma1 as usb4_port + device pci 07.3 alias tbt_pcie_rp3 off end + end device pci 08.0 alias gna off end device pci 09.0 alias npk off end device pci 0a.0 alias crashlog off end
Tim Wawrzynczak has uploaded a new patch set (#2) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/halvor/overridetree.cb M src/mainboard/google/volteer/variants/lindar/overridetree.cb M src/mainboard/google/volteer/variants/malefor/overridetree.cb M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 8 files changed, 59 insertions(+), 125 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/2
Tim Wawrzynczak has uploaded a new patch set (#3) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/mainboard/google/deltaur/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/halvor/overridetree.cb M src/mainboard/google/volteer/variants/lindar/overridetree.cb M src/mainboard/google/volteer/variants/malefor/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 11 files changed, 95 insertions(+), 137 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/3
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end I think we should avoid the mistake we did with CNVi device and instead use a virtual generic device:
``` device pci 07.0 alias tbt_pcie_rp0 off chip drivers/intel/usb4/pcie use tbt_dma0 as usb4_port device generic 0 on end end end ```
We can probably encode the port_id in generic ID and drop another register field as well.
Adding the virtual generic device allows us to drop the requirement that the mainboard devicetree has to decorate the TBT devices with "chip drivers/intel/usb4/pcie". All that would be required in mainboard is setting the tbt_pcie_rpX device as on/off depending upon the hardware design.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
I think we should avoid the mistake we did with CNVi device and instead use a virtual generic device […]
The driver will need to have pci ops and *_resource handlers for an attached pci device to get properly set up. as long as the driver is set up for pci it should work even if the child is declared as generic in devicetree, but we should probably be clear what is attached to the root port and make this 'device pci 00.0 on end'
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
The driver will need to have pci ops and *_resource handlers for an attached pci device to get prope […]
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
The driver will need to have pci ops and *_resource handlers for an attached pci device to get properly set up.
Are you referring to the driver that would handle the TBT RP device? I think if we don't have any chip driver associated with it in the device tree, then it would be handled by the generic PCI device driver? And hence the device ops provided by it?
as long as the driver is set up for pci it should work even if the child is declared as generic in devicetree, but we should probably be clear what is attached to the root port and make this 'device pci 00.0 on end'
The child device would basically be a dummy/virtual one which would only be used for generating _DSD under the parent RP. Do we really want that to be a PCI device? All the actual PCI devices sitting behind the RP should be enumerated and initialized by the PCI bus scan under the RP, right?
Tim Wawrzynczak has uploaded a new patch set (#4) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 25 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/4
Tim Wawrzynczak has uploaded a new patch set (#8) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 3 files changed, 24 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/8
Tim Wawrzynczak has uploaded a new patch set (#10) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 26 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/10
Tim Wawrzynczak has uploaded a new patch set (#11) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 26 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/11
Tim Wawrzynczak has uploaded a new patch set (#12) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/common/block/usb4/usb4.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 5 files changed, 30 insertions(+), 111 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/12
Tim Wawrzynczak has uploaded a new patch set (#13) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 26 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/13
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
The driver will need to have pci ops and *_resource handlers for an attached pci device to get properly set up.
Are you referring to the driver that would handle the TBT RP device? I think if we don't have any chip driver associated with it in the device tree, then it would be handled by the generic PCI device driver? And hence the device ops provided by it?
This is for the device attached to the RP. I ran into this trying to set up the NVMe device this way for RTD3, it didn't do the basic PCI setup on the NVMe unless the chip under the RP had PCI ops.
as long as the driver is set up for pci it should work even if the child is declared as generic in devicetree, but we should probably be clear what is attached to the root port and make this 'device pci 00.0 on end'
The child device would basically be a dummy/virtual one which would only be used for generating _DSD under the parent RP. Do we really want that to be a PCI device? All the actual PCI devices sitting behind the RP should be enumerated and initialized by the PCI bus scan under the RP, right?
Generic can work too as long as the driver itself provides PCI ops, but I just think we should be explicit since if something is discovered it will present as a PCI device.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... PS13, Line 17: 1 I think this can also be function 0 since these are all separate root ports. (but I don't actually have a USB4 PCIe device to test..)
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... PS13, Line 17: 1
I think this can also be function 0 since these are all separate root ports. […]
I don't either 😞 but that's what the ASL had, Think it should be changed?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/13/src/soc/intel/tigerlake/ch... PS13, Line 17: 1
I don't either 😞 but that's what the ASL had, Think it should be changed?
Ah in that case probably best to leave it as is.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
The driver will need to have pci ops and *_resource handlers for an attached pci device to get properly set up.
Are you referring to the driver that would handle the TBT RP device? I think if we don't have any chip driver associated with it in the device tree, then it would be handled by the generic PCI device driver? And hence the device ops provided by it?
This is for the device attached to the RP. I ran into this trying to set up the NVMe device this way for RTD3, it didn't do the basic PCI setup on the NVMe unless the chip under the RP had PCI ops.
Ah okay. That makes sense.
as long as the driver is set up for pci it should work even if the child is declared as generic in devicetree, but we should probably be clear what is attached to the root port and make this 'device pci 00.0 on end'
The child device would basically be a dummy/virtual one which would only be used for generating _DSD under the parent RP. Do we really want that to be a PCI device? All the actual PCI devices sitting behind the RP should be enumerated and initialized by the PCI bus scan under the RP, right?
Generic can work too as long as the driver itself provides PCI ops, but I just think we should be explicit since if something is discovered it will present as a PCI device.
But, is the device we are adding here really a PCI device? i.e. the child device is actually a dummy/virtual device. If PCI enumeration ends up finding an actual PCI device under the RP, would it be correct to associate it with drivers/intel/usb4/pcie? We are currently setting scan_bus as scan_generic_bus in drivers/intel/usb4/pcie which might not be correct for a PCI device especially if it turns out to be a bridge. Or am I thinking about this wrong? We can go ahead with the current change if you think that PCI device is the correct representation and revisit this later.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 14: Code-Review+2
Tim Wawrzynczak has uploaded a new patch set (#15) to the change originally created by Duncan Laurie. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 26 insertions(+), 110 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/44917/15
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 15: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
The driver will need to have pci ops and *_resource handlers for an attached pci device to get […]
Done
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
soc/intel/tigerlake: Enable and use USB4 PCIe driver
This change enables the USB4/Thunderbolt common layer for Intel SOC, and enables the Intel USB4 PCIe driver. This moves the _DSD variables from the DSDT into the SSDT and allows them to be configured for each board if necessary.
Change-Id: I2564512d951046e015c148db42fdaf2d4b8b81dd Signed-off-by: Duncan Laurie dlaurie@google.com Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/44917 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/chipset.cb 4 files changed, 26 insertions(+), 110 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 4df2e85..9433080 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -48,6 +48,8 @@ select SOC_INTEL_COMMON_BLOCK_SA select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_BLOCK_USB4 + select SOC_INTEL_COMMON_BLOCK_USB4_PCIE select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl index f7c4117..085990d 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl @@ -92,30 +92,3 @@ { Return (Package() { 0x6D, 4 }) } - -Method (_DSD, 0) -{ - Return( - Package() - { - /* Thunderbolt GUID for IMR_VALID at ../drivers/acpi/property.c */ - ToUUID("C44D002F-69F9-4E7D-A904-A7BAABDF43F7"), - Package () - { - Package (2) { "IMR_VALID", 1 } - }, - - /* Thunderbolt GUID for WAKE_SUPPORTED at ../drivers/acpi/property.c */ - ToUUID("6C501103-C189-4296-BA72-9BF5A26EBE5D"), - Package () - { - Package (2) { "WAKE_SUPPORTED", 1 } - } - } - ) -} - -Method (_DSM, 4, Serialized) -{ - Return (Buffer() { 0 }) -} diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl index b4c0cef..08d8900 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl @@ -220,85 +220,6 @@ } }
-Method (_DSD, 0) { - If ((TUID == 0) || (TUID == 1)) { - Return ( Package() { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, - Package (2) { "usb4-port-number", TUID }, - } - }) - } ElseIf (TUID == 2) { - Return ( Package () { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, - Package (2) { "usb4-port-number", 0 }, - } - }) - } Else { /* TUID == 3 */ - Return ( Package () { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, - - /* pci_acpi_set_untrusted at ../drivers/pci/pci-acpi.c */ - ToUUID("EFCC06CC-73AC-4BC3-BFF0-76143807C389"), - Package () { - Package (2) { "ExternalFacingPort", 1 }, /* TBT/CIO port */ - /* - * UID of the TBT RP on platform, range is: 0, 1 ..., - * (NumOfTBTRP - 1). - */ - Package (2) { "UID", TUID }, - }, - ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), - Package () { - Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, - Package (2) { "usb4-port-number", 1 }, - } - }) - } -} - Method (_S0W, 0x0, NotSerialized) { Return (0x4) diff --git a/src/soc/intel/tigerlake/chipset.cb b/src/soc/intel/tigerlake/chipset.cb index 4f55125..b60801c 100644 --- a/src/soc/intel/tigerlake/chipset.cb +++ b/src/soc/intel/tigerlake/chipset.cb @@ -5,10 +5,30 @@ device pci 04.0 alias dptf off end device pci 05.0 alias ipu off end device pci 06.0 alias peg off end - device pci 07.0 alias tbt_pcie_rp0 off end - device pci 07.1 alias tbt_pcie_rp1 off end - device pci 07.2 alias tbt_pcie_rp2 off end - device pci 07.3 alias tbt_pcie_rp3 off end + device pci 07.0 alias tbt_pcie_rp0 off + chip soc/intel/common/block/usb4 + use tbt_dma0 as usb4_port + device generic 0 on end + end + end + device pci 07.1 alias tbt_pcie_rp1 off + chip soc/intel/common/block/usb4 + use tbt_dma0 as usb4_port + device generic 1 on end + end + end + device pci 07.2 alias tbt_pcie_rp2 off + chip soc/intel/common/block/usb4 + use tbt_dma1 as usb4_port + device generic 0 on end + end + end + device pci 07.3 alias tbt_pcie_rp3 off + chip soc/intel/common/block/usb4 + use tbt_dma1 as usb4_port + device generic 1 on end + end + end device pci 08.0 alias gna off end device pci 09.0 alias npk off end device pci 0a.0 alias crashlog off end
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
Done
Sorry I was sidetracked with some non-work stuff yesterday and still trying to get a useful work area set up at my new/temp home so I didn't get a chance to look deeper at this.
It looks like with this as generic we get some unhappy output at boot:
PCI: 00:07.0 scanning... do_pci_scan_bridge for PCI: 00:07.0 PCI: pci_scan_bus for bus 01 child GENERIC: 0.0 not a PCI device (repeated a couple dozen times) PCI: Leftover static devices: GENERIC: 0.0 PCI: Check your devicetree.cb.
And we don't get the devices generated in the SSDT. I will dig into it more as I have a TBT dock coming to test with.
In the end for volteer it doesn't matter too much if the pci bridges behind a TBT device actually get scanned and set up since we don't want external PCI enumerated in the firmware anyway, we just need the ACPI tables and leave the rest to the OS for now.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end Aah I see now what you were saying.
1. child GENERIC: 0.0 not a PCI device (repeated a couple dozen times)
This is because `pci_scan_get_dev()` expects the children of the bridge to be only PCI devices. If it doesn't find that, it prints this out as it is looking for existing device structure among the children of the bridge.
2. PCI: Leftover static devices: GENERIC: 0.0
This is because `do_pci_scan_bridge()` only calls `do_scan_bus()` for actual PCI devices using min and max devfn. It never really scans the generic device and so ends up removing it as unused static device.
In the end for volteer it doesn't matter too much if the pci bridges behind a TBT device actually get scanned and set up since we don't want external PCI enumerated in the firmware anyway, we just need the ACPI tables and leave the rest to the OS for now.
That's true. I think the question would be how do we want to treat dummy/virtual devices added under PCIe bridges like USB4 or NVMe. Do we want those devices to be simply PCI devices and use that as proxy to add SSDT information under the parent? Or do we want to enable coreboot infrastructure to allow scanning for non-PCI virtual devices under the bridge?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44917 )
Change subject: soc/intel/tigerlake: Enable and use USB4 PCIe driver ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/44917/3/src/soc/intel/tigerlake/chi... PS3, Line 8: chip drivers/intel/usb4/pcie : register "port_id" = "0" : use tbt_dma0 as usb4_port : device pci 07.0 alias tbt_pcie_rp0 off end : end
Aah I see now what you were saying. […]
So, I think this should help with #2:
diff --git a/src/device/pci_device.c b/src/device/pci_device.c index ce3e50967a..78ebdba73f 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -1283,6 +1283,10 @@ void pci_scan_bus(struct bus *bus, unsigned int min_devfn,
prev = &bus->children; for (dev = bus->children; dev; dev = dev->sibling) { + + if (dev->path.type != DEVICE_PATH_PCI) + continue; + /* * The device is only considered leftover if it is not hidden * and it has a Vendor ID of 0 (the default for a device that
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Nick Vaccaro (Code Review)
-
Tim Wawrzynczak (Code Review)