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

Antonio Argenziano
2018-11-06T17:18:07Z

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);
> }
>