* [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@lists.freedesktop.org, chris@chris-wilson.co.uk; +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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox