John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly (CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across xhci and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 182 insertions(+), 180 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/1
diff --git a/src/soc/intel/tigerlake/acpi/tcss.asl b/src/soc/intel/tigerlake/acpi/tcss.asl index 48f2689..93bd32c 100644 --- a/src/soc/intel/tigerlake/acpi/tcss.asl +++ b/src/soc/intel/tigerlake/acpi/tcss.asl @@ -126,6 +126,46 @@ } Return (0) } + + Method (_OSC, 4, Serialized) + { + /* + * Operating System Capabilities for USB4 + * Arg0: UUID = {23A0D13A-26AB-486C-9C5F-0FFA525A575A} + * Arg1: Revision ID = 1 + * Arg2: Count of entries (DWORD) in Arg3 (Integer): 3 + * Arg3: DWORD capabilities buffer: + * First DWORD: The standard definiton bits is used to return errors. + * Second DWORD: OSPM support field for USB4, bits [31:0] reserved. + * Third DWORD: OSPM control field for USB4. + * bit 0: USB tunneling + * bit 1: DisplayPort tunneling + * bit 2: PCIe tunneling + * bit 3: Inter-domain USB4 internet protocol + * bit 31:4: reserved + * Return: The platform acknolwedges the Capabilities buffer by returning + * a buffer of DWORD of the same length. Preserved bits in the control + * field from the platform to OSPM, while masked/cleared bits in the + * control field indicate that the platform does not permit OSPM control + * of the respective capabilities or features. + */ + Name (CTRL, 0) /* Control field value */ + If (Arg0 == ToUUID("23A0D13A-26AB-486C-9C5F-0FFA525A575A")) { + CreateDWordField(Arg3, 0, CDW1) + CreateDWordField(Arg3, 8, CDW3) + CTRL = CDW3 + + If (Arg1 != 1) { + CDW1 |= 0x8 /* Unknown revision */ + } + CTRL |= 0xb + CDW3 = CTRL + Return (Arg3) + } Else { + CDW1 |= 0x4 /* Unrecognized UUID */ + Return (Arg3) + } + } }
Scope (_GPE) @@ -424,17 +464,6 @@ }
/* - * Below is a variable to store devices connect state for TBT PCIe RP before - * entering D3 cold. - * Value 0 - no device connected before enter D3 cold, no need to send - * CONNECT_TOPOLOGY in D3 cold exit. - * Value 1 - has device connected before enter D3 cold, need to send - * CONNECT_TOPOLOGY in D3 cold exit. - */ - Name (CTP0, 0) /* Variable of device connecet status for TBT0 group. */ - Name (CTP1, 0) /* Variable of device connecet status for TBT1 group. */ - - /* * TBT Group0 ON method */ Method (TG0N, 0) @@ -455,28 +484,6 @@ /* RP1 D3 cold exit. */ _SB.PCI0.TRP1.D3CX() } - - /* - * Need to send Connect-Topology command when TBT host - * controller back to D0 from D3. - */ - If (_SB.PCI0.TDM0.ALCT == 1) { - If (CTP0 == 1) { - /* - * Send Connect-Topology command if there is - * device present on PCIe RP. - */ - _SB.PCI0.TDM0.CNTP() - - /* Indicate to wait Connect-Topology command. */ - _SB.PCI0.TDM0.WACT = 1 - - /* Clear the connect states. */ - CTP0 = 0 - } - /* Disallow to send Connect-Topology command. */ - _SB.PCI0.TDM0.ALCT = 0 - } } Else { Printf("Drop TG0N due to it is already exit D3 cold.") } @@ -500,16 +507,10 @@
Printf("Push TBT RPs to D3Cold together") If (_SB.PCI0.TRP0.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP0.PDSX == 1) { - CTP0 = 1 - } /* Put RP0 to D3 cold. */ _SB.PCI0.TRP0.D3CE() } If (_SB.PCI0.TRP1.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP1.PDSX == 1) { - CTP0 = 1 - } /* Put RP1 to D3 cold. */ _SB.PCI0.TRP1.D3CE() } @@ -538,28 +539,6 @@ /* RP3 D3 cold exit. */ _SB.PCI0.TRP3.D3CX() } - - /* - * Need to send Connect-Topology command when TBT host - * controller back to D0 from D3. - */ - If (_SB.PCI0.TDM1.ALCT == 1) { - If (CTP1 == 1) { - /* - * Send Connect-Topology command if there is - * device present on PCIe RP. - */ - _SB.PCI0.TDM1.CNTP() - - /* Indicate to wait Connect-Topology command. */ - _SB.PCI0.TDM1.WACT = 1 - - /* Clear the connect states. */ - CTP1 = 0 - } - /* Disallow to send Connect-Topology cmd. */ - _SB.PCI0.TDM1.ALCT = 0 - } } Else { Printf("Drop TG1N due to it is already exit D3 cold.") } @@ -583,16 +562,10 @@
Printf("Push TBT RPs to D3Cold together") If (_SB.PCI0.TRP2.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP2.PDSX == 1) { - CTP1 = 1 - } /* Put RP2 to D3 cold. */ _SB.PCI0.TRP2.D3CE() } If (_SB.PCI0.TRP3.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP3.PDSX == 1) { - CTP1 = 1 - } /* Put RP3 to D3 cold */ _SB.PCI0.TRP3.D3CE() } diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl index 66950a6..cca69c8 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl @@ -81,58 +81,10 @@ } }
-/* - * TCSS TBT CONNECT_TOPOLOGY MailBox Command Method - */ -Method (CNTP, 0, Serialized) -{ - Local0 = 0 - /* Set Force Power if it is not set */ - If (DFPE == 0) { - DMAD = 0x22 - DFPE = 1 - /* - * Poll the TBT NVM FW Ready bit with timeout(default is 500ms) before - * send the TBT MailBox command. - */ - While ((INFR == 0) && (Local0 < 500)) { - Sleep (1) - Local0++ - } - } - If (Local0 != 100) { - ITMB (0x3E) /* 0x3E, PCIE2TBT_CONNECT_TOPOLOGY_COMMAND */ - } -} - Name (STAT, 0x1) /* Variable to save power state 1 - D0, 0 - D3C */ -Name (ALCT, 0x0) /* Connect-Topology cmd can be sent or not 1 - yes, 0 - no */ -/* - * Wait Connect-Topology cmd done - * 0 - no need to wait - * 1 - need to wait - * 2 - wait in progress - */ -Name (WACT, 0x0)
Method (_PS0, 0, Serialized) { - If (WACT == 1) { - /* - * PCIe rp0/rp1 is grouped with DMA0 and PCIe rp2/rp3 is grouped wit DMA1. - * Whenever the Connect-Topology command is in the process, WACT flag is set 1. - * PCIe root ports 0/1/2/3/ and DMA 0/1 _PS0 method set WACT to 2 to indicate - * other thread's _PS0 to wait for the command completion. WACT is cleared to - * be 0 after command is finished. - */ - WACT = 2 - WFCC (100) /* Wait for command complete. */ - WACT = 0 - } ElseIf (WACT == 2) { - While (WACT != 0) { - Sleep (5) - } - } }
Method (_PS3, 0, Serialized) @@ -178,7 +130,6 @@ { DD3E = 1 /* Enable DMA RTD3 */ STAT = 0 - ALCT = 0x1 /* Allow to send Connect-Topology cmd. */ }
/* diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl index a7eafa4..8172f03 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl @@ -64,7 +64,7 @@ */ Method (_DSM, 4, Serialized) { - Return (Buffer() {0x00}) + return (Buffer() { 0x00 }) }
Device (PXSX) @@ -195,34 +195,8 @@ If (PMEX == 1) { PMEX = 0 /* Disable Power Management SCI */ } + Sleep(100) /* Wait for 100ms before return to OS starts any DS activities. */ - If ((TUID == 0) || (TUID == 1)) { - If (_SB.PCI0.TDM0.WACT == 1) { - /* - * Indicate other thread's _PS0 to wait the response. - */ - _SB.PCI0.TDM0.WACT = 2 - _SB.PCI0.TDM0.WFCC (10) /* Wait for command complete. */ - _SB.PCI0.TDM0.WACT = 0 - } ElseIf (_SB.PCI0.TDM0.WACT == 2) { - While (_SB.PCI0.TDM0.WACT != 0) { - Sleep (5) - } - } - } Else { - If (_SB.PCI0.TDM1.WACT == 1) { - /* - * Indicate other thread's _PS0 to wait the response. - */ - _SB.PCI0.TDM1.WACT = 2 - _SB.PCI0.TDM1.WFCC (10) /* Wait for command complete. */ - _SB.PCI0.TDM1.WACT = 0 - } ElseIf (_SB.PCI0.TDM1.WACT == 2) { - While (_SB.PCI0.TDM1.WACT != 0) { - Sleep (5) - } - } - } }
Method (_PS3, 0, Serialized) @@ -246,27 +220,82 @@ }
Method (_DSD, 0) { - Return ( - Package () { - /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ - ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), - Package () - { - Package (2) { "HotPlugSupportInD3", 1 }, - }, + 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 }, - } - } - ) + /* 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) diff --git a/src/soc/intel/tigerlake/acpi/tcss_xhci.asl b/src/soc/intel/tigerlake/acpi/tcss_xhci.asl index 259b92a..8c62a97 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_xhci.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_xhci.asl @@ -115,23 +115,72 @@ }
/* Super Speed Ports */ - Device (SS01) - { - Name (_ADR, 0x02) - } + Device (SS01) + { + Name (_ADR, 0x02) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, + Package (2) { "usb4-port-number", 0 } + } + }) + } + }
- Device (SS02) - { - Name (_ADR, 0x03) - } + Device (SS02) + { + Name (_ADR, 0x03) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, + Package (2) { "usb4-port-number", 1 } + } + }) + } + }
- Device (SS03) - { - Name (_ADR, 0x04) - } + Device (SS03) + { + Name (_ADR, 0x04) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, + Package (2) { "usb4-port-number", 0 } + } + }) + } + }
- Device (SS04) - { - Name (_ADR, 0x05) - } + Device (SS04) + { + Name (_ADR, 0x05) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, + Package (2) { "usb4-port-number", 1 } + } + }) + } + } + }
Hello build bot (Jenkins), Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly (CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across xhci and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 182 insertions(+), 235 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/2
Divya S Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 2: Code-Review+1
This is needed for SW CM power management support, also disables PCIe tunneling. TBT FW for this is already merged in CL:3107594.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+1
This is needed for SW CM power management support, also disables PCIe tunneling. TBT FW for this is already merged in CL:3107594.
Hi Divya; What will we need to do to re-enable it (from the kernel)? A few folks on our side are looking at PCIe related behaviour (IOMMU enablement, resource allocation etc), so would be good to know if this is something that can be re-enabled via sysfs or kernel command line.
Divya S Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 2:
Let me check with kernel team and get back to you.
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly (CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across xhci and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 240 insertions(+), 242 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/3
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly (CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across xhci and PCIe root ports. Enable TCSS device PM based on devices on/off configuration from platform devicetree setting.
BUG=b:140645231 TEST=Check Type C device connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 240 insertions(+), 242 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 2:
(23 comments)
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@10 PS2, Line 10: doman domain
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@10 PS2, Line 10: internet : protocol Internet Protocol
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@11 PS2, Line 11: Topogoly Topology
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@13 PS2, Line 13: xhci XHCI
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@16 PS2, Line 16: TEST=Check Type C device connection/enumeration with SW CM. all ports, all orientations?
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@9 PS2, Line 9: This change adds support for SW CM. Add Operating System Capabilities : (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet : protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly : (CNTP) command because kernel driver directly works with SW CM Thunderbolt : firmware. Update _DSD method for USB4 support across xhci and PCIe root ports. : : BUG=b:140645231 : TEST=Check Type C device connection/enumeration with SW CM. 72 characters wide for commit msg please
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 138: definiton definition
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 138: is are
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 146: acknolwedges acknowledges
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 147: Preserved bits in the control : * field from the platform to OSPM, while masked/cleared bits in the : * control field indicate that the platform does not permit OSPM control : * of the respective capabilities or features I think you can just simplify this by saying: "Masked/Cleared bits in the control field indicate that the platform does not permit OSPM control of the respective capabilities or features."
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 155: CreateDWordField(Arg3, 8, CDW3) I'm confused, doesn't Arg3 only contain 3 DWords? So this index (8) would be out of bounds?
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 158: 1 symbolic constant please (looks like a revision?)
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 159: 0x8 /* Unknown revision */ if you made a symbolic constant for 8 meaning UNKNOWN_REVISION (or similar) you wouldn't need the comment.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 160: } Also don't you want to Return (Arg3) if the revision is unknown?
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 161: 0xb symbolic constant please (maybe USB4_TUNNELS_ALL_EXCEPT_PCIE ?)
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 165: 0x4 /* Unrecognized UUID */ if you made a symbolic constant for 8 meaning UNRECOGNIZED_UUID (or similar) you wouldn't need the comment
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_dma.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 30: : Method (_PS0, 0, Serialized) : { : } : : Method (_PS3, 0, Serialized) : { : } _PS0 and _PS3 are now empty, please remove them.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 67: return didn't need to change the case here
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 199: Sleep(100) /* Wait for 100ms before return to OS starts any DS activities. */ If all this Method is doing is disabling SCIs, does the 100ms sleep still need to be here?
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 224: 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 }, : } This is extremely repetitive, could you refactor out the outer Package() definition from each branch of the conditional into a Method? Method (DSDH, 2) { Return Package () { .... Package() { "usb4-host-interface", Arg0 }, Package() { "usb4-port-number", Arg1 } .... } }
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 223: 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 }, : } : }) : } tabs please
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_xhci.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 125: ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 0 } : } Same thing here, could you refactor out this inner part of each of these 4 methods _DSD methods into a helper like above?
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 118: Device (SS01) : { : Name (_ADR, 0x02) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 0 } : } : }) : } : } : : Device (SS02) : { : Name (_ADR, 0x03) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 1 } : } : }) : } : } : : Device (SS03) : { : Name (_ADR, 0x04) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, : Package (2) { "usb4-port-number", 0 } : } : }) : } : } : : Device (SS04) : { : Name (_ADR, 0x05) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, : Package (2) { "usb4-port-number", 1 } : } : }) : } : } tabs please
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 4:
(23 comments)
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@10 PS2, Line 10: doman
domain
Done
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@10 PS2, Line 10: internet : protocol
Internet Protocol
Done
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@11 PS2, Line 11: Topogoly
Topology
Done
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@13 PS2, Line 13: xhci
XHCI
Done
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@16 PS2, Line 16: TEST=Check Type C device connection/enumeration with SW CM.
all ports, all orientations?
Done
https://review.coreboot.org/c/coreboot/+/42295/2//COMMIT_MSG@9 PS2, Line 9: This change adds support for SW CM. Add Operating System Capabilities : (_OSC) method to enable USB/DisplayPort/Inter-doman USB4 internet : protocol tunneling and disable PCIe tunneling. Remove Connect Topogoly : (CNTP) command because kernel driver directly works with SW CM Thunderbolt : firmware. Update _DSD method for USB4 support across xhci and PCIe root ports. : : BUG=b:140645231 : TEST=Check Type C device connection/enumeration with SW CM.
72 characters wide for commit msg please
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 138: definiton
definition
Ack
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 138: is
are
Ack
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 146: acknolwedges
acknowledges
Ack
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 147: Preserved bits in the control : * field from the platform to OSPM, while masked/cleared bits in the : * control field indicate that the platform does not permit OSPM control : * of the respective capabilities or features
I think you can just simplify this by saying: […]
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 155: CreateDWordField(Arg3, 8, CDW3)
I'm confused, doesn't Arg3 only contain 3 DWords? So this index (8) would be out of bounds?
corrected.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 158: 1
symbolic constant please (looks like a revision?)
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 159: 0x8 /* Unknown revision */
if you made a symbolic constant for 8 meaning UNKNOWN_REVISION (or similar) you wouldn't need the co […]
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 160: }
Also don't you want to Return (Arg3) if the revision is unknown?
If the revision is unknown, Arg3 is returned as "Return (Arg3)".
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 161: 0xb
symbolic constant please (maybe USB4_TUNNELS_ALL_EXCEPT_PCIE ?)
Added symbolic constants.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 165: 0x4 /* Unrecognized UUID */
if you made a symbolic constant for 8 meaning UNRECOGNIZED_UUID (or similar) you wouldn't need the c […]
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_dma.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 30: : Method (_PS0, 0, Serialized) : { : } : : Method (_PS3, 0, Serialized) : { : }
_PS0 and _PS3 are now empty, please remove them.
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 67: return
didn't need to change the case here
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 199: Sleep(100) /* Wait for 100ms before return to OS starts any DS activities. */
If all this Method is doing is disabling SCIs, does the 100ms sleep still need to be here?
PCIe spec mandates to wait 100ms after link has been trained before sending configuration request to the device. It is conserved to keep this delay along with kernel driver. Check it late if there is need for optimization.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 224: 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 }, : }
This is extremely repetitive, could you refactor out the outer Package() definition from each branch […]
It seems arguments won't be got into the package() in method.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 223: 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 }, : } : }) : }
tabs please
Done
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_xhci.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 125: ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 0 } : }
Same thing here, could you refactor out this inner part of each of these 4 methods _DSD methods into […]
It seems arguments are not got into Package in method.
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 118: Device (SS01) : { : Name (_ADR, 0x02) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 0 } : } : }) : } : } : : Device (SS02) : { : Name (_ADR, 0x03) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, : Package (2) { "usb4-port-number", 1 } : } : }) : } : } : : Device (SS03) : { : Name (_ADR, 0x04) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, : Package (2) { "usb4-port-number", 0 } : } : }) : } : } : : Device (SS04) : { : Name (_ADR, 0x05) : Method (_DSD, 0, NotSerialized) : { : Return( Package () : { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () : { : Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, : Package (2) { "usb4-port-number", 1 } : } : }) : } : }
tabs please
Done
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#5).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-domain USB4 Internet Protocol tunneling and disable PCIe tunneling. Remove Connect Topology (CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across XHCI and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device all ports connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 217 insertions(+), 220 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/5
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-domain USB4 Internet Protocol tunneling and enable PCIe tunneling as well. Remove Connect Topology(CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across XHCI and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device all ports connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 217 insertions(+), 220 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/6
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 224: 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 }, : }
It seems arguments won't be got into the package() in method.
For integers it should work but for others you'd probably need to pass a reference to the object with RefOf() and then DeRefOf() from inside the method.
It can get messy quick because IIRC not every Package element works the same way. But for specific cases like this where the argument types can be known it should be possible.
BUT, I have a patch series that moves this entire _DSD to generated code so we can specify these in the devicetree. (Not convenient to merge _DSD across DSDT and SSDT) So for now I'm not too concerned about the duplication.
Divya Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6:
Patch Set 2:
Patch Set 2: Code-Review+1
This is needed for SW CM power management support, also disables PCIe tunneling. TBT FW for this is already merged in CL:3107594.
Hi Divya; What will we need to do to re-enable it (from the kernel)? A few folks on our side are looking at PCIe related behaviour (IOMMU enablement, resource allocation etc), so would be good to know if this is something that can be re-enabled via sysfs or kernel command line.
Based on verification we see no impact with this bit set / clear for PCIe tunneling. But from the comments we got, we understand that clearing this bit should disable the PCIe tunneling permanently. Since we do not want that at this point, in this CL we will keep this bit enabled. Latest patchset already addresses this. For disabling PCIe tunneling we can further discuss here 141545149.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/42295/2/src/soc/intel/tigerlake/acp... PS2, Line 224: 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 }, : }
For integers it should work but for others you'd probably need to pass a reference to the object wit […]
Thanks.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... PS6, Line 138: Method (_OSC, 4, Serialized) : { : /* : * Operating System Capabilities for USB4 : * Arg0: UUID = {23A0D13A-26AB-486C-9C5F-0FFA525A575A} : * Arg1: Revision ID = 1 : * Arg2: Count of entries (DWORD) in Arg3 (Integer): 3 : * Arg3: DWORD capabilities buffer: : * First DWORD: The standard definition bits are used to return errors. : * Second DWORD: OSPM support field for USB4, bits [31:0] reserved. : * Third DWORD: OSPM control field for USB4. : * bit 0: USB tunneling : * bit 1: DisplayPort tunneling : * bit 2: PCIe tunneling : * bit 3: Inter-domain USB4 internet protocol : * bit 31:4: reserved : * Return: The platform acknowledges the capabilities buffer by returning : * a buffer of DWORD of the same length. Masked/Cleared bits in the : * control field indicate that the platform does not permit OSPM : * control of the respectively capabilities or features. : */ : Name (CTRL, 0) /* Control field value */ : If (Arg0 == ToUUID("23A0D13A-26AB-486C-9C5F-0FFA525A575A")) { : CreateDWordField(Arg3, 0, CDW1) : CreateDWordField(Arg3, 2, CDW3) : CTRL = CDW3 : : If (Arg1 != REVISION_ID) { : CDW1 |= UNRECOGNIZED_REVISION : } : CTRL |= USB_TUNNELING | DISPLAY_PORT_TUNNELING | PCIE_TUNNELING | : INTER_DOMAIN_USB4_INTERNET_PROTOCOL : CDW3 = CTRL : Return (Arg3) : } Else { : CDW1 |= UNRECOGNIZED_UUID : Return (Arg3) : } : } Thank you, this is much cleaner now 😊 but all of this should be indented with tabs, not spaces.
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... PS6, Line 306: 0x7090 Maybe another symbolic constant? MCHBAR_TCSS_DEVEN_OFFSET ?
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... PS6, Line 138: Method (_OSC, 4, Serialized) : { : /* : * Operating System Capabilities for USB4 : * Arg0: UUID = {23A0D13A-26AB-486C-9C5F-0FFA525A575A} : * Arg1: Revision ID = 1 : * Arg2: Count of entries (DWORD) in Arg3 (Integer): 3 : * Arg3: DWORD capabilities buffer: : * First DWORD: The standard definition bits are used to return errors. : * Second DWORD: OSPM support field for USB4, bits [31:0] reserved. : * Third DWORD: OSPM control field for USB4. : * bit 0: USB tunneling : * bit 1: DisplayPort tunneling : * bit 2: PCIe tunneling : * bit 3: Inter-domain USB4 internet protocol : * bit 31:4: reserved : * Return: The platform acknowledges the capabilities buffer by returning : * a buffer of DWORD of the same length. Masked/Cleared bits in the : * control field indicate that the platform does not permit OSPM : * control of the respectively capabilities or features. : */ : Name (CTRL, 0) /* Control field value */ : If (Arg0 == ToUUID("23A0D13A-26AB-486C-9C5F-0FFA525A575A")) { : CreateDWordField(Arg3, 0, CDW1) : CreateDWordField(Arg3, 2, CDW3) : CTRL = CDW3 : : If (Arg1 != REVISION_ID) { : CDW1 |= UNRECOGNIZED_REVISION : } : CTRL |= USB_TUNNELING | DISPLAY_PORT_TUNNELING | PCIE_TUNNELING | : INTER_DOMAIN_USB4_INTERNET_PROTOCOL : CDW3 = CTRL : Return (Arg3) : } Else { : CDW1 |= UNRECOGNIZED_UUID : Return (Arg3) : } : }
Thank you, this is much cleaner now 😊 […]
Done
https://review.coreboot.org/c/coreboot/+/42295/6/src/soc/intel/tigerlake/acp... PS6, Line 306: 0x7090
Maybe another symbolic constant? MCHBAR_TCSS_DEVEN_OFFSET ?
Done
Hello build bot (Jenkins), Furquan Shaikh, Wonkyu Kim, Duncan Laurie, Shamile Khan, Paul Menzel, Nick Vaccaro, Tim Wawrzynczak, Patrick Rudolph, Prashant Malani, Divya S Sasidharan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42295
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-domain USB4 Internet Protocol tunneling and enable PCIe tunneling as well. Remove Connect Topology(CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across XHCI and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device all ports connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 219 insertions(+), 220 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/42295/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 7: Code-Review+2
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
soc/intel/tigerlake: Update TCSS for SW CM support
This change adds support for SW CM. Add Operating System Capabilities (_OSC) method to enable USB/DisplayPort/Inter-domain USB4 Internet Protocol tunneling and enable PCIe tunneling as well. Remove Connect Topology(CNTP) command because kernel driver directly works with SW CM Thunderbolt firmware. Update _DSD method for USB4 support across XHCI and PCIe root ports.
BUG=b:140645231 TEST=Check Type C device all ports connection/enumeration with SW CM.
Signed-off-by: John Zhao john.zhao@intel.com Change-Id: I859c5075882e40d7be30d4ba88cc825886712b74 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42295 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/acpi/tcss.asl M src/soc/intel/tigerlake/acpi/tcss_dma.asl M src/soc/intel/tigerlake/acpi/tcss_pcierp.asl M src/soc/intel/tigerlake/acpi/tcss_xhci.asl 4 files changed, 219 insertions(+), 220 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/acpi/tcss.asl b/src/soc/intel/tigerlake/acpi/tcss.asl index 48f2689..2fda693 100644 --- a/src/soc/intel/tigerlake/acpi/tcss.asl +++ b/src/soc/intel/tigerlake/acpi/tcss.asl @@ -29,9 +29,19 @@ #define MAILBOX_BIOS_CMD_TCSS_DEVEN_INTERFACE 0x00000015 #define TCSS_DEVEN_MAILBOX_SUBCMD_GET_STATUS 0 /* Sub-command 0 */ #define TCSS_DEVEN_MAILBOX_SUBCMD_TCSS_CHANGE_REQ 1 /* Sub-command 1 */ - #define TCSS_IOM_ACK_TIMEOUT_IN_MS 100
+#define MCHBAR_TCSS_DEVEN_OFFSET 0x7090 + +#define REVISION_ID 1 +#define UNRECOGNIZED_UUID 0x4 +#define UNRECOGNIZED_REVISION 0x8 + +#define USB_TUNNELING 0x1 +#define DISPLAY_PORT_TUNNELING 0x2 +#define PCIE_TUNNELING 0x4 +#define INTER_DOMAIN_USB4_INTERNET_PROTOCOL 0x8 + Scope (_SB) { /* Device base address */ @@ -126,6 +136,46 @@ } Return (0) } + + Method (_OSC, 4, Serialized) + { + /* + * Operating System Capabilities for USB4 + * Arg0: UUID = {23A0D13A-26AB-486C-9C5F-0FFA525A575A} + * Arg1: Revision ID = 1 + * Arg2: Count of entries (DWORD) in Arg3 (Integer): 3 + * Arg3: DWORD capabilities buffer: + * First DWORD: The standard definition bits are used to return errors. + * Second DWORD: OSPM support field for USB4, bits [31:0] reserved. + * Third DWORD: OSPM control field for USB4. + * bit 0: USB tunneling + * bit 1: DisplayPort tunneling + * bit 2: PCIe tunneling + * bit 3: Inter-domain USB4 internet protocol + * bit 31:4: reserved + * Return: The platform acknowledges the capabilities buffer by returning + * a buffer of DWORD of the same length. Masked/Cleared bits in the + * control field indicate that the platform does not permit OSPM + * control of the respectively capabilities or features. + */ + Name (CTRL, 0) /* Control field value */ + If (Arg0 == ToUUID("23A0D13A-26AB-486C-9C5F-0FFA525A575A")) { + CreateDWordField(Arg3, 0, CDW1) + CreateDWordField(Arg3, 2, CDW3) + CTRL = CDW3 + + If (Arg1 != REVISION_ID) { + CDW1 |= UNRECOGNIZED_REVISION + } + CTRL |= USB_TUNNELING | DISPLAY_PORT_TUNNELING | PCIE_TUNNELING | + INTER_DOMAIN_USB4_INTERNET_PROTOCOL + CDW3 = CTRL + Return (Arg3) + } Else { + CDW1 |= UNRECOGNIZED_UUID + Return (Arg3) + } + } }
Scope (_GPE) @@ -252,6 +302,25 @@ Scope (_SB.PCI0) { /* + * Operation region defined to access the TCSS_DEVEN. Get the MCHBAR in offset + * 0x48 in B0:D0:F0. TCSS device enable base address is in offset 0x7090 of MCHBAR. + */ + OperationRegion (TDEN, SystemMemory, (GMHB() + MCHBAR_TCSS_DEVEN_OFFSET), 0x4) + Field (TDEN, ByteAcc, NoLock, Preserve) + { + TRE0, 1, /* PCIE0_EN */ + TRE1, 1, /* PCIE1_EN */ + TRE2, 1, /* PCIE2_EN */ + TRE3, 1, /* PCIE3_EN */ + , 4, + THCE, 1, /* XHCI_EN */ + TDCE, 1, /* XDCI_EN */ + DME0, 1, /* TBT_DMA0_EN */ + DME1, 1, /* TBT_DMA1_EN */ + , 20 + } + + /* * Operation region defined to access the IOM REGBAR. Get the MCHBAR in offset * 0x48 in B0:D0:F0. REGBAR Base address is in offset 0x7110 of MCHBAR. */ @@ -424,17 +493,6 @@ }
/* - * Below is a variable to store devices connect state for TBT PCIe RP before - * entering D3 cold. - * Value 0 - no device connected before enter D3 cold, no need to send - * CONNECT_TOPOLOGY in D3 cold exit. - * Value 1 - has device connected before enter D3 cold, need to send - * CONNECT_TOPOLOGY in D3 cold exit. - */ - Name (CTP0, 0) /* Variable of device connecet status for TBT0 group. */ - Name (CTP1, 0) /* Variable of device connecet status for TBT1 group. */ - - /* * TBT Group0 ON method */ Method (TG0N, 0) @@ -455,28 +513,6 @@ /* RP1 D3 cold exit. */ _SB.PCI0.TRP1.D3CX() } - - /* - * Need to send Connect-Topology command when TBT host - * controller back to D0 from D3. - */ - If (_SB.PCI0.TDM0.ALCT == 1) { - If (CTP0 == 1) { - /* - * Send Connect-Topology command if there is - * device present on PCIe RP. - */ - _SB.PCI0.TDM0.CNTP() - - /* Indicate to wait Connect-Topology command. */ - _SB.PCI0.TDM0.WACT = 1 - - /* Clear the connect states. */ - CTP0 = 0 - } - /* Disallow to send Connect-Topology command. */ - _SB.PCI0.TDM0.ALCT = 0 - } } Else { Printf("Drop TG0N due to it is already exit D3 cold.") } @@ -500,16 +536,10 @@
Printf("Push TBT RPs to D3Cold together") If (_SB.PCI0.TRP0.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP0.PDSX == 1) { - CTP0 = 1 - } /* Put RP0 to D3 cold. */ _SB.PCI0.TRP0.D3CE() } If (_SB.PCI0.TRP1.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP1.PDSX == 1) { - CTP0 = 1 - } /* Put RP1 to D3 cold. */ _SB.PCI0.TRP1.D3CE() } @@ -538,28 +568,6 @@ /* RP3 D3 cold exit. */ _SB.PCI0.TRP3.D3CX() } - - /* - * Need to send Connect-Topology command when TBT host - * controller back to D0 from D3. - */ - If (_SB.PCI0.TDM1.ALCT == 1) { - If (CTP1 == 1) { - /* - * Send Connect-Topology command if there is - * device present on PCIe RP. - */ - _SB.PCI0.TDM1.CNTP() - - /* Indicate to wait Connect-Topology command. */ - _SB.PCI0.TDM1.WACT = 1 - - /* Clear the connect states. */ - CTP1 = 0 - } - /* Disallow to send Connect-Topology cmd. */ - _SB.PCI0.TDM1.ALCT = 0 - } } Else { Printf("Drop TG1N due to it is already exit D3 cold.") } @@ -583,16 +591,10 @@
Printf("Push TBT RPs to D3Cold together") If (_SB.PCI0.TRP2.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP2.PDSX == 1) { - CTP1 = 1 - } /* Put RP2 to D3 cold. */ _SB.PCI0.TRP2.D3CE() } If (_SB.PCI0.TRP3.VDID != 0xFFFFFFFF) { - If (_SB.PCI0.TRP3.PDSX == 1) { - CTP1 = 1 - } /* Put RP3 to D3 cold */ _SB.PCI0.TRP3.D3CE() } @@ -763,7 +765,11 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (THCE == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } #include "tcss_xhci.asl" } @@ -781,7 +787,11 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (DME0 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } #include "tcss_dma.asl" } @@ -799,7 +809,11 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (DME1 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } #include "tcss_dma.asl" } @@ -818,8 +832,13 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (TRE0 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } + Method (_INI) { LTEN = 0 @@ -843,8 +862,13 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (TRE1 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } + Method (_INI) { LTEN = 0 @@ -868,8 +892,13 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (TRE2 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } + Method (_INI) { LTEN = 0 @@ -893,8 +922,13 @@
Method (_STA, 0x0, NotSerialized) { - Return (0x0F) + If (TRE3 == 1) { + Return (0x0F) + } Else { + Return (0x0) + } } + Method (_INI) { LTEN = 0 diff --git a/src/soc/intel/tigerlake/acpi/tcss_dma.asl b/src/soc/intel/tigerlake/acpi/tcss_dma.asl index 66950a6..8eab92f 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_dma.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_dma.asl @@ -26,118 +26,7 @@ DMAD, 8 /* 31:24 DMA Active Delay */ }
-/* - * TBT MailBox Command Method - * Arg0 - MailBox Cmd ID - */ -Method (ITMB, 1, Serialized) -{ - Local0 = Arg0 | 0x1 /* 0x1, PCIE2TBT_VLD_B */ - P2TB = Local0 -} - -/* - * Wait For Command Completed - * Arg0 - TimeOut value (unit is 1 millisecond) - */ -Method (WFCC, 1, Serialized) -{ - WTBS (Arg0) - P2TB = 0 - WTBC (Arg0) -} - -/* - * Wait For Command Set - * Arg0 - TimeOut value - */ -Method (WTBS, 1, Serialized) -{ - Local0 = Arg0 - While (Local0 > 0) { - /* Wait for Bit to Set. */ - If (TB2P & 0x1) { /* 0x1, TBT2PCIE_DON_R */ - Break - } - Local0-- - Sleep (1) - } -} - -/* - * Wait For Command Clear - * Arg0 - TimeOut value - */ -Method (WTBC, 1, Serialized) -{ - Local0 = Arg0 - While (Local0 > 0) { - /* Wait for Bit to Clear. */ - If ((TB2P & 0x1) != 0x0) { /* 0x1, TBT2PCIE_DON_R */ - Break - } - Local0-- - Sleep (1) - } -} - -/* - * TCSS TBT CONNECT_TOPOLOGY MailBox Command Method - */ -Method (CNTP, 0, Serialized) -{ - Local0 = 0 - /* Set Force Power if it is not set */ - If (DFPE == 0) { - DMAD = 0x22 - DFPE = 1 - /* - * Poll the TBT NVM FW Ready bit with timeout(default is 500ms) before - * send the TBT MailBox command. - */ - While ((INFR == 0) && (Local0 < 500)) { - Sleep (1) - Local0++ - } - } - If (Local0 != 100) { - ITMB (0x3E) /* 0x3E, PCIE2TBT_CONNECT_TOPOLOGY_COMMAND */ - } -} - Name (STAT, 0x1) /* Variable to save power state 1 - D0, 0 - D3C */ -Name (ALCT, 0x0) /* Connect-Topology cmd can be sent or not 1 - yes, 0 - no */ -/* - * Wait Connect-Topology cmd done - * 0 - no need to wait - * 1 - need to wait - * 2 - wait in progress - */ -Name (WACT, 0x0) - -Method (_PS0, 0, Serialized) -{ - If (WACT == 1) { - /* - * PCIe rp0/rp1 is grouped with DMA0 and PCIe rp2/rp3 is grouped wit DMA1. - * Whenever the Connect-Topology command is in the process, WACT flag is set 1. - * PCIe root ports 0/1/2/3/ and DMA 0/1 _PS0 method set WACT to 2 to indicate - * other thread's _PS0 to wait for the command completion. WACT is cleared to - * be 0 after command is finished. - */ - WACT = 2 - WFCC (100) /* Wait for command complete. */ - WACT = 0 - } ElseIf (WACT == 2) { - While (WACT != 0) { - Sleep (5) - } - } -} - -Method (_PS3, 0, Serialized) -{ -}
Method (_S0W, 0x0) { @@ -178,7 +67,6 @@ { DD3E = 1 /* Enable DMA RTD3 */ STAT = 0 - ALCT = 0x1 /* Allow to send Connect-Topology cmd. */ }
/* diff --git a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl index a7eafa4..096a673 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_pcierp.asl @@ -64,7 +64,7 @@ */ Method (_DSM, 4, Serialized) { - Return (Buffer() {0x00}) + Return (Buffer() { 0x00 }) }
Device (PXSX) @@ -195,34 +195,8 @@ If (PMEX == 1) { PMEX = 0 /* Disable Power Management SCI */ } - Sleep(100) /* Wait for 100ms before return to OS starts any DS activities. */ - If ((TUID == 0) || (TUID == 1)) { - If (_SB.PCI0.TDM0.WACT == 1) { - /* - * Indicate other thread's _PS0 to wait the response. - */ - _SB.PCI0.TDM0.WACT = 2 - _SB.PCI0.TDM0.WFCC (10) /* Wait for command complete. */ - _SB.PCI0.TDM0.WACT = 0 - } ElseIf (_SB.PCI0.TDM0.WACT == 2) { - While (_SB.PCI0.TDM0.WACT != 0) { - Sleep (5) - } - } - } Else { - If (_SB.PCI0.TDM1.WACT == 1) { - /* - * Indicate other thread's _PS0 to wait the response. - */ - _SB.PCI0.TDM1.WACT = 2 - _SB.PCI0.TDM1.WFCC (10) /* Wait for command complete. */ - _SB.PCI0.TDM1.WACT = 0 - } ElseIf (_SB.PCI0.TDM1.WACT == 2) { - While (_SB.PCI0.TDM1.WACT != 0) { - Sleep (5) - } - } - } + + Sleep(100) /* Wait for 100ms before return to OS starts any OS activities. */ }
Method (_PS3, 0, Serialized) @@ -246,8 +220,8 @@ }
Method (_DSD, 0) { - Return ( - Package () { + If ((TUID == 0) || (TUID == 1)) { + Return ( Package() { /* acpi_pci_bridge_d3 at ../drivers/pci/pci-acpi.c */ ToUUID("6211E2C0-58A3-4AF3-90E1-927A4E0C55A4"), Package () @@ -264,9 +238,64 @@ * (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) diff --git a/src/soc/intel/tigerlake/acpi/tcss_xhci.asl b/src/soc/intel/tigerlake/acpi/tcss_xhci.asl index 259b92a..c32deed 100644 --- a/src/soc/intel/tigerlake/acpi/tcss_xhci.asl +++ b/src/soc/intel/tigerlake/acpi/tcss_xhci.asl @@ -118,20 +118,68 @@ Device (SS01) { Name (_ADR, 0x02) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, + Package (2) { "usb4-port-number", 0 } + } + }) + } }
Device (SS02) { Name (_ADR, 0x03) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM0 }, + Package (2) { "usb4-port-number", 1 } + } + }) + } }
Device (SS03) { Name (_ADR, 0x04) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, + Package (2) { "usb4-port-number", 0 } + } + }) + } }
Device (SS04) { Name (_ADR, 0x05) + Method (_DSD, 0, NotSerialized) + { + Return( Package () + { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package () + { + Package (2) { "usb4-host-interface", _SB.PCI0.TDM1 }, + Package (2) { "usb4-port-number", 1 } + } + }) + } } }
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 9:
Patch Set 2:
Let me check with kernel team and get back to you.
Divya, were you able to follow up on this? Kindly let us know, so that the people working on USB4 related functionality are aware of any functionality changes.
Divya Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42295 )
Change subject: soc/intel/tigerlake: Update TCSS for SW CM support ......................................................................
Patch Set 9:
Patch Set 9:
Patch Set 2:
Let me check with kernel team and get back to you.
Divya, were you able to follow up on this? Kindly let us know, so that the people working on USB4 related functionality are aware of any functionality changes.
Let me add to internal thread from the comments we got, we understand that clearing this bit should disable the PCIe tunneling permanently. Hence the change in latest patchset is to enable PCIe tunneling from coreboot.
-
Divya S Sasidharan (Code Review) -
Divya Sasidharan (Code Review)
-
Duncan Laurie (Code Review)
-
John Zhao (Code Review)
-
Prashant Malani (Code Review)
-
Tim Wawrzynczak (Code Review)