Commit 0f90603c authored by Imre Deak's avatar Imre Deak

drm/i915: Fix hibernation with ACPI S0 target state

After

commit dd9f31c7
Author: Imre Deak <imre.deak@intel.com>
Date:   Wed Aug 16 17:46:07 2017 +0300

    drm/i915/gen9+: Set same power state before hibernation image
    save/restore

during hibernation/suspend the power domain functionality got disabled,
after which resume could leave it incorrectly disabled if the ACPI
target state was S0 during suspend and i915 was not loaded by the loader
kernel.

This was caused by not considering if we resumed from hibernation as the
condition for power domains reiniting.

Fix this by simply tracking if we suspended power domains during system
suspend and reinit power domains accordingly during resume. This will
result in reiniting power domains always when resuming from hibernation,
regardless of the platform and whether or not i915 is loaded by the
loader kernel.

The reason we didn't catch this earlier is that the enabled/disabled
state of power domains during PMSG_FREEZE/PMSG_QUIESCE is platform
and kernel config dependent: on my SKL the target state is S4
during PMSG_FREEZE and (with the driver loaded in the loader kernel)
S0 during PMSG_QUIESCE. On the reporter's machine it's S0 during
PMSG_FREEZE but (contrary to this) power domains are not initialized
during PMSG_QUIESCE since i915 is not loaded in the loader kernel, or
it's loaded but without the DMC firmware being available.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105196
Reported-and-tested-by: amn-bas@hotmail.com
Fixes: dd9f31c7 ("drm/i915/gen9+: Set same power state before hibernation image save/restore")
Cc: amn-bas@hotmail.com
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322143642.26883-1-imre.deak@intel.com
parent 46b3617d
...@@ -1607,15 +1607,12 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) ...@@ -1607,15 +1607,12 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
{ {
struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_private *dev_priv = to_i915(dev);
struct pci_dev *pdev = dev_priv->drm.pdev; struct pci_dev *pdev = dev_priv->drm.pdev;
bool fw_csr;
int ret; int ret;
disable_rpm_wakeref_asserts(dev_priv); disable_rpm_wakeref_asserts(dev_priv);
intel_display_set_init_power(dev_priv, false); intel_display_set_init_power(dev_priv, false);
fw_csr = !IS_GEN9_LP(dev_priv) && !hibernation &&
suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
/* /*
* In case of firmware assisted context save/restore don't manually * In case of firmware assisted context save/restore don't manually
* deinit the power domains. This also means the CSR/DMC firmware will * deinit the power domains. This also means the CSR/DMC firmware will
...@@ -1623,8 +1620,11 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) ...@@ -1623,8 +1620,11 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
* also enable deeper system power states that would be blocked if the * also enable deeper system power states that would be blocked if the
* firmware was inactive. * firmware was inactive.
*/ */
if (!fw_csr) if (IS_GEN9_LP(dev_priv) || hibernation || !suspend_to_idle(dev_priv) ||
dev_priv->csr.dmc_payload == NULL) {
intel_power_domains_suspend(dev_priv); intel_power_domains_suspend(dev_priv);
dev_priv->power_domains_suspended = true;
}
ret = 0; ret = 0;
if (IS_GEN9_LP(dev_priv)) if (IS_GEN9_LP(dev_priv))
...@@ -1636,8 +1636,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) ...@@ -1636,8 +1636,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
if (ret) { if (ret) {
DRM_ERROR("Suspend complete failed: %d\n", ret); DRM_ERROR("Suspend complete failed: %d\n", ret);
if (!fw_csr) if (dev_priv->power_domains_suspended) {
intel_power_domains_init_hw(dev_priv, true); intel_power_domains_init_hw(dev_priv, true);
dev_priv->power_domains_suspended = false;
}
goto out; goto out;
} }
...@@ -1658,8 +1660,6 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) ...@@ -1658,8 +1660,6 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
if (!(hibernation && INTEL_GEN(dev_priv) < 6)) if (!(hibernation && INTEL_GEN(dev_priv) < 6))
pci_set_power_state(pdev, PCI_D3hot); pci_set_power_state(pdev, PCI_D3hot);
dev_priv->suspended_to_idle = suspend_to_idle(dev_priv);
out: out:
enable_rpm_wakeref_asserts(dev_priv); enable_rpm_wakeref_asserts(dev_priv);
...@@ -1826,8 +1826,7 @@ static int i915_drm_resume_early(struct drm_device *dev) ...@@ -1826,8 +1826,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_resume_early(dev_priv); intel_uncore_resume_early(dev_priv);
if (IS_GEN9_LP(dev_priv)) { if (IS_GEN9_LP(dev_priv)) {
if (!dev_priv->suspended_to_idle) gen9_sanitize_dc_state(dev_priv);
gen9_sanitize_dc_state(dev_priv);
bxt_disable_dc9(dev_priv); bxt_disable_dc9(dev_priv);
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
hsw_disable_pc8(dev_priv); hsw_disable_pc8(dev_priv);
...@@ -1835,8 +1834,7 @@ static int i915_drm_resume_early(struct drm_device *dev) ...@@ -1835,8 +1834,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
intel_uncore_sanitize(dev_priv); intel_uncore_sanitize(dev_priv);
if (IS_GEN9_LP(dev_priv) || if (dev_priv->power_domains_suspended)
!(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload))
intel_power_domains_init_hw(dev_priv, true); intel_power_domains_init_hw(dev_priv, true);
else else
intel_display_set_init_power(dev_priv, true); intel_display_set_init_power(dev_priv, true);
...@@ -1846,7 +1844,7 @@ static int i915_drm_resume_early(struct drm_device *dev) ...@@ -1846,7 +1844,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
enable_rpm_wakeref_asserts(dev_priv); enable_rpm_wakeref_asserts(dev_priv);
out: out:
dev_priv->suspended_to_idle = false; dev_priv->power_domains_suspended = false;
return ret; return ret;
} }
......
...@@ -1851,7 +1851,7 @@ struct drm_i915_private { ...@@ -1851,7 +1851,7 @@ struct drm_i915_private {
u32 bxt_phy_grc; u32 bxt_phy_grc;
u32 suspend_count; u32 suspend_count;
bool suspended_to_idle; bool power_domains_suspended;
struct i915_suspend_saved_registers regfile; struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state; struct vlv_s0ix_state vlv_s0ix_state;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment