From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by gabe.freedesktop.org (Postfix) with ESMTPS id AA517D8B59 for ; Tue, 9 Aug 2022 14:36:47 +0000 (UTC) Received: by mail-ed1-x52c.google.com with SMTP id e13so15323147edj.12 for ; Tue, 09 Aug 2022 07:36:47 -0700 (PDT) Message-ID: <210664fd-359a-4700-c540-3d81981e04b3@gmail.com> Date: Tue, 9 Aug 2022 17:36:39 +0300 MIME-Version: 1.0 Content-Language: en-US To: "Sharma, Swati2" , Bhanuprakash Modem , igt-dev@lists.freedesktop.org, petri.latvala@intel.com References: <20220726031116.974261-1-bhanuprakash.modem@intel.com> <4330ad72-d7b5-80eb-0236-83944fd6f063@intel.com> From: Juha-Pekka Heikkila In-Reply-To: <4330ad72-d7b5-80eb-0236-83944fd6f063@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [igt-dev] [i-g-t] tests/kms_cursor_crc: Drop max-size test List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: juhapekka.heikkila@gmail.com Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 27.7.2022 21.42, Sharma, Swati2 wrote: > LGTM > Reviewed-by:  Swati Sharma > > On 26-Jul-22 8:41 AM, Bhanuprakash Modem wrote: >> As we already covering it by the other tests, there is no point in >> doing the "max-size" test. Hence dropping it. This doesn't look correct. >> >> This patch will also do some cleanup to avoid skips in igt_dynamic. >> >> Signed-off-by: Bhanuprakash Modem >> --- >>   tests/kms_cursor_crc.c | 47 ++++++++++++++++++++---------------------- >>   1 file changed, 22 insertions(+), 25 deletions(-) >> >> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c >> index f07a045a..14d4c4ca 100644 >> --- a/tests/kms_cursor_crc.c >> +++ b/tests/kms_cursor_crc.c >> @@ -576,7 +576,7 @@ static void test_cursor_opaque(data_t *data) >>       test_cursor_alpha(data); >>   } >> -static void require_cursor_size(data_t *data, int w, int h) >> +static bool require_cursor_size(data_t *data, int w, int h) >>   { >>       igt_fb_t primary_fb; >>       drmModeModeInfo *mode; >> @@ -618,14 +618,11 @@ static void require_cursor_size(data_t *data, >> int w, int h) >>       igt_remove_fb(data->drm_fd, &primary_fb); >>       igt_output_set_pipe(output, PIPE_NONE); >> -    igt_skip_on_f(ret, "Cursor size %dx%d not supported by driver\n", >> w, h); >> +    return !!ret; >>   } >>   static void run_test(data_t *data, void (*testfunc)(data_t *), int >> cursor_w, int cursor_h) >>   { >> -    if (data->fb.fb_id != 0) >> -        require_cursor_size(data, cursor_w, cursor_h); >> - >>       prepare_crtc(data, cursor_w, cursor_h); >>       testfunc(data); >>       cleanup_crtc(data); >> @@ -696,24 +693,15 @@ static void run_size_tests(data_t *data, void >> (*testfunc)(data_t *), >>       char name[32]; >>       enum pipe pipe; >> -    if (w == 0 && h == 0) >> -        strcpy(name, "max-size"); >> -    else >> -        snprintf(name, sizeof(name), "%dx%d", w, h); >> - >> -    if (w == 0 && h == 0) { >> -        w = data->cursor_max_w; >> -        h = data->cursor_max_h; >> -        /* >> -         * No point in doing the "max-size" test if >> -         * it was already covered by the other tests. >> -         */ >> -        igt_require_f(w != h || w > 512 || h > 512 || >> -                  !is_power_of_two(w) || !is_power_of_two(h), >> -                  "Cursor max size %dx%d already covered by other >> tests\n", >> -                  w, h); Reading from above, for cursor maximum size if any of: * width is not equal to height * width > 512 or height > 512 * (width & (width - 1)) != 0 or (height & (height - 1)) != 0 then maximum cursor size was not covered by any other test. You can see from where run_size_tests is called why these conditions. /Juha-Pekka >> -    } >> +    snprintf(name, sizeof(name), "%dx%d", w, h); >> + >>       create_cursor_fb(data, w, h); >> +    if (require_cursor_size(data, w, h)) { >> +        igt_debug("Cursor size %dx%d not supported by driver\n", w, h); >> + >> +        igt_remove_fb(data->drm_fd, &data->fb); >> +        return; >> +    } >>       for_each_pipe(&data->display, pipe) { >>           data->pipe = pipe; >> @@ -807,6 +795,12 @@ static void run_tests_on_pipe(data_t *data) >>               data->pipe = pipe; >>               data->flags = TEST_DPMS; >> +            if (require_cursor_size(data, data->cursor_max_w, >> data->cursor_max_h)) { >> +                igt_debug("Cursor size %dx%d not supported by driver\n", >> +                      data->cursor_max_w, data->cursor_max_h); >> +                continue; >> +            } >> + >>               igt_dynamic_f("pipe-%s-%s", >>                         kmstest_pipe_name(pipe), >>                         data->output->name) >> @@ -822,6 +816,12 @@ static void run_tests_on_pipe(data_t *data) >>               data->pipe = pipe; >>               data->flags = TEST_SUSPEND; >> +            if (require_cursor_size(data, data->cursor_max_w, >> data->cursor_max_h)) { >> +                igt_debug("Cursor size %dx%d not supported by driver\n", >> +                      data->cursor_max_w, data->cursor_max_h); >> +                continue; >> +            } >> + >>               igt_dynamic_f("pipe-%s-%s", >>                         kmstest_pipe_name(pipe), >>                         data->output->name) >> @@ -855,9 +855,6 @@ static void run_tests_on_pipe(data_t *data) >>                       igt_subtest_group >>                           run_size_tests(data, size_tests[i].testfunc, >> w, h); >>                   } >> - >> -                igt_subtest_group >> -                    run_size_tests(data, size_tests[i].testfunc, 0, 0); >>               } >>           } >>       } >