| Message ID | 20180820235445.czxuegylrnnxqrzn@smtp.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] Skip VBlank tests in modules without VBlank | expand |
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
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.
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
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
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]);
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(-)