Re: [Intel-gfx] [PATCH i-g-t 05/11] tests/perf: rework oa-exponent test

[Lionel Landwerlin]

On 22/08/17 17:13, Matthew Auld wrote:

On 22 August 2017 at 15:56, Lionel Landwerlin
 wrote:

On 10/08/17 14:15, Matthew Auld wrote:

On 4 August 2017 at 12:20, Lionel Landwerlin
 wrote:

New issues that were discovered while making the tests work on Gen8+ :

  - we need to measure timings between periodic reports and discard all
other kind of reports

  - it seems periodicity of the reports can be affected outside of RC6
(frequency change), we can detect this by looking at the amount of
clock cycles per timestamp deltas

I think this would be easier to review if we split this into two patches...

Also, somewhat worrying is that I've yet to see the oa-exponents test
pass on my BDW machine.

See here:
https://paste.fedoraproject.org/paste/SmOw7eHEGOTjrLsvPVcZOw/raw

Was your screen off?

I don't think so, same result with it on/off though. I'm guessing that
is passes for you then?

Here's the pastebin again, since the other one is now toast:
https://paste.fedoraproject.org/paste/tjSb4jPZ87sWjhj7qAD~UA/raw

Chris helped prove that this probably be power management preventing the 
unit to write its report to memory.
I don't think there is a way to tune that from userspace, so I guess 
I'll try to detect this. It's pretty reproducible on my system with 
screen off.

Maybe you're on battery only?

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

Re: [Intel-gfx] [PATCH i-g-t 05/11] tests/perf: rework oa-exponent test

[Matthew Auld]

On 22 August 2017 at 15:56, Lionel Landwerlin
 wrote:
> On 10/08/17 14:15, Matthew Auld wrote:
>
> On 4 August 2017 at 12:20, Lionel Landwerlin
>  wrote:
>
> New issues that were discovered while making the tests work on Gen8+ :
>
>  - we need to measure timings between periodic reports and discard all
>other kind of reports
>
>  - it seems periodicity of the reports can be affected outside of RC6
>(frequency change), we can detect this by looking at the amount of
>clock cycles per timestamp deltas
>
> I think this would be easier to review if we split this into two patches...
>
> Also, somewhat worrying is that I've yet to see the oa-exponents test
> pass on my BDW machine.
>
> See here:
> https://paste.fedoraproject.org/paste/SmOw7eHEGOTjrLsvPVcZOw/raw
>
> Was your screen off?

I don't think so, same result with it on/off though. I'm guessing that
is passes for you then?

Here's the pastebin again, since the other one is now toast:
https://paste.fedoraproject.org/paste/tjSb4jPZ87sWjhj7qAD~UA/raw
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 05/11] tests/perf: rework oa-exponent test

[Lionel Landwerlin]

On 10/08/17 14:15, Matthew Auld wrote:

On 4 August 2017 at 12:20, Lionel Landwerlin
  wrote:

New issues that were discovered while making the tests work on Gen8+ :

  - we need to measure timings between periodic reports and discard all
other kind of reports

  - it seems periodicity of the reports can be affected outside of RC6
(frequency change), we can detect this by looking at the amount of
clock cycles per timestamp deltas

I think this would be easier to review if we split this into two patches...

Also, somewhat worrying is that I've yet to see the oa-exponents test
pass on my BDW machine.

See here:
https://paste.fedoraproject.org/paste/SmOw7eHEGOTjrLsvPVcZOw/raw


Was your screen off?

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

Re: [Intel-gfx] [PATCH i-g-t 05/11] tests/perf: rework oa-exponent test

[Matthew Auld]

On 4 August 2017 at 12:20, Lionel Landwerlin
 wrote:
> New issues that were discovered while making the tests work on Gen8+ :
>
>  - we need to measure timings between periodic reports and discard all
>other kind of reports
>
>  - it seems periodicity of the reports can be affected outside of RC6
>(frequency change), we can detect this by looking at the amount of
>clock cycles per timestamp deltas

I think this would be easier to review if we split this into two patches...

Also, somewhat worrying is that I've yet to see the oa-exponents test
pass on my BDW machine.

See here:
https://paste.fedoraproject.org/paste/SmOw7eHEGOTjrLsvPVcZOw/raw

>
> Signed-off-by: Lionel Landwerlin 
> ---
>  tests/perf.c | 786 
> ---
>  1 file changed, 594 insertions(+), 192 deletions(-)
>
> diff --git a/tests/perf.c b/tests/perf.c
> index 7f9bc66d..5c7a2a34 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -300,6 +301,25 @@ static uint32_t (*read_report_ticks)(uint32_t *report,
>  static void (*sanity_check_reports)(uint32_t *oa_report0, uint32_t 
> *oa_report1,
> enum drm_i915_oa_format format);
>
> +static bool
> +timestamp_delta_within(uint32_t delta,
> +  uint32_t expected_delta,
> +  uint32_t margin)
> +{
> +   return delta >= (expected_delta - margin) &&
> +  delta <= (expected_delta + margin);
> +}
> +
> +static bool
> +double_value_within(double value,
> +   double expected,
> +   double percent_margin)
> +{
> +   return value >= (expected - expected * percent_margin / 100.0) &&
> +  value <= (expected + expected * percent_margin / 100.0);
> +
> +}
> +
>  static void
>  __perf_close(int fd)
>  {
> @@ -476,6 +496,20 @@ gen8_read_report_ticks(uint32_t *report, enum 
> drm_i915_oa_format format)
> return report[3];
>  }
>
> +static void
> +gen8_read_report_clock_ratios(uint32_t *report,
> + uint32_t *slice_freq_mhz,
> + uint32_t *unslice_freq_mhz)
> +{
> +   uint32_t unslice_freq = report[0] & 0x1ff;
> +   uint32_t slice_freq_low = (report[0] >> 25) & 0x7f;
> +   uint32_t slice_freq_high = (report[0] >> 9) & 0x3;
> +   uint32_t slice_freq = slice_freq_low | (slice_freq_high << 7);
> +
> +   *slice_freq_mhz = (slice_freq * 1) / 1000;
> +   *unslice_freq_mhz = (unslice_freq * 1) / 1000;
> +}
> +
>  static const char *
>  gen8_read_report_reason(const uint32_t *report)
>  {
> @@ -498,29 +532,6 @@ gen8_read_report_reason(const uint32_t *report)
> return "unknown";
>  }
>
> -static bool
> -oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report)
> -{
> -   if (IS_HASWELL(devid)) {
> -   /* For Haswell we don't have a documented report reason field
> -* (though empirically report[0] bit 10 does seem to correlate
> -* with a timer trigger reason) so we instead infer which
> -* reports are timer triggered by checking if the least
> -* significant bits are zero and the exponent bit is set.
> -*/
> -   uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1;
> -
> -   if ((report[1] & oa_exponent_mask) != (1 << oa_exponent))
> -   return true;
> -   } else {
> -   if ((report[0] >> OAREPORT_REASON_SHIFT) &
> -   OAREPORT_REASON_TIMER)
> -   return true;
> -   }
> -
> -   return false;
> -}
> -
>  static uint64_t
>  timebase_scale(uint32_t u32_delta)
>  {
> @@ -568,6 +579,29 @@ oa_exponent_to_ns(int exponent)
>  }
>
>  static bool
> +oa_report_is_periodic(uint32_t oa_exponent, const uint32_t *report)
> +{
> +   if (IS_HASWELL(devid)) {
> +   /* For Haswell we don't have a documented report reason field
> +* (though empirically report[0] bit 10 does seem to correlate
> +* with a timer trigger reason) so we instead infer which
> +* reports are timer triggered by checking if the least
> +* significant bits are zero and the exponent bit is set.
> +*/
> +   uint32_t oa_exponent_mask = (1 << (oa_exponent + 1)) - 1;
> +
> +   if ((report[1] & oa_exponent_mask) == (1 << oa_exponent))
> +   return true;
> +   } else {
> +   if ((report[0] >> OAREPORT_REASON_SHIFT) &
> +   OAREPORT_REASON_TIMER)
> +   return true;
> +   }
> +
> +   return false;
> +}
> +
> +static bool
>  oa_report_ctx_is_valid(uint32_t *report)
>  {
> if (IS_HASWELL(devid))