All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
@ 2016-04-07 11:24 Chris Wilson
  2016-04-07 12:13 ` Ville Syrjälä
  2016-04-07 15:29 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 9+ messages in thread
From: Chris Wilson @ 2016-04-07 11:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Len Brown

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.

Reported-by: Len Brown <lenb@kernel.org>
Refernces: https://bugs.freedesktop.org/show_bug.cgi?id=94852
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Len Brown <lenb@kernel.org>
Cc: Ville Syrjälä <ville.syrjala@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;
+	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

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 11:24 [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters Chris Wilson
@ 2016-04-07 12:13 ` Ville Syrjälä
  2016-04-07 12:37   ` Chris Wilson
  2016-04-07 15:29 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-07 12:13 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Len Brown

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@kernel.org>
> Refernces: https://bugs.freedesktop.org/show_bug.cgi?id=94852
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Ville Syrjälä <ville.syrjala@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 12:13 ` Ville Syrjälä
@ 2016-04-07 12:37   ` Chris Wilson
  2016-04-07 12:49     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-04-07 12:37 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 03:13:51PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 07, 2016 at 12:24:13PM +0100, Chris Wilson wrote:
> > +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.

This was the bit I was uncertain about. Does the high range bit imply
that is a 24-bit register? Or that the freq is measured differently?

Judging by calc_residency() and assuming that the counter wraps at
UINT32_MAX, this is still the right calcuations of when it does wrap
irrespective of that bit. Hopefully just needs a tweak to freq?

I hope we can just ignore the timer aspect of this patch, and if so then
handling the 64bit emulation ourselves is also pointless...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 12:37   ` Chris Wilson
@ 2016-04-07 12:49     ` Ville Syrjälä
  2016-04-07 12:59       ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-07 12:49 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 01:37:35PM +0100, Chris Wilson wrote:
> On Thu, Apr 07, 2016 at 03:13:51PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 07, 2016 at 12:24:13PM +0100, Chris Wilson wrote:
> > > +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.
> 
> This was the bit I was uncertain about. Does the high range bit imply
> that is a 24-bit register? Or that the freq is measured differently?

The hardware apparently has a 40bit counter internally. In low range
mode the register exposes bits [31:0] of the counter, in high range
you get to see bits [39:8].

> 
> Judging by calc_residency() and assuming that the counter wraps at
> UINT32_MAX, this is still the right calcuations of when it does wrap
> irrespective of that bit. Hopefully just needs a tweak to freq?
> 
> I hope we can just ignore the timer aspect of this patch, and if so then
> handling the 64bit emulation ourselves is also pointless...
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 12:49     ` Ville Syrjälä
@ 2016-04-07 12:59       ` Chris Wilson
  2016-04-07 13:18         ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-04-07 12:59 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 03:49:01PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 07, 2016 at 01:37:35PM +0100, Chris Wilson wrote:
> > On Thu, Apr 07, 2016 at 03:13:51PM +0300, Ville Syrjälä wrote:
> > > On Thu, Apr 07, 2016 at 12:24:13PM +0100, Chris Wilson wrote:
> > > > +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.
> > 
> > This was the bit I was uncertain about. Does the high range bit imply
> > that is a 24-bit register? Or that the freq is measured differently?
> 
> The hardware apparently has a 40bit counter internally. In low range
> mode the register exposes bits [31:0] of the counter, in high range
> you get to see bits [39:8].

In that case the frequency would be reduced by >>8.

Can we set that bit ourselves? That puts the overflow into the 1 hour
mark. Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 12:59       ` Chris Wilson
@ 2016-04-07 13:18         ` Ville Syrjälä
  2016-04-07 13:58           ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-07 13:18 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 01:59:44PM +0100, Chris Wilson wrote:
> On Thu, Apr 07, 2016 at 03:49:01PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 07, 2016 at 01:37:35PM +0100, Chris Wilson wrote:
> > > On Thu, Apr 07, 2016 at 03:13:51PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Apr 07, 2016 at 12:24:13PM +0100, Chris Wilson wrote:
> > > > > +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.
> > > 
> > > This was the bit I was uncertain about. Does the high range bit imply
> > > that is a 24-bit register? Or that the freq is measured differently?
> > 
> > The hardware apparently has a 40bit counter internally. In low range
> > mode the register exposes bits [31:0] of the counter, in high range
> > you get to see bits [39:8].
> 
> In that case the frequency would be reduced by >>8.
> 
> Can we set that bit ourselves? That puts the overflow into the 1 hour
> mark. Thanks,

I don't know if it's safe to frob the bit. I worry that something
outside our control might depend on it staying put.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 13:18         ` Ville Syrjälä
@ 2016-04-07 13:58           ` Chris Wilson
  2016-04-07 14:17             ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-04-07 13:58 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 04:18:16PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 07, 2016 at 01:59:44PM +0100, Chris Wilson wrote:
> > Can we set that bit ourselves? That puts the overflow into the 1 hour
> > mark. Thanks,
> 
> I don't know if it's safe to frob the bit. I worry that something
> outside our control might depend on it staying put.

A quick frob of the bit says that it is RO. When I try to set it, it
doesn't stick. :(
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 13:58           ` Chris Wilson
@ 2016-04-07 14:17             ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-07 14:17 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Len Brown

On Thu, Apr 07, 2016 at 02:58:01PM +0100, Chris Wilson wrote:
> On Thu, Apr 07, 2016 at 04:18:16PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 07, 2016 at 01:59:44PM +0100, Chris Wilson wrote:
> > > Can we set that bit ourselves? That puts the overflow into the 1 hour
> > > mark. Thanks,
> > 
> > I don't know if it's safe to frob the bit. I worry that something
> > outside our control might depend on it staying put.
> 
> A quick frob of the bit says that it is RO. When I try to set it, it
> doesn't stick. :(

Same here on my VLV. I was able to toggle it on my BSW. Perhaps
something can lock it down, and my BSW BIOS just doesn't do that.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.BAT: failure for RFC drm/i915: Emulate 64bit registers for residency counters
  2016-04-07 11:24 [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters Chris Wilson
  2016-04-07 12:13 ` Ville Syrjälä
@ 2016-04-07 15:29 ` Patchwork
  1 sibling, 0 replies; 9+ messages in thread
From: Patchwork @ 2016-04-07 15:29 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: RFC drm/i915: Emulate 64bit registers for residency counters
URL   : https://patchwork.freedesktop.org/series/5414/
State : failure

== Summary ==

Series 5414v1 RFC drm/i915: Emulate 64bit registers for residency counters
http://patchwork.freedesktop.org/api/1.0/series/5414/revisions/1/mbox/

Test drv_getparams_basic:
        Subgroup basic-subslice-total:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test drv_module_reload_basic:
                incomplete -> PASS       (bsw-nuc-2)
Test gem_basic:
        Subgroup bad-close:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup create-close:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup create-fd-close:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_ctx_param_basic:
        Subgroup basic:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup invalid-ctx-set:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup invalid-param-get:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup invalid-param-set:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup root-set:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test gem_ctx_switch:
        Subgroup basic-default:
                skip       -> PASS       (bsw-nuc-2)
Test gem_exec_basic:
        Subgroup basic-blt:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup gtt-bsd:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup readonly-bsd:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup readonly-render:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup readonly-vebox:
                skip       -> PASS       (bsw-nuc-2)
Test gem_exec_store:
        Subgroup basic-bsd:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup basic-default:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup basic-vebox:
                skip       -> PASS       (bsw-nuc-2)
Test gem_exec_whisper:
        Subgroup basic:
                skip       -> PASS       (bsw-nuc-2)
Test gem_mmap_gtt:
        Subgroup basic-write:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-write-gtt:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-write-read:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_ringfill:
        Subgroup basic-default-hang:
                skip       -> PASS       (bsw-nuc-2)
Test gem_sync:
        Subgroup basic-blt:
                skip       -> PASS       (bsw-nuc-2)
        Subgroup basic-vebox:
                skip       -> PASS       (bsw-nuc-2)
Test gem_tiled_pread_basic:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_addfb_basic:
        Subgroup addfb25-modifier-no-flag:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup addfb25-y-tiled:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup bad-pitch-65536:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-x-tiled:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup framebuffer-vs-set-tiling:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup too-wide:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-fail -> PASS       (bsw-nuc-2)
Test kms_force_connector_basic:
        Subgroup force-edid:
                skip       -> PASS       (ivb-t430s)
        Subgroup prune-stale-modes:
                pass       -> SKIP       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c-frame-sequence:
                dmesg-fail -> PASS       (bsw-nuc-2)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test prime_self_import:
        Subgroup basic-with_fd_dup:
                dmesg-warn -> PASS       (bsw-nuc-2)
        Subgroup basic-with_one_bo_two_files:
                dmesg-warn -> PASS       (bsw-nuc-2)

bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:150  dwarn:8   dfail:1   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:196  pass:179  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:196  pass:131  dwarn:0   dfail:0   fail:0   skip:65 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:196  pass:185  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:196  pass:162  dwarn:0   dfail:0   fail:0   skip:34 
BOOT FAILED for snb-x220t

Results at /archive/results/CI_IGT_test/Patchwork_1829/

851708c7e97537ed618fadbe5d342eaf8fa5146d drm-intel-nightly: 2016y-04m-07d-13h-56m-00s UTC integration manifest
e84c6d3b5d7d02bc64530a14ee05cf23aef5d9ac RFC drm/i915: Emulate 64bit registers for residency counters

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-04-07 15:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07 11:24 [PATCH] RFC drm/i915: Emulate 64bit registers for residency counters Chris Wilson
2016-04-07 12:13 ` Ville Syrjälä
2016-04-07 12:37   ` Chris Wilson
2016-04-07 12:49     ` Ville Syrjälä
2016-04-07 12:59       ` Chris Wilson
2016-04-07 13:18         ` Ville Syrjälä
2016-04-07 13:58           ` Chris Wilson
2016-04-07 14:17             ` Ville Syrjälä
2016-04-07 15:29 ` ✗ Fi.CI.BAT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.