[igt-dev] ✓ Fi.CI.BAT: success for Skip VBlank tests in modules without VBlank (rev2)
Rodrigo Siqueira
rodrigosiqueiramelo at gmail.com
Mon Jan 14 10:38:24 UTC 2019
On 01/14, Daniel Vetter wrote:
> On Sun, Jan 13, 2019 at 06:31:07PM -0200, Rodrigo Siqueira wrote:
> > Hi Arkadiusz,
> >
> > On 08/27, Arkadiusz Hiler wrote:
> > > On Thu, Aug 23, 2018 at 05:47:41PM +0000, Patchwork wrote:
> > > > == Series Details ==
> > > >
> > > > Series: Skip VBlank tests in modules without VBlank (rev2)
> > > > URL : https://patchwork.freedesktop.org/series/48468/
> > > > State : success
> > > >
> > > > == Summary ==
> > > >
> > > > = CI Bug Log - changes from CI_DRM_4702 -> IGTPW_1741 =
> > > >
> > > > == Summary - WARNING ==
> > > >
> > > > Minor unknown changes coming with IGTPW_1741 need to be verified
> > > > manually.
> > > >
> > > > If you think the reported changes have nothing to do with the changes
> > > > introduced in IGTPW_1741, please notify your bug team to allow them
> > > > to document this new failure mode, which will reduce false positives in CI.
> > > >
> > > > External URL: https://patchwork.freedesktop.org/api/1.0/series/48468/revisions/2/mbox/
> > > >
> > > > == Possible new issues ==
> > > >
> > > > Here are the unknown changes that may have been introduced in IGTPW_1741:
> > > >
> > > > === IGT changes ===
> > > >
> > > > ==== Warnings ====
> > > >
> > > > igt at kms_flip@basic-flip-vs-wf_vblank:
> > > > fi-hsw-peppy: PASS -> SKIP
> > > > fi-snb-2600: PASS -> SKIP
> > > > fi-skl-6260u: PASS -> SKIP
> > > > fi-hsw-4770r: PASS -> SKIP
> > > > {fi-bdw-samus}: PASS -> SKIP
> > > > {fi-cfl-8109u}: PASS -> SKIP
> > > > fi-blb-e6850: PASS -> SKIP
> > > > fi-kbl-r: PASS -> SKIP
> > > > fi-bwr-2160: PASS -> SKIP
> > > > fi-bdw-5557u: PASS -> SKIP
> > > > fi-skl-6600u: PASS -> SKIP
> > > > fi-kbl-7560u: PASS -> SKIP
> > > > fi-hsw-4770: PASS -> SKIP
> > > > fi-byt-j1900: PASS -> SKIP
> > > > fi-skl-6700k2: PASS -> SKIP
> > > > fi-ivb-3770: PASS -> SKIP
> > > > fi-cnl-psr: PASS -> SKIP
> > > > {fi-bsw-kefka}: PASS -> SKIP
> > > > fi-skl-6700hq: PASS -> SKIP
> > > > fi-bxt-j4205: PASS -> SKIP
> > > > fi-bsw-n3050: PASS -> SKIP
> > > > fi-skl-gvtdvm: PASS -> SKIP
> > > > {fi-byt-clapper}: PASS -> SKIP
> > > > fi-gdg-551: PASS -> SKIP
> > > > fi-glk-dsi: PASS -> SKIP
> > > > fi-glk-j4005: PASS -> SKIP
> > > > fi-skl-guc: PASS -> SKIP
> > > > fi-kbl-7567u: PASS -> SKIP
> > > > fi-bdw-gvtdvm: PASS -> SKIP
> > > > fi-ivb-3520m: PASS -> SKIP
> > > > fi-kbl-7500u: PASS -> SKIP
> > > > fi-cfl-8700k: PASS -> SKIP
> > > > fi-whl-u: PASS -> SKIP
> > > > fi-pnv-d510: PASS -> SKIP
> > > > fi-snb-2520m: PASS -> SKIP
> > > > fi-cfl-s3: PASS -> SKIP
> > > > {fi-skl-iommu}: PASS -> SKIP
> > > > fi-cfl-guc: PASS -> SKIP
> > > > fi-byt-n2820: PASS -> SKIP
> > > > fi-skl-6770hq: PASS -> SKIP
> > > > fi-elk-e7500: PASS -> SKIP
> > > > fi-ilk-650: PASS -> SKIP
> > >
> > >
> > > Hey,
> > >
> > > The patch makes overall sense and thanks for sending it.
> > >
> > > The test result changes seen above suggest that something is off with
> > > the logic in the patch or flags for the tests.
> > >
> > > There are more of those in the IGT run (NOTE: if you see +43, that means
> > > that 43 changes like that were observerd).
> > >
> > > They were passing before now they are skipping. Please investigate that.
> >
> > First of all, sorry for the long time to take a look at this.
> >
> > I tried to understand the skipping problem by investigating the test
> > “basic-flip-vs-wf_vblank()” in “kms_flip.c”, and I replicated the issue
> > in my local machine running i915 driver. Without my patch, the
> > "flip-vs-wf_vblank" worked like a charm; however, after applied my patch
> > I got the following log:
> >
> > IGT-Version: 1.23-g8d81c2c2 (x86_64) (Linux: 4.20.0-arch1-1-ARCH x86_64)
> > Using monotonic timestamps
> > Starting subtest: basic-flip-vs-wf_vblank
> > Beginning basic-flip-vs-wf_vblank on pipe A, connector eDP-1
> > 1920x1080 60 1920 2028 2076 2100 1080 1090 1100 1126 0xa 0x48 142000
> > Expected frametime: 16652us; measured 16652.4us +- 5.726us accuracy 0.10%
> >
> > basic-flip-vs-wf_vblank on pipe A, connector eDP-1: PASSED
> >
> > Beginning basic-flip-vs-wf_vblank on pipe B, connector eDP-1
> > 1920x1080 60 1920 2028 2076 2100 1080 1090 1100 1126 0xa 0x48 142000
> > Test requirement not met in function run_test_on_crtc_set, file ../tests/kms_flip.c:1317:
> > Test requirement: vblank
> > There is no Vblank
> > Last errno: 22, Invalid argument
> > Subtest basic-flip-vs-wf_vblank: SKIP (4.441s)
> >
> > The test was skipped because we get EINVAL (-22) which means many things
> > in this case (I'll detail this ahead). To better understand the problem,
> > I highlighted the function that I used in the patch for checking if a
> > driver has or not vblank support:
> >
> > +bool kms_has_vblank(int fd)
> > +{
> > + drmVBlank dummy_vbl;
> > +
> > + memset(&dummy_vbl, 0, sizeof(drmVBlank));
> > + dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
> > +
> > + return drmWaitVBlank(fd, &dummy_vbl) == 0;
> > +}
> >
> > As you can see in the return line, we just check if “drmWaitVBlank()”
> > returns 0 and this is the problem. If you take a look at
> > “drivers/gpu/drm/drm_vblank.c” (kernel), the function
> > “drm_wait_vblank_ioctl()”, you'll notice that the only error returned to
> > the user space is “EINVAL”; which includes the check if a driver support
> > vblank or not (“dev->irq_enabled”).
> >
> > IMHO the correct solution is changing the “drm_wait_vblank_ioctl()” to
> > return “EOPNOTSUPP” in the case that a driver does not support vblank. I
> > I already sent a patch to change it, and you can see the discussion here:
> >
> > https://lkml.org/lkml/2018/10/15/609
> >
> > So... for checking my point, I applied the above patch and made a change
> > in “kms_has_vblank” as follows:
> >
> > +bool kms_has_vblank(int fd)
> > +{
> > + drmVBlank dummy_vbl;
> > +
> > + memset(&dummy_vbl, 0, sizeof(drmVBlank));
> > + dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
> > +
> > + drmWaitVBlank(fd, &dummy_vbl);
> > + return (errno != EOPNOTSUPP);
> > +}
> >
> > After that, the “basic-flip-vs-wf_vblank()” worked as expected. I tested
> > in i915, Bosch (no vblank), and VKMS. Everything looks right now.
> >
> > So... make sense? Do you see another alternative to solve this problem?
>
> Yup. Even with your patch we can still get -EINVAL (when the crtc is off),
> so merging the EOPNOTSUPP patches first, then this here, and only skipping
> if EOPNOTSUPP is the only way to go I think.
>
> The above is probably good in a comment in kms_has_vblank().
Hi, thanks for the review.
I will prepare a V3 with this fix.
> Btw I thought the EOPNOTSUPP was merged already, why is it stuck? Or
> should retesting this fix things?
About the patch, we have some discussion about it because two tests failed
in the CI. Probably I have to send another patch for making the tests
correctly handling EOPNOTSUPP.
Thanks.
> -Daniel
> >
> > Additionally, it is weird for me that “drm_wait_vblank_ioctl()” returns
> > EINVAL in some case related to “basic-flip-vs-wf_vblank()”.
> >
> > Best Regards
> >
> > > --
> > > Cheers,
> > > Arek
> > >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
More information about the igt-dev
mailing list