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

Daniel Vetter
2018-11-06T22:06:12Z

On Tue, Nov 6, 2018 at 6:18 PM Antonio Argenziano
<antonio.argenziano@intel.com> wrote:
>
>
>
> 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().

Oops, squashed it accidentally into the wrong patch :-/ One the
missing hunks are in the next one. I'll rebase a bunch and resend.
-Daniel

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

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch