* [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup @ 2022-09-22 0:58 Nidhi Gupta 2022-09-22 0:58 ` [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic Nidhi Gupta ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Nidhi Gupta @ 2022-09-22 0:58 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta *** BLURB HERE *** Nidhi Gupta (2): tests/kms_writeback: Convert tests to dynamic tests/kms_writeback: Test cleanup tests/kms_writeback.c | 59 ++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 15 deletions(-) -- 2.36.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic 2022-09-22 0:58 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta @ 2022-09-22 0:58 ` Nidhi Gupta 2022-09-22 10:18 ` Modem, Bhanuprakash 2022-09-22 0:58 ` [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-22 13:22 ` [igt-dev] ✗ Fi.CI.BUILD: failure for tests/kms_writeback: Test Cleanup (rev3) Patchwork 2 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-09-22 0:58 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta Convert the existing subtests to dynamic subtests. Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 57 +++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 9d134585..3781fa34 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -214,32 +214,38 @@ static void test_invalid_parameters(igt_output_t *output, igt_fb_t *valid_fb, ig uint32_t fb_id; bool ptr_valid; int32_t *out_fence_ptr; + const char *name; } invalid_tests[] = { { /* No output buffer, but the WRITEBACK_OUT_FENCE_PTR set. */ .fb_id = 0, .ptr_valid = true, .out_fence_ptr = &out_fence, + .name = "WRITEBACK_OUT_FENCE_PTR-set", }, { /* Invalid output buffer. */ .fb_id = invalid_fb->fb_id, .ptr_valid = true, .out_fence_ptr = &out_fence, + .name = "Invalid-output-buffer", }, { /* Invalid WRITEBACK_OUT_FENCE_PTR. */ .fb_id = valid_fb->fb_id, .ptr_valid = false, .out_fence_ptr = (int32_t *)0x8, + .name = "Invalid-WRITEBACK_OUT_FENCE_PTR", }, }; for (i = 0; i < ARRAY_SIZE(invalid_tests); i++) { - ret = do_writeback_test(output, invalid_tests[i].fb_id, - invalid_tests[i].out_fence_ptr, - invalid_tests[i].ptr_valid); - igt_assert(ret != 0); + igt_dynamic_f("%s", invalid_tests[i].name) { + ret = do_writeback_test(output, invalid_tests[i].fb_id, + invalid_tests[i].out_fence_ptr, + invalid_tests[i].ptr_valid); + igt_assert(ret != 0); + } } } @@ -247,18 +253,39 @@ static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t * { int ret; + struct { + const char *name; + uint32_t fb_id; + int i, expected_ret; + } fb_id_tests[] = { - /* Invalid object for WRITEBACK_FB_ID */ - ret = do_writeback_test(output, output->id, NULL, false); - igt_assert(ret == -EINVAL); + { + .name = "Invalid-object", + .fb_id = output->id, + .expected_ret = -EINVAL, + }, + + { + .name = "Zero-WRITEBACK_FB_ID", + .fb_id = 0, + .expected_ret = 0, + }, + + { + .name = "Valid-output-buffer", + .fb_id = valid_fb->fb_id, + .expected_ret = 0, + }, + }; + + for (int i = 0; i < ARRAY_SIZE(fb_id_tests); i++) { + igt_dynamic_f("%s", fb_id_tests[i].name) { + ret = do_writeback_test(output, fb_id_tests[i].fb_id, NULL, false); + igt_assert(ret == fb_id_tests[i].expected_ret); + } + } - /* Zero WRITEBACK_FB_ID */ - ret = do_writeback_test(output, 0, NULL, false); - igt_assert(ret == 0); - /* Valid output buffer */ - ret = do_writeback_test(output, valid_fb->fb_id, NULL, false); - igt_assert(ret == 0); } static void fill_fb(igt_fb_t *fb, uint32_t pixel) @@ -553,7 +580,7 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_describe("Writeback has a couple of parameters linked together" "(output framebuffer and fence); this test goes through" "the combination of possible bad options"); - igt_subtest("writeback-invalid-parameters") { + igt_subtest_with_dynamic("writeback-invalid-parameters") { igt_fb_t invalid_output_fb; igt_skip_on(data.dump_check || data.list_modes); @@ -570,7 +597,7 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) } igt_describe("Validate WRITEBACK_FB_ID with valid and invalid options"); - igt_subtest("writeback-fb-id") { + igt_subtest_with_dynamic("writeback-fb-id") { igt_fb_t output_fb; igt_skip_on(data.dump_check || data.list_modes); -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic 2022-09-22 0:58 ` [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic Nidhi Gupta @ 2022-09-22 10:18 ` Modem, Bhanuprakash 0 siblings, 0 replies; 12+ messages in thread From: Modem, Bhanuprakash @ 2022-09-22 10:18 UTC (permalink / raw) To: Nidhi Gupta, igt-dev On Thu-22-09-2022 06:28 am, Nidhi Gupta wrote: > Convert the existing subtests to dynamic subtests. > > Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> > --- > tests/kms_writeback.c | 57 +++++++++++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 15 deletions(-) > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c > index 9d134585..3781fa34 100644 > --- a/tests/kms_writeback.c > +++ b/tests/kms_writeback.c > @@ -214,32 +214,38 @@ static void test_invalid_parameters(igt_output_t *output, igt_fb_t *valid_fb, ig > uint32_t fb_id; > bool ptr_valid; > int32_t *out_fence_ptr; > + const char *name; > } invalid_tests[] = { > { > /* No output buffer, but the WRITEBACK_OUT_FENCE_PTR set. */ > .fb_id = 0, > .ptr_valid = true, > .out_fence_ptr = &out_fence, > + .name = "WRITEBACK_OUT_FENCE_PTR-set", Please avoid mixed case, also use "-" instead of "_". This comment is applicable to all places. > }, > { > /* Invalid output buffer. */ > .fb_id = invalid_fb->fb_id, > .ptr_valid = true, > .out_fence_ptr = &out_fence, > + .name = "Invalid-output-buffer", > }, > { > /* Invalid WRITEBACK_OUT_FENCE_PTR. */ > .fb_id = valid_fb->fb_id, > .ptr_valid = false, > .out_fence_ptr = (int32_t *)0x8, > + .name = "Invalid-WRITEBACK_OUT_FENCE_PTR", > }, > }; > > for (i = 0; i < ARRAY_SIZE(invalid_tests); i++) { > - ret = do_writeback_test(output, invalid_tests[i].fb_id, > - invalid_tests[i].out_fence_ptr, > - invalid_tests[i].ptr_valid); > - igt_assert(ret != 0); > + igt_dynamic_f("%s", invalid_tests[i].name) { > + ret = do_writeback_test(output, invalid_tests[i].fb_id, > + invalid_tests[i].out_fence_ptr, > + invalid_tests[i].ptr_valid); > + igt_assert(ret != 0); > + } > } > } > > @@ -247,18 +253,39 @@ static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t * > { > > int ret; > + struct { > + const char *name; > + uint32_t fb_id; > + int i, expected_ret; > + } fb_id_tests[] = { > > - /* Invalid object for WRITEBACK_FB_ID */ > - ret = do_writeback_test(output, output->id, NULL, false); > - igt_assert(ret == -EINVAL); > + { > + .name = "Invalid-object", > + .fb_id = output->id, > + .expected_ret = -EINVAL, > + }, > + Newlines are not required. > + { > + .name = "Zero-WRITEBACK_FB_ID", > + .fb_id = 0, > + .expected_ret = 0, > + }, > + > + { > + .name = "Valid-output-buffer", > + .fb_id = valid_fb->fb_id, > + .expected_ret = 0, > + }, > + }; > + > + for (int i = 0; i < ARRAY_SIZE(fb_id_tests); i++) { > + igt_dynamic_f("%s", fb_id_tests[i].name) { > + ret = do_writeback_test(output, fb_id_tests[i].fb_id, NULL, false); > + igt_assert(ret == fb_id_tests[i].expected_ret); > + } > + } > > - /* Zero WRITEBACK_FB_ID */ > - ret = do_writeback_test(output, 0, NULL, false); > - igt_assert(ret == 0); > > - /* Valid output buffer */ > - ret = do_writeback_test(output, valid_fb->fb_id, NULL, false); > - igt_assert(ret == 0); > } > > static void fill_fb(igt_fb_t *fb, uint32_t pixel) > @@ -553,7 +580,7 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > igt_describe("Writeback has a couple of parameters linked together" > "(output framebuffer and fence); this test goes through" > "the combination of possible bad options"); > - igt_subtest("writeback-invalid-parameters") { > + igt_subtest_with_dynamic("writeback-invalid-parameters") { > igt_fb_t invalid_output_fb; > > igt_skip_on(data.dump_check || data.list_modes); > @@ -570,7 +597,7 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > } > > igt_describe("Validate WRITEBACK_FB_ID with valid and invalid options"); > - igt_subtest("writeback-fb-id") { > + igt_subtest_with_dynamic("writeback-fb-id") { > igt_fb_t output_fb; > > igt_skip_on(data.dump_check || data.list_modes); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-22 0:58 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-22 0:58 ` [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic Nidhi Gupta @ 2022-09-22 0:58 ` Nidhi Gupta 2022-09-22 10:02 ` Modem, Bhanuprakash 2022-09-22 13:22 ` [igt-dev] ✗ Fi.CI.BUILD: failure for tests/kms_writeback: Test Cleanup (rev3) Patchwork 2 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-09-22 0:58 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta Sanitize the system state before starting the subtest. Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 3781fa34..5149e302 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -521,6 +521,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) kmstest_set_vt_graphics_mode(); + igt_display_reset(display); + igt_display_require(&display, display.drm_fd); igt_require(display.is_atomic); -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-22 0:58 ` [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup Nidhi Gupta @ 2022-09-22 10:02 ` Modem, Bhanuprakash 0 siblings, 0 replies; 12+ messages in thread From: Modem, Bhanuprakash @ 2022-09-22 10:02 UTC (permalink / raw) To: Nidhi Gupta, igt-dev On Thu-22-09-2022 06:28 am, Nidhi Gupta wrote: > Sanitize the system state before starting the subtest. > > Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> > --- > tests/kms_writeback.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c > index 3781fa34..5149e302 100644 > --- a/tests/kms_writeback.c > +++ b/tests/kms_writeback.c > @@ -521,6 +521,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > > kmstest_set_vt_graphics_mode(); > > + igt_display_reset(display); > + > igt_display_require(&display, display.drm_fd); Please drop this I can see igt_display_require() is already present. https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n493 Found some other cleanups: Example: https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n113 https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n144 Why do we need hard-coded values for override_mode? Can't we use default mode? https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n536 https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n559 https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n576 https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/kms_writeback.c#n592 As this check is common in all subtests, please move this to igt_fixture - Bhanu > > igt_require(display.is_atomic); ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] ✗ Fi.CI.BUILD: failure for tests/kms_writeback: Test Cleanup (rev3) 2022-09-22 0:58 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-22 0:58 ` [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic Nidhi Gupta 2022-09-22 0:58 ` [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup Nidhi Gupta @ 2022-09-22 13:22 ` Patchwork 2 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2022-09-22 13:22 UTC (permalink / raw) To: Nidhi Gupta; +Cc: igt-dev == Series Details == Series: tests/kms_writeback: Test Cleanup (rev3) URL : https://patchwork.freedesktop.org/series/107764/ State : failure == Summary == IGT patchset build failed on latest successful build 129bea58b9424f0a0da995311a219fdf57732594 kms_flip_event_leak: Create dynamic subtests [325/672] Linking target tests/kms_dp_aux_dev [326/672] Linking target tests/kms_display_modes [327/672] Linking target tests/kms_flip [328/672] Linking target tests/kms_dp_tiled_display [329/672] Linking target tests/kms_invalid_mode [330/672] Linking target tests/kms_hdmi_inject [331/672] Linking target tests/kms_flip_event_leak [332/672] Linking target benchmarks/gem_blt [333/672] Linking target benchmarks/gem_busy [334/672] Linking target tests/kms_getfb [335/672] Linking target tests/kms_hdr [336/672] Linking target tests/kms_dsc [337/672] Linking target tests/gen3_render_tiledy_blits [338/672] Linking target tests/kms_lease [339/672] Linking target tests/kms_multipipe_modeset [340/672] Linking target tests/kms_panel_fitting [341/672] Linking target tests/kms_pipe_crc_basic [342/672] Linking target tests/kms_plane_cursor [343/672] Linking target tests/kms_plane [344/672] Linking target tests/kms_plane_alpha_blend [345/672] Linking target tests/kms_plane_lowres [346/672] Linking target tests/kms_plane_scaling [347/672] Linking target tests/kms_plane_multiple [348/672] Linking target tests/kms_prop_blob [349/672] Linking target tests/kms_prime [350/672] Linking target tests/kms_rotation_crc [351/672] Linking target tests/kms_properties [352/672] Linking target tests/kms_rmfb [353/672] Linking target tests/kms_scaling_modes [354/672] Linking target tests/kms_selftest [355/672] Linking target tests/kms_sequence [356/672] Linking target tests/kms_vrr [357/672] Linking target tests/kms_setmode [358/672] Linking target tests/kms_sysfs_edid_timing [359/672] Linking target tests/kms_tv_load_detect [360/672] Linking target tests/kms_universal_plane [361/672] Linking target tests/kms_vblank [362/672] Compiling C object tests/kms_writeback.p/kms_writeback.c.o FAILED: tests/kms_writeback.p/kms_writeback.c.o ccache cc -Itests/kms_writeback.p -Itests -I../tests -I../include/drm-uapi -I../include/linux-uapi -Ilib -I../lib -I../lib/stubs/syscalls -I. -I.. -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libpng12 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/x86_64-linux-gnu -I/usr/include/alsa -I/usr/include -I/home/cidrm/kernel_headers/include -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fcommon -pthread -MD -MQ tests/kms_writeback.p/kms_writeback.c.o -MF tests/kms_writeback.p/kms_writeback.c.o.d -o tests/kms_writeback.p/kms_writeback.c.o -c ../tests/kms_writeback.c ../tests/kms_writeback.c: In function ‘__igt_unique____real_main507’: ../tests/kms_writeback.c:524:21: error: incompatible type for argument 1 of ‘igt_display_reset’ igt_display_reset(display); ^~~~~~~ In file included from ../lib/igt.h:38, from ../tests/kms_writeback.c:31: ../lib/igt_kms.h:455:39: note: expected ‘igt_display_t *’ {aka ‘struct igt_display *’} but argument is of type ‘igt_display_t’ {aka ‘struct igt_display’} void igt_display_reset(igt_display_t *display); ~~~~~~~~~~~~~~~^~~~~~~ ninja: build stopped: subcommand failed. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup @ 2022-09-30 5:41 Nidhi Gupta 2022-09-30 5:41 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 0 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-09-30 5:41 UTC (permalink / raw) To: igt-dev From: Jeevan B <jeevan.b@intel.com> *** BLURB HERE *** Nidhi Gupta (1): tests/kms_writeback: Test cleanup Zbigniew Kempczyński (1): lib/intel_memory_region: Remove function which returns batch size in regions lib/i915/intel_memory_region.c | 14 --------- lib/i915/intel_memory_region.h | 1 - tests/kms_writeback.c | 52 ++++++++++------------------------ 3 files changed, 15 insertions(+), 52 deletions(-) -- 2.36.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-30 5:41 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta @ 2022-09-30 5:41 ` Nidhi Gupta 0 siblings, 0 replies; 12+ messages in thread From: Nidhi Gupta @ 2022-09-30 5:41 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta v1: move igt_skip_on(data.dump_check || data.list_modes) to igt_fixture as it is common for all subtests. (Bhanu) v2: replaced hard coded mode with default mode. (Bhanu) Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 52 +++++++++++++------------------------------ 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 9d134585..8846d6d8 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -107,46 +107,27 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output, static igt_output_t *kms_writeback_get_output(igt_display_t *display) { - int i; enum pipe pipe; + igt_output_t *output; - drmModeModeInfo override_mode = { - .clock = 25175, - .hdisplay = 640, - .hsync_start = 656, - .hsync_end = 752, - .htotal = 800, - .hskew = 0, - .vdisplay = 480, - .vsync_start = 490, - .vsync_end = 492, - .vtotal = 525, - .vscan = 0, - .vrefresh = 60, - .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, - .name = {"640x480-60"}, - }; - - for (i = 0; i < display->n_outputs; i++) { - igt_output_t *output = &display->outputs[i]; + drmModeModeInfo override_mode; + for_each_pipe_with_valid_output(display, pipe, output) { + igt_output_set_pipe(output, pipe); if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) continue; - for_each_pipe(display, pipe) { - igt_output_set_pipe(output, pipe); + override_mode = *igt_output_get_mode(output); - if (data.custom_mode) - override_mode = data.user_mode; - if (data.builtin_mode) - override_mode = output->config.connector->modes[data.mode_index]; - - if (check_writeback_config(display, output, override_mode)) { - igt_debug("Using connector %u:%s on pipe %d\n", - output->config.connector->connector_id, - output->name, pipe); + if (data.custom_mode) + override_mode = data.user_mode; + if (data.builtin_mode) + override_mode = output->config.connector->modes[data.mode_index]; + if (check_writeback_config(display, output, override_mode)) { + igt_debug("Using connector %u:%s on pipe %d\n", + output->config.connector->connector_id, + output->name, pipe); return output; - } } igt_debug("We found %u:%s, but this test will not be able to use it.\n", @@ -155,7 +136,6 @@ static igt_output_t *kms_writeback_get_output(igt_display_t *display) /* Restore any connectors we don't use, so we don't trip on them later */ kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_UNSPECIFIED); } - return NULL; } @@ -498,6 +478,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_require(display.is_atomic); + igt_skip_on(data.dump_check || data.list_modes); + output = kms_writeback_get_output(&display); igt_require(output); @@ -533,7 +515,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) drmModePropertyBlobRes *formats_blob; const char *valid_chars; - igt_skip_on(data.dump_check || data.list_modes); formats_blob = get_writeback_formats_blob(output); valid_chars = "01234568 ABCGNRUVXY"; @@ -556,7 +537,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-invalid-parameters") { igt_fb_t invalid_output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay / 2, mode.vdisplay / 2, DRM_FORMAT_XRGB8888, @@ -573,7 +553,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-fb-id") { igt_fb_t output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, @@ -589,7 +568,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-check-output") { igt_fb_t output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, DRM_FORMAT_XRGB8888, igt_fb_mod_to_tiling(0), -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup @ 2022-09-30 5:32 Jeevan B 2022-09-30 5:32 ` [igt-dev] [PATCH 2/2] " Jeevan B 0 siblings, 1 reply; 12+ messages in thread From: Jeevan B @ 2022-09-30 5:32 UTC (permalink / raw) To: igt-dev *** BLURB HERE *** Nidhi Gupta (1): tests/kms_writeback: Test cleanup Zbigniew Kempczyński (1): lib/intel_memory_region: Remove function which returns batch size in regions lib/i915/intel_memory_region.c | 14 --------- lib/i915/intel_memory_region.h | 1 - tests/kms_writeback.c | 52 ++++++++++------------------------ 3 files changed, 15 insertions(+), 52 deletions(-) -- 2.36.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-30 5:32 [igt-dev] [PATCH 0/2] " Jeevan B @ 2022-09-30 5:32 ` Jeevan B 0 siblings, 0 replies; 12+ messages in thread From: Jeevan B @ 2022-09-30 5:32 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta From: Nidhi Gupta <nidhi1.gupta@intel.com> v1: move igt_skip_on(data.dump_check || data.list_modes) to igt_fixture as it is common for all subtests. (Bhanu) v2: replaced hard coded mode with default mode. (Bhanu) Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 52 +++++++++++++------------------------------ 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 9d134585..8846d6d8 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -107,46 +107,27 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output, static igt_output_t *kms_writeback_get_output(igt_display_t *display) { - int i; enum pipe pipe; + igt_output_t *output; - drmModeModeInfo override_mode = { - .clock = 25175, - .hdisplay = 640, - .hsync_start = 656, - .hsync_end = 752, - .htotal = 800, - .hskew = 0, - .vdisplay = 480, - .vsync_start = 490, - .vsync_end = 492, - .vtotal = 525, - .vscan = 0, - .vrefresh = 60, - .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, - .name = {"640x480-60"}, - }; - - for (i = 0; i < display->n_outputs; i++) { - igt_output_t *output = &display->outputs[i]; + drmModeModeInfo override_mode; + for_each_pipe_with_valid_output(display, pipe, output) { + igt_output_set_pipe(output, pipe); if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) continue; - for_each_pipe(display, pipe) { - igt_output_set_pipe(output, pipe); + override_mode = *igt_output_get_mode(output); - if (data.custom_mode) - override_mode = data.user_mode; - if (data.builtin_mode) - override_mode = output->config.connector->modes[data.mode_index]; - - if (check_writeback_config(display, output, override_mode)) { - igt_debug("Using connector %u:%s on pipe %d\n", - output->config.connector->connector_id, - output->name, pipe); + if (data.custom_mode) + override_mode = data.user_mode; + if (data.builtin_mode) + override_mode = output->config.connector->modes[data.mode_index]; + if (check_writeback_config(display, output, override_mode)) { + igt_debug("Using connector %u:%s on pipe %d\n", + output->config.connector->connector_id, + output->name, pipe); return output; - } } igt_debug("We found %u:%s, but this test will not be able to use it.\n", @@ -155,7 +136,6 @@ static igt_output_t *kms_writeback_get_output(igt_display_t *display) /* Restore any connectors we don't use, so we don't trip on them later */ kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_UNSPECIFIED); } - return NULL; } @@ -498,6 +478,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_require(display.is_atomic); + igt_skip_on(data.dump_check || data.list_modes); + output = kms_writeback_get_output(&display); igt_require(output); @@ -533,7 +515,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) drmModePropertyBlobRes *formats_blob; const char *valid_chars; - igt_skip_on(data.dump_check || data.list_modes); formats_blob = get_writeback_formats_blob(output); valid_chars = "01234568 ABCGNRUVXY"; @@ -556,7 +537,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-invalid-parameters") { igt_fb_t invalid_output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay / 2, mode.vdisplay / 2, DRM_FORMAT_XRGB8888, @@ -573,7 +553,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-fb-id") { igt_fb_t output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, @@ -589,7 +568,6 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_subtest("writeback-check-output") { igt_fb_t output_fb; - igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, DRM_FORMAT_XRGB8888, igt_fb_mod_to_tiling(0), -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup @ 2022-09-08 8:48 Nidhi Gupta 2022-09-08 8:48 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 0 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-09-08 8:48 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta *** BLURB HERE *** Nidhi Gupta (2): tests/kms_writeback: Convert tests to dynamic tests/kms_writeback: Test cleanup tests/kms_writeback.c | 58 ++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 15 deletions(-) -- 2.36.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-08 8:48 [igt-dev] [PATCH 0/2] " Nidhi Gupta @ 2022-09-08 8:48 ` Nidhi Gupta 2022-09-08 18:03 ` Jessica Zhang 0 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-09-08 8:48 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta Sanitize the system state before starting the subtest. Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 2c5421ce..79a5760f 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -186,6 +186,7 @@ static int do_writeback_test(igt_output_t *output, uint32_t fb_id, { int ret; igt_display_t *display = output->display; + igt_display_reset(display); struct kmstest_connector_config *config = &output->config; igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID, config->crtc->crtc_id); -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup 2022-09-08 8:48 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta @ 2022-09-08 18:03 ` Jessica Zhang 0 siblings, 0 replies; 12+ messages in thread From: Jessica Zhang @ 2022-09-08 18:03 UTC (permalink / raw) To: Nidhi Gupta, igt-dev Hi Nidhi, On 9/8/2022 1:48 AM, Nidhi Gupta wrote: > Sanitize the system state before starting the subtest. > > Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> > --- > tests/kms_writeback.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c > index 2c5421ce..79a5760f 100644 > --- a/tests/kms_writeback.c > +++ b/tests/kms_writeback.c > @@ -186,6 +186,7 @@ static int do_writeback_test(igt_output_t *output, uint32_t fb_id, > { > int ret; > igt_display_t *display = output->display; > + igt_display_reset(display); I might not have communicated this clearly in the v1 comments, but igt_display_reset() will undo all the setup done in the first igt_fixture [1] and disable the CRTCs. Furthermore, it would cause the commit in do_writeback_test() [2] to fail, since the CRTC would be disabled. If you want to call igt_display_reset(), you'll have to re-do the setup within that fixture. Thanks, Jessica Zhang [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L491 [2] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L198 > struct kmstest_connector_config *config = &output->config; > > igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID, config->crtc->crtc_id); > -- > 2.36.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 0/2] tests/kms_writeback: Test Cleanup @ 2022-08-25 21:06 Nidhi Gupta 2022-08-25 21:06 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 0 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-08-25 21:06 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta Nidhi Gupta (2): tests/kms_writeback: Convert tests to dynamic tests/kms_writeback: Test Cleanup tests/kms_writeback.c | 65 ++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 16 deletions(-) -- 2.36.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH 2/2] tests/kms_writeback: Test Cleanup 2022-08-25 21:06 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test Cleanup Nidhi Gupta @ 2022-08-25 21:06 ` Nidhi Gupta 2022-08-26 18:49 ` Jessica Zhang 0 siblings, 1 reply; 12+ messages in thread From: Nidhi Gupta @ 2022-08-25 21:06 UTC (permalink / raw) To: igt-dev; +Cc: Nidhi Gupta Sanitize the system state before starting the subtest. Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> --- tests/kms_writeback.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c index 2508b1e1..0c6c64b3 100644 --- a/tests/kms_writeback.c +++ b/tests/kms_writeback.c @@ -169,6 +169,7 @@ static void detach_crtc(igt_display_t *display, igt_output_t *output) if (get_writeback_fb_id(output) == 0) return; + igt_display_fini(&display); igt_output_set_pipe(output, PIPE_NONE); igt_display_commit2(display, COMMIT_ATOMIC); } @@ -580,6 +581,9 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) "the combination of possible bad options"); igt_subtest_with_dynamic("writeback-invalid-parameters") { igt_fb_t invalid_output_fb; + igt_display_reset(&display); + igt_display_commit(&display); + igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay / 2, @@ -597,6 +601,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_describe("Validate WRITEBACK_FB_ID with valid and invalid options"); igt_subtest_with_dynamic("writeback-fb-id") { igt_fb_t output_fb; + igt_display_reset(&display); + igt_display_commit(&display); igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, @@ -613,6 +619,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) igt_describe("Check writeback output with CRC validation"); igt_subtest_with_dynamic("writeback-check-output") { igt_fb_t output_fb; + igt_display_reset(&display); + igt_display_commit(&display); igt_skip_on(data.dump_check || data.list_modes); fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, -- 2.36.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH 2/2] tests/kms_writeback: Test Cleanup 2022-08-25 21:06 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta @ 2022-08-26 18:49 ` Jessica Zhang 0 siblings, 0 replies; 12+ messages in thread From: Jessica Zhang @ 2022-08-26 18:49 UTC (permalink / raw) To: Nidhi Gupta, igt-dev Hi Nidhi, On 8/25/2022 2:06 PM, Nidhi Gupta wrote: > Sanitize the system state before starting the subtest. > > Signed-off-by: Nidhi Gupta <nidhi1.gupta@intel.com> > --- > tests/kms_writeback.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c > index 2508b1e1..0c6c64b3 100644 > --- a/tests/kms_writeback.c > +++ b/tests/kms_writeback.c > @@ -169,6 +169,7 @@ static void detach_crtc(igt_display_t *display, igt_output_t *output) > if (get_writeback_fb_id(output) == 0) > return; > > + igt_display_fini(&display); igt_display_fini() frees the memory for various members of the igt_display_t struct [1]. Since there's already a call to igt_display_fini() in the same fixture that detach_crtc() is called [2], adding an extra call here will cause a double free. [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L2752 [2] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L607 > igt_output_set_pipe(output, PIPE_NONE); > igt_display_commit2(display, COMMIT_ATOMIC); > } > @@ -580,6 +581,9 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > "the combination of possible bad options"); > igt_subtest_with_dynamic("writeback-invalid-parameters") { > igt_fb_t invalid_output_fb; > + igt_display_reset(&display); > + igt_display_commit(&display); I have a doubt about this: There's a lot of set up going on in the initial igt_fixture here [3], specifically within kms_writeback_get_output() [4], where the output pipe is set. If you just reset the display then do a commit, the output->pending pipe will be set to NONE within igt_output_reset() [5]. This will cause the commit to detach the CRTC (in a similar manner to what detach_crtc() does [6]). If you don't redo the setup from the initial igt_fixture after resetting the display, the commits within do_writeback_test() will fail since there won't be a CRTC attached to the connector. Thanks, Jessica Zhang [3] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L491 [4] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L108 [5] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/lib/igt_kms.c#L2180 [6] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_writeback.c#L167 > + > > igt_skip_on(data.dump_check || data.list_modes); > fb_id = igt_create_fb(display.drm_fd, mode.hdisplay / 2, > @@ -597,6 +601,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > igt_describe("Validate WRITEBACK_FB_ID with valid and invalid options"); > igt_subtest_with_dynamic("writeback-fb-id") { > igt_fb_t output_fb; > + igt_display_reset(&display); > + igt_display_commit(&display); > > igt_skip_on(data.dump_check || data.list_modes); > fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, > @@ -613,6 +619,8 @@ igt_main_args("b:c:dl", long_options, help_str, opt_handler, NULL) > igt_describe("Check writeback output with CRC validation"); > igt_subtest_with_dynamic("writeback-check-output") { > igt_fb_t output_fb; > + igt_display_reset(&display); > + igt_display_commit(&display); > > igt_skip_on(data.dump_check || data.list_modes); > fb_id = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay, > -- > 2.36.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-30 5:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-22 0:58 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-22 0:58 ` [igt-dev] [PATCH 1/2] tests/kms_writeback: Convert tests to dynamic Nidhi Gupta 2022-09-22 10:18 ` Modem, Bhanuprakash 2022-09-22 0:58 ` [igt-dev] [PATCH 2/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-22 10:02 ` Modem, Bhanuprakash 2022-09-22 13:22 ` [igt-dev] ✗ Fi.CI.BUILD: failure for tests/kms_writeback: Test Cleanup (rev3) Patchwork -- strict thread matches above, loose matches on Subject: below -- 2022-09-30 5:41 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test cleanup Nidhi Gupta 2022-09-30 5:41 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 2022-09-30 5:32 [igt-dev] [PATCH 0/2] " Jeevan B 2022-09-30 5:32 ` [igt-dev] [PATCH 2/2] " Jeevan B 2022-09-08 8:48 [igt-dev] [PATCH 0/2] " Nidhi Gupta 2022-09-08 8:48 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 2022-09-08 18:03 ` Jessica Zhang 2022-08-25 21:06 [igt-dev] [PATCH 0/2] tests/kms_writeback: Test Cleanup Nidhi Gupta 2022-08-25 21:06 ` [igt-dev] [PATCH 2/2] " Nidhi Gupta 2022-08-26 18:49 ` Jessica Zhang
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.