Re: [PATCH] PCI/PM: Assume ports without DLL Link Active train links in 100 ms

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Thu, Aug 20, 2020 at 11:36:37AM -0400, Lyude Paul wrote:
> On Thu, 2020-08-20 at 10:13 +0200, Lukas Wunner wrote:
> > On Wed, Aug 19, 2020 at 04:06:25PM +0300, Mika Westerberg wrote:
> > > Sec 7.5.3.6 requires such Ports to support DLL Link Active reporting, but
> > > at least the Intel JHL6240 Thunderbolt 3 Bridge [8086:15c0] and the Intel
> > > JHL7540 Thunderbolt 3 Bridge [8086:15ea] do not.
> > [...]
> > > +	 * Also do the same for devices that have power management disabled
> > > +	 * by their driver and are completely power managed through the
> > > +	 * root port power resource instead. This is a special case for
> > > +	 * nouveau.
> > >  	 */
> > > -	if (!pci_is_pcie(dev)) {
> > > +	if (!pci_is_pcie(dev) || !child->pm_cap) {
> > 
> > It sounds like the above-mentioned Thunderbolt controllers are broken,
> > not the Nvidia cards, so to me (as an outside observer) it would seem
> > more logical that a quirk for the former is needed.  The code comment
> > suggests that nouveau somehow has a problem, but that doesn't seem to
> > be the case (IIUC).  Also, it's a little ugly to have references to
> > specific drivers in PCI core code.
> > 
> > Maybe this can be fixed with quirks for the Thunderbolt controllers
> > which set a flag, and that flag causes the 1000 msec wait to be skipped?
>
> Sorry, some stuff came up yesterday so I didn't get the time to go through my
> laptops and test them. I do agree with this though - I'd be worried as well that
> nouveau might not be the only driver out there that needs this kind of delay

I actually expect that nouveau is the only one because it is doing some
PM tricks to get the runtime PM working, which is that it leaves the GPU
device in D0 and puts the parent root port into D3cold. The BIOS ASL
code has some assumptions there and I think this 1000 ms delay just
works that around by luck ;-)

IIRC Bjorn suggested quirking the affected downstream ports when I
originally sent the patch but I thought we could make this solution more
generic. Which of course, did not work too well.

I can look into the quirk solution instead if this is what people
prefer.