Downstream drivers are getting the wrong values from
pcie_bandwidth_available() which is causing problems for performance
of eGPUs.
This series overhauls Thunderbolt related device detection and uses
the changes to change the behavior of pcie_bandwidth_available().
NOTE: This series is currently based on top of v6.6 + this change that
will be merged for 6.7:
Link: https://patchwork.freedesktop.org/patch/564738/
v1->v2:
* Rename is_thunderbolt
* Look for _DSD instead of link
* Drop pci_is_thunderbolt_attached() from all drivers
* Adjust links
* Adjust commit messages
* Add quirk for Tiger Lake
Mario Limonciello (9):
drm/nouveau: Switch from pci_is_thunderbolt_attached() to
dev_is_removable()
drm/radeon: Switch from pci_is_thunderbolt_attached() to
dev_is_removable()
PCI: Drop pci_is_thunderbolt_attached()
PCI: Move the `PCI_CLASS_SERIAL_USB_USB4` definition to common header
PCI: pciehp: Move check for is_thunderbolt into a quirk
PCI: Rename is_thunderbolt to is_tunneled
PCI: ACPI: Detect PCIe root ports that are used for tunneling
PCI: Exclude PCIe ports used for tunneling in
pcie_bandwidth_available()
PCI: Add a quirk to mark 0x8086 : 0x9a23 as supporting PCIe tunneling
drivers/gpu/drm/nouveau/nouveau_vga.c | 6 +-
drivers/gpu/drm/radeon/radeon_device.c | 4 +-
drivers/gpu/drm/radeon/radeon_kms.c | 2 +-
drivers/pci/hotplug/pciehp_hpc.c | 6 +-
drivers/pci/pci-acpi.c | 16 ++++++
drivers/pci/pci.c | 76 +++++++++++++++++---------
drivers/pci/probe.c | 2 +-
drivers/pci/quirks.c | 31 +++++++++++
drivers/platform/x86/apple-gmux.c | 2 +-
drivers/thunderbolt/nhi.h | 2 -
include/linux/pci.h | 25 +--------
include/linux/pci_ids.h | 1 +
12 files changed, 109 insertions(+), 64 deletions(-)
--
2.34.1
pci_is_thunderbolt_attached() only works for Intel TBT devices. Switch to
using dev_is_removable() to be able to detect USB4 devices as well.
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/gpu/drm/nouveau/nouveau_vga.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index f8bf0ec26844..14215b7ca187 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -94,8 +94,8 @@ nouveau_vga_init(struct nouveau_drm *drm)
vga_client_register(pdev, nouveau_vga_set_decode);
- /* don't register Thunderbolt eGPU with vga_switcheroo */
- if (pci_is_thunderbolt_attached(pdev))
+ /* don't register USB4/Thunderbolt eGPU with vga_switcheroo */
+ if (dev_is_removable(&pdev->dev))
return;
vga_switcheroo_register_client(pdev, &nouveau_switcheroo_ops, runtime);
@@ -118,7 +118,7 @@ nouveau_vga_fini(struct nouveau_drm *drm)
vga_client_unregister(pdev);
- if (pci_is_thunderbolt_attached(pdev))
+ if (dev_is_removable(&pdev->dev))
return;
vga_switcheroo_unregister_client(pdev);
--
2.34.1
`PCI_CLASS_SERIAL_USB_USB4` may be used by code outside of thunderbolt.
Move the declaration into the common pci_ids.h header.
Acked-by: Mika Westerberberg <[email protected]>
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/thunderbolt/nhi.h | 2 --
include/linux/pci_ids.h | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index 0f029ce75882..675ddefe283c 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -91,6 +91,4 @@ extern const struct tb_nhi_ops icl_nhi_ops;
#define PCI_DEVICE_ID_INTEL_RPL_NHI0 0xa73e
#define PCI_DEVICE_ID_INTEL_RPL_NHI1 0xa76d
-#define PCI_CLASS_SERIAL_USB_USB4 0x0c0340
-
#endif
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 91b457de262e..1fc8b5e72f80 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -121,6 +121,7 @@
#define PCI_CLASS_SERIAL_USB_OHCI 0x0c0310
#define PCI_CLASS_SERIAL_USB_EHCI 0x0c0320
#define PCI_CLASS_SERIAL_USB_XHCI 0x0c0330
+#define PCI_CLASS_SERIAL_USB_USB4 0x0c0340
#define PCI_CLASS_SERIAL_USB_DEVICE 0x0c03fe
#define PCI_CLASS_SERIAL_FIBER 0x0c04
#define PCI_CLASS_SERIAL_SMBUS 0x0c05
--
2.34.1
The `is_thunderbolt` bit has been used to indicate that a PCIe device
contained the Intel VSEC which is used by various parts of the kernel
to change behavior. To later allow usage with USB4 controllers as well,
rename this to `is_tunneled`.
Signed-off-by: Mario Limonciello <[email protected]>
---
drivers/pci/pci.c | 2 +-
drivers/pci/probe.c | 2 +-
drivers/platform/x86/apple-gmux.c | 2 +-
include/linux/pci.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59c01d68c6d5..d9aa5a39f585 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3032,7 +3032,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
return true;
/* Even the oldest 2010 Thunderbolt controller supports D3. */
- if (bridge->is_thunderbolt)
+ if (bridge->is_tunneled)
return true;
/* Platform might know better if the bridge supports D3 */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 795534589b98..518413d15402 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1597,7 +1597,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
/* Is the device part of a Thunderbolt controller? */
vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_TBT);
if (vsec)
- dev->is_thunderbolt = 1;
+ dev->is_tunneled = 1;
}
static void set_pcie_untrusted(struct pci_dev *dev)
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 1417e230edbd..20315aa4463a 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -774,7 +774,7 @@ static int gmux_resume(struct device *dev)
static int is_thunderbolt(struct device *dev, void *data)
{
- return to_pci_dev(dev)->is_thunderbolt;
+ return to_pci_dev(dev)->is_tunneled;
}
static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 439c2dac8a3e..b1724f25fb02 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -440,7 +440,7 @@ struct pci_dev {
unsigned int is_virtfn:1;
unsigned int is_hotplug_bridge:1;
unsigned int shpc_managed:1; /* SHPC owned by shpchp */
- unsigned int is_thunderbolt:1; /* Thunderbolt controller */
+ unsigned int is_tunneled:1; /* Tunneled TBT or USB4 link */
unsigned int no_command_complete:1; /* No command completion */
/*
* Devices marked being untrusted are the ones that can potentially
--
2.34.1
On Fri, Nov 03, 2023 at 02:07:49PM -0500, Mario Limonciello wrote:
> Downstream drivers are getting the wrong values from
> pcie_bandwidth_available() which is causing problems for performance
> of eGPUs.
>
> This series overhauls Thunderbolt related device detection and uses
> the changes to change the behavior of pcie_bandwidth_available().
>
> NOTE: This series is currently based on top of v6.6 + this change that
> will be merged for 6.7:
> Link: https://patchwork.freedesktop.org/patch/564738/
Thanks, Mario, I'll look at this soon after v6.7-rc1 (probably Nov
12), so the amdgpu patch should be in mainline by then.
> v1->v2:
> * Rename is_thunderbolt
> * Look for _DSD instead of link
> * Drop pci_is_thunderbolt_attached() from all drivers
> * Adjust links
> * Adjust commit messages
> * Add quirk for Tiger Lake
>
> Mario Limonciello (9):
> drm/nouveau: Switch from pci_is_thunderbolt_attached() to
> dev_is_removable()
> drm/radeon: Switch from pci_is_thunderbolt_attached() to
> dev_is_removable()
> PCI: Drop pci_is_thunderbolt_attached()
> PCI: Move the `PCI_CLASS_SERIAL_USB_USB4` definition to common header
> PCI: pciehp: Move check for is_thunderbolt into a quirk
> PCI: Rename is_thunderbolt to is_tunneled
> PCI: ACPI: Detect PCIe root ports that are used for tunneling
> PCI: Exclude PCIe ports used for tunneling in
> pcie_bandwidth_available()
> PCI: Add a quirk to mark 0x8086 : 0x9a23 as supporting PCIe tunneling
>
> drivers/gpu/drm/nouveau/nouveau_vga.c | 6 +-
> drivers/gpu/drm/radeon/radeon_device.c | 4 +-
> drivers/gpu/drm/radeon/radeon_kms.c | 2 +-
> drivers/pci/hotplug/pciehp_hpc.c | 6 +-
> drivers/pci/pci-acpi.c | 16 ++++++
> drivers/pci/pci.c | 76 +++++++++++++++++---------
> drivers/pci/probe.c | 2 +-
> drivers/pci/quirks.c | 31 +++++++++++
> drivers/platform/x86/apple-gmux.c | 2 +-
> drivers/thunderbolt/nhi.h | 2 -
> include/linux/pci.h | 25 +--------
> include/linux/pci_ids.h | 1 +
> 12 files changed, 109 insertions(+), 64 deletions(-)
>
> --
> 2.34.1
>
Hi,
On 11/3/23 20:07, Mario Limonciello wrote:
> The `is_thunderbolt` bit has been used to indicate that a PCIe device
> contained the Intel VSEC which is used by various parts of the kernel
> to change behavior. To later allow usage with USB4 controllers as well,
> rename this to `is_tunneled`.
>
> Signed-off-by: Mario Limonciello <[email protected]>
Here is my ack for the trivial drivers/platform/x86/apple-gmux.c change:
Acked-by: Hans de Goede <[email protected]>
Bjorn, feel free to route this through the PCI tree.
Regards,
Hans
> ---
> drivers/pci/pci.c | 2 +-
> drivers/pci/probe.c | 2 +-
> drivers/platform/x86/apple-gmux.c | 2 +-
> include/linux/pci.h | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 59c01d68c6d5..d9aa5a39f585 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3032,7 +3032,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> return true;
>
> /* Even the oldest 2010 Thunderbolt controller supports D3. */
> - if (bridge->is_thunderbolt)
> + if (bridge->is_tunneled)
> return true;
>
> /* Platform might know better if the bridge supports D3 */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 795534589b98..518413d15402 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1597,7 +1597,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
> /* Is the device part of a Thunderbolt controller? */
> vsec = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_TBT);
> if (vsec)
> - dev->is_thunderbolt = 1;
> + dev->is_tunneled = 1;
> }
>
> static void set_pcie_untrusted(struct pci_dev *dev)
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 1417e230edbd..20315aa4463a 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -774,7 +774,7 @@ static int gmux_resume(struct device *dev)
>
> static int is_thunderbolt(struct device *dev, void *data)
> {
> - return to_pci_dev(dev)->is_thunderbolt;
> + return to_pci_dev(dev)->is_tunneled;
> }
>
> static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 439c2dac8a3e..b1724f25fb02 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -440,7 +440,7 @@ struct pci_dev {
> unsigned int is_virtfn:1;
> unsigned int is_hotplug_bridge:1;
> unsigned int shpc_managed:1; /* SHPC owned by shpchp */
> - unsigned int is_thunderbolt:1; /* Thunderbolt controller */
> + unsigned int is_tunneled:1; /* Tunneled TBT or USB4 link */
> unsigned int no_command_complete:1; /* No command completion */
> /*
> * Devices marked being untrusted are the ones that can potentially
On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote:
> The `is_thunderbolt` bit has been used to indicate that a PCIe device
> contained the Intel VSEC which is used by various parts of the kernel
> to change behavior. To later allow usage with USB4 controllers as well,
> rename this to `is_tunneled`.
This doesn't seem to make sense. is_thunderbolt indicates that a device
is part of a Thunderbolt controller. See the code comment:
> - unsigned int is_thunderbolt:1; /* Thunderbolt controller */
A Thunderbolt controller is not necessarily tunneled. The PCIe switch,
NHI and XHCI of the Thunderbolt host controller are not tunneled at all.
Thanks,
Lukas
On Fri, 3 Nov 2023, Mario Limonciello wrote:
> pci_is_thunderbolt_attached() only works for Intel TBT devices. Switch to
> using dev_is_removable() to be able to detect USB4 devices as well.
Please extend this with more details. I had to lookup the TBT change to
be able to make any guess why you're doing this (and it's still a guess
at best).
--
i.
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> drivers/gpu/drm/nouveau/nouveau_vga.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
> index f8bf0ec26844..14215b7ca187 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_vga.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
> @@ -94,8 +94,8 @@ nouveau_vga_init(struct nouveau_drm *drm)
>
> vga_client_register(pdev, nouveau_vga_set_decode);
>
> - /* don't register Thunderbolt eGPU with vga_switcheroo */
> - if (pci_is_thunderbolt_attached(pdev))
> + /* don't register USB4/Thunderbolt eGPU with vga_switcheroo */
> + if (dev_is_removable(&pdev->dev))
> return;
>
> vga_switcheroo_register_client(pdev, &nouveau_switcheroo_ops, runtime);
> @@ -118,7 +118,7 @@ nouveau_vga_fini(struct nouveau_drm *drm)
>
> vga_client_unregister(pdev);
>
> - if (pci_is_thunderbolt_attached(pdev))
> + if (dev_is_removable(&pdev->dev))
> return;
>
> vga_switcheroo_unregister_client(pdev);
>
On Mon, Nov 06, 2023 at 02:25:24PM +0200, Ilpo Järvinen wrote:
> On Fri, 3 Nov 2023, Mario Limonciello wrote:
>
> > pci_is_thunderbolt_attached() only works for Intel TBT devices. Switch to
> > using dev_is_removable() to be able to detect USB4 devices as well.
>
> Please extend this with more details. I had to lookup the TBT change to
> be able to make any guess why you're doing this (and it's still a guess
> at best).
Also please write it as Thunderbolt not TBT so that it is clear what we
are talking about. Ditto to all patches.
On 11/6/2023 10:47, Mika Westerberg wrote:
> On Mon, Nov 06, 2023 at 02:25:24PM +0200, Ilpo Järvinen wrote:
>> On Fri, 3 Nov 2023, Mario Limonciello wrote:
>>
>>> pci_is_thunderbolt_attached() only works for Intel TBT devices. Switch to
>>> using dev_is_removable() to be able to detect USB4 devices as well.
>>
>> Please extend this with more details. I had to lookup the TBT change to
>> be able to make any guess why you're doing this (and it's still a guess
>> at best).
>
> Also please write it as Thunderbolt not TBT so that it is clear what we
> are talking about. Ditto to all patches.
Ack, thanks I will add more detail to these first few patches and make
that s/TBT/Thunderbolt/ change when I rebase this on 6.7-rc1.
On 11/5/2023 11:39, Lukas Wunner wrote:
> On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote:
>> The `is_thunderbolt` bit has been used to indicate that a PCIe device
>> contained the Intel VSEC which is used by various parts of the kernel
>> to change behavior. To later allow usage with USB4 controllers as well,
>> rename this to `is_tunneled`.
>
> This doesn't seem to make sense. is_thunderbolt indicates that a device
> is part of a Thunderbolt controller. See the code comment:
>
>> - unsigned int is_thunderbolt:1; /* Thunderbolt controller */
>
> A Thunderbolt controller is not necessarily tunneled. The PCIe switch,
> NHI and XHCI of the Thunderbolt host controller are not tunneled at all.
>
> Thanks,
>
> Lukas
I could really use some clarification which PCIe devices actually
contain the Intel VSEC.
Is it in all 3 of those PCIe devices and not just the switch?
If so, I think I would rather introduce a separate bit. So after this
series we would have:
is_tunneled:1
is_thunderbolt:1
no_command_complete:1
* TBT1 devices would set no_command_complete
- The consumer would be pcie_init()
* All devices with the Intel VSEC would set is_thunderbolt and the two
consumers would be:
- apple-gmux.c
- pci_bridge_d3_possible()
* USB4 devices and PCIe switches with the VSEC would set is_tunneled.
On Mon, Nov 06, 2023 at 10:59:13AM -0600, Mario Limonciello wrote:
> On 11/5/2023 11:39, Lukas Wunner wrote:
> > On Fri, Nov 03, 2023 at 02:07:55PM -0500, Mario Limonciello wrote:
> > > The `is_thunderbolt` bit has been used to indicate that a PCIe device
> > > contained the Intel VSEC which is used by various parts of the kernel
> > > to change behavior. To later allow usage with USB4 controllers as well,
> > > rename this to `is_tunneled`.
> >
> > This doesn't seem to make sense. is_thunderbolt indicates that a device
> > is part of a Thunderbolt controller. See the code comment:
> >
> > > - unsigned int is_thunderbolt:1; /* Thunderbolt controller */
> >
> > A Thunderbolt controller is not necessarily tunneled. The PCIe switch,
> > NHI and XHCI of the Thunderbolt host controller are not tunneled at all.
>
> I could really use some clarification which PCIe devices actually contain
> the Intel VSEC.
>
> Is it in all 3 of those PCIe devices and not just the switch?
Yes, I've just double-checked Light Ridge, Cactus Ridge, Alpine Ridge.
Thanks,
Lukas