On 08/11/2019 08:56, Chris Wilson wrote:
On gen7, we have to avoid concurrent access to the same mmio cacheline,
and so coordinate all mmio access with the uncore->lock. However, for
pmu, we want to avoid perturbing the system and disabling interrupts
unnecessarily, so restrict the w/a to gen7 where it is requied to
prevent machine hangs.

Signed-off-by: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
---
  drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index dbde80a376cb..9e1627782284 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -292,6 +292,11 @@ add_sample(struct i915_pmu_sample *sample, u32 val)
        sample->cur += val;
  }
+static bool exclusive_mmio_access(const struct drm_i915_private *i915)
+{
+       return IS_GEN(i915, 7);
+}
+
  static void
  engines_sample(struct intel_gt *gt, unsigned int period_ns)
  {
@@ -311,7 +316,8 @@ engines_sample(struct intel_gt *gt, unsigned int period_ns)
                if (!intel_engine_pm_get_if_awake(engine))
                        continue;
- spin_lock_irqsave(&engine->uncore->lock, flags);
+               if (exclusive_mmio_access(i915))
+                       spin_lock_irqsave(&engine->uncore->lock, flags);
val = ENGINE_READ_FW(engine, RING_CTL);
                if (val == 0) /* powerwell off => engine idle */
@@ -342,7 +348,8 @@ engines_sample(struct intel_gt *gt, unsigned int period_ns)
                        add_sample(&pmu->sample[I915_SAMPLE_BUSY], period_ns);
skip:
-               spin_unlock_irqrestore(&engine->uncore->lock, flags);
+               if (exclusive_mmio_access(i915))
+                       spin_unlock_irqrestore(&engine->uncore->lock, flags);
                intel_engine_pm_put(engine);
        }
  }


For bonus points cache the check in a local? Or cache the lock in a local for even more win, spinlock_t *lock = exclusive_mmio_access(..) ? &.. : NULL ?

Reviewed-by: Tvrtko Ursulin <[email protected]>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to