Re: [PATCH i-g-t v2] tests/kms_plane: Remove redundant CRC frame sequence check

Hi Jason-JH,

On 8/11/2026 9:44 PM, Jason-JH Lin wrote:
> The capture_crc() function validated that the CRC frame sequence
> returned by igt_pipe_crc_get_for_frame() matches the expected value.
> However, igt_pipe_crc_get_for_frame() already guarantees
> crc->frame >= expected via its internal loop:
>
>      do {
>          read_one_crc(pipe_crc, crc);
>      } while (igt_vblank_before(crc->frame, vblank));
This isn't fully true IMHO. The capture CRC function actually ensured 
the exact match of frame sequence and with this patch we're just 
guaranteeing '>='.
So we need more context here from: 
https://patchwork.freedesktop.org/series/168037/
>
> The additional check in capture_crc() is therefore redundant.
> Remove it and rely on the library's existing guarantee.
>
> Signed-off-by: Jason-JH Lin <[email protected]>
> ---
>   tests/kms_plane.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 12dfbfe1d82b..fe8ee2ab26ab 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -765,11 +765,6 @@ static int num_unique_crcs(const igt_crc_t crc[], int num_crc)
>   static void capture_crc(data_t *data, unsigned int vblank, igt_crc_t *crc)
>   {
>   	igt_pipe_crc_get_for_frame(data->drm_fd, data->pipe_crc, vblank, crc);

Also if this is only igt_pipe_crc_get_for_frame now, ideally we can just 
remove this function itself and call the helper directly. But before 
doing that, as the existing assert was added by a patch from Ville and 
rb'ed by JP, I'll request an ack from them or if they have any inputs on 
this.

Regards,
Karthik.B.S
> -
> -	igt_fail_on_f(!igt_skip_crc_compare && !igt_run_in_simulation() &&
> -		      crc->has_valid_frame && crc->frame != vblank,
> -		      "Got CRC for the wrong frame (got %u, expected %u). CRC buffer overflow?\n",
> -		      crc->frame, vblank);
>   }
>   
>   static void capture_format_crcs_single(data_t *data, igt_crtc_t *crtc,