[igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests

[Ville Syrjälä]

On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
> !igt_skip_crc_compare || igt_check_crc_equal() is always true
> which is not correct.
> 
> Fixes: 1a42910d

Hang on. Now you're saying the regression in detecting actual failures
was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
igt_check_crc_equal()") rather than the dynamic subtest conversion
commit (which is what the previous Fixes line claimed)?

So apparently the test may have been broken for 1.5 years now and
no one realize it.

Hmm. And idea just came to me. Could we introduce some kind of fairly
generic (say, igt_fb level) knob that would attempt to introduce a real
crc mismatch into all (or at least most) tests? Eg. maybe igt_fb could
frob around with the pixels a bit or something? This would at least
allow one to easily verify that the test *may* start to fail when
turning that knob on. Whether it actually starts to fail for a given
pixel frobbry isn't 100% guaraneed though since we could just end up
with a crc collision by dumb luck. Ideally we would have some way to
guarantee the failure, and then we would just make CI run every test
a second time with knob turned on to validate that the test can
actually detect the failure. But maybe this is a pipe dream.

> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> ---
>  tests/kms_color.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index ba06947b..e2462aa3 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -510,7 +510,7 @@ static bool test_pipe_ctm(data_t *data,
>  	/* Verify that the CRC of the software computed output is
>  	 * equal to the CRC of the CTM matrix transformation output.
>  	 */
> -	ret &= !igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
> +	ret = igt_skip_crc_compare || igt_check_crc_equal(&crc_software, &crc_hardware);
> 
>  	igt_plane_set_fb(primary, NULL);
>  	igt_output_set_pipe(output, PIPE_NONE);
> --
> 2.37.3

-- 
Ville Syrjälä
Intel