[i-g-t] Skip VBlank tests in modules without VBlank
diff mbox series

Message ID 20180820235445.czxuegylrnnxqrzn@smtp.gmail.com
State New
Headers show
Series
  • [i-g-t] Skip VBlank tests in modules without VBlank
Related show

Commit Message

Rodrigo Siqueira Aug. 20, 2018, 11:54 p.m. UTC
The kms_flip test does not support drivers without VBlank which exclude
some virtual drivers. This patch adds a function that checks if a module
has a VBlank or not; if a module has VBlank than kms_flip will execute
all the VBlank tests, otherwise, VBlank tests will be skipped.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 lib/igt_aux.c    | 14 ++++++++++++++
 lib/igt_aux.h    |  2 ++
 tests/kms_flip.c | 26 ++++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

Comments

Chris Wilson Aug. 21, 2018, 10:16 a.m. UTC | #1
Quoting Rodrigo Siqueira (2018-08-21 00:54:45)
> The kms_flip test does not support drivers without VBlank which exclude
> some virtual drivers. This patch adds a function that checks if a module
> has a VBlank or not; if a module has VBlank than kms_flip will execute
> all the VBlank tests, otherwise, VBlank tests will be skipped.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  lib/igt_aux.c    | 14 ++++++++++++++
>  lib/igt_aux.h    |  2 ++
>  tests/kms_flip.c | 26 ++++++++++++++++++++++++--
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 1250d5c5..da5be4bb 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -564,6 +564,20 @@ bool igt_aub_dump_enabled(void)
>         return dump_aub;
>  }
>  
> +bool igt_there_is_vblank(int drm_fd)

I would suggest breaking out in the kms namespace,

bool kms_has_vblank(int fd);

> +{
> +       drmVBlank dummy_vbl;
> +       int ret;
> +

Either memset() the rest or use a named initializer.

> +       dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
> +       ret = drmWaitVBlank(drm_fd, &dummy_vbl);

return drmWaitVblank() == 0;
-Chris
Rodrigo Siqueira Aug. 21, 2018, 2:29 p.m. UTC | #2
On 08/21, Chris Wilson wrote:
> Quoting Rodrigo Siqueira (2018-08-21 00:54:45)
> > The kms_flip test does not support drivers without VBlank which exclude
> > some virtual drivers. This patch adds a function that checks if a module
> > has a VBlank or not; if a module has VBlank than kms_flip will execute
> > all the VBlank tests, otherwise, VBlank tests will be skipped.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  lib/igt_aux.c    | 14 ++++++++++++++
> >  lib/igt_aux.h    |  2 ++
> >  tests/kms_flip.c | 26 ++++++++++++++++++++++++--
> >  3 files changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 1250d5c5..da5be4bb 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -564,6 +564,20 @@ bool igt_aub_dump_enabled(void)
> >         return dump_aub;
> >  }
> >  
> > +bool igt_there_is_vblank(int drm_fd)
> 
> I would suggest breaking out in the kms namespace,
> 
> bool kms_has_vblank(int fd);
Hi,

Do you mean to move this function to igt_kms, right? If so, how about
kmstest_has_vblank.

> > +{
> > +       drmVBlank dummy_vbl;
> > +       int ret;
> > +
> 
> Either memset() the rest or use a named initializer.
> 
> > +       dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
> > +       ret = drmWaitVBlank(drm_fd, &dummy_vbl);
> 
> return drmWaitVblank() == 0;
> -Chris

Thanks for the feedback! I will apply the suggestions.
Chris Wilson Aug. 21, 2018, 3:19 p.m. UTC | #3
Quoting Rodrigo Siqueira (2018-08-21 15:29:03)
> On 08/21, Chris Wilson wrote:
> > Quoting Rodrigo Siqueira (2018-08-21 00:54:45)
> > > The kms_flip test does not support drivers without VBlank which exclude
> > > some virtual drivers. This patch adds a function that checks if a module
> > > has a VBlank or not; if a module has VBlank than kms_flip will execute
> > > all the VBlank tests, otherwise, VBlank tests will be skipped.
> > > 
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > ---
> > >  lib/igt_aux.c    | 14 ++++++++++++++
> > >  lib/igt_aux.h    |  2 ++
> > >  tests/kms_flip.c | 26 ++++++++++++++++++++++++--
> > >  3 files changed, 40 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > > index 1250d5c5..da5be4bb 100644
> > > --- a/lib/igt_aux.c
> > > +++ b/lib/igt_aux.c
> > > @@ -564,6 +564,20 @@ bool igt_aub_dump_enabled(void)
> > >         return dump_aub;
> > >  }
> > >  
> > > +bool igt_there_is_vblank(int drm_fd)
> > 
> > I would suggest breaking out in the kms namespace,
> > 
> > bool kms_has_vblank(int fd);
> Hi,
> 
> Do you mean to move this function to igt_kms, right? If so, how about
> kmstest_has_vblank.

Yes, start igt_kms.c to avoid letting igt_aux.c keep growing. (Its
mostly for unstructured miscellany). Personally, I think since this is
talking to the kernel and not our own intermediary, kms_has_vblank()
is more appropriate than kmstest_foo.
-Chris
Rodrigo Siqueira Aug. 21, 2018, 5:04 p.m. UTC | #4
On 08/21, Chris Wilson wrote:
> Quoting Rodrigo Siqueira (2018-08-21 15:29:03)
> > On 08/21, Chris Wilson wrote:
> > > Quoting Rodrigo Siqueira (2018-08-21 00:54:45)
> > > > The kms_flip test does not support drivers without VBlank which exclude
> > > > some virtual drivers. This patch adds a function that checks if a module
> > > > has a VBlank or not; if a module has VBlank than kms_flip will execute
> > > > all the VBlank tests, otherwise, VBlank tests will be skipped.
> > > > 
> > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > ---
> > > >  lib/igt_aux.c    | 14 ++++++++++++++
> > > >  lib/igt_aux.h    |  2 ++
> > > >  tests/kms_flip.c | 26 ++++++++++++++++++++++++--
> > > >  3 files changed, 40 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > > > index 1250d5c5..da5be4bb 100644
> > > > --- a/lib/igt_aux.c
> > > > +++ b/lib/igt_aux.c
> > > > @@ -564,6 +564,20 @@ bool igt_aub_dump_enabled(void)
> > > >         return dump_aub;
> > > >  }
> > > >  
> > > > +bool igt_there_is_vblank(int drm_fd)
> > > 
> > > I would suggest breaking out in the kms namespace,
> > > 
> > > bool kms_has_vblank(int fd);
> > Hi,
> > 
> > Do you mean to move this function to igt_kms, right? If so, how about
> > kmstest_has_vblank.
> 
> Yes, start igt_kms.c to avoid letting igt_aux.c keep growing. (Its
> mostly for unstructured miscellany). Personally, I think since this is
> talking to the kernel and not our own intermediary, kms_has_vblank()
> is more appropriate than kmstest_foo.

I got it now! Thanks! :)
I will work on the second version

> -Chris

Patch
diff mbox series

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 1250d5c5..da5be4bb 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -564,6 +564,20 @@  bool igt_aub_dump_enabled(void)
 	return dump_aub;
 }
 
+bool igt_there_is_vblank(int drm_fd)
+{
+	drmVBlank dummy_vbl;
+	int ret;
+
+	dummy_vbl.request.type = DRM_VBLANK_ABSOLUTE;
+	ret = drmWaitVBlank(drm_fd, &dummy_vbl);
+
+	if (ret < 0)
+		return false;
+
+	return true;
+}
+
 /* other helpers */
 /**
  * igt_exchange_int:
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index ef89faa9..933055e8 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -119,6 +119,8 @@  bool igt_check_boolean_env_var(const char *env_var, bool default_value);
 
 bool igt_aub_dump_enabled(void);
 
+bool igt_there_is_vblank(int fd);
+
 /* suspend/hibernate and auto-resume system */
 
 /**
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 393d690a..770fa5f7 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -73,6 +73,7 @@ 
 #define TEST_TS_CONT		(1 << 27)
 #define TEST_BO_TOOBIG		(1 << 28)
 
+#define TEST_NO_VBLANK		(1 << 29)
 #define TEST_BASIC		(1 << 30)
 
 #define EVENT_FLIP		(1 << 0)
@@ -125,6 +126,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_TS_CONT | 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;
@@ -493,11 +506,11 @@  static void check_state(const struct test_output *o, const struct event_state *e
 	/* check only valid if no modeset happens in between, that increments by
 	 * (1 << 23) on each step. This bounding matches the one in
 	 * DRM_IOCTL_WAIT_VBLANK. */
-	if (!(o->flags & (TEST_DPMS | TEST_MODESET)))
+	if (!(o->flags & (TEST_DPMS | TEST_MODESET | TEST_NO_VBLANK))) {
 		igt_assert_f(es->current_seq - (es->last_seq + o->seq_step) <= 1UL << 23,
 			     "unexpected %s seq %u, should be >= %u\n",
 			     es->name, es->current_seq, es->last_seq + o->seq_step);
-
+	}
 	/* Check that the vblank frame didn't wrap unexpectedly. */
 	if (o->flags & TEST_TS_CONT) {
 		/* Ignore seq_step here since vblank waits time out immediately
@@ -1204,6 +1217,7 @@  static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
 	unsigned bo_size = 0;
 	uint64_t tiling;
 	int i;
+	bool vblank = true;
 
 	switch (crtc_count) {
 	case 1:
@@ -1297,6 +1311,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 = igt_there_is_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]);