[PATCH v4 1/1] tests/kms_async_flips: subtests to validate async flips on atomic path

Karthik B S karthik.b.s at intel.com
Fri Nov 29 04:48:49 UTC 2024


Hi Santhosh,

On 11/21/2024 10:59 PM, Sharma, Swati2 wrote:
> Hi Santosh,
>
> Please fix the subject.Subject should be in imperative mood.
>
> Something like "Add subtests to validate async flips on atomic path"

Agree on this comment. With this fixed,

Reviewed-by: Karthik B S <karthik.b.s at intel.com>

>
> On 21-11-2024 05:24 pm, Santhosh Reddy Guddati wrote:
>> subtest to validate async flips with atomic commit
>>
>> v2: refactor repeated code to function (Karthik).
>> explicitly set atomic_path to false in legacy tests (Karthik).
>>
>> v3: fix test_crc and test_invalid subtests.
>>
>> v4: Add note for invalid_test for not using perform_flip (Karthik).
>>
>> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
>> ---
>>   tests/kms_async_flips.c | 173 ++++++++++++++++++++++++++++++++++------
>>   1 file changed, 147 insertions(+), 26 deletions(-)
>>
>> diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
>> index 5dec71291..6e00be3de 100644
>> --- a/tests/kms_async_flips.c
>> +++ b/tests/kms_async_flips.c
>> @@ -67,6 +67,26 @@
>>    * SUBTEST: invalid-async-flip
>>    * Description: Negative case to verify if changes in fb are 
>> rejected from kernel as expected
>>    *
>> + * SUBTEST: alternate-sync-async-flip-atomic
>> + * Description: Verify the async flip functionality and the fps 
>> during async flips using atomic path
>> + *
>> + * SUBTEST: async-flip-with-page-flip-events-atomic
>> + * Description: Wait for page flip events in between successive 
>> asynchronous flips using atomic path
>> + *
>> + * SUBTEST: test-time-stamp-atomic
>> + * Description: Verify that the async flip timestamp does not 
>> coincide with either previous
>> + *              or next vblank when async flip is done using atomic 
>> path
>> + *
>> + * SUBTEST: test-cursor-atomic
>> + * Description: Verify that the DRM_IOCTL_MODE_CURSOR passes after 
>> async flip with atomic commit
>> + *
>> + * SUBTEST: invalid-async-flip-atomic
>> + * Description: Negative case to verify if changes in fb are 
>> rejected from kernel
>> + *              as expected when async flip is done using atomic path
>> + *
>> + * SUBTEST: crc-atomic
>> + * Description: Use CRC to verify async flip scans out the correct 
>> framebuffer with atomic commit
>> + *
>>    * SUBTEST: async-flip-suspend-resume
>>    * Description: Verify the async flip functionality with suspend 
>> and resume cycle
>>    */
>> @@ -105,6 +125,7 @@ typedef struct {
>>       bool suspend_resume;
>>       bool allow_fail;
>>       struct buf_ops *bops;
>> +    bool atomic_path;
>>   } data_t;
>>     static void flip_handler(int fd_, unsigned int sequence, unsigned 
>> int tv_sec,
>> @@ -259,6 +280,21 @@ static bool async_flip_needs_extra_frame(data_t 
>> *data)
>>       return intel_display_ver(devid) >= 9 || IS_BROADWELL(devid);
>>   }
>>   +static int perform_flip(data_t *data, int frame, int flags)
>> +{
>> +    int ret;
>> +
>> +    if (!data->atomic_path) {
>> +        ret = drmModePageFlip(data->drm_fd, data->crtc_id,
>> +                      data->bufs[frame % NUM_FBS].fb_id, flags, data);
>> +    } else {
>> +        igt_plane_set_fb(data->plane, &data->bufs[frame % NUM_FBS]);
>> +        ret = igt_display_try_commit_atomic(&data->display, flags, 
>> data);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>   static void test_async_flip(data_t *data)
>>   {
>>       int ret, frame;
>> @@ -276,9 +312,7 @@ static void test_async_flip(data_t *data)
>>           if (data->alternate_sync_async) {
>>               flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
>>   -            ret = drmModePageFlip(data->drm_fd, data->crtc_id,
>> -                          data->bufs[frame % NUM_FBS].fb_id,
>> -                          flags, data);
>> +            ret = perform_flip(data, frame, flags);
>>                 igt_assert_eq(ret, 0);
>>   @@ -287,19 +321,16 @@ static void test_async_flip(data_t *data)
>>               flags |= DRM_MODE_PAGE_FLIP_ASYNC;
>>                 if (async_flip_needs_extra_frame(data)) {
>> -                ret = drmModePageFlip(data->drm_fd, data->crtc_id,
>> -                              data->bufs[frame % NUM_FBS].fb_id,
>> -                              flags, data);
>>   +                ret = perform_flip(data, frame, flags);
>>                   igt_assert_eq(ret, 0);
>>                     wait_flip_event(data);
>>               }
>>           }
>>   -        ret = drmModePageFlip(data->drm_fd, data->crtc_id,
>> -                      data->bufs[frame % NUM_FBS].fb_id,
>> -                      flags, data);
>> +        ret = perform_flip(data, frame, flags);
>> +
>>           if (frame == 1 && data->allow_fail)
>>               igt_skip_on(ret == -EINVAL);
>>           else
>> @@ -428,9 +459,7 @@ static void test_cursor(data_t *data)
>>         do_ioctl(data->drm_fd, DRM_IOCTL_MODE_CURSOR, &cur);
>>   -    ret = drmModePageFlip(data->drm_fd, data->crtc_id,
>> -                  data->bufs[0].fb_id,
>> -                  flags, data);
>> +    ret = perform_flip(data, 0, flags);
>>         igt_assert_eq(ret, 0);
>>   @@ -450,6 +479,7 @@ static void test_invalid(data_t *data)
>>       int ret, width, height;
>>       struct igt_fb fb[2];
>>       drmModeModeInfo *mode;
>> +    int flags;
>>         igt_display_commit2(&data->display, data->display.is_atomic ? 
>> COMMIT_ATOMIC : COMMIT_LEGACY);
>>   @@ -457,6 +487,8 @@ static void test_invalid(data_t *data)
>>       width = mode->hdisplay;
>>       height = mode->vdisplay;
>>   +    flags = DRM_MODE_PAGE_FLIP_ASYNC | DRM_MODE_PAGE_FLIP_EVENT;
>> +
>>       igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
>>                 I915_FORMAT_MOD_X_TILED, &fb[0]);
>>       igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
>> @@ -465,16 +497,30 @@ static void test_invalid(data_t *data)
>>       igt_plane_set_fb(data->plane, &fb[0]);
>>       igt_display_commit2(&data->display, data->display.is_atomic ? 
>> COMMIT_ATOMIC : COMMIT_LEGACY);
>>   -    /* first async flip is expected to allow modifier changes */
>> -    ret = drmModePageFlip(data->drm_fd, data->crtc_id, fb[1].fb_id,
>> -                  DRM_MODE_PAGE_FLIP_ASYNC | 
>> DRM_MODE_PAGE_FLIP_EVENT, data);
>> -    igt_assert_eq(ret, 0);
>> -    wait_flip_event(data);
>> +    /* Note: Not using perform_flips here as this subtest passes
>> +     * the fb_id parameter differently.
>> +     */
>> +    if (!data->atomic_path) {
>> +        /* first async flip is expected to allow modifier changes */
>> +        ret = drmModePageFlip(data->drm_fd, data->crtc_id, 
>> fb[1].fb_id, flags, data);
>> +        igt_assert_eq(ret, 0);
>>   -    /* subsequent async flips should reject modifier changes */
>> -    ret = drmModePageFlip(data->drm_fd, data->crtc_id, fb[0].fb_id,
>> -                  DRM_MODE_PAGE_FLIP_ASYNC | 
>> DRM_MODE_PAGE_FLIP_EVENT, data);
>> -    igt_assert(ret == -EINVAL);
>> +        wait_flip_event(data);
>> +
>> +        /* subsequent async flips should reject modifier changes */
>> +        ret = drmModePageFlip(data->drm_fd, data->crtc_id, 
>> fb[0].fb_id, flags, data);
>> +        igt_assert(ret == -EINVAL);
>> +    } else {
>> +        igt_plane_set_fb(data->plane, &fb[1]);
>> +        ret = igt_display_try_commit_atomic(&data->display, flags, 
>> data);
>> +        igt_assert_eq(ret, 0);
>> +
>> +        wait_flip_event(data);
>> +
>> +        igt_plane_set_fb(data->plane, &fb[0]);
>> +        ret = igt_display_try_commit_atomic(&data->display, flags, 
>> data);
>> +        igt_assert(ret == -EINVAL);
>> +    }
>>         /* TODO: Add verification for changes in stride, pixel format */
>>   @@ -588,6 +634,8 @@ static void test_crc(data_t *data)
>>       int ret, width, height;
>>       drmModeModeInfoPtr mode;
>>   +    igt_display_commit2(&data->display, data->display.is_atomic ? 
>> COMMIT_ATOMIC : COMMIT_LEGACY);
>> +
>>       /* make things faster by using a smallish mode */
>>       mode = &data->output->config.connector->modes[0];
>>       width = mode->hdisplay;
>> @@ -620,8 +668,9 @@ static void test_crc(data_t *data)
>>           paint_fb(data, &data->bufs[frame], 1, height, 0xff0000ff);
>>             data->flip_pending = true;
>> -        ret = drmModePageFlip(data->drm_fd, data->crtc_id, 
>> data->bufs[frame].fb_id,
>> -                      DRM_MODE_PAGE_FLIP_ASYNC | 
>> DRM_MODE_PAGE_FLIP_EVENT, data);
>> +
>> +        ret = perform_flip(data, frame, DRM_MODE_PAGE_FLIP_ASYNC |
>> +                   DRM_MODE_PAGE_FLIP_EVENT);
>>           igt_assert_eq(ret, 0);
>>             wait_events_crc(data);
>> @@ -708,6 +757,18 @@ igt_main
>>           igt_describe("Wait for page flip events in between 
>> successive asynchronous flips");
>> igt_subtest_with_dynamic("async-flip-with-page-flip-events") {
>>               data.alternate_sync_async = false;
>> +            data.atomic_path = false;
>> +            if (is_intel_device(data.drm_fd))
>> +                run_test_with_modifiers(&data, test_async_flip);
>> +            else
>> +                run_test(&data, test_async_flip);
>> +        }
>> +
>> +        igt_describe("Wait for page flip events in between successive "
>> +                 "asynchronous flips using atomic path");
>> + igt_subtest_with_dynamic("async-flip-with-page-flip-events-atomic") {
>> +            data.alternate_sync_async = false;
>> +            data.atomic_path = true;
>>               if (is_intel_device(data.drm_fd))
>>                   run_test_with_modifiers(&data, test_async_flip);
>>               else
>> @@ -717,12 +778,30 @@ igt_main
>>           igt_describe("Alternate between sync and async flips");
>>           igt_subtest_with_dynamic("alternate-sync-async-flip") {
>>               data.alternate_sync_async = true;
>> +            data.atomic_path = false;
>> +            run_test(&data, test_async_flip);
>> +        }
>> +
>> +        igt_describe("Alternate between sync and async flips using 
>> atomic path");
>> + igt_subtest_with_dynamic("alternate-sync-async-flip-atomic") {
>> +            data.alternate_sync_async = true;
>> +            data.atomic_path = true;
>>               run_test(&data, test_async_flip);
>>           }
>>   -        igt_describe("Verify that the async flip timestamp does 
>> not coincide with either previous or next vblank");
>> -        igt_subtest_with_dynamic("test-time-stamp")
>> +        igt_describe("Verify that the async flip timestamp does not "
>> +                 "coincide with either previous or next vblank");
>> +        igt_subtest_with_dynamic("test-time-stamp") {
>> +            data.atomic_path = false;
>>               run_test(&data, test_timestamp);
>> +        }
>> +
>> +        igt_describe("Verify that the async flip timestamp does not 
>> coincide "
>> +                 "with either previous or next vblank with atomic 
>> path");
>> +        igt_subtest_with_dynamic("test-time-stamp-atomic") {
>> +            data.atomic_path = true;
>> +            run_test(&data, test_timestamp);
>> +        }
>>       }
>>         igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes 
>> after async flip");
>> @@ -733,9 +812,25 @@ igt_main
>>            * supported in cursor plane.
>>            */
>> igt_skip_on_f(i915_psr2_selective_fetch_check(data.drm_fd, NULL),
>> -                  "PSR2 sel fetch causes cursor to be added to 
>> primary plane " \
>> +                  "PSR2 sel fetch causes cursor to be added to 
>> primary plane "
>>                     "pages flips and async flip is not supported in 
>> cursor\n");
>>   +        data.atomic_path = false;
>> +        run_test(&data, test_cursor);
>> +    }
>> +
>> +    igt_describe("Verify that the DRM_IOCTL_MODE_CURSOR passes after "
>> +             "async flip with atomic commit");
>> +    igt_subtest_with_dynamic("test-cursor-atomic") {
>> +        /*
>> +         * Intel's PSR2 selective fetch adds other planes to state when
>> +         * necessary, causing the async flip to fail because async 
>> flip is not
>> +         * supported in cursor plane.
>> +         */
>> + igt_skip_on_f(i915_psr2_selective_fetch_check(data.drm_fd, NULL),
>> +                  "PSR2 sel fetch causes cursor to be added to 
>> primary plane "
>> +                  "pages flips and async flip is not supported in 
>> cursor\n");
>> +        data.atomic_path = true;
>>           run_test(&data, test_cursor);
>>       }
>>   @@ -748,6 +843,21 @@ igt_main
>> igt_require(igt_display_has_format_mod(&data.display, 
>> DRM_FORMAT_XRGB8888,
>>                                  I915_FORMAT_MOD_Y_TILED));
>>   +        data.atomic_path = false;
>> +        run_test(&data, test_invalid);
>> +    }
>> +
>> +    igt_describe("Negative case to verify if changes in fb are 
>> rejected "
>> +             "from kernel as expected when async flip is done using 
>> atomic path");
>> +    igt_subtest_with_dynamic("invalid-async-flip-atomic") {
>> +        data.atomic_path = true;
>> +        /* TODO: support more vendors */
>> +        igt_require(is_intel_device(data.drm_fd));
>> + igt_require(igt_display_has_format_mod(&data.display, 
>> DRM_FORMAT_XRGB8888,
>> +                               I915_FORMAT_MOD_X_TILED));
>> + igt_require(igt_display_has_format_mod(&data.display, 
>> DRM_FORMAT_XRGB8888,
>> +                               I915_FORMAT_MOD_Y_TILED));
>> +
>>           run_test(&data, test_invalid);
>>       }
>>   @@ -756,6 +866,17 @@ igt_main
>>           /* Devices without CRC can't run this test */
>>           igt_require_pipe_crc(data.drm_fd);
>>   +        data.atomic_path = false;
>> +        run_test(&data, test_crc);
>> +    }
>> +
>> +    igt_describe("Use CRC to verify async flip scans out the correct 
>> framebuffer "
>> +             "with atomic commit");
>> +    igt_subtest_with_dynamic("crc-atomic") {
>> +        /* Devices without CRC can't run this test */
>> +        igt_require_pipe_crc(data.drm_fd);
>> +
>> +        data.atomic_path = true;
>>           run_test(&data, test_crc);
>>       }


More information about the igt-dev mailing list