[igt-dev] [PATCH i-g-t] tests/kms_plane: survive cdclk caused modeset
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Wed Apr 8 19:08:31 UTC 2020
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.
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> 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
>>>>>>>>
>>>>>>
>>>>>
>>>
>
More information about the igt-dev
mailing list