[igt-dev] [PATCH] tests/kms_color: Reduce skips on some ctm-* tests

[Melissa Wen]

On 12/19, Alex Hung wrote:
> 64d0ff72 introduced a workaround for ctm-max but the condition caused
> other ctm tests to skip unintentionally on some platforms.

Hi Alex,

Can't we just drop this gamma/degamma setup for CTM tests?

I remember thinking about removing this setup before... From DRM docs,
"Each of the properties are optional." so I don't see an explicity
reason for degamma/gamma dependencies. I think we should only test the
CTM property here, or change the name of the subtest to describe a full
color mgmt testing.

I took a look at previous discussions on having or not degamma/gamma
here and if it's an intel limitation, than I prefer to explicitly
restrict it for i915 HW. What do you think?

Best regards,

Melissa

> 
> The patch improves the condition by keeping the original condition
> for i915 and checking degamma and gamma props for others. The
> following subtests will be executed instead of skipped.
> 
>   kms_color at ctm-0-25@*
>   kms_color at ctm-0-50@*
>   kms_color at ctm-0-75@*
>   kms_color at ctm-red-to-blue@*
>   kms_color at ctm-negative@*
> 
> Signed-off-by: Alex Hung <alex.hung at amd.com>
> ---
>  tests/kms_color.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index d016cefb..450bf7ca 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -446,6 +446,26 @@ end:
>  	return ret;
>  }
>  
> +static bool is_gamma_degamma_supported(data_t *data,
> +				       igt_plane_t *primary,
> +				       color_t *before,
> +				       color_t *after)
> +{
> +	/*
> +	 * Don't program LUT's for max CTM cases, as limitation of
> +	 * representing intermediate values between 0 and 1.0 causes
> +	 * rounding issues and inaccuracies leading to crc mismatch.
> +	 */
> +	if (is_i915_device(data->drm_fd) && memcmp(before, after, sizeof(color_t)))
> +		return true;
> +
> +	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT) &&
> +	    igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT))
> +		return true;
> +
> +	return false;
> +}
> +
>  /*
>   * Draw 3 rectangles using before colors with the ctm matrix apply and verify
>   * the CRC is equal to using after colors with an identify ctm matrix.
> @@ -493,14 +513,7 @@ static bool test_pipe_ctm(data_t *data,
>  	igt_assert(fb_modeset_id);
>  	igt_plane_set_fb(primary, &fb_modeset);
>  
> -	/*
> -	 * Don't program LUT's for max CTM cases, as limitation of
> -	 * representing intermediate values between 0 and 1.0 causes
> -	 * rounding issues and inaccuracies leading to crc mismatch.
> -	 */
> -	if (memcmp(before, after, sizeof(color_t))) {
> -		igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT));
> -		igt_require(igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT));
> +	if (is_gamma_degamma_supported(data, primary, before, after)) {
>  
>  		degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  		gamma_linear = generate_table(data->gamma_lut_size, 1.0);
> @@ -508,7 +521,6 @@ static bool test_pipe_ctm(data_t *data,
>  		set_degamma(data, primary->pipe, degamma_linear);
>  		set_gamma(data, primary->pipe, gamma_linear);
>  	} else {
> -		/* Disable Degamma and Gamma for ctm max test */
>  		disable_degamma(primary->pipe);
>  		disable_gamma(primary->pipe);
>  	}
> -- 
> 2.25.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20221220/d423bcd4/attachment.sig>