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

[Mika Kahola <mika.kahola@intel.com>]

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