drm/i915: Acquire PUNIT->PMIC bus for intel_uncore_forcewake_reset()

intel_uncore_forcewake_reset() does forcewake puts and gets as such
we need to make sure that no-one tries to access the PUNIT->PMIC bus
(on systems where this bus is shared) while it runs, otherwise bad
things happen.

Normally this is taken care of by the i915_pmic_bus_access_notifier()
which does an intel_uncore_forcewake_get(FORCEWAKE_ALL) when some other
driver tries to access the PMIC bus, so that later forcewake gets are
no-ops (for the duration of the bus access).

But intel_uncore_forcewake_reset gets called in 3 cases:
1) Before registering the pmic_bus_access_notifier
2) After unregistering the pmic_bus_access_notifier
3) To reset forcewake state on a GPU reset

In all 3 cases the i915_pmic_bus_access_notifier() protection is
insufficient.

This commit fixes this race by calling iosf_mbi_punit_acquire() before
calling intel_uncore_forcewake_reset(). In the case where it is called
directly after unregistering the pmic_bus_access_notifier, we need to
hold the punit-lock over both calls to avoid a race where
intel_uncore_fw_release_timer() may execute between the 2 calls.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171019111620.26761-3-hdegoede@redhat.com

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 0a6d952..211acee 100644 (file)
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -345,6 +345,7 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
        return HRTIMER_NORESTART;
 }
 
+/* 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)
 {
@@ -353,6 +354,8 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
        int retry_count = 100;
        enum forcewake_domains fw, active_domains;
 
+       iosf_mbi_assert_punit_acquired();
+
        /* Hold uncore.lock across reset to prevent any register access
         * with forcewake not set correctly. Wait until all pending
         * timers are run before holding.
@@ -532,14 +535,18 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
                                   GT_FIFO_CTL_RC6_POLICY_STALL);
        }
 
+       iosf_mbi_punit_acquire();
        intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
+       iosf_mbi_punit_release();
 }
 
 void intel_uncore_suspend(struct drm_i915_private *dev_priv)
 {
-       iosf_mbi_unregister_pmic_bus_access_notifier(
+       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);
+       iosf_mbi_punit_release();
 }
 
 void intel_uncore_resume_early(struct drm_i915_private *dev_priv)
@@ -1419,12 +1426,14 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 
 void intel_uncore_fini(struct drm_i915_private *dev_priv)
 {
-       iosf_mbi_unregister_pmic_bus_access_notifier(
-               &dev_priv->uncore.pmic_bus_access_nb);
-
        /* Paranoia: make sure we have disabled everything before we exit. */
        intel_uncore_sanitize(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);
+       iosf_mbi_punit_release();
 }
 
 static const struct reg_whitelist {

diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
index f52a4ab..2f63676 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
+++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
@@ -148,7 +148,10 @@ static int intel_uncore_check_forcewake_domains(struct drm_i915_private *dev_pri
        for_each_set_bit(offset, valid, FW_RANGE) {
                i915_reg_t reg = { offset };
 
+               iosf_mbi_punit_acquire();
                intel_uncore_forcewake_reset(dev_priv, false);
+               iosf_mbi_punit_release();
+
                check_for_unclaimed_mmio(dev_priv);
 
                (void)I915_READ(reg);