Re: [PATCH i-g-t v2 1/1] tests/kms_cursor_legacy: relax checks on MTK

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

Hi Xiandong,

On Tue, 2026-03-17 at 16:40 +0800, Xiandong Wang wrote:
> From: Xiandong Wang <Xiandong.Wang@mediatek.com>
> 
> kms_cursor_legacy uses tight vblank timing assertions that can be
> unstable
> on MTK under higher system load, leading to intermittent failures.
> 
> Use relaxed upper-bound checks for the MTK-specific path in the
> affected
> subtests, while preserving the existing assertions for other drivers.
> 
> This improves test robustness on MTK without changing expectations on
> non-MTK platforms.
> 
> Signed-off-by: Xiandong Wang <Xiandong.Wang@mediatek.com>
> ---
>  tests/kms_cursor_legacy.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 5399ef481..e6ab33eca 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -1038,12 +1038,17 @@ static void flip_vs_cursor(igt_display_t
> *display, enum flip_test mode, int nloo
>  		igt_set_timeout(1, "Stuck page flip");
>  		igt_ignore_warn(read(display->drm_fd, &vbl,
> sizeof(vbl)));
>  
> -		if (!mode_requires_extra_vblank(mode))
> -			igt_assert_eq(igt_crtc_get_vblank(crtc, 0),
> -				      vblank_start + 1);
> -		else
> +		if (!mode_requires_extra_vblank(mode)) {
> +			if (is_mtk_device(display->drm_fd))
> +				igt_assert_lte(igt_crtc_get_vblank(c
> rtc, 0),
> +					vblank_start + 1);

It seems you got a igt_crtc_get_vblank(crtc, 0) == vblank_start here.

Since changing from igt_assert_eq to igt_assert_lte is a relaxation of
the assertion condition and igt_crtc_get_vblank(crtc, 0) won't less
than vblank_start. It would not cause any regressions on other
platform. Therefore, there is no need to guard this relaxation
specifically for MTK devices.

> +			else
> +				igt_assert_eq(igt_crtc_get_vblank(cr
> tc, 0),
> +					vblank_start + 1);
> +		} else {
>  			igt_assert_lte(igt_crtc_get_vblank(crtc, 0),
>  				       vblank_start + 2);
> +		}

I think it could be simply replaced to:

if (!mode_requires_extra_vblank(mode))
	igt_assert_lte(igt_crtc_get_vblank(crtc, 0),
					   vblank_start + 1);
else
	igt_assert_lte(igt_crtc_get_vblank(crtc, 0),
 					   vblank_start + 2);

>  
>  		igt_reset_timeout();
>  	} while (nloops--);
> @@ -1442,8 +1447,12 @@ static void cursor_vs_flip(igt_display_t
> *display, enum flip_test mode, int nloo
>  			vblank_last = vbl.sequence;
>  		}
>  
> -		if (!cursor_slowpath(display, mode))
> -			igt_assert_lte(vbl.sequence, vblank_start +
> 5 * vrefresh / 8);
> +		if (!cursor_slowpath(display, mode)) {
> +			if (is_mtk_device(display->drm_fd))
> +				igt_assert_lte(vbl.sequence,
> vblank_start + ((5 * vrefresh / 8) * 4));
> +			else
> +				igt_assert_lte(vbl.sequence,
> vblank_start + 5 * vrefresh / 8);
> +		}

It's about the threshold. Maybe you can do something like this:

threshold = 5 * vrefresh / 8;

/* Add some comment for why we need this... */
if (is_mtk_device(display->drm_fd))
	threshold = 4 * threshold;

if (!cursor_slowpath(display, mode))
	igt_assert_lte(vbl.sequence, vblank_start + threshold);

Regards,
Jason-JH Lin