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

[Rodrigo Siqueira]

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.
However, I was not 100% sure about this decision since this code will
only test the first if condition inside drm_wait_vblank_ioctl().
Additionally, I could not find a good way to test the drmWaitVBlank()
with and without this flag.

Please, correct if I'm wrong. I'm fighting to understand the vblank API.

Thanks for you review :)

> > +
> > +	drmWaitVBlank(fd, &dummy_vbl);
> > +	return (errno != EOPNOTSUPP);
> > +}
> > +
> >  /*
> >   * A small modeset API
> >   */
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 407f3d64..1cc15eea 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -230,6 +230,8 @@ void kmstest_wait_for_pageflip(int fd);
> >  unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int flags);
> >  void igt_assert_plane_visible(int fd, enum pipe pipe, bool visibility);
> >  
> > +bool kms_has_vblank(int fd);
> > +
> >  /*
> >   * A small modeset API
> >   */
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > index dfa5a69e..614efc5b 100755
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -71,6 +71,7 @@
> >  #define TEST_SUSPEND		(1 << 26)
> >  #define TEST_BO_TOOBIG		(1 << 28)
> >  
> > +#define TEST_NO_VBLANK		(1 << 29)
> >  #define TEST_BASIC		(1 << 30)
> >  
> >  #define EVENT_FLIP		(1 << 0)
> > @@ -126,6 +127,18 @@ struct event_state {
> >  	int seq_step;
> >  };
> >  
> > +static bool vblank_dependence(int flags)
> > +{
> > +	int vblank_flags = TEST_VBLANK | TEST_VBLANK_BLOCK |
> > +			   TEST_VBLANK_ABSOLUTE | TEST_VBLANK_EXPIRED_SEQ |
> > +			   TEST_CHECK_TS | TEST_VBLANK_RACE;
> > +
> > +	if (flags & vblank_flags)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static float timeval_float(const struct timeval *tv)
> >  {
> >  	return tv->tv_sec + tv->tv_usec / 1000000.0f;
> > @@ -1177,6 +1190,7 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> >  	unsigned bo_size = 0;
> >  	uint64_t tiling;
> >  	int i;
> > +	int vblank = true;
> >  
> >  	switch (crtc_count) {
> >  	case RUN_TEST:
> > @@ -1260,6 +1274,14 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> >  	}
> >  	igt_assert(fb_is_bound(o, o->fb_ids[0]));
> >  
> > +	vblank = kms_has_vblank(drm_fd);
> > +	if (!vblank) {
> > +		if (vblank_dependence(o->flags))
> > +			igt_require_f(vblank, "There is no VBlank\n");
> > +		else
> > +			o->flags |= TEST_NO_VBLANK;
> > +	}
> > +
> >  	/* quiescent the hw a bit so ensure we don't miss a single frame */
> >  	if (o->flags & TEST_CHECK_TS)
> >  		calibrate_ts(o, crtc_idxs[0]);
> > -- 
> > 2.21.0
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190318/d51723b7/attachment.sig>