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

Radhakrishna Sripada radhakrishna.sripada at intel.com
Fri Oct 12 07:20:30 UTC 2018


On Thu, Oct 11, 2018 at 02:21:52PM -0700, Radhakrishna Sripada wrote:
> On Fri, Oct 05, 2018 at 04:34:37PM -0700, Manasi Navare wrote:
> > This patch adds a basic kms test to validate the display stream
> > compression functionality if supported on DP/eDP connector.
> > Currently this has only one subtest to force the DSC on all
> > the connectors that support it with default parameters.
> > This will be expanded to add more subtests to tweak DSC parameters.
> > 
> > Cc: 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     | 372 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 373 insertions(+)
> >  create mode 100644 tests/kms_dp_dsc.c
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index c84933f1..c807aad3 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -207,6 +207,7 @@ TESTS_progs = \
> >  	kms_tv_load_detect \
> >  	kms_universal_plane \
> >  	kms_vblank \
> > +	kms_dp_dsc \
> >  	meta_test \
> >  	perf \
> >  	perf_pmu \
> > diff --git a/tests/kms_dp_dsc.c b/tests/kms_dp_dsc.c
> > new file mode 100644
> > index 00000000..d0fd2ae3
> > --- /dev/null
> > +++ b/tests/kms_dp_dsc.c
> > @@ -0,0 +1,372 @@
> > +/*
> > + * Copyright © 2017 Intel Corporation
> > + *
> > + * Displayport Display Stream Compression test
> > + *
> > + * Authors:
> > + * Manasi Navare <manasi.d.navare at intel.com>
> > + *
> > + * Elements of the modeset code adapted from David Herrmann's
> > + * DRM modeset example
> > + *
> > + */
> > +#include "igt.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>
> > +
> > +int drm_fd;
> > +drmModeRes *resources;
> > +uint64_t tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> > +
> > +struct connector {
> > +	uint32_t id;
> > +	int mode_valid;
> > +	drmModeModeInfo mode;
> > +	drmModeConnector *connector;
> > +	drmModeEncoder *encoder;
> > +	int crtc, pipe;
> > +};
> Can kmstest_connector_config be used to simplify this structure?
> > +
> > +struct dsc_config {
> > +	char test_name[128];
> > +	int mode_width;
> > +	int mode_height;
> > +	int bpp;
> > +	int depth;
> > +	bool dsc_supported;
> > +	bool dsc_enable;
> > +};
> > +
> > +static FILE *fopenat(int dir, const char *name, const char *mode)
> > +{
> > +	int fd = openat(dir, name, O_RDWR);
> > +	return fdopen(fd, mode);
> > +}
> > +
> > +static bool get_dp_dsc_support(char *test_connector_name)
> > +{
> > +	int dir = igt_debugfs_dir(drm_fd);
> > +	FILE *dsc_support_fp = NULL;
> > +	char *line = NULL;
> > +	size_t line_size = 0;
> > +	char buf[128] = {0}, supported_str[5], enabled_str[5];
> > +	bool supported = false;
> > +
> > +	strcpy(buf, test_connector_name);
> > +	strcat(buf, "/i915_dsc_support");
> > +	dsc_support_fp = fopenat(dir, buf, "r");
> > +	igt_require(dsc_support_fp);
> > +
> > +	while (getline(&line, &line_size, dsc_support_fp) > 0) {
> > +		char *support, *enabled;
> > +
> > +		enabled = strstr(line, "Enabled: ");
> > +		igt_assert_eq(sscanf(enabled, "Enabled: %s", enabled_str), 1);
> > +		if (strcmp(enabled_str, "yes") == 0) {
> > +			igt_info("DSC is enabled on %s\n", test_connector_name);
> > +			supported = true;
> > +			break;
> > +		} else {
> > +			getline(&line, &line_size, dsc_support_fp);
> > +			support = strstr(line, "Supported: ");
> > +		igt_assert_eq(sscanf(support, "Supported: %s", supported_str), 1);
> > +		if (strcmp(supported_str, "yes") == 0)
> > +			supported = true;
> > +		else if (strcmp(supported_str, "no") == 0)
> > +			supported = false;
> > +		else
> > +			igt_fail_on_f(true, "Unknown DSC supported status '%s'\n",
> > +				      supported_str);
> > +		break;
> > +		}
> > +	}
> > +
> This function could make use of igt_debugfs_simple_read the way kms_psr does to make it cleaner.
> > +	free(line);
> > +	fclose(dsc_support_fp);
> > +
> > +	return supported;
> > +}
> > +
> > +static void set_dp_dsc_enable(bool enable, char *test_connector_name)
> > +{
> > +	int dir = igt_debugfs_dir(drm_fd);
> > +	FILE *dsc_enable_fp = NULL;
> > +	char buf[128] = {0};
> > +
> > +	strcpy(buf, test_connector_name);
> > +	strcat(buf, "/i915_dsc_support");
> > +	dsc_enable_fp = fopenat(dir, buf, "w+");
> > +	igt_require(dsc_enable_fp);
> > +	rewind(dsc_enable_fp);
> > +	igt_info("Setting DSC_ENABLE to %s for %s\n", (enable) ? "true" : "false", test_connector_name);
> Can we use igt_sysfs_write here?
> > +	fprintf(dsc_enable_fp, "%d", enable);
> > +	fflush(dsc_enable_fp);
> > +	fclose(dsc_enable_fp);
> > +}
> > +
> > +static void dump_connectors_fd(int drmfd)
> > +{
> > +	int i, j;
> > +
> > +	drmModeRes *mode_resources = drmModeGetResources(drmfd);
> > +
> > +        if (!mode_resources) {
> > +		igt_warn("drmModeGetResources failed: %s\n", strerror(errno));
> > +		return;
> > +	}
> > +
> > +	igt_info("Connectors:\n");
> > +	igt_info("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\n");
> > +	for (i = 0; i < mode_resources->count_connectors; i++) {
> > +		drmModeConnector *connector;
> > +
> > +		connector = drmModeGetConnectorCurrent(drmfd,
> > +						       mode_resources->connectors[i]);
> > +		if (!connector) {
> > +			igt_warn("could not get connector %i: %s\n",
> > +				 mode_resources->connectors[i], strerror(errno));
> > +			continue;
> > +		}
> > +
> > +		igt_info("%d\t%d\t%s\t%s\t%dx%d\t\t%d\n",
> > +			 connector->connector_id, connector->encoder_id,
> > +			 kmstest_connector_status_str(connector->connection),
> > +			 kmstest_connector_type_str(connector->connector_type),
> > +			 connector->mmWidth, connector->mmHeight,
> > +			 connector->count_modes);
> > +
> > +		if (!connector->count_modes)
> > +			continue;
> > +
> > +		igt_info("  modes:\n");
> > +		igt_info("  name refresh (Hz) hdisp hss hse htot vdisp ""vss vse vtot flags type clock\n");
> > +		for (j = 0; j < connector->count_modes; j++){
> > +			igt_info("[%d]", j);
> > +			kmstest_dump_mode(&connector->modes[j]);
> > +		}
> > +
> > +		drmModeFreeConnector(connector);
> > +	}
> > +	igt_info("\n");
> > +
> > +	drmModeFreeResources(mode_resources);
> > +}
> > +
> > +static void dump_crtcs_fd(int drmfd)
> > +{
> > +	int i;
> > +	drmModeRes *mode_resources = drmModeGetResources(drmfd);
> > +
> > +	igt_info("CRTCs:\n");
> > +	igt_info("id\tfb\tpos\tsize\n");
> > +	for (i = 0; i < mode_resources->count_crtcs; i++) {
> > +		drmModeCrtc *crtc;
> > +
> > +		crtc = drmModeGetCrtc(drmfd, mode_resources->crtcs[i]);
> > +		if (!crtc) {
> > +			igt_warn("could not get crtc %i: %s\n",
> > +				 mode_resources->crtcs[i], strerror(errno));
> > +			continue;
> > +		}
> > +		igt_info("%d\t%d\t(%d,%d)\t(%dx%d)\n",
> > +			 crtc->crtc_id, crtc->buffer_id, crtc->x,
> > +			 crtc->y, crtc->width, crtc->height);
> > +		kmstest_dump_mode(&crtc->mode);
> > +
> > +		drmModeFreeCrtc(crtc);
> > +	}
> > +	igt_info("\n");
> > +
> > +	drmModeFreeResources(mode_resources);
> > +}
> > +
> > +static void dump_info(void)
> > +{
> > +	dump_connectors_fd(drm_fd);
> > +	dump_crtcs_fd(drm_fd);
> > +}
> > +
> > +static int
> > +set_mode(struct connector *c, struct dsc_config *test_config, bool set_mode)
> > +{
> > +	unsigned int fb_id = 0;
> > +	struct igt_fb fb_info;
> > +	int ret = 0;
> > +
> > +	if (!set_mode) {
> > +		ret = drmModeSetCrtc(drm_fd, c->crtc, 0, 0, 0,
> > +				     NULL, 0, NULL);
> > +		if (ret)
> > +			igt_warn("Failed to unset mode");
> > +		return ret;
> > +	}
> > +
> > +	fb_id = igt_create_pattern_fb(drm_fd, test_config->mode_width,
> > +				      test_config->mode_height,
> > +				      igt_bpp_depth_to_drm_format(test_config->bpp,
> > +								  test_config->depth),
> > +				      tiling, &fb_info);
> > +
> > +	igt_info("CRTC(%u):[%d]", c->crtc, 0);
> > +	kmstest_dump_mode(&c->mode);
> > +	drmModeSetCrtc(drm_fd, c->crtc, -1, 0, 0, NULL, 0, NULL);
> > +	ret = drmModeSetCrtc(drm_fd, c->crtc, fb_id, 0, 0,
> > +			     &c->id, 1, &c->mode);
> > +	sleep(5);
> > +	if (ret < 0) {
> > +		igt_warn("Failed to set mode (%dx%d@%dHz): %s\n",
> > +			 test_config->mode_width, test_config->mode_height,
> > +			 c->mode.vrefresh, strerror(errno));
> > +		igt_remove_fb(drm_fd, &fb_info);
> > +
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +
> > +/*
> > + * Re-probe connectors and do a modeset with and wihout DSC
> > + *
> > + * Returns:
> > + * 0: On Success
> > + * -1: On failure
> > + */
> > +static int update_display(struct dsc_config *test_config)
> > +{
> > +	struct connector *connectors;
> > +	struct kmstest_connector_config config;
> > +	int cnt, conn_cnt = 0, ret = 0, edp_cnt = 0, dp_cnt = 0;
> > +	unsigned long crtc_idx_mask = -1UL;
> > +	char conn_buf[128] = {0};
> > +
> > +	resources = drmModeGetResources(drm_fd);
> > +	if (!resources) {
> > +		igt_warn("drmModeGetResources failed: %s\n", strerror(errno));
> > +		return -1;
> > +	}
> > +
> > +	connectors = calloc(resources->count_connectors,
> > +			    sizeof(struct connector));
> > +	if (!connectors)
> > +		return -1;
> > +
> Is it possible to use igt_display_t, followed by igt_display_require in igt_fixture?
> Or is it way too much overhead in collecting all non-required resources?
> > +	/* Find any connected displays */
> > +	for (cnt = 0; cnt < resources->count_connectors; cnt++) {
> > +		struct connector *c = &connectors[cnt];
> > +		c->id = resources->connectors[cnt];
> > +		if(!kmstest_get_connector_config(drm_fd, c->id, crtc_idx_mask,
> > +						&config)) {
> > +			c->mode_valid = 0;
> > +			continue;
> > +		}
> > +		c->connector = config.connector;
> > +		c->encoder = config.encoder;
> > +		c->mode = config.default_mode;
> > +		c->crtc = config.crtc->crtc_id;
> > +		c->pipe = config.pipe;
> > +		c->mode_valid = 1;
> > +
> > +		if (c->connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +		    c->connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +
> > +			if (c->connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> > +				conn_cnt = ++ edp_cnt;
> > +			else
> > +				conn_cnt = ++ dp_cnt;
> > +			if (c->connector->connection == DRM_MODE_CONNECTED) {
> > +				sprintf(conn_buf, "%s-%d",
> > +					kmstest_connector_type_str(c->connector->connector_type),
> > +					conn_cnt);
> > +				test_config->dsc_supported =
> > +					get_dp_dsc_support(conn_buf);
> > +				if (!strcmp(test_config->test_name,
> > +					    "force_dsc_enable_basic")) {
> Will this code path run?
Ignore the above comment.

Thanks,
RK
> > +					if (test_config->dsc_supported) {
> > +						igt_info ("DSC is supported on %s\n",
> > +							  conn_buf);
> > +						test_config->mode_width = c->mode.hdisplay;
> > +						test_config->mode_height = c->mode.vdisplay;
> > +						test_config->bpp = 32;
> > +						test_config->depth = 24;
> > +						set_dp_dsc_enable(test_config->dsc_enable,
> > +							  conn_buf);
> > +						ret = set_mode(c, test_config,
> > +							       true);
> > +					} else {
> > +						igt_info ("DSC is not supported on %s\n",
> > +							  conn_buf);
> > +					}
> > +				}
> > +				crtc_idx_mask &= ~(1 << c->pipe);
> > +			}
> > +		}
> > +	}
> > +
> > +	free(connectors);
> > +	drmModeFreeResources(resources);
> > +	return ret;
> > +}
> > +
> > +static int opt_handler(int opt, int opt_index, void *opt_dump)
> > +{
> > +	bool *dump = opt_dump;
> > +
> > +	switch (opt) {
> > +	case 'i':
> > +		*dump = true;
> > +		break;
> > +	default:
> > +		igt_assert(0);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	const char *help_str =
> > +		"  --info\t\tDump Information about connectors. (still needs DRM access)\n";
> > +	static struct option long_options[] = {
> > +		{"info", 0, 0, 'i'},
> > +		{0, 0, 0, 0}
> > +	};
> > +	int ret = 0;
> > +	struct dsc_config test_config;
> > +	bool opt_dump = false;
> > +	igt_subtest_init_parse_opts(&argc, argv, "", long_options, help_str,
> > +				    opt_handler, &opt_dump);
> > +
> > +	igt_skip_on_simulation();
> > +
> > +	igt_fixture {
> > +		kmstest_set_vt_graphics_mode();
> > +		drm_fd = drm_open_driver(DRIVER_ANY);
> > +		if (opt_dump)
> > +			dump_info();
> > +	}
> > +
> > +	igt_subtest("force_dsc_enable_basic") {
> > +		strcpy(test_config.test_name,"force_dsc_enable_basic");
> > +		ret = update_display(&test_config);
> > +		igt_assert_eq(ret, 0);
> > +	}
> > +
> > +	close(drm_fd);
> > +
> > +	igt_assert_eq(ret, 0);
> Is this assert required?
> 
> - Radhakrishna(RK) Sripada
> > +
> > +	igt_info("DSC testing done\n");
> > +	igt_exit();
> > +
> > +}
> > -- 
> > 2.18.0
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev


More information about the igt-dev mailing list