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

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

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.

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