Re: [PATCH] PCI: Only put >= 2015 root ports into D3 on Intel

[Limonciello, Mario]

> > AFAICT the actual issue is entirely a wakeup platform firmware sequencing
> > issue
> > while in a hardware sleep state and not PMEs.
> >
> > It's only exposed by putting the root ports into D3 over s2idle.
> But there are two ways to enter s2idle (well or the S0ix whatever is the
> AMD term for that). Either through system sleep or simply waiting that
> all the needed devices runtime suspend. There should be no difference
> from device perspective AFAICT.
On AMD all devices in runtime suspend and SoC entering system
suspend aren't the same state.
> > As an experiment on an unpatched kernel if I avoid letting amd-pmc bind then
> > the
> > hardware will never enter a hardware sleep state over Linux s2idle and this
> > issue
> > doesn't occur.
> >
> > That shows that PMEs *do* work from D3cold.
> >
> > With all of this I have to wonder if the Windows behavior of what to do with
> > the root
> > ports is tied to the uPEP requirements specified in the firmware.
> >
> > Linux doesn't do any enforcement or adjustments from what uPEP indicates.
> >
> > The uPEP constraints for the root port in question in an affected AMD system
> > has:
> >
> >                      Package (0x04)
> >                      {
> >                          Zero,
> >                          "\\_SB.PCI0.GP19",
> >                          Zero,
> >                          Zero
> >                      },
> >
> > AMD's parsing is through 'lpi_device_get_constraints_amd' so that structure
> > shows
> > as not enabled and doesn't specify any D-state requirements.
> AFAIK this object does not exist in ChromeOS so Linux cannot use it
> there.
OK that means that if we came up with a solution that utilized
uPEP that it would have to remain optional.
> > What do they specify for Intel on a matching root port?
> I think the corresponding entry in ADL-P system for TBT PCIe root port 0
> looks like this:
>
> 	Package (0x03)
> 	{
> 	    "\\_SB.PC00.TRP0",
> 	    Zero,
> 	    Package (0x02)
> 	    {
> 		Zero,
> 		Package (0x02)
> 		{
> 		    0xFF,
> 		    0x03
> 		}
> 	    }
> 	},
>
> I'm not entirely sure what does it tell? ;-)

It's parsed using `lpi_device_get_constraints`.

So should I follow it right this means for ACPI device
\\_SB.PC00.TRP0 the constraint is disabled.

It's described as
Version 0, UID 0xFF has a minimum D-state of 3.

That means my idea to try to only change D-states at
suspend for enabled constraints won't help.