On Mon, Sep 23, 2019 at 5:10 PM Daniel Drake <drake@xxxxxxxxxxxx> wrote: > Unfortunately it's not quite as simple as quirking d3_delay though, > because the problem still happens upon resume from s2idle. In that > case, pci_dev_d3_sleep() is not called at all. > > if (state == PCI_D3hot || dev->current_state == PCI_D3hot) > pci_dev_d3_sleep(dev); > > In the runtime resume case, dev->current_state == PCI_D3hot here (even > though pci_set_power_state had been called to put it into D3cold), so > the msleep happens. > But in the system sleep (s2idle) case, dev->current_state == > PCI_D3cold here, so no sleep happens. > That is strangely inconsistent - is it a bug? In more detail: During runtime suspend: pci_set_power_state() is called with state=D3cold - This calls pci_raw_set_power_state() with state=D3hot -- After transitioning to D3hot, dev->current_state is updated to D3hot based on pmcsr readback acpi_device_set_power() is called with state=D3cold - acpi_power_transition() is called with state=D3cold, updates adev->power.state to D3cold - adev->power.state is updated (again) to D0 pci_update_current_state() is called - platform_pci_get_power_state() returns D3cold - dev->current_state is updated to D3cold Observations: everything seems to be in order During runtime resume: pci_update_current_state() is called - platform_pci_get_power_state() returns D3cold - dev->current_state is updated to D3cold pci_set_power_state() is called with state=D0 - Calls pci_platform_power_transition -- Calls acpi_pci_set_power_state -> acpi_device_set_power with state=D0 : --- acpi_power_transition() updates adev->power.state to D0 --- adev->power.state is updated (again) to D0 -- Calls pci_update_current_state --- platform_pci_get_power_state() returns D0 (so this is ignored) --- dev->current_state is updated to D3 via pmcsr read - D3cold delay happens (good) - Calls pci_raw_set_power_state() with state=D0 -- current_state is D3 so the D3 delay happens (good) (I quirked this delay to 20ms) -- device is transitioned to D0 and dev->current_state is updated to D0 from pmcsr read Observations: everything seems to be in order, USB is working after resume During s2idle suspend: Exactly the same as runtime suspend above. Device transitions into D3cold and dev->current_state ends up as D3cold. Everything seems to be in order. During s2idle resume: acpi_device_set_power() is called with state=D0 - acpi_power_transition() is called with state=D0, updates adev->power.state to D0 - adev->power.state is updated (again) to D0 pci_raw_set_power_state() is calld with state=D0 -- dev->current_state is D3cold so no D3 delay happens *** -- device fails to transitioned to D0, pmcsr read indicates it's still in D3. Observations: that's a pretty clear difference between the s2idle resume and runtime resume paths. The s2idle resume path is arrived at via pci_pm_default_resume_early() --> pci_power_up(). Should the s2idle resume path be modified to call into pci_update_current_state() to change the current_state to D3hot based on pmcsr (like the runtime resume path does)? Or should pci_raw_set_power_state() be modified to also apply the d3_delay when transitioning from D3cold to D0? Thanks Daniel
