Re: [igt-dev] ✓ Fi.CI.BAT: success for Skip VBlank tests in modules without VBlank (rev2)

[Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>]

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@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?

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev