Re: [igt-dev] [PATCH i-g-t] tests/perf: adapt report timing delta acceptance for ICL

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 13/10/2018 00:32, Souza, Jose wrote:
> On Wed, 2018-10-10 at 18:49 +0100, Lionel Landwerlin wrote:
>> The behavior of the OA unit is a tiny bit different on ICL. It
>> appears
>> to be a bit sloppier on the timings of its OA reports (missing the
>> deadline by one period quite often). Let's add an acceptance delta.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   tests/perf.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/perf.c b/tests/perf.c
>> index 6a6001d5..a92de221 100644
>> --- a/tests/perf.c
>> +++ b/tests/perf.c
>> @@ -1674,6 +1674,16 @@ static void load_helper_fini(void)
>>   		drm_intel_bufmgr_destroy(lh.bufmgr);
>>   }
>>   
>> +static bool expected_report_timing_delta(uint32_t delta, uint32_t
>> expected_delta)
>> +{
>> +	/*
>> +	 * On ICL, the OA unit appears to be a bit more relaxed about
>> +	 * its timing for emitting OA reports (often missing the
>> +	 * deadline by 1 timestamp).
>> +	 */
> Maybe only relax for ICL?
>
>> +	return delta <= (expected_delta + 3);
>> +}
>> +
>>   static void
>>   test_oa_exponents(void)
>>   {
>> @@ -1774,9 +1784,9 @@ test_oa_exponents(void)
>>   			igt_debug("report%04i ts=%08x hw_id=0x%08x
>> delta=%u %s\n", i,
>>   				  timer_reports[i].report[1],
>>   				  oa_report_get_ctx_id(timer_reports[i]
>> .report),
>> -				  delta, delta ==
>> expected_timestamp_delta ? "" : "******");
>> +				  delta,
>> expected_report_timing_delta(delta,expected_timestamp_delta) ? "" :
>> "******");
>
> Nitpick: s/delta,
> expected_report_timing_delta(delta,expected_timestamp_delta) ? "" :
> "******");/delta, expected_report_timing_delta(delta,
> expected_timestamp_delta) ? "" : "******");
>
>>   
>> -			matches += expected_timestamp_delta <= delta;
>> +			matches +=
>> expected_report_timing_delta(delta,expected_timestamp_delta);
>>   		}
>>   
>>   		igt_debug("matches=%u/%u\n", matches, n_timer_reports -
>> 1);
>
> I tested the patch with gen11 rendercopy and it worked, big thanks for
> fixing it.
>
> With the minor fixes above:
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>
> I saw that you subimited the 'libs: Adding rendercopy support for
> gen11' separated and Kasia have reviewed but I think this one should be
> merged first, maybe submited the second version of this PR with both
> patches?

Thanks, pushed with the changes you requested.

-
Lionel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev