* [igt-dev] [PATCH i-g-t v4 1/2] lib: Require drmModeResources()
@ 2018-09-13 20:39 José Roberto de Souza
2018-09-13 20:39 ` [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display José Roberto de Souza
0 siblings, 1 reply; 4+ messages in thread
From: José Roberto de Souza @ 2018-09-13 20:39 UTC (permalink / raw)
To: igt-dev
From: Chris Wilson <chris@chris-wilson.co.uk>
If modesetting is not supported, the drmModeGetResources() call will
fail with -EINVAL (or -ENOTSUPP). As no displays are connected, skip.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4563bfd9..c20cf1eb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1857,7 +1857,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
display->drm_fd = drm_fd;
resources = drmModeGetResources(display->drm_fd);
- igt_assert(resources);
+ igt_require(resources);
/*
* We cache the number of pipes, that number is a physical limit of the
--
2.19.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display
2018-09-13 20:39 [igt-dev] [PATCH i-g-t v4 1/2] lib: Require drmModeResources() José Roberto de Souza
@ 2018-09-13 20:39 ` José Roberto de Souza
2018-09-13 21:02 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: José Roberto de Souza @ 2018-09-13 20:39 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
'lib: Require drmModeResources()' take care of all tests that calls
igt_display_init(), here handling other tests that needs display and
do not call it.
v2:
- Not skipping all tests in debugfs_test, perf_pmu and perf_pmu,
now skiping only the required subtests
v3:
- Renamed igt_require_display to igt_display_required, to keep naming
consistent
- Added igt_display_available()
- Checking if display is available by quering a modeset capability
instead of drmModeGetResources()
- Not skiping read_all_entries tests when display is disabled
v4:
- Using 'lib: Require drmModeResources()', so all tests already
handled by that patch had the igt_display_require() call removed.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
lib/igt_kms.c | 30 +++++++++++++-
lib/igt_kms.h | 2 +
tests/debugfs_test.c | 15 +++++--
tests/kms_3d.c | 1 +
tests/kms_addfb_basic.c | 4 +-
tests/kms_draw_crc.c | 1 +
tests/kms_fbcon_fbt.c | 1 +
tests/kms_force_connector_basic.c | 1 +
tests/kms_getfb.c | 4 +-
tests/kms_hdmi_inject.c | 1 +
tests/kms_setmode.c | 1 +
tests/kms_sysfs_edid_timing.c | 5 +++
tests/kms_tv_load_detect.c | 1 +
tests/perf_pmu.c | 16 +++----
tests/pm_lpsp.c | 1 +
tests/pm_rpm.c | 69 ++++++++++++++++++++++++-------
tests/prime_vgem.c | 2 +
tests/testdisplay.c | 1 +
18 files changed, 129 insertions(+), 27 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c20cf1eb..2bc92c81 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2018,6 +2018,34 @@ int igt_display_get_n_pipes(igt_display_t *display)
return display->n_pipes;
}
+/**
+ * igt_display_require:
+ * @drm_fd: a drm file descriptor
+ *
+ * Checks if driver supports modeset and have display enabled.
+ */
+void igt_display_require(int fd)
+{
+ bool available = igt_display_available(fd);
+ igt_require_f(available, "drm driver do not support modeset\n");
+}
+
+/**
+ * igt_display_available:
+ * @drm_fd: a drm file descriptor
+ *
+ * Return true if driver supports modeset and have display enabled.
+ */
+bool igt_display_available(int fd)
+{
+ uint64_t cap;
+
+ /* Check if driver support modeset by checking one of the modeset
+ * specific capabilities, in case of error it is not supported.
+ */
+ return !drmGetCap(fd, DRM_CAP_DUMB_BUFFER, &cap);
+}
+
/**
* igt_display_require_output:
* @display: A pointer to an #igt_display_t structure
@@ -3895,7 +3923,7 @@ void igt_enable_connectors(void)
drm_fd = drm_open_driver(DRIVER_ANY);
res = drmModeGetResources(drm_fd);
- igt_assert(res != NULL);
+ igt_require(res != NULL);
for (int i = 0; i < res->count_connectors; i++) {
drmModeConnector *c;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 3862efa2..feafeda9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -388,6 +388,8 @@ void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *use
int igt_display_try_commit2(igt_display_t *display, enum igt_commit_style s);
int igt_display_drop_events(igt_display_t *display);
int igt_display_get_n_pipes(igt_display_t *display);
+void igt_display_require(int fd);
+bool igt_display_available(int fd);
void igt_display_require_output(igt_display_t *display);
void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe);
diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
index 2e87e442..79243b1a 100644
--- a/tests/debugfs_test.c
+++ b/tests/debugfs_test.c
@@ -102,10 +102,16 @@ igt_main
debugfs = igt_debugfs_dir(fd);
kmstest_set_vt_graphics_mode();
- igt_display_init(&display, fd);
+ if (igt_display_available(fd))
+ igt_display_init(&display, fd);
+ else
+ display.n_pipes = 0;
}
igt_subtest("read_all_entries") {
+ if (!display.n_pipes)
+ goto skip_modeset;
+
/* try to light all pipes */
for_each_pipe(&display, pipe) {
igt_output_t *output;
@@ -132,7 +138,7 @@ igt_main
}
igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
-
+skip_modeset:
read_and_discard_sysfs_entries(debugfs, 0);
}
@@ -140,6 +146,9 @@ igt_main
igt_output_t *output;
igt_plane_t *plane;
+ if (!display.n_pipes)
+ goto skip_modeset2;
+
for_each_connected_output(&display, output)
igt_output_set_pipe(output, PIPE_NONE);
@@ -148,7 +157,7 @@ igt_main
igt_plane_set_fb(plane, NULL);
igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
-
+skip_modeset2:
read_and_discard_sysfs_entries(debugfs, 0);
}
diff --git a/tests/kms_3d.c b/tests/kms_3d.c
index bfc981ee..18764c41 100644
--- a/tests/kms_3d.c
+++ b/tests/kms_3d.c
@@ -36,6 +36,7 @@ igt_simple_main
int mode_count, connector_id;
drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ igt_display_require(drm_fd);
res = drmModeGetResources(drm_fd);
igt_assert(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) >= 0);
diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index ce48d24f..5d3f9b85 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -671,8 +671,10 @@ int fd;
igt_main
{
- igt_fixture
+ igt_fixture {
fd = drm_open_driver_master(DRIVER_ANY);
+ igt_display_require(fd);
+ }
invalid_tests(fd);
diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index 86dcf392..51c92b71 100644
--- a/tests/kms_draw_crc.c
+++ b/tests/kms_draw_crc.c
@@ -251,6 +251,7 @@ static void setup_environment(void)
drm_fd = drm_open_driver_master(DRIVER_INTEL);
igt_require(drm_fd >= 0);
+ igt_display_require(drm_fd);
drm_res = drmModeGetResources(drm_fd);
igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 138eda9b..017be053 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -293,6 +293,7 @@ static void setup_environment(void)
drm_fd = drm_open_driver_master(DRIVER_INTEL);
igt_require(drm_fd >= 0);
+ igt_display_require(drm_fd);
igt_assert(close(drm_fd) == 0);
}
diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
index 89431232..720d79bd 100644
--- a/tests/kms_force_connector_basic.c
+++ b/tests/kms_force_connector_basic.c
@@ -89,6 +89,7 @@ int main(int argc, char **argv)
unsigned vga_connector_id = 0;
drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ igt_display_require(drm_fd);
res = drmModeGetResources(drm_fd);
igt_assert(res);
diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
index 81d796a4..67088883 100644
--- a/tests/kms_getfb.c
+++ b/tests/kms_getfb.c
@@ -198,8 +198,10 @@ igt_main
{
int fd;
- igt_fixture
+ igt_fixture {
fd = drm_open_driver_master(DRIVER_ANY);
+ igt_display_require(fd);
+ }
igt_subtest_group
test_handle_input(fd);
diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index 22570a4b..66ed2094 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -254,6 +254,7 @@ igt_main
igt_fixture {
drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ igt_display_require(drm_fd);
res = drmModeGetResources(drm_fd);
connector = get_connector(drm_fd, res);
diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 47d04fb5..93d9a76d 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -854,6 +854,7 @@ int main(int argc, char **argv)
igt_fixture {
drm_fd = drm_open_driver_master(DRIVER_ANY);
+ igt_display_require(drm_fd);
if (!dry_run)
kmstest_set_vt_graphics_mode();
diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
index 12013881..2a3350fe 100644
--- a/tests/kms_sysfs_edid_timing.c
+++ b/tests/kms_sysfs_edid_timing.c
@@ -41,6 +41,11 @@ igt_simple_main
{
DIR *dirp;
struct dirent *de;
+ int drm_fd;
+
+ drm_fd = drm_open_driver_master(DRIVER_ANY);
+ igt_display_require(drm_fd);
+ close(drm_fd);
dirp = opendir("/sys/class/drm");
igt_assert(dirp != NULL);
diff --git a/tests/kms_tv_load_detect.c b/tests/kms_tv_load_detect.c
index 5684b267..a05afd15 100644
--- a/tests/kms_tv_load_detect.c
+++ b/tests/kms_tv_load_detect.c
@@ -37,6 +37,7 @@ int main(int argc, char **argv)
igt_fixture {
drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ igt_display_require(drm_fd);
res = drmModeGetResources(drm_fd);
igt_assert(res);
diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 6ab2595b..83dd9ee2 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -1395,15 +1395,17 @@ test_rc6(int gem_fd, unsigned int flags)
fd = open_pmu(I915_PMU_RC6_RESIDENCY);
if (flags & TEST_RUNTIME_PM) {
- drmModeRes *res;
+ if (igt_display_available(gem_fd)) {
+ drmModeRes *res;
- res = drmModeGetResources(gem_fd);
- igt_assert(res);
+ res = drmModeGetResources(gem_fd);
+ igt_assert(res);
- /* force all connectors off */
- kmstest_set_vt_graphics_mode();
- kmstest_unset_all_crtcs(gem_fd, res);
- drmModeFreeResources(res);
+ /* force all connectors off */
+ kmstest_set_vt_graphics_mode();
+ kmstest_unset_all_crtcs(gem_fd, res);
+ drmModeFreeResources(res);
+ }
igt_require(igt_setup_runtime_pm());
igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
index a741cb78..01c8f79e 100644
--- a/tests/pm_lpsp.c
+++ b/tests/pm_lpsp.c
@@ -195,6 +195,7 @@ igt_main
drm_fd = drm_open_driver_master(DRIVER_INTEL);
igt_require(drm_fd >= 0);
+ igt_display_require(drm_fd);
devid = intel_get_drm_devid(drm_fd);
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index c24fd95b..cf38596b 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -222,6 +222,9 @@ static void disable_all_screens_dpms(struct mode_set_data *data)
{
int i;
+ if (!data->res)
+ return;
+
for (i = 0; i < data->res->count_connectors; i++) {
drmModeConnectorPtr c = data->connectors[i];
@@ -231,6 +234,9 @@ static void disable_all_screens_dpms(struct mode_set_data *data)
static void disable_all_screens(struct mode_set_data *data)
{
+ if (!data->res)
+ return;
+
kmstest_unset_all_crtcs(drm_fd, data->res);
}
@@ -329,6 +335,8 @@ static bool enable_one_screen_with_type(struct mode_set_data *data,
{
struct modeset_params *params = NULL;
+ igt_display_require(drm_fd);
+
switch (type) {
case SCREEN_TYPE_ANY:
params = default_mode_params;
@@ -388,6 +396,11 @@ static void init_mode_set_data(struct mode_set_data *data)
{
int i;
+ data->devid = intel_get_drm_devid(drm_fd);
+
+ if (!igt_display_available(drm_fd))
+ return;
+
data->res = drmModeGetResources(drm_fd);
igt_assert(data->res);
igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
@@ -398,8 +411,6 @@ static void init_mode_set_data(struct mode_set_data *data)
data->edids[i] = get_connector_edid(data->connectors[i], i);
}
- data->devid = intel_get_drm_devid(drm_fd);
-
kmstest_set_vt_graphics_mode();
init_modeset_cached_params(&ms_data);
@@ -409,11 +420,15 @@ static void fini_mode_set_data(struct mode_set_data *data)
{
int i;
+ if (!data->res)
+ return;
+
for (i = 0; i < data->res->count_connectors; i++) {
drmModeFreeConnector(data->connectors[i]);
drmModeFreePropertyBlob(data->edids[i]);
}
drmModeFreeResources(data->res);
+ data->res = NULL;
}
static void get_drm_info(struct compare_data *data)
@@ -574,6 +589,9 @@ static int count_drm_valid_edids(struct mode_set_data *data)
{
int i, ret = 0;
+ if (!data->res)
+ return ret;
+
for (i = 0; i < data->res->count_connectors; i++)
if (data->edids[i] && edid_is_valid(data->edids[i]->data))
ret++;
@@ -637,6 +655,9 @@ static int count_vga_outputs(struct mode_set_data *data)
{
int i, count = 0;
+ if (!data->res)
+ return count;
+
for (i = 0; i < data->res->count_connectors; i++)
if (data->connectors[i]->connector_type ==
DRM_MODE_CONNECTOR_VGA)
@@ -730,7 +751,8 @@ static bool setup_environment(void)
igt_info("Runtime PM support: %d\n", has_runtime_pm);
igt_info("PC8 residency support: %d\n", has_pc8);
igt_require(has_runtime_pm);
- igt_require(dmc_loaded());
+ if (ms_data.res)
+ igt_require(dmc_loaded());
out:
disable_all_screens(&ms_data);
@@ -773,6 +795,9 @@ static void pc8_residency_subtest(void)
"Machine is not reaching PC8+ states, please check its "
"configuration.\n");
+ if (!ms_data.res)
+ return;
+
/* Make sure PC8+ residencies stop! */
enable_one_screen(&ms_data);
igt_assert_f(!pc8_plus_residency_changed(10),
@@ -783,6 +808,8 @@ static void modeset_subtest(enum screen_type type, int rounds, int wait_flags)
{
int i;
+ igt_display_require(drm_fd);
+
if (wait_flags & WAIT_PC8_RES)
igt_require(has_pc8);
@@ -864,6 +891,7 @@ static void i2c_subtest_check_environment(void)
/* Try to use raw I2C, which also needs interrupts. */
static void i2c_subtest(void)
{
+ igt_display_require(drm_fd);
i2c_subtest_check_environment();
enable_one_screen_and_wait(&ms_data);
@@ -967,7 +995,8 @@ static void gem_mmap_subtest(bool gtt_mmap)
uint8_t *gem_buf;
/* Create, map and set data while the device is active. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
handle = gem_create(drm_fd, buf_size);
@@ -998,7 +1027,8 @@ static void gem_mmap_subtest(bool gtt_mmap)
igt_assert(wait_for_suspended());
/* Now resume and see if it's still there. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
for (i = 0; i < buf_size; i++)
igt_assert(gem_buf[i] == (~i & 0xFF));
@@ -1027,7 +1057,8 @@ static void gem_mmap_subtest(bool gtt_mmap)
igt_assert(wait_for_suspended());
/* Resume and check if it's still there. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
for (i = 0; i < buf_size; i++)
igt_assert(gem_buf[i] == (i & 0xFF));
@@ -1050,7 +1081,8 @@ static void gem_pread_subtest(void)
memset(read_buf, 0, buf_size);
/* Create and set data while the device is active. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
handle = gem_create(drm_fd, buf_size);
@@ -1080,7 +1112,8 @@ static void gem_pread_subtest(void)
igt_assert(wait_for_suspended());
/* Now resume and see if it's still there. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
memset(read_buf, 0, buf_size);
gem_read(drm_fd, handle, 0, read_buf, buf_size);
@@ -1187,7 +1220,8 @@ static void gem_execbuf_subtest(void)
uint32_t color;
/* Create and set data while the device is active. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
handle = gem_create(drm_fd, dst_size);
@@ -1219,7 +1253,8 @@ static void gem_execbuf_subtest(void)
}
/* Now resume and check for it again. */
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
memset(cpu_buf, 0, dst_size);
gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
@@ -1388,8 +1423,10 @@ static void pci_d3_state_subtest(void)
disable_all_screens_and_wait(&ms_data);
igt_assert(igt_wait(device_in_pci_d3(), 2000, 100));
- enable_one_screen_and_wait(&ms_data);
- igt_assert(!device_in_pci_d3());
+ if (ms_data.res) {
+ enable_one_screen_and_wait(&ms_data);
+ igt_assert(!device_in_pci_d3());
+ }
}
static void __attribute__((noreturn)) stay_subtest(void)
@@ -1453,7 +1490,8 @@ static void system_suspend_modeset_subtest(void)
igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
igt_assert(wait_for_suspended());
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
disable_all_screens_and_wait(&ms_data);
}
@@ -1461,6 +1499,8 @@ static void system_suspend_modeset_subtest(void)
* produced WARNs on this case. */
static void dpms_mode_unset_subtest(enum screen_type type)
{
+ igt_display_require(drm_fd);
+
disable_all_screens_and_wait(&ms_data);
igt_require(enable_one_screen_with_type(&ms_data, type));
@@ -1783,7 +1823,8 @@ static void pm_test_tiling(void)
igt_assert(tiling_modes[i] == ti);
}
- enable_one_screen_and_wait(&ms_data);
+ if (ms_data.res)
+ enable_one_screen_and_wait(&ms_data);
for (j = 0, k = 1 << off_bit;
k <= gtt_obj_max_size; k <<= 1, j++) {
diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
index b821fbb8..399e4ebe 100644
--- a/tests/prime_vgem.c
+++ b/tests/prime_vgem.c
@@ -747,6 +747,8 @@ static void test_flip(int i915, int vgem, unsigned hang)
struct vgem_bo bo[2];
uint32_t fb_id[2], handle[2], crtc_id;
+ igt_display_require(i915);
+
signal(SIGHUP, sighandler);
for (int i = 0; i < 2; i++) {
diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 0ff98a2b..28f64a75 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -716,6 +716,7 @@ int main(int argc, char **argv)
test_all_modes = 1;
drm_fd = drm_open_driver(DRIVER_ANY);
+ igt_display_require(drm_fd);
if (test_stereo_modes &&
drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) < 0) {
--
2.19.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display
2018-09-13 20:39 ` [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display José Roberto de Souza
@ 2018-09-13 21:02 ` Chris Wilson
2018-09-13 23:15 ` Souza, Jose
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2018-09-13 21:02 UTC (permalink / raw)
To: José Roberto de Souza, igt-dev; +Cc: Jani Nikula
Quoting José Roberto de Souza (2018-09-13 21:39:35)
> 'lib: Require drmModeResources()' take care of all tests that calls
> igt_display_init(), here handling other tests that needs display and
> do not call it.
>
> v2:
> - Not skipping all tests in debugfs_test, perf_pmu and perf_pmu,
> now skiping only the required subtests
>
> v3:
> - Renamed igt_require_display to igt_display_required, to keep naming
> consistent
> - Added igt_display_available()
> - Checking if display is available by quering a modeset capability
> instead of drmModeGetResources()
> - Not skiping read_all_entries tests when display is disabled
>
> v4:
> - Using 'lib: Require drmModeResources()', so all tests already
> handled by that patch had the igt_display_require() call removed.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> lib/igt_kms.c | 30 +++++++++++++-
> lib/igt_kms.h | 2 +
> tests/debugfs_test.c | 15 +++++--
> tests/kms_3d.c | 1 +
> tests/kms_addfb_basic.c | 4 +-
> tests/kms_draw_crc.c | 1 +
> tests/kms_fbcon_fbt.c | 1 +
> tests/kms_force_connector_basic.c | 1 +
> tests/kms_getfb.c | 4 +-
> tests/kms_hdmi_inject.c | 1 +
> tests/kms_setmode.c | 1 +
> tests/kms_sysfs_edid_timing.c | 5 +++
> tests/kms_tv_load_detect.c | 1 +
> tests/perf_pmu.c | 16 +++----
> tests/pm_lpsp.c | 1 +
> tests/pm_rpm.c | 69 ++++++++++++++++++++++++-------
> tests/prime_vgem.c | 2 +
> tests/testdisplay.c | 1 +
> 18 files changed, 129 insertions(+), 27 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index c20cf1eb..2bc92c81 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2018,6 +2018,34 @@ int igt_display_get_n_pipes(igt_display_t *display)
> return display->n_pipes;
> }
>
> +/**
> + * igt_display_require:
> + * @drm_fd: a drm file descriptor
> + *
> + * Checks if driver supports modeset and have display enabled.
> + */
> +void igt_display_require(int fd)
> +{
> + bool available = igt_display_available(fd);
> + igt_require_f(available, "drm driver do not support modeset\n");
> +}
> +
> +/**
> + * igt_display_available:
> + * @drm_fd: a drm file descriptor
> + *
> + * Return true if driver supports modeset and have display enabled.
> + */
> +bool igt_display_available(int fd)
> +{
> + uint64_t cap;
> +
> + /* Check if driver support modeset by checking one of the modeset
> + * specific capabilities, in case of error it is not supported.
> + */
> + return !drmGetCap(fd, DRM_CAP_DUMB_BUFFER, &cap);
> +}
> +
> /**
> * igt_display_require_output:
> * @display: A pointer to an #igt_display_t structure
> @@ -3895,7 +3923,7 @@ void igt_enable_connectors(void)
> drm_fd = drm_open_driver(DRIVER_ANY);
>
> res = drmModeGetResources(drm_fd);
> - igt_assert(res != NULL);
> + igt_require(res != NULL);
>
> for (int i = 0; i < res->count_connectors; i++) {
> drmModeConnector *c;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 3862efa2..feafeda9 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -388,6 +388,8 @@ void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *use
> int igt_display_try_commit2(igt_display_t *display, enum igt_commit_style s);
> int igt_display_drop_events(igt_display_t *display);
> int igt_display_get_n_pipes(igt_display_t *display);
> +void igt_display_require(int fd);
> +bool igt_display_available(int fd);
> void igt_display_require_output(igt_display_t *display);
> void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe);
>
> diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> index 2e87e442..79243b1a 100644
> --- a/tests/debugfs_test.c
> +++ b/tests/debugfs_test.c
> @@ -102,10 +102,16 @@ igt_main
> debugfs = igt_debugfs_dir(fd);
>
> kmstest_set_vt_graphics_mode();
> - igt_display_init(&display, fd);
> + if (igt_display_available(fd))
> + igt_display_init(&display, fd);
> + else
> + display.n_pipes = 0;
Bleurgh. That's why I wasn't so keen on display_init including the
require.
Could we compromise by igt_require(igt_display_init(()); for tests that
need it?
> }
>
> igt_subtest("read_all_entries") {
> + if (!display.n_pipes)
> + goto skip_modeset;
> +
> /* try to light all pipes */
> for_each_pipe(&display, pipe) {
But why isn't for_each_pipe() a no-op if !n_pipes?
It seems to be for(pipe = 0; pipe < display.n_pipes; pipe++)
> igt_output_t *output;
> @@ -132,7 +138,7 @@ igt_main
> }
>
> igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
So presumably its ^. Something like
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 99d4c37..549d4e0 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3260,6 +3260,10 @@ static int do_display_commit(igt_display_t *display,
{
int i, ret = 0;
enum pipe pipe;
+
+ if (!display->n_pipes)
+ return 0;
+
LOG_INDENT(display, "commit");
igt_display_refresh(display);
@@ -3312,6 +3316,9 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
{
int ret;
+ if (!display->n_pipes)
+ return 0;
+
LOG_INDENT(display, "commit");
igt_display_refresh(display);
as suitable defense.
> +skip_modeset:
> read_and_discard_sysfs_entries(debugfs, 0);
> }
>
> @@ -140,6 +146,9 @@ igt_main
> igt_output_t *output;
> igt_plane_t *plane;
>
> + if (!display.n_pipes)
> + goto skip_modeset2;
> +
> for_each_connected_output(&display, output)
> igt_output_set_pipe(output, PIPE_NONE);
>
> @@ -148,7 +157,7 @@ igt_main
> igt_plane_set_fb(plane, NULL);
>
> igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> -
> +skip_modeset2:
> read_and_discard_sysfs_entries(debugfs, 0);
> }
>
> diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> index bfc981ee..18764c41 100644
> --- a/tests/kms_3d.c
> +++ b/tests/kms_3d.c
> @@ -36,6 +36,7 @@ igt_simple_main
> int mode_count, connector_id;
>
> drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + igt_display_require(drm_fd);
> res = drmModeGetResources(drm_fd);
>
> igt_assert(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) >= 0);
s/igt_assert/igt_require/
> diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> index ce48d24f..5d3f9b85 100644
> --- a/tests/kms_addfb_basic.c
> +++ b/tests/kms_addfb_basic.c
> @@ -671,8 +671,10 @@ int fd;
>
> igt_main
> {
> - igt_fixture
> + igt_fixture {
> fd = drm_open_driver_master(DRIVER_ANY);
> + igt_display_require(fd);
igt_require(has_addfb2()); is how we do it normally.
> + }
>
> invalid_tests(fd);
>
> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> index 86dcf392..51c92b71 100644
> --- a/tests/kms_draw_crc.c
> +++ b/tests/kms_draw_crc.c
> @@ -251,6 +251,7 @@ static void setup_environment(void)
>
> drm_fd = drm_open_driver_master(DRIVER_INTEL);
> igt_require(drm_fd >= 0);
> + igt_display_require(drm_fd);
>
> drm_res = drmModeGetResources(drm_fd);
igt_require(drm_res)?
> diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> index 81d796a4..67088883 100644
> --- a/tests/kms_getfb.c
> +++ b/tests/kms_getfb.c
> @@ -198,8 +198,10 @@ igt_main
> {
> int fd;
>
> - igt_fixture
> + igt_fixture {
> fd = drm_open_driver_master(DRIVER_ANY);
> + igt_display_require(fd);
This should be another igt_require(has_getfb());
So this looks like it becomes a lot simpler if we take the path of
disallowing DRIVER_MODESET for INTEL_INFO()->num_pipes == 0 and suggest
that all other drivers that allow disabling their KMS interface also
drop the MODESET feature.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display
2018-09-13 21:02 ` Chris Wilson
@ 2018-09-13 23:15 ` Souza, Jose
0 siblings, 0 replies; 4+ messages in thread
From: Souza, Jose @ 2018-09-13 23:15 UTC (permalink / raw)
To: igt-dev, chris; +Cc: Nikula, Jani
On Thu, 2018-09-13 at 22:02 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-09-13 21:39:35)
> > 'lib: Require drmModeResources()' take care of all tests that calls
> > igt_display_init(), here handling other tests that needs display
> > and
> > do not call it.
> >
> > v2:
> > - Not skipping all tests in debugfs_test, perf_pmu and perf_pmu,
> > now skiping only the required subtests
> >
> > v3:
> > - Renamed igt_require_display to igt_display_required, to keep
> > naming
> > consistent
> > - Added igt_display_available()
> > - Checking if display is available by quering a modeset capability
> > instead of drmModeGetResources()
> > - Not skiping read_all_entries tests when display is disabled
> >
> > v4:
> > - Using 'lib: Require drmModeResources()', so all tests already
> > handled by that patch had the igt_display_require() call removed.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > lib/igt_kms.c | 30 +++++++++++++-
> > lib/igt_kms.h | 2 +
> > tests/debugfs_test.c | 15 +++++--
> > tests/kms_3d.c | 1 +
> > tests/kms_addfb_basic.c | 4 +-
> > tests/kms_draw_crc.c | 1 +
> > tests/kms_fbcon_fbt.c | 1 +
> > tests/kms_force_connector_basic.c | 1 +
> > tests/kms_getfb.c | 4 +-
> > tests/kms_hdmi_inject.c | 1 +
> > tests/kms_setmode.c | 1 +
> > tests/kms_sysfs_edid_timing.c | 5 +++
> > tests/kms_tv_load_detect.c | 1 +
> > tests/perf_pmu.c | 16 +++----
> > tests/pm_lpsp.c | 1 +
> > tests/pm_rpm.c | 69 ++++++++++++++++++++++++---
> > ----
> > tests/prime_vgem.c | 2 +
> > tests/testdisplay.c | 1 +
> > 18 files changed, 129 insertions(+), 27 deletions(-)
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index c20cf1eb..2bc92c81 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -2018,6 +2018,34 @@ int igt_display_get_n_pipes(igt_display_t
> > *display)
> > return display->n_pipes;
> > }
> >
> > +/**
> > + * igt_display_require:
> > + * @drm_fd: a drm file descriptor
> > + *
> > + * Checks if driver supports modeset and have display enabled.
> > + */
> > +void igt_display_require(int fd)
> > +{
> > + bool available = igt_display_available(fd);
> > + igt_require_f(available, "drm driver do not support
> > modeset\n");
> > +}
> > +
> > +/**
> > + * igt_display_available:
> > + * @drm_fd: a drm file descriptor
> > + *
> > + * Return true if driver supports modeset and have display
> > enabled.
> > + */
> > +bool igt_display_available(int fd)
> > +{
> > + uint64_t cap;
> > +
> > + /* Check if driver support modeset by checking one of the
> > modeset
> > + * specific capabilities, in case of error it is not
> > supported.
> > + */
> > + return !drmGetCap(fd, DRM_CAP_DUMB_BUFFER, &cap);
> > +}
> > +
> > /**
> > * igt_display_require_output:
> > * @display: A pointer to an #igt_display_t structure
> > @@ -3895,7 +3923,7 @@ void igt_enable_connectors(void)
> > drm_fd = drm_open_driver(DRIVER_ANY);
> >
> > res = drmModeGetResources(drm_fd);
> > - igt_assert(res != NULL);
> > + igt_require(res != NULL);
> >
> > for (int i = 0; i < res->count_connectors; i++) {
> > drmModeConnector *c;
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 3862efa2..feafeda9 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -388,6 +388,8 @@ void igt_display_commit_atomic(igt_display_t
> > *display, uint32_t flags, void *use
> > int igt_display_try_commit2(igt_display_t *display, enum
> > igt_commit_style s);
> > int igt_display_drop_events(igt_display_t *display);
> > int igt_display_get_n_pipes(igt_display_t *display);
> > +void igt_display_require(int fd);
> > +bool igt_display_available(int fd);
> > void igt_display_require_output(igt_display_t *display);
> > void igt_display_require_output_on_pipe(igt_display_t *display,
> > enum pipe pipe);
> >
> > diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> > index 2e87e442..79243b1a 100644
> > --- a/tests/debugfs_test.c
> > +++ b/tests/debugfs_test.c
> > @@ -102,10 +102,16 @@ igt_main
> > debugfs = igt_debugfs_dir(fd);
> >
> > kmstest_set_vt_graphics_mode();
> > - igt_display_init(&display, fd);
> > + if (igt_display_available(fd))
> > + igt_display_init(&display, fd);
> > + else
> > + display.n_pipes = 0;
>
> Bleurgh. That's why I wasn't so keen on display_init including the
> require.
>
> Could we compromise by igt_require(igt_display_init(()); for tests
> that
> need it?
You mean zero igt_display_t when KMS is not suported and return false?
If so, it will add igt_require(igt_display_init(()); to several tests,
I don't think it is worthy to just make debugfs_test more pretty.
>
> > }
> >
> > igt_subtest("read_all_entries") {
> > + if (!display.n_pipes)
> > + goto skip_modeset;
> > +
> > /* try to light all pipes */
> > for_each_pipe(&display, pipe) {
>
> But why isn't for_each_pipe() a no-op if !n_pipes?
>
> It seems to be for(pipe = 0; pipe < display.n_pipes; pipe++)
As you got bellow it fails in the igt_display_commit2, here just
skiping both before hand but I'm fine with both ways.
>
> > igt_output_t *output;
> > @@ -132,7 +138,7 @@ igt_main
> > }
> >
> > igt_display_commit2(&display, display.is_atomic ?
> > COMMIT_ATOMIC : COMMIT_LEGACY);
>
> So presumably its ^. Something like
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 99d4c37..549d4e0 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3260,6 +3260,10 @@ static int do_display_commit(igt_display_t
> *display,
> {
> int i, ret = 0;
> enum pipe pipe;
> +
> + if (!display->n_pipes)
> + return 0;
> +
> LOG_INDENT(display, "commit");
>
> igt_display_refresh(display);
> @@ -3312,6 +3316,9 @@ int igt_display_try_commit_atomic(igt_display_t
> *display, uint32_t flags, void *
> {
> int ret;
>
> + if (!display->n_pipes)
> + return 0;
> +
> LOG_INDENT(display, "commit");
>
> igt_display_refresh(display);
>
> as suitable defense.
not sure, it may be misleading return 0 in those functions, if
DRIVER_MODESET is unset user would not even be able to initialize
igt_display_t anyways unless we go by your
'igt_require(igt_display_init(())' suggestion.
>
> > +skip_modeset:
> > read_and_discard_sysfs_entries(debugfs, 0);
> > }
> >
> > @@ -140,6 +146,9 @@ igt_main
> > igt_output_t *output;
> > igt_plane_t *plane;
> >
> > + if (!display.n_pipes)
> > + goto skip_modeset2;
> > +
> > for_each_connected_output(&display, output)
> > igt_output_set_pipe(output, PIPE_NONE);
> >
> > @@ -148,7 +157,7 @@ igt_main
> > igt_plane_set_fb(plane, NULL);
> >
> > igt_display_commit2(&display, display.is_atomic ?
> > COMMIT_ATOMIC : COMMIT_LEGACY);
> > -
> > +skip_modeset2:
> > read_and_discard_sysfs_entries(debugfs, 0);
> > }
> >
> > diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> > index bfc981ee..18764c41 100644
> > --- a/tests/kms_3d.c
> > +++ b/tests/kms_3d.c
> > @@ -36,6 +36,7 @@ igt_simple_main
> > int mode_count, connector_id;
> >
> > drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > + igt_display_require(drm_fd);
> > res = drmModeGetResources(drm_fd);
> >
> > igt_assert(drmSetClientCap(drm_fd,
> > DRM_CLIENT_CAP_STEREO_3D, 1) >= 0);
>
> s/igt_assert/igt_require/
drm_setclientcap() is returning 0 for every client capability even with
DRIVER_MODESET unset. I sent a patch fixing it in kernel so we can use
igt_require here.
>
> > diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> > index ce48d24f..5d3f9b85 100644
> > --- a/tests/kms_addfb_basic.c
> > +++ b/tests/kms_addfb_basic.c
> > @@ -671,8 +671,10 @@ int fd;
> >
> > igt_main
> > {
> > - igt_fixture
> > + igt_fixture {
> > fd = drm_open_driver_master(DRIVER_ANY);
> > + igt_display_require(fd);
>
> igt_require(has_addfb2()); is how we do it normally.
I saw your patch adding that but as you are still waiting for comments
in kernel patches, so skiping it for now.
>
> > + }
> >
> > invalid_tests(fd);
> >
> > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > index 86dcf392..51c92b71 100644
> > --- a/tests/kms_draw_crc.c
> > +++ b/tests/kms_draw_crc.c
> > @@ -251,6 +251,7 @@ static void setup_environment(void)
> >
> > drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > igt_require(drm_fd >= 0);
> > + igt_display_require(drm_fd);
> >
> > drm_res = drmModeGetResources(drm_fd);
>
> igt_require(drm_res)?
Sounds good, applying this to the other similar cases.
There is now only 4 users of igt_display_require()
- kms_addfb_basic
- kms_getfb
both will die when you patches are merged
- prime_vgem
it uses DRM_IOCTL_MODE_ADDFB2 and drmModePageFlip() the second
one returns -EINVAL with KMS disbaled and for other error cases, so I
guess is better just skip in the begiging of test_flip() with
igt_display_require()
- pm_rpm
discussed above
And igt_display_available() have:
- debugfs_test
discussed above
>
> > diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> > index 81d796a4..67088883 100644
> > --- a/tests/kms_getfb.c
> > +++ b/tests/kms_getfb.c
> > @@ -198,8 +198,10 @@ igt_main
> > {
> > int fd;
> >
> > - igt_fixture
> > + igt_fixture {
> > fd = drm_open_driver_master(DRIVER_ANY);
> > + igt_display_require(fd);
>
> This should be another igt_require(has_getfb());
>
> So this looks like it becomes a lot simpler if we take the path of
> disallowing DRIVER_MODESET for INTEL_INFO()->num_pipes == 0 and
> suggest
> that all other drivers that allow disabling their KMS interface also
> drop the MODESET feature.
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-13 23:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 20:39 [igt-dev] [PATCH i-g-t v4 1/2] lib: Require drmModeResources() José Roberto de Souza
2018-09-13 20:39 ` [igt-dev] [PATCH i-g-t v4 2/2] tests: Check and skip unhandled tests that needs display José Roberto de Souza
2018-09-13 21:02 ` Chris Wilson
2018-09-13 23:15 ` Souza, Jose
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.