[Intel-gfx] [PATCH i-g-t] tests: add pc8

Daniel Vetter daniel at ffwll.ch
Mon Aug 5 08:05:51 CEST 2013


On Mon, Jul 29, 2013 at 05:53:27PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> This test chekcs our code that enables Package C8+. The environment
> requirements for this test are quite complicated:
>   - The machine needs to be properly configured to reach PC8+ when
>     possible, which means all the power management policies and device
>     drivers must be properly configured.
>   - Before running the test, the machine needs to have 0 PC8+
>     residency. If you run the test after the machine already has PC8+
>     residency, it means you'll be comparing post-PC8+ with post-PC8+,
>     so you won't be able to get things lost after the first time we
>     enter PC8+.
>       - This means that after you run the test once, you have to
> 	reboot, unless you use the "--skip-zero-pc8-check" option, but
> 	you really need to know what you're doing.

This needs to be made more robust since I want to include this test into
the normal -nightly testruns. Does comparing the difference of the
registers not work instead of checking for 0?

>   - You need at least one output connected.

The test should skip (retun 77;) if that's the case.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  tests/.gitignore  |   1 +
>  tests/Makefile.am |   1 +
>  tests/pc8.c       | 555 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 557 insertions(+)
>  create mode 100644 tests/pc8.c
> 
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 451ab2d..c70fdb7 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -90,6 +90,7 @@ getstats
>  getversion
>  kms_flip
>  kms_render
> +pc8
>  prime_nv_api
>  prime_nv_pcopy
>  prime_nv_test
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index a59c25f..3507716 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -2,6 +2,7 @@ if BUILD_TESTS
>  noinst_PROGRAMS = \
>  	gem_stress \
>  	ddi_compute_wrpll \
> +	pc8 \
>  	$(TESTS_progs) \
>  	$(TESTS_progs_M) \
>  	$(HANG) \
> diff --git a/tests/pc8.c b/tests/pc8.c
> new file mode 100644
> index 0000000..7fdfeb5
> --- /dev/null
> +++ b/tests/pc8.c
> @@ -0,0 +1,555 @@
> +/*
> + * 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 at intel.com>
> + *
> + */
> +
> +#include <stdio.h>
> +#include <assert.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <string.h>
> +
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include "drm.h"
> +#include "drmtest.h"
> +#include "intel_batchbuffer.h"
> +#include "intel_gpu_tools.h"
> +
> +#define MSR_PC8_RES	0x630
> +#define MSR_PC9_RES	0x631
> +#define MSR_PC10_RES	0x632
> +
> +#define MAX_CONNECTORS	32
> +#define MAX_ENCODERS	32
> +#define MAX_CRTCS	16
> +
> +int drm_fd;
> +
> +/* Stuff used when creating FBs and mode setting. */
> +struct mode_set_data {
> +	drmModeResPtr res;
> +	drmModeConnectorPtr connectors[MAX_CONNECTORS];
> +
> +	drm_intel_bufmgr *bufmgr;
> +	uint32_t devid;
> +};
> +
> +enum compare_step {
> +	STEP_RESET,
> +	STEP_RESTORED,
> +};
> +
> +/* Stuff we query at different times so we can compare. */
> +struct compare_data {
> +	drmModeResPtr res;
> +	drmModeEncoderPtr encoders[MAX_ENCODERS];
> +	drmModeConnectorPtr connectors[MAX_CONNECTORS];
> +	drmModeCrtcPtr crtcs[MAX_CRTCS];
> +	drmModePropertyBlobPtr edids[MAX_CONNECTORS];
> +
> +	struct {
> +		/* We know these are lost */
> +		uint32_t arb_mode;
> +		uint32_t tilectl;
> +
> +		/* Stuff touched at init_clock_gating, so we can make sure we
> +		 * don't need to call it when reiniting. */
> +		uint32_t gen6_ucgctl2;
> +		uint32_t gen7_l3cntlreg1;
> +		uint32_t transa_chicken1;
> +
> +		uint32_t deier;
> +		uint32_t gtier;
> +
> +		uint32_t ddi_buf_trans_a_1;
> +		uint32_t ddi_buf_trans_b_5;
> +		uint32_t ddi_buf_trans_c_10;
> +		uint32_t ddi_buf_trans_d_15;
> +		uint32_t ddi_buf_trans_e_20;
> +	} regs;
> +};
> +
> +static uint64_t get_residency(int fd, uint32_t type)
> +{
> +	int rc;
> +	uint64_t ret;
> +
> +	rc = pread(fd, &ret, sizeof(uint64_t), type);
> +	assert(rc == sizeof(uint64_t));
> +
> +	return ret;
> +}
> +
> +static bool pc8_plus_residency_is_zero(int fd)
> +{
> +	uint64_t res_pc8, res_pc9, res_pc10;
> +
> +	res_pc8 = get_residency(fd, MSR_PC8_RES);
> +	res_pc9 = get_residency(fd, MSR_PC9_RES);
> +	res_pc10 = get_residency(fd, MSR_PC10_RES);
> +
> +	if (res_pc8 != 0 || res_pc9 != 0 || res_pc10 != 0)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool pc8_plus_residency_changed(int fd, unsigned int timeout_sec)
> +{
> +	unsigned int i;
> +	uint64_t res_pc8, res_pc9, res_pc10;
> +	int to_sleep = 100 * 1000;
> +
> +	res_pc8 = get_residency(fd, MSR_PC8_RES);
> +	res_pc9 = get_residency(fd, MSR_PC9_RES);
> +	res_pc10 = get_residency(fd, MSR_PC10_RES);
> +
> +	for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
> +		if (res_pc8 != get_residency(fd, MSR_PC8_RES) ||
> +		    res_pc9 != get_residency(fd, MSR_PC9_RES) ||
> +		    res_pc10 != get_residency(fd, MSR_PC10_RES)) {
> +			return true;
> +		}
> +		usleep(to_sleep);
> +	}
> +
> +	return false;
> +}
> +
> +static void disable_all_screens(struct mode_set_data *data)
> +{
> +	int i, rc;
> +
> +	for (i = 0; i < data->res->count_crtcs; i++) {
> +		rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
> +				    NULL, 0, NULL);
> +		assert(rc == 0);
> +	}
> +}
> +
> +static uint32_t create_fb(struct mode_set_data *data, 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_one_screen(struct mode_set_data *data)
> +{
> +	uint32_t crtc_id = 0, buffer_id = 0, connector_id = 0;
> +	drmModeModeInfoPtr mode = NULL;
> +	int i, rc;
> +
> +	for (i = 0; i < data->res->count_connectors; i++) {
> +		drmModeConnectorPtr c = data->connectors[i];
> +
> +		if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
> +			connector_id = c->connector_id;
> +			mode = &c->modes[0];
> +			break;
> +		}
> +	}
> +
> +	crtc_id = data->res->crtcs[0];
> +	buffer_id = create_fb(data, mode->hdisplay, mode->vdisplay);
> +
> +	assert(crtc_id);
> +	assert(buffer_id);
> +	assert(connector_id);
> +	assert(mode);
> +
> +	rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, &connector_id,
> +			    1, mode);
> +	assert(rc == 0);
> +}
> +
> +static void get_connector_edid(struct compare_data *data, int index)
> +{
> +	unsigned int i;
> +	drmModeConnectorPtr connector = data->connectors[index];
> +	drmModeObjectPropertiesPtr props;
> +
> +	props = drmModeObjectGetProperties(drm_fd, connector->connector_id,
> +					   DRM_MODE_OBJECT_CONNECTOR);
> +
> +	for (i = 0; i < props->count_props; i++) {
> +		drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
> +							     props->props[i]);
> +
> +		if (strcmp(prop->name, "EDID") == 0) {
> +			assert(prop->flags & DRM_MODE_PROP_BLOB);
> +			assert(prop->count_blobs == 0);
> +			data->edids[index] = drmModeGetPropertyBlob(drm_fd,
> +							props->prop_values[i]);
> +		}
> +
> +		drmModeFreeProperty(prop);
> +	}
> +
> +	drmModeFreeObjectProperties(props);
> +}
> +
> +static void init_mode_set_data(struct mode_set_data *data)
> +{
> +	int i;
> +
> +	data->res = drmModeGetResources(drm_fd);
> +	assert(data->res);
> +	assert(data->res->count_connectors <= MAX_CONNECTORS);
> +
> +	for (i = 0; i < data->res->count_connectors; i++)
> +		data->connectors[i] = drmModeGetConnector(drm_fd,
> +						data->res->connectors[i]);
> +
> +	data->bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
> +	data->devid = intel_get_drm_devid(drm_fd);
> +
> +	do_or_die(drmtest_set_vt_graphics_mode());
> +	drm_intel_bufmgr_gem_enable_reuse(data->bufmgr);
> +}
> +
> +static void fini_mode_set_data(struct mode_set_data *data)
> +{
> +	int i;
> +
> +	drm_intel_bufmgr_destroy(data->bufmgr);
> +
> +	for (i = 0; i < data->res->count_connectors; i++)
> +		drmModeFreeConnector(data->connectors[i]);
> +	drmModeFreeResources(data->res);
> +}
> +
> +static void get_drm_info(struct compare_data *data)
> +{
> +	int i;
> +
> +	data->res = drmModeGetResources(drm_fd);
> +	assert(data->res);
> +
> +	assert(data->res->count_connectors <= MAX_CONNECTORS);
> +	assert(data->res->count_encoders <= MAX_ENCODERS);
> +	assert(data->res->count_crtcs <= MAX_CRTCS);
> +
> +	for (i = 0; i < data->res->count_connectors; i++) {
> +		data->connectors[i] = drmModeGetConnector(drm_fd,
> +						data->res->connectors[i]);
> +		get_connector_edid(data, i);
> +	}
> +	for (i = 0; i < data->res->count_encoders; i++)
> +		data->encoders[i] = drmModeGetEncoder(drm_fd,
> +						data->res->encoders[i]);
> +	for (i = 0; i < data->res->count_crtcs; i++)
> +		data->crtcs[i] = drmModeGetCrtc(drm_fd, data->res->crtcs[i]);
> +
> +	intel_register_access_init(intel_get_pci_device(), 0);
> +	data->regs.arb_mode = INREG(0x4030);
> +	data->regs.tilectl = INREG(0x101000);
> +	data->regs.gen6_ucgctl2 = INREG(0x9404);
> +	data->regs.gen7_l3cntlreg1 = INREG(0xB0C1);
> +	data->regs.transa_chicken1 = INREG(0xF0060);
> +	data->regs.deier = INREG(0x4400C);
> +	data->regs.gtier = INREG(0x4401C);
> +	data->regs.ddi_buf_trans_a_1 = INREG(0x64E00);
> +	data->regs.ddi_buf_trans_b_5 = INREG(0x64E70);
> +	data->regs.ddi_buf_trans_c_10 = INREG(0x64EE0);
> +	data->regs.ddi_buf_trans_d_15 = INREG(0x64F58);
> +	data->regs.ddi_buf_trans_e_20 = INREG(0x64FCC);
> +	intel_register_access_fini();
> +}
> +
> +static void free_drm_info(struct compare_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < data->res->count_connectors; i++) {
> +		drmModeFreeConnector(data->connectors[i]);
> +		drmModeFreePropertyBlob(data->edids[i]);
> +	}
> +	for (i = 0; i < data->res->count_encoders; i++)
> +		drmModeFreeEncoder(data->encoders[i]);
> +	for (i = 0; i < data->res->count_crtcs; i++)
> +		drmModeFreeCrtc(data->crtcs[i]);
> +
> +	drmModeFreeResources(data->res);
> +}
> +
> +#define COMPARE(d1, d2, data) assert(d1->data == d2->data)
> +#define COMPARE_ARRAY(d1, d2, size, data) do { \
> +	for (i = 0; i < size; i++) \
> +		assert(d1->data[i] == d2->data[i]); \
> +} while (0)
> +
> +static void assert_drm_resources_equal(struct compare_data *d1,
> +				       struct compare_data *d2)
> +{
> +	COMPARE(d1, d2, res->count_connectors);
> +	COMPARE(d1, d2, res->count_encoders);
> +	COMPARE(d1, d2, res->count_crtcs);
> +	COMPARE(d1, d2, res->min_width);
> +	COMPARE(d1, d2, res->max_width);
> +	COMPARE(d1, d2, res->min_height);
> +	COMPARE(d1, d2, res->max_height);
> +}
> +
> +static void assert_modes_equal(drmModeModeInfoPtr m1, drmModeModeInfoPtr m2)
> +{
> +	COMPARE(m1, m2, clock);
> +	COMPARE(m1, m2, hdisplay);
> +	COMPARE(m1, m2, hsync_start);
> +	COMPARE(m1, m2, hsync_end);
> +	COMPARE(m1, m2, htotal);
> +	COMPARE(m1, m2, hskew);
> +	COMPARE(m1, m2, vdisplay);
> +	COMPARE(m1, m2, vsync_start);
> +	COMPARE(m1, m2, vsync_end);
> +	COMPARE(m1, m2, vtotal);
> +	COMPARE(m1, m2, vscan);
> +	COMPARE(m1, m2, vrefresh);
> +	COMPARE(m1, m2, flags);
> +	COMPARE(m1, m2, type);
> +	assert(strcmp(m1->name, m2->name) == 0);
> +}
> +
> +static void assert_drm_connectors_equal(drmModeConnectorPtr c1,
> +					drmModeConnectorPtr c2)
> +{
> +	int i;
> +
> +	COMPARE(c1, c2, connector_id);
> +	COMPARE(c1, c2, connector_type);
> +	COMPARE(c1, c2, connector_type_id);
> +	COMPARE(c1, c2, mmWidth);
> +	COMPARE(c1, c2, mmHeight);
> +	COMPARE(c1, c2, count_modes);
> +	COMPARE(c1, c2, count_props);
> +	COMPARE(c1, c2, count_encoders);
> +	COMPARE_ARRAY(c1, c2, c1->count_props, props);
> +	COMPARE_ARRAY(c1, c2, c1->count_encoders, encoders);
> +
> +	for (i = 0; i < c1->count_modes; i++)
> +		assert_modes_equal(&c1->modes[0], &c2->modes[0]);
> +}
> +
> +static void assert_drm_encoders_equal(drmModeEncoderPtr e1,
> +				      drmModeEncoderPtr e2)
> +{
> +	COMPARE(e1, e2, encoder_id);
> +	COMPARE(e1, e2, encoder_type);
> +	COMPARE(e1, e2, possible_crtcs);
> +	COMPARE(e1, e2, possible_clones);
> +}
> +
> +static void assert_drm_crtcs_equal(drmModeCrtcPtr c1, drmModeCrtcPtr c2)
> +{
> +	COMPARE(c1, c2, crtc_id);
> +}
> +
> +static void assert_drm_edids_equal(drmModePropertyBlobPtr e1,
> +				   drmModePropertyBlobPtr e2)
> +{
> +	if (!e1 && !e2)
> +		return;
> +	assert(e1 && e2);
> +
> +	COMPARE(e1, e2, id);
> +	COMPARE(e1, e2, length);
> +
> +	assert(memcmp(e1->data, e2->data, e1->length) == 0);
> +}

