[trimming cc a bit to avoid spamming folks likely uninterested in PCI PM
intricacies]
On Wed, Feb 21, 2018 at 10:57:14AM +0100, Rafael J. Wysocki wrote:
> On Tue, Feb 20, 2018 at 10:29 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > On Sun, Feb 18, 2018 at 09:38:32AM +0100, Lukas Wunner wrote:
> > > --- a/drivers/pci/pci-driver.c
> > > +++ b/drivers/pci/pci-driver.c
> > > @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev)
> > >
> > > /*
> > > * If pci_dev->driver is not set (unbound), the device should
> > > - * always remain in D0 regardless of the runtime PM status
> > > + * always remain in D0 regardless of the runtime PM status.
> > > + * But if its parent can go to D3cold, this device may have
> > > + * been in D3cold as well and require restoration of its BARs.
> > > */
> > > - if (!pci_dev->driver)
> > > + if (!pci_dev->driver) {
> > > + pci_restore_bars(pci_dev);
> >
> > If we do decide not to do a full-blown restore, how do we decide
> > whether to use pci_restore_bars() or pci_restore_config_space()?
> >
> > I'm not sure why we have both.
>
> Me neither.
pci_restore_config_space() configures the BARs from saved_config_space[]
in struct pci_dev. This array needs to have been populated beforehand.
If it's never been populated, the function can't be used. The sole
caller of that function, pci_restore_state(), therefore checks whether
the state_saved bit in stuct pci_dev is true.
According to the code comment in the sole caller of pci_restore_bars(),
pci_raw_set_power_state(), there are systems where device are in D3hot
on boot. Moreover devices where the No_Soft_Reset bit is 0 are in
D0uninitialized when coming out of D3hot.
For those devices, pci_restore_bars() is called to configure the BARs
from resource[] in struct pci_dev, which was populated on bus scan.
pci_restore_config_space() can't be used here because the first time
the devices are brought into D0, saved_config_space[] hasn't been
populated yet. It's normally only populated when the device is suspended.
It might be possible to replace the invocation of pci_restore_bars()
with pci_restore_state() if pci_save_state() is called on bus scan for
devices in D3hot whose No_Soft_Reset bit is 0.
An alternative approach would be to avoid storing BARs in
saved_config_space[], and modify pci_restore_config_space() to restore
those from resource[]. That would save 24 bytes in struct pci_dev.
But it would only work if the BARs are always in sync with resource[],
I'm not sure if there are situations when this isn't the case.
Thanks,
Lukas
