From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by gabe.freedesktop.org (Postfix) with ESMTPS id 95A0A10E0E7 for ; Fri, 13 Oct 2023 11:49:33 +0000 (UTC) Received: by mail-wr1-x42c.google.com with SMTP id ffacd0b85a97d-3231dff4343so1288537f8f.0 for ; Fri, 13 Oct 2023 04:49:33 -0700 (PDT) Message-ID: <046b5eeb-215c-b100-9674-a19b8fe25f9e@gmail.com> Date: Fri, 13 Oct 2023 14:49:22 +0300 MIME-Version: 1.0 Content-Language: en-US To: Kamil Konieczny , igt-dev@lists.freedesktop.org, Hersen Wu , rodrigo.siqueira@amd.com, aurabindo.pillai@amd.com, alex.hung@amd.com, hamza.mahfooz@amd.com, sunpeng.li@amd.com, markyacoub@google.com References: <20231002142023.713415-1-hersenxs.wu@amd.com> <20231013113841.taitcycfsxxzgzyn@kamilkon-desk.igk.intel.com> From: Juha-Pekka Heikkila In-Reply-To: <20231013113841.taitcycfsxxzgzyn@kamilkon-desk.igk.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [igt-dev] [PATCH] [i-g-t] tests/kms_cursor_crc: Fix test intermittent failures on AMD gpu List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: juhapekka.heikkila@gmail.com Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: 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 >> --- >> 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 >>