[Intel-gfx] [PATCH] drm/i915: Explicitly track active fw_domain timers

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Stop guessing over whether we have an extra wakeref held by the delayed
> fw put, and track it explicitly for the sake of debug.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 13 ++++++-------
>  drivers/gpu/drm/i915/intel_uncore.h |  1 +
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 5f0367fd3200..768fee3c59aa 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -78,6 +78,8 @@ fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
>  static inline void
>  fw_domain_arm_timer(struct intel_uncore_forcewake_domain *d)
>  {
> +	GEM_BUG_ON(d->uncore->fw_domains_timer & d->mask);
> +	d->uncore->fw_domains_timer |= d->mask;
>  	d->wake_count++;
>  	hrtimer_start_range_ns(&d->timer,
>  			       NSEC_PER_MSEC,
> @@ -353,9 +355,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
>  		return HRTIMER_RESTART;
>  
>  	spin_lock_irqsave(&uncore->lock, irqflags);
> -	if (WARN_ON(domain->wake_count == 0))
> -		domain->wake_count++;
>  
> +	uncore->fw_domains_timer &= ~domain->mask;
>  	if (--domain->wake_count == 0)
>  		uncore->funcs.force_wake_put(uncore, domain->mask);
>  
> @@ -673,8 +674,7 @@ static void __intel_uncore_forcewake_put(struct intel_uncore *uncore,
>  	fw_domains &= uncore->fw_domains;
>  
>  	for_each_fw_domain_masked(domain, fw_domains, uncore, tmp) {
> -		if (WARN_ON(domain->wake_count == 0))
> -			continue;
> +		GEM_BUG_ON(!domain->wake_count);

First impression was harsh. But on second thought, if we can't get
our counts right, then what hope is there. None.

>  
>  		if (--domain->wake_count) {
>  			domain->active = true;
> @@ -764,7 +764,7 @@ void assert_forcewakes_active(struct intel_uncore *uncore,
>  		unsigned int actual = READ_ONCE(domain->wake_count);
>  		unsigned int expect = 1;
>  
> -		if (hrtimer_active(&domain->timer) && READ_ONCE(domain->active))
> +		if (uncore->fw_domains_timer & domain->mask)
>  			expect++; /* pending automatic release */

assetion of timer active on release path seems also superfluous
as the count is our guard.



Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

>  
>  		if (WARN(actual < expect,
> @@ -1160,8 +1160,7 @@ static noinline void ___force_wake_auto(struct intel_uncore *uncore,
>  static inline void __force_wake_auto(struct intel_uncore *uncore,
>  				     enum forcewake_domains fw_domains)
>  {
> -	if (WARN_ON(!fw_domains))
> -		return;
> +	GEM_BUG_ON(!fw_domains);
>  
>  	/* Turn on all requested but inactive supported forcewake domains. */
>  	fw_domains &= uncore->fw_domains;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 7108475d9b24..2f6ffa309669 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -123,6 +123,7 @@ struct intel_uncore {
>  
>  	enum forcewake_domains fw_domains;
>  	enum forcewake_domains fw_domains_active;
> +	enum forcewake_domains fw_domains_timer;
>  	enum forcewake_domains fw_domains_saved; /* user domains saved for S3 */
>  
>  	struct intel_uncore_forcewake_domain {
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx