Re: [igt-dev] [PATCH i-g-t] tests: Check and skip tests when driver don't have display enabled

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2018-09-12 at 09:06 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-09-12 00:42:52)
> > Right now i915 is not doing the full job to complete disable
> > display
> > when i915.disable_display parameters is set, when that is completed
> > it will cause several tests to fail, so here skiping all the tests
> > that depends on modeset or display block when those are not
> > available.
> > 
> > v2: Not skipping all tests in debugfs_test, perf_pmu and perf_pmu,
> > now skiping only the required subtests
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_kms.c                     | 20 +++++++++
> >  lib/igt_kms.h                     |  1 +
> >  tests/debugfs_test.c              | 13 +++++-
> >  tests/gem_exec_nop.c              |  1 +
> >  tests/kms_3d.c                    |  1 +
> >  tests/kms_addfb_basic.c           |  4 +-
> >  tests/kms_atomic.c                |  1 +
> >  tests/kms_atomic_interruptible.c  |  1 +
> >  tests/kms_available_modes_crc.c   |  1 +
> >  tests/kms_busy.c                  |  1 +
> >  tests/kms_ccs.c                   |  1 +
> >  tests/kms_chamelium.c             |  1 +
> >  tests/kms_chv_cursor_fail.c       |  1 +
> >  tests/kms_color.c                 |  1 +
> >  tests/kms_concurrent.c            |  1 +
> >  tests/kms_crtc_background_color.c |  1 +
> >  tests/kms_cursor_crc.c            |  1 +
> >  tests/kms_cursor_legacy.c         |  1 +
> >  tests/kms_draw_crc.c              |  1 +
> >  tests/kms_fbcon_fbt.c             |  1 +
> >  tests/kms_fence_pin_leak.c        |  1 +
> >  tests/kms_flip.c                  |  1 +
> >  tests/kms_flip_event_leak.c       |  1 +
> >  tests/kms_flip_tiling.c           |  1 +
> >  tests/kms_force_connector_basic.c |  1 +
> >  tests/kms_frontbuffer_tracking.c  |  1 +
> >  tests/kms_getfb.c                 |  4 +-
> >  tests/kms_hdmi_inject.c           |  1 +
> >  tests/kms_invalid_dotclock.c      |  1 +
> >  tests/kms_legacy_colorkey.c       |  1 +
> >  tests/kms_mmap_write_crc.c        |  1 +
> >  tests/kms_panel_fitting.c         |  1 +
> >  tests/kms_pipe_b_c_ivb.c          |  1 +
> >  tests/kms_pipe_crc_basic.c        |  1 +
> >  tests/kms_plane.c                 |  1 +
> >  tests/kms_plane_lowres.c          |  1 +
> >  tests/kms_plane_multiple.c        |  1 +
> >  tests/kms_plane_scaling.c         |  1 +
> >  tests/kms_properties.c            |  1 +
> >  tests/kms_psr.c                   |  1 +
> >  tests/kms_pwrite_crc.c            |  1 +
> >  tests/kms_rmfb.c                  |  1 +
> >  tests/kms_rotation_crc.c          |  1 +
> >  tests/kms_setmode.c               |  1 +
> >  tests/kms_sysfs_edid_timing.c     |  5 +++
> >  tests/kms_tv_load_detect.c        |  1 +
> >  tests/kms_universal_plane.c       |  1 +
> >  tests/kms_vblank.c                |  1 +
> >  tests/perf_pmu.c                  | 13 +++---
> >  tests/pm_backlight.c              |  5 ++-
> >  tests/pm_lpsp.c                   |  1 +
> >  tests/pm_rpm.c                    | 69 ++++++++++++++++++++++++---
> > ----
> >  tests/prime_vgem.c                |  2 +
> >  tests/testdisplay.c               |  1 +
> >  54 files changed, 155 insertions(+), 25 deletions(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 4563bfd9..a12d4b79 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -2018,6 +2018,26 @@ int igt_display_get_n_pipes(igt_display_t
> > *display)
> >         return display->n_pipes;
> >  }
> >  
> > +/**
> > + * igt_display_require_output:
> > + * @drm_fd: a drm file descriptor
> > + *
> > + * Checks if driver supports modeset and have display enabled.
> > + */
> > +void igt_require_display(int fd)
> > +{
> > +       drmModeRes *resources;
> > +       int pipes;
> > +
> > +       resources = drmModeGetResources(fd);
> > +       igt_require_f(resources, "drm driver do not support
> > modeset\n");
> 
> Do you still not know how to do this without an allocation?

Okay, now I changed it.

> >         igt_subtest("read_all_entries") {
> > +               if (!display.n_pipes) {
> > +                       igt_require_display(fd);
> > +                       igt_display_init(&display, fd);
> 
> No, this test does not depend on there being a display.

Okay, so I will just jump to read_and_discard_sysfs_entries()

> 
> >                 /* try to light all pipes */
> >                 for_each_pipe(&display, pipe) {
> >                         igt_output_t *output;
> > @@ -140,6 +146,11 @@ igt_main
> >                 igt_output_t *output;
> >                 igt_plane_t *plane;
> >  
> > +               if (!display.n_pipes) {
> > +                       igt_require_display(fd);
> > +                       igt_display_init(&display, fd);
> > +               }
> > +
> >                 for_each_connected_output(&display, output)
> >                         igt_output_set_pipe(output, PIPE_NONE);
> >  
> > diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
> > index 74d27522..1cd3d70d 100644
> > --- a/tests/gem_exec_nop.c
> > +++ b/tests/gem_exec_nop.c
> > @@ -279,6 +279,7 @@ static void headless(int fd, uint32_t handle)
> >         drmModeRes *res;
> >         double n_display, n_headless;
> >  
> > +       igt_require_display(fd);
> >         res = drmModeGetResources(fd);
> >         igt_assert(res);
> 
> This already skips without a display.

Oh true, if the igt_device_set_master() fails it skips, thanks.

> 
> I expect all the others that iterate over resources handle the
> absence
> of their target setup similarly.
> 
> > diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> > index bfc981ee..830b3321 100644
> > --- a/tests/kms_3d.c
> > +++ b/tests/kms_3d.c
> > @@ -36,6 +36,7 @@ igt_simple_main
> >         int mode_count, connector_id;
> >  
> >         drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +       igt_require_display(drm_fd);
> >         res = drmModeGetResources(drm_fd);
> >  
> >         igt_assert(drmSetClientCap(drm_fd,
> > DRM_CLIENT_CAP_STEREO_3D, 1) >= 0);
> > diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> > index ce48d24f..ce4caa5e 100644
> > --- a/tests/kms_addfb_basic.c
> > +++ b/tests/kms_addfb_basic.c
> > @@ -671,8 +671,10 @@ int fd;
> >  
> >  igt_main
> >  {
> > -       igt_fixture
> > +       igt_fixture {
> >                 fd = drm_open_driver_master(DRIVER_ANY);
> > +               igt_require_display(fd);
> > +       }
> 
> Does not require a display, just the addfb interface.

You mean add the addfb interface in kernel when loading driver with
display off? Okay this test is not setting the FB to any CRTC but why
would we want to test that with display off?
Also to do that we would need to keep DRIVER_MODESET set with display
off otherwise drm_mode_addfb2() would fail.

> 
> Etc.
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev