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

[Limonciello, Mario]

On 5/17/2023 8:48 PM, Limonciello, Mario wrote:
> 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.
Actually this is exactly it.  The function is missing a call

for platform_pci_power_manageable().

I'll send out a v2.