[igt-dev] [PATCH i-g-t] test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP

Srivatsa, Anusha anusha.srivatsa at intel.com
Mon Dec 17 19:44:26 UTC 2018



>-----Original Message-----
>From: Latvala, Petri
>Sent: Monday, December 17, 2018 4:00 AM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: igt-dev at lists.freedesktop.org; Navare, Manasi D
><manasi.d.navare at intel.com>; Argenziano, Antonio
><antonio.argenziano at intel.com>; Pandiyan, Dhinakaran
><dhinakaran.pandiyan at intel.com>; Ville Syrjälä <ville.syrjala at linux.intel.com>
>Subject: Re: [PATCH i-g-t] test/kms_dp_dsc: Basic KMS test to validate VESA DSC
>on DP/eDP
>
>On Wed, Dec 12, 2018 at 04:57:39PM -0800, Anusha wrote:
>> From: Manasi Navare <manasi.d.navare at intel.com>
>>
>> This patch adds a basic kms test to validate the display stream
>> compression functionality if supported on DP/eDP connector.
>> Currently this has only two subtests to force the DSC on all the eDP
>> and DP connectors that support it with default parameters.
>> This will be expanded to add more subtests to tweak DSC parameters.
>>
>> v6: (from Anusha)
>> * Fix run_test() (Petri)
>> * Fix update_display() to avoid leaks. (Petri)
>> v5:
>> * Fix test cleanup to avoid crash (Petri)
>> v4:
>> * Future proof for more test types (Petri)
>> * Fix alphabetical order (Petri)
>> * s/igt_display_init/igt_display_require (Petri)
>> * Remove blank lines after return (Petri)
>> v3:
>> * Use array of connectors and loop through (Petri)
>> * Also check for FEC on DP connectors (Manasi)
>> * Add a Pipe_A restriction on DP (Ville)
>> v2:
>> * Use IGT wrappers for all (DK, Antonio)
>> * Split into two subtests for eDP and DP types (Petri)
>>
>> Cc: Petri Latvala <petri.latvala at intel.com>
>> Cc: Antonio Argenziano <antonio.argenziano at intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
>> ---
>>  tests/Makefile.sources |   1 +
>>  tests/kms_dp_dsc.c     | 264 +++++++++++++++++++++++++++++++++++++++++
>>  tests/meson.build      |   1 +
>>  3 files changed, 266 insertions(+)
>>  create mode 100644 tests/kms_dp_dsc.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
>> eedde1e8..a3c24c99 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -54,6 +54,7 @@ TESTS_progs = \
>>  	kms_crtc_background_color \
>>  	kms_cursor_crc \
>>  	kms_cursor_legacy \
>> +	kms_dp_dsc \
>>  	kms_draw_crc \
>>  	kms_fbcon_fbt \
>>  	kms_fence_pin_leak \
>> diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c new file mode
>> 100644 index 00000000..884a7717
>> --- /dev/null
>> +++ b/tests/kms_dp_dsc.c
>> @@ -0,0 +1,264 @@
>> +/*
>> + * Copyright © 2018 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a
>> + * copy of this software and associated documentation files (the
>> +"Software"),
>> + * to deal in the Software without restriction, including without
>> +limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> +sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom
>> +the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> +the next
>> + * paragraph) shall be included in all copies or substantial portions
>> +of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
>EVENT
>> +SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
>DAMAGES
>> +OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> +OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Displayport Display Stream Compression test
>> + * Until the CRC support is added this needs to be invoked with
>> +--interactive
>> + * to manually verify if the test pattern is seen without corruption
>> +for each
>> + * subtest.
>> + *
>> + * Authors:
>> + * Manasi Navare <manasi.d.navare at intel.com>
>> + *
>> + */
>> +#include "igt.h"
>> +#include "igt_sysfs.h"
>> +#include <errno.h>
>> +#include <getopt.h>
>> +#include <math.h>
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +#include <strings.h>
>> +#include <unistd.h>
>> +#include <stdlib.h>
>> +#include <signal.h>
>> +#include <time.h>
>> +#include <fcntl.h>
>> +#include <termios.h>
>> +
>> +enum dsc_test_type
>> +{
>> +	test_basic_dsc_enable
>> +};
>> +
>> +typedef struct {
>> +	int drm_fd;
>> +	int debugfs_fd;
>> +	uint32_t id;
>> +	igt_display_t display;
>> +	struct igt_fb fb_test_pattern;
>> +	igt_output_t *output;
>> +	int mode_valid;
>> +	drmModeModeInfo *mode;
>> +	drmModeConnector *connector;
>> +	drmModeEncoder *encoder;
>> +	int crtc;
>> +	enum pipe pipe;
>> +	char conn_name[128];
>> +} data_t;
>> +
>> +static inline void manual(const char *expected) {
>> +	igt_debug_manual_check("all", expected); }
>> +
>> +static bool is_dp_dsc_supported(data_t *data) {
>> +	char file_name[128] = {0};
>> +	char buf[512];
>> +
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_fec_support");
>> +	igt_require(igt_debugfs_simple_read(data->debugfs_fd, file_name, buf,
>> +					    sizeof(buf)) > 0);
>> +	igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> +	return strstr(buf, "DSC_Sink_Support: yes"); }
>> +
>> +static bool is_dp_fec_supported(data_t *data) {
>> +	char file_name[128] = {0};
>> +	char buf[512];
>> +
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_fec_support");
>> +	igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> +	return strstr(buf, "FEC_Sink_Support: yes"); }
>> +
>> +static bool is_dp_dsc_enabled(data_t *data) {
>> +	char file_name[128] = {0};
>> +	char buf[512];
>> +
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_fec_support");
>> +	igt_debugfs_read(data->drm_fd, file_name, buf);
>> +
>> +	return strstr(buf, "DSC_Enabled: yes"); }
>> +
>> +static void force_dp_dsc_enable(data_t *data) {
>> +	char file_name[128] = {0};
>> +	int ret;
>> +
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_fec_support");
>> +	igt_debug ("Forcing DSC enable on %s\n", data->conn_name);
>> +	ret = igt_sysfs_write(data->debugfs_fd, file_name, "1", 1);
>> +	igt_assert_f(ret > 0, "debugfs_write failed"); }
>> +
>> +static void clear_dp_dsc_enable(data_t *data) {
>> +	char file_name[128] = {0};
>> +	int ret;
>> +
>> +	strcpy(file_name, data->conn_name);
>> +	strcat(file_name, "/i915_dsc_fec_support");
>> +	igt_debug ("Clearing DSC enable on %s\n", data->conn_name);
>> +	ret = igt_sysfs_write(data->debugfs_fd, file_name, "0", 1);
>> +	igt_assert_f(ret > 0, "debugfs_write failed"); }
>> +
>> +static void test_cleanup(data_t *data) {
>> +	igt_plane_t *primary;
>> +
>> +	if (data->output) {
>> +	primary = igt_output_get_plane_type(data->output,
>> +					    DRM_PLANE_TYPE_PRIMARY);
>> +	igt_plane_set_fb(primary, NULL);
>> +	igt_display_commit(&data->display);
>> +
>> +	igt_remove_fb(data->drm_fd, &data->fb_test_pattern);
>> +	}
>> +}
>
>
>Indentation on the if is wrong.

Oops...will fix it.

>
>> +
>> +
>> +/*
>> + * Re-probe connectors and do a modeset with DSC
>> + *
>> + */
>> +static void update_display(data_t *data, enum dsc_test_type
>> +test_type) {
>> +	igt_plane_t *primary;
>> +	data->mode = igt_output_get_mode(data->output);
>> +	data->connector = data->output->config.connector;
>> +
>> +	if (data->connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort &&
>> +	    data->pipe == PIPE_A) {
>> +		igt_debug ("DSC not supported on Pipe A on external DP\n");
>> +		return;
>> +	}
>> +
>> +	if (test_type == test_basic_dsc_enable) {
>> +		igt_debug ("DSC is supported on %s\n",
>> +			   data->conn_name);
>> +		force_dp_dsc_enable(data);
>> +		/* Clear the fb in order to avoid leaks */
>> +		igt_remove_fb(data->drm_fd, &data->fb_test_pattern);
>> +		igt_create_pattern_fb(data->drm_fd, data->mode->hdisplay,
>> +				      data->mode->vdisplay,
>> +				      DRM_FORMAT_XRGB8888,
>> +				      LOCAL_DRM_FORMAT_MOD_NONE,
>> +				      &data->fb_test_pattern);
>> +		primary = igt_output_get_plane_type(data->output,
>> +
>DRM_PLANE_TYPE_PRIMARY);
>> +		igt_plane_set_fb(primary, &data->fb_test_pattern);
>> +		igt_display_commit(&data->display);
>> +		/* Until we have CRC check support, manually check if RGB test
>pattern has no corruption */
>> +		manual ("RGB test pattern without corruption");
>> +		igt_assert_f(is_dp_dsc_enabled(data),
>> +			     "Default DSC enable failed on Connector: %s Pipe:
>%s",
>> +			     data->conn_name,
>> +			     kmstest_pipe_name(data->pipe));
>> +		clear_dp_dsc_enable(data);
>> +	} else {
>> +		igt_assert(!"Unknown test type\n");
>> +	}
>> +}
>> +
>> +static void run_test(data_t *data, igt_output_t *output,
>> +		     enum dsc_test_type test_type)
>> +{
>> +	enum pipe pipe;
>> +
>> +	for_each_pipe_static(pipe) {
>
>
>This doesn't need to be the _static loop, for_each_pipe(data->display, pipe) can
>be used.

Hmmm...looking at both again, it says for_each_pipe_static() is to be used to enumerate per-pipe subtests and for_
each_pipe() cannot be used to enumerate per-pipe subtests. So, if we want to add more subtests wont having _static be useful?

>
>> +		if (igt_pipe_connector_valid(pipe, output)) {
>> +			igt_output_set_pipe(output, pipe);
>> +			data->pipe = pipe;
>> +			data->output = output;
>> +			update_display(data, test_type);
>> +		}
>> +	}
>> +}
>> +
>> +igt_main
>> +{
>> +	data_t data = {};
>> +	igt_output_t *output;
>> +	drmModeRes *res;
>> +	drmModeConnector *connector;
>> +	int i, test_conn_cnt, test_cnt;
>> +	int tests[] = {DRM_MODE_CONNECTOR_eDP,
>> +DRM_MODE_CONNECTOR_DisplayPort};
>> +
>> +	igt_fixture {
>> +		data.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>> +		kmstest_set_vt_graphics_mode();
>> +		igt_display_require(&data.display, data.drm_fd);
>> +		//igt_display_require_output(&data.display);
>> +		igt_require(res = drmModeGetResources(data.drm_fd));
>> +	}
>> +
>> +	for (test_cnt = 0; test_cnt < ARRAY_SIZE(tests); test_cnt++) {
>> +
>> +		igt_subtest_f("basic-dsc-enable-%s",
>> +			      kmstest_connector_type_str(tests[test_cnt])) {
>> +			test_conn_cnt = 0;
>> +			for (i = 0; i < res->count_connectors; i++) {
>> +				connector =
>drmModeGetConnectorCurrent(data.drm_fd,
>> +								       res-
>>connectors[i]);
>> +				if (connector->connection !=
>DRM_MODE_CONNECTED ||
>> +				    connector->connector_type !=
>> +				    tests[test_cnt])
>> +					continue;
>> +				test_conn_cnt++;
>> +				output =
>igt_output_from_connector(&data.display, connector);
>> +				sprintf(data.conn_name, "%s-%d",
>> +					kmstest_connector_type_str(connector-
>>connector_type),
>> +					test_conn_cnt);
>> +				igt_require_f(is_dp_dsc_supported(&data),
>> +					      "DSC not supported on connector %s
>\n",
>> +					      data.conn_name);
>
>This will make the subtest skip when checking DP-1, without trying
>DP-2 at all. Is that intended?
>
No. it has to test for DP-2 as well..
But even if the igt_require() here fails, the loop will move on to the next connector right?

Anusha 


Anusha 
>
>--
>Petri Latvala
>
>
>
>> +				if (connector->connector_type ==
>DRM_MODE_CONNECTOR_DisplayPort)
>> +
>	igt_require_f(is_dp_fec_supported(&data),
>> +						      "DSC cannot be enabled
>without FEC on %s\n",
>> +						      data.conn_name);
>> +				run_test(&data, output, test_basic_dsc_enable);
>> +			}
>> +			test_cleanup(&data);
>> +			igt_skip_on(test_conn_cnt == 0);
>> +		}
>> +	}
>> +
>> +	igt_fixture {
>> +		drmModeFreeConnector(connector);
>> +		drmModeFreeResources(res);
>> +		close(data.debugfs_fd);
>> +		close(data.drm_fd);
>> +		igt_display_fini(&data.display);
>> +	}
>> +	igt_exit();
>> +}
>> diff --git a/tests/meson.build b/tests/meson.build
>> index b8a6e61b..e14ab2b4 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -25,6 +25,7 @@ test_progs = [
>>  	'kms_crtc_background_color',
>>  	'kms_cursor_crc',
>>  	'kms_cursor_legacy',
>> +	'kms_dp_dsc',
>>  	'kms_draw_crc',
>>  	'kms_fbcon_fbt',
>>  	'kms_fence_pin_leak',
>> --
>> 2.17.0
>>


More information about the igt-dev mailing list