Change in coreboot[master]: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default
Set the default state of the TCSS PCIe RP0 to hidden so that coreboot does not allocate resources to this hotplug root port. The default behavior on the reference design is that there is only one USB4 port attached to port C1 while port C0 is only a USB3 port.
Meanwhile the Voxel and Terrador variants do have USB4 on both C0 and C1 ports, so these boards change the default to 'on' so that coreboot does allocate resources for the hotplug port.
BUG=b:159143739 BRANCH=volteer TEST=build volteer and voxel and check the resulting static.c to ensure the device is hidden or not. Also boot with the two different configurations and ensure resources are assigned or not. Finally check that S0ix still functions with the C0 port set to 'hidden' after authorizing a PCIe tunnel on port C1.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I8bb05ae8cd14412854212b7ed189cfa43d602c1d --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb 3 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/47198/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 6a8553c..f443421 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -414,7 +414,10 @@ device generic 0 on end end end # DPTF 0x9A03 - device ref tbt_pcie_rp0 on + # Volteer reference design does not have PCIe on Type-C port C0 so it should + # not have hotplug resources allocated. Marking the device hidden will ensure + # it is still enabled so it can participate in power management. + device ref tbt_pcie_rp0 hidden probe DB_USB USB4_GEN2 probe DB_USB USB4_GEN3 end diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index e0dac7a..7fd1c86 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -188,6 +188,11 @@ device generic 0 on end end end + # This variant has USB4/PCIe on both ports so RP0 must be enabled + # in order for hotplug resources to be assigned to Type-C Port C0. + device ref tbt_pcie_rp0 on + probe DB_USB USB4_GEN3 + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. diff --git a/src/mainboard/google/volteer/variants/voxel/overridetree.cb b/src/mainboard/google/volteer/variants/voxel/overridetree.cb index 7c95c50..064f3f3 100644 --- a/src/mainboard/google/volteer/variants/voxel/overridetree.cb +++ b/src/mainboard/google/volteer/variants/voxel/overridetree.cb @@ -202,6 +202,11 @@ device generic 0 on end end end + # This variant has USB4/PCIe on both ports so RP0 must be enabled + # in order for hotplug resources to be assigned to Type-C Port C0. + device ref tbt_pcie_rp0 on + probe DB_USB USB4_GEN3 + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
Patch Set 3: Code-Review+2
clever
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... PS5, Line 418: so it can participate in power management. Where does the power management happen? Is it in the OS (part of ACPI)?
Shouldn't this port be disabled and turned off in coreboot/FSP?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... PS5, Line 418: so it can participate in power management.
Where does the power management happen? Is it in the OS (part of ACPI)? […]
I think because both root ports are part of the same UBS4 host controller they are both part of the power management flow, and the controller can't enter D3 until both root ports have been transitioned. (currently this all in static ASL for USB4/TBT)
There might be a way to make the PM work with coreboot disabling the port but when I tried it before the system failed to go into S0ix with errors, and I no longer have a TBT dock to test with so I'm wary to touch that ASL.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/47198/5/src/mainboard/google/voltee... PS5, Line 418: so it can participate in power management.
I think because both root ports are part of the same UBS4 host controller they are both part of the […]
Ack
Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47198 )
Change subject: mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default ......................................................................
mb/google/volteer/variants: Set TCSS PCIe RP0 to hidden by default
Set the default state of the TCSS PCIe RP0 to hidden so that coreboot does not allocate resources to this hotplug root port. The default behavior on the reference design is that there is only one USB4 port attached to port C1 while port C0 is only a USB3 port.
Meanwhile the Voxel and Terrador variants do have USB4 on both C0 and C1 ports, so these boards change the default to 'on' so that coreboot does allocate resources for the hotplug port.
BUG=b:159143739 BRANCH=volteer TEST=build volteer and voxel and check the resulting static.c to ensure the device is hidden or not. Also boot with the two different configurations and ensure resources are assigned or not. Finally check that S0ix still functions with the C0 port set to 'hidden' after authorizing a PCIe tunnel on port C1.
Signed-off-by: Duncan Laurie dlaurie@google.com Change-Id: I8bb05ae8cd14412854212b7ed189cfa43d602c1d Reviewed-on: https://review.coreboot.org/c/coreboot/+/47198 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb 3 files changed, 14 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index fa807ba..bc93ec9 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -414,7 +414,10 @@ device generic 0 on end end end # DPTF 0x9A03 - device ref tbt_pcie_rp0 on + # Volteer reference design does not have PCIe on Type-C port C0 so it should + # not have hotplug resources allocated. Marking the device hidden will ensure + # it is still enabled so it can participate in power management. + device ref tbt_pcie_rp0 hidden probe DB_USB USB4_GEN2 probe DB_USB USB4_GEN3 end diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index 87959b8..2a5b98d 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -187,6 +187,11 @@ device generic 0 on end end end + # This variant has USB4/PCIe on both ports so RP0 must be enabled + # in order for hotplug resources to be assigned to Type-C Port C0. + device ref tbt_pcie_rp0 on + probe DB_USB USB4_GEN3 + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. diff --git a/src/mainboard/google/volteer/variants/voxel/overridetree.cb b/src/mainboard/google/volteer/variants/voxel/overridetree.cb index cdbde98..dd29553 100644 --- a/src/mainboard/google/volteer/variants/voxel/overridetree.cb +++ b/src/mainboard/google/volteer/variants/voxel/overridetree.cb @@ -202,6 +202,11 @@ device generic 0 on end end end + # This variant has USB4/PCIe on both ports so RP0 must be enabled + # in order for hotplug resources to be assigned to Type-C Port C0. + device ref tbt_pcie_rp0 on + probe DB_USB USB4_GEN3 + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy.
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Tim Wawrzynczak (Code Review)