[Intel-gfx] [PATCH] drm/i915/gen9+: Set same power state before hibernation image save/restore

[Ville Syrjälä]

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 at intel.com>
> Reported-and-tested-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Wang Wendy <wendy.wang at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> 
> ---
> 
> [ 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?

Patch lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

>  
>  	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