Re: [Intel-gfx] [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.f...@collabora.com wrote:
> >>From: Robert Foss 
> >>
> >>Add subtest test_sync_merge that tests merging fences and the validity of 
> >>the
> >>resulting merged fence.
> >>
> >>Signed-off-by: Robert Foss 
> >>Reviewed-by: Eric Engestrom 
> >>---
> >> 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-XX";
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: [Intel-gfx] [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.f...@collabora.com wrote:

From: Robert Foss 

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

Signed-off-by: Robert Foss 
Reviewed-by: Eric Engestrom 
---
 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: [Intel-gfx] [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.f...@collabora.com wrote:
> From: Robert Foss 
> 
> Add subtest test_sync_merge that tests merging fences and the validity of the
> resulting merged fence.
> 
> Signed-off-by: Robert Foss 
> Reviewed-by: Eric Engestrom 
> ---
>  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

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

[robert . foss]

From: Robert Foss 

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

Signed-off-by: Robert Foss 
Reviewed-by: Eric Engestrom 
---
 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