PCI / PM: Do not clear state_saved in pci_pm_freeze() when smart suspend is set
PATCH 1/1 PCI / PM: Do not clear state_saved in pci_pm_freeze() when smart suspend is set
If a driver uses DPM_FLAG_SMART_SUSPEND and the device is already runtime suspended when hibernate is started PCI core skips runtime resuming the device but still clears pci_dev->state_saved. After the hibernation image is written pci_pm_thaw_noirq() makes sure subsequent thaw phases for the device are also skipped leaving it runtime suspended with pci_dev->state_saved == false.
When the device is eventually runtime resumed pci_pm_runtime_resume() restores config space by calling pci_restore_standard_config(), however because pci_dev->state_saved == false pci_restore_state() never actually restores the config space leaving the device in a state that is not what the driver might expect.
For example here is what happens for intel-lpss I2C devices once the hibernation snapshot is taken:
intel-lpss 0000:00:15.0: power state changed by ACPI to D0 intel-lpss 0000:00:1e.0: power state changed by ACPI to D3cold video LNXVIDEO:00: Restoring backlight state
PM: hibernation exit
i2c_designware i2c_designware.1: Unknown Synopsys component type: 0xffffffff i2c_designware i2c_designware.0: Unknown Synopsys component type: 0xffffffff i2c_designware i2c_designware.1: timeout in disabling adapter i2c_designware i2c_designware.0: timeout in disabling adapter
Since PCI config space is not restored the device is still in D3hot making MMIO register reads return 0xffffffff.
Fix this by clearing pci_dev->state_saved only if we actually end up runtime resuming the device.
--- drivers/pci/pci-driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/pci-driver.c +3 -2
@@ -958,10 +958,11 @@ static int pci_pm_freeze(struct device *dev)* devices should not be touched during freeze/thaw transitions, * however. */ -if (!dev_pm_test_driver_flags (dev,DPM_FLAG_SMART_SUSPEND )) +if (!dev_pm_smart_suspend_and_suspended (dev)) {pm_runtime_resume (dev); + pci_dev->state_saved = false; + } - pci_dev->state_saved = false;if (pm->freeze) {int error;
Signature · 1 line
2.17.0
Quoted 25 lines · CEST Mika Westerberg
If a driver uses
DPM_FLAG_SMART_SUSPENDand the device is already runtime suspended when hibernate is started PCI core skips runtime resuming the device but still clearspci_dev->state_saved. After the hibernation image is writtenpci_pm_thaw_noirq()makes sure subsequent thaw phases for the device are also skipped leaving it runtime suspended withpci_dev->state_saved== false.When the device is eventually runtime resumed
pci_pm_runtime_resume()restores config space by callingpci_restore_standard_config(), however becausepci_dev->state_saved== falsepci_restore_state()never actually restores the config space leaving the device in a state that is not what the driver might expect.For example here is what happens for intel-lpss I2C devices once the hibernation snapshot is taken:
intel-lpss 0000:00:15.0: power state changed by ACPI to D0 intel-lpss 0000:00:1e.0: power state changed by ACPI to D3cold video LNXVIDEO:00: Restoring backlight state
PM: hibernation exit
i2c_designwarei2c_designware.1: Unknown Synopsys component type:0xffffffffi2c_designwarei2c_designware.0: Unknown Synopsys component type:0xffffffffi2c_designwarei2c_designware.1: timeout in disabling adapteri2c_designwarei2c_designware.0: timeout in disabling adapter
Quoted 8 lines
Since PCI config space is not restored the device is still in D3hot making MMIO register reads return
0xffffffff.Fix this by clearing
pci_dev->state_savedonly if we actually end up runtime resuming the device.Fixes c4b65157aeef ("PCI / PM: Take SMART_SUSPEND driver flag into account")Signed-off-by Mika Westerberg <[email protected]>
Since this fixes a commit that went in through the PM tree, I've queued it up.
Bjorn, please let me know if you prefer to route it through the PCI tree.
Quoted 23 lines
--- drivers/pci/pci-driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/pci/pci-driver.c +3
-2@@ -958,10 +958,11 @@ static int pci_pm_freeze(struct device *dev)* devices should not be touched during freeze/thaw transitions, * however. */ -if (!dev_pm_test_driver_flags (dev,DPM_FLAG_SMART_SUSPEND )) +if (!dev_pm_smart_suspend_and_suspended (dev)) {pm_runtime_resume (dev); + pci_dev->state_saved = false; + } - pci_dev->state_saved = false;if (pm->freeze) {int error;
Quoted 32 lines · Rafael J. Wysocki
Quoted 31 lines · CEST Mika Westerberg
If a driver uses
DPM_FLAG_SMART_SUSPENDand the device is already runtime suspended when hibernate is started PCI core skips runtime resuming the device but still clearspci_dev->state_saved. After the hibernation image is writtenpci_pm_thaw_noirq()makes sure subsequent thaw phases for the device are also skipped leaving it runtime suspended withpci_dev->state_saved== false.When the device is eventually runtime resumed
pci_pm_runtime_resume()restores config space by callingpci_restore_standard_config(), however becausepci_dev->state_saved== falsepci_restore_state()never actually restores the config space leaving the device in a state that is not what the driver might expect.For example here is what happens for intel-lpss I2C devices once the hibernation snapshot is taken:
intel-lpss 0000:00:15.0: power state changed by ACPI to D0 intel-lpss 0000:00:1e.0: power state changed by ACPI to D3cold video LNXVIDEO:00: Restoring backlight state
PM: hibernation exit
i2c_designwarei2c_designware.1: Unknown Synopsys component type:0xffffffffi2c_designwarei2c_designware.0: Unknown Synopsys component type:0xffffffffi2c_designwarei2c_designware.1: timeout in disabling adapteri2c_designwarei2c_designware.0: timeout in disabling adapterSince PCI config space is not restored the device is still in D3hot making MMIO register reads return
0xffffffff.Fix this by clearing
pci_dev->state_savedonly if we actually end up runtime resuming the device.
Quoted 6 lines
Quoted 2 lines
Fixes c4b65157aeef ("PCI / PM: Take SMART_SUSPEND driver flag into account")Signed-off-by Mika Westerberg <[email protected]>Since this fixes a commit that went in through the PM tree, I've queued it up.
Bjorn, please let me know if you prefer to route it through the PCI tree.
Thanks for taking it, that's perfect!
Quoted 25 lines
Quoted 23 lines
--- drivers/pci/pci-driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/pci/pci-driver.c +3
-2@@ -958,10 +958,11 @@ static int pci_pm_freeze(struct device *dev)* devices should not be touched during freeze/thaw transitions, * however. */ -if (!dev_pm_test_driver_flags (dev,DPM_FLAG_SMART_SUSPEND )) +if (!dev_pm_smart_suspend_and_suspended (dev)) {pm_runtime_resume (dev); + pci_dev->state_saved = false; + } - pci_dev->state_saved = false;if (pm->freeze) {int error;