Re: [igt-dev] ✗ Fi.CI.BAT: failure for tests: Add variable refresh rate tests (rev2)

[Petri Latvala <petri.latvala@intel.com>]

On Fri, Jan 25, 2019 at 05:21:41PM +0000, Kazlauskas, Nicholas wrote:
> On 1/25/19 11:43 AM, Saarinen, Jani wrote:
> > HI,
> > 
> >> -----Original Message-----
> >> From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of
> >> Kazlauskas, Nicholas
> >> Sent: perjantai 25. tammikuuta 2019 17.21
> >> To: igt-dev@lists.freedesktop.org; Latvala, Petri <petri.latvala@intel.com>
> >> Subject: Re: [igt-dev] ✗ Fi.CI.BAT: failure for tests: Add variable refresh rate tests
> >> (rev2)
> >>
> >> On 1/25/19 4:57 AM, Petri Latvala wrote:
> >>> On Thu, Jan 10, 2019 at 11:09:09AM +0000, Patchwork wrote:
> >>>> == Series Details ==
> >>>>
> >>>> Series: tests: Add variable refresh rate tests (rev2)
> >>>> URL   : https://patchwork.freedesktop.org/series/54960/
> >>>> State : failure
> >>>>
> >>>> == Summary ==
> >>>>
> >>>> CI Bug Log - changes from IGT_4758 -> IGTPW_2212
> >>>> ====================================================
> >>>>
> >>>> Summary
> >>>> -------
> >>>>
> >>>>     **FAILURE**
> >>>>
> >>>>     Serious unknown changes coming with IGTPW_2212 absolutely need to be
> >>>>     verified manually.
> >>>>
> >>>>     If you think the reported changes have nothing to do with the changes
> >>>>     introduced in IGTPW_2212, 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/54960/revisions/2/
> >>>>
> >>>> Possible new issues
> >>>> -------------------
> >>>>
> >>>>     Here are the unknown changes that may have been introduced in
> >> IGTPW_2212:
> >>>>
> >>>> ### IGT changes ###
> >>>>
> >>>> #### Possible regressions ####
> >>>>
> >>>>     * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
> >>>>       - fi-pnv-d510:        PASS -> FAIL +19
> >>>>
> >>>>     * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
> >>>>       - fi-blb-e6850:       PASS -> FAIL +18
> >>>>
> >>>>     * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
> >>>>       - fi-elk-e7500:       PASS -> FAIL +19
> >>>>
> >>>>     * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
> >>>>       - fi-gdg-551:         PASS -> FAIL +19
> >>>>
> >>>>     * igt@kms_pipe_crc_basic@read-crc-pipe-a:
> >>>>       - fi-bwr-2160:        PASS -> FAIL +19
> >>>>
> >>>
> >>>
> >>> Just about all of these failures are this:
> >>>
> >>>
> >>> (kms_cursor_legacy:2598) igt_kms-DEBUG: display: commit {
> >>> (kms_cursor_legacy:2598) igt_kms-CRITICAL: Test assertion failure function
> >> igt_pipe_commit, file ../lib/igt_kms.c:2780:
> >>> (kms_cursor_legacy:2598) igt_kms-CRITICAL: Failed assertion:
> >>> pipe->props[i]
> >>> (kms_cursor_legacy:2598) igt_core-INFO: Stack trace:
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #0 ../lib/igt_core.c:1472
> >> __igt_fail_assert()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #1 ../lib/igt_kms.c:2782
> >> igt_pipe_commit()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #2 ../lib/igt_kms.c:3313
> >> do_display_commit()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #3 ../lib/igt_kms.c:3428
> >> igt_display_commit2()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #4 ../tests/kms_cursor_legacy.c:522
> >> basic_flip_cursor()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #5
> >> ../tests/kms_cursor_legacy.c:1491 __real_main1358()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #6
> >> ../tests/kms_cursor_legacy.c:1358 main()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #7 ../csu/libc-start.c:344
> >> __libc_start_main()
> >>> (kms_cursor_legacy:2598) igt_core-INFO:   #8 [_start+0x2a]
> >>>
> >>
> >> I think that's expected.
> >>
> >> The vrr_enabled property is a default one in the kernel for 5.0+ but doesn't exist
> >> before that. Since it's default and could affect other test results if left enabled
> >> from another test I chose to add it to the list of default properties being reset. So
> >> it was probably missing the property in those tests and failing.
> >>
> >> I don't know which version of the kernel was running the test but I don't think it
> >> was 5.0 at least (considering the date it ran).
> > Well...
> > https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2212/fi-elk-e7500/boot0.log
> > Linux version 5.0.0-rc1-CI-CI_DRM_5390+
> 
> Oh, thanks! I didn't know there was an easy way to view the actual dmesg 
> log.
> 
> However, that assertion is definitely indicating that one of the pipe 
> properties (likely vrr_enabled) doesn't have a valid prop_id (likely 
> because it wasn't found).
> 
> FWIW I don't see these failures locally when testing with the patch in 
> the tree, but I'm also still on a 4.20 tree with the VRR patches 
> applied. I'm pretty sure the VRR patches were targeting 5.0 at least so 
> those patches should be in...


All the machines with the fails were old and for gen < 5 we don't have
DRIVER_ATOMIC. vrr_enabled is an atomic-only prop?

We support testing old kernels as well so some effort needs to be put
towards that anyway. Clearing the vrr prop when resetting to default
is good, but sprinkle some igt_*_has_prop() around it.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev