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

[Limonciello, Mario]

On 5/17/2023 7:58 AM, Mika Westerberg wrote:
> On Wed, May 17, 2023 at 07:33:17AM -0500, Limonciello, Mario wrote:
> > > > 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.
> I should correct that the wakes may be configured differently, though.
>
> > On AMD all devices in runtime suspend and SoC entering system
> > suspend aren't the same state.
> Okay.
>
> > > > 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.
> Right.
>
> > > > 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.
> I see, so it needs to be in D3 for this "constraint" to be valid.
>
> > That means my idea to try to only change D-states at
> > suspend for enabled constraints won't help.
> :(

At least on an Alder Lake P system can you check
whether your root ports actually respond
affirmatively to acpi_pci_bridge_d3() or they need
to fall back to that logic?

In my case the problematic ones don't have _PRW or
_S0W, which might explain why Windows doesn't try
to use D3 (hot or cold) for them either.

If the root ports on your current systems do respond
through acpi_pci_bridge_d3() a possible solution here
might be an allow list for systems from 2015-2018 rather
than everything > 2015.