On 08/02/2018 15:46, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

We can implement limited RC6 counter wrap-around protection under the
assumption that clients will be reading this value more frequently than
the wrap period on a given platform.

With the typical wrap-around period being ~90 minutes, even with the
exception of Baytrail which wraps every 13 seconds, this sounds like a
reasonable assumption.

Implementation works by storing a 64-bit software copy of a hardware RC6
counter, along with the previous HW counter snapshot. This enables it to
detect wrap is polled frequently enough and keep the software copy
monotonically incrementing.

v2:
  * Missed GEN6_GT_GFX_RC6_LOCKED when considering slot sizing and
    indexing.
  * Fixed off-by-one in wrap-around handling. (Chris Wilson)

v3:
  * Simplify index checking by using unsigned int. (Chris Wilson)
  * Expand the comment to explain why indexing works.

v4:
  * Use __int128 if supported.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk> # v3
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/intel_pm.c | 72 +++++++++++++++++++++++++++++++++++------
  2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ad1fc845cd1b..28a2671a26c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -946,6 +946,8 @@ struct intel_rps {
struct intel_rc6 {
        bool enabled;
+       u64 prev_hw_residency[4];
+       u64 cur_residency[4];
  };
struct intel_llc_pstate {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 41f26ab46501..24d9423c5f9e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -9417,15 +9417,16 @@ static u64 vlv_residency_raw(struct drm_i915_private 
*dev_priv,
                             const i915_reg_t reg)
  {
        u32 lower, upper, tmp;
-       unsigned long flags;
        int loop = 2;
- /* The register accessed do not need forcewake. We borrow
+       /*
+        * The register accessed do not need forcewake. We borrow
         * uncore lock to prevent concurrent access to range reg.
         */
-       spin_lock_irqsave(&dev_priv->uncore.lock, flags);
+       lockdep_assert_held(&dev_priv->uncore.lock);
- /* vlv and chv residency counters are 40 bits in width.
+       /*
+        * vlv and chv residency counters are 40 bits in width.
         * With a control bit, we can choose between upper or lower
         * 32bit window into this counter.
         *
@@ -9449,29 +9450,54 @@ static u64 vlv_residency_raw(struct drm_i915_private 
*dev_priv,
                upper = I915_READ_FW(reg);
        } while (upper != tmp && --loop);
- /* Everywhere else we always use VLV_COUNTER_CONTROL with the
+       /*
+        * Everywhere else we always use VLV_COUNTER_CONTROL with the
         * VLV_COUNT_RANGE_HIGH bit set - so it is safe to leave it set
         * now.
         */
- spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
-
        return lower | (u64)upper << 8;
  }
u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
                           const i915_reg_t reg)
  {
-       u64 time_hw;
+#if IS_ENABLED(CONFIG_ARCH_SUPPORTS_INT128)
+       unsigned __int128 acc;
+#else
+       u64 acc;
+#endif
+       u64 time_hw, prev_hw, overflow_hw;
+       unsigned int fw_domains;
+       unsigned long flags;
+       unsigned int i;
        u32 mul, div;
if (!HAS_RC6(dev_priv))
                return 0;
+ /*
+        * Store previous hw counter values for counter wrap-around handling.
+        *
+        * There are only four interesting registers and they live next to each
+        * other so we can use the relative address, compared to the smallest
+        * one as the index into driver storage.
+        */
+       i = (i915_mmio_reg_offset(reg) -
+            i915_mmio_reg_offset(GEN6_GT_GFX_RC6_LOCKED)) / sizeof(u32);
+       if (WARN_ON_ONCE(i >= ARRAY_SIZE(dev_priv->gt_pm.rc6.cur_residency)))
+               return 0;
+
+       fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ);
+
+       spin_lock_irqsave(&dev_priv->uncore.lock, flags);
+       intel_uncore_forcewake_get__locked(dev_priv, fw_domains);
+
        /* On VLV and CHV, residency time is in CZ units rather than 1.28us */
        if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
                mul = 1000000;
                div = dev_priv->czclk_freq;
+               overflow_hw = BIT_ULL(40);
                time_hw = vlv_residency_raw(dev_priv, reg);
        } else {
                /* 833.33ns units on Gen9LP, 1.28us elsewhere. */
@@ -9483,10 +9509,36 @@ u64 intel_rc6_residency_ns(struct drm_i915_private 
*dev_priv,
                        div = 1;
                }
- time_hw = I915_READ(reg);
+               overflow_hw = BIT_ULL(32);
+               time_hw = I915_READ_FW(reg);
        }
- return DIV_ROUND_UP_ULL(time_hw * mul, div);
+       /*
+        * Counter wrap handling.
+        *
+        * But relying on a sufficient frequency of queries otherwise counters
+        * can still wrap.
+        */
+       prev_hw = dev_priv->gt_pm.rc6.prev_hw_residency[i];
+       dev_priv->gt_pm.rc6.prev_hw_residency[i] = time_hw;
+
+       /* RC6 delta from last sample. */
+       if (time_hw >= prev_hw)
+               time_hw -= prev_hw;
+       else
+               time_hw += overflow_hw - prev_hw;
+
+       /* Add delta to RC6 extended raw driver copy. */
+       time_hw += dev_priv->gt_pm.rc6.cur_residency[i];
+       dev_priv->gt_pm.rc6.cur_residency[i] = time_hw;
+
+       intel_uncore_forcewake_put__locked(dev_priv, fw_domains);
+       spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
+
+       acc = time_hw;
+       acc *= mul;
+
+       return DIV_ROUND_UP_ULL(acc, div);

Nope, this casts to 64-bit before division..

Regards,

Tvrtko

  }
u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to