Em sex, 2020-04-03 às 14:47 +0530, Karthik B S escreveu:
> Asynchronous flips are issued using the page flip IOCTL.
> The test consists of two subtests. The first subtest waits for
> the page flip event to be recevied before giving the next flip,
> and the second subtest doesn't wait for page flip events.
>
> The test passes if the IOCTL is succesful.
>
> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> Suggested-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
I feel I deserve a little more credit than just a Suggested-by since
this still has a lot of code I wrote :).
> ---
> tests/Makefile.sources | 1 +
> tests/kms_async.c | 206 +++++++++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 3 files changed, 208 insertions(+)
> create mode 100644 tests/kms_async.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 4e44c98c..eedf4fcb 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -29,6 +29,7 @@ TESTS_progs = \
> fbdev \
> kms_3d \
> kms_addfb_basic \
> + kms_async \
> kms_atomic \
> kms_atomic_interruptible \
> kms_atomic_transition \
> diff --git a/tests/kms_async.c b/tests/kms_async.c
> new file mode 100644
> index 00000000..0e21f9f6
> --- /dev/null
> +++ b/tests/kms_async.c
> @@ -0,0 +1,206 @@
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
* Authors: Paulo Zanoni <email>
* Karthik B S <email>
> + */
> +
> +#include "igt.h"
> +#include "igt_aux.h"
> +#include <poll.h>
> +
> +#define BUFS 4
> +#define WARM_UP_TIME 2
> +#define RUN_TIME 10
Each subtest takes 12 seconds to run, by definition (sans timeouts).
Maybe for IGT we want to make this a little faster? When I wrote this
code I needed very precise FPS values, IGT only needs to assert things
are working as expected.
1s WARM_UP_TIME and 4s RUN_TIME are probably very fine, and even less
than that is also probably fine for IGT. Please play with the tunables.
IGT needs to be as fast as it can while still achieving 100%
reliability.
> +
> +typedef struct {
> + int drm_fd;
> + igt_display_t display;
> +} data_t;
> +
> +static drmModeConnectorPtr find_connector_for_modeset(data_t *data)
> +{
> + igt_output_t *output;
> + drmModeConnectorPtr ret = NULL;
> +
> + for_each_connected_output(&data->display, output) {
> + if (output->config.connector->count_modes > 0) {
> + ret = output->config.connector;
> + break;
> + }
> + }
> +
> + igt_assert_f(ret, "Connector NOT found\n");
> + return ret;
> +}
> +
> +static void flip_handler(int fd_, unsigned int sequence, unsigned int tv_sec,
> + unsigned int tv_usec, void *_data)
> +{
> + static unsigned int last_ms;
> + unsigned int cur_ms;
> +
> + igt_assert(_data == NULL);
> +
> + cur_ms = tv_sec * 1000 + tv_usec / 1000;
> +
> + igt_debug("Flip interval: %dms\n", cur_ms - last_ms);
> +
> + last_ms = cur_ms;
> +
> + igt_debug("Flip event received, fd:%d seq:%u tv_sec:%u, "
> + "tv_usec:%u data:%p\n",
> + fd_, sequence, tv_sec, tv_usec, _data);
In my original implementation these messages were under if (0) because
they significantly affected FPS.
I think we should probably have some kind of igt_assert() here instead
of printing stuff. This program is supposed to achieve thousands of
frames per second even on slow hardware (I think I got 2500 on a slow
GLK?). Any intervals that resemble the monitor refresh rate by a 10x
error rate are probably failures.
> +}
> +
> +static void wait_flip_event(data_t *data)
> +{
> + int ret;
> + drmEventContext evctx;
> + struct pollfd pfd;
> + static int timeouts = 0;
> +
> + evctx.version = 2;
> + evctx.vblank_handler = NULL;
> + evctx.page_flip_handler = flip_handler;
> +
> + pfd.fd = data->drm_fd;
> + pfd.events = POLLIN;
> + pfd.revents = 0;
> +
> + ret = poll(&pfd, 1, 10000);
10s timeouts are not igt-friendly. I used a giant number because I
wanted to read the logs. IGT doesn't need nor want 10s timeouts.
> +
> + switch (ret) {
> + case 0:
> + timeouts++;
> + igt_debug("Poll timeout %d!\n", timeouts);
I don't remember exactly, but I think a timeout was a failure? Maybe
igt_assert() here?
> + break;
> + case 1:
> + ret = drmHandleEvent(data->drm_fd, &evctx);
> + igt_assert(ret == 0);
> + break;
> + default:
> + /* unexpected */
> + igt_assert(0);
> + }
> +}
> +
> +static void make_fb(data_t *data, struct igt_fb *fb,
> + drmModeConnectorPtr connector, int index)
> +{
> + uint32_t width, height;
> + int rec_width;
> +
> + width = connector->modes[0].hdisplay;
> + height = connector->modes[0].vdisplay;
> +
> + rec_width = width / (BUFS * 2);
> +
> + igt_create_fb(data->drm_fd, width, height, DRM_FORMAT_ARGB8888,
> + LOCAL_I915_FORMAT_MOD_X_TILED, fb);
> + igt_draw_fill_fb(data->drm_fd, fb, 0x88);
> + igt_draw_rect_fb(data->drm_fd, NULL, NULL, fb, IGT_DRAW_MMAP_CPU,
> + rec_width*2+rec_width*index, height/4,
> + rec_width, height/2, rand());
Please follow the IGT coding style regarding spaces.
> +}
> +
> +static void test_async_flip(data_t *data, bool wait_for_flips)
> +{
> + struct igt_fb bufs[BUFS];
> + drmModeResPtr res;
> + drmModeConnectorPtr connector;
> + uint32_t crtc_id;
> + int i, ret, frame, warm_end_frame;
> + long long int fps;
> + struct timeval start, end, diff;
> + bool warming_up = true;
> +
>From here...
> + res = drmModeGetResources(data->drm_fd);
> + igt_assert(res);
> +
> + kmstest_unset_all_crtcs(data->drm_fd, res);
> +
> + connector = find_connector_for_modeset(data);
> + crtc_id = kmstest_find_crtc_for_connector(data->drm_fd,
> + res, connector, 0);
> +
> + for (i = 0; i < BUFS; i++)
> + make_fb(data, &bufs[i], connector, i);
> +
> + ret = drmModeSetCrtc(data->drm_fd, crtc_id, bufs[0].fb_id, 0, 0,
> + &connector->connector_id, 1, &connector->modes[0]);
> + igt_assert(ret == 0);
... to here we probably don't want to repeat for every subtest,
especially if we add more in the future. Make them part of the fixture
and adjust the subtests to deal with it.
> +
> + gettimeofday(&start, NULL);
> + frame = 1;
> + do {
> + int flags = DRM_MODE_PAGE_FLIP_ASYNC;
> +
> + if (wait_for_flips)
> + flags |= DRM_MODE_PAGE_FLIP_EVENT;
> +
> + ret = drmModePageFlip(data->drm_fd, crtc_id,
> + bufs[frame % 4].fb_id,
> + flags, NULL);
> +
> + igt_assert(ret == 0 || ret == -EBUSY);
> +
> + if (wait_for_flips)
> + wait_flip_event(data);
> +
> + gettimeofday(&end, NULL);
> + timersub(&end, &start, &diff);
> +
> + /* 2s of warm-up time for the freq to stabilize */
> + if (warming_up && diff.tv_sec >= WARM_UP_TIME) {
> + warming_up = false;
> + warm_end_frame = frame;
> + start = end;
> + }
> +
> + frame++;
> + } while (diff.tv_sec < RUN_TIME);
> +
> + fps = (frame - warm_end_frame) * 1000 / RUN_TIME;
> + igt_info("fps: %lld.%03lld\n", fps / 1000, fps % 1000);
We have to igt_assert() that FPS is a value significantly bigger than
the mode refresh rate.
> +
> + for (i = 0; i < BUFS; i++)
> + igt_remove_fb(data->drm_fd, &bufs[i]);
> +}
> +
> +igt_main
> +{
> + data_t data;
> +
> + igt_fixture {
> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + kmstest_set_vt_graphics_mode();
> + igt_display_require(&data.display, data.drm_fd);
> + igt_display_require_output(&data.display);
We have to SKIP in case async flips are not supported by the Kernel.
Thanks,
Paulo
> + }
> +
> + igt_subtest("async-flip-with-page-flip-events")
> + test_async_flip(&data, true);
> + igt_subtest("async-flip-without-page-flip-events")
> + test_async_flip(&data, false);
> +
> + igt_fixture
> + igt_display_fini(&data.display);
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index e882f4dc..0830b1c9 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -13,6 +13,7 @@ test_progs = [
> 'fbdev',
> 'kms_3d',
> 'kms_addfb_basic',
> + 'kms_async',
> 'kms_atomic',
> 'kms_atomic_interruptible',
> 'kms_atomic_transition',