On Wed, Aug 05, 2020 at 03:30:00PM +0000, Derrick, Jonathan wrote: > On Wed, 2020-08-05 at 15:54 +0800, You-Sheng Yang wrote: > > On 2020-08-01 01:15, Jon Derrick wrote: > > > The pci_save_state call in vmd_suspend can be performed by > > > pci_pm_suspend_irq. This allows the call to pci_prepare_to_sleep into > > > ASPM flow. > > > > > > The pci_restore_state call in vmd_resume was restoring state after > > > pci_pm_resume->pci_restore_standard_config had already restored state. > > > It's also been suspected that the config state should be restored before > > > re-requesting IRQs. > > > > > > Remove the pci_{save,restore}_state calls in vmd_{suspend,resume} in > > > order to allow proper flow through PCI core power management ASPM code. > > > > I had a try on this patch but `lspci` still shows ASPM Disabled. > > Anything prerequisite missing here? > > > > Is enabling L0s/L1/etc on a device something that the driver should be > doing? No. ASPM should be completely managed by the PCI core. There are a few drivers that *do* muck with ASPM, but they are broken and they cause problems. Drivers can use pci_disable_link_state() to completely disable ASPM states, e.g., if they are known to be broken in hardware. But they should not update the Link Control register directly because there are specific requirements that involve both ends of the link, not just the endpoint. Bjorn