On Tue, 2019-06-04 at 23:30 -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).
> 
> V7: Changes since V6
>  - Skip TEST_DPMS with TEST_MODESET and TEST_EBUSY if the device does
>    not support vblank

Hmm, I'm not sure I understand this line. If I'm reading this correctly
it refers to this line:

    if (!(o->flags & (TEST_DPMS | TEST_MODESET | TEST_NO_VBLANK)))

So the check was previously skipped on TEST_DPMS and TEST_MODESET. It's
now skipped on TEST_NO_VBLANK too. So I'm a little confused with the
changelog line: should it say "skip seq number checking it the device
doesn't support vblank"?

Am I missing something?

Apart from this, the patch itself looks good to me.

> V6: Set errno to zero before call drmWaitVBlank() (Chris Wilson)
> 
> V5: Drop the DRM_VBLANK_NEXTONMISS (Chris Wilson)
> 
> 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 <[email protected]>
> ---
>  lib/igt_kms.c    | 21 +++++++++++++++++++++
>  lib/igt_kms.h    |  2 ++
>  tests/kms_flip.c | 24 +++++++++++++++++++++++-
>  3 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d7d711a7..8a465f67 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1673,6 +1673,27 @@ void igt_assert_plane_visible(int fd, enum pipe pipe, 
> int plane_index, bool visi
>       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;
> +
> +     errno = 0;
> +     drmWaitVBlank(fd, &dummy_vbl);
> +     return (errno != EOPNOTSUPP);
> +}
> +
>  /*
>   * A small modeset API
>   */
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4ac28131..5b5cf274 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -229,6 +229,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, int plane_index, 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 d7c1f9cf..2a158d97 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 | TEST_EBUSY;
> +
> +     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;
> @@ -494,7 +507,7 @@ 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);
> @@ -1176,6 +1189,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 RUN_TEST:
> @@ -1259,6 +1273,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]);
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to