[Intel-gfx] [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
Ville Syrjälä
ville.syrjala at linux.intel.com
Thu Apr 7 12:13:51 UTC 2016
On Thu, Apr 07, 2016 at 12:24:13PM +0100, Chris Wilson wrote:
> RC6 residency counters are kept by the hardware in 32bit registers,
> which then overflow in a couple of hours. Ideally, we want a continuous
> count of the residency since boot and so we must maintain an overflow
> counter ourselves. Utilities such as powertop read these registers
> through sysfs and the unexpected wrapping produces inconsistent results
> and garbage for the user.
>
> In order to detect the overflow, without intervention, we keep a list of
> the interesting registers and read them every few seconds.
>
> RFC: Every 14 seconds!!! Is it worth imposing that timer on everybody
> for the few users of sysfs? Does userspace only truly care about the
> relative value and not the absolute value since boot? If so, should we
> just limit the timer to when the sysfs file is open? That would need
> userspace to keep said file open for as long as it wants accurate
> results.
At least I don't want to have my VLV wake up every 16 seconds.
My idea to handle this was to expose the max counter value in sysfs
and let powertop deal with it. Unfortunately when I looked at powertop
there didn't seem to a clear way to influence the polling interval from
the gfx related code.
>
> Reported-by: Len Brown <lenb at kernel.org>
> Refernces: https://bugs.freedesktop.org/show_bug.cgi?id=94852
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Len Brown <lenb at kernel.org>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 12 ++++++
> drivers/gpu/drm/i915/i915_sysfs.c | 56 ++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_uncore.c | 80 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 131 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a1f78f275c55..313d21ec39b7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -659,6 +659,16 @@ struct intel_uncore {
>
> struct intel_uncore_funcs funcs;
>
> + /* A few interesting registers track variables over time,
> + * such as RC6 residencies, in 32bit registers which overflow within
> + * a matter of seconds (~14s). However, we want to present them as a
> + * continuous quantity to the user and so we need to keep an overflow
> + * counter.
> + */
> + struct list_head reg64_emu_list;
> + struct timer_list reg64_emu_timer;
> + unsigned long reg64_emu_timeout;
> +
> unsigned fifo_count;
> enum forcewake_domains fw_domains;
>
> @@ -3580,6 +3590,8 @@ __raw_write(64, q)
> #define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
> #define POSTING_READ_FW(reg__) (void)I915_READ_FW(reg__)
>
> +u64 i915_read64emu(struct drm_i915_private *dev_priv, i915_reg_t reg);
> +
> /* "Broadcast RGB" property */
> #define INTEL_BROADCAST_RGB_AUTO 0
> #define INTEL_BROADCAST_RGB_FULL 1
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 2d576b7ff299..6073aa4deaba 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -35,13 +35,27 @@
> #define dev_to_drm_minor(d) dev_get_drvdata((d))
>
> #ifdef CONFIG_PM
> -static u32 calc_residency(struct drm_device *dev,
> - i915_reg_t reg)
> +static unsigned long calc_overflow_jiffies(struct drm_device *dev)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + u32 overflow_ms;
> +
> + /* How many ticks per millisecond? */
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + overflow_ms = ~0u / dev_priv->czclk_freq;
Needs to account for the high range bit.
> + else if (IS_BROXTON(dev_priv))
> + overflow_ms = ~0u / 1200; /* tick every 833ns */
> + else
> + overflow_ms = ~0u / 780; /* tick every 1.28us */
> +
> + return msecs_to_jiffies(overflow_ms);
> +}
> +
> +static unsigned long long calc_residency(struct drm_device *dev, i915_reg_t reg)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> u64 raw_time; /* 32b value may overflow during fixed point math */
> u64 units = 128ULL, div = 100000ULL;
> - u32 ret;
>
> if (!intel_enable_rc6(dev))
> return 0;
> @@ -49,22 +63,22 @@ static u32 calc_residency(struct drm_device *dev,
> intel_runtime_pm_get(dev_priv);
>
> /* On VLV and CHV, residency time is in CZ units rather than 1.28us */
> - if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> units = 1;
> div = dev_priv->czclk_freq;
>
> if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
> units <<= 8;
> - } else if (IS_BROXTON(dev)) {
> + } else if (IS_BROXTON(dev_priv)) {
> units = 1;
> div = 1200; /* 833.33ns */
> }
>
> - raw_time = I915_READ(reg) * units;
> - ret = DIV_ROUND_UP_ULL(raw_time, div);
> + raw_time = i915_read64emu(dev_priv, reg) * units;
>
> intel_runtime_pm_put(dev_priv);
> - return ret;
> +
> + return DIV_ROUND_UP_ULL(raw_time, div);
> }
>
> static ssize_t
> @@ -78,32 +92,32 @@ static ssize_t
> show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf)
> {
> struct drm_minor *dminor = dev_get_drvdata(kdev);
> - u32 rc6_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6);
> - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency);
> + return snprintf(buf, PAGE_SIZE, "%llu\n",
> + calc_residency(dminor->dev, GEN6_GT_GFX_RC6));
> }
>
> static ssize_t
> show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf)
> {
> struct drm_minor *dminor = dev_to_drm_minor(kdev);
> - u32 rc6p_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6p);
> - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency);
> + return snprintf(buf, PAGE_SIZE, "%llu\n",
> + calc_residency(dminor->dev, GEN6_GT_GFX_RC6p));
> }
>
> static ssize_t
> show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf)
> {
> struct drm_minor *dminor = dev_to_drm_minor(kdev);
> - u32 rc6pp_residency = calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp);
> - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency);
> + return snprintf(buf, PAGE_SIZE, "%llu\n",
> + calc_residency(dminor->dev, GEN6_GT_GFX_RC6pp));
> }
>
> static ssize_t
> show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf)
> {
> struct drm_minor *dminor = dev_get_drvdata(kdev);
> - u32 rc6_residency = calc_residency(dminor->dev, VLV_GT_MEDIA_RC6);
> - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency);
> + return snprintf(buf, PAGE_SIZE, "%llu\n",
> + calc_residency(dminor->dev, VLV_GT_MEDIA_RC6));
> }
>
> static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL);
> @@ -593,22 +607,30 @@ static struct bin_attribute error_state_attr = {
>
> void i915_setup_sysfs(struct drm_device *dev)
> {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> int ret;
>
> #ifdef CONFIG_PM
> if (HAS_RC6(dev)) {
> + dev_priv->uncore.reg64_emu_timeout =
> + min(dev_priv->uncore.reg64_emu_timeout,
> + calc_overflow_jiffies(dev) / 2);
> + calc_residency(dev, GEN6_GT_GFX_RC6);
> ret = sysfs_merge_group(&dev->primary->kdev->kobj,
> &rc6_attr_group);
> if (ret)
> DRM_ERROR("RC6 residency sysfs setup failed\n");
> }
> if (HAS_RC6p(dev)) {
> + calc_residency(dev, GEN6_GT_GFX_RC6p);
> + calc_residency(dev, GEN6_GT_GFX_RC6pp);
> ret = sysfs_merge_group(&dev->primary->kdev->kobj,
> &rc6p_attr_group);
> if (ret)
> DRM_ERROR("RC6p residency sysfs setup failed\n");
> }
> if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> + calc_residency(dev, VLV_GT_MEDIA_RC6);
> ret = sysfs_merge_group(&dev->primary->kdev->kobj,
> &media_rc6_attr_group);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index fbc1d215ca67..74b3468c3b1a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1247,6 +1247,70 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
> WARN_ON(dev_priv->uncore.fw_domains == 0);
> }
>
> +struct reg64emu {
> + i915_reg_t reg;
> + u64 last;
> + struct list_head link;
> +};
> +
> +static u64 __read64emu(struct drm_i915_private *dev_priv, struct reg64emu *emu)
> +{
> + u64 result;
> +
> + result = I915_READ(emu->reg) | (emu->last & 0xffffffff00000000);
> + if (lower_32_bits(result) < lower_32_bits(emu->last))
> + result += 1ull << 32;
> + emu->last = result;
> +
> + return result;
> +}
> +
> +u64 i915_read64emu(struct drm_i915_private *dev_priv, i915_reg_t reg)
> +{
> + struct reg64emu *emu;
> +
> + list_for_each_entry(emu, &dev_priv->uncore.reg64_emu_list, link)
> + if (i915_mmio_reg_offset(emu->reg) == i915_mmio_reg_offset(reg))
> + goto read;
> +
> + emu = kmalloc(sizeof(*emu), GFP_KERNEL);
> + if (emu == NULL)
> + return I915_READ(reg);
> +
> + emu->reg = reg;
> + emu->last = 0;
> + list_add(&emu->link, &dev_priv->uncore.reg64_emu_list);
> +
> + if (!timer_pending(&dev_priv->uncore.reg64_emu_timer))
> + mod_timer(&dev_priv->uncore.reg64_emu_timer,
> + jiffies + dev_priv->uncore.reg64_emu_timeout);
> +
> +read:
> + return __read64emu(dev_priv, emu);
> +}
> +
> +static void intel_uncore_reg64_emu_timer(unsigned long arg)
> +{
> + struct drm_i915_private *dev_priv = (struct drm_i915_private *)arg;
> + struct reg64emu *emu;
> +
> + list_for_each_entry(emu, &dev_priv->uncore.reg64_emu_list, link)
> + __read64emu(dev_priv, emu);
> +
> + mod_timer(&dev_priv->uncore.reg64_emu_timer,
> + jiffies + dev_priv->uncore.reg64_emu_timeout);
> +}
> +
> +static void intel_uncore_init_regemu64(struct drm_i915_private *dev_priv)
> +{
> + dev_priv->uncore.reg64_emu_timeout = MAX_JIFFY_OFFSET;
> +
> + INIT_LIST_HEAD(&dev_priv->uncore.reg64_emu_list);
> + setup_timer(&dev_priv->uncore.reg64_emu_timer,
> + intel_uncore_reg64_emu_timer,
> + (unsigned long)dev_priv);
> +}
> +
> void intel_uncore_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1306,16 +1370,32 @@ void intel_uncore_init(struct drm_device *dev)
> ASSIGN_READ_MMIO_VFUNCS(vgpu);
> }
>
> + intel_uncore_init_regemu64(dev_priv);
> +
> i915_check_and_clear_faults(dev);
> }
> #undef ASSIGN_WRITE_MMIO_VFUNCS
> #undef ASSIGN_READ_MMIO_VFUNCS
>
> +static void intel_uncore_fini_reg64emu(struct drm_device *dev)
> +{
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct reg64emu *emu, *en;
> +
> + list_for_each_entry_safe(emu, en, &i915->uncore.reg64_emu_list, link)
> + kfree(emu);
> +
> + INIT_LIST_HEAD(&i915->uncore.reg64_emu_list);
> + del_timer_sync(&i915->uncore.reg64_emu_timer);
> +}
> +
> void intel_uncore_fini(struct drm_device *dev)
> {
> /* Paranoia: make sure we have disabled everything before we exit. */
> intel_uncore_sanitize(dev);
> intel_uncore_forcewake_reset(dev, false);
> +
> + intel_uncore_fini_reg64emu(dev);
> }
>
> #define GEN_RANGE(l, h) GENMASK(h, l)
> --
> 2.8.0.rc3
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list