Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 17 May 2023 15:58:11 +0300
From:   Mika Westerberg <mika.westerberg@linux.intel.com>
To:     "Limonciello, Mario" <mario.limonciello@amd.com>
Cc:     "Limonciello, Mario" <mlimonci@amd.com>,
        Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        S-k Shyam-sundar <Shyam-sundar.S-k@amd.com>,
        Natikar Basavaraj <Basavaraj.Natikar@amd.com>,
        Deucher Alexander <Alexander.Deucher@amd.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Iain Lane <iain@orangesquash.org.uk>
Subject: Re: [PATCH] PCI: Only put >= 2015 root ports into D3 on Intel
Message-ID: <20230517125811.GG45886@black.fi.intel.com>
References: <20230515231515.1440-1-mario.limonciello@amd.com>
 <20230516055918.GS66750@black.fi.intel.com>
 <f0417d28-0c1a-f46e-9cbd-f20f2ac1e9f9@amd.com>
 <20230517071527.GU66750@black.fi.intel.com>
 <8e7e23dc-f01b-6f78-f383-7706795e386e@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8e7e23dc-f01b-6f78-f383-7706795e386e@amd.com>
Precedence: bulk

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.

:(