[PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Maarten Lankhorst]

Export 2 functions, igt_signal_helper_get_num and
igt_signal_helper_get_hz.

This will allow tests to measure how much time in a test was spent
in a uninterruptible state, which is useful when testing whether
certain ioctl's can be interrupted or not.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_aux.c | 30 +++++++++++++++++++++++++++---
 lib/igt_aux.h |  2 ++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 86a213c2032f..265e43f399e7 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -275,7 +275,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool enable)
 }
 
 static struct igt_helper_process signal_helper;
-long long int sig_stat;
+static int64_t sig_stat;
 static void __attribute__((noreturn)) signal_helper_process(pid_t pid)
 {
 	/* Interrupt the parent process at 500Hz, just to be annoying */
@@ -314,6 +314,9 @@ void igt_fork_signal_helper(void)
 	if (igt_only_list_subtests())
 		return;
 
+	/* Reset number of signalscaught */
+	sig_stat = 0;
+
 	/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
 	 * an unexpecting process it spuriously wakes up and does nothing.
 	 * Most other signals (e.g. SIGUSR1) cause the process to die if they
@@ -348,8 +351,29 @@ void igt_stop_signal_helper(void)
 		return;
 
 	igt_stop_helper(&signal_helper);
+}
 
-	sig_stat = 0;
+/**
+ * igt_signal_helper_get_num:
+ *
+ * Return the amount of signals generated since the last time
+ * igt_fork_signal_helper() was called.
+ *
+ * This is reset to 0 on every call to igt_fork_signal_helper.
+ */
+int64_t igt_signal_helper_get_num(void)
+{
+	return sig_stat;
+}
+
+/**
+ * igt_signal_helper_get_hz:
+ *
+ * Return the approximate amount of signals generated per second.
+ */
+int igt_signal_helper_get_hz(void)
+{
+	return 50;
 }
 
 static struct igt_helper_process shrink_helper;
@@ -357,7 +381,7 @@ static void __attribute__((noreturn)) shrink_helper_process(int fd, pid_t pid)
 {
 	while (1) {
 		igt_drop_caches_set(fd, DROP_SHRINK_ALL);
-		usleep(1000 * 1000 / 50);
+		usleep(1000 * 1000 / igt_signal_helper_get_hz());
 		if (kill(pid, 0)) /* Parent has died, so must we. */
 			exit(0);
 	}
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 499a16796ebb..7e080089dcbc 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -55,6 +55,8 @@ extern int num_trash_bos;
 /* generally useful helpers */
 void igt_fork_signal_helper(void);
 void igt_stop_signal_helper(void);
+int64_t igt_signal_helper_get_num(void);
+int igt_signal_helper_get_hz(void);
 
 void igt_fork_shrink_helper(int fd);
 void igt_stop_shrink_helper(void);
-- 
2.11.0

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

[PATCH i-g-t 2/3] lib/igt_kms: Remove vblank wait after plane update.

[Maarten Lankhorst]

With the conversion to atomic, this is already handled in the core.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_kms.c | 15 ---------------
 lib/igt_kms.h |  1 -
 2 files changed, 16 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6390229f1546..14e2701c3afd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2374,8 +2374,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
 
 	CHECK_RETURN(ret, fail_on_error);
 
-	primary->pipe->enabled = (fb_id != 0);
-
 	return 0;
 }
 
@@ -2413,10 +2411,8 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
 			   enum igt_commit_style s,
 			   bool fail_on_error)
 {
-	igt_display_t *display = pipe->display;
 	int i;
 	int ret;
-	bool need_wait_for_vblank = false;
 
 	if (pipe->background_changed) {
 		igt_crtc_set_property(pipe, pipe->background_property,
@@ -2435,21 +2431,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
 	for (i = 0; i < pipe->n_planes; i++) {
 		igt_plane_t *plane = &pipe->planes[i];
 
-		if (plane->fb_changed || plane->position_changed || plane->size_changed)
-			need_wait_for_vblank = true;
-
 		ret = igt_plane_commit(plane, pipe, s, fail_on_error);
 		CHECK_RETURN(ret, fail_on_error);
 	}
 
-	/*
-	 * If the crtc is enabled, wait until the next vblank before returning
-	 * if we made changes to any of the planes.
-	 */
-	if (need_wait_for_vblank && pipe->enabled) {
-		igt_wait_for_vblank(display->drm_fd, pipe->pipe);
-	}
-
 	return 0;
 }
 
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 35428f3e9675..d18a92600933 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -323,7 +323,6 @@ typedef struct {
 struct igt_pipe {
 	igt_display_t *display;
 	enum pipe pipe;
-	bool enabled;
 
 	int n_planes;
 	int plane_cursor;
-- 
2.11.0

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

Re: [PATCH i-g-t 2/3] lib/igt_kms: Remove vblank wait after plane update.

[Mika Kahola]

On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
> With the conversion to atomic, this is already handled in the core.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  lib/igt_kms.c | 15 ---------------
>  lib/igt_kms.h |  1 -
>  2 files changed, 16 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 6390229f1546..14e2701c3afd 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2374,8 +2374,6 @@ static int
> igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  
>  	CHECK_RETURN(ret, fail_on_error);
>  
> -	primary->pipe->enabled = (fb_id != 0);
> -
How was this related to vblank wait removal?

>  	return 0;
>  }
>  
> @@ -2413,10 +2411,8 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>  			   enum igt_commit_style s,
>  			   bool fail_on_error)
>  {
> -	igt_display_t *display = pipe->display;
>  	int i;
>  	int ret;
> -	bool need_wait_for_vblank = false;
>  
>  	if (pipe->background_changed) {
>  		igt_crtc_set_property(pipe, pipe-
> >background_property,
> @@ -2435,21 +2431,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>  	for (i = 0; i < pipe->n_planes; i++) {
>  		igt_plane_t *plane = &pipe->planes[i];
>  
> -		if (plane->fb_changed || plane->position_changed ||
> plane->size_changed)
> -			need_wait_for_vblank = true;
> -
>  		ret = igt_plane_commit(plane, pipe, s,
> fail_on_error);
>  		CHECK_RETURN(ret, fail_on_error);
>  	}
>  
> -	/*
> -	 * If the crtc is enabled, wait until the next vblank before
> returning
> -	 * if we made changes to any of the planes.
> -	 */
> -	if (need_wait_for_vblank && pipe->enabled) {
> -		igt_wait_for_vblank(display->drm_fd, pipe->pipe);
> -	}
> -
>  	return 0;
>  }
>  
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 35428f3e9675..d18a92600933 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -323,7 +323,6 @@ typedef struct {
>  struct igt_pipe {
>  	igt_display_t *display;
>  	enum pipe pipe;
> -	bool enabled;
>  
>  	int n_planes;
>  	int plane_cursor;
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 2/3] lib/igt_kms: Remove vblank wait after plane update.

[Maarten Lankhorst]

Op 04-08-17 om 10:07 schreef Mika Kahola:
> On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
>> With the conversion to atomic, this is already handled in the core.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  lib/igt_kms.c | 15 ---------------
>>  lib/igt_kms.h |  1 -
>>  2 files changed, 16 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 6390229f1546..14e2701c3afd 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -2374,8 +2374,6 @@ static int
>> igt_primary_plane_commit_legacy(igt_plane_t *primary,
>>  
>>  	CHECK_RETURN(ret, fail_on_error);
>>  
>> -	primary->pipe->enabled = (fb_id != 0);
>> -
> How was this related to vblank wait removal?
There are no users of pipe->enabled left, so it's removed.
>>  	return 0;
>>  }
>>  
>> @@ -2413,10 +2411,8 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>>  			   enum igt_commit_style s,
>>  			   bool fail_on_error)
>>  {
>> -	igt_display_t *display = pipe->display;
>>  	int i;
>>  	int ret;
>> -	bool need_wait_for_vblank = false;
>>  
>>  	if (pipe->background_changed) {
>>  		igt_crtc_set_property(pipe, pipe-
>>> background_property,
>> @@ -2435,21 +2431,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>>  	for (i = 0; i < pipe->n_planes; i++) {
>>  		igt_plane_t *plane = &pipe->planes[i];
>>  
>> -		if (plane->fb_changed || plane->position_changed ||
>> plane->size_changed)
>> -			need_wait_for_vblank = true;
>> -
>>  		ret = igt_plane_commit(plane, pipe, s,
>> fail_on_error);
>>  		CHECK_RETURN(ret, fail_on_error);
>>  	}
>>  
>> -	/*
>> -	 * If the crtc is enabled, wait until the next vblank before
>> returning
>> -	 * if we made changes to any of the planes.
>> -	 */
>> -	if (need_wait_for_vblank && pipe->enabled) {
>> -		igt_wait_for_vblank(display->drm_fd, pipe->pipe);
>> -	}
>> -
>>  	return 0;
>>  }
>>  
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 35428f3e9675..d18a92600933 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -323,7 +323,6 @@ typedef struct {
>>  struct igt_pipe {
>>  	igt_display_t *display;
>>  	enum pipe pipe;
>> -	bool enabled;
>>  
>>  	int n_planes;
>>  	int plane_cursor;


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

Re: [PATCH i-g-t 2/3] lib/igt_kms: Remove vblank wait after plane update.

[Mika Kahola]

On Mon, 2017-08-07 at 10:42 +0200, Maarten Lankhorst wrote:
> Op 04-08-17 om 10:07 schreef Mika Kahola:
> > 
> > On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
> > > 
> > > With the conversion to atomic, this is already handled in the
> > > core.
> > > 
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.c
> > > om>
> > > ---
> > >  lib/igt_kms.c | 15 ---------------
> > >  lib/igt_kms.h |  1 -
> > >  2 files changed, 16 deletions(-)
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index 6390229f1546..14e2701c3afd 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -2374,8 +2374,6 @@ static int
> > > igt_primary_plane_commit_legacy(igt_plane_t *primary,
> > >  
> > >  	CHECK_RETURN(ret, fail_on_error);
> > >  
> > > -	primary->pipe->enabled = (fb_id != 0);
> > > -
> > How was this related to vblank wait removal?
> There are no users of pipe->enabled left, so it's removed.
Ok. You can add my

Reviewed-by: Mika Kahola <mika.kahola@intel.com>


> > 
> > > 
> > >  	return 0;
> > >  }
> > >  
> > > @@ -2413,10 +2411,8 @@ static int igt_pipe_commit(igt_pipe_t
> > > *pipe,
> > >  			   enum igt_commit_style s,
> > >  			   bool fail_on_error)
> > >  {
> > > -	igt_display_t *display = pipe->display;
> > >  	int i;
> > >  	int ret;
> > > -	bool need_wait_for_vblank = false;
> > >  
> > >  	if (pipe->background_changed) {
> > >  		igt_crtc_set_property(pipe, pipe-
> > > > 
> > > > background_property,
> > > @@ -2435,21 +2431,10 @@ static int igt_pipe_commit(igt_pipe_t
> > > *pipe,
> > >  	for (i = 0; i < pipe->n_planes; i++) {
> > >  		igt_plane_t *plane = &pipe->planes[i];
> > >  
> > > -		if (plane->fb_changed || plane->position_changed 
> > > ||
> > > plane->size_changed)
> > > -			need_wait_for_vblank = true;
> > > -
> > >  		ret = igt_plane_commit(plane, pipe, s,
> > > fail_on_error);
> > >  		CHECK_RETURN(ret, fail_on_error);
> > >  	}
> > >  
> > > -	/*
> > > -	 * If the crtc is enabled, wait until the next vblank
> > > before
> > > returning
> > > -	 * if we made changes to any of the planes.
> > > -	 */
> > > -	if (need_wait_for_vblank && pipe->enabled) {
> > > -		igt_wait_for_vblank(display->drm_fd, pipe-
> > > >pipe);
> > > -	}
> > > -
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > index 35428f3e9675..d18a92600933 100644
> > > --- a/lib/igt_kms.h
> > > +++ b/lib/igt_kms.h
> > > @@ -323,7 +323,6 @@ typedef struct {
> > >  struct igt_pipe {
> > >  	igt_display_t *display;
> > >  	enum pipe pipe;
> > > -	bool enabled;
> > >  
> > >  	int n_planes;
> > >  	int plane_cursor;
> 
-- 
Mika Kahola - Intel OTC

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

[PATCH i-g-t 3/3] tests: Add kms_atomic_interruptible test.

[Maarten Lankhorst]

To make sure that we have test exposure when allowing atomic ioctl's to
fail interruptibly, we add a test that will fail to lock the
mutexes until the fence is signaled.

If the locking is done interruptibly, our signal helper will interrupt
often, and with the statistics we can ensure that we received
enough interrupts to pass.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/Makefile.sources           |   1 +
 tests/kms_atomic_interruptible.c | 193 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+)
 create mode 100644 tests/kms_atomic_interruptible.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 5b98a5a371b8..9a6a846ab78a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -171,6 +171,7 @@ TESTS_progs = \
 	kms_3d \
 	kms_addfb_basic \
 	kms_atomic \
+	kms_atomic_interruptible \
 	kms_atomic_transition \
 	kms_busy \
 	kms_ccs \
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
new file mode 100644
index 000000000000..4015988c0e2c
--- /dev/null
+++ b/tests/kms_atomic_interruptible.c
@@ -0,0 +1,193 @@
+/*
+ * Copyright © 2016 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 "drmtest.h"
+#include "sw_sync.h"
+
+enum plane_test_type
+{
+	test_setmode,
+	test_setplane,
+	test_pageflip
+};
+
+static int block_plane(igt_display_t *display, igt_output_t *output, enum plane_test_type test_type, igt_plane_t *plane)
+{
+	int timeline = sw_sync_timeline_create();
+
+	igt_fork(child, 1) {
+		/* Ignore the signal helper, we need to block indefinitely on the fence. */
+		signal(SIGCONT, SIG_IGN);
+
+		if (test_type == test_setmode) {
+			igt_output_set_pipe(output, PIPE_NONE);
+			igt_plane_set_fb(plane, NULL);
+		}
+		igt_plane_set_fence_fd(plane, sw_sync_timeline_create_fence(timeline, 1));
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+	}
+
+	return timeline;
+}
+
+static void unblock(int block)
+{
+	sw_sync_timeline_inc(block, 1);
+	close(block);
+}
+
+static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t *output,
+			   enum plane_test_type test_type, unsigned plane_type, enum igt_commit_style style)
+{
+	drmModeModeInfo *mode;
+	igt_fb_t fb, fb2;
+	igt_plane_t *primary, *plane;
+	int block;
+
+	/*
+	 * Make sure we start with everything disabled to force a real modeset.
+	 * igt_display_init only sets sw state, and assumes the first test doesn't care
+	 * about hw state.
+	 */
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_output_set_pipe(output, pipe);
+
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	plane = igt_output_get_plane_type(output, plane_type);
+	mode = igt_output_get_mode(output);
+
+	igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
+
+	switch (plane_type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+		igt_create_fb(display->drm_fd, 64, 64,
+		      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
+		break;
+	}
+
+	if (test_type != test_setmode) {
+		igt_plane_set_fb(primary, &fb);
+		igt_display_commit2(display, COMMIT_ATOMIC);
+	}
+
+	igt_plane_set_fb(plane, &fb2);
+
+	block = block_plane(display, output, test_type, plane);
+	sleep(1);
+
+	igt_fork(child, 1) {
+		signal(SIGCONT, SIG_IGN);
+		igt_assert(sleep(5) == 0);
+
+		unblock(block);
+	}
+
+	/* run the test */
+	igt_fork_signal_helper();
+	if (test_type != test_pageflip)
+		igt_display_commit2(display, style);
+	else
+		do_or_die(drmModePageFlip(display->drm_fd, display->pipes[pipe].crtc_id, fb.fb_id, 0, NULL));
+	igt_stop_signal_helper();
+
+	/* Test whether we did interrupt the test? */
+	igt_assert_lt(2 * igt_signal_helper_get_hz(), igt_signal_helper_get_num());
+
+	igt_waitchildren();
+
+	igt_plane_set_fb(primary, NULL);
+	igt_output_set_pipe(output, PIPE_NONE);
+	igt_remove_fb(display->drm_fd, &fb);
+}
+
+igt_main
+{
+	igt_display_t display;
+	igt_output_t *output;
+	enum pipe pipe;
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+
+		kmstest_set_vt_graphics_mode();
+
+		igt_display_init(&display, display.drm_fd);
+
+		igt_require(display.is_atomic);
+
+		igt_display_require_output(&display);
+
+		igt_require_sw_sync();
+	}
+
+	igt_subtest("legacy-setmode")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
+			break;
+		}
+
+	igt_subtest("atomic-setmode")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_ATOMIC);
+			break;
+		}
+
+	igt_subtest("legacy-pageflip")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_pageflip, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
+			break;
+		}
+
+	igt_subtest("legacy-cursor")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_LEGACY);
+			break;
+		}
+
+	igt_subtest("universal-setplane-primary")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_setplane, DRM_PLANE_TYPE_PRIMARY, COMMIT_UNIVERSAL);
+			break;
+		}
+
+	igt_subtest("universal-setplane-cursor")
+		for_each_pipe_with_valid_output(&display, pipe, output) {
+			run_plane_test(&display, pipe, output, test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_UNIVERSAL);
+			break;
+		}
+
+	/* TODO: DPMS, gamma_set, setprop, getprop */
+	igt_fixture {
+		igt_display_fini(&display);
+	}
+}
-- 
2.11.0

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

Re: [PATCH i-g-t 3/3] tests: Add kms_atomic_interruptible test.

[Mika Kahola]

On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
> To make sure that we have test exposure when allowing atomic ioctl's
> to
> fail interruptibly, we add a test that will fail to lock the
> mutexes until the fence is signaled.
> 
> If the locking is done interruptibly, our signal helper will
> interrupt
> often, and with the statistics we can ensure that we received
> enough interrupts to pass.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/Makefile.sources           |   1 +
>  tests/kms_atomic_interruptible.c | 193
> +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 194 insertions(+)
>  create mode 100644 tests/kms_atomic_interruptible.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 5b98a5a371b8..9a6a846ab78a 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -171,6 +171,7 @@ TESTS_progs = \
>  	kms_3d \
>  	kms_addfb_basic \
>  	kms_atomic \
> +	kms_atomic_interruptible \
>  	kms_atomic_transition \
>  	kms_busy \
>  	kms_ccs \
> diff --git a/tests/kms_atomic_interruptible.c
> b/tests/kms_atomic_interruptible.c
> new file mode 100644
> index 000000000000..4015988c0e2c
> --- /dev/null
> +++ b/tests/kms_atomic_interruptible.c
> @@ -0,0 +1,193 @@
> +/*
> + * Copyright © 2016 Intel Corporation
Update a year here?
> + *
> + * 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 "drmtest.h"
> +#include "sw_sync.h"
> +
> +enum plane_test_type
> +{
> +	test_setmode,
> +	test_setplane,
> +	test_pageflip
> +};
> +
> +static int block_plane(igt_display_t *display, igt_output_t *output,
> enum plane_test_type test_type, igt_plane_t *plane)
> +{
> +	int timeline = sw_sync_timeline_create();
> +
> +	igt_fork(child, 1) {
> +		/* Ignore the signal helper, we need to block
> indefinitely on the fence. */
> +		signal(SIGCONT, SIG_IGN);
> +
> +		if (test_type == test_setmode) {
> +			igt_output_set_pipe(output, PIPE_NONE);
> +			igt_plane_set_fb(plane, NULL);
> +		}
> +		igt_plane_set_fence_fd(plane,
> sw_sync_timeline_create_fence(timeline, 1));
> +
> +		igt_display_commit2(display, COMMIT_ATOMIC);
> +	}
> +
> +	return timeline;
> +}
> +
> +static void unblock(int block)
> +{
> +	sw_sync_timeline_inc(block, 1);
> +	close(block);
> +}
> +
> +static void run_plane_test(igt_display_t *display, enum pipe pipe,
> igt_output_t *output,
> +			   enum plane_test_type test_type, unsigned
> plane_type, enum igt_commit_style style)
> +{
> +	drmModeModeInfo *mode;
> +	igt_fb_t fb, fb2;
> +	igt_plane_t *primary, *plane;
> +	int block;
> +
> +	/*
> +	 * Make sure we start with everything disabled to force a
> real modeset.
> +	 * igt_display_init only sets sw state, and assumes the
> first test doesn't care
> +	 * about hw state.
> +	 */
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	igt_output_set_pipe(output, pipe);
> +
> +	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> +	plane = igt_output_get_plane_type(output, plane_type);
> +	mode = igt_output_get_mode(output);
> +
> +	igt_create_fb(display->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> +		      DRM_FORMAT_XRGB8888,
> LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> +
> +	switch (plane_type) {
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		igt_create_fb(display->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
> +		break;
> +	case DRM_PLANE_TYPE_CURSOR:
> +		igt_create_fb(display->drm_fd, 64, 64,
> +		      DRM_FORMAT_ARGB8888,
> LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
> +		break;
> +	}
We could squeeze this into a one line and get rid of the switch()?
Anyway this is more matter of a taste and style than functionality.

> +
> +	if (test_type != test_setmode) {
> +		igt_plane_set_fb(primary, &fb);
> +		igt_display_commit2(display, COMMIT_ATOMIC);
> +	}
> +
> +	igt_plane_set_fb(plane, &fb2);
> +
> +	block = block_plane(display, output, test_type, plane);
> +	sleep(1);
> +
> +	igt_fork(child, 1) {
> +		signal(SIGCONT, SIG_IGN);
> +		igt_assert(sleep(5) == 0);
> +
> +		unblock(block);
> +	}
> +
> +	/* run the test */
> +	igt_fork_signal_helper();
> +	if (test_type != test_pageflip)
> +		igt_display_commit2(display, style);
> +	else
> +		do_or_die(drmModePageFlip(display->drm_fd, display-
> >pipes[pipe].crtc_id, fb.fb_id, 0, NULL));
> +	igt_stop_signal_helper();
> +
> +	/* Test whether we did interrupt the test? */
> +	igt_assert_lt(2 * igt_signal_helper_get_hz(),
> igt_signal_helper_get_num());
> +
> +	igt_waitchildren();
> +
> +	igt_plane_set_fb(primary, NULL);
> +	igt_output_set_pipe(output, PIPE_NONE);
> +	igt_remove_fb(display->drm_fd, &fb);
> +}
> +
> +igt_main
> +{
> +	igt_display_t display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
> +
> +		kmstest_set_vt_graphics_mode();
> +
> +		igt_display_init(&display, display.drm_fd);
> +
> +		igt_require(display.is_atomic);
> +
> +		igt_display_require_output(&display);
> +
> +		igt_require_sw_sync();
> +	}
> +
> +	igt_subtest("legacy-setmode")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
> +			break;
> +		}
> +
> +	igt_subtest("atomic-setmode")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_ATOMIC);
> +			break;
> +		}
> +
> +	igt_subtest("legacy-pageflip")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_pageflip, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
> +			break;
> +		}
> +
> +	igt_subtest("legacy-cursor")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_LEGACY);
> +			break;
> +		}
> +
> +	igt_subtest("universal-setplane-primary")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_setplane, DRM_PLANE_TYPE_PRIMARY, COMMIT_UNIVERSAL);
> +			break;
> +		}
> +
> +	igt_subtest("universal-setplane-cursor")
> +		for_each_pipe_with_valid_output(&display, pipe,
> output) {
> +			run_plane_test(&display, pipe, output,
> test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_UNIVERSAL);
> +			break;
> +		}
> +
> +	/* TODO: DPMS, gamma_set, setprop, getprop */
> +	igt_fixture {
> +		igt_display_fini(&display);
> +	}
> +}
Otherwise looks good to me.

-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 3/3] tests: Add kms_atomic_interruptible test.

[Maarten Lankhorst]

Op 04-08-17 om 09:50 schreef Mika Kahola:
> On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
>> To make sure that we have test exposure when allowing atomic ioctl's
>> to
>> fail interruptibly, we add a test that will fail to lock the
>> mutexes until the fence is signaled.
>>
>> If the locking is done interruptibly, our signal helper will
>> interrupt
>> often, and with the statistics we can ensure that we received
>> enough interrupts to pass.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  tests/Makefile.sources           |   1 +
>>  tests/kms_atomic_interruptible.c | 193
>> +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 194 insertions(+)
>>  create mode 100644 tests/kms_atomic_interruptible.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 5b98a5a371b8..9a6a846ab78a 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -171,6 +171,7 @@ TESTS_progs = \
>>  	kms_3d \
>>  	kms_addfb_basic \
>>  	kms_atomic \
>> +	kms_atomic_interruptible \
>>  	kms_atomic_transition \
>>  	kms_busy \
>>  	kms_ccs \
>> diff --git a/tests/kms_atomic_interruptible.c
>> b/tests/kms_atomic_interruptible.c
>> new file mode 100644
>> index 000000000000..4015988c0e2c
>> --- /dev/null
>> +++ b/tests/kms_atomic_interruptible.c
>> @@ -0,0 +1,193 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
> Update a year here?
>> + *
>> + * 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 "drmtest.h"
>> +#include "sw_sync.h"
>> +
>> +enum plane_test_type
>> +{
>> +	test_setmode,
>> +	test_setplane,
>> +	test_pageflip
>> +};
>> +
>> +static int block_plane(igt_display_t *display, igt_output_t *output,
>> enum plane_test_type test_type, igt_plane_t *plane)
>> +{
>> +	int timeline = sw_sync_timeline_create();
>> +
>> +	igt_fork(child, 1) {
>> +		/* Ignore the signal helper, we need to block
>> indefinitely on the fence. */
>> +		signal(SIGCONT, SIG_IGN);
>> +
>> +		if (test_type == test_setmode) {
>> +			igt_output_set_pipe(output, PIPE_NONE);
>> +			igt_plane_set_fb(plane, NULL);
>> +		}
>> +		igt_plane_set_fence_fd(plane,
>> sw_sync_timeline_create_fence(timeline, 1));
>> +
>> +		igt_display_commit2(display, COMMIT_ATOMIC);
>> +	}
>> +
>> +	return timeline;
>> +}
>> +
>> +static void unblock(int block)
>> +{
>> +	sw_sync_timeline_inc(block, 1);
>> +	close(block);
>> +}
>> +
>> +static void run_plane_test(igt_display_t *display, enum pipe pipe,
>> igt_output_t *output,
>> +			   enum plane_test_type test_type, unsigned
>> plane_type, enum igt_commit_style style)
>> +{
>> +	drmModeModeInfo *mode;
>> +	igt_fb_t fb, fb2;
>> +	igt_plane_t *primary, *plane;
>> +	int block;
>> +
>> +	/*
>> +	 * Make sure we start with everything disabled to force a
>> real modeset.
>> +	 * igt_display_init only sets sw state, and assumes the
>> first test doesn't care
>> +	 * about hw state.
>> +	 */
>> +	igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +	igt_output_set_pipe(output, pipe);
>> +
>> +	primary = igt_output_get_plane_type(output,
>> DRM_PLANE_TYPE_PRIMARY);
>> +	plane = igt_output_get_plane_type(output, plane_type);
>> +	mode = igt_output_get_mode(output);
>> +
>> +	igt_create_fb(display->drm_fd, mode->hdisplay, mode-
>>> vdisplay,
>> +		      DRM_FORMAT_XRGB8888,
>> LOCAL_DRM_FORMAT_MOD_NONE, &fb);
>> +
>> +	switch (plane_type) {
>> +	case DRM_PLANE_TYPE_PRIMARY:
>> +		igt_create_fb(display->drm_fd, mode->hdisplay, mode-
>>> vdisplay,
>> +			      DRM_FORMAT_XRGB8888,
>> LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
>> +		break;
>> +	case DRM_PLANE_TYPE_CURSOR:
>> +		igt_create_fb(display->drm_fd, 64, 64,
>> +		      DRM_FORMAT_ARGB8888,
>> LOCAL_DRM_FORMAT_MOD_NONE, &fb2);
>> +		break;
>> +	}
> We could squeeze this into a one line and get rid of the switch()?
> Anyway this is more matter of a taste and style than functionality.
Was thinking of adding overlay, but that's probably overkill for now.
>> +
>> +	if (test_type != test_setmode) {
>> +		igt_plane_set_fb(primary, &fb);
>> +		igt_display_commit2(display, COMMIT_ATOMIC);
>> +	}
>> +
>> +	igt_plane_set_fb(plane, &fb2);
>> +
>> +	block = block_plane(display, output, test_type, plane);
>> +	sleep(1);
>> +
>> +	igt_fork(child, 1) {
>> +		signal(SIGCONT, SIG_IGN);
>> +		igt_assert(sleep(5) == 0);
>> +
>> +		unblock(block);
>> +	}
>> +
>> +	/* run the test */
>> +	igt_fork_signal_helper();
>> +	if (test_type != test_pageflip)
>> +		igt_display_commit2(display, style);
>> +	else
>> +		do_or_die(drmModePageFlip(display->drm_fd, display-
>>> pipes[pipe].crtc_id, fb.fb_id, 0, NULL));
>> +	igt_stop_signal_helper();
>> +
>> +	/* Test whether we did interrupt the test? */
>> +	igt_assert_lt(2 * igt_signal_helper_get_hz(),
>> igt_signal_helper_get_num());
>> +
>> +	igt_waitchildren();
>> +
>> +	igt_plane_set_fb(primary, NULL);
>> +	igt_output_set_pipe(output, PIPE_NONE);
>> +	igt_remove_fb(display->drm_fd, &fb);
>> +}
>> +
>> +igt_main
>> +{
>> +	igt_display_t display;
>> +	igt_output_t *output;
>> +	enum pipe pipe;
>> +
>> +	igt_skip_on_simulation();
>> +
>> +	igt_fixture {
>> +		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
>> +
>> +		kmstest_set_vt_graphics_mode();
>> +
>> +		igt_display_init(&display, display.drm_fd);
>> +
>> +		igt_require(display.is_atomic);
>> +
>> +		igt_display_require_output(&display);
>> +
>> +		igt_require_sw_sync();
>> +	}
>> +
>> +	igt_subtest("legacy-setmode")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
>> +			break;
>> +		}
>> +
>> +	igt_subtest("atomic-setmode")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_setmode, DRM_PLANE_TYPE_PRIMARY, COMMIT_ATOMIC);
>> +			break;
>> +		}
>> +
>> +	igt_subtest("legacy-pageflip")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_pageflip, DRM_PLANE_TYPE_PRIMARY, COMMIT_LEGACY);
>> +			break;
>> +		}
>> +
>> +	igt_subtest("legacy-cursor")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_LEGACY);
>> +			break;
>> +		}
>> +
>> +	igt_subtest("universal-setplane-primary")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_setplane, DRM_PLANE_TYPE_PRIMARY, COMMIT_UNIVERSAL);
>> +			break;
>> +		}
>> +
>> +	igt_subtest("universal-setplane-cursor")
>> +		for_each_pipe_with_valid_output(&display, pipe,
>> output) {
>> +			run_plane_test(&display, pipe, output,
>> test_setplane, DRM_PLANE_TYPE_CURSOR, COMMIT_UNIVERSAL);
>> +			break;
>> +		}
>> +
>> +	/* TODO: DPMS, gamma_set, setprop, getprop */
>> +	igt_fixture {
>> +		igt_display_fini(&display);
>> +	}
>> +}
> Otherwise looks good to me.
>

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

Re: [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Mika Kahola]

On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
> Export 2 functions, igt_signal_helper_get_num and
> igt_signal_helper_get_hz.
> 
> This will allow tests to measure how much time in a test was spent
> in a uninterruptible state, which is useful when testing whether
> certain ioctl's can be interrupted or not.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  lib/igt_aux.c | 30 +++++++++++++++++++++++++++---
>  lib/igt_aux.h |  2 ++
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 86a213c2032f..265e43f399e7 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -275,7 +275,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter
> *iter, bool enable)
>  }
>  
>  static struct igt_helper_process signal_helper;
> -long long int sig_stat;
> +static int64_t sig_stat;
>  static void __attribute__((noreturn)) signal_helper_process(pid_t
> pid)
>  {
>  	/* Interrupt the parent process at 500Hz, just to be
> annoying */
> @@ -314,6 +314,9 @@ void igt_fork_signal_helper(void)
>  	if (igt_only_list_subtests())
>  		return;
>  
> +	/* Reset number of signalscaught */
> +	sig_stat = 0;
> +
>  	/* We pick SIGCONT as it is a "safe" signal - if we send
> SIGCONT to
>  	 * an unexpecting process it spuriously wakes up and does
> nothing.
>  	 * Most other signals (e.g. SIGUSR1) cause the process to
> die if they
> @@ -348,8 +351,29 @@ void igt_stop_signal_helper(void)
>  		return;
>  
>  	igt_stop_helper(&signal_helper);
> +}
>  
> -	sig_stat = 0;
> +/**
> + * igt_signal_helper_get_num:
> + *
> + * Return the amount of signals generated since the last time
> + * igt_fork_signal_helper() was called.
> + *
> + * This is reset to 0 on every call to igt_fork_signal_helper.
> + */
> +int64_t igt_signal_helper_get_num(void)
> +{
> +	return sig_stat;
> +}
> +
> +/**
> + * igt_signal_helper_get_hz:
> + *
> + * Return the approximate amount of signals generated per second.
> + */
> +int igt_signal_helper_get_hz(void)
> +{
> +	return 50;
>  }
I wonder if this is really necessary? The function returns hardcoded
value and it is only used in one place i.e. third patch of this series.
>  
>  static struct igt_helper_process shrink_helper;
> @@ -357,7 +381,7 @@ static void __attribute__((noreturn))
> shrink_helper_process(int fd, pid_t pid)
>  {
>  	while (1) {
>  		igt_drop_caches_set(fd, DROP_SHRINK_ALL);
> -		usleep(1000 * 1000 / 50);
> +		usleep(1000 * 1000 / igt_signal_helper_get_hz());
>  		if (kill(pid, 0)) /* Parent has died, so must we. */
>  			exit(0);
>  	}
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 499a16796ebb..7e080089dcbc 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -55,6 +55,8 @@ extern int num_trash_bos;
>  /* generally useful helpers */
>  void igt_fork_signal_helper(void);
>  void igt_stop_signal_helper(void);
> +int64_t igt_signal_helper_get_num(void);
> +int igt_signal_helper_get_hz(void);
>  
>  void igt_fork_shrink_helper(int fd);
>  void igt_stop_shrink_helper(void);
-- 
Mika Kahola - Intel OTC

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

Re: [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-08-02 11:29:17)
> Export 2 functions, igt_signal_helper_get_num and
> igt_signal_helper_get_hz.
> 
> This will allow tests to measure how much time in a test was spent
> in a uninterruptible state, which is useful when testing whether
> certain ioctl's can be interrupted or not.

Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
was interrupted. Refine it to suit your purposes, it is the surgical
scalpel compared to the shotgun of signal_helper.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Maarten Lankhorst]

Op 04-08-17 om 09:50 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2017-08-02 11:29:17)
>> Export 2 functions, igt_signal_helper_get_num and
>> igt_signal_helper_get_hz.
>>
>> This will allow tests to measure how much time in a test was spent
>> in a uninterruptible state, which is useful when testing whether
>> certain ioctl's can be interrupted or not.
> Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> was interrupted. Refine it to suit your purposes, it is the surgical
> scalpel compared to the shotgun of signal_helper.
> -Chris

I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose..

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

Re: [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-08-07 10:45:38)
> Op 04-08-17 om 09:50 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2017-08-02 11:29:17)
> >> Export 2 functions, igt_signal_helper_get_num and
> >> igt_signal_helper_get_hz.
> >>
> >> This will allow tests to measure how much time in a test was spent
> >> in a uninterruptible state, which is useful when testing whether
> >> certain ioctl's can be interrupted or not.
> > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> > was interrupted. Refine it to suit your purposes, it is the surgical
> > scalpel compared to the shotgun of signal_helper.
> > -Chris
> 
> I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose..

You know that we igt uses igt_ioctl so that we can switch the ioctl
wrapper. What you have here is sig_ioctl so rather than painting the
shotgun signal helper a different colour, use the interface that
actually supports what you have in mind.
-Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Daniel Vetter]

On Mon, Aug 07, 2017 at 10:59:55AM +0100, Chris Wilson wrote:
> Quoting Maarten Lankhorst (2017-08-07 10:45:38)
> > Op 04-08-17 om 09:50 schreef Chris Wilson:
> > > Quoting Maarten Lankhorst (2017-08-02 11:29:17)
> > >> Export 2 functions, igt_signal_helper_get_num and
> > >> igt_signal_helper_get_hz.
> > >>
> > >> This will allow tests to measure how much time in a test was spent
> > >> in a uninterruptible state, which is useful when testing whether
> > >> certain ioctl's can be interrupted or not.
> > > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> > > was interrupted. Refine it to suit your purposes, it is the surgical
> > > scalpel compared to the shotgun of signal_helper.
> > > -Chris
> > 
> > I've been using the igt display helpers now so I don't have to worry about the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and that makes the code a lot more verbose..
> 
> You know that we igt uses igt_ioctl so that we can switch the ioctl
> wrapper. What you have here is sig_ioctl so rather than painting the
> shotgun signal helper a different colour, use the interface that
> actually supports what you have in mind.

Might be good to spicy the docs up a bit to make this clearer, maybe even
with an example and some links between the different bits? Mika/Maarten?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx