[Nouveau] [PATCH v2 3/9] PCI: drop `is_thunderbolt` attribute

[Limonciello, Mario]

[Public]

> -----Original Message-----
> From: Mika Westerberg <mika.westerberg at linux.intel.com>
> Sent: Friday, February 11, 2022 04:24
> To: Limonciello, Mario <Mario.Limonciello at amd.com>
> Cc: Bjorn Helgaas <bhelgaas at google.com>; Andreas Noever
> <andreas.noever at gmail.com>; open list:PCI SUBSYSTEM <linux-
> pci at vger.kernel.org>; open list:THUNDERBOLT DRIVER <linux-
> usb at vger.kernel.org>; open list:RADEON and AMDGPU DRM DRIVERS <amd-
> gfx at lists.freedesktop.org>; open list:DRM DRIVERS <dri-
> devel at lists.freedesktop.org>; open list:DRM DRIVER FOR NVIDIA
> GEFORCE/QUADRO GPUS <nouveau at lists.freedesktop.org>; open list:X86
> PLATFORM DRIVERS <platform-driver-x86 at vger.kernel.org>; Michael Jamet
> <michael.jamet at intel.com>; Yehezkel Bernat <YehezkelShB at gmail.com>;
> Lukas Wunner <lukas at wunner.de>; Deucher, Alexander
> <Alexander.Deucher at amd.com>
> Subject: Re: [PATCH v2 3/9] PCI: drop `is_thunderbolt` attribute
> 
> Hi Mario,
> 
> On Thu, Feb 10, 2022 at 04:43:23PM -0600, Mario Limonciello wrote:
> > The `is_thunderbolt` attribute is currently a dumping ground for a
> > variety of things.
> >
> > Instead use the driver core removable attribute to indicate the
> > detail a device is attached to a thunderbolt or USB4 chain.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> > ---
> >  drivers/pci/pci.c                 |  2 +-
> >  drivers/pci/probe.c               | 20 +++++++-------------
> >  drivers/platform/x86/apple-gmux.c |  2 +-
> >  include/linux/pci.h               |  5 ++---
> >  4 files changed, 11 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 9ecce435fb3f..1264984d5e6d 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -2955,7 +2955,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 (dev_is_removable(&bridge->dev))
> 
> For this, I'm not entirely sure this is what we want. The purpose of
> this check is to enable port power management of Apple systems with
> Intel Thunderbolt controller and therefore checking for "removable" here
> is kind of misleading IMHO.
> 
> I wonder if we could instead remove the check completely here and rely
> on the below:
> 
> 	if (platform_pci_bridge_d3(bridge))
> 		return true;
> 
> and that would then look like:
> 
> static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
> {
> 	if (pci_use_mid_pm())
> 		return false;
> 
> 	if (acpi_pci_bridge_d3(dev))
> 		return true;
> 
> 	if (device_property_read_bool(&dev->dev, "HotPlugSupportInD3"))
> 		return true;
> 
> 	return false;
> }
> 
> and then make a quirk in quirks.c that adds the software node property
> for the Apple systems? Or something along those lines.
> 
> @Lukas, what do you think?

I took a stab at doing this for V3, but I'm unsure whether ALL of the TBT controllers
in pci_ids.h have been used in Apple laptops, so it might be a bit wasteful of a quirk
list.  If there is a known list somewhere that is shorter than that, it may be possible
to pare down.  Lukas, if you can please look closely at patch 3 of v3.