[igt-dev] [PATCH V5 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank

[Chris Wilson]

Quoting Rodrigo Siqueira (2019-03-18 21:35:44)
> On 03/18, Ville Syrjälä wrote:
> > On Sat, Mar 16, 2019 at 11:00:23AM -0300, Rodrigo Siqueira wrote:
> > > The kms_flip test relies on VBlank support, and this situation may
> > > exclude some virtual drivers to take advantage of this set of tests.
> > > This commit adds a mechanism that checks if a module has VBlank. If the
> > > target module has VBlank support, kms_flip will run all the VBlank
> > > tests; otherwise, the VBlank tests will be skipped. Additionally, this
> > > commit improves the test coverage by checks if the function
> > > drmWaitVBlank() returns EOPNOTSUPP (i.e., no VBlank support).
> > > 
> > > V4: Replace DRM_VBLANK_ABSOLUTE by DRM_VBLANK_RELATIVE and
> > > DRM_VBLANK_NEXTONMISS
> > > 
> > > V3: Add documentation (Daniel Vetter)
> > > 
> > > V2: Add new branch coverage to check if VBlank is enabled or not and
> > > update commit message
> > > 
> > > V1: Chris Wilson
> > >   - Change function name from igt_there_is_vblank to kms_has_vblank
> > >   - Move vblank function check from igt_aux to igt_kms
> > >   - Utilizes memset in dummy_vbl variable
> > >   - Directly return the result of drmWaitVBlank()
> > > 
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>
> > > ---
> > >  lib/igt_kms.c    | 20 ++++++++++++++++++++
> > >  lib/igt_kms.h    |  2 ++
> > >  tests/kms_flip.c | 22 ++++++++++++++++++++++
> > >  3 files changed, 44 insertions(+)
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index e1eacc1e..1d2d7188 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -1655,6 +1655,26 @@ void igt_assert_plane_visible(int fd, enum pipe pipe, bool visibility)
> > >     igt_assert_eq(visible, visibility);
> > >  }
> > >  
> > > +/**
> > > + * kms_has_vblank:
> > > + * @fd: DRM fd
> > > + *
> > > + * Get the VBlank errno after an attempt to call drmWaitVBlank(). This
> > > + * function is useful for checking if a driver has support or not for VBlank.
> > > + *
> > > + * Returns: true if target driver has VBlank support, otherwise return false.
> > > + */
> > > +bool kms_has_vblank(int fd)
> > > +{
> > > +   drmVBlank dummy_vbl;
> > > +
> > > +   memset(&dummy_vbl, 0, sizeof(drmVBlank));
> > > +   dummy_vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_NEXTONMISS;
> > 
> > Why the NEXTONMISS?
> 
> I added this flag because I was suspecting that in case of any problem
> during the kms_has_vblank() execution the flag NEXTONMISS will wait for
> the next VBlank and avoid to generate problems in the subsequent tests.

That is true, and a valid use for using NEXTONMISS if you want to align
your code to the start of a vblank to avoid overrunning into the next
vblank.

However, that is not the purpose of kms_has_vblank()! Whose only purpose
is answer the question of whether the device has vblank support, and so
should be as quick and simple as possible, so the trivial query of the
current vblank counter.
-Chris