Re: [igt-dev] [PATCH] [i-g-t] tests/kms_cursor_crc: Fix test intermittent failures on AMD gpu

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 13.10.2023 14.38, Kamil Konieczny wrote:
> Hi Hersen,
> On 2023-10-02 at 10:20:23 -0400, Hersen Wu wrote:
>> Wait for two more vblanks before reading crc on AMD gpu.
>>
>> Without waiting for two vblanks, AMD cursor updates may not
>> synchronized to the same frame of pipe, crc generated may
>> not be reliable.
>>
>> Signed-off-by: Hersen Wu <hersenxs.wu@amd.com>
>> ---
>>   tests/kms_cursor_crc.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
>> index ba29ff65d..e3259e147 100644
>> --- a/tests/kms_cursor_crc.c
>> +++ b/tests/kms_cursor_crc.c
>> @@ -276,6 +276,15 @@ static void do_single_test(data_t *data, int x, int y, bool hw_test,
>>   		restore_image(data, swbufidx, &((cursorarea){x, y, data->curw, data->curh}));
>>   		igt_plane_set_fb(data->primary, &data->primary_fb[swbufidx]);
>>   		igt_display_commit(display);
>> +
>> +		/* Wait for two more vblanks since cursor updates may not
>> +		 * synchronized to the same frame on AMD HW
>> +		 */
>> +		if (is_amdgpu_device(data->drm_fd))
> ----------- ^^^^^^^^^^^^^^^^
> Why not
>          if(data->vblank_wait_count)
> 
> as for msm driver we also need to wait? Btw how it worked before
> for msm without that wait here? If you need to wait 2+2 maybe set
> it already to 4 and drop this hunk?
> 

As I understood before issue with msm is different. With msm it's about 
async cursor settling in place while on amd its about crc settling. 
Here's normal flip, nothing about cursor here, hence no point in waiting 
with msm. That comment above check for amd will need to be updated.

/Juha-Pekka

>> +			igt_wait_for_vblank_count(data->drm_fd,
>> +				display->pipes[data->pipe].crtc_offset,
>> +				data->vblank_wait_count);
>> +
>>   		igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc);
>>   		igt_assert_crc_equal(&crc, hwcrc);
>>   	}
>> @@ -1079,7 +1088,11 @@ igt_main_args("e", NULL, help_str, opt_handler, NULL)
>>   
>>   		igt_require_pipe_crc(data.drm_fd);
>>   
>> -		data.vblank_wait_count = is_msm_device(data.drm_fd) ? 2 : 1;
>> +		/* Wait for two more vblanks since cursor updates may not
>> +		 * synchronized to the same frame on AMD HW
>> +		 */
>> +		data.vblank_wait_count =
>> +			(is_msm_device(data.drm_fd) || is_amdgpu_device(data.drm_fd)) ? 2 : 1;
>>   	}
>>   
>>   	data.cursor_max_w = cursor_width;
>> -- 
>> 2.25.1
>>