On Fri, Sep 15, 2017 at 06:02:33PM +0300, Ville Syrjälä wrote: > On Wed, Aug 16, 2017 at 05:46:07PM +0300, Imre Deak wrote: > > Atm, on GEN9 big core platforms before saving the hibernation image we > > uninitialize the display, disabling power wells manually, while before > > restoring the image we keep things powered (letting HW/DMC power down > > things as needed). The state mismatch will trigger the following error: > > > > DC state mismatch (0x0 -> 0x2) > > > > While the restore handler knows how to initialize the display from an > > unknown state (due to a different loader kernel or not having i915 > > loaded in the loader kernel) we should still use the same state for > > consistency before image saving and restoring. Do this by uniniting the > > display before restoring the image too. > > > > Bugzilla: https://bugs.freedesktop.org/attachment.cgi?id=133376 > > Reported-and-tested-by: Wang Wendy <wendy.wang@xxxxxxxxx> > > Reported-and-tested-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > Cc: Wang Wendy <wendy.wang@xxxxxxxxx> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > --- > > > > [ No Fixes: line, since I'm not aware of any other issues caused by this > > besides the error message. ] > > --- > > drivers/gpu/drm/i915/i915_drv.c | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 43100229613c..d816ba715cb6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1571,7 +1571,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) > > > > intel_display_set_init_power(dev_priv, false); > > > > - fw_csr = !IS_GEN9_LP(dev_priv) && > > + 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 > > @@ -2055,11 +2055,14 @@ static int i915_pm_resume(struct device *kdev) > > /* freeze: before creating the hibernation_image */ > > static int i915_pm_freeze(struct device *kdev) > > { > > + struct drm_device *dev = &kdev_to_i915(kdev)->drm; > > int ret; > > > > - ret = i915_pm_suspend(kdev); > > - if (ret) > > - return ret; > > + if (dev->switch_power_state != DRM_SWITCH_POWER_OFF) { > > + ret = i915_drm_suspend(dev); > > + if (ret) > > + return ret; > > + } > > Hmm. This hunk is here just for consistentency I take it? Yes, to make i915_pm_freeze() and i915_pm_freeze_late() similar. > > Patch lgtm > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > ret = i915_gem_freeze(kdev_to_i915(kdev)); > > if (ret) > > @@ -2070,11 +2073,14 @@ static int i915_pm_freeze(struct device *kdev) > > > > static int i915_pm_freeze_late(struct device *kdev) > > { > > + struct drm_device *dev = &kdev_to_i915(kdev)->drm; > > int ret; > > > > - ret = i915_pm_suspend_late(kdev); > > - if (ret) > > - return ret; > > + if (dev->switch_power_state != DRM_SWITCH_POWER_OFF) { > > + ret = i915_drm_suspend_late(dev, true); > > + if (ret) > > + return ret; > > + } > > > > ret = i915_gem_freeze_late(kdev_to_i915(kdev)); > > if (ret) > > -- > > 2.13.2 > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