Imo the important part is just checking whether the EDID is still the
same, but I guess we can keep all the other checks, too.

> +
> +static void compare_registers(struct compare_data *d1, struct compare_data *d2,
> +			      enum compare_step step)
> +{
> +	/* Things that shouldn't change at all. */
> +	COMPARE(d1, d2, regs.gen6_ucgctl2);
> +	COMPARE(d1, d2, regs.gen7_l3cntlreg1);
> +	COMPARE(d1, d2, regs.transa_chicken1);
> +	COMPARE(d1, d2, regs.arb_mode);
> +	COMPARE(d1, d2, regs.tilectl);
> +	assert(d1->regs.deier & (1 << 31));
> +	assert(d2->regs.deier & (1 << 31));
> +
> +	switch (step) {
> +	case STEP_RESET:
> +		assert(d2->regs.gtier == 0);
> +		assert(d2->regs.ddi_buf_trans_a_1 == 0);
> +		assert(d2->regs.ddi_buf_trans_b_5 == 0);
> +		assert(d2->regs.ddi_buf_trans_c_10 == 0);
> +		assert(d2->regs.ddi_buf_trans_d_15 == 0);
> +		assert(d2->regs.ddi_buf_trans_e_20 == 0);
> +		break;
> +	case STEP_RESTORED:
> +		COMPARE(d1, d2, regs.arb_mode);
> +		COMPARE(d1, d2, regs.tilectl);
> +		COMPARE(d1, d2, regs.gtier);
> +		COMPARE(d1, d2, regs.ddi_buf_trans_a_1);
> +		COMPARE(d1, d2, regs.ddi_buf_trans_b_5);
> +		COMPARE(d1, d2, regs.ddi_buf_trans_c_10);
> +		COMPARE(d1, d2, regs.ddi_buf_trans_d_15);
> +		COMPARE(d1, d2, regs.ddi_buf_trans_e_20);
> +		break;
> +	default:
> +		assert(0);
> +	}
> +}

This is really risky since we've just demonstrated that we can hard-hang
hsw if someone concurrently accesses registers. I guess hiding this behind
a debug switch would be good.

> +
> +static void assert_drm_infos_equal(struct compare_data *d1,
> +				   struct compare_data *d2,
> +				   enum compare_step step)
> +{
> +	int i;
> +
> +	assert_drm_resources_equal(d1, d2);
> +
> +	for (i = 0; i < d1->res->count_connectors; i++) {
> +		assert_drm_connectors_equal(d1->connectors[i],
> +					    d2->connectors[i]);
> +		assert_drm_edids_equal(d1->edids[i], d2->edids[i]);
> +	}
> +
> +	for (i = 0; i < d1->res->count_encoders; i++)
> +		assert_drm_encoders_equal(d1->encoders[i], d2->encoders[i]);
> +
> +	for (i = 0; i < d1->res->count_crtcs; i++)
> +		assert_drm_crtcs_equal(d1->crtcs[i], d2->crtcs[i]);
> +
> +	compare_registers(d1, d2, step);
> +}
> +
> +#define BUF_SIZE	(8 << 20)
> +
> +/* to avoid stupid depencies on libdrm, copy&paste */
> +struct local_drm_i915_gem_wait {
> +	/** Handle of BO we shall wait on */
> +	__u32 bo_handle;
> +	__u32 flags;
> +	/** Number of nanoseconds to wait, Returns time remaining. */
> +	__u64 timeout_ns;
> +};
> +#define WAIT_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2c, struct local_drm_i915_gem_wait)
> +
> +static int gem_bo_wait_timeout(int fd, uint32_t handle, uint64_t *timeout_ns)
> +{
> +	struct local_drm_i915_gem_wait wait;
> +	int ret;
> +
> +	assert(timeout_ns);
> +
> +	wait.bo_handle = handle;
> +	wait.timeout_ns = *timeout_ns;
> +	wait.flags = 0;
> +	ret = drmIoctl(fd, WAIT_IOCTL, &wait);
> +	*timeout_ns = wait.timeout_ns;
> +
> +	return ret ? -errno : 0;
> +}

This should be extracted to lib/drmtest.c for all testcases.

> +
> +static void test_batch(struct mode_set_data *data)
> +{
> +	uint64_t timeout = 1000 * 1000 * 1000 * 2;
> +	drm_intel_bo *dst;
> +
> +	dst = drm_intel_bo_alloc(data->bufmgr, "dst", BUF_SIZE, 4096);
> +	if (gem_bo_wait_timeout(drm_fd, dst->handle, &timeout) == -EINVAL)
> +		printf("Kernel doesn't support wait_timeout\n");
> +}

Since I don't submit a "submit a batch" anywhere this won't actually test
much ...

> +
> +int main(int argc, char *argv[])
> +{
> +	struct mode_set_data ms_data;
> +	struct compare_data pre_pc8, during_pc8, post_pc8;
> +	int msr_fd;
> +	bool skip_zero_pc8_check = false;
> +
> +	if (argc > 1) {
> +		/* With this option we can test/debug/develop the tool without
> +		 * needing to reboot every time. */
> +		if (strcmp(argv[1], "--skip-zero-pc8-check") == 0)
> +			skip_zero_pc8_check = true;
> +	}
> +
> +	drm_fd = drm_open_any();
> +	assert(drm_fd > 0);
> +
> +	init_mode_set_data(&ms_data);
> +
> +	if (!IS_HASWELL(ms_data.devid)) {
> +		printf("PC8+ feature only supported on Haswell.\n");
> +		return 77;
> +	}
> +
> +	msr_fd = open("/dev/cpu/0/msr", O_RDONLY);
> +	assert(msr_fd != -1);
> +
> +	/* Make sure the machine still didn't enter PC8+, otherwise we won't be
> +	 * able to really compare pre-PC8+ with post-PC8+. */
> +	printf("Pre-PC8+\n");
> +	assert(skip_zero_pc8_check || pc8_plus_residency_is_zero(msr_fd));
> +	get_drm_info(&pre_pc8);
> +	test_batch(&ms_data);
> +	assert(skip_zero_pc8_check || pc8_plus_residency_is_zero(msr_fd));
> +
> +	printf("PC8+\n");
> +	disable_all_screens(&ms_data);
> +	sleep(1);
> +
> +	assert(pc8_plus_residency_changed(msr_fd, 10));
> +	get_drm_info(&during_pc8);
> +	test_batch(&ms_data);
> +	assert(pc8_plus_residency_changed(msr_fd, 10));

Imo we should test each functional piece seperately in it's own loop, i.e.

for each foo in test_batch, test_edid_connector1, test_edid_connector2,
... ; do
	enter pc8+
	sleep 1
	run $foo
	exit pc8+
done

In the current implementation this doesn't matter, but if we change it
so that some of the tests will exit pc8+ and if we furthermore add a
slight delay for reentering pc8+ then the test won't test a lot any more.
So I think we need this for test robustness.

Also I think we should add a basic raw i2c transaction probe test. There
are a bunch of platforms out there which use GMBUS for other fun stuff
than reading EDIDs from external screens.

I think it would be good to also convert this test to the subtest
infrastructure so that QA can track the different pieces better. Note
though that the list of subtests can't change at runtime, so we can only
have one generic edid/raw-i2c subtest.

Cheers, Daniel

> +
> +	assert_drm_infos_equal(&pre_pc8, &during_pc8, STEP_RESET);
> +
> +	printf("Post-PC8+\n");
> +	enable_one_screen(&ms_data);
> +	sleep(1);
> +
> +	assert(!pc8_plus_residency_changed(msr_fd, 5));
> +	get_drm_info(&post_pc8);
> +	test_batch(&ms_data);
> +	assert(!pc8_plus_residency_changed(msr_fd, 5));
> +
> +	assert_drm_infos_equal(&pre_pc8, &post_pc8, STEP_RESTORED);
> +
> +	free_drm_info(&pre_pc8);
> +	free_drm_info(&during_pc8);
> +	free_drm_info(&post_pc8);
> +	fini_mode_set_data(&ms_data);
> +	close(msr_fd);
> +	drmClose(drm_fd);
> +
> +	printf("Done!\n");
> +	return 0;
> +}
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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



More information about the Intel-gfx mailing list