On 2020-03-23 08:32, Anshuman Gupta wrote: > Initialize the mode set params for lpsp/non-lpsp screen > based upon their output lpsp capability instead > of edp/non-edp output type. > > v2: > - CI failures fixup. This patch is: Acked-by: Martin Peres Congrats on your series, Anshuman! My only gripe with it is letting the audio free-wheel even when we don't need it and could reach LPSP. That being said, if you were to remove the sound drivers unloading, the series would be very close to a mergeable state. It's OK if the new tests fail, they can be fixed with a corresponding i915 patch series. Thanks again! Martin > > Signed-off-by: Anshuman Gupta > --- > tests/i915/i915_pm_rpm.c | 53 +++++++++++++++++++++++----------------- > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c > index 4f8124dc..e2fb732c 100644 > --- a/tests/i915/i915_pm_rpm.c > +++ b/tests/i915/i915_pm_rpm.c > @@ -94,6 +94,7 @@ struct mode_set_data { > drmModeResPtr res; > drmModeConnectorPtr connectors[MAX_CONNECTORS]; > drmModePropertyBlobPtr edids[MAX_CONNECTORS]; > + igt_display_t display; > > uint32_t devid; > }; > @@ -254,29 +255,29 @@ static bool init_modeset_params_for_type(struct mode_set_data *data, > { > drmModeConnectorPtr connector = NULL; > drmModeModeInfoPtr mode = NULL; > + igt_output_t *output = NULL; > + igt_display_t *display = &data->display; > > - if (!data->res) > + if (!data->res || !display) > return false; > > - for (int i = 0; i < data->res->count_connectors; i++) { > - drmModeConnectorPtr c = data->connectors[i]; > + for_each_connected_output(display, output) { > + drmModeConnectorPtr c = output->config.connector; > > if (type == SCREEN_TYPE_LPSP && > - c->connector_type != DRM_MODE_CONNECTOR_eDP) > + !igt_output_is_lpsp_capable(drm_fd, output)) > continue; > > if (type == SCREEN_TYPE_NON_LPSP && > - c->connector_type == DRM_MODE_CONNECTOR_eDP) > + igt_output_is_lpsp_capable(drm_fd, output)) > continue; > > - if (c->connection == DRM_MODE_CONNECTED && c->count_modes) { > - connector = c; > - mode = &c->modes[0]; > - break; > - } > + connector = c; > + mode = igt_output_get_mode(output); > + break; > } > > - if (!connector) > + if (!connector || !mode) > return false; > > igt_create_pattern_fb(drm_fd, mode->hdisplay, mode->vdisplay, > @@ -397,6 +398,7 @@ static void init_mode_set_data(struct mode_set_data *data) > kmstest_set_vt_graphics_mode(); > } > > + igt_display_require(&data->display, drm_fd); > data->devid = intel_get_drm_devid(drm_fd); > init_modeset_cached_params(&ms_data); > } > @@ -410,6 +412,8 @@ static void fini_mode_set_data(struct mode_set_data *data) > } > drmModeFreeResources(data->res); > } > + > + igt_display_fini(&data->display); > } > > static void get_drm_info(struct compare_data *data) > @@ -760,7 +764,7 @@ static void dump_file(int dir, const char *filename) > free(contents); > } > > -static bool setup_environment(void) > +static bool setup_environment(bool display_disabled) > { > if (has_runtime_pm) > goto out; > @@ -772,7 +776,8 @@ static bool setup_environment(void) > debugfs = igt_debugfs_dir(drm_fd); > igt_require(debugfs != -1); > > - init_mode_set_data(&ms_data); > + if (!display_disabled) > + init_mode_set_data(&ms_data); > > igt_pm_enable_sata_link_power_management(); > > @@ -785,13 +790,14 @@ static bool setup_environment(void) > igt_require(igt_pm_dmc_loaded(debugfs)); > > out: > - disable_all_screens(&ms_data); > + if (!display_disabled) > + disable_all_screens(&ms_data); > dump_file(debugfs, "i915_runtime_pm_status"); > > return wait_for_suspended(); > } > > -static void teardown_environment(void) > +static void teardown_environment(bool display_disabled) > { > close(msr_fd); > if (has_pc8) > @@ -801,7 +807,8 @@ static void teardown_environment(void) > > igt_pm_restore_sata_link_power_management(); > > - fini_mode_set_data(&ms_data); > + if (!display_disabled) > + fini_mode_set_data(&ms_data); > > close(debugfs); > close(drm_fd); > @@ -2008,7 +2015,7 @@ static struct option long_options[] = { > igt_main_args("", long_options, help_str, opt_handler, NULL) > { > igt_subtest("basic-rte") { > - igt_assert(setup_environment()); > + igt_assert(setup_environment(false)); > basic_subtest(); > } > > @@ -2016,7 +2023,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > * PC8+. We don't want bug reports from cases where the machine is just > * not properly configured. */ > igt_fixture > - igt_require(setup_environment()); > + igt_require(setup_environment(false)); > > if (stay) > igt_subtest("stay") > @@ -2146,7 +2153,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > } > > igt_fixture > - teardown_environment(); > + teardown_environment(false); > > igt_subtest("module-reload") { > igt_debug("Reload w/o display\n"); > @@ -2155,9 +2162,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > igt_kmsg(KMSG_INFO "Reloading i915 w/o display\n"); > igt_assert_eq(igt_i915_driver_load("disable_display=1 mmio_debug=-1"), 0); > > - igt_assert(setup_environment()); > + igt_assert(setup_environment(true)); > igt_assert(igt_wait(device_in_pci_d3(), 2000, 100)); > - teardown_environment(); > + teardown_environment(true); > > igt_debug("Reload as normal\n"); > igt_i915_driver_unload(); > @@ -2165,11 +2172,11 @@ igt_main_args("", long_options, help_str, opt_handler, NULL) > igt_kmsg(KMSG_INFO "Reloading i915 as normal\n"); > igt_assert_eq(igt_i915_driver_load("mmio_debug=-1"), 0); > > - igt_assert(setup_environment()); > + igt_assert(setup_environment(false)); > igt_assert(igt_wait(device_in_pci_d3(), 2000, 100)); > if (enable_one_screen_with_type(&ms_data, SCREEN_TYPE_ANY)) > drm_resources_equal_subtest(); > - teardown_environment(); > + teardown_environment(false); > > /* Remove our mmio_debugging module */ > igt_i915_driver_unload(); >