Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
