All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips
@ 2020-04-03  9:17 Karthik B S
  2020-04-03 10:15 ` [igt-dev] ✗ GitLab.Pipeline: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Karthik B S @ 2020-04-03  9:17 UTC (permalink / raw)
  To: igt-dev

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@intel.com>
Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 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.
+ *
+ */
+
+#include "igt.h"
+#include "igt_aux.h"
+#include <poll.h>
+
+#define BUFS 4
+#define WARM_UP_TIME 2
+#define RUN_TIME 10
+
+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);
+}
+
+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);
+
+	switch (ret) {
+	case 0:
+		timeouts++;
+		igt_debug("Poll timeout %d!\n", timeouts);
+		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());
+}
+
+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;
+
+	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);
+
+	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);
+
+	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);
+	}
+
+	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',
-- 
2.22.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [igt-dev] ✗ GitLab.Pipeline: failure for tests/kms_async: Add test to validate asynchronous flips
  2020-04-03  9:17 [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips Karthik B S
@ 2020-04-03 10:15 ` Patchwork
  2020-04-03 10:23 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-04-03 10:15 UTC (permalink / raw)
  To: Karthik B S; +Cc: igt-dev

== Series Details ==

Series: tests/kms_async: Add test to validate asynchronous flips
URL   : https://patchwork.freedesktop.org/series/75453/
State : failure

== Summary ==

ERROR! This series introduces new undocumented tests:

kms_async
kms_async@async-flip-with-page-flip-events
kms_async@async-flip-without-page-flip-events

Can you document them as per the requirement in the [CONTRIBUTING.md]?

[Documentation] has more details on how to do this.

Here are few examples:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/0316695d03aa46108296b27f3982ec93200c7a6e
https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/443cc658e1e6b492ee17bf4f4d891029eb7a205d

Thanks in advance!

[CONTRIBUTING.md]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md#L19
[Documentation]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe

Other than that, pipeline status: SUCCESS.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/127990 for the overview.

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/127990
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_async: Add test to validate asynchronous flips
  2020-04-03  9:17 [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips Karthik B S
  2020-04-03 10:15 ` [igt-dev] ✗ GitLab.Pipeline: failure for " Patchwork
@ 2020-04-03 10:23 ` Patchwork
  2020-04-03 17:09 ` [igt-dev] [PATCH i-g-t] " Paulo Zanoni
  2020-04-03 17:31 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-04-03 10:23 UTC (permalink / raw)
  To: Karthik B S; +Cc: igt-dev

== Series Details ==

Series: tests/kms_async: Add test to validate asynchronous flips
URL   : https://patchwork.freedesktop.org/series/75453/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8243 -> IGTPW_4400
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html

Known issues
------------

  Here are the changes found in IGTPW_4400 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_fence_blits@basic:
    - fi-blb-e6850:       [PASS][1] -> [DMESG-WARN][2] ([i915#1612])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/fi-blb-e6850/igt@gem_tiled_fence_blits@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/fi-blb-e6850/igt@gem_tiled_fence_blits@basic.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][3] -> [FAIL][4] ([i915#262])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-dsi:         [INCOMPLETE][5] ([i915#189]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@execlists:
    - fi-bxt-dsi:         [INCOMPLETE][7] ([i915#656]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/fi-bxt-dsi/igt@i915_selftest@live@execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/fi-bxt-dsi/igt@i915_selftest@live@execlists.html

  
  [i915#1612]: https://gitlab.freedesktop.org/drm/intel/issues/1612
  [i915#189]: https://gitlab.freedesktop.org/drm/intel/issues/189
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656


Participating hosts (41 -> 43)
------------------------------

  Additional (8): fi-skl-6770hq fi-bwr-2160 fi-snb-2520m fi-ivb-3770 fi-cfl-8109u fi-skl-lmem fi-byt-n2820 fi-skl-6600u 
  Missing    (6): fi-tgl-dsi fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5560 -> IGTPW_4400

  CI-20190529: 20190529
  CI_DRM_8243: 45ccb1b8606b6ba1a5d4f8a8b4dda27bd8dbb04c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4400: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html
  IGT_5560: 213062c7dcf0cbc8069cbb5f91acbc494def33fd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_async@async-flip-without-page-flip-events
+igt@kms_async@async-flip-with-page-flip-events

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips
  2020-04-03  9:17 [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips Karthik B S
  2020-04-03 10:15 ` [igt-dev] ✗ GitLab.Pipeline: failure for " Patchwork
  2020-04-03 10:23 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-04-03 17:09 ` Paulo Zanoni
  2020-04-08  6:36   ` Karthik B S
  2020-04-03 17:31 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork
  3 siblings, 1 reply; 7+ messages in thread
From: Paulo Zanoni @ 2020-04-03 17:09 UTC (permalink / raw)
  To: Karthik B S, igt-dev

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@intel.com>
> Suggested-by: Paulo Zanoni <paulo.r.zanoni@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',

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for tests/kms_async: Add test to validate asynchronous flips
  2020-04-03  9:17 [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips Karthik B S
                   ` (2 preceding siblings ...)
  2020-04-03 17:09 ` [igt-dev] [PATCH i-g-t] " Paulo Zanoni
@ 2020-04-03 17:31 ` Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2020-04-03 17:31 UTC (permalink / raw)
  To: Karthik B S; +Cc: igt-dev

== Series Details ==

Series: tests/kms_async: Add test to validate asynchronous flips
URL   : https://patchwork.freedesktop.org/series/75453/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8243_full -> IGTPW_4400_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4400_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4400_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_4400_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_mmap_gtt@cpuset-medium-copy-xy:
    - shard-iclb:         [PASS][1] -> [FAIL][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb6/igt@gem_mmap_gtt@cpuset-medium-copy-xy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb1/igt@gem_mmap_gtt@cpuset-medium-copy-xy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [PASS][3] -> [FAIL][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk7/igt@gem_tiled_swapping@non-threaded.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk8/igt@gem_tiled_swapping@non-threaded.html

  * {igt@kms_async@async-flip-with-page-flip-events} (NEW):
    - shard-snb:          NOTRUN -> [FAIL][5] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-snb2/igt@kms_async@async-flip-with-page-flip-events.html
    - shard-iclb:         NOTRUN -> [FAIL][6] +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb5/igt@kms_async@async-flip-with-page-flip-events.html
    - shard-hsw:          NOTRUN -> [FAIL][7] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-hsw4/igt@kms_async@async-flip-with-page-flip-events.html

  * {igt@kms_async@async-flip-without-page-flip-events} (NEW):
    - shard-glk:          NOTRUN -> [FAIL][8] +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk4/igt@kms_async@async-flip-without-page-flip-events.html
    - shard-apl:          NOTRUN -> [FAIL][9] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl6/igt@kms_async@async-flip-without-page-flip-events.html
    - shard-tglb:         NOTRUN -> [FAIL][10] +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-tglb6/igt@kms_async@async-flip-without-page-flip-events.html
    - shard-kbl:          NOTRUN -> [FAIL][11] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl4/igt@kms_async@async-flip-without-page-flip-events.html

  
#### Warnings ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-kbl:          [FAIL][12] ([i915#93] / [i915#95]) -> [FAIL][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl3/igt@gem_tiled_swapping@non-threaded.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl6/igt@gem_tiled_swapping@non-threaded.html

  
New tests
---------

  New tests have been introduced between CI_DRM_8243_full and IGTPW_4400_full:

### New IGT tests (3) ###

  * igt@kms_async@async-flip-with-page-flip-events:
    - Statuses : 7 fail(s)
    - Exec time: [0.10, 1.29] s

  * igt@kms_async@async-flip-without-page-flip-events:
    - Statuses : 7 fail(s)
    - Exec time: [0.12, 1.27] s

  * igt@perf_pmu@faulting-read:
    - Statuses :
    - Exec time: [None] s

  

Known issues
------------

  Here are the changes found in IGTPW_4400_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@pm-caching:
    - shard-tglb:         [PASS][14] -> [SKIP][15] ([i915#1316] / [i915#579])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-tglb5/igt@i915_pm_rpm@pm-caching.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-tglb8/igt@i915_pm_rpm@pm-caching.html
    - shard-glk:          [PASS][16] -> [SKIP][17] ([fdo#109271])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk1/igt@i915_pm_rpm@pm-caching.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk7/igt@i915_pm_rpm@pm-caching.html
    - shard-iclb:         [PASS][18] -> [SKIP][19] ([i915#1316] / [i915#579])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb6/igt@i915_pm_rpm@pm-caching.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb8/igt@i915_pm_rpm@pm-caching.html
    - shard-hsw:          [PASS][20] -> [SKIP][21] ([fdo#109271])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-hsw6/igt@i915_pm_rpm@pm-caching.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-hsw2/igt@i915_pm_rpm@pm-caching.html

  * igt@i915_selftest@live@execlists:
    - shard-apl:          [PASS][22] -> [INCOMPLETE][23] ([i915#656])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl2/igt@i915_selftest@live@execlists.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl7/igt@i915_selftest@live@execlists.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-apl:          [PASS][24] -> [DMESG-WARN][25] ([i915#180]) +2 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque:
    - shard-glk:          [PASS][26] -> [FAIL][27] ([i915#54])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk9/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk9/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html

  * igt@kms_cursor_crc@pipe-c-cursor-dpms:
    - shard-apl:          [PASS][28] -> [FAIL][29] ([i915#54]) +1 similar issue
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-dpms.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-dpms.html
    - shard-kbl:          [PASS][30] -> [FAIL][31] ([i915#54]) +1 similar issue
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-dpms.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-dpms.html

  * igt@kms_draw_crc@draw-method-rgb565-blt-ytiled:
    - shard-glk:          [PASS][32] -> [FAIL][33] ([i915#52] / [i915#54]) +2 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-blt-ytiled.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-blt-ytiled.html

  * igt@kms_draw_crc@fill-fb:
    - shard-apl:          [PASS][34] -> [FAIL][35] ([i915#95])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl2/igt@kms_draw_crc@fill-fb.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl3/igt@kms_draw_crc@fill-fb.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [PASS][36] -> [FAIL][37] ([i915#1525] / [i915#95])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl3/igt@kms_fbcon_fbt@fbc-suspend.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html
    - shard-kbl:          [PASS][38] -> [FAIL][39] ([i915#64] / [i915#93] / [i915#95])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl4/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-snb:          [PASS][40] -> [DMESG-WARN][41] ([i915#42])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-snb4/igt@kms_flip@flip-vs-suspend.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-snb2/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [PASS][42] -> [DMESG-WARN][43] ([i915#180])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_prime@basic-crc:
    - shard-apl:          [PASS][44] -> [FAIL][45] ([i915#1031] / [i915#95])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl8/igt@kms_prime@basic-crc.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl2/igt@kms_prime@basic-crc.html
    - shard-kbl:          [PASS][46] -> [FAIL][47] ([i915#1031] / [i915#93] / [i915#95])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl4/igt@kms_prime@basic-crc.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl1/igt@kms_prime@basic-crc.html

  * igt@kms_psr@cursor_plane_move:
    - shard-tglb:         [PASS][48] -> [SKIP][49] ([i915#668]) +4 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-tglb8/igt@kms_psr@cursor_plane_move.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-tglb6/igt@kms_psr@cursor_plane_move.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [PASS][50] -> [SKIP][51] ([fdo#109441]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb3/igt@kms_psr@psr2_sprite_blt.html

  * igt@prime_vgem@wait-bsd2:
    - shard-iclb:         [PASS][52] -> [SKIP][53] ([fdo#109276]) +4 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb2/igt@prime_vgem@wait-bsd2.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb5/igt@prime_vgem@wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@legacy-engines-mixed-process@blt:
    - shard-apl:          [FAIL][54] ([i915#1528]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl6/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl2/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html

  * igt@gem_exec_params@invalid-bsd-ring:
    - shard-iclb:         [SKIP][56] ([fdo#109276]) -> [PASS][57] +1 similar issue
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb5/igt@gem_exec_params@invalid-bsd-ring.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb4/igt@gem_exec_params@invalid-bsd-ring.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-tglb:         [FAIL][58] -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-tglb2/igt@gem_tiled_swapping@non-threaded.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-tglb8/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][60] ([i915#454]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-hsw:          [FAIL][62] ([i915#1066]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-hsw6/igt@i915_pm_rc6_residency@rc6-idle.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-hsw2/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen:
    - shard-kbl:          [FAIL][64] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][65] +2 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
    - shard-apl:          [FAIL][66] ([i915#54] / [i915#95]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl8/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl7/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html

  * igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy:
    - shard-kbl:          [FAIL][68] ([i915#93] / [i915#95]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl4/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl4/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html

  * igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled:
    - shard-glk:          [FAIL][70] ([i915#177] / [i915#52] / [i915#54]) -> [PASS][71]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk2/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk3/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html

  * igt@kms_draw_crc@draw-method-rgb565-pwrite-ytiled:
    - shard-glk:          [FAIL][72] ([i915#52] / [i915#54]) -> [PASS][73] +3 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk5/igt@kms_draw_crc@draw-method-rgb565-pwrite-ytiled.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk2/igt@kms_draw_crc@draw-method-rgb565-pwrite-ytiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled:
    - shard-kbl:          [FAIL][74] ([fdo#108145] / [i915#177] / [i915#52] / [i915#54] / [i915#93] / [i915#95]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl1/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl1/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
    - shard-apl:          [FAIL][76] ([fdo#108145] / [i915#52] / [i915#54] / [i915#95]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-wc-untiled.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][78] ([i915#61]) -> [PASS][79] +1 similar issue
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-hsw6/igt@kms_flip@flip-vs-suspend.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-hsw2/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          [DMESG-WARN][80] ([i915#180]) -> [PASS][81] +3 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-glk:          [FAIL][82] ([i915#49]) -> [PASS][83]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-glk4/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-glk9/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][84] ([i915#180]) -> [PASS][85] +1 similar issue
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_cursor@pipe-a-overlay-size-64:
    - shard-apl:          [FAIL][86] ([i915#1559] / [i915#95]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl1/igt@kms_plane_cursor@pipe-a-overlay-size-64.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl3/igt@kms_plane_cursor@pipe-a-overlay-size-64.html
    - shard-kbl:          [FAIL][88] ([i915#1559] / [i915#93] / [i915#95]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-kbl6/igt@kms_plane_cursor@pipe-a-overlay-size-64.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-kbl4/igt@kms_plane_cursor@pipe-a-overlay-size-64.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][90] ([fdo#109441]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-iclb8/igt@kms_psr@psr2_no_drrs.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * {igt@perf@blocking-parameterized}:
    - shard-hsw:          [FAIL][92] ([i915#1542]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-hsw6/igt@perf@blocking-parameterized.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-hsw4/igt@perf@blocking-parameterized.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-tilingchange:
    - shard-tglb:         [FAIL][94] ([i915#1524]) -> [SKIP][95] ([i915#668])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-tilingchange.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-tilingchange.html

  * igt@runner@aborted:
    - shard-apl:          [FAIL][96] ([i915#1423]) -> ([FAIL][97], [FAIL][98]) ([i915#1423] / [i915#529])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8243/shard-apl6/igt@runner@aborted.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl7/igt@runner@aborted.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/shard-apl1/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1031]: https://gitlab.freedesktop.org/drm/intel/issues/1031
  [i915#1066]: https://gitlab.freedesktop.org/drm/intel/issues/1066
  [i915#1316]: https://gitlab.freedesktop.org/drm/intel/issues/1316
  [i915#1423]: https://gitlab.freedesktop.org/drm/intel/issues/1423
  [i915#1524]: https://gitlab.freedesktop.org/drm/intel/issues/1524
  [i915#1525]: https://gitlab.freedesktop.org/drm/intel/issues/1525
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1559]: https://gitlab.freedesktop.org/drm/intel/issues/1559
  [i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#42]: https://gitlab.freedesktop.org/drm/intel/issues/42
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#529]: https://gitlab.freedesktop.org/drm/intel/issues/529
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#64]: https://gitlab.freedesktop.org/drm/intel/issues/64
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 8)
------------------------------

  Missing    (2): pig-skl-6260u pig-glk-j5005 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_5560 -> IGTPW_4400
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_8243: 45ccb1b8606b6ba1a5d4f8a8b4dda27bd8dbb04c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4400: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html
  IGT_5560: 213062c7dcf0cbc8069cbb5f91acbc494def33fd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4400/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips
  2020-04-03 17:09 ` [igt-dev] [PATCH i-g-t] " Paulo Zanoni
@ 2020-04-08  6:36   ` Karthik B S
  2020-04-08 23:37     ` Paulo Zanoni
  0 siblings, 1 reply; 7+ messages in thread
From: Karthik B S @ 2020-04-08  6:36 UTC (permalink / raw)
  To: Paulo Zanoni, igt-dev

Thanks a lot for the review.

On 4/3/2020 10:39 PM, Paulo Zanoni wrote:
> 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@intel.com>
>> Suggested-by: Paulo Zanoni <paulo.r.zanoni@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 :).
> 

Sorry for that. I'll update in rev2. :)

> 
>> ---
>>   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>
>

Noted.

>> + */
>> +
>> +#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.
> 

Will update this.

>> +
>> +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.
> 

Does this mean if vblank interval is 16ms, then intervals of
 >1.6ms is a failure? Did I understand this right or Am I missing 
something here.

>> +}
>> +
>> +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.
> 

Will update this.

> 
>> +
>> +	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.
> 

Will update this accordingly.

>> +}
>> +
>> +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.
> 

Sure. I'll make changes to the subtests accordingly.

>> +
>> +	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.
> 

Didn't get this? FPS is expected to be bigger than RR right?
Could you please help me here, to understand what I'm missing?

> 
>> +
>> +	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.
> 

Could you please let me know how I can do this? Since I can't test with 
a try_commit as we're using the page flip IOCTL in this test.

Any other way with which I can check this?

Thanks,
Karthik

> 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',
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips
  2020-04-08  6:36   ` Karthik B S
@ 2020-04-08 23:37     ` Paulo Zanoni
  0 siblings, 0 replies; 7+ messages in thread
From: Paulo Zanoni @ 2020-04-08 23:37 UTC (permalink / raw)
  To: Karthik B S, igt-dev

Em qua, 2020-04-08 às 12:06 +0530, Karthik B S escreveu:
> Thanks a lot for the review.
> 
> On 4/3/2020 10:39 PM, Paulo Zanoni wrote:
> > 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@intel.com>
> > > Suggested-by: Paulo Zanoni <paulo.r.zanoni@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 :).
> > 
> 
> Sorry for that. I'll update in rev2. :)
> 
> > > ---
> > >   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>
> > 
> 
> Noted.
> 
> > > + */
> > > +
> > > +#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.
> > 
> 
> Will update this.
> 
> > > +
> > > +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.
> > 
> 
> Does this mean if vblank interval is 16ms, then intervals of
>  >1.6ms is a failure? Did I understand this right or Am I missing 
> something here.

This test is supposed to flip waaaaay faster than the monitor refresh
rate. So if the refresh rate is 60hz, we should be getting events much
faster than once per 16ms. We need to define a threshold where we can
be confident that issues are detected (no false positives and no false
negatives). I just came up with the 10x thing because I feel it's safe,
but maybe I'm wrong and 10x is not the best value, I only ever tested
one machine. This is the kind of thing we may have to tune after the
test is in production by CI.

> 
> > > +}
> > > +
> > > +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.
> > 
> 
> Will update this.
> 
> > > +
> > > +	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.
> > 
> 
> Will update this accordingly.
> 
> > > +}
> > > +
> > > +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.
> > 
> 
> Sure. I'll make changes to the subtests accordingly.
> 
> > > +
> > > +	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.
> > 
> 
> Didn't get this?

Printing the value is not enough, humans are not monitoring the IGT
output. We need assertions.

> FPS is expected to be bigger than RR right?

Right, like waaay bigger.

> Could you please help me here, to understand what I'm missing?

Something like:

int threshold = 10; /* TODO: may need tuning */
igt_assert(fps / 1000 > refresh_rate * threshold);

> 
> > > +
> > > +	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.
> > 
> 
> Could you please let me know how I can do this? Since I can't test with 
> a try_commit as we're using the page flip IOCTL in this test.
> 
> Any other way with which I can check this?

A quick check on the libdrm headers shows me DRM_CAP_ASYNC_PAGE_FLIP.
Maybe that's enough? Otherwise go with whatever error drmModePageFlip()
returns.

> 
> Thanks,
> Karthik
> 
> > 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',

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-04-08 23:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03  9:17 [igt-dev] [PATCH i-g-t] tests/kms_async: Add test to validate asynchronous flips Karthik B S
2020-04-03 10:15 ` [igt-dev] ✗ GitLab.Pipeline: failure for " Patchwork
2020-04-03 10:23 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2020-04-03 17:09 ` [igt-dev] [PATCH i-g-t] " Paulo Zanoni
2020-04-08  6:36   ` Karthik B S
2020-04-08 23:37     ` Paulo Zanoni
2020-04-03 17:31 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.