| Message ID | 20180808210842.3555-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Restore user forcewake domains across suspend | expand |
On Wed, Aug 08, 2018 at 10:08:42PM +0100, Chris Wilson wrote: > On suspend, we cancel the automatic forcewake and clear all other sources > of forcewake so the machine can sleep before we do suspend. However, we > expose the forcewake to userspace (only via debugfs, but nevertheless we > do) and want to restore that upon resume or else our accounting will be > off and we may not acquire the forcewake before we use it. So record > which domains we cleared on suspend and reacquire them early on resume. > > v2: Hold the spinlock to appease our sanitychecks > > Reported-by: Imre Deak <imre.deak@linux.intel.com> > Fixes: b8473050805f ("drm/i915: Fix forcewake active domain tracking") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Cc: Imre Deak <imre.deak@linux.intel.com> The issue happens to fix itself by the first reg access needing an auto FW during resume, but such an access isn't guaranteed: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 44 ++++++++++--------- > drivers/gpu/drm/i915/intel_uncore.h | 1 + > drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 +- > 3 files changed, 26 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 284be151f645..cf40361fe181 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -369,8 +369,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer) > } > > /* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */ > -static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > - bool restore) > +static unsigned int > +intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv) > { > unsigned long irqflags; > struct intel_uncore_forcewake_domain *domain; > @@ -422,20 +422,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); > > fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains); > - > - if (restore) { /* If reset with a user forcewake, try to restore */ > - if (fw) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, fw); > - > - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > - dev_priv->uncore.fifo_count = > - fifo_free_entries(dev_priv); > - } > - > - if (!restore) > - assert_forcewakes_inactive(dev_priv); > + assert_forcewakes_inactive(dev_priv); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > + > + return fw; /* track the lost user forcewake domains */ > } > > static u64 gen9_edram_size(struct drm_i915_private *dev_priv) > @@ -544,7 +535,7 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) > } > > static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > - bool restore_forcewake) > + unsigned int restore_forcewake) > { > /* clear out unclaimed reg detection bit */ > if (check_for_unclaimed_mmio(dev_priv)) > @@ -559,7 +550,17 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > } > > iosf_mbi_punit_acquire(); > - intel_uncore_forcewake_reset(dev_priv, restore_forcewake); > + intel_uncore_forcewake_reset(dev_priv); > + if (restore_forcewake) { > + spin_lock_irq(&dev_priv->uncore.lock); > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > + restore_forcewake); > + > + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > + dev_priv->uncore.fifo_count = > + fifo_free_entries(dev_priv); > + spin_unlock_irq(&dev_priv->uncore.lock); > + } > iosf_mbi_punit_release(); > } > > @@ -568,13 +569,16 @@ void intel_uncore_suspend(struct drm_i915_private *dev_priv) > iosf_mbi_punit_acquire(); > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > &dev_priv->uncore.pmic_bus_access_nb); > - intel_uncore_forcewake_reset(dev_priv, false); > + dev_priv->uncore.fw_domains_user = > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > } > > void intel_uncore_resume_early(struct drm_i915_private *dev_priv) > { > - __intel_uncore_early_sanitize(dev_priv, true); > + __intel_uncore_early_sanitize(dev_priv, > + dev_priv->uncore.fw_domains_user); > + > iosf_mbi_register_pmic_bus_access_notifier( > &dev_priv->uncore.pmic_bus_access_nb); > i915_check_and_clear_faults(dev_priv); > @@ -1555,7 +1559,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) > > intel_uncore_edram_detect(dev_priv); > intel_uncore_fw_domains_init(dev_priv); > - __intel_uncore_early_sanitize(dev_priv, false); > + __intel_uncore_early_sanitize(dev_priv, 0); > > dev_priv->uncore.unclaimed_mmio_check = 1; > dev_priv->uncore.pmic_bus_access_nb.notifier_call = > @@ -1642,7 +1646,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv) > iosf_mbi_punit_acquire(); > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > &dev_priv->uncore.pmic_bus_access_nb); > - intel_uncore_forcewake_reset(dev_priv, false); > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > } > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h > index 2fbe93178fb2..137d8ac856fe 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.h > +++ b/drivers/gpu/drm/i915/intel_uncore.h > @@ -104,6 +104,7 @@ struct intel_uncore { > > enum forcewake_domains fw_domains; > enum forcewake_domains fw_domains_active; > + enum forcewake_domains fw_domains_user; > > u32 fw_set; > u32 fw_clear; > diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c > index 47bc5b2ddb56..81d9d31042a9 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c > +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c > @@ -160,7 +160,7 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri > i915_reg_t reg = { offset }; > > iosf_mbi_punit_acquire(); > - intel_uncore_forcewake_reset(dev_priv, false); > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > > check_for_unclaimed_mmio(dev_priv); > -- > 2.18.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 08/08/2018 22:08, Chris Wilson wrote: > On suspend, we cancel the automatic forcewake and clear all other sources > of forcewake so the machine can sleep before we do suspend. However, we > expose the forcewake to userspace (only via debugfs, but nevertheless we > do) and want to restore that upon resume or else our accounting will be > off and we may not acquire the forcewake before we use it. So record > which domains we cleared on suspend and reacquire them early on resume. > > v2: Hold the spinlock to appease our sanitychecks > > Reported-by: Imre Deak <imre.deak@linux.intel.com> > Fixes: b8473050805f ("drm/i915: Fix forcewake active domain tracking") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Cc: Imre Deak <imre.deak@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_uncore.c | 44 ++++++++++--------- > drivers/gpu/drm/i915/intel_uncore.h | 1 + > drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 +- > 3 files changed, 26 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > index 284be151f645..cf40361fe181 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -369,8 +369,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer) > } > > /* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */ > -static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > - bool restore) > +static unsigned int > +intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv) > { > unsigned long irqflags; > struct intel_uncore_forcewake_domain *domain; > @@ -422,20 +422,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); > > fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains); > - > - if (restore) { /* If reset with a user forcewake, try to restore */ > - if (fw) > - dev_priv->uncore.funcs.force_wake_get(dev_priv, fw); > - > - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > - dev_priv->uncore.fifo_count = > - fifo_free_entries(dev_priv); > - } > - > - if (!restore) > - assert_forcewakes_inactive(dev_priv); > + assert_forcewakes_inactive(dev_priv); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > + > + return fw; /* track the lost user forcewake domains */ > } > > static u64 gen9_edram_size(struct drm_i915_private *dev_priv) > @@ -544,7 +535,7 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) > } > > static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > - bool restore_forcewake) > + unsigned int restore_forcewake) > { > /* clear out unclaimed reg detection bit */ > if (check_for_unclaimed_mmio(dev_priv)) > @@ -559,7 +550,17 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > } > > iosf_mbi_punit_acquire(); > - intel_uncore_forcewake_reset(dev_priv, restore_forcewake); > + intel_uncore_forcewake_reset(dev_priv); > + if (restore_forcewake) { > + spin_lock_irq(&dev_priv->uncore.lock); > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > + restore_forcewake); > + > + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > + dev_priv->uncore.fifo_count = > + fifo_free_entries(dev_priv); > + spin_unlock_irq(&dev_priv->uncore.lock); > + } > iosf_mbi_punit_release(); > } > > @@ -568,13 +569,16 @@ void intel_uncore_suspend(struct drm_i915_private *dev_priv) > iosf_mbi_punit_acquire(); > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > &dev_priv->uncore.pmic_bus_access_nb); > - intel_uncore_forcewake_reset(dev_priv, false); > + dev_priv->uncore.fw_domains_user = > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > } > > void intel_uncore_resume_early(struct drm_i915_private *dev_priv) > { > - __intel_uncore_early_sanitize(dev_priv, true); > + __intel_uncore_early_sanitize(dev_priv, > + dev_priv->uncore.fw_domains_user); > + > iosf_mbi_register_pmic_bus_access_notifier( > &dev_priv->uncore.pmic_bus_access_nb); > i915_check_and_clear_faults(dev_priv); > @@ -1555,7 +1559,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) > > intel_uncore_edram_detect(dev_priv); > intel_uncore_fw_domains_init(dev_priv); > - __intel_uncore_early_sanitize(dev_priv, false); > + __intel_uncore_early_sanitize(dev_priv, 0); I wonder if a tweak to not have a parameter but function acts on uncore.fw_domains_user on it's own, and consumes it after restoring form it. But it is just a different tweak on the same theme so never mind. > > dev_priv->uncore.unclaimed_mmio_check = 1; > dev_priv->uncore.pmic_bus_access_nb.notifier_call = > @@ -1642,7 +1646,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv) > iosf_mbi_punit_acquire(); > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > &dev_priv->uncore.pmic_bus_access_nb); > - intel_uncore_forcewake_reset(dev_priv, false); > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > } > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h > index 2fbe93178fb2..137d8ac856fe 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.h > +++ b/drivers/gpu/drm/i915/intel_uncore.h > @@ -104,6 +104,7 @@ struct intel_uncore { > > enum forcewake_domains fw_domains; > enum forcewake_domains fw_domains_active; > + enum forcewake_domains fw_domains_user; Maybe call it fw_domains_saved so it is obvious it is special since user domains are not always tracked in it. > > u32 fw_set; > u32 fw_clear; > diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c > index 47bc5b2ddb56..81d9d31042a9 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c > +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c > @@ -160,7 +160,7 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri > i915_reg_t reg = { offset }; > > iosf_mbi_punit_acquire(); > - intel_uncore_forcewake_reset(dev_priv, false); > + intel_uncore_forcewake_reset(dev_priv); > iosf_mbi_punit_release(); > > check_for_unclaimed_mmio(dev_priv); > AFAICS it was obviously broken and this fixes it. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-08-09 12:46:05) > > On 08/08/2018 22:08, Chris Wilson wrote: > > On suspend, we cancel the automatic forcewake and clear all other sources > > of forcewake so the machine can sleep before we do suspend. However, we > > expose the forcewake to userspace (only via debugfs, but nevertheless we > > do) and want to restore that upon resume or else our accounting will be > > off and we may not acquire the forcewake before we use it. So record > > which domains we cleared on suspend and reacquire them early on resume. > > > > v2: Hold the spinlock to appease our sanitychecks > > > > Reported-by: Imre Deak <imre.deak@linux.intel.com> > > Fixes: b8473050805f ("drm/i915: Fix forcewake active domain tracking") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > > Cc: Imre Deak <imre.deak@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_uncore.c | 44 ++++++++++--------- > > drivers/gpu/drm/i915/intel_uncore.h | 1 + > > drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 +- > > 3 files changed, 26 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c > > index 284be151f645..cf40361fe181 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.c > > +++ b/drivers/gpu/drm/i915/intel_uncore.c > > @@ -369,8 +369,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer) > > } > > > > /* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */ > > -static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > > - bool restore) > > +static unsigned int > > +intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv) > > { > > unsigned long irqflags; > > struct intel_uncore_forcewake_domain *domain; > > @@ -422,20 +422,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, > > dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); > > > > fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains); > > - > > - if (restore) { /* If reset with a user forcewake, try to restore */ > > - if (fw) > > - dev_priv->uncore.funcs.force_wake_get(dev_priv, fw); > > - > > - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > > - dev_priv->uncore.fifo_count = > > - fifo_free_entries(dev_priv); > > - } > > - > > - if (!restore) > > - assert_forcewakes_inactive(dev_priv); > > + assert_forcewakes_inactive(dev_priv); > > > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > + > > + return fw; /* track the lost user forcewake domains */ > > } > > > > static u64 gen9_edram_size(struct drm_i915_private *dev_priv) > > @@ -544,7 +535,7 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) > > } > > > > static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > > - bool restore_forcewake) > > + unsigned int restore_forcewake) > > { > > /* clear out unclaimed reg detection bit */ > > if (check_for_unclaimed_mmio(dev_priv)) > > @@ -559,7 +550,17 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, > > } > > > > iosf_mbi_punit_acquire(); > > - intel_uncore_forcewake_reset(dev_priv, restore_forcewake); > > + intel_uncore_forcewake_reset(dev_priv); > > + if (restore_forcewake) { > > + spin_lock_irq(&dev_priv->uncore.lock); > > + dev_priv->uncore.funcs.force_wake_get(dev_priv, > > + restore_forcewake); > > + > > + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) > > + dev_priv->uncore.fifo_count = > > + fifo_free_entries(dev_priv); > > + spin_unlock_irq(&dev_priv->uncore.lock); > > + } > > iosf_mbi_punit_release(); > > } > > > > @@ -568,13 +569,16 @@ void intel_uncore_suspend(struct drm_i915_private *dev_priv) > > iosf_mbi_punit_acquire(); > > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > > &dev_priv->uncore.pmic_bus_access_nb); > > - intel_uncore_forcewake_reset(dev_priv, false); > > + dev_priv->uncore.fw_domains_user = > > + intel_uncore_forcewake_reset(dev_priv); > > iosf_mbi_punit_release(); > > } > > > > void intel_uncore_resume_early(struct drm_i915_private *dev_priv) > > { > > - __intel_uncore_early_sanitize(dev_priv, true); > > + __intel_uncore_early_sanitize(dev_priv, > > + dev_priv->uncore.fw_domains_user); > > + > > iosf_mbi_register_pmic_bus_access_notifier( > > &dev_priv->uncore.pmic_bus_access_nb); > > i915_check_and_clear_faults(dev_priv); > > @@ -1555,7 +1559,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) > > > > intel_uncore_edram_detect(dev_priv); > > intel_uncore_fw_domains_init(dev_priv); > > - __intel_uncore_early_sanitize(dev_priv, false); > > + __intel_uncore_early_sanitize(dev_priv, 0); > > I wonder if a tweak to not have a parameter but function acts on > uncore.fw_domains_user on it's own, and consumes it after restoring form > it. But it is just a different tweak on the same theme so never mind. > > > > > dev_priv->uncore.unclaimed_mmio_check = 1; > > dev_priv->uncore.pmic_bus_access_nb.notifier_call = > > @@ -1642,7 +1646,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv) > > iosf_mbi_punit_acquire(); > > iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( > > &dev_priv->uncore.pmic_bus_access_nb); > > - intel_uncore_forcewake_reset(dev_priv, false); > > + intel_uncore_forcewake_reset(dev_priv); > > iosf_mbi_punit_release(); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h > > index 2fbe93178fb2..137d8ac856fe 100644 > > --- a/drivers/gpu/drm/i915/intel_uncore.h > > +++ b/drivers/gpu/drm/i915/intel_uncore.h > > @@ -104,6 +104,7 @@ struct intel_uncore { > > > > enum forcewake_domains fw_domains; > > enum forcewake_domains fw_domains_active; > > + enum forcewake_domains fw_domains_user; > > Maybe call it fw_domains_saved so it is obvious it is special since user > domains are not always tracked in it. Tweaked s/user/saved/ and gave it a short comment. Thanks for reporting the issue Imre, and for both of you in reviewing it. -Chris
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 284be151f645..cf40361fe181 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -369,8 +369,8 @@ intel_uncore_fw_release_timer(struct hrtimer *timer) } /* Note callers must have acquired the PUNIT->PMIC bus, before calling this. */ -static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, - bool restore) +static unsigned int +intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv) { unsigned long irqflags; struct intel_uncore_forcewake_domain *domain; @@ -422,20 +422,11 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, dev_priv->uncore.funcs.force_wake_put(dev_priv, fw); fw_domains_reset(dev_priv, dev_priv->uncore.fw_domains); - - if (restore) { /* If reset with a user forcewake, try to restore */ - if (fw) - dev_priv->uncore.funcs.force_wake_get(dev_priv, fw); - - if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) - dev_priv->uncore.fifo_count = - fifo_free_entries(dev_priv); - } - - if (!restore) - assert_forcewakes_inactive(dev_priv); + assert_forcewakes_inactive(dev_priv); spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); + + return fw; /* track the lost user forcewake domains */ } static u64 gen9_edram_size(struct drm_i915_private *dev_priv) @@ -544,7 +535,7 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv) } static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, - bool restore_forcewake) + unsigned int restore_forcewake) { /* clear out unclaimed reg detection bit */ if (check_for_unclaimed_mmio(dev_priv)) @@ -559,7 +550,17 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv, } iosf_mbi_punit_acquire(); - intel_uncore_forcewake_reset(dev_priv, restore_forcewake); + intel_uncore_forcewake_reset(dev_priv); + if (restore_forcewake) { + spin_lock_irq(&dev_priv->uncore.lock); + dev_priv->uncore.funcs.force_wake_get(dev_priv, + restore_forcewake); + + if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv)) + dev_priv->uncore.fifo_count = + fifo_free_entries(dev_priv); + spin_unlock_irq(&dev_priv->uncore.lock); + } iosf_mbi_punit_release(); } @@ -568,13 +569,16 @@ void intel_uncore_suspend(struct drm_i915_private *dev_priv) iosf_mbi_punit_acquire(); iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( &dev_priv->uncore.pmic_bus_access_nb); - intel_uncore_forcewake_reset(dev_priv, false); + dev_priv->uncore.fw_domains_user = + intel_uncore_forcewake_reset(dev_priv); iosf_mbi_punit_release(); } void intel_uncore_resume_early(struct drm_i915_private *dev_priv) { - __intel_uncore_early_sanitize(dev_priv, true); + __intel_uncore_early_sanitize(dev_priv, + dev_priv->uncore.fw_domains_user); + iosf_mbi_register_pmic_bus_access_notifier( &dev_priv->uncore.pmic_bus_access_nb); i915_check_and_clear_faults(dev_priv); @@ -1555,7 +1559,7 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) intel_uncore_edram_detect(dev_priv); intel_uncore_fw_domains_init(dev_priv); - __intel_uncore_early_sanitize(dev_priv, false); + __intel_uncore_early_sanitize(dev_priv, 0); dev_priv->uncore.unclaimed_mmio_check = 1; dev_priv->uncore.pmic_bus_access_nb.notifier_call = @@ -1642,7 +1646,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv) iosf_mbi_punit_acquire(); iosf_mbi_unregister_pmic_bus_access_notifier_unlocked( &dev_priv->uncore.pmic_bus_access_nb); - intel_uncore_forcewake_reset(dev_priv, false); + intel_uncore_forcewake_reset(dev_priv); iosf_mbi_punit_release(); } diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index 2fbe93178fb2..137d8ac856fe 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -104,6 +104,7 @@ struct intel_uncore { enum forcewake_domains fw_domains; enum forcewake_domains fw_domains_active; + enum forcewake_domains fw_domains_user; u32 fw_set; u32 fw_clear; diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c index 47bc5b2ddb56..81d9d31042a9 100644 --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c @@ -160,7 +160,7 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri i915_reg_t reg = { offset }; iosf_mbi_punit_acquire(); - intel_uncore_forcewake_reset(dev_priv, false); + intel_uncore_forcewake_reset(dev_priv); iosf_mbi_punit_release(); check_for_unclaimed_mmio(dev_priv);
On suspend, we cancel the automatic forcewake and clear all other sources of forcewake so the machine can sleep before we do suspend. However, we expose the forcewake to userspace (only via debugfs, but nevertheless we do) and want to restore that upon resume or else our accounting will be off and we may not acquire the forcewake before we use it. So record which domains we cleared on suspend and reacquire them early on resume. v2: Hold the spinlock to appease our sanitychecks Reported-by: Imre Deak <imre.deak@linux.intel.com> Fixes: b8473050805f ("drm/i915: Fix forcewake active domain tracking") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Cc: Imre Deak <imre.deak@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 44 ++++++++++--------- drivers/gpu/drm/i915/intel_uncore.h | 1 + drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 +- 3 files changed, 26 insertions(+), 21 deletions(-)