[Intel-gfx] [PATCH i-g-t] tests: add pc8
Paulo Zanoni
przanoni at gmail.com
Mon Aug 5 15:42:08 CEST 2013
2013/8/5 Daniel Vetter <daniel at ffwll.ch>:
> 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.
Notice I added the test to noinst_PROGRAMS, so QA won't be running it
for now, even if we merge it. Why? Because of these restrictions
mentioned above.
> Does comparing the difference of the
> registers not work instead of checking for 0?
Can you please elaborate more on this sentence? I'm not sure what
you're suggesting here.
The problem is: let's imagine there's some important register that we
initialize when starting the driver, but then we don't touch it
anymore. And this register is one of the registers that get reset when
we enter PC8, but our code doesn't fix it after leaving PC8. So if you
boot your machine, do something to allow PC8+ and then disallow it,
the register will go back to the "reset" state (which isn't guaranteed
to be 0x0, especially on display registers). Then you run tests/pc8:
it reads the register (which contains the "reset" value instead of the
real value set by our driver when it got loaded, because we already
entered PC8+ once), enters PC8+, leaves PC8+, reads the register
again, notices the value is the same and then gives us a "PASS". On
the other hand, if you didn't reach PC8+ before running tests/pc8,
then it would have noticed the value of the register changes.
In other words: if some register gets initialized by our driver to a
non-default value, but this value gets lost after we enter/leave PC8+,
we will *only* be able to notice the difference when comparing a state
where we *never* entered PC8+ against a state where we "already
entered PC8+ at least once", because after the first time you
enter/leave PC8+ you'll already have reset the register, so you'll be
comparing bugged state against bugged state.
>
>> - 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.
Shouldn't we add some sort of debugfs interface to read/write
registers then? We don't want to hang the machine while running
intel_reg_dumper and all our other tools... These tests are important
since they catch bugs that actually *happened* while I was developing
this feature. Most of the other tests are for things people have
suggested, not bugs that really happened...
>
>> +
>> +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
This is not possible since the whole goal is to compare the state
"before we have ever entered PC8+" with "after we have entered PC8+".
See the big explanation above. But I could try to split the "collect
data" with the "compare data" steps, so the "compare data" could
become the individual tests you requested.
>
> 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.
That's why we have functions that sleep for up to 10 seconds waiting
for the PC8+ residency to move. If we take too much time to go back to
PC8, then we should increase those 10second timeouts. In fact we could
make those timeouts become even more than 1 minute since we expect to
return before that (i.e., when the PC8+ residency moves).
> 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
--
Paulo Zanoni
More information about the Intel-gfx
mailing list