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@xxxxxxxxxxxxxxx>
Fixes: b8473050805f ("drm/i915: Fix forcewake active domain tracking")
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Cc: Imre Deak <imre.deak@xxxxxxxxxxxxxxx>
---
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@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
