[Intel-gfx] [PATCH 2/2] drm/i915: Use init power domain during reset

[Mika Kuoppala]

Ville Syrjälä <ville.syrjala at linux.intel.com> writes:

> 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.
>
> Hmm. Aren't we holding a forcewake around the reset already? Also with
> the GT hung it shouldn't really be dropping into RC6 anyway, which
> should keep the thing in pc2 (IIRC) or higher. I guess if we do a
> simulated hang it might be in rc6, but the forcewake should be enough
> protection there.
>

We are holding forcewake and pm ref. So this is paranoia.
But as the dmc don't obey us leading to state mismatch,
I thought this is the way to force (throught the hw sync)
the dm state as disabled before we hit the reset button.

Another way would be to introduce intel_power_domains_reset().
Looks like _*suspend() does too much for reset.

> Deep package C-state is the only link I could think of between the GT
> and DMC. Otherwise I can't think of any real link between the two. So
> quite baffled by this.
>

Me too. This is all ducttape until the root cause is known.
-Mika

>> 
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala at 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);
>>  
>>  		/*
>> @@ -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
>
> -- 
> Ville Syrjälä
> Intel OTC