Re: [PATCH] drm/i915: Handle RC6 counter wrap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Tvrtko Ursulin (2018-02-07 16:37:10)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> 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.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 57 ++++++++++++++++++++++++++++++++++-------
>  2 files changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ad1fc845cd1b..c5cba355bfa7 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[3];
> +       u64 cur_residency[3];
>  };
>  
>  struct intel_llc_pstate {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 41f26ab46501..6480285d094c 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,44 @@ 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;
> +       u64 time_hw, prev_hw, max_hw;
> +       unsigned int fw_domains;
> +       unsigned long flags;
>         u32 mul, div;
> +       int i;
>  
>         if (!HAS_RC6(dev_priv))
>                 return 0;
>  
> +       /* Counter wrap handling stores previous hw counter values. */
> +       i = i915_mmio_reg_offset(reg) / sizeof(u32) -
> +           i915_mmio_reg_offset(GEN6_GT_GFX_RC6) / sizeof(u32);
> +       if (WARN_ON_ONCE(i < 0 ||
> +                        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;
> +               max_hw = BIT_ULL(40) - 1;
>                 time_hw = vlv_residency_raw(dev_priv, reg);
>         } else {
>                 /* 833.33ns units on Gen9LP, 1.28us elsewhere. */
> @@ -9483,9 +9499,32 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
>                         div = 1;
>                 }
>  
> -               time_hw = I915_READ(reg);
> +               max_hw = BIT_ULL(32) - 1;
> +               time_hw = I915_READ_FW(reg);
>         }
>  
> +       /*
> +        * 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 += max_hw - prev_hw;

I think max_hw is off-by-one.

If prev_hw is the maximum value possible ~0, and time_hw is 0, the
increment should be one. The above adds on 0.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux