Re: [igt-dev] [PATCH v3 1/2] tests/kms_pipe_crc_basic: Sanity check for CRC mismatches

[Karthik B S <karthik.b.s@intel.com>]

On 8/7/2020 12:03 AM, Bhanuprakash Modem wrote:
> We’ve seen multiple CRC related issues on older platforms and
> pre-silicon environment, most of the time we end up with debugging.
> 
> This patch works as a crc-sanity test, which can verify that the
> CRC mechanism is clean from the platform side before debugging the
> actual CRC mismatches or other CRC related issues.
> 
> This patch will create two framebuffers (fb0 & fb1) with the same
> color info, flip fb0 with primary plane & collect CRC as reference.
> Then flip fb1 with primary plane, collect the CRC and compare with
> the reference CRC. There should be no CRC mismatch.
> 
> v2:
> * Run subtest to all pipes (Karthik & Swati)
> * Fix commit message & few comments (Karthik)
> * Use meaningful name for functions (Swati)
> * Remove unwanted checks (Karthik)
> v3:
> * Rename subtest to compare-crc-sanitycheck-pipe- (Petri)
> 
> Cc: Swati Sharma <swati2.sharma@intel.com>
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: Jeevan B <jeevan.b@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arek@hiler.eu>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>   tests/kms_pipe_crc_basic.c | 73 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 73 insertions(+)
> 
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index cb93c1ad..c3c4cdd8 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -154,6 +154,75 @@ static void test_read_crc(data_t *data, enum pipe pipe, unsigned flags)
>   	}
>   }
>   
> +/*
> + * CRC-sanity test, to make sure there would be no CRC mismatches
> + *
> + * - Create two framebuffers (FB0 & FB1) with same color info
> + * - Flip FB0 with the Primary plane & collect the CRC as ref CRC.
> + * - Flip FB1 with the Primary plane, collect the CRC & compare with
> + *   the ref CRC.
> + *
> + *   No CRC mismatch should happen
> + */
> +static void test_compare_crc(data_t *data, enum pipe pipe)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *primary;
> +	drmModeModeInfo *mode;
> +	igt_crc_t ref_crc, crc;
> +	igt_pipe_crc_t *pipe_crc = NULL;
> +	struct igt_fb fb0, fb1;
> +	igt_output_t *output = igt_get_single_output_for_pipe(display, pipe);
> +
> +	igt_require_f(output, "No connector found for pipe %s\n",
> +			kmstest_pipe_name(pipe));
> +
> +	igt_require_pipe(display, pipe);

This check is redundant as you are already calling 
'igt_get_single_output_for_pipe' before this.

With this removed, LGTM.
Reviewed-by: Karthik B S <karthik.b.s@intel.com>
> +	igt_display_reset(display);
> +	igt_output_set_pipe(output, pipe);
> +
> +	mode = igt_output_get_mode(output);
> +
> +	/* Create two framebuffers with the same color info. */
> +	igt_create_color_fb(data->drm_fd,
> +			mode->hdisplay, mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			1.0, 1.0, 1.0,
> +			&fb0);
> +	igt_create_color_fb(data->drm_fd,
> +			mode->hdisplay, mode->vdisplay,
> +			DRM_FORMAT_XRGB8888,
> +			LOCAL_DRM_FORMAT_MOD_NONE,
> +			1.0, 1.0, 1.0,
> +			&fb1);
> +
> +	/* Flip FB0 with the Primary plane & collect the CRC as ref CRC. */
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, &fb0);
> +	igt_display_commit(display);
> +
> +	pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> +				    INTEL_PIPE_CRC_SOURCE_AUTO);
> +	igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
> +
> +	/* Flip FB1 with the Primary plane & compare the CRC with ref CRC. */
> +	igt_plane_set_fb(primary, &fb1);
> +	igt_display_commit(display);
> +
> +	igt_pipe_crc_collect_crc(pipe_crc, &crc);
> +	igt_assert_crc_equal(&crc, &ref_crc);
> +
> +	/* Clean-up */
> +	igt_pipe_crc_free(pipe_crc);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_output_set_pipe(output, PIPE_NONE);
> +	igt_display_commit(display);
> +
> +	igt_remove_fb(data->drm_fd, &fb0);
> +	igt_remove_fb(data->drm_fd, &fb1);
> +}
> +
>   data_t data = {0, };
>   
>   igt_main
> @@ -209,6 +278,10 @@ igt_main
>   
>   			igt_disallow_hang(data.drm_fd, hang);
>   		}
> +
> +		igt_describe("Basic sanity check for CRC mismatches");
> +		igt_subtest_f("compare-crc-sanitycheck-pipe-%s", kmstest_pipe_name(pipe))
> +			test_compare_crc(&data, pipe);
>   	}
>   
>   	igt_fixture {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev