Skip to content

Commit 300efa9

Browse files
ideakjlahtine-intel
authored andcommitted
drm/i915: Fix hibernation with ACPI S0 target state
After commit dd9f31c 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: dd9f31c ("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: Imre Deak <imre.deak@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180322143642.26883-1-imre.deak@intel.com (cherry picked from commit 0f90603) Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
1 parent 76cb9d3 commit 300efa9

File tree

2 files changed

+11
-13
lines changed

2 files changed

+11
-13
lines changed

drivers/gpu/drm/i915/i915_drv.c

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,24 +1611,24 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
16111611
{
16121612
struct drm_i915_private *dev_priv = to_i915(dev);
16131613
struct pci_dev *pdev = dev_priv->drm.pdev;
1614-
bool fw_csr;
16151614
int ret;
16161615

16171616
disable_rpm_wakeref_asserts(dev_priv);
16181617

16191618
intel_display_set_init_power(dev_priv, false);
16201619

1621-
fw_csr = !IS_GEN9_LP(dev_priv) && !hibernation &&
1622-
suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
16231620
/*
16241621
* In case of firmware assisted context save/restore don't manually
16251622
* deinit the power domains. This also means the CSR/DMC firmware will
16261623
* stay active, it will power down any HW resources as required and
16271624
* also enable deeper system power states that would be blocked if the
16281625
* firmware was inactive.
16291626
*/
1630-
if (!fw_csr)
1627+
if (IS_GEN9_LP(dev_priv) || hibernation || !suspend_to_idle(dev_priv) ||
1628+
dev_priv->csr.dmc_payload == NULL) {
16311629
intel_power_domains_suspend(dev_priv);
1630+
dev_priv->power_domains_suspended = true;
1631+
}
16321632

16331633
ret = 0;
16341634
if (IS_GEN9_LP(dev_priv))
@@ -1640,8 +1640,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
16401640

16411641
if (ret) {
16421642
DRM_ERROR("Suspend complete failed: %d\n", ret);
1643-
if (!fw_csr)
1643+
if (dev_priv->power_domains_suspended) {
16441644
intel_power_domains_init_hw(dev_priv, true);
1645+
dev_priv->power_domains_suspended = false;
1646+
}
16451647

16461648
goto out;
16471649
}
@@ -1662,8 +1664,6 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
16621664
if (!(hibernation && INTEL_GEN(dev_priv) < 6))
16631665
pci_set_power_state(pdev, PCI_D3hot);
16641666

1665-
dev_priv->suspended_to_idle = suspend_to_idle(dev_priv);
1666-
16671667
out:
16681668
enable_rpm_wakeref_asserts(dev_priv);
16691669

@@ -1830,17 +1830,15 @@ static int i915_drm_resume_early(struct drm_device *dev)
18301830
intel_uncore_resume_early(dev_priv);
18311831

18321832
if (IS_GEN9_LP(dev_priv)) {
1833-
if (!dev_priv->suspended_to_idle)
1834-
gen9_sanitize_dc_state(dev_priv);
1833+
gen9_sanitize_dc_state(dev_priv);
18351834
bxt_disable_dc9(dev_priv);
18361835
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
18371836
hsw_disable_pc8(dev_priv);
18381837
}
18391838

18401839
intel_uncore_sanitize(dev_priv);
18411840

1842-
if (IS_GEN9_LP(dev_priv) ||
1843-
!(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload))
1841+
if (dev_priv->power_domains_suspended)
18441842
intel_power_domains_init_hw(dev_priv, true);
18451843
else
18461844
intel_display_set_init_power(dev_priv, true);
@@ -1850,7 +1848,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
18501848
enable_rpm_wakeref_asserts(dev_priv);
18511849

18521850
out:
1853-
dev_priv->suspended_to_idle = false;
1851+
dev_priv->power_domains_suspended = false;
18541852

18551853
return ret;
18561854
}

drivers/gpu/drm/i915/i915_drv.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2119,7 +2119,7 @@ struct drm_i915_private {
21192119
u32 bxt_phy_grc;
21202120

21212121
u32 suspend_count;
2122-
bool suspended_to_idle;
2122+
bool power_domains_suspended;
21232123
struct i915_suspend_saved_registers regfile;
21242124
struct vlv_s0ix_state vlv_s0ix_state;
21252125

0 commit comments

Comments
 (0)