| Message ID | 1488477700-30550-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > We were passively acting on the high counter value bit > and as it was never set, we were only utilizing the > the 32bits of resolution. As the divisor with these platforms > is quite high, the wrap around happened in the less than 13 seconds. > > If we toggle the resolution bit in the control register and > read twice we can get 8 bits more, bringing the wrap in > 54 minute range. > > Reported-by: Len Brown <len.brown@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852 > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Len Brown <len.brown@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index af0ac9f..807d7be 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, > > /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - units = 1; > + u32 lower, upper; > div = dev_priv->czclk_freq; > > - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) > - units <<= 8; > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); > + upper = I915_READ(reg); > + > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); > + lower = I915_READ(reg); Is this true??? There is an internal 40bit counter to which we have a 32bit window (i.e. changing the counter control doesn't reset the count). Note that you should recheck the high range afterwards to guard against constructing an erroneous value across wraparound. And this should also leave it set to high for other users on vlv. -Chris
On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > We were passively acting on the high counter value bit > and as it was never set, we were only utilizing the > the 32bits of resolution. As the divisor with these platforms > is quite high, the wrap around happened in the less than 13 seconds. > > If we toggle the resolution bit in the control register and Can't be done on all machines. IIRC both Chris and me tried this at some point and on some machines the register was locked. Also I'm not sure if some piece of firmware depends on the original setting. > read twice we can get 8 bits more, bringing the wrap in > 54 minute range. > > Reported-by: Len Brown <len.brown@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Len Brown <len.brown@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index af0ac9f..807d7be 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, > > /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - units = 1; > + u32 lower, upper; > div = dev_priv->czclk_freq; > > - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) > - units <<= 8; > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); > + upper = I915_READ(reg); > + > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); > + lower = I915_READ(reg); > + > + raw_time = lower | (u64)upper << 8; > + goto out; > } else if (IS_GEN9_LP(dev_priv)) { > units = 1; > div = 1200; /* 833.33ns */ > } > > raw_time = I915_READ(reg) * units; > + out: > ret = DIV_ROUND_UP_ULL(raw_time, div); > > intel_runtime_pm_put(dev_priv); > -- > 2.7.4
Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: >> We were passively acting on the high counter value bit >> and as it was never set, we were only utilizing the >> the 32bits of resolution. As the divisor with these platforms >> is quite high, the wrap around happened in the less than 13 seconds. >> >> If we toggle the resolution bit in the control register and > > Can't be done on all machines. IIRC both Chris and me tried this at > some point and on some machines the register was locked. Also I'm > not sure if some piece of firmware depends on the original setting. > With J1900 byt it seems to work. -Mika >> read twice we can get 8 bits more, bringing the wrap in >> 54 minute range. >> >> Reported-by: Len Brown <len.brown@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Len Brown <len.brown@intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c >> index af0ac9f..807d7be 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, >> >> /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ >> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { >> - units = 1; >> + u32 lower, upper; >> div = dev_priv->czclk_freq; >> >> - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) >> - units <<= 8; >> + I915_WRITE(VLV_COUNTER_CONTROL, >> + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); >> + upper = I915_READ(reg); >> + >> + I915_WRITE(VLV_COUNTER_CONTROL, >> + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); >> + lower = I915_READ(reg); >> + >> + raw_time = lower | (u64)upper << 8; >> + goto out; >> } else if (IS_GEN9_LP(dev_priv)) { >> units = 1; >> div = 1200; /* 833.33ns */ >> } >> >> raw_time = I915_READ(reg) * units; >> + out: >> ret = DIV_ROUND_UP_ULL(raw_time, div); >> >> intel_runtime_pm_put(dev_priv); >> -- >> 2.7.4 > > -- > Ville Syrjälä > Intel OTC
On Thu, Mar 02, 2017 at 06:13:15PM +0000, Chris Wilson wrote: > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > > We were passively acting on the high counter value bit > > and as it was never set, we were only utilizing the > > the 32bits of resolution. As the divisor with these platforms > > is quite high, the wrap around happened in the less than 13 seconds. > > > > If we toggle the resolution bit in the control register and > > read twice we can get 8 bits more, bringing the wrap in > > 54 minute range. > > > > Reported-by: Len Brown <len.brown@intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852 Hah, that was Len complaining about the wrap at 91 minutes. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: >> We were passively acting on the high counter value bit >> and as it was never set, we were only utilizing the >> the 32bits of resolution. As the divisor with these platforms >> is quite high, the wrap around happened in the less than 13 seconds. >> >> If we toggle the resolution bit in the control register and >> read twice we can get 8 bits more, bringing the wrap in >> 54 minute range. >> >> Reported-by: Len Brown <len.brown@intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852 >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Len Brown <len.brown@intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c >> index af0ac9f..807d7be 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, >> >> /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ >> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { >> - units = 1; >> + u32 lower, upper; >> div = dev_priv->czclk_freq; >> >> - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) >> - units <<= 8; >> + I915_WRITE(VLV_COUNTER_CONTROL, >> + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); >> + upper = I915_READ(reg); >> + >> + I915_WRITE(VLV_COUNTER_CONTROL, >> + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); >> + lower = I915_READ(reg); > > Is this true??? There is an internal 40bit counter to which we have a > 32bit window (i.e. changing the counter control doesn't reset the > count). Yes, evidence suggests so with prod byt. But I am not sure if same holds true with chv. > > Note that you should recheck the high range afterwards to guard against > constructing an erroneous value across wraparound. And this should also > leave it set to high for other users on vlv. Other user being debugs? Perhaps u64 utility function to read it from both places then. -Mika > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Thu, Mar 02, 2017 at 08:13:31PM +0200, Ville Syrjälä wrote: > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > > We were passively acting on the high counter value bit > > and as it was never set, we were only utilizing the > > the 32bits of resolution. As the divisor with these platforms > > is quite high, the wrap around happened in the less than 13 seconds. > > > > If we toggle the resolution bit in the control register and > > Can't be done on all machines. IIRC both Chris and me tried this at > some point and on some machines the register was locked. Also I'm > not sure if some piece of firmware depends on the original setting. > Ville Syrjälä wrote: > 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. Bah. Humbug. -Chris
On Thu, Mar 02, 2017 at 08:20:24PM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > >> We were passively acting on the high counter value bit > >> and as it was never set, we were only utilizing the > >> the 32bits of resolution. As the divisor with these platforms > >> is quite high, the wrap around happened in the less than 13 seconds. > >> > >> If we toggle the resolution bit in the control register and > >> read twice we can get 8 bits more, bringing the wrap in > >> 54 minute range. > >> > >> Reported-by: Len Brown <len.brown@intel.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94852 > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Len Brown <len.brown@intel.com> > >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- > >> 1 file changed, 12 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > >> index af0ac9f..807d7be 100644 > >> --- a/drivers/gpu/drm/i915/i915_sysfs.c > >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c > >> @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, > >> > >> /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ > >> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > >> - units = 1; > >> + u32 lower, upper; > >> div = dev_priv->czclk_freq; > >> > >> - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) > >> - units <<= 8; > >> + I915_WRITE(VLV_COUNTER_CONTROL, > >> + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); > >> + upper = I915_READ(reg); > >> + > >> + I915_WRITE(VLV_COUNTER_CONTROL, > >> + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); > >> + lower = I915_READ(reg); > > > > Is this true??? There is an internal 40bit counter to which we have a > > 32bit window (i.e. changing the counter control doesn't reset the > > count). > > Yes, evidence suggests so with prod byt. But I am not sure if same > holds true with chv. And checked it's not returning the same value on both reads? > > Note that you should recheck the high range afterwards to guard against > > constructing an erroneous value across wraparound. And this should also > > leave it set to high for other users on vlv. > > Other user being debugs? RPS. (And I've a pmu that wants it.) > Perhaps u64 utility function to read it from both places then. A definite possibility. -Chris
On Thu, Mar 02, 2017 at 06:21:37PM +0000, Chris Wilson wrote: > On Thu, Mar 02, 2017 at 08:13:31PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > > > We were passively acting on the high counter value bit > > > and as it was never set, we were only utilizing the > > > the 32bits of resolution. As the divisor with these platforms > > > is quite high, the wrap around happened in the less than 13 seconds. > > > > > > If we toggle the resolution bit in the control register and > > > > Can't be done on all machines. IIRC both Chris and me tried this at > > some point and on some machines the register was locked. Also I'm > > not sure if some piece of firmware depends on the original setting. > > > Ville Syrjälä wrote: > > 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. > > Bah. Humbug. Hmm. Actually now that I tried it again it seems to stick at least on one BSW and two VLV machines. I'm not 100% those VLV machines are what I tried before, but the third one I have has died so I can't re-test it. The other explanation for the failure I saw earlier could be that I forgot to set the mask bit. Interestingly the mask bit reads as 0 on VLV and as 1 on CHV, but when writing both need it to be set (as is expected for a mask bit). BTW I found this note in the Gunit docs: "Each of these counters will start counting on reset and continue counting when that particular event happens. There will be a 40-bit counter. 0x13_8104[15] selects if the lower 32 bits or the upper 32 bits of the 40-bit counter are read out. 0x13_8104[7:0] will have individual enables for each of the above counters" Hmm. It also looks like we're explicitly enabling the high range mode on CHV in rps init. But on VLV we don't touch that bit. I can't see this register being mentioned in the BIOS spec, so I'm not sure who came up with this code. Oh, actually it seems we were enabling the high range bit on VLV too until commit 31685c258e0b ("drm/i915/vlv: WA for Turbo and RC6 to work together.") So all this leads to me to think that maybe it's safe to frob this register after all. If it were super important for turbo/rc6 I would have expected to see it in the BIOS spec.
Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > On Thu, Mar 02, 2017 at 06:21:37PM +0000, Chris Wilson wrote: >> On Thu, Mar 02, 2017 at 08:13:31PM +0200, Ville Syrjälä wrote: >> > On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: >> > > We were passively acting on the high counter value bit >> > > and as it was never set, we were only utilizing the >> > > the 32bits of resolution. As the divisor with these platforms >> > > is quite high, the wrap around happened in the less than 13 seconds. >> > > >> > > If we toggle the resolution bit in the control register and >> > >> > Can't be done on all machines. IIRC both Chris and me tried this at >> > some point and on some machines the register was locked. Also I'm >> > not sure if some piece of firmware depends on the original setting. >> >> > Ville Syrjälä wrote: >> > 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. >> >> Bah. Humbug. > > Hmm. Actually now that I tried it again it seems to stick at least on > one BSW and two VLV machines. I'm not 100% those VLV machines are what I > tried before, but the third one I have has died so I can't re-test it. > > The other explanation for the failure I saw earlier could be that I > forgot to set the mask bit. Interestingly the mask bit reads as 0 on > VLV and as 1 on CHV, but when writing both need it to be set (as is > expected for a mask bit). I falled for this trap atleast on the first try. The bspec nomenclature on the page was somewhat nonstandard, mask bits misleadingly 'RO' so my brain skipped those at first read... -Mika > > BTW I found this note in the Gunit docs: > "Each of these counters will start counting on reset and continue > counting when that particular event happens. There will be a 40-bit > counter. 0x13_8104[15] selects if the lower 32 bits or the upper > 32 bits of the 40-bit counter are read out. 0x13_8104[7:0] will > have individual enables for each of the above counters" > > Hmm. It also looks like we're explicitly enabling the high range mode on > CHV in rps init. But on VLV we don't touch that bit. I can't see this > register being mentioned in the BIOS spec, so I'm not sure who came up > with this code. Oh, actually it seems we were enabling the high range > bit on VLV too until commit 31685c258e0b ("drm/i915/vlv: WA for Turbo > and RC6 to work together.") > > So all this leads to me to think that maybe it's safe to frob this > register after all. If it were super important for turbo/rc6 I would > have expected to see it in the BIOS spec. > > -- > Ville Syrjälä > Intel OTC
On Thu, Mar 02, 2017 at 08:01:40PM +0200, Mika Kuoppala wrote: > We were passively acting on the high counter value bit > and as it was never set, we were only utilizing the > the 32bits of resolution. As the divisor with these platforms > is quite high, the wrap around happened in the less than 13 seconds. > > If we toggle the resolution bit in the control register and > read twice we can get 8 bits more, bringing the wrap in > 54 minute range. > > Reported-by: Len Brown <len.brown@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Len Brown <len.brown@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index af0ac9f..807d7be 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, > > /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - units = 1; > + u32 lower, upper; > div = dev_priv->czclk_freq; > > - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) > - units <<= 8; > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); > + upper = I915_READ(reg); > + > + I915_WRITE(VLV_COUNTER_CONTROL, > + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); > + lower = I915_READ(reg); u64 vlv_read_counter(struct drm_i915_private *dev_priv, i915_reg_t reg) { u32 upper, lower, tmp, saved_ctl; unsigned int fw_domains; fw_domains = intel_uncore_forcewake_for_reg(dev_priv, reg, FW_REG_READ); /* Lock out others from using VLV_COUNTER_CONTROL */ spin_lock_irq(&dev_priv->uncore.lock); intel_uncore_forcewake_get__locked(dev_priv); saved_ctl = I915_READ_FW(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH; I915_WRITE_FW(VLV_COUNTER_CONTROL, MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); tmp = I915_READ_FW(reg); do { upper = tmp; I915_WRITE_FW(VLV_COUNTER_CONTROL, _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); lower = I915_READ(reg); I915_WRITE_FW(VLV_COUNTER_CONTROL, MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); tmp = I915_READ_FW(reg); } while (tmp != upper); I915_WRITE_FW(VLV_COUNTER_CONTROL, VLV_COUNT_RANGE_HIGH << 16 | save_ctl); intel_uncore_forcewake_put__locked(dev_priv); spin_lock_irq(&dev_priv->uncore.lock); return (u64)upper << 8 | lower; }
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index af0ac9f..807d7be 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -53,17 +53,26 @@ static u32 calc_residency(struct drm_i915_private *dev_priv, /* On VLV and CHV, residency time is in CZ units rather than 1.28us */ if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { - units = 1; + u32 lower, upper; div = dev_priv->czclk_freq; - if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH) - units <<= 8; + I915_WRITE(VLV_COUNTER_CONTROL, + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH)); + upper = I915_READ(reg); + + I915_WRITE(VLV_COUNTER_CONTROL, + _MASKED_BIT_DISABLE(VLV_COUNT_RANGE_HIGH)); + lower = I915_READ(reg); + + raw_time = lower | (u64)upper << 8; + goto out; } else if (IS_GEN9_LP(dev_priv)) { units = 1; div = 1200; /* 833.33ns */ } raw_time = I915_READ(reg) * units; + out: ret = DIV_ROUND_UP_ULL(raw_time, div); intel_runtime_pm_put(dev_priv);
We were passively acting on the high counter value bit and as it was never set, we were only utilizing the the 32bits of resolution. As the divisor with these platforms is quite high, the wrap around happened in the less than 13 seconds. If we toggle the resolution bit in the control register and read twice we can get 8 bits more, bringing the wrap in 54 minute range. Reported-by: Len Brown <len.brown@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Len Brown <len.brown@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_sysfs.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)