Re: [PATCH 2/2] tests: add kms_edp_vdd_race

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We recently fixed a bug where it was impossible to do I2C transactions
> on eDP panels when they were disabled. Now it should be possible to do
> these transactions when the panel is disabled, but there's a race
> condition that triggers dmesg errors if we try do do the I2C
> transactions and set a mode on the panel at the same time. This
> program should reproduce this bug and check dmesg for errors.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Like I've said in the previous mail I think the generic dmesg error
checking should be somewhere generic (and probably in piglit). Otherwise
the test looks good. And the naming also matches the new convention ;-)
-Daniel

> ---
>  tests/.gitignore         |   1 +
>  tests/Makefile.am        |   2 +
>  tests/kms_edp_vdd_race.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 229 insertions(+)
>  create mode 100644 tests/kms_edp_vdd_race.c
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 09ea074..ddb61f9 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -102,6 +102,7 @@ igt_no_exit_list_only
>  igt_no_subtest
>  kms_addfb
>  kms_cursor_crc
> +kms_edp_vdd_race
>  kms_flip
>  kms_pipe_crc_basic
>  kms_render
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 0426ec0..955d2a3 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -52,6 +52,7 @@ TESTS_progs_M = \
>  	gem_write_read_ring_switch \
>  	kms_addfb \
>  	kms_cursor_crc \
> +	kms_edp_vdd_race \
>  	kms_flip \
>  	kms_pipe_crc_basic \
>  	kms_render \
> @@ -236,6 +237,7 @@ gem_fence_thrash_LDADD = $(LDADD) -lpthread
>  gem_flink_race_LDADD = $(LDADD) -lpthread
>  gem_threaded_access_tiled_LDADD = $(LDADD) -lpthread
>  prime_self_import_LDADD = $(LDADD) -lpthread
> +kms_edp_vdd_race_LDADD = $(LDADD) -lpthread
>  
>  gem_wait_render_timeout_LDADD = $(LDADD) -lrt
>  kms_flip_LDADD = $(LDADD) -lrt
> diff --git a/tests/kms_edp_vdd_race.c b/tests/kms_edp_vdd_race.c
> new file mode 100644
> index 0000000..a6bff65
> --- /dev/null
> +++ b/tests/kms_edp_vdd_race.c
> @@ -0,0 +1,226 @@
> +/*
> + * Copyright © 2013 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.
> + *
> + * Authors: Paulo Zanoni <paulo.r.zanoni@intel.com>
> + *
> + */
> +
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-dev.h>
> +#include <pthread.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +
> +#include "drmtest.h"
> +
> +int drm_fd, kmsg_fd;
> +drmModeResPtr res;
> +drmModeConnectorPtr edp_connector;
> +bool stop;
> +
> +static void disable_all_screens(void)
> +{
> +	int i, rc;
> +
> +	for (i = 0; i < res->count_crtcs; i++) {
> +		rc = drmModeSetCrtc(drm_fd, res->crtcs[i], -1, 0, 0,
> +				    NULL, 0, NULL);
> +		igt_assert(rc == 0);
> +	}
> +}
> +
> +static void find_edp_connector(void)
> +{
> +	int i;
> +	drmModeConnectorPtr c;
> +
> +	edp_connector = NULL;
> +	for (i = 0; i < res->count_connectors; i++) {
> +		c = drmModeGetConnector(drm_fd, res->connectors[i]);
> +
> +		if (c->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +			igt_require(c->connection == DRM_MODE_CONNECTED);
> +			edp_connector = c;
> +			break;
> +		}
> +
> +		drmModeFreeConnector(c);
> +	}
> +	igt_require(edp_connector);
> +}
> +
> +static void read_edid_ioctl(int fd)
> +{
> +	unsigned char edid[128] = {};
> +	struct i2c_msg msgs[] = {
> +		{ /* Start at 0. */
> +			.addr = 0x50,
> +			.flags = 0,
> +			.len = 1,
> +			.buf = edid,
> +		}, { /* Now read the EDID. */
> +			.addr = 0x50,
> +			.flags = I2C_M_RD,
> +			.len = 128,
> +			.buf = edid,
> +		}
> +	};
> +	struct i2c_rdwr_ioctl_data msgset = {
> +		.msgs = msgs,
> +		.nmsgs = 2,
> +	};
> +
> +	ioctl(fd, I2C_RDWR, &msgset);
> +}
> +
> +/* TODO: We're currently just trying to read all the I2C files. We should try to
> + * find which one is really the eDP I2C file and just read from it. */
> +static void read_i2c_edid(void)
> +{
> +	int fd;
> +	DIR *dir;
> +
> +	struct dirent *dirent;
> +	char full_name[32];
> +
> +	dir = opendir("/dev/");
> +	igt_assert(dir);
> +
> +	while ((dirent = readdir(dir))) {
> +		if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
> +			snprintf(full_name, 32, "/dev/%s", dirent->d_name);
> +			fd = open(full_name, O_RDWR);
> +			igt_assert(fd != -1);
> +			read_edid_ioctl(fd);
> +			close(fd);
> +		}
> +	}
> +
> +	closedir(dir);
> +}
> +
> +static void *i2c_thread_func(void *unused)
> +{
> +	while (!stop)
> +		read_i2c_edid();
> +
> +	pthread_exit(NULL);
> +}
> +
> +static uint32_t create_fb(int width, int height)
> +{
> +	struct kmstest_fb fb;
> +	cairo_t *cr;
> +	uint32_t buffer_id;
> +
> +	buffer_id = kmstest_create_fb(drm_fd, width, height, 32, 24, false,
> +				      &fb);
> +	cr = kmstest_get_cairo_ctx(drm_fd, &fb);
> +	kmstest_paint_test_pattern(cr, width, height);
> +	return buffer_id;
> +}
> +
> +static void enable_edp_screen(void)
> +{
> +	int rc;
> +	uint32_t buffer_id = 0, crtc_id, connector_id;
> +	drmModeModeInfoPtr mode;
> +
> +	crtc_id = res->crtcs[0];
> +	connector_id = edp_connector->connector_id;
> +	mode = &edp_connector->modes[0];
> +
> +	buffer_id = create_fb(mode->hdisplay, mode->vdisplay);
> +
> +	rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, &connector_id,
> +			    1, mode);
> +	igt_assert(rc == 0);
> +}
> +
> +static void *modeset_thread_func(void *unused)
> +{
> +	while (!stop) {
> +		disable_all_screens();
> +		sleep(1);
> +		enable_edp_screen();
> +		sleep(1);
> +	}
> +
> +	pthread_exit(NULL);
> +}
> +
> +/* This test exercises a race condition that happens when we're trying to read
> + * the I2C data from an eDP panel at the same time we're trying to set a mode on
> + * the same panel. If we have the bug, we print error messages on dmesg, which
> + * we should catch with the kmsg functions. */
> +static void i2c_modeset_vdd_race(void)
> +{
> +	pthread_t i2c_thread, modeset_thread;
> +	void *status;
> +
> +	kmsg_error_reset(kmsg_fd);
> +
> +	stop = false;
> +	pthread_create(&i2c_thread, NULL, i2c_thread_func, NULL);
> +	pthread_create(&modeset_thread, NULL, modeset_thread_func, NULL);
> +
> +	/* This effectively sleeps for 100 seconds, but kills the program in
> +	 * case there's error on dmesg. */
> +	kmsg_error_detect(kmsg_fd, 100 * 1000, "");
> +
> +	stop = true;
> +	pthread_join(i2c_thread, &status);
> +	pthread_join(modeset_thread, &status);
> +
> +	/* Make sure we check everything after the threads have actually
> +	 * stopped. */
> +	kmsg_error_detect(kmsg_fd, 1 * 1000, "");
> +}
> +
> +igt_main
> +{
> +	igt_fixture {
> +		drm_fd = drm_open_any();
> +		igt_require(drm_fd >= 0);
> +
> +		res = drmModeGetResources(drm_fd);
> +		igt_assert(res);
> +
> +		kmsg_fd = kmsg_error_setup();
> +
> +		find_edp_connector();
> +	}
> +
> +	igt_subtest("i2c-modeset-vdd-race")
> +		i2c_modeset_vdd_race();
> +
> +	igt_fixture {
> +		kmsg_error_teardown(kmsg_fd);
> +		drmModeFreeConnector(edp_connector);
> +		drmModeFreeResources(res);
> +		close(drm_fd);
> +	}
> +}
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch