[PATCH] drm/i915: Extend residency counter ranges on chv and byt

[PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Mika Kuoppala]

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);
+
+		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

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

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Chris Wilson]

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

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

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Ville Syrjälä]

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ä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Mika Kuoppala]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Chris Wilson]

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, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Mika Kuoppala]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Chris Wilson]

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

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

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Chris Wilson]

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

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

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Ville Syrjälä]

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ä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Extend residency counter ranges on chv and byt

[Patchwork]

== Series Details ==

Series: drm/i915: Extend residency counter ranges on chv and byt
URL   : https://patchwork.freedesktop.org/series/20578/
State : success

== Summary ==

Series 20578v1 drm/i915: Extend residency counter ranges on chv and byt
https://patchwork.freedesktop.org/api/1.0/series/20578/revisions/1/mbox/

Test gem_busy:
        Subgroup basic-hang-default:
                fail       -> PASS       (fi-hsw-4770r)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

06dbfb1f6e3af2f332189d566a9f2f1227acda76 drm-tip: 2017y-03m-02d-18h-25m-10s UTC integration manifest
adb3a6c drm/i915: Extend residency counter ranges on chv and byt

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4043/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Mika Kuoppala]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Extend residency counter ranges on chv and byt

[Chris Wilson]

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;
}

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