Re: [PATCH v3 1/7] drm/nouveau: Switch from pci_is_thunderbolt_attached() to dev_is_removable()

[Ilpo Järvinen]

On Tue, 14 Nov 2023, Mario Limonciello wrote:

> pci_is_thunderbolt_attached() looks at the hierarchy of the PCIe device
> to determine if any bridge along the way has the is_thunderbolt bit set.
> This bit will only be set when one of the devices in the hierarchy is an
> Intel Thunderbolt device.
> 
> However PCIe devices can be connected to USB4 hubs and routers which won't
> necessarily set the is_thunderbolt bit. These devices will however be
> marked as externally facing which means they are marked removable by
> pci_set_removable().
> 
> Look whether the device is marked removable to determine it's
> connected to a Thunderbolt controller or USB4 router.

(I hate having to say this) but I still feel the actual reason why you're 
doing this change is missing from the above and has to be read between the 
lines.

Perhaps something along these lines matches what's your intent here:

The external GPU check added in commit 1d3c11030103 ("drm/nouveau: Don't 
register Thunderbolt eGPU with vga_switcheroo") covers only Thunderbolt
devices but removable PCIe devices can also be connected to USB4 hubs and 
routers. Those devices won't necessarily set the is_thunderbolt bit that
pci_is_thunderbolt_attached() checks for.

USB4 hubs and routers are however marked as externally facing which means 
they are marked removable by pci_set_removable().

Extend the check for removable device beyond Thunderbolt devices by 
using dev_is_removable() that covers also USB4 routers.

?

-- 
 i.

> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> v2->v3:
>  * Update commit message
> ---
>  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);
>