[igt-dev] ✓ Fi.CI.BAT: success for Skip VBlank tests in modules without VBlank (rev2)
Daniel Vetter
daniel at ffwll.ch
Mon Jan 14 10:19:32 UTC 2019
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().
Btw I thought the EOPNOTSUPP was merged already, why is it stuck? Or
should retesting this fix things?
-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