Re: [PATCH 32/32] PCI: Whitelist Thunderbolt ports for runtime D3

[Lukas Wunner <lukas@wunner.de>]

Hi Mika,

sorry for the delay!

On Thu, Jun 21, 2018 at 02:13:54PM +0300, Mika Westerberg wrote:
> On Sat, Jun 16, 2018 at 09:25:00PM +0200, Lukas Wunner wrote:
> > +		/* Even the oldest 2010 Thunderbolt controller supports D3. */
> > +		if (bridge->is_thunderbolt)
> > +			return true;
> 
> I have a small concern here. In PC world native PCIe hotplug used with
> Thunderbolt comes in two flavors:
> 
>   - Native PCI hotplug without runtime PM
>   - Native PCI hotplug with runtime PM
> 
> The former works so that even if it uses native PCIe hotplug, the power
> management is done so that the Thunderbolt host router is hot-removed
> when there is nothing connected (and that brings the power savings).
> With the above change we start putting all Thunderbolt PCIe hotplug
> ports to D3 runtime. While this probably works (and I tested it on the
> same Dell) I don't think Windows does it and it may lead to a path that
> has not been tested very thoroughly by OEMs.
> 
> So I wonder if it makes sense to restrict this particular check to Apple
> hardware at this point?

I'm not familiar at all with Windows, so this your call.  Normally I'd say,
if there's no known case which forces us to constrain this to Apple, we
probably shouldn't be overzealous.  OTOH I imagine you have to deal with
Windows and broken BIOSes on a daily basis and can anticipate if problems
are to be expected.

The "host router" is the NHI, right?  So IIUC all downstream ports and
the upstream port of the TB PCIe switch may go to D3hot, same for the
root port.  And your concern is that hotplug addition or removal of the
NHI might not work if those ports are in D3hot?

What I don't get is, if we constrain this to Apple, TB ports on non-Apple
systems may not runtime suspend (e.g. if the OEM botched the BIOS date
to be < 2015) and then the "Native PCI hotplug with runtime PM" case
will be broken, won't it?

Thanks,

Lukas