From: "Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>
To: Swati Sharma <swati2.sharma@intel.com>, <igt-dev@lists.freedesktop.org>
Subject: Re: [igt-dev] [PATCH i-g-t 1/2] tests/kms_universal_plane: convert subtests to dynamic subtests
Date: Tue, 26 Sep 2023 21:43:37 +0530 [thread overview]
Message-ID: <0c2e957d-34fb-4ea8-6a65-8c01888d2e16@intel.com> (raw)
In-Reply-To: <20230918095033.257372-2-swati2.sharma@intel.com>
Hi Swati,
On Mon-18-09-2023 03:20 pm, Swati Sharma wrote:
> Convert subtests to dynamic subtests.
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
> tests/kms_universal_plane.c | 122 +++++++++---------------------------
> 1 file changed, 31 insertions(+), 91 deletions(-)
>
> diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
> index 8aa43a929..45278916d 100644
> --- a/tests/kms_universal_plane.c
> +++ b/tests/kms_universal_plane.c
> @@ -97,6 +97,8 @@ typedef struct {
> int drm_fd;
> igt_display_t display;
> int display_ver;
> + enum pipe pipe;
> + igt_output_t *output;
Why do we need this change, since you are passing pipe & output as args
in each function call?
> } data_t;
>
> typedef struct {
> @@ -200,11 +202,6 @@ functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> int num_primary = 0, num_cursor = 0;
> int i;
>
> - igt_require_pipe(display, pipe);
> -
> - igt_info("Testing connector %s using pipe %s\n", igt_output_name(output),
> - kmstest_pipe_name(pipe));
> -
> functional_test_init(&test, output, pipe);
>
> /*
> @@ -443,11 +440,6 @@ sanity_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> int i;
> int expect = 0;
>
> - igt_require_pipe(&data->display, pipe);
> -
> - igt_info("Using (pipe %s + %s) to run the subtest.\n",
> - kmstest_pipe_name(pipe), igt_output_name(output));
> -
> igt_output_set_pipe(output, pipe);
> mode = igt_output_get_mode(output);
>
> @@ -560,11 +552,6 @@ pageflip_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> fd_set fds;
> int ret = 0;
>
> - igt_require_pipe(&data->display, pipe);
> -
> - igt_info("Using (pipe %s + %s) to run the subtest.\n",
> - kmstest_pipe_name(pipe), igt_output_name(output));
> -
> igt_output_set_pipe(output, pipe);
>
> pageflip_test_init(&test, output, pipe);
> @@ -676,13 +663,9 @@ cursor_leak_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> int r, g, b;
> int count1, count2;
>
> - igt_require_pipe(display, pipe);
> igt_require(display->has_cursor_plane);
> igt_require_intel(data->drm_fd);
>
> - igt_info("Using (pipe %s + %s) to run the subtest.\n",
> - kmstest_pipe_name(pipe), igt_output_name(output));
> -
> igt_output_set_pipe(output, pipe);
> mode = igt_output_get_mode(output);
>
> @@ -717,7 +700,6 @@ cursor_leak_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> igt_skip("Primary and/or cursor are unavailable\n");
> }
>
> -
Nit: Unrelated change.
> igt_plane_set_fb(primary, &background_fb);
> igt_display_commit2(display, COMMIT_LEGACY);
>
> @@ -808,10 +790,6 @@ pageflip_win_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
> int ret = 0;
>
> igt_skip_on(is_intel_device(data->drm_fd) && data->display_ver < 9);
Please don't skip the dynamic subtest, instead just don't run it.
Also, please fix igt_require in cursor_leak_test_pipe().
> - igt_require_pipe(&data->display, pipe);
> -
> - igt_info("Using (pipe %s + %s) to run the subtest.\n",
> - kmstest_pipe_name(pipe), igt_output_name(output));
>
> igt_output_set_pipe(output, pipe);
>
> @@ -865,103 +843,68 @@ pipe_output_combo_valid(igt_display_t *display,
> }
>
> static void
> -run_tests_for_pipe(data_t *data, enum pipe pipe)
> +run_tests(data_t *data)
> {
> - igt_output_t *output;
> -
> - igt_fixture {
> - int valid_tests = 0;
> -
> - igt_require_pipe(&data->display, pipe);
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output)
> - valid_tests++;
> -
> - igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> - }
> -
> igt_describe("Check the switching between different primary plane fbs with CRTC off");
> - igt_subtest_f("universal-plane-pipe-%s-functional",
> - kmstest_pipe_name(pipe)) {
> - bool found = false;
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (!pipe_output_combo_valid(&data->display, pipe, output))
> + igt_subtest_with_dynamic_f("universal-plane-functional") {
--------------------------------^
'_f' variant is not required if the subtest name is fixed.
> + for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
This API will limit the execution to single pipe, are we not missing any
coverage?
> + if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
> continue;
>
> - found = true;
> - functional_test_pipe(data, pipe, output);
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name)
-----------------------------------------------------------------------------------^
Please use igt_output_name() for output name.
- Bhanu
> + functional_test_pipe(data, data->pipe, data->output);
> }
> - igt_require_f(found, "No valid pipe/output combo found.\n");
> }
>
> igt_describe("Test for scale-up or scale-down using universal plane API without covering CRTC");
> - igt_subtest_f("universal-plane-pipe-%s-sanity",
> - kmstest_pipe_name(pipe)) {
> - bool found = false;
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (!pipe_output_combo_valid(&data->display, pipe, output))
> + igt_subtest_with_dynamic_f("universal-plane-sanity") {
> + for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
> + if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
> continue;
>
> - found = true;
> - sanity_test_pipe(data, pipe, output);
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name)
> + sanity_test_pipe(data, data->pipe, data->output);
> }
> - igt_require_f(found, "No valid pipe/output combo found.\n");
> }
>
> igt_describe("Check pageflips while primary plane is disabled before IOCTL or between IOCTL"
> - " and pageflip execution");
> - igt_subtest_f("disable-primary-vs-flip-pipe-%s",
> - kmstest_pipe_name(pipe)) {
> - bool found = false;
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (!pipe_output_combo_valid(&data->display, pipe, output))
> + " and pageflip execution");
> + igt_subtest_with_dynamic_f("disable-primary-vs-flip") {
> + for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
> + if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
> continue;
>
> - found = true;
> - pageflip_test_pipe(data, pipe, output);
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name)
> + pageflip_test_pipe(data, data->pipe, data->output);
> }
> - igt_require_f(found, "No valid pipe/output combo found.\n");
> }
>
> igt_describe("Check for cursor leaks after performing cursor operations");
> - igt_subtest_f("cursor-fb-leak-pipe-%s",
> - kmstest_pipe_name(pipe)) {
> - bool found = false;
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (!pipe_output_combo_valid(&data->display, pipe, output))
> + igt_subtest_with_dynamic_f("cursor-fb-leak") {
> + for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
> + if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
> continue;
>
> - found = true;
> - cursor_leak_test_pipe(data, pipe, output);
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name)
> + cursor_leak_test_pipe(data, data->pipe, data->output);
> }
> - igt_require_f(found, "No valid pipe/output combo found.\n");
> }
>
> igt_describe("Check if pageflip succeeds in windowed setting");
> - igt_subtest_f("universal-plane-pageflip-windowed-pipe-%s",
> - kmstest_pipe_name(pipe)) {
> - bool found = false;
> -
> - for_each_valid_output_on_pipe(&data->display, pipe, output) {
> - if (!pipe_output_combo_valid(&data->display, pipe, output))
> + igt_subtest_with_dynamic_f("universal-plane-pageflip-windowed") {
> + for_each_pipe_with_single_output(&data->display, data->pipe, data->output) {
> + if (!pipe_output_combo_valid(&data->display, data->pipe, data->output))
> continue;
>
> - found = true;
> - pageflip_win_test_pipe(data, pipe, output);
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(data->pipe), data->output->name)
> + pageflip_win_test_pipe(data, data->pipe, data->output);
> }
> - igt_require_f(found, "No valid pipe/output combo found.\n");
> }
> }
>
> -static data_t data;
> -
> igt_main
> {
> - enum pipe pipe;
> + data_t data;
>
> igt_fixture {
> data.drm_fd = drm_open_driver_master(DRIVER_ANY);
> @@ -975,10 +918,7 @@ igt_main
> igt_display_require_output(&data.display);
> }
>
> - for_each_pipe_static(pipe) {
> - igt_subtest_group
> - run_tests_for_pipe(&data, pipe);
> - }
> + run_tests(&data);
>
> igt_fixture {
> igt_display_fini(&data.display);
next prev parent reply other threads:[~2023-09-26 16:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-18 9:50 [igt-dev] [PATCH i-g-t 0/2] tests/kms_universal_plane: Dynamic conversion Swati Sharma
2023-09-18 9:50 ` [igt-dev] [PATCH i-g-t 1/2] tests/kms_universal_plane: convert subtests to dynamic subtests Swati Sharma
2023-09-26 16:13 ` Modem, Bhanuprakash [this message]
2023-09-18 9:50 ` [igt-dev] [PATCH i-g-t 2/2] tests/kms_universal_plane: update testplan doc Swati Sharma
2023-09-26 16:14 ` Modem, Bhanuprakash
2023-09-18 12:28 ` [igt-dev] ✓ CI.xeBAT: success for tests/kms_universal_plane: Dynamic conversion Patchwork
2023-09-18 12:30 ` [igt-dev] ✓ Fi.CI.BAT: " Patchwork
2023-09-19 13:40 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2023-09-22 15:20 ` Patchwork
2023-09-23 5:23 ` Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c2e957d-34fb-4ea8-6a65-8c01888d2e16@intel.com \
--to=bhanuprakash.modem@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=swati2.sharma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox