* [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank
@ 2019-06-05 2:30 Rodrigo Siqueira
2019-06-06 11:18 ` Ser, Simon
0 siblings, 1 reply; 4+ messages in thread
From: Rodrigo Siqueira @ 2019-06-05 2:30 UTC (permalink / raw)
To: Simon Ser, Petri Latvala, Arkadiusz Hiler, Chris Wilson,
Daniel Vetter
Cc: igt-dev, intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 4773 bytes --]
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
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 <rodrigosiqueiramelo@gmail.com>
---
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]);
--
2.21.0
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank
2019-06-05 2:30 [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank Rodrigo Siqueira
@ 2019-06-06 11:18 ` Ser, Simon
2019-06-06 15:28 ` Rodrigo Siqueira
0 siblings, 1 reply; 4+ messages in thread
From: Ser, Simon @ 2019-06-06 11:18 UTC (permalink / raw)
To: Hiler, Arkadiusz, daniel.vetter@ffwll.ch, Latvala, Petri,
rodrigosiqueiramelo@gmail.com, chris@chris-wilson.co.uk
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
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 <rodrigosiqueiramelo@gmail.com>
> ---
> 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank
2019-06-06 11:18 ` Ser, Simon
@ 2019-06-06 15:28 ` Rodrigo Siqueira
2019-06-07 10:43 ` Ser, Simon
0 siblings, 1 reply; 4+ messages in thread
From: Rodrigo Siqueira @ 2019-06-06 15:28 UTC (permalink / raw)
To: Ser, Simon
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
igt-dev@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 6231 bytes --]
On 06/06, Ser, Simon wrote:
> 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.
Hi Simon,
Thank you for your review.
About the changelog message, I'll update it to:
Skip seq number checking and busy flip if the device doesn't support vblank
Can I add your Reviewed-by before applying this patch?
> > 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 <rodrigosiqueiramelo@gmail.com>
> > ---
> > 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]);
--
Rodrigo Siqueira
https://siqueira.tech
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank
2019-06-06 15:28 ` Rodrigo Siqueira
@ 2019-06-07 10:43 ` Ser, Simon
0 siblings, 0 replies; 4+ messages in thread
From: Ser, Simon @ 2019-06-07 10:43 UTC (permalink / raw)
To: rodrigosiqueiramelo@gmail.com
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
igt-dev@lists.freedesktop.org
On Thu, 2019-06-06 at 12:28 -0300, Rodrigo Siqueira wrote:
> On 06/06, Ser, Simon wrote:
> > 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.
>
> Hi Simon,
>
> Thank you for your review.
>
> About the changelog message, I'll update it to:
>
> Skip seq number checking and busy flip if the device doesn't support vblank
>
> Can I add your Reviewed-by before applying this patch?
Yes!
Reviewed-by: Simon Ser <simon.ser@intel.com>
> > > 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 <rodrigosiqueiramelo@gmail.com>
> > > ---
> > > 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-07 10:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-05 2:30 [PATCH V8 i-g-t] tests/kms_flip: Skip VBlank tests in modules without VBlank Rodrigo Siqueira
2019-06-06 11:18 ` Ser, Simon
2019-06-06 15:28 ` Rodrigo Siqueira
2019-06-07 10:43 ` Ser, Simon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox