On Fri, Feb 04, 2022 at 04:29:56PM -0600, Bjorn Helgaas wrote: > I've never liked "is_thunderbolt" because it tells us nothing about > what functionality is of interest, so it's an unmaintainable mess. > > Right now: > > - We assume Root Ports and Switch Ports marked "is_thunderbolt" > support D3 (pci_bridge_d3_possible()). We don't allow D3 on hotplug bridges because: /* * Hotplug ports handled natively by the OS were not validated * by vendors for runtime D3 at least until 2018 because there * was no OS support. */ if (bridge->is_hotplug_bridge) return false; And we don't allow D3 on older non-hotplug bridges because: /* * It should be safe to put PCIe ports from 2015 or newer * to D3. */ if (dmi_get_bios_year() >= 2015) return true; However we must allow D3 on *Thunderbolt* bridges to take advantage of power savings. So the following check is an exception of the above-stated rules: /* Even the oldest 2010 Thunderbolt controller supports D3. */ if (bridge->is_thunderbolt) return true; This is most likely necessary for AMD Thunderbolt as well, but could be achieved by adding another check to pci_bridge_d3_possible() which returns true for the USB4 class. > - Downstream Ports marked "is_thunderbolt" don't support native > hotplug Command Completed events, even if they claim they do > (pcie_init()). That's a quirk needed for older Thunderbolt controllers. It could be replaced by a check for the device IDs listed in 493fb50e958c. It most likely does not affect AMD Thunderbolt. > - Apparently, if *any* device in the system is marked > "is_thunderbolt", a GPU external DP port is not fully switchable > because ? (gmux_probe()). This could be replaced by a DMI check for the affected MacBook Pro models. Those happen not to possess a Thunderbolt controller, so checking for Thunderbolt presence seemed simpler and more clever at the time... I can produce a list of affected models if you want. This does not affect AMD Thunderbolt. > - Whether an AMD GPU is attached via Thunderbolt tells us something > about what sort of power control and runtime power management we > can do (amdgpu_driver_load_kms(), radeon_driver_load_kms()). External eGPUs are not supposed to be managed by vga_switcheroo (which is only responsible for switching between a chipset-integrated iGPU and an on-board discrete dGPU), that's what these checks are for. This does affect AMD Thunderbolt. > - We don't register Thunderbolt eGPU devices with VGA switcheroo > because ? (nouveau_vga_init(), radeon_device_init()). Same as above. > - If an AMD GPU is attached via Thunderbolt, we program different > ASPM time values because ? (nbio_v2_3_enable_aspm()). That wasn't introduced by me, so not sure what the rationale is. Let me know if I can help clarify things further so that we can find a solution that you feel more comfortable with. Thanks, Lukas
