Re: [PATCH igt 1/6] tests/kms_draw_crc: remove unnecessary mode unset calls

Paulo Zanoni
2016-12-27T15:40:23Z

Em Ter, 2016-12-27 às 15:59 +0100, Daniel Vetter escreveu:
> On Thu, Dec 22, 2016 at 06:42:03PM -0200, Paulo Zanoni wrote:
> >
> > I couldn't think of a reason why we would need to unset the CRTCs
> > before doing the modesets on this test, so remove all the mode
> > unset
> > calls.
>
> There was (not sure whether still is) an issue with the igt_kms
> library
> that it kept the existing config, and that then lead to impossible
> configs. I think we're now clearing just the sw state correctly
> though, so
> doing it with a full commit (and all the time-wasting implied) would
> be
> overkill.
>
> Anyway, that was the reason, worth to double check this is indeed
> working
> now. You probably need multiple screens to make it go boom.

I don't use the igt_kms midlayer stuff... I do the libdrm KMS IOCTLs
directly because IMHO they're much simpler and predictable and easier
to use and with no side-effects, so that's definitely not a problem.
And now you just gave me another reason to not port the program to the
igt_kms API :).

I'm pretty sure the mode unsets were just to be extra-safe in
guaranteeing that whatever crtc/connector/mode we chose would work, but
that's not really a problem.

> -Daniel
>
> >
> >
> > Before:
> > $ time -p sudo ./kms_draw_crc
> > real 44.74
> > $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> > ./kms_draw_crc --run-subtest $i; done
> > real 121.61
> >
> > After:
> > $ time -p sudo ./kms_draw_crc
> > real 7.40
> > $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> > ./kms_draw_crc --run-subtest $i; done
> > real 14.32
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  tests/kms_draw_crc.c | 7 -------
> >  1 file changed, 7 deletions(-)
> >
> > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > index cb28052..e163981 100644
> > --- a/tests/kms_draw_crc.c
> > +++ b/tests/kms_draw_crc.c
> > @@ -143,7 +143,6 @@ static void get_method_crc(enum igt_draw_method
> > method, uint32_t drm_format,
> >
> >   igt_pipe_crc_collect_crc(pipe_crc, crc);
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> >   igt_remove_fb(drm_fd, &fb);
> >  }
> >
> > @@ -152,8 +151,6 @@ static void draw_method_subtest(enum
> > igt_draw_method method,
> >  {
> >   igt_crc_t crc;
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> >   /* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
> > parameter for
> >    * comparison. Cache the value so we don't recompute it
> > for every single
> >    * subtest. */
> > @@ -184,7 +181,6 @@ static void get_fill_crc(uint64_t tiling,
> > igt_crc_t *crc)
> >
> >   igt_pipe_crc_collect_crc(pipe_crc, crc);
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> >   igt_remove_fb(drm_fd, &fb);
> >  }
> >
> > @@ -194,8 +190,6 @@ static void fill_fb_subtest(void)
> >   struct igt_fb fb;
> >   igt_crc_t base_crc, crc;
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> >   igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode-
> > >vdisplay,
> >         DRM_FORMAT_XRGB8888,
> > LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> >
> > @@ -214,7 +208,6 @@ static void fill_fb_subtest(void)
> >   get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
> >   igt_assert_crc_equal(&crc, &base_crc);
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> >   igt_remove_fb(drm_fd, &fb);
> >  }
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>