[PATCH i-g-t v5 00/13] Implement sw_sync test

[PATCH i-g-t v5 00/13] Implement sw_sync test

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This series implements the sw_sync test and the lib/sw_sync helper functions for said test.

Gustavo Padovans sw_sync series was just de-staged in gregkh-staging/staging-next [1], and this test is targeted at verifying the functionality implemented in that series. 

The sw_sync subtests range from very basic tests of the sw_sync functionality, to stress testing and randomized tests.

[1] http://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/

Changes since v1:
Added "Reviewed-by: Eric Engestrom <eric@engestrom.ch>" tag

  lib/sw_sync:
  - Fixed fd value checking
  - Fixed fd validity check in sw_sync_fd_close()
  - Moved sw_sync related paths to define
  - Switched from malloc+memset to calloc in sync_file_info()
  - Switched sizeof to dereferenced ptr

  tests/sw_sync:
  - Moved lib/sw_sync related code to lib/sw_sync commit
  - Replaced memset with assignment in loop


Changes since v2:

  lib/sw_sync:
  - Replaced fd validity check in sw_sync_timeline_create()
  - Replace sw_sync_XXX_destroy() functions with close()
  - Simplified sw_sync_timeline_inc() comparison
  - Changed sw_sync_merge() return value to -errno
  - Changed name of sw_sync_fence_size() to sw_sync_fence_count()
  - Reworked implementation of sw_sync_fence_count()
  - Reworked implementation of sw_sync_fence_count_status()

  tests/sw_sync:
  - Replace sw_sync_XXX_destroy() functions with close()


Changes since v3:

  lib/sw_sync:
  - Changed sw_sync_fence_create() to take uint32_t seqno
  - Added raw __sw_sync_fence_create() and failure check sw_sync_fence_create()

  tests/sw_sync:
  - Switch to using __sw_sync_fence_create() for failure cases


Changes since v4:

  lib/sw_sync:
  - Fixed whitespace in sw_sync_fence_count()
  - Fixed whitespace and style of sw_sync_wait()
  - Fixed whitespace in __sw_sync_fence_count_status()

Robert Foss (13):
  lib/sw_sync: Add helper functions for managing synchronization
    primitives
  tests/sw_sync: Add sw_sync test
  tests/sw_sync: Add subtest test_alloc_fence
  tests/sw_sync: Add subtest test_alloc_fence_invalid_timeline
  tests/sw_sync: Add subtest test_alloc_merge_fence
  tests/sw_sync: Add subtest test_sync_wait
  tests/sw_sync: Add subtest test_sync_merge
  tests/sw_sync: Add subtest test_sync_merge_same
  tests/sw_sync: Add subtest test_sync_multi_consumer
  tests/sw_sync: Add subtest test_sync_multi_consumer_producer
  tests/sw_sync: Add subtest test_sync_random_merge
  tests/sw_sync: Add subtest test_sync_multi_timeline_wait
  tests/sw_sync: Add subtest test_sync_multi_producer_single_consumer

 lib/Makefile.sources   |   2 +
 lib/sw_sync.c          | 181 +++++++++++++
 lib/sw_sync.h          |  48 ++++
 tests/Makefile.sources |   1 +
 tests/sw_sync.c        | 694 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 926 insertions(+)
 create mode 100644 lib/sw_sync.c
 create mode 100644 lib/sw_sync.h
 create mode 100644 tests/sw_sync.c

-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 01/13] lib/sw_sync: Add helper functions for managing synchronization primitives

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

Base functions to help testing the Sync File Framework (explicit fencing
mechanism ported from Android).
These functions allow you to create, use and destroy timelines and fences.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 lib/Makefile.sources |   2 +
 lib/sw_sync.c        | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/sw_sync.h        |  48 ++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 lib/sw_sync.c
 create mode 100644 lib/sw_sync.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index bac9a7f..3dc7c3c 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -61,6 +61,8 @@ lib_source_list =	 	\
 	rendercopy_gen8.c	\
 	rendercopy_gen9.c	\
 	rendercopy.h		\
+	sw_sync.c		\
+	sw_sync.h		\
 	intel_reg_map.c		\
 	intel_iosf.c		\
 	igt_kms.c		\
diff --git a/lib/sw_sync.c b/lib/sw_sync.c
new file mode 100644
index 0000000..85d9226
--- /dev/null
+++ b/lib/sw_sync.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright 2012 Google, Inc
+ * Copyright © 2016 Collabora, Ltd.
+ *
+ * Based on the implementation from the Android Open Source Project
+ *
+ * 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:
+ *    Robert Foss <robert.foss@collabora.com>
+ */
+
+#ifndef ANDROID
+#define _GNU_SOURCE
+#else
+#include <libgen.h>
+#endif
+#include <fcntl.h>
+#include <poll.h>
+#include <stdint.h>
+#include <linux/sync_file.h>
+#include <sys/ioctl.h>
+
+#include "sw_sync.h"
+#include "drmtest.h"
+#include "ioctl_wrappers.h"
+
+#ifndef SW_SYNC_IOC_INC
+struct sw_sync_create_fence_data {
+	__u32	value;
+	char	name[32];
+	__s32	fence;
+};
+
+#define SW_SYNC_IOC_MAGIC		'W'
+#define SW_SYNC_IOC_CREATE_FENCE	_IOWR(SW_SYNC_IOC_MAGIC, 0,\
+						struct sw_sync_create_fence_data)
+#define SW_SYNC_IOC_INC			_IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
+#endif
+
+#define DEVFS_SW_SYNC   "/dev/sw_sync"
+#define DEBUGFS_SW_SYNC "/sys/kernel/debug/sync/sw_sync"
+
+
+int sw_sync_fd_is_valid(int fd)
+{
+	int status;
+
+	if (fd < 0)
+		return 0;
+
+	status = fcntl(fd, F_GETFD, 0);
+	return status >= 0;
+}
+
+int sw_sync_timeline_create(void)
+{
+	int fd = open(DEVFS_SW_SYNC, O_RDWR);
+
+	if (fd < 0)
+		fd = open(DEBUGFS_SW_SYNC, O_RDWR);
+
+	igt_assert(sw_sync_fd_is_valid(fd));
+
+	return fd;
+}
+
+int __sw_sync_fence_create(int fd, uint32_t seqno)
+{
+
+	struct sw_sync_create_fence_data data;
+
+	memset(&data, 0, sizeof(data));
+	data.value = seqno;
+
+	if (igt_ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, &data))
+		return -errno;
+
+	return data.fence;
+}
+
+int sw_sync_fence_create(int fd, uint32_t seqno)
+{
+	int fence = __sw_sync_fence_create(fd, seqno);
+	igt_assert(fence >= 0);
+	return fence;
+}
+
+void sw_sync_timeline_inc(int fd, uint32_t count)
+{
+	uint32_t arg = count;
+
+	do_ioctl(fd, SW_SYNC_IOC_INC, &arg);
+}
+
+int sw_sync_merge(int fd1, int fd2)
+{
+	struct sync_merge_data data = {};
+	int err;
+
+	data.fd2 = fd2;
+
+	err = ioctl(fd1, SYNC_IOC_MERGE, &data);
+	if (err < 0)
+		return -errno;
+
+	sw_sync_fd_is_valid(data.fence);
+
+	return data.fence;
+}
+
+int sw_sync_wait(int fence, int timeout)
+{
+	struct pollfd pfd = { fence, POLLIN };
+
+	return poll(&pfd, 1, timeout);
+}
+
+int sw_sync_fence_count(int fd)
+{
+	struct sync_file_info info;
+
+	memset(&info, 0, sizeof(info));
+	if (ioctl(fd, SYNC_IOC_FILE_INFO, &info))
+		return -errno;
+
+	return info.num_fences;
+}
+
+static int __sw_sync_fence_count_status(int fd, int status)
+{
+	struct sync_file_info info;
+	struct sync_fence_info *fence_info;
+	int count;
+	int i;
+
+	memset(&info, 0, sizeof(info));
+	if (ioctl(fd, SYNC_IOC_FILE_INFO, &info))
+		return -errno;
+
+	fence_info = calloc(info.num_fences, sizeof(*fence_info));
+	if (!fence_info)
+		return -ENOMEM;
+
+	info.sync_fence_info = (uintptr_t)fence_info;
+	if (ioctl(fd, SYNC_IOC_FILE_INFO, &info)) {
+		count = -errno;
+	} else {
+		count = 0;
+		for (i = 0 ; i < info.num_fences ; i++)
+			count += fence_info[i].status == status;
+	}
+
+	free(fence_info);
+
+	return count;
+}
+
+int sw_sync_fence_count_status(int fd, int status)
+{
+	int count = __sw_sync_fence_count_status(fd, status);
+	igt_assert(count >= 0);
+	return count;
+}
diff --git a/lib/sw_sync.h b/lib/sw_sync.h
new file mode 100644
index 0000000..fee9249
--- /dev/null
+++ b/lib/sw_sync.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Google, Inc
+ * Copyright © 2016 Collabora, Ltd.
+ *
+ * Based on the implementation from the Android Open Source Project
+ *
+ * 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:
+ *    Robert Foss <robert.foss@collabora.com>
+ */
+
+#ifndef SW_SYNC_H
+#define SW_SYNC_H
+
+#define SW_SYNC_FENCE_STATUS_ERROR		(-1)
+#define SW_SYNC_FENCE_STATUS_ACTIVE		(0)
+#define SW_SYNC_FENCE_STATUS_SIGNALED	(1)
+
+int sw_sync_fd_is_valid(int fd);
+int sw_sync_timeline_create(void);
+int __sw_sync_fence_create(int fd, uint32_t seqno);
+int sw_sync_fence_create(int fd, uint32_t seqno);
+void sw_sync_timeline_inc(int fd, uint32_t count);
+int sw_sync_merge(int fd1, int fd2);
+int sw_sync_wait(int fence, int timeout);
+int sw_sync_fence_count(int fd);
+int sw_sync_fence_count_status(int fd, int status);
+
+#endif
+
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 02/13] tests/sw_sync: Add sw_sync test

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

Add initial tests for sw_sync.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/Makefile.sources |  1 +
 tests/sw_sync.c        | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 tests/sw_sync.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 72a58ad..0ba769f 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -125,6 +125,7 @@ TESTS_progs_M = \
 	prime_mmap_kms \
 	prime_self_import \
 	prime_vgem \
+	sw_sync \
 	template \
 	vgem_basic \
 	vgem_slow \
diff --git a/tests/sw_sync.c b/tests/sw_sync.c
new file mode 100644
index 0000000..47e2dfa
--- /dev/null
+++ b/tests/sw_sync.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2012 Google, Inc
+ * Copyright © 2016 Collabora, Ltd.
+ *
+ * Based on the implementation from the Android Open Source Project
+ *
+ * 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:
+ *    Robert Foss <robert.foss@collabora.com>
+ */
+
+#include <stdint.h>
+#include <unistd.h>
+
+#include "sw_sync.h"
+#include "igt.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION("Test SW Sync Framework");
+
+static void test_alloc_timeline(void)
+{
+	int timeline;
+
+	timeline = sw_sync_timeline_create();
+	close(timeline);
+}
+
+igt_main
+{
+	igt_subtest("alloc_timeline")
+		test_alloc_timeline();
+}
+
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 03/13] tests/sw_sync: Add subtest test_alloc_fence

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

Add subtest alloc_fence that verifies that it's possible to allocate a fence
on a timeline.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 47e2dfa..8f6208b 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -44,9 +44,25 @@ static void test_alloc_timeline(void)
 	close(timeline);
 }
 
+static void test_alloc_fence(void)
+{
+	int in_fence;
+	int timeline;
+
+	timeline = sw_sync_timeline_create();
+	in_fence = sw_sync_fence_create(timeline, 0);
+
+	close(in_fence);
+	close(timeline);
+}
+
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
 		test_alloc_timeline();
+
+	igt_subtest("alloc_fence")
+		test_alloc_fence();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 04/13] tests/sw_sync: Add subtest test_alloc_fence_invalid_timeline

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtests tests that creating fences on negative timelines fail.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 8f6208b..75bd471 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -56,6 +56,10 @@ static void test_alloc_fence(void)
 	close(timeline);
 }
 
+static void test_alloc_fence_invalid_timeline(void)
+{
+	igt_assert(__sw_sync_fence_create(-1, 0) < 0);
+}
 
 igt_main
 {
@@ -64,5 +68,8 @@ igt_main
 
 	igt_subtest("alloc_fence")
 		test_alloc_fence();
+
+	igt_subtest("alloc_fence_invalid_timeline")
+		test_alloc_fence_invalid_timeline();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 05/13] tests/sw_sync: Add subtest test_alloc_merge_fence

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies that merging two fences works in the simples possible
case.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 75bd471..fcb2f57 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -61,6 +61,26 @@ static void test_alloc_fence_invalid_timeline(void)
 	igt_assert(__sw_sync_fence_create(-1, 0) < 0);
 }
 
+static void test_alloc_merge_fence(void)
+{
+	int in_fence[2];
+	int fence_merge;
+	int timeline[2];
+
+	timeline[0] = sw_sync_timeline_create();
+	timeline[1] = sw_sync_timeline_create();
+
+	in_fence[0] = sw_sync_fence_create(timeline[0], 1);
+	in_fence[1] = sw_sync_fence_create(timeline[1], 1);
+	fence_merge = sw_sync_merge(in_fence[1], in_fence[0]);
+
+	close(in_fence[0]);
+	close(in_fence[1]);
+	close(fence_merge);
+	close(timeline[0]);
+	close(timeline[1]);
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -71,5 +91,8 @@ igt_main
 
 	igt_subtest("alloc_fence_invalid_timeline")
 		test_alloc_fence_invalid_timeline();
+
+	igt_subtest("alloc_merge_fence")
+		test_alloc_merge_fence();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 06/13] tests/sw_sync: Add subtest test_sync_wait

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies that waiting on fences works properly.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index fcb2f57..3061279 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -81,6 +81,41 @@ static void test_alloc_merge_fence(void)
 	close(timeline[1]);
 }
 
+static void test_sync_wait(void)
+{
+	int fence, ret;
+	int timeline;
+
+	timeline = sw_sync_timeline_create();
+	fence = sw_sync_fence_create(timeline, 5);
+
+	/* Wait on fence until timeout */
+	ret = sw_sync_wait(fence, 0);
+	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");
+
+	/* Advance timeline from 0 -> 1 */
+	sw_sync_timeline_inc(timeline, 1);
+
+	/* Wait on fence until timeout */
+	ret = sw_sync_wait(fence, 0);
+	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");
+
+	/* Signal the fence */
+	sw_sync_timeline_inc(timeline, 4);
+
+	/* Wait successfully */
+	ret = sw_sync_wait(fence, 0);
+	igt_assert_f(ret > 0, "Failure waiting on fence\n");
+
+	/* Go even further, and confirm wait still succeeds */
+	sw_sync_timeline_inc(timeline, 10);
+	ret = sw_sync_wait(fence, 0);
+	igt_assert_f(ret > 0, "Failure waiting ahead\n");
+
+	close(fence);
+	close(timeline);
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -94,5 +129,8 @@ igt_main
 
 	igt_subtest("alloc_merge_fence")
 		test_alloc_merge_fence();
+
+	igt_subtest("sync_wait")
+		test_sync_wait();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 07/13] tests/sw_sync: Add subtest test_sync_merge

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

Add subtest test_sync_merge that tests merging fences and the validity of the
resulting merged fence.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 3061279..26226bd 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -116,6 +116,70 @@ static void test_sync_wait(void)
 	close(timeline);
 }
 
+static void test_sync_merge(void)
+{
+	int in_fence[3];
+	int fence_merge;
+	int timeline;
+	int active, signaled;
+
+	timeline = sw_sync_timeline_create();
+	in_fence[0] = sw_sync_fence_create(timeline, 1);
+	in_fence[1] = sw_sync_fence_create(timeline, 2);
+	in_fence[2] = sw_sync_fence_create(timeline, 3);
+
+	fence_merge = sw_sync_merge(in_fence[0], in_fence[1]);
+	fence_merge = sw_sync_merge(in_fence[2], fence_merge);
+
+	/* confirm all fences have one active point (even d) */
+	active = sw_sync_fence_count_status(in_fence[0],
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 1, "in_fence[0] has too many active fences\n");
+	active = sw_sync_fence_count_status(in_fence[1],
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 1, "in_fence[1] has too many active fences\n");
+	active = sw_sync_fence_count_status(in_fence[2],
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 1, "in_fence[2] has too many active fences\n");
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 1, "fence_merge has too many active fences\n");
+
+	/* confirm that fence_merge is not signaled until the max of fence 0,1,2 */
+	sw_sync_timeline_inc(timeline, 1);
+	signaled = sw_sync_fence_count_status(in_fence[0],
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(signaled == 1, "in_fence[0] did not signal\n");
+	igt_assert_f(active == 1, "fence_merge signaled too early\n");
+
+	sw_sync_timeline_inc(timeline, 1);
+	signaled = sw_sync_fence_count_status(in_fence[1],
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(signaled == 1, "in_fence[1] did not signal\n");
+	igt_assert_f(active == 1, "fence_merge signaled too early\n");
+
+	sw_sync_timeline_inc(timeline, 1);
+	signaled = sw_sync_fence_count_status(in_fence[2],
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(signaled == 1, "in_fence[2] did not signal\n");
+	signaled = sw_sync_fence_count_status(fence_merge,
+					       SW_SYNC_FENCE_STATUS_SIGNALED);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 0 && signaled == 1,
+		     "fence_merge did not signal\n");
+
+	sw_sync_fence_destroy(in_fence[0]);
+	sw_sync_fence_destroy(in_fence[1]);
+	sw_sync_fence_destroy(in_fence[2]);
+	sw_sync_fence_destroy(fence_merge);
+	sw_sync_timeline_destroy(timeline);
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -132,5 +196,8 @@ igt_main
 
 	igt_subtest("sync_wait")
 		test_sync_wait();
+
+	igt_subtest("sync_merge")
+		test_sync_merge();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 08/13] tests/sw_sync: Add subtest test_sync_merge_same

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies merging a fence with itself does not fail.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 26226bd..ff87cfe 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -173,11 +173,35 @@ static void test_sync_merge(void)
 	igt_assert_f(active == 0 && signaled == 1,
 		     "fence_merge did not signal\n");
 
-	sw_sync_fence_destroy(in_fence[0]);
-	sw_sync_fence_destroy(in_fence[1]);
-	sw_sync_fence_destroy(in_fence[2]);
-	sw_sync_fence_destroy(fence_merge);
-	sw_sync_timeline_destroy(timeline);
+	close(in_fence[0]);
+	close(in_fence[1]);
+	close(in_fence[2]);
+	close(fence_merge);
+	close(timeline);
+}
+
+static void test_sync_merge_same(void)
+{
+	int in_fence[2];
+	int timeline;
+	int signaled;
+
+	timeline = sw_sync_timeline_create();
+	in_fence[0] = sw_sync_fence_create(timeline, 1);
+	in_fence[1] = sw_sync_merge(in_fence[0], in_fence[0]);
+
+	signaled = sw_sync_fence_count_status(in_fence[0],
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(signaled == 0, "fence signaled too early\n");
+
+	sw_sync_timeline_inc(timeline, 1);
+	signaled = sw_sync_fence_count_status(in_fence[0],
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(signaled == 1, "fence did not signal\n");
+
+	close(in_fence[0]);
+	close(in_fence[1]);
+	close(timeline);
 }
 
 igt_main
@@ -199,5 +223,8 @@ igt_main
 
 	igt_subtest("sync_merge")
 		test_sync_merge();
+
+	igt_subtest("sync_merge_same")
+		test_sync_merge_same();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 09/13] tests/sw_sync: Add subtest test_sync_multi_consumer

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies the access ordering of multiple consumer threads.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index ff87cfe..e607b75 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -27,6 +27,8 @@
  *    Robert Foss <robert.foss@collabora.com>
  */
 
+#include <pthread.h>
+#include <semaphore.h>
 #include <stdint.h>
 #include <unistd.h>
 
@@ -36,6 +38,15 @@
 
 IGT_TEST_DESCRIPTION("Test SW Sync Framework");
 
+typedef struct {
+	int timeline;
+	uint32_t thread_id;
+	uint32_t nbr_threads;
+	uint32_t nbr_iterations;
+	volatile uint32_t * volatile counter;
+	sem_t *sem;
+} data_t;
+
 static void test_alloc_timeline(void)
 {
 	int timeline;
@@ -204,6 +215,95 @@ static void test_sync_merge_same(void)
 	close(timeline);
 }
 
+static void * test_sync_multi_consumer_thread(void *arg)
+{
+	data_t *data = arg;
+	int thread_id = data->thread_id;
+	int nbr_threads = data->nbr_threads;
+	int timeline = data->timeline;
+	int iterations = data->nbr_iterations;
+	int ret, i;
+
+	for (i = 0; i < iterations; i++) {
+		int next_point = i * nbr_threads + thread_id;
+		int fence = sw_sync_fence_create(timeline, next_point);
+
+		ret = sw_sync_wait(fence, 1000);
+		if (ret <= 0)
+		{
+			return (void *) 1;
+		}
+
+		if (*(data->counter) != next_point)
+		{
+			return (void *) 1;
+		}
+
+		sem_post(data->sem);
+		close(fence);
+	}
+	return NULL;
+}
+
+static void test_sync_multi_consumer(void)
+{
+	const uint32_t nbr_threads = 8;
+	const uint32_t nbr_iterations = 1 << 14;
+	data_t data_arr[nbr_threads];
+	pthread_t thread_arr[nbr_threads];
+	sem_t sem;
+	int timeline;
+	volatile uint32_t counter = 0;
+	uintptr_t thread_ret = 0;
+	data_t data;
+	int i, ret;
+
+	sem_init(&sem, 0, 0);
+	timeline = sw_sync_timeline_create();
+
+	data.nbr_iterations = nbr_iterations;
+	data.nbr_threads = nbr_threads;
+	data.counter = &counter;
+	data.timeline = timeline;
+	data.sem = &sem;
+
+	/* Start sync threads. */
+	for (i = 0; i < nbr_threads; i++)
+	{
+		data_arr[i] = data;
+		data_arr[i].thread_id = i;
+		ret = pthread_create(&thread_arr[i], NULL,
+				     test_sync_multi_consumer_thread,
+				     (void *) &(data_arr[i]));
+		igt_assert_eq(ret, 0);
+	}
+
+	/* Produce 'content'. */
+	for (i = 0; i < nbr_threads * nbr_iterations; i++)
+	{
+		sem_wait(&sem);
+
+		counter++;
+		sw_sync_timeline_inc(timeline, 1);
+	}
+
+	/* Wait for threads to complete. */
+	for (i = 0; i < nbr_threads; i++)
+	{
+		uintptr_t local_thread_ret;
+		pthread_join(thread_arr[i], (void **)&local_thread_ret);
+		thread_ret |= local_thread_ret;
+	}
+
+	close(timeline);
+	sem_destroy(&sem);
+
+	igt_assert_f(counter == nbr_threads * nbr_iterations,
+		     "Counter has unexpected value.\n");
+
+	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -226,5 +326,8 @@ igt_main
 
 	igt_subtest("sync_merge_same")
 		test_sync_merge_same();
+
+	igt_subtest("sync_multi_consumer")
+		test_sync_multi_consumer();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 10/13] tests/sw_sync: Add subtest test_sync_multi_consumer_producer

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This test verifies that stressing the kernel by creating multiple
consumer/producer threads that wait on a single timeline to be incremented
by another conumer/producer thread does not fail.
And that the order amongst the threads is maintained.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index e607b75..13a5643 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -304,6 +304,86 @@ static void test_sync_multi_consumer(void)
 	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
 }
 
+static void * test_sync_multi_consumer_producer_thread(void *arg)
+{
+	data_t *data = arg;
+	int thread_id = data->thread_id;
+	int nbr_threads = data->nbr_threads;
+	int timeline = data->timeline;
+	int iterations = data->nbr_iterations;
+	int ret, i;
+
+	for (i = 0; i < iterations; i++) {
+		int next_point = i * nbr_threads + thread_id;
+		int fence = sw_sync_fence_create(timeline, next_point);
+
+		ret = sw_sync_wait(fence, 1000);
+		if (ret <= 0)
+		{
+			return (void *) 1;
+		}
+
+		if (*(data->counter) != next_point)
+		{
+			return (void *) 1;
+		}
+
+		(*data->counter)++;
+
+		/* Kick off the next thread. */
+		sw_sync_timeline_inc(timeline, 1);
+
+		close(fence);
+	}
+	return NULL;
+}
+
+static void test_sync_multi_consumer_producer(void)
+{
+	const uint32_t nbr_threads = 8;
+	const uint32_t nbr_iterations = 1 << 14;
+	data_t data_arr[nbr_threads];
+	pthread_t thread_arr[nbr_threads];
+	int timeline;
+	volatile uint32_t counter = 0;
+	uintptr_t thread_ret = 0;
+	data_t data;
+	int i, ret;
+
+	timeline = sw_sync_timeline_create();
+
+	data.nbr_iterations = nbr_iterations;
+	data.nbr_threads = nbr_threads;
+	data.counter = &counter;
+	data.timeline = timeline;
+
+	/* Start consumer threads. */
+	for (i = 0; i < nbr_threads; i++)
+	{
+		data_arr[i] = data;
+		data_arr[i].thread_id = i;
+		ret = pthread_create(&thread_arr[i], NULL,
+				     test_sync_multi_consumer_producer_thread,
+				     (void *) &(data_arr[i]));
+		igt_assert_eq(ret, 0);
+	}
+
+	/* Wait for threads to complete. */
+	for (i = 0; i < nbr_threads; i++)
+	{
+		uintptr_t local_thread_ret;
+		pthread_join(thread_arr[i], (void **)&local_thread_ret);
+		thread_ret |= local_thread_ret;
+	}
+
+	close(timeline);
+
+	igt_assert_f(counter == nbr_threads * nbr_iterations,
+		     "Counter has unexpected value.\n");
+
+	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -329,5 +409,8 @@ igt_main
 
 	igt_subtest("sync_multi_consumer")
 		test_sync_multi_consumer();
+
+	igt_subtest("sync_multi_consumer_producer")
+		test_sync_multi_consumer_producer();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 11/13] tests/sw_sync: Add subtest test_sync_random_merge

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies that creating many timelines and merging random fences
from each timeline with eachother results in merged fences that are fully
functional.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 13a5643..bd86111 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -384,6 +384,76 @@ static void test_sync_multi_consumer_producer(void)
 	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
 }
 
+static void test_sync_random_merge(void)
+{
+	int i, size, ret;
+	const int nbr_timeline = 32;
+	const int nbr_merge = 1024;
+	int fence_map[nbr_timeline];
+	int timeline_arr[nbr_timeline];
+	int fence, tmpfence, merged;
+	int timeline, timeline_offset, sync_pt;
+
+	srand(time(NULL));
+
+	for (i = 0; i < nbr_timeline; i++) {
+		timeline_arr[i] = sw_sync_timeline_create();
+		fence_map[i] = -1;
+	}
+
+	sync_pt = rand();
+	fence = sw_sync_fence_create(timeline_arr[0], sync_pt);
+
+	fence_map[0] = sync_pt;
+
+	/* Randomly create syncpoints out of a fixed set of timelines,
+	 * and merge them together.
+	 */
+	for (i = 0; i < nbr_merge; i++) {
+		/* Generate syncpoint. */
+		timeline_offset = rand() % nbr_timeline;
+		timeline = timeline_arr[timeline_offset];
+		sync_pt = rand();
+
+		/* Keep track of the latest sync_pt in each timeline. */
+		if (fence_map[timeline_offset] == -1)
+			fence_map[timeline_offset] = sync_pt;
+		else if (fence_map[timeline_offset] < sync_pt)
+			fence_map[timeline_offset] = sync_pt;
+
+		/* Merge. */
+		tmpfence = sw_sync_fence_create(timeline, sync_pt);
+		merged = sw_sync_merge(tmpfence, fence);
+		close(tmpfence);
+		close(fence);
+		fence = merged;
+	}
+
+	size = 0;
+	for (i = 0; i < nbr_timeline; i++)
+		if (fence_map[i] != -1)
+			size++;
+
+	/* Trigger the merged fence. */
+	for (i = 0; i < nbr_timeline; i++) {
+		if (fence_map[i] != -1) {
+			ret = sw_sync_wait(fence, 0);
+			igt_assert_f(ret == 0,
+				    "Failure waiting on fence until timeout\n");
+			/* Increment the timeline to the last sync_pt */
+			sw_sync_timeline_inc(timeline_arr[i], fence_map[i]);
+		}
+	}
+
+	/* Check that the fence is triggered. */
+	ret = sw_sync_wait(fence, 0);
+	igt_assert_f(ret > 0, "Failure triggering fence\n");
+
+	close(fence);
+	for (i = 0; i < nbr_timeline; i++)
+		close(timeline_arr[i]);
+}
+
 igt_main
 {
 	igt_subtest("alloc_timeline")
@@ -412,5 +482,8 @@ igt_main
 
 	igt_subtest("sync_multi_consumer_producer")
 		test_sync_multi_consumer_producer();
+
+	igt_subtest("sync_random_merge")
+		test_sync_random_merge();
 }
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 12/13] tests/sw_sync: Add subtest test_sync_multi_timeline_wait

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest verifies that waiting, timing out on a wait and that counting
fences in various states works.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index bd86111..9ec2dc2 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -215,6 +215,69 @@ static void test_sync_merge_same(void)
 	close(timeline);
 }
 
+static void test_sync_multi_timeline_wait(void)
+{
+	int timeline[3];
+	int in_fence[3];
+	int fence_merge;
+	int active, signaled, ret;
+
+	timeline[0] = sw_sync_timeline_create();
+	timeline[1] = sw_sync_timeline_create();
+	timeline[2] = sw_sync_timeline_create();
+
+	in_fence[0] = sw_sync_fence_create(timeline[0], 5);
+	in_fence[1] = sw_sync_fence_create(timeline[1], 5);
+	in_fence[2] = sw_sync_fence_create(timeline[2], 5);
+
+	fence_merge = sw_sync_merge(in_fence[0], in_fence[1]);
+	fence_merge = sw_sync_merge(in_fence[2], fence_merge);
+
+	/* Confirm fence isn't signaled */
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	igt_assert_f(active == 3, "Fence signaled too early\n");
+
+	ret = sw_sync_wait(fence_merge, 0);
+	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");
+
+	sw_sync_timeline_inc(timeline[0], 5);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	signaled = sw_sync_fence_count_status(fence_merge,
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(active == 2 && signaled == 1,
+		    "Fence did not signal properly\n");
+
+	sw_sync_timeline_inc(timeline[1], 5);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	signaled = sw_sync_fence_count_status(fence_merge,
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(active == 1 && signaled == 2,
+		    "Fence did not signal properly\n");
+
+	sw_sync_timeline_inc(timeline[2], 5);
+	active = sw_sync_fence_count_status(fence_merge,
+					    SW_SYNC_FENCE_STATUS_ACTIVE);
+	signaled = sw_sync_fence_count_status(fence_merge,
+					      SW_SYNC_FENCE_STATUS_SIGNALED);
+	igt_assert_f(active == 0 && signaled == 3,
+		     "Fence did not signal properly\n");
+
+	/* confirm you can successfully wait */
+	ret = sw_sync_wait(fence_merge, 100);
+	igt_assert_f(ret > 0, "Failure waiting on signaled fence\n");
+
+	close(in_fence[0]);
+	close(in_fence[1]);
+	close(in_fence[2]);
+	close(fence_merge);
+	close(timeline[0]);
+	close(timeline[1]);
+	close(timeline[2]);
+}
+
 static void * test_sync_multi_consumer_thread(void *arg)
 {
 	data_t *data = arg;
@@ -477,6 +540,9 @@ igt_main
 	igt_subtest("sync_merge_same")
 		test_sync_merge_same();
 
+	igt_subtest("sync_multi_timeline_wait")
+		test_sync_multi_timeline_wait();
+
 	igt_subtest("sync_multi_consumer")
 		test_sync_multi_consumer();
 
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v5 13/13] tests/sw_sync: Add subtest test_sync_multi_producer_single_consumer

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This subtest runs a single consumer thread and multiple producer thread that
are synchronized using multiple timelines.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
Reviewed-by: Eric Engestrom <eric@engestrom.ch>
---
 tests/sw_sync.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 9ec2dc2..1af9a6b 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -447,6 +447,142 @@ static void test_sync_multi_consumer_producer(void)
 	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
 }
 
+static int test_mspc_wait_on_fence(int fence)
+{
+	int error, active;
+
+	do {
+		error = sw_sync_fence_count_status(fence,
+						   SW_SYNC_FENCE_STATUS_ERROR);
+		igt_assert_f(error == 0, "Error occurred on fence\n");
+		active = sw_sync_fence_count_status(fence,
+						    SW_SYNC_FENCE_STATUS_ACTIVE);
+	} while (active);
+
+	return 0;
+}
+
+static struct {
+	int iterations;
+	int threads;
+	int counter;
+	int cons_timeline;
+	int *prod_timeline;
+	pthread_mutex_t lock;
+} test_mpsc_data;
+
+static int mpsc_producer_thread(void *d)
+{
+	int id = (long)d;
+	int fence, i;
+	int *prod_timeline = test_mpsc_data.prod_timeline;
+	int cons_timeline = test_mpsc_data.cons_timeline;
+	int iterations = test_mpsc_data.iterations;
+
+	for (i = 0; i < iterations; i++) {
+		fence = sw_sync_fence_create(cons_timeline, i);
+
+		/* Wait for the consumer to finish. Use alternate
+		 * means of waiting on the fence
+		 */
+		if ((iterations + id) % 8 != 0) {
+			igt_assert_f(sw_sync_wait(fence, -1) > 0,
+				     "Failure waiting on fence\n");
+		} else {
+			igt_assert_f(test_mspc_wait_on_fence(fence) == 0,
+				     "Failure waiting on fence\n");
+		}
+
+		/* Every producer increments the counter, the consumer
+		 * checks and erases it
+		 */
+		pthread_mutex_lock(&test_mpsc_data.lock);
+		test_mpsc_data.counter++;
+		pthread_mutex_unlock(&test_mpsc_data.lock);
+
+		sw_sync_timeline_inc(prod_timeline[id], 1);
+		close(fence);
+	}
+
+	return 0;
+}
+
+static int mpsc_consumer_thread(void)
+{
+	int fence, merged, tmp, it, i;
+	int *prod_timeline = test_mpsc_data.prod_timeline;
+	int cons_timeline = test_mpsc_data.cons_timeline;
+	int iterations = test_mpsc_data.iterations;
+	int n = test_mpsc_data.threads;
+
+	for (it = 1; it <= iterations; it++) {
+		fence = sw_sync_fence_create(prod_timeline[0], it);
+		for (i = 1; i < n; i++) {
+			tmp = sw_sync_fence_create(prod_timeline[i], it);
+			merged = sw_sync_merge(tmp, fence);
+			close(tmp);
+			close(fence);
+			fence = merged;
+		}
+
+		/* Make sure we see an increment from every producer thread.
+		 * Vary the means by which we wait.
+		 */
+		if (iterations % 8 != 0) {
+			igt_assert_f(sw_sync_wait(fence, -1) == 0,
+				    "Producers did not increment as expected\n");
+		} else {
+			igt_assert_f(test_mspc_wait_on_fence(fence) == 0,
+				     "Failure waiting on fence\n");
+		}
+
+		igt_assert_f(test_mpsc_data.counter == n * it,
+			     "Counter value mismatch\n");
+
+		/* Release the producer threads */
+		sw_sync_timeline_inc(cons_timeline, 1);
+		close(fence);
+	}
+
+	return 0;
+}
+
+/* IMPORTANT NOTE: if you see this test failing on your system, it may be
+ * due to a shortage of file descriptors. Please ensure your system has
+ * a sensible limit for this test to finish correctly.
+ */
+static void test_sync_multi_producer_single_consumer(void)
+{
+	int iterations = 1 << 12;
+	int n = 5;
+	int prod_timeline[n];
+	int cons_timeline;
+	pthread_t threads[n];
+	long i;
+
+	cons_timeline = sw_sync_timeline_create();
+	for (i = 0; i < n; i++)
+		prod_timeline[i] = sw_sync_timeline_create();
+
+	test_mpsc_data.prod_timeline = prod_timeline;
+	test_mpsc_data.cons_timeline = cons_timeline;
+	test_mpsc_data.iterations = iterations;
+	test_mpsc_data.threads = n;
+	test_mpsc_data.counter = 0;
+	pthread_mutex_init(&test_mpsc_data.lock, NULL);
+
+	for (i = 0; i < n; i++) {
+		pthread_create(&threads[i], NULL, (void * (*)(void *))
+			       mpsc_producer_thread,
+			       (void *)i);
+	}
+
+	mpsc_consumer_thread();
+
+	for (i = 0; i < n; i++)
+		pthread_join(threads[i], NULL);
+}
+
 static void test_sync_random_merge(void)
 {
 	int i, size, ret;
@@ -549,6 +685,9 @@ igt_main
 	igt_subtest("sync_multi_consumer_producer")
 		test_sync_multi_consumer_producer();
 
+	igt_subtest("sync_multi_producer_single_consumer")
+		test_sync_multi_producer_single_consumer();
+
 	igt_subtest("sync_random_merge")
 		test_sync_random_merge();
 }
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 01/13] lib/sw_sync: Add helper functions for managing synchronization primitives

[Chris Wilson]

On Thu, Sep 15, 2016 at 02:40:06PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> Base functions to help testing the Sync File Framework (explicit fencing
> mechanism ported from Android).
> These functions allow you to create, use and destroy timelines and fences.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>

Some minor cosmetics remain but well below the level of caring,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 02/13] tests/sw_sync: Add sw_sync test

[Chris Wilson]

On Thu, Sep 15, 2016 at 02:40:07PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> Add initial tests for sw_sync.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> ---
>  tests/Makefile.sources |  1 +
>  tests/sw_sync.c        | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
>  create mode 100644 tests/sw_sync.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 72a58ad..0ba769f 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -125,6 +125,7 @@ TESTS_progs_M = \
>  	prime_mmap_kms \
>  	prime_self_import \
>  	prime_vgem \
> +	sw_sync \
>  	template \
>  	vgem_basic \
>  	vgem_slow \
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> new file mode 100644
> index 0000000..47e2dfa
> --- /dev/null
> +++ b/tests/sw_sync.c
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright 2012 Google, Inc
> + * Copyright © 2016 Collabora, Ltd.
> + *
> + * Based on the implementation from the Android Open Source Project
> + *
> + * 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:
> + *    Robert Foss <robert.foss@collabora.com>
> + */
> +
> +#include <stdint.h>
> +#include <unistd.h>
> +
> +#include "sw_sync.h"
> +#include "igt.h"
> +#include "igt_aux.h"
> +
> +IGT_TEST_DESCRIPTION("Test SW Sync Framework");
> +
> +static void test_alloc_timeline(void)
> +{
> +	int timeline;
> +
> +	timeline = sw_sync_timeline_create();
> +	close(timeline);
> +}
> +
> +igt_main
> +{

If I were to run this on a kernel without sw_sync support, it would
fail. We want it to skip instead!

in lib/sw_sync.c:

static bool kernel_has_sw_sync(void)
{
	bool err;

 	igt_ignore_warn(system("/sbin/modprobe -s r sw_sync"));

	err = false;
	if (access("/dev/sw_sync", R_OK | W_OK) < 0) {
		char buf[128];

		snprintf(buf, sizeof(buf), "%s/sw_sync", igt_debugfs_mount());
		err = access("/sys/kernel//sw_sync", R_OK | W_OK) < 0;
	}

	return !err;
}

void igt_require_sw_sync(void)
{
	igt_require(kernel_has_sw_sync());
	
}

then here:

igt_main
{

	igt_fixture {
		igt_require_sw_sync();
	}

> +	igt_subtest("alloc_timeline")
> +		test_alloc_timeline();
> +}

}

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 06/13] tests/sw_sync: Add subtest test_sync_wait

[Chris Wilson]

On Thu, Sep 15, 2016 at 02:40:11PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> This subtest verifies that waiting on fences works properly.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> ---
>  tests/sw_sync.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index fcb2f57..3061279 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -81,6 +81,41 @@ static void test_alloc_merge_fence(void)
>  	close(timeline[1]);
>  }
>  
> +static void test_sync_wait(void)

These are not testing waits but busy queries.

static void test_sync_busy(void)
> +{
> +	int fence, ret;
> +	int timeline;
> +
> +	timeline = sw_sync_timeline_create();
> +	fence = sw_sync_fence_create(timeline, 5);
> +
> +	/* Wait on fence until timeout */

Misleading comment

> +	ret = sw_sync_wait(fence, 0);
> +	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");

igt_assert_f(ret == 0, "Fence created in an unexpectedly signaled state\n");

> +
> +	/* Advance timeline from 0 -> 1 */
> +	sw_sync_timeline_inc(timeline, 1);
> +
> +	/* Wait on fence until timeout */
> +	ret = sw_sync_wait(fence, 0);
> +	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");

igt_assert_f(ret == 0, "Fence signaled earlier (timeline value 1, fence seqno 5)\n");

> +
> +	/* Signal the fence */

/* Advance timeline from 1 -> 5: signaling the fence (seqno 5)*/
> +	sw_sync_timeline_inc(timeline, 4);

> +
> +	/* Wait successfully */

Usless comment

> +	ret = sw_sync_wait(fence, 0);
> +	igt_assert_f(ret > 0, "Failure waiting on fence\n");

igt_assert_f(ret == 0, "Fence not signaled (timeline value 5, fence seqno 5)\n");

If we have a timeline info, we could use %d to say the current value.

Another test would be to then

seqno = 0;
for (i = 0; i < n_primes; i++) {
	seqno += primes[i];
	sw_sync_timeline_inc(timeline, primes[i]);
	igt_assert_eq(sw_sync_timeline_get_seqno(timeline), seqno);
}

> +
> +	/* Go even further, and confirm wait still succeeds */
> +	sw_sync_timeline_inc(timeline, 10);
> +	ret = sw_sync_wait(fence, 0);
> +	igt_assert_f(ret > 0, "Failure waiting ahead\n");

igt_assert_f(ret == 0, "Fence now not signaled! (timeline value 10, fence seqno 5)\n");

> +
> +	close(fence);
> +	close(timeline);
> +}

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 07/13] tests/sw_sync: Add subtest test_sync_merge

[Chris Wilson]

On Thu, Sep 15, 2016 at 02:40:12PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> Add subtest test_sync_merge that tests merging fences and the validity of the
> resulting merged fence.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> ---
>  tests/sw_sync.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index 3061279..26226bd 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -116,6 +116,70 @@ static void test_sync_wait(void)
>  	close(timeline);
>  }
>  
> +static void test_sync_merge(void)
> +{
> +	int in_fence[3];
> +	int fence_merge;
> +	int timeline;
> +	int active, signaled;
> +
> +	timeline = sw_sync_timeline_create();
> +	in_fence[0] = sw_sync_fence_create(timeline, 1);
> +	in_fence[1] = sw_sync_fence_create(timeline, 2);
> +	in_fence[2] = sw_sync_fence_create(timeline, 3);
> +
> +	fence_merge = sw_sync_merge(in_fence[0], in_fence[1]);
> +	fence_merge = sw_sync_merge(in_fence[2], fence_merge);

sw_sync_merge() really does need the negative tests:

invalid fd (-1),
device fd (/dev/dri/card0),
file fd.

should cover the most common errors (fuzz testing will hit the rest!)

> +
> +	/* confirm all fences have one active point (even d) */
> +	active = sw_sync_fence_count_status(in_fence[0],
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(active == 1, "in_fence[0] has too many active fences\n");
> +	active = sw_sync_fence_count_status(in_fence[1],
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(active == 1, "in_fence[1] has too many active fences\n");
> +	active = sw_sync_fence_count_status(in_fence[2],
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(active == 1, "in_fence[2] has too many active fences\n");
> +	active = sw_sync_fence_count_status(fence_merge,
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(active == 1, "fence_merge has too many active fences\n");
> +
> +	/* confirm that fence_merge is not signaled until the max of fence 0,1,2 */
> +	sw_sync_timeline_inc(timeline, 1);

> +	signaled = sw_sync_fence_count_status(in_fence[0],
> +					      SW_SYNC_FENCE_STATUS_SIGNALED);

This is missing from the earlier test_sync_busy().

> +	active = sw_sync_fence_count_status(fence_merge,
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(signaled == 1, "in_fence[0] did not signal\n");
> +	igt_assert_f(active == 1, "fence_merge signaled too early\n");
> +
> +	sw_sync_timeline_inc(timeline, 1);
> +	signaled = sw_sync_fence_count_status(in_fence[1],
> +					      SW_SYNC_FENCE_STATUS_SIGNALED);
> +	active = sw_sync_fence_count_status(fence_merge,
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(signaled == 1, "in_fence[1] did not signal\n");
> +	igt_assert_f(active == 1, "fence_merge signaled too early\n");
> +
> +	sw_sync_timeline_inc(timeline, 1);
> +	signaled = sw_sync_fence_count_status(in_fence[2],
> +					      SW_SYNC_FENCE_STATUS_SIGNALED);
> +	igt_assert_f(signaled == 1, "in_fence[2] did not signal\n");
> +	signaled = sw_sync_fence_count_status(fence_merge,
> +					       SW_SYNC_FENCE_STATUS_SIGNALED);
> +	active = sw_sync_fence_count_status(fence_merge,
> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
> +	igt_assert_f(active == 0 && signaled == 1,
> +		     "fence_merge did not signal\n");

Hmm, counting STATUS_SIGNALED / STATUS_ACTIVE is not behaving how I
would intuitively expect.

At this point, timeline.seqno = 3, I would expect count_signaled(merge)
== 3 (and count_fences(merge) == 3 => merge is now signaled).
But that's just my expectations
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 06/13] tests/sw_sync: Add subtest test_sync_wait

[Chris Wilson]

On Thu, Sep 15, 2016 at 02:40:11PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> This subtest verifies that waiting on fences works properly.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> ---
>  tests/sw_sync.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index fcb2f57..3061279 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -81,6 +81,41 @@ static void test_alloc_merge_fence(void)
>  	close(timeline[1]);
>  }
>  
> +static void test_sync_wait(void)
> +{

Ah, you don't have a real test_sync_wait()

Nor do you check that fences are inherited across fork() or can be sent
over unix sockets.

For test_sync_wait(), some along the lines of create timeline in parent,
fork + create fence in child (wakeup parent) wait(fence), .timeout =
10s), in parent sleep another 1s, increment timeline. Usual assertions.

If the fence was created in the parent and forked to the child, the
parent can assert the fence was signaled to ensure that we expect the
wait to succeed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 02/13] tests/sw_sync: Add sw_sync test

[Robert Foss]

On 2016-09-15 04:22 PM, Chris Wilson wrote:
> On Thu, Sep 15, 2016 at 02:40:07PM -0400, robert.foss@collabora.com wrote:
>> From: Robert Foss <robert.foss@collabora.com>
>>
>> Add initial tests for sw_sync.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>> ---
>>  tests/Makefile.sources |  1 +
>>  tests/sw_sync.c        | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 53 insertions(+)
>>  create mode 100644 tests/sw_sync.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 72a58ad..0ba769f 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -125,6 +125,7 @@ TESTS_progs_M = \
>>  	prime_mmap_kms \
>>  	prime_self_import \
>>  	prime_vgem \
>> +	sw_sync \
>>  	template \
>>  	vgem_basic \
>>  	vgem_slow \
>> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
>> new file mode 100644
>> index 0000000..47e2dfa
>> --- /dev/null
>> +++ b/tests/sw_sync.c
>> @@ -0,0 +1,52 @@
>> +/*
>> + * Copyright 2012 Google, Inc
>> + * Copyright © 2016 Collabora, Ltd.
>> + *
>> + * Based on the implementation from the Android Open Source Project
>> + *
>> + * 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:
>> + *    Robert Foss <robert.foss@collabora.com>
>> + */
>> +
>> +#include <stdint.h>
>> +#include <unistd.h>
>> +
>> +#include "sw_sync.h"
>> +#include "igt.h"
>> +#include "igt_aux.h"
>> +
>> +IGT_TEST_DESCRIPTION("Test SW Sync Framework");
>> +
>> +static void test_alloc_timeline(void)
>> +{
>> +	int timeline;
>> +
>> +	timeline = sw_sync_timeline_create();
>> +	close(timeline);
>> +}
>> +
>> +igt_main
>> +{
>
> If I were to run this on a kernel without sw_sync support, it would
> fail. We want it to skip instead!
>
> in lib/sw_sync.c:
>
> static bool kernel_has_sw_sync(void)
> {
> 	bool err;
>
>  	igt_ignore_warn(system("/sbin/modprobe -s r sw_sync"));
>
> 	err = false;
> 	if (access("/dev/sw_sync", R_OK | W_OK) < 0) {
> 		char buf[128];
>
> 		snprintf(buf, sizeof(buf), "%s/sw_sync", igt_debugfs_mount());
> 		err = access("/sys/kernel//sw_sync", R_OK | W_OK) < 0;
> 	}
>
> 	return !err;
> }
>
> void igt_require_sw_sync(void)
> {
> 	igt_require(kernel_has_sw_sync());
> 	
> }
>
> then here:
>
> igt_main
> {
>
> 	igt_fixture {
> 		igt_require_sw_sync();
> 	}
>
>> +	igt_subtest("alloc_timeline")
>> +		test_alloc_timeline();
>> +}
>
> }


This is a solid improvement, igt_debugfs_mount() does not however seem 
to be available on upstream/master.
I've found your patch that includes it amongst other things, would you 
like me to extract it and include it into this series?

Rob.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 06/13] tests/sw_sync: Add subtest test_sync_wait

[Robert Foss]

On 2016-09-15 04:32 PM, Chris Wilson wrote:
> On Thu, Sep 15, 2016 at 02:40:11PM -0400, robert.foss@collabora.com wrote:
>> From: Robert Foss <robert.foss@collabora.com>
>>
>> This subtest verifies that waiting on fences works properly.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>> ---
>>  tests/sw_sync.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
>> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
>> index fcb2f57..3061279 100644
>> --- a/tests/sw_sync.c
>> +++ b/tests/sw_sync.c
>> @@ -81,6 +81,41 @@ static void test_alloc_merge_fence(void)
>>  	close(timeline[1]);
>>  }
>>
>> +static void test_sync_wait(void)
>
> These are not testing waits but busy queries.

test_sync_wait refers to sw_sync_wait, which may or may not be 
meaningful to refer to.
Do you still prefer test_sync_busy?

>
> static void test_sync_busy(void)
>> +{
>> +	int fence, ret;
>> +	int timeline;
>> +
>> +	timeline = sw_sync_timeline_create();
>> +	fence = sw_sync_fence_create(timeline, 5);
>> +
>> +	/* Wait on fence until timeout */
>
> Misleading comment

Agreed.

>
>> +	ret = sw_sync_wait(fence, 0);
>> +	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");
>
> igt_assert_f(ret == 0, "Fence created in an unexpectedly signaled state\n");

Agreed.

>
>> +
>> +	/* Advance timeline from 0 -> 1 */
>> +	sw_sync_timeline_inc(timeline, 1);
>> +
>> +	/* Wait on fence until timeout */
>> +	ret = sw_sync_wait(fence, 0);
>> +	igt_assert_f(ret == 0, "Failure waiting on fence until timeout\n");
>
> igt_assert_f(ret == 0, "Fence signaled earlier (timeline value 1, fence seqno 5)\n");

Agreed.

>
>> +
>> +	/* Signal the fence */
>
> /* Advance timeline from 1 -> 5: signaling the fence (seqno 5)*/

Agreed.

>> +	sw_sync_timeline_inc(timeline, 4);
>
>> +
>> +	/* Wait successfully */
>
> Usless comment

Agreed.

>
>> +	ret = sw_sync_wait(fence, 0);
>> +	igt_assert_f(ret > 0, "Failure waiting on fence\n");
>
> igt_assert_f(ret == 0, "Fence not signaled (timeline value 5, fence seqno 5)\n");
>
> If we have a timeline info, we could use %d to say the current value.
>
> Another test would be to then
>
> seqno = 0;
> for (i = 0; i < n_primes; i++) {
> 	seqno += primes[i];
> 	sw_sync_timeline_inc(timeline, primes[i]);
> 	igt_assert_eq(sw_sync_timeline_get_seqno(timeline), seqno);
> }
>

This looks like a good addition, but primes has not previously been 
defined. Do you have preference for primes or would any increment, like 
random be ok?

>> +
>> +	/* Go even further, and confirm wait still succeeds */
>> +	sw_sync_timeline_inc(timeline, 10);
>> +	ret = sw_sync_wait(fence, 0);
>> +	igt_assert_f(ret > 0, "Failure waiting ahead\n");
>
> igt_assert_f(ret == 0, "Fence now not signaled! (timeline value 10, fence seqno 5)\n");

Agreed.
Thanks for the thorough review!


Rob.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 07/13] tests/sw_sync: Add subtest test_sync_merge

[Robert Foss]

On 2016-09-15 04:41 PM, Chris Wilson wrote:
> On Thu, Sep 15, 2016 at 02:40:12PM -0400, robert.foss@collabora.com wrote:
>> From: Robert Foss <robert.foss@collabora.com>
>>
>> Add subtest test_sync_merge that tests merging fences and the validity of the
>> resulting merged fence.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> Reviewed-by: Eric Engestrom <eric@engestrom.ch>
>> ---
>>  tests/sw_sync.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
>> index 3061279..26226bd 100644
>> --- a/tests/sw_sync.c
>> +++ b/tests/sw_sync.c
>> @@ -116,6 +116,70 @@ static void test_sync_wait(void)
>>  	close(timeline);
>>  }
>>
>> +static void test_sync_merge(void)
>> +{
>> +	int in_fence[3];
>> +	int fence_merge;
>> +	int timeline;
>> +	int active, signaled;
>> +
>> +	timeline = sw_sync_timeline_create();
>> +	in_fence[0] = sw_sync_fence_create(timeline, 1);
>> +	in_fence[1] = sw_sync_fence_create(timeline, 2);
>> +	in_fence[2] = sw_sync_fence_create(timeline, 3);
>> +
>> +	fence_merge = sw_sync_merge(in_fence[0], in_fence[1]);
>> +	fence_merge = sw_sync_merge(in_fence[2], fence_merge);
>
> sw_sync_merge() really does need the negative tests:
>
> invalid fd (-1),
> device fd (/dev/dri/card0),
> file fd.

Open other descriptors sounds like a good idea, but for device and file 
fds, which ones will always be available and open-able on any system?

>
> should cover the most common errors (fuzz testing will hit the rest!)
>
>> +
>> +	/* confirm all fences have one active point (even d) */
>> +	active = sw_sync_fence_count_status(in_fence[0],
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(active == 1, "in_fence[0] has too many active fences\n");
>> +	active = sw_sync_fence_count_status(in_fence[1],
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(active == 1, "in_fence[1] has too many active fences\n");
>> +	active = sw_sync_fence_count_status(in_fence[2],
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(active == 1, "in_fence[2] has too many active fences\n");
>> +	active = sw_sync_fence_count_status(fence_merge,
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(active == 1, "fence_merge has too many active fences\n");
>> +
>> +	/* confirm that fence_merge is not signaled until the max of fence 0,1,2 */
>> +	sw_sync_timeline_inc(timeline, 1);
>
>> +	signaled = sw_sync_fence_count_status(in_fence[0],
>> +					      SW_SYNC_FENCE_STATUS_SIGNALED);
>
> This is missing from the earlier test_sync_busy().
>
>> +	active = sw_sync_fence_count_status(fence_merge,
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(signaled == 1, "in_fence[0] did not signal\n");
>> +	igt_assert_f(active == 1, "fence_merge signaled too early\n");
>> +
>> +	sw_sync_timeline_inc(timeline, 1);
>> +	signaled = sw_sync_fence_count_status(in_fence[1],
>> +					      SW_SYNC_FENCE_STATUS_SIGNALED);
>> +	active = sw_sync_fence_count_status(fence_merge,
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(signaled == 1, "in_fence[1] did not signal\n");
>> +	igt_assert_f(active == 1, "fence_merge signaled too early\n");
>> +
>> +	sw_sync_timeline_inc(timeline, 1);
>> +	signaled = sw_sync_fence_count_status(in_fence[2],
>> +					      SW_SYNC_FENCE_STATUS_SIGNALED);
>> +	igt_assert_f(signaled == 1, "in_fence[2] did not signal\n");
>> +	signaled = sw_sync_fence_count_status(fence_merge,
>> +					       SW_SYNC_FENCE_STATUS_SIGNALED);
>> +	active = sw_sync_fence_count_status(fence_merge,
>> +					    SW_SYNC_FENCE_STATUS_ACTIVE);
>> +	igt_assert_f(active == 0 && signaled == 1,
>> +		     "fence_merge did not signal\n");
>
> Hmm, counting STATUS_SIGNALED / STATUS_ACTIVE is not behaving how I
> would intuitively expect.
>
> At this point, timeline.seqno = 3, I would expect count_signaled(merge)
> == 3 (and count_fences(merge) == 3 => merge is now signaled).
> But that's just my expectations

I'll have a look into this.


Rob.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 07/13] tests/sw_sync: Add subtest test_sync_merge

[Chris Wilson]

On Thu, Sep 15, 2016 at 08:27:15PM -0400, Robert Foss wrote:
> 
> 
> On 2016-09-15 04:41 PM, Chris Wilson wrote:
> >On Thu, Sep 15, 2016 at 02:40:12PM -0400, robert.foss@collabora.com wrote:
> >>From: Robert Foss <robert.foss@collabora.com>
> >>
> >>Add subtest test_sync_merge that tests merging fences and the validity of the
> >>resulting merged fence.
> >>
> >>Signed-off-by: Robert Foss <robert.foss@collabora.com>
> >>Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> >>---
> >> tests/sw_sync.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 67 insertions(+)
> >>
> >>diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> >>index 3061279..26226bd 100644
> >>--- a/tests/sw_sync.c
> >>+++ b/tests/sw_sync.c
> >>@@ -116,6 +116,70 @@ static void test_sync_wait(void)
> >> 	close(timeline);
> >> }
> >>
> >>+static void test_sync_merge(void)
> >>+{
> >>+	int in_fence[3];
> >>+	int fence_merge;
> >>+	int timeline;
> >>+	int active, signaled;
> >>+
> >>+	timeline = sw_sync_timeline_create();
> >>+	in_fence[0] = sw_sync_fence_create(timeline, 1);
> >>+	in_fence[1] = sw_sync_fence_create(timeline, 2);
> >>+	in_fence[2] = sw_sync_fence_create(timeline, 3);
> >>+
> >>+	fence_merge = sw_sync_merge(in_fence[0], in_fence[1]);
> >>+	fence_merge = sw_sync_merge(in_fence[2], fence_merge);
> >
> >sw_sync_merge() really does need the negative tests:
> >
> >invalid fd (-1),
> >device fd (/dev/dri/card0),
> >file fd.
> 
> Open other descriptors sounds like a good idea, but for device and

fd = drm_open_driver(DRIVER_ANY)

tmppath[] = "/tmp/igt-XXXXXX";
fd = mkstemp(tmppath);
if (fd != -1) {
	unlink(tmppath);
	test_with_invalid_fd(fd);
	close(fd);
}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 06/13] tests/sw_sync: Add subtest test_sync_wait

[Chris Wilson]

On Thu, Sep 15, 2016 at 06:25:13PM -0400, Robert Foss wrote:
> 
> 
> On 2016-09-15 04:32 PM, Chris Wilson wrote:
> >On Thu, Sep 15, 2016 at 02:40:11PM -0400, robert.foss@collabora.com wrote:
> >>From: Robert Foss <robert.foss@collabora.com>
> >>
> >>This subtest verifies that waiting on fences works properly.
> >>
> >>Signed-off-by: Robert Foss <robert.foss@collabora.com>
> >>Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> >>---
> >> tests/sw_sync.c | 38 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 38 insertions(+)
> >>
> >>diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> >>index fcb2f57..3061279 100644
> >>--- a/tests/sw_sync.c
> >>+++ b/tests/sw_sync.c
> >>@@ -81,6 +81,41 @@ static void test_alloc_merge_fence(void)
> >> 	close(timeline[1]);
> >> }
> >>
> >>+static void test_sync_wait(void)
> >
> >These are not testing waits but busy queries.
> 
> test_sync_wait refers to sw_sync_wait, which may or may not be
> meaningful to refer to.
> Do you still prefer test_sync_busy?

Yes. Querying the busy status (i.e. !signaled) is a common activity, and
quite distinct to waiting.

> >Another test would be to then
> >
> >seqno = 0;
> >for (i = 0; i < n_primes; i++) {
> >	seqno += primes[i];
> >	sw_sync_timeline_inc(timeline, primes[i]);
> >	igt_assert_eq(sw_sync_timeline_get_seqno(timeline), seqno);
> >}
> >
> 
> This looks like a good addition, but primes has not previously been
> defined. Do you have preference for primes or would any increment,
> like random be ok?

random would be ok, but for fun I just added
igt_next_prime_number(x) and for_each_prime_number(prime, N). There are
advantages to using primes and advantages to using random(), and
advantages to using highly structured input.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v5 02/13] tests/sw_sync: Add sw_sync test

[Chris Wilson]

On Thu, Sep 15, 2016 at 05:05:47PM -0400, Robert Foss wrote:
> This is a solid improvement, igt_debugfs_mount() does not however
> seem to be available on upstream/master.

Done. igt_debugfs_mount() is now available.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx