Re: [igt-dev] [i-g-t] tests/kms_cursor_crc: Drop max-size test

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

On Tue-09-08-2022 08:06 pm, Juha-Pekka Heikkila wrote:
> On 27.7.2022 21.42, Sharma, Swati2 wrote:
>> LGTM
>> Reviewed-by:  Swati Sharma <swati2.sharma@intel.com>
>>
>> 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 <bhanuprakash.modem@intel.com>
>>> ---
>>>   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.

 From the called function, max cursor size is configured as 64x64. So 
the condition for igt_require_f() isn't met (all above mentioned 
conditions were failed), means 64x64 is already covered.

igt_main() {
  867         uint64_t cursor_width = 64, cursor_height = 64;

  887         data.cursor_max_w = cursor_width;
  888         data.cursor_max_h = cursor_height;
}

- Bhanu

> 
> /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);
>>>               }
>>>           }
>>>       }
>>
>