[igt-dev] [PATCH i-g-t] igt: tests: kms_vblank: Stop using magic value

[Martin Peres]

On 02/04/2019 16:00, Lisovskiy, Stanislav wrote:
> On Tue, 2019-03-19 at 15:46 +0200, Stanislav Lisovskiy wrote:
>> Lot of tests fail, when assertion checking
>> how many vblanks should have passed during
>> suspend/resume is compared to magic value 150.
>>
>> At the same time even for failed cases, sometimes
>> it is clearly visible that there were no issue -
>> simply suspend took longer on that machine.
>> If suspend took around 10 s and we get roughly
>> 60 vblanks per second the value then should be
>> around 600 and not 150.
>>
>> This change removes 150 magic value and starts
>> to use calculation of what it is expected to
>> be instead of being hardcoded.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104894
>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
>> ---
>>  tests/kms_vblank.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> Martin, could you please comment, what was I supposed to fix
> here, based on our offline discussion, as I seem to have forgotten
> this already..

Pretty sure what I was asking was to make a window of acceptable vblank
values, for example [expected-2, expected+2].

The reason why we can't be sure what value this will be is because we
don't know how early vblank gets disabled compared to when the monotonic
counter gets disabled. This is especially true in the presence of
multiple monitors (no idea if we first wait for vblank on all monitors
before suspend or not).

I hope this is what you wanted to hear!
> 
>>
>> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
>> index dafadb58..80b2a23a 100644
>> --- a/tests/kms_vblank.c
>> +++ b/tests/kms_vblank.c
>> @@ -338,8 +338,13 @@ static void vblank_ts_cont(data_t *data, int fd,
>> int nchildren)
>>  	igt_output_t *output = data->output;
>>  	int seq1, seq2;
>>  	union drm_wait_vblank vbl;
>> +	struct timespec start, end;
>> +	int estimated_vblanks = 0;
>> +	int vrefresh = igt_output_get_mode(output)->vrefresh;
>> +	double time_elapsed;
>>  
>>  	seq1 = get_vblank(fd, data->pipe, 0);
>> +	clock_gettime(CLOCK_MONOTONIC, &start);
>>  
>>  	if (data->flags & DPMS) {
>>  		igt_output_set_prop_value(output, IGT_CONNECTOR_DPMS,
>> DRM_MODE_DPMS_OFF);
>> @@ -377,11 +382,17 @@ static void vblank_ts_cont(data_t *data, int
>> fd, int nchildren)
>>  	}
>>  
>>  	seq2 = get_vblank(fd, data->pipe, 0);
>> +	clock_gettime(CLOCK_MONOTONIC, &end);
>> +
>> +	time_elapsed = igt_time_elapsed(&start, &end);
>> +	estimated_vblanks = (int)(time_elapsed * vrefresh) + 1;
>>  
>>  	igt_debug("testing ts continuity: Current frame %u, old frame
>> %u\n", seq2, seq1);
>>  
>> -	igt_assert_f(seq2 - seq1 >= 0, "unexpected vblank seq %u,
>> should be >= %u\n", seq2, seq1);
>> -	igt_assert_f(seq2 - seq1 <= 150, "unexpected vblank seq %u,
>> should be < %u\n", seq2, seq1 + 150);
>> +	igt_assert_f(seq2 - seq1 >= 0, "elapsed %f(%d vblanks)
>> unexpected vblank seq %u, should be >= %u\n", time_elapsed,
>> +			estimated_vblanks, seq2, seq1);
>> +	igt_assert_f(seq2 - seq1 <= estimated_vblanks, "elapsed %f(%d
>> vblanks) unexpected vblank seq %u, should be < %u\n", time_elapsed,
>> +			estimated_vblanks, seq2, seq1 +
>> estimated_vblanks);
>>  }
>>  
>>  static void run_subtests_for_pipe(data_t *data)
>