Re: [PATCH i-g-t] tests/kms_color: Enable ctm-limited-range subtest

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Feb 14, 2025 at 09:40:11PM +0530, Swati Sharma wrote:
> This tests is currently disabled since CRC computed on Intel
> hardware seems to include data on the lower bits, this is
> preventing us to CRC checks.
> 
> Let's try to enable it back and check behavior on newer Intel
> platforms.
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  tests/kms_color.c | 166 ++++++++++++++++++++++++++--------------------
>  1 file changed, 93 insertions(+), 73 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 4b71d3dd3..c3b285b4e 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -58,6 +58,7 @@
>   * @0-75:           for 0.75 transparency
>   * @blue-to-red:    from blue to red
>   * @green-to-red:   from green to red
> + * @limited-range:  with identity matrix
>   * @max:            for maximum transparency
>   * @negative:       for negative transparency
>   * @red-to-blue:    from red to blue
> @@ -623,107 +624,97 @@ static bool test_pipe_ctm(data_t *data,
>   * This test is currently disabled as the CRC computed on Intel hardware seems
>   * to include data on the lower bits, this is preventing us to CRC checks.
>   */
> -#if 0
> -static void test_pipe_limited_range_ctm(data_t *data,
> +static bool test_pipe_limited_range_ctm(data_t *data,
>  					igt_plane_t *primary)
>  {
>  	double limited_result = 235.0 / 255.0;
> -	static const color_t red_green_blue_limited[] = {
> +	color_t red_green_blue_limited[] = {
>  		{ limited_result, 0.0, 0.0 },
>  		{ 0.0, limited_result, 0.0 },
> -		{ 0.0, 0.0, limited_result },
> +		{ 0.0, 0.0, limited_result }
>  	};

This whole thing is fundementally broken. We can't generate
limited range output without using the CSC post offsets,
which are not exposed via the current CTM uapi.

> -	static const color_t red_green_blue_full[] = {
> +	color_t red_green_blue_full[] = {
>  		{ 0.5, 0.0, 0.0 },
>  		{ 0.0, 0.5, 0.0 },
> -		{ 0.0, 0.0, 0.5 },
> -	};
> -	static const double ctm[] = {
> -		1.0, 0.0, 0.0,
> -		0.0, 1.0, 0.0,
> -		0.0, 0.0, 1.0,
> +		{ 0.0, 0.0, 0.5 }
>  	};
> +	double ctm[] = { 1.0, 0.0, 0.0,
> +			0.0, 1.0, 0.0,
> +			0.0, 0.0, 1.0 };
>  	gamma_lut_t *degamma_linear, *gamma_linear;
> -	igt_output_t *output;
> -	bool has_broadcast_rgb_output = false;
> +	igt_output_t *output = data->output;
> +	drmModeModeInfo *mode = data->mode;
> +	struct igt_fb fb_modeset, fb;
> +	int fb_id, fb_modeset_id;
> +	igt_crc_t crc_full, crc_limited;
>  	igt_display_t *display = &data->display;
> +	bool ret = false;
> +
> +	igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM));
>  
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> -		drmModeModeInfo *mode;
> -		struct igt_fb fb_modeset, fb;
> -		igt_crc_t crc_full, crc_limited;
> -		int fb_id, fb_modeset_id;
> -
> -		if (!igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> -			continue;
> -
> -		has_broadcast_rgb_output = true;
> +	igt_output_set_pipe(output, primary->pipe->pipe);
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> -		mode = igt_output_get_mode(output);
> +	/* Create a framebuffer at the size of the output. */
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      mode->hdisplay,
> +			      mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      DRM_FORMAT_MOD_LINEAR,
> +			      &fb);
> +	igt_assert(fb_id);
>  
> -		/* Create a framebuffer at the size of the output. */
> -		fb_id = igt_create_fb(data->drm_fd,
> +	fb_modeset_id = igt_create_fb(data->drm_fd,
>  				      mode->hdisplay,
>  				      mode->vdisplay,
>  				      DRM_FORMAT_XRGB8888,
>  				      DRM_FORMAT_MOD_LINEAR,
> -				      &fb);
> -		igt_assert(fb_id);
> -
> -		fb_modeset_id = igt_create_fb(data->drm_fd,
> -					      mode->hdisplay,
> -					      mode->vdisplay,
> -					      DRM_FORMAT_XRGB8888,
> -					      DRM_FORMAT_MOD_LINEAR,
> -					      &fb_modeset);
> -		igt_assert(fb_modeset_id);
> -		igt_plane_set_fb(primary, &fb_modeset);
> +				      &fb_modeset);
> +	igt_assert(fb_modeset_id);
> +	igt_plane_set_fb(primary, &fb_modeset);
>  
> -		set_degamma(data, primary->pipe, degamma_linear);
> -		set_gamma(data, primary->pipe, gamma_linear);
> -		set_ctm(primary->pipe, ctm);
> -
> -		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
> -		paint_rectangles(data, mode, red_green_blue_limited, &fb);
> -		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd,
> -				display->pipes[primary->pipe->pipe].crtc_offset);
> -		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
> -
> -		/* Set the output into limited range. */
> -		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
> -		paint_rectangles(data, mode, red_green_blue_full, &fb);
> -		igt_plane_set_fb(primary, &fb);
> -		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd,
> -				display->pipes[primary->pipe->pipe].crtc_offset);
> -		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
> -
> -		/* And reset.. */
> -		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
> -		igt_plane_set_fb(primary, NULL);
> -		igt_output_set_pipe(output, PIPE_NONE);
> -
> -		/* Verify that the CRC of the software computed output is
> -		 * equal to the CRC of the CTM matrix transformation output.
> -		 */
> -		igt_assert_crc_equal(&crc_full, &crc_limited);
> +	set_degamma(data, primary->pipe, degamma_linear);
> +	set_gamma(data, primary->pipe, gamma_linear);
> +	set_ctm(primary->pipe, ctm);
>  
> -		igt_remove_fb(data->drm_fd, &fb);
> -		igt_remove_fb(data->drm_fd, &fb_modeset);
> -	}
> +	igt_output_set_prop_value(output,
> +				  IGT_CONNECTOR_BROADCAST_RGB,
> +				  BROADCAST_RGB_FULL);
> +	paint_rectangles(data, mode, red_green_blue_limited, &fb);
> +	igt_plane_set_fb(primary, &fb);
> +	igt_display_commit(&data->display);
> +	igt_wait_for_vblank(data->drm_fd,
> +			display->pipes[primary->pipe->pipe].crtc_offset);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
> +
> +	/* Set the output into limited range. */
> +	igt_output_set_prop_value(output,
> +				  IGT_CONNECTOR_BROADCAST_RGB,
> +				  BROADCAST_RGB_16_235);
> +	paint_rectangles(data, mode, red_green_blue_full, &fb);
> +	igt_plane_set_fb(primary, &fb);
> +	igt_display_commit(&data->display);
> +	igt_wait_for_vblank(data->drm_fd,
> +			display->pipes[primary->pipe->pipe].crtc_offset);
> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
> +
> +	/* And reset.. */
> +	igt_output_set_prop_value(output,
> +				  IGT_CONNECTOR_BROADCAST_RGB,
> +				  BROADCAST_RGB_FULL);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_output_set_pipe(output, PIPE_NONE);
> +	igt_display_commit(&data->display);
> +
> +	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_full, &crc_limited);
>  
>  	free_lut(gamma_linear);
>  	free_lut(degamma_linear);
>  
> -	igt_require(has_broadcast_rgb_output);
> +	return ret;
>  }
> -#endif
>  
>  static void
>  prep_pipe(data_t *data, enum pipe p)
> @@ -1000,6 +991,25 @@ run_invalid_tests_for_pipe(data_t *data)
>  	}
>  }
>  
> +static void
> +run_limited_range_ctm_test_for_pipe(data_t *data, enum pipe p,
> +		bool (*test_t)(data_t*, igt_plane_t*))
> +{
> +	test_setup(data, p);
> +
> +	igt_require(igt_output_has_prop(data->output, IGT_CONNECTOR_BROADCAST_RGB));
> +
> +	data->color_depth = 8;
> +	data->drm_format = DRM_FORMAT_XRGB8888;
> +	data->mode = igt_output_get_mode(data->output);
> +
> +	igt_require(pipe_output_combo_valid(data, p));
> +
> +	igt_assert(test_t(data, data->primary));
> +
> +	test_cleanup(data);
> +}
> +
>  static void
>  run_tests_for_pipe(data_t *data)
>  {
> @@ -1156,6 +1166,16 @@ run_tests_for_pipe(data_t *data)
>  		}
>  	}
>  
> +	igt_describe("Compare after applying ctm matrix & identity matrix");
> +	igt_subtest_with_dynamic("ctm-limited-range") {
> +		for_each_pipe_with_valid_output(&data->display, pipe, data->output) {
> +			igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe),
> +				       igt_output_name(data->output))
> +				run_limited_range_ctm_test_for_pipe(data, pipe,
> +								    test_pipe_limited_range_ctm);
> +			}
> +		}
> +
>  	igt_fixture
>  		igt_require(data->display.is_atomic);
>  
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel