Re: [PATCH] drm/i915: Fix mismatched INIT power domain disabling during suspend

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 13, 2016 at 02:34:06PM +0300, Imre Deak wrote:
> Currently the display INIT power domain disabling/enabling happens in a
> mismatched way in the suspend/resume_early hooks respectively. This can
> leave display power wells incorrectly disabled in the resume hook if the
> suspend sequence is aborted for some reason resulting in the
> suspend/resume hooks getting called but the suspend_late/resume_early
> hooks being skipped. In particular this change fixes "Unclaimed read
> from register 0x1e1204" on BYT/BSW triggered from i915_drm_resume()->
> intel_pps_unlock_regs_wa() when suspending with /sys/power/pm_test set
> to devices.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9b3bfc..5adabac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1430,8 +1430,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>  	dev_priv->suspend_count++;
>  
> -	intel_display_set_init_power(dev_priv, false);
> -
>  	intel_csr_ucode_suspend(dev_priv);
>  
>  out:
> @@ -1449,6 +1447,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>  
>  	disable_rpm_wakeref_asserts(dev_priv);
>  
> +	intel_display_set_init_power(dev_priv, false);
> +

Makes sense in most cases.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	fw_csr = !IS_BROXTON(dev_priv) &&
>  		suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;

However this part is quite confusing. We do the init_power_put
unconditionally, but the init_power_get is done from the init_hw()
which is conditional on this supend_to_idle special casing. So it seems
quite assymmetrical, and I can no longer claim to understand it.

>  	/*
> -- 
> 2.5.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx