[igt-dev] [v3] tests/i915/kms_dsc: Validate dsc with diff plane formats

Modem, Bhanuprakash bhanuprakash.modem at intel.com
Tue Jun 14 05:45:51 UTC 2022


On Fri-10-06-2022 05:36 pm, Swati Sharma wrote:
> Already existing subtest is modified so that dsc can be validated
> using different plane modifiers. Few 8/10/16 bpc RGB and YUV formats
> are added.
> 
> v2: minor fixes
> v3: -renaming
>      -addition of new planar format test
> 
> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
> ---
>   tests/i915/kms_dsc.c | 68 +++++++++++++++++++++++++++++++-------------
>   1 file changed, 48 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/i915/kms_dsc.c b/tests/i915/kms_dsc.c
> index c94c64c4..7fe7d3d8 100644
> --- a/tests/i915/kms_dsc.c
> +++ b/tests/i915/kms_dsc.c
> @@ -47,11 +47,11 @@
>   IGT_TEST_DESCRIPTION("Test to validate display stream compression");
>   
>   #define HDISPLAY_5K	5120
> -#define XRGB8888_DRM_FORMAT_MIN_BPP	8
> +#define MIN_BPP		8
>   
>   enum dsc_test_type {
> -	TEST_BASIC_DSC_ENABLE,
> -	TEST_DSC_COMPRESSION_BPP
> +	TEST_BASIC_DSC,
> +	TEST_DSC_BPP
>   };
>   
>   typedef struct {
> @@ -68,6 +68,17 @@ typedef struct {
>   bool force_dsc_en_orig;
>   int force_dsc_restore_fd = -1;
>   
> +const struct {
> +	const int format;
> +	const char format_str[20];
> +} test_list[] = {
> +	{DRM_FORMAT_XRGB8888, "XRGB8888"},
> +	{DRM_FORMAT_XYUV8888, "XYUV8888"},
> +	{DRM_FORMAT_XRGB2101010, "XRGB2101010"},
> +	{DRM_FORMAT_XRGB16161616F, "XRGB16161616F"},
> +	{DRM_FORMAT_YUYV, "YUYV"},
> +};
> +
>   static inline void manual(const char *expected)
>   {
>   	igt_debug_interactive_mode_check("all", expected);
> @@ -171,7 +182,7 @@ static bool check_big_joiner_test_constraint(data_t *data,
>   	igt_output_t *output = data->output;
>   	drmModeModeInfo *mode = get_highres_mode(output);
>   
> -	if (test_type == TEST_DSC_COMPRESSION_BPP &&
> +	if (test_type == TEST_DSC_BPP &&
>   	    mode->hdisplay >= HDISPLAY_5K) {
>   		igt_debug("Bigjoiner does not support force bpp on %s\n",
>   			   output->name);
> @@ -224,7 +235,7 @@ static void test_cleanup(data_t *data)
>   }
>   
>   /* re-probe connectors and do a modeset with DSC */
> -static void update_display(data_t *data, enum dsc_test_type test_type)
> +static void update_display(data_t *data, enum dsc_test_type test_type, unsigned int plane_format)
>   {
>   	bool enabled;
>   	igt_plane_t *primary;
> @@ -240,7 +251,7 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
>   	save_force_dsc_en(data);
>   	force_dsc_enable(data);
>   
> -	if (test_type == TEST_DSC_COMPRESSION_BPP) {
> +	if (test_type == TEST_DSC_BPP) {
>   		igt_debug("Trying to set BPP to %d\n", data->compression_bpp);
>   		force_dsc_enable_bpp(data);
>   	}
> @@ -252,10 +263,14 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
>   	igt_output_override_mode(output, mode);
>   
>   	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_skip_on(!igt_plane_has_format_mod(primary, plane_format,
> +		    DRM_FORMAT_MOD_LINEAR));
> +
>   	igt_create_pattern_fb(data->drm_fd,
>   			      mode->hdisplay,
>   			      mode->vdisplay,
> -			      DRM_FORMAT_XRGB8888,
> +			      plane_format,
>   			      DRM_FORMAT_MOD_LINEAR,
>   			      &data->fb_test_pattern);
>   
> @@ -281,11 +296,12 @@ static void update_display(data_t *data, enum dsc_test_type test_type)
>   	test_cleanup(data);
>   }
>   
> -static void test_dsc(data_t *data, enum dsc_test_type test_type, int bpp)
> +static void test_dsc(data_t *data, enum dsc_test_type test_type, int bpp, unsigned int plane_format)
>   {
>   	igt_display_t *display = &data->display;
>   	igt_output_t *output;
> -	char test_name[10];
> +	char name1[20];
> +	char name2[20];
>   	enum pipe pipe;
>   
>   	for_each_pipe_with_valid_output(display, pipe, output) {
> @@ -305,10 +321,11 @@ static void test_dsc(data_t *data, enum dsc_test_type test_type, int bpp)
>   		if (!check_big_joiner_pipe_constraint(data))
>   			continue;
>   
> -		snprintf(test_name, sizeof(test_name), "-%dbpp", data->compression_bpp);
> +		snprintf(name1, sizeof(name1), "-%dbpp", data->compression_bpp);
> +		snprintf(name2, sizeof(name2), "-%s", igt_format_str(plane_format));
>   		igt_dynamic_f("pipe-%s-%s%s",  kmstest_pipe_name(data->pipe), data->output->name,
> -			     (test_type == TEST_DSC_COMPRESSION_BPP) ? test_name : "")
> -			update_display(data, test_type);
> +			     (test_type == TEST_DSC_BPP) ? name1 : name2)

Can we move this check to where you constructing the test name? So that 
we can manage with single variable for test name.	

> +			update_display(data, test_type, plane_format);
>   	}
>   }
>   
> @@ -332,25 +349,36 @@ igt_main
>   	igt_describe("Tests basic display stream compression functionality if supported "
>   		     "by a connector by forcing DSC on all connectors that support it "
>   		     "with default parameters");
> -	igt_subtest_with_dynamic("basic-dsc-enable")
> -			test_dsc(&data, TEST_BASIC_DSC_ENABLE, 0);
> +	igt_subtest_with_dynamic("basic-dsc")
> +			test_dsc(&data, TEST_BASIC_DSC, 0, DRM_FORMAT_XRGB8888);

Why do we need this extra subtest, as we are doing the same with 
"dsc-with-formats"?

IMO, we just need to drop "basic-dsc".

> +
> +	igt_describe("Tests basic display stream compression functionality if supported "
> +		     "by a connector by forcing DSC on all connectors that support it "
> +		     "with default parameters and creating fb with diff formats");
> +	igt_subtest_with_dynamic("dsc-with-formats") {
> +		for (int k = 0; k < ARRAY_SIZE(test_list); k++)
> +			test_dsc(&data, TEST_BASIC_DSC, 0, test_list[k].format);
> +	}
>   
>   	igt_fixture
>   		igt_require(intel_display_ver(data.devid) >= 13);
>   
> +	/* output bpp/compressed bpp supported is 8 to 23 (pipe_bpp -1)
> +	 * i.e. 8 to 23. so, here we are considering compressed bpp as min(8), mean (8+23/2)
> +	 * and max(23).
> +	 */
>   	igt_describe("Tests basic display stream compression functionality if supported "
>   		     "by a connector by forcing DSC on all connectors that support it "
>   		     "with certain BPP as the output BPP for the connector");
> -	igt_subtest_with_dynamic("XRGB8888-dsc-compression") {
> +	igt_subtest_with_dynamic("dsc-with-bpp") {
>   		uint32_t bpp_list[] = {
> -			XRGB8888_DRM_FORMAT_MIN_BPP,
> -			(XRGB8888_DRM_FORMAT_MIN_BPP  +
> -			(XRGB8888_DRM_FORMAT_MIN_BPP * 3) - 1) / 2,
> -			(XRGB8888_DRM_FORMAT_MIN_BPP * 3) - 1
> +			MIN_BPP,
> +			(MIN_BPP  + (MIN_BPP * 3) - 1) / 2,
> +			(MIN_BPP * 3) - 1
>   		};
>   
>   		for (int j = 0; j < ARRAY_SIZE(bpp_list); j++)
> -			test_dsc(&data, TEST_DSC_COMPRESSION_BPP, bpp_list[j]);
> +			test_dsc(&data, TEST_DSC_BPP, bpp_list[j], DRM_FORMAT_XRGB8888);

Why are we limiting this test to XRGB8888 only? Is it overkill to run on 
all formats mentioned in test_list[]?

>   	}
>   
>   	igt_fixture {



More information about the igt-dev mailing list