Re: [PATCH 1/7] PCI: Restore BARs on runtime resume despite being unbound

[Lukas Wunner <lukas@wunner.de>]

On Wed, Feb 21, 2018 at 01:39:34PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, February 21, 2018 10:57:14 AM CET Rafael J. Wysocki wrote:
> > So if pci_pm_runtime_suspend() is modified to call pci_save_state()
> > before returning 0 in the !dev->driver case, we can just move the
> > pci_restore_standard_config() invocation in pci_pm_runtime_resume() up
> > to the very top and check dev->driver later.
> 
> I mean something like the patch below, overall (untested).
> 
> Tentatively-signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Okay I've tested this successfully now.  I'll have to respin the series
at least one more time to address the unnecessary initialization Bjorn
spotted in patch [5/7] and will then replace patch [1/7] with this one.

I'll wait a few more days before respinning to allow for further
comments, in particular I'm hoping for feedback from Takashi and
someone testing this on Optimus/ATPX.

Thanks!

Lukas

> ---
>  drivers/pci/pci-driver.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/pci/pci-driver.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-driver.c
> +++ linux-pm/drivers/pci/pci-driver.c
> @@ -1224,11 +1224,14 @@ static int pci_pm_runtime_suspend(struct
>  	int error;
>  
>  	/*
> -	 * If pci_dev->driver is not set (unbound), the device should
> -	 * always remain in D0 regardless of the runtime PM status
> +	 * If pci_dev->driver is not set (unbound), the device may go to D3cold,
> +	 * but only if the bridge above it is suspended.  In case that happens,
> +	 * save its config space.
>  	 */
> -	if (!pci_dev->driver)
> +	if (!pci_dev->driver) {
> +		pci_save_state(pci_dev);
>  		return 0;
> +	}
>  
>  	if (!pm || !pm->runtime_suspend)
>  		return -ENOSYS;
> @@ -1276,16 +1279,17 @@ static int pci_pm_runtime_resume(struct
>  	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>  
>  	/*
> -	 * If pci_dev->driver is not set (unbound), the device should
> -	 * always remain in D0 regardless of the runtime PM status
> +	 * Regardless of whether or not the driver is there, the device might
> +	 * have been put into D3cold as a result of suspending the bridge above
> +	 * it, so restore the config spaces of all devices here.
>  	 */
> +	pci_restore_standard_config(pci_dev);
>  	if (!pci_dev->driver)
>  		return 0;
>  
>  	if (!pm || !pm->runtime_resume)
>  		return -ENOSYS;
>  
> -	pci_restore_standard_config(pci_dev);
>  	pci_fixup_device(pci_fixup_resume_early, pci_dev);
>  	pci_enable_wake(pci_dev, PCI_D0, false);
>  	pci_fixup_device(pci_fixup_resume, pci_dev);
>