Re: [PATCH i-g-t 4/4] tests/kms_color_pipeline: collect reference CRC once per pipe

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Chaitanya

On 02-02-2026 11:34 am, Chaitanya Kumar Borah wrote:
> The reference (software-equivalent) CRC was being recomputed
s/was/is
> for every plane even though it is identical across planes for
> a given test configuration. Refactor the test to collect the
> reference CRC once per pipe and reuse it for all planes.
>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
>   tests/kms_color_pipeline.c | 60 ++++++++++++++++++++++++++++----------
>   1 file changed, 44 insertions(+), 16 deletions(-)
>
> diff --git a/tests/kms_color_pipeline.c b/tests/kms_color_pipeline.c
> index e15f4d79a..8cc055591 100644
> --- a/tests/kms_color_pipeline.c
> +++ b/tests/kms_color_pipeline.c
> @@ -88,16 +88,52 @@ static bool ctm_colorop_only(kms_colorop_t *colorops[])
>   	return true;
>   }
>   
> +static void
> +test_grab_crc(data_t *data, igt_output_t *output,
> +	      const color_t *fb_color, igt_crc_t *crc /* out */)
Function to be renamed to capture_ref_crc() ?

Also, shall we add some comment stating why we are capturing CRC of 
primary plane as ref and not current plane?

> +{
> +	struct igt_fb fb;
> +	drmModeModeInfo *mode;
> +	igt_plane_t *primary;
> +	char *crc_str;
> +	int ret;
> +
> +	primary = igt_output_get_plane(output, 0);
Instead of 0, use DRM_PLANE_TYPE_PRIMARY
> +
> +	mode = igt_output_get_mode(output);
> +	igt_assert_fd(igt_create_fb(data->drm_fd,
> +				    mode->hdisplay, mode->vdisplay,
> +				    DRM_FORMAT_XRGB8888,
> +				    DRM_FORMAT_MOD_LINEAR,
> +				    &fb));
> +
> +	paint_rectangles(data, mode, fb_color, &fb);
> +	igt_plane_set_fb(primary, &fb);
> +	ret = igt_display_try_commit_atomic(&data->display, 0, NULL);
> +	igt_skip_on(ret != 0);
Shouldn't we have assert here instead?
> +
> +	igt_pipe_crc_collect_crc(data->pipe_crc, crc);
Initially before collecting CRC, we were waiting for vblank, shouldn't 
we add same?
> +
> +	igt_plane_set_fb(primary, NULL);
> +	igt_display_commit_atomic(&data->display, 0, NULL);
> +
> +	igt_remove_fb(data->drm_fd, &fb);
> +
> +	crc_str = igt_crc_to_string(crc);
> +	igt_debug("CRC for reference fb: %s\n", crc_str);
> +	free(crc_str);
> +}
> +
>   static void _test_plane_colorops(data_t *data, enum pipe pipe,
>   				 igt_plane_t *plane,
>   				 const color_t *fb_colors,
> -				 const color_t *exp_colors,
> +				 igt_crc_t *crc_ref,
>   				 kms_colorop_t *colorops[])
>   {
>   	igt_display_t *display = &data->display;
>   	drmModeModeInfo *mode = data->mode;
>   	igt_colorop_t *color_pipeline;
> -	igt_crc_t crc_ref, crc_pipe;
> +	igt_crc_t crc_pipe;
>   	struct igt_fb fb;
>   
>   	color_pipeline = get_color_pipeline(display, plane, colorops);
> @@ -110,18 +146,6 @@ static void _test_plane_colorops(data_t *data, enum pipe pipe,
>   				 DRM_FORMAT_XRGB8888,
>   				 DRM_FORMAT_MOD_LINEAR,
>   				 &fb));
> -	igt_plane_set_fb(plane, &fb);
> -
> -	igt_display_commit_atomic(&data->display, 0, NULL);
> -
> -	/* Reference (software-equivalent) CRC */
> -	set_color_pipeline_bypass(plane);
> -	paint_rectangles(data, mode, exp_colors, &fb);
> -
> -	igt_plane_set_fb(plane, &fb);
> -	igt_display_commit_atomic(&data->display, 0, NULL);
> -	igt_wait_for_vblank(plane->crtc);
> -	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_ref);
>   
>   	/* Hardware pipeline CRC */
>   	set_color_pipeline(display, plane, colorops, color_pipeline);
> @@ -139,7 +163,7 @@ static void _test_plane_colorops(data_t *data, enum pipe pipe,
>   	igt_wait_for_vblank(plane->crtc);
>   	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_pipe);
>   
> -	igt_assert_crc_equal(&crc_ref, &crc_pipe);
> +	igt_assert_crc_equal(crc_ref, &crc_pipe);
>   
>   	/* Cleanup per-test state */
>   	set_color_pipeline_bypass(plane);
> @@ -158,6 +182,9 @@ static void test_plane_colorops(data_t *data, enum pipe pipe,
>   	int n_planes = igt_crtc_for_pipe(&data->display, pipe)->n_planes;
>   	igt_output_t *output = data->output;
>   	igt_plane_t *plane;
> +	igt_crc_t reference_crc;
Can we make this as ref_crc?
> +
> +	test_grab_crc(data, output, exp_colors, &reference_crc);
>   
>   	for (int plane_id = 0; plane_id < n_planes; plane_id++) {
>   		plane = igt_output_get_plane(output, plane_id);
> @@ -166,7 +193,8 @@ static void test_plane_colorops(data_t *data, enum pipe pipe,
>   			continue;
>   
>   		igt_dynamic_f("pipe-%s-plane-%u", kmstest_pipe_name(pipe), plane_id)
> -			_test_plane_colorops(data, pipe, plane, fb_colors, exp_colors, colorops);
> +			_test_plane_colorops(data, pipe, plane, fb_colors,
> +					     &reference_crc, colorops);
>   	}
>   }
>