[igt-dev] [PATCH i-g-t] tests/kms_plane: survive cdclk caused modeset
Ville Syrjälä
ville.syrjala at linux.intel.com
Thu Apr 9 16:08:07 UTC 2020
On Wed, Apr 08, 2020 at 10:08:31PM +0300, Juha-Pekka Heikkila wrote:
> On 7.4.2020 20.42, Ville Syrjälä wrote:
> > On Tue, Apr 07, 2020 at 08:24:01PM +0300, Juha-Pekka Heikkila wrote:
> >> On 7.4.2020 20.10, Ville Syrjälä wrote:
> >>> On Tue, Apr 07, 2020 at 08:07:16PM +0300, Juha-Pekka Heikkila wrote:
> >>>> On 7.4.2020 19.22, Juha-Pekka Heikkila wrote:
> >>>>> On 7.4.2020 19.08, Ville Syrjälä wrote:
> >>>>>> On Tue, Apr 07, 2020 at 06:54:09PM +0300, Juha-Pekka Heikkila wrote:
> >>>>>>> On 7.4.2020 18.36, Ville Syrjälä wrote:
> >>>>>>>> On Tue, Apr 07, 2020 at 02:09:04PM +0300, Juha-Pekka Heikkila wrote:
> >>>>>>>>> This change will slow this test down a bit. In mid test starting
> >>>>>>>>> to use higher bpp pixel format (say 64bpp) can cause modeset.
> >>>>>>>>> Use blocking commit so there's wait for modeset to happen.
> >>>>>>>>
> >>>>>>>> We already wait for the event the next time around. So this
> >>>>>>>> doesn't make sense to me.
> >>>>>>>
> >>>>>>> There's those logs like this
> >>>>>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_470/fi-icl-guc/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html
> >>>>>>>
> >>>>>>>
> >>>>>>> where going to 64bpp pixel format will cause modeset and fail running
> >>>>>>> test.
> >>>>>>
> >>>>>> Hmm. Seems we don't have drm.debug=0x10 enabled so we don't see the
> >>>>>> actual debug message :( Anyways, the real bug seems to be the lack of
> >>>>>> ALLOW_MODESET.
> >>>>>
> >>>>> I'll try that and see how it changes situation. I originally had idea
> >>>>> modeset will block the flip and making this commit blocking would always
> >>>>> force things always into correct order as current test didn't fail every
> >>>>> time for me.
> >>>>
> >>>> Having that commit saying
> >>>>
> >>>> igt_display_commit_atomic(&data->display,
> >>>> DRM_MODE_ATOMIC_ALLOW_MODESET |
> >>>> DRM_MODE_ATOMIC_NONBLOCK |
> >>>> DRM_MODE_PAGE_FLIP_EVENT, NULL);
> >>>>
> >>>> I see test failing as before so it will not alone fix this issue. On my
> >>>> ICL box the test fail maybe 1/5 times, with the patch on list it never
> >>>> failed so far.
> >>>
> >>> Why does it fail?
> >>
> >>
> >> This is failure with ALLOW_MODESET
> >> ---
> >> (kms_plane:31238) igt_kms-DEBUG: display: }
> >> (kms_plane:31238) igt_kms-CRITICAL: Test assertion failure function
> >> igt_display_commit_atomic, file ../lib/igt_kms.c:3508:
> >> (kms_plane:31238) igt_kms-CRITICAL: Failed assertion: ret == 0
> >> (kms_plane:31238) igt_kms-CRITICAL: Last errno: 16, Device or resource busy
> >> (kms_plane:31238) igt_kms-CRITICAL: error: -16 != 0
> >> (kms_plane:31238) igt_core-INFO: Stack trace:
> >> (kms_plane:31238) igt_core-INFO: #0 ../lib/igt_core.c:1676
> >> __igt_fail_assert()
> >> (kms_plane:31238) igt_core-INFO: #1 [igt_display_commit_atomic+0x44]
> >> (kms_plane:31238) igt_core-INFO: #2 ../tests/kms_plane.c:599
> >> capture_format_crcs.constprop.14()
> >> (kms_plane:31238) igt_core-INFO: #3 ../tests/kms_plane.c:652
> >> test_format_plane_colors.constprop.13()
> >> (kms_plane:31238) igt_core-INFO: #4 ../tests/kms_plane.c:728
> >> test_pixel_formats.constprop.9()
> >> (kms_plane:31238) igt_core-INFO: #5 ../tests/kms_plane.c:950
> >> run_tests_for_pipe_plane.constprop.8()
> >> (kms_plane:31238) igt_core-INFO: #6 ../tests/kms_plane.c:1019
> >> __real_main1006()
> >> (kms_plane:31238) igt_core-INFO: #7 ../tests/kms_plane.c:1006 main()
> >> (kms_plane:31238) igt_core-INFO: #8 ../csu/libc-start.c:344
> >> __libc_start_main()
> >> (kms_plane:31238) igt_core-INFO: #9 [_start+0x2a]
> >> **** END ****
> >> Stack trace:
> >> #0 ../lib/igt_core.c:1676 __igt_fail_assert()
> >> #1 [igt_display_commit_atomic+0x44]
> >> #2 ../tests/kms_plane.c:599 capture_format_crcs.constprop.14()
> >> #3 ../tests/kms_plane.c:652 test_format_plane_colors.constprop.13()
> >> #4 ../tests/kms_plane.c:728 test_pixel_formats.constprop.9()
> >> #5 ../tests/kms_plane.c:950 run_tests_for_pipe_plane.constprop.8()
> >> #6 ../tests/kms_plane.c:1019 __real_main1006()
> >> #7 ../tests/kms_plane.c:1006 main()
> >> #8 ../csu/libc-start.c:344 __libc_start_main()
> >> #9 [_start+0x2a]
> >> Subtest pixel-format-pipe-A-planes-source-clamping: FAIL (22.543s)
> >
> > That doesn't tell anyone anything useful. Only kernel logs can help
> > diagnose the problem.
> >
> >
> >> --
> >>
> >> Failure is ebusy. It seems round where it kick me out is not always the
> >> same.
> >
> > Hmm. Sounds like we have a race between sending the event vs. submitting
> > a new commit :(
> >
> > Hmm. What wasn't there a patch from Daniel to convert modeset commits to
> > blocking to "fix" a race just like this? Ah, yeah it was EBUSY due to
> > multiple pipes and noblocking modesets. Do you have multiple pipes
> > enablad when it fails? That could certainly explain the race.
> >
>
> Quickly looking there shouldn't be multiple pipes enabled. Test is one
> of those simple loops that start with for_each_plane_on_pipe(..){..} and
> different pipes are run as different tests.
Can't see anything else returning -EBUSY except if flip_done is not
completed, and that one should be done before we send the event.
>
> >>
> >>>
> >>>>
> >>>>>
> >>>>>>
> >>>>>>> I was testing this on ICL and see errors randomly, on ci those
> >>>>>>> seem to be less random. Making this one commit blocking will cause
> >>>>>>> modeset to settle without interrupting test, at least on my ICL.
> >>>>>>>
> >>>>>>> If there's a way to sort those pixel formats according to bpp and start
> >>>>>>> with highest there's no need for this.
> >>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Fixes: https://gitlab.freedesktop.org/drm/intel/issues/1214
> >>>>>>>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> >>>>>>>>> ---
> >>>>>>>>> tests/kms_plane.c | 6 ++----
> >>>>>>>>> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> >>>>>>>>> index 805795cd..2324fb6e 100644
> >>>>>>>>> --- a/tests/kms_plane.c
> >>>>>>>>> +++ b/tests/kms_plane.c
> >>>>>>>>> @@ -569,12 +569,10 @@ static void capture_format_crcs(data_t *data,
> >>>>>>>>> enum pipe pipe,
> >>>>>>>>> if (data->display.is_atomic) {
> >>>>>>>>> /*
> >>>>>>>>> - * Use non-blocking commits to allow the next fb
> >>>>>>>>> - * to be prepared in parallel while the current fb
> >>>>>>>>> - * awaits to be latched.
> >>>>>>>>> + * Use blocking commit because there maybe
> >>>>>>>>> + * modeset when going to higher bpp pixel format.
> >>>>>>>>> */
> >>>>>>>>> igt_display_commit_atomic(&data->display,
> >>>>>>>>> - DRM_MODE_ATOMIC_NONBLOCK |
> >>>>>>>>> DRM_MODE_PAGE_FLIP_EVENT, NULL);
> >>>>>>>>> } else {
> >>>>>>>>> /*
> >>>>>>>>> --
> >>>>>>>>> 2.17.1
> >>>>>>>>>
> >>>>>>>>> _______________________________________________
> >>>>>>>>> igt-dev mailing list
> >>>>>>>>> igt-dev at lists.freedesktop.org
> >>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> >>>>>>>>
> >>>>>>
> >>>>>
> >>>
> >
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list