Re: [PATCH i-g-t v5 2/2] tests/kms_color: Add multi-format coverage for pipe color tests

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

Hi Pranay

On 05-06-2026 12:11 pm, Pranay Samala wrote:
> Extend kms_color tests to run across multiple pixel formats at dynamic
> subtest level instead of being limited to single format for gamma/degamma
> and CTM tests.
> For platforms where only a single format is relevant (e.g. 10-bit
> paths), limit the iteration accordingly to avoid redundant testing.
>
> This improves coverage for format-dependent pipe color pipeline
> behavior.
>
> v2:
> - Add DRM_FORMAT_P010 format
>
> v4:
> - Split the patches (Swati)
>
> v5:
> - Consolidate format structure with bpc info (Swati)
> - Move format support check out of dynamic subtest scope (Swati)
>
> Signed-off-by: Pranay Samala <pranay.samala@intel.com>
> ---
>   tests/kms_color.c | 127 ++++++++++++++++++++++++++++++----------------
>   1 file changed, 83 insertions(+), 44 deletions(-)
>
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index f4b792fdd..8d08820ed 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -78,6 +78,18 @@
>   
>   IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
>   
> +static const struct {
> +	const char *name;
> +	uint32_t format;
> +	int bpc;
> +} formats[] = {
> +	{ "XRGB8888", DRM_FORMAT_XRGB8888, 8 },
> +	{ "YUYV", DRM_FORMAT_YUYV, 8 },
> +	{ "NV12", DRM_FORMAT_NV12, 8 },
> +	{ "XRGB2101010", DRM_FORMAT_XRGB2101010, 10 },
> +	{ "P010", DRM_FORMAT_P010, 10 },
> +};
> +
>   static unsigned int create_color_test_fb(data_t *data, int w, int h,
>   					 uint32_t format, struct igt_fb *fb)
>   {
> @@ -733,12 +745,33 @@ run_gamma_degamma_tests_for_crtc(data_t *data, igt_crtc_t *crtc,
>   	 * for CRC checks with framebuffer references.
>   	 */
>   	data->color_depth = 8;

With this,  10-bit formats will be tested but the LUT precision mask in 
coeffs_to_lut() will truncate to 8-bit. The reference CRCs are computed 
assuming 8-bit precision while the framebuffer is 10-bit — the test 
isn't exercising 10-bit behavior meaningfully.

Set data->color_depth = formats[i].bpc inside format loop and validate 
that gamma/degamma tests work at 10-bit precision like how you did in CTM

Also, do we have mtk 10bit restriction in gamma/degamma too?

++ Jason

> -	data->drm_format = DRM_FORMAT_XRGB8888;
>   	data->mode = igt_output_get_mode(data->output);
>   
>   	igt_require(crtc_output_combo_valid(data, crtc));
>   
> -	igt_assert(test_t(data, data->primary));
> +	for (int i = 0; i < ARRAY_SIZE(formats); i++) {
> +		/*
> +		 * legacy gamma path internally uses XRGB8888; avoid misleading
> +		 * dynamic names for non-RGB formats.
> +		 */
> +		if ((test_t == test_pipe_legacy_gamma ||
> +		     test_t == test_pipe_legacy_gamma_reset) &&
> +		    formats[i].format != DRM_FORMAT_XRGB8888)
> +			continue;
> +
> +		if (!igt_plane_has_format_mod(data->primary, formats[i].format,
> +					      DRM_FORMAT_MOD_LINEAR))
> +			continue;
> +
> +		igt_dynamic_f("pipe-%s-%s-%s", igt_crtc_name(crtc),
> +			      igt_output_name(data->output),
> +			      formats[i].name) {
> +			igt_info("Running on " IGT_FORMAT_FMT " format\n",
> +				 IGT_FORMAT_ARGS(formats[i].format));
> +			data->drm_format = formats[i].format;
> +			igt_assert(test_t(data, data->primary));
> +		}
> +	}
>   
>   	test_cleanup(data);
>   }
> @@ -758,24 +791,20 @@ run_ctm_tests_for_crtc(data_t *data, igt_crtc_t *crtc,
>   		       const double *ctm,
>   		       int iter)
>   {
> -	bool success = false;
> -	bool depth_10bit = false;
> +	bool success;
> +	bool mtk_10bpc_only;

can declaration and assignment be combined here?

mtk_10bpc_only = is_mtk_device()

>   	double delta;
>   	int i;
>   
>   	test_setup(data, crtc);
>   
> -	/* MediaTek can only support bit-ture in 10-bit depth pre color */
> -	if (is_mtk_device(data->drm_fd))
> -		depth_10bit = true;
> +	/* MediaTek can only support CTM in 10-bit depth per color. */
> +	mtk_10bpc_only = is_mtk_device(data->drm_fd);
>   
>   	/*
>   	 * We assume an 8bits or 10bits depth per color for degamma/gamma LUTs
>   	 * for CRC checks with framebuffer references.
>   	 */
> -	data->color_depth = depth_10bit ? 10 : 8;
> -	delta = 1.0 / (1 << data->color_depth);
> -	data->drm_format = depth_10bit ? DRM_FORMAT_XRGB2101010 : DRM_FORMAT_XRGB8888;
>   	data->mode = igt_output_get_mode(data->output);
>   
>   	igt_require(crtc_output_combo_valid(data, crtc));
> @@ -783,30 +812,44 @@ run_ctm_tests_for_crtc(data_t *data, igt_crtc_t *crtc,
>   	if (!iter)
>   		iter = 1;
>   
> -	/*
> -	 * We tests a few values around the expected result because
> -	 * it depends on the hardware we're dealing with, we can either
> -	 * get clamped or rounded values and we also need to account
> -	 * for odd number of items in the LUTs.
> -	 */
We can keep this comment
> -	for (i = 0; i < iter; i++) {
> -		color_t expected_colors[3] = {
> -			fb_colors[0],
> -			fb_colors[1],
> -			fb_colors[2],
> -		};
> -
> -		transform_color(&expected_colors[0], ctm, delta * (i - (iter / 2)));
> -		transform_color(&expected_colors[1], ctm, delta * (i - (iter / 2)));
> -		transform_color(&expected_colors[2], ctm, delta * (i - (iter / 2)));
> -
> -		if (test_pipe_ctm(data, data->primary, fb_colors,
> -				  expected_colors, ctm)) {
> -			success = true;
> -			break;
> +	for (int fi = 0; fi < ARRAY_SIZE(formats); fi++) {
> +		/* MTK runs 10bpc only; non-MTK runs all supported bpc formats. */
This comment is redundant.
> +		if (mtk_10bpc_only && formats[fi].bpc != 10)
> +			continue;
> +
> +		data->color_depth = formats[fi].bpc;
> +		delta = 1.0 / (1 << data->color_depth);
> +
> +		if (!igt_plane_has_format_mod(data->primary, formats[fi].format,
> +					      DRM_FORMAT_MOD_LINEAR))
> +			continue;
> +
> +		igt_dynamic_f("pipe-%s-%s-%s", igt_crtc_name(crtc),
> +			      igt_output_name(data->output),
> +			      formats[fi].name) {
> +			success = false;
> +			data->drm_format = formats[fi].format;
> +
> +			for (i = 0; i < iter; i++) {
> +				color_t expected_colors[3] = {
> +					fb_colors[0],
> +					fb_colors[1],
> +					fb_colors[2],
> +				};
> +
> +				transform_color(&expected_colors[0], ctm, delta * (i - (iter / 2)));
> +				transform_color(&expected_colors[1], ctm, delta * (i - (iter / 2)));
> +				transform_color(&expected_colors[2], ctm, delta * (i - (iter / 2)));
> +
> +				if (test_pipe_ctm(data, data->primary, fb_colors,
> +						  expected_colors, ctm)) {
> +					success = true;
> +					break;
> +				}
> +			}
> +			igt_assert(success);
>   		}
>   	}
> -	igt_assert(success);
>   
>   	test_cleanup(data);
>   }
> @@ -1100,11 +1143,9 @@ run_tests_for_pipe(data_t *data)
>   		igt_describe_f("%s", gamma_degamma_tests[i].desc);
>   		igt_subtest_with_dynamic_f("%s", gamma_degamma_tests[i].name) {
>   			for_each_crtc_with_valid_output(&data->display, crtc, data->output) {
> -				igt_dynamic_f("pipe-%s-%s", igt_crtc_name(crtc),
> -					      igt_output_name(data->output))
> -					run_gamma_degamma_tests_for_crtc(data,
> -									 crtc,
> -									 gamma_degamma_tests[i].test_t);
> +				run_gamma_degamma_tests_for_crtc(data,
> +								 crtc,
> +								 gamma_degamma_tests[i].test_t);
>   			}
>   		}
>   	}
> @@ -1113,13 +1154,11 @@ run_tests_for_pipe(data_t *data)
>   		igt_describe_f("%s", ctm_tests[i].desc);
>   		igt_subtest_with_dynamic_f("%s", ctm_tests[i].name) {
>   			for_each_crtc_with_valid_output(&data->display, crtc, data->output) {
> -				igt_dynamic_f("pipe-%s-%s", igt_crtc_name(crtc),
> -					      igt_output_name(data->output))
> -					run_ctm_tests_for_crtc(data,
> -							       crtc,
> -							       ctm_tests[i].fb_colors,
> -							       ctm_tests[i].ctm,
> -							       ctm_tests[i].iter);
> +				run_ctm_tests_for_crtc(data,
> +						       crtc,
> +						       ctm_tests[i].fb_colors,
> +						       ctm_tests[i].ctm,
> +						       ctm_tests[i].iter);
>   				if (igt_run_in_simulation())
>   					break;
>   			}