Re: [igt-dev] [PATCH i-g-t 2/5] tests/debugfs: use igt_display_require

[Antonio Argenziano <antonio.argenziano@intel.com>]

On 02/11/18 02:57, Daniel Vetter wrote:
> Need to extract into a test subgroup to make sure we only skip the
> tests that need display support.
> 
> v2: Chris pointed out that "read-all-entries" was the original non-kms
> tests, and we don't want to skip that if there's no output. Make a
> seperate test for this.

Is the separate test implemented in this patch? I only see you adding a 
kms_tests() where 'read_all_entries' is  after an igt_display_require().

> 
> Also, that kind of where libraries magically second-guess what the
> test might have wanted when it supplies an invalid request is exactly
> why I want to fix the igt_display_init API regression.
> 
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   tests/debugfs_test.c | 37 ++++++++++++++++++++++++-------------
>   1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> index 2e87e4420b15..eb32932ed686 100644
> --- a/tests/debugfs_test.c
> +++ b/tests/debugfs_test.c
> @@ -87,23 +87,14 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
>   	closedir(dir);
>   }
>   
> -igt_main
> +static void kms_tests(int fd, int debugfs)
>   {
> -	int fd = -1, debugfs;
>   	igt_display_t display;
>   	struct igt_fb fb[IGT_MAX_PIPES];
>   	enum pipe pipe;
>   
> -	igt_skip_on_simulation();
> -
> -	igt_fixture {
> -		fd = drm_open_driver_master(DRIVER_INTEL);
> -		igt_require_gem(fd);
> -		debugfs = igt_debugfs_dir(fd);
> -
> -		kmstest_set_vt_graphics_mode();
> -		igt_display_init(&display, fd);
> -	}
> +	igt_fixture
> +		igt_display_require(&display, fd);
>   
>   	igt_subtest("read_all_entries") {
>   		/* try to light all pipes */
> @@ -152,6 +143,27 @@ igt_main
>   		read_and_discard_sysfs_entries(debugfs, 0);
>   	}
>   
> +	igt_fixture
> +		igt_display_fini(&display);
> +}
> +
> +igt_main
> +{
> +	int fd = -1, debugfs;
> +
> +	igt_skip_on_simulation();

I think this^ can be removed.

Antonio

> +
> +	igt_fixture {
> +		fd = drm_open_driver_master(DRIVER_INTEL);
> +		igt_require_gem(fd);
> +		debugfs = igt_debugfs_dir(fd);
> +
> +		kmstest_set_vt_graphics_mode();
> +	}
> +
> +	igt_subtest_group
> +		kms_tests(fd, debugfs);
> +
>   	igt_subtest("emon_crash") {
>   		int i;
>   		/*
> @@ -174,7 +186,6 @@ igt_main
>   	}
>   
>   	igt_fixture {
> -		igt_display_fini(&display);
>   		close(debugfs);
>   		close(fd);
>   	}
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev