From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0BECB6E1E5 for ; Tue, 14 Apr 2020 09:48:48 +0000 (UTC) Date: Tue, 14 Apr 2020 12:48:45 +0300 From: Petri Latvala Message-ID: <20200414094845.GO9497@platvala-desk.ger.corp.intel.com> References: <20200411111427.2749626-1-chris@chris-wilson.co.uk> <20200411111427.2749626-2-chris@chris-wilson.co.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200411111427.2749626-2-chris@chris-wilson.co.uk> Subject: Re: [igt-dev] [PATCH i-g-t 2/2] kms_flip: Convert to dynamic CRTC test groups List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Chris Wilson Cc: igt-dev@lists.freedesktop.org List-ID: On Sat, Apr 11, 2020 at 12:14:27PM +0100, Chris Wilson wrote: > Use igt_dynamic_f for the dynamic discovered CRTC sets for each test. > > Signed-off-by: Chris Wilson > Cc: Petri Latvala > Cc: Martin Peres > --- > tests/kms_flip.c | 93 ++++++++++++++++++++++++------------------------ > 1 file changed, 47 insertions(+), 46 deletions(-) > > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > index 4535ecc8b..41d33f75a 100755 > --- a/tests/kms_flip.c > +++ b/tests/kms_flip.c > @@ -1185,52 +1185,17 @@ static void calibrate_ts(struct test_output *o, int crtc_idx) > o->vblank_interval = mean; > } > > -static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > - int crtc_count, int duration_ms) > +static void __run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > + int crtc_count, int duration_ms) > { > - char test_name[128]; > - unsigned elapsed; > unsigned bo_size = 0; > + bool vblank = true; > + unsigned elapsed; > uint64_t tiling; > int i; > - bool vblank = true; > - > - switch (crtc_count) { > - case RUN_TEST: > - connector_find_preferred_mode(o->_connector[0], crtc_idxs[0], o); > - if (!o->mode_valid) > - return; > - snprintf(test_name, sizeof(test_name), > - "%s on pipe %s, connector %s-%d", > - igt_subtest_name(), > - kmstest_pipe_name(o->_pipe[0]), > - kmstest_connector_type_str(o->kconnector[0]->connector_type), > - o->kconnector[0]->connector_type_id); > - break; > - case RUN_PAIR: > - connector_find_compatible_mode(crtc_idxs[0], crtc_idxs[1], o); > - if (!o->mode_valid) > - return; > - snprintf(test_name, sizeof(test_name), > - "%s on pipe %s:%s, connector %s-%d:%s-%d", > - igt_subtest_name(), > - kmstest_pipe_name(o->_pipe[0]), > - kmstest_pipe_name(o->_pipe[1]), > - kmstest_connector_type_str(o->kconnector[0]->connector_type), > - o->kconnector[0]->connector_type_id, > - kmstest_connector_type_str(o->kconnector[1]->connector_type), > - o->kconnector[1]->connector_type_id); > - break; > - default: > - igt_assert(0); > - } > - > - igt_assert_eq(o->count, crtc_count); > > last_connector = o->kconnector[0]; > > - igt_info("Beginning %s\n", test_name); > - > if (o->flags & TEST_PAN) > o->fb_width *= 2; > > @@ -1274,7 +1239,6 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > */ > igt_assert_f(crtc_count > 1 || crtc_idxs[0] < 2, > "set_mode may only fail on the 3rd pipe or in multiple crtc tests\n"); > - igt_info("\n%s: SKIPPED\n\n", test_name); > goto out; > } > igt_assert(fb_is_bound(o, o->fb_ids[0])); > @@ -1322,8 +1286,6 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > if (o->flags & TEST_VBLANK) > check_final_state(o, &o->vblank_state, elapsed); > > - igt_info("\n%s: PASSED\n\n", test_name); > - > out: > igt_remove_fb(drm_fd, &o->fb_info[2]); > igt_remove_fb(drm_fd, &o->fb_info[1]); > @@ -1334,6 +1296,45 @@ out: > free_test_output(o); > } > > +static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > + int crtc_count, int duration_ms) > +{ > + char test_name[128]; > + > + switch (crtc_count) { > + case RUN_TEST: > + connector_find_preferred_mode(o->_connector[0], crtc_idxs[0], o); > + if (!o->mode_valid) > + return; > + snprintf(test_name, sizeof(test_name), > + "%s::%s-%d", > + kmstest_pipe_name(o->_pipe[0]), > + kmstest_connector_type_str(o->kconnector[0]->connector_type), > + o->kconnector[0]->connector_type_id); > + break; > + case RUN_PAIR: > + connector_find_compatible_mode(crtc_idxs[0], crtc_idxs[1], o); > + if (!o->mode_valid) > + return; > + snprintf(test_name, sizeof(test_name), > + "%s:%s::%s-%d:%s-%d", > + kmstest_pipe_name(o->_pipe[0]), > + kmstest_pipe_name(o->_pipe[1]), > + kmstest_connector_type_str(o->kconnector[0]->connector_type), > + o->kconnector[0]->connector_type_id, > + kmstest_connector_type_str(o->kconnector[1]->connector_type), > + o->kconnector[1]->connector_type_id); While in principle I have nothing against expanding the alphabet of valid subtest name, it brings in repercussions that I'm not comfortable reviewing before at least three cups of coffee and even then I'll have to require some unit tests in the scary json tests directory. My runner-comms branch would help immensely for the parsing of these subtest names in runner but alas ETIME... Is it too unreadable to name these as "pipe-%s-%s-%d" for RUN_TEST ("pipe-A-eDP-1") "pipe-%s-pipe-%s-%s-%d-%s-%d" for RUN_PAIR ("pipe-A-pipe-B-eDP-1-DP-2") ? -- Petri Latvala _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev