[igt-dev] [PATCH i-g-t] tests/kms_plane: survive cdclk caused modeset

[Ville Syrjälä]

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?

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