[igt-dev] [PATCH i-g-t] tests/kms_cursor_legacy: Remove vblank counter check immediately after flip_nonblocking

[Rob Clark]

On Fri, Dec 17, 2021 at 4:51 AM Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
>
> On Fri, Dec 10, 2021 at 09:20:06AM -0800, Jessica Zhang wrote:
> > Currently, there is a check that the vblank counter shouldn't change after
> > a nonblocking page flip and DRM_IOCTL_MODE_CURSOR call. This assumes
> > that ioctl is performed before the commit from the nonblocking flip is
> > dequeued.
> >
> > The DRM_IOCTL_MODE_CURSOR callstack is as such:
> >       drm_mode_cursor_ioctl
> >       drm_mode_cursor_common
> >       drm_mode_cursor_universal
> >       __setplane_atomic
> >       drm_atomic_helper_update_plane
> >       drm_atomic_commit
> >
> > Becuase the commit work queue is scheduled earlier than
> > DRM_CURSOR_IOCTL, it will finish before the ioctl. Thus, we can't
> > assume that the current vblank count will match vblank_start after the
> > cursor ioctl finishes.
>
> Your driver is broken. The legacy cursor ioctl is expected to
> not get stuck behind the page flip.

Well, perhaps the atomic helpers are broken in that case?  But I don't
really agree that the legacy cursor was *that* well defined that we
can call this a driver bug.

What I'd propose instead is to do reverse the order of the pageflip
and cursor updates, so the cursor updates don't get stuck behind the
pageflip.

BR,
-R

>
> >
> > Tested-on: Chromebook Lazor (Trogdor)
> >
> > Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> > ---
> >  tests/kms_cursor_legacy.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> > index d50155e8..82c01bc8 100644
> > --- a/tests/kms_cursor_legacy.c
> > +++ b/tests/kms_cursor_legacy.c
> > @@ -1307,8 +1307,6 @@ static void flip_vs_cursor_crc(igt_display_t *display, bool atomic)
> >               flip_nonblocking(display, pipe, atomic, &fb_info, NULL);
> >               do_ioctl(display->drm_fd, DRM_IOCTL_MODE_CURSOR, &arg[i]);
> >
> > -             igt_assert_eq(kmstest_get_vblank(display->drm_fd, pipe, 0), vblank_start);
> > -
> >               igt_set_timeout(1, "Stuck page flip");
> >               igt_ignore_warn(read(display->drm_fd, &vbl, sizeof(vbl)));
> >               igt_reset_timeout();
> > --
> > 2.31.0
>
> --
> Ville Syrjälä
> Intel