[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:53:42)
> On 03/18, Chris Wilson wrote:
> > 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.
> 
> Nice! Thanks for your answer.
> 
> So, I suppose that use DRM_VBLANK_RELATIVE is enough for this case,
> right?

Yes, that should return immediately both on success and on failure.
-Chris