[Intel-gfx] [PATCH] drm/i915: Only arm the forcewake release timer on the final put

[Dave Gordon]

On 24/03/16 13:32, Chris Wilson wrote:
> If we arm the release timer on acquiring the forcewake, we will release
> the forcewake on the jiffie afterwards. If we only arm the release timer
> on the final put, we will release the forcewake slightly later instead.

Yes, I had wondered why we armed to timer on forcewake_get(). So 
definitely agree it should be on last put().

> Much, much worse, we did not acquire a refcount for the armed timing
> during the get(), and so unbalanced our forcewake counting.

Hmm ... refcount /looks/ OK to me? But I'd need to check more ...

.Dave.

> Reported-by: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uncore.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 96799392c2c7..d857168c6c9b 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -60,6 +60,7 @@ fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
>   static inline void
>   fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d)
>   {
> +	d->wake_count++;
>   	mod_timer_pinned(&d->timer, jiffies + 1);
>   }
>
> @@ -491,7 +492,6 @@ static void __intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
>   		if (--domain->wake_count)
>   			continue;
>
> -		domain->wake_count++;
>   		fw_domain_arm_timer(domain);
>   	}
>   }
> @@ -733,7 +733,6 @@ static inline void __force_wake_get(struct drm_i915_private *dev_priv,
>   		}
>
>   		domain->wake_count++;
> -		fw_domain_arm_timer(domain);
>   	}
>
>   	if (fw_domains)
>