Re: [PATCH 2/2] drm/i915: Use init power domain during reset

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jan 19, 2016 at 09:50:09PM +0200, Mika Kuoppala wrote:
> If we have driver failure in our power well and/or dc
> state keeping, we might try to reset without powers.
> 
> Evidence shows that resetting the chip with dc6 enabled
> don't lead to desired results. The dmc kept it's enabled
> dc6 state over the reset. On subsequent init the rings
> just hanged right from the start so they didn't reset
> properly or that the dmc interfered with the init.
> 
> Harden the reset by setting power domains to be in
> init mode during reset. This causes the hw state to be
> flushed so dc6 will be forcibly disabled also.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 25a89373df63..78e242b5c357 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2510,6 +2510,14 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
>  		 */
>  		intel_runtime_pm_get(dev_priv);
>  
> +		/* Even if we hold the pm ref, we still might have inconsistent
> +		 * power states due to driver failure. Trying to reset without
> +		 * powers or with wrong dmc firmware state is futile. Flush
> +		 * our power well and dc states ensuring that we reset with
> +		 * powers enabled.
> +		 */
> +		intel_display_set_init_power(dev_priv, true);
> +
>  		intel_prepare_reset(dev);

Since this seems just dc6 related there's good reason to believe it's the
display mmio writes in intel_prepare/finish_reset(). Can you just grab
relevant power wells for the register we write in there instead of
grabbing everything at the top level here?

Asking for that since with atomic this shouldn't happen any more ...
-Daniel

>  
>  		/*
> @@ -2522,6 +2530,8 @@ static void i915_reset_and_wakeup(struct drm_device *dev)
>  
>  		intel_finish_reset(dev);
>  
> +		intel_display_set_init_power(dev_priv, false);
> +
>  		intel_runtime_pm_put(dev_priv);
>  
>  		if (ret == 0) {
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx