* [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4.
@ 2016-04-25 15:05 robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes robert.foss
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: robert.foss @ 2016-04-25 15:05 UTC (permalink / raw)
To: marius.c.vlad, daniel.vetter, tomeu.vizoso; +Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Changes since v2:
- Rebased onto trunk which already contains kms_flip_event_leak changes.
Changes since v1:
- kms_vblank: Removed un-used members of data_t struct.
- Rename plane_counter to n_planes.
- Removed un-needed handling CURSOR plane location.
- Added names for additional planes in update kmstest_plane_name.
Robert Foss (4):
lib/igt_kms: Add support for up to 10 planes.
lib/igt_kms: Fix plane counting in igt_display_init.
lib/igt_kms: Switch to verbose assert.
kms_vblank: Switch from using crtc0 statically to explicitly setting
mode.
lib/igt_kms.c | 14 ++++-
lib/igt_kms.h | 11 +++-
tests/kms_vblank.c | 179 +++++++++++++++++++++++++++++++++++++++++------------
3 files changed, 161 insertions(+), 43 deletions(-)
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes.
2016-04-25 15:05 [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4 robert.foss
@ 2016-04-25 15:05 ` robert.foss
2016-04-26 11:47 ` Tomeu Vizoso
2016-04-25 15:05 ` [PATCH i-g-t v3 2/4] lib/igt_kms: Fix plane counting in igt_display_init robert.foss
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: robert.foss @ 2016-04-25 15:05 UTC (permalink / raw)
To: marius.c.vlad, daniel.vetter, tomeu.vizoso; +Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Increase the number of planes supported to 10.
kmstest_plane_name only previously supported 4 planes,
this patch adds support for up to 10 planes.
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
lib/igt_kms.c | 6 ++++++
lib/igt_kms.h | 11 ++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index ef24a49..36ecd4a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -320,6 +320,12 @@ const char *kmstest_plane_name(enum igt_plane plane)
[IGT_PLANE_1] = "plane1",
[IGT_PLANE_2] = "plane2",
[IGT_PLANE_3] = "plane3",
+ [IGT_PLANE_4] = "plane4",
+ [IGT_PLANE_5] = "plane5",
+ [IGT_PLANE_6] = "plane6",
+ [IGT_PLANE_7] = "plane7",
+ [IGT_PLANE_8] = "plane8",
+ [IGT_PLANE_9] = "plane9",
[IGT_PLANE_CURSOR] = "cursor",
};
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 5c83401..b3fe1b2 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -48,12 +48,21 @@ enum pipe {
};
const char *kmstest_pipe_name(enum pipe pipe);
-/* We namespace this enum to not conflict with the Android i915_drm.h */
+/**
+ * We namespace this enum to not conflict with the Android i915_drm.h
+ * IGT_PLANE_CURSOR is always the last plane.
+ */
enum igt_plane {
IGT_PLANE_1 = 0,
IGT_PLANE_PRIMARY = IGT_PLANE_1,
IGT_PLANE_2,
IGT_PLANE_3,
+ IGT_PLANE_4,
+ IGT_PLANE_5,
+ IGT_PLANE_6,
+ IGT_PLANE_7,
+ IGT_PLANE_8,
+ IGT_PLANE_9,
IGT_PLANE_CURSOR,
IGT_MAX_PLANES,
};
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH i-g-t v3 2/4] lib/igt_kms: Fix plane counting in igt_display_init.
2016-04-25 15:05 [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4 robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes robert.foss
@ 2016-04-25 15:05 ` robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 3/4] lib/igt_kms: Switch to verbose assert robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode robert.foss
3 siblings, 0 replies; 8+ messages in thread
From: robert.foss @ 2016-04-25 15:05 UTC (permalink / raw)
To: marius.c.vlad, daniel.vetter, tomeu.vizoso; +Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Fix issue where the plane counting fails due to the number and
configuration of planes being unlike the intel configuration.
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
lib/igt_kms.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 36ecd4a..5751913 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1289,6 +1289,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_plane_t *plane;
int p = IGT_PLANE_2;
int j, type;
+ uint8_t n_planes = 0;
pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
@@ -1336,8 +1337,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
break;
}
+ n_planes++;
plane->pipe = pipe;
plane->drm_plane = drm_plane;
+
if (is_atomic == 0) {
display->is_atomic = 1;
igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
@@ -1386,8 +1389,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
plane->is_cursor = true;
}
- /* planes = 1 primary, (p-1) sprites, 1 cursor */
- pipe->n_planes = p + 1;
+ pipe->n_planes = n_planes;
/* make sure we don't overflow the plane array */
igt_assert(pipe->n_planes <= IGT_MAX_PLANES);
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH i-g-t v3 3/4] lib/igt_kms: Switch to verbose assert.
2016-04-25 15:05 [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4 robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 2/4] lib/igt_kms: Fix plane counting in igt_display_init robert.foss
@ 2016-04-25 15:05 ` robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode robert.foss
3 siblings, 0 replies; 8+ messages in thread
From: robert.foss @ 2016-04-25 15:05 UTC (permalink / raw)
To: marius.c.vlad, daniel.vetter, tomeu.vizoso; +Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Switch igt_assert to igt_assert_lte to provide more diagnostic
information.
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
lib/igt_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5751913..99b6279 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1392,7 +1392,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
pipe->n_planes = n_planes;
/* make sure we don't overflow the plane array */
- igt_assert(pipe->n_planes <= IGT_MAX_PLANES);
+ igt_assert_lte(pipe->n_planes, IGT_MAX_PLANES);
}
/*
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode.
2016-04-25 15:05 [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4 robert.foss
` (2 preceding siblings ...)
2016-04-25 15:05 ` [PATCH i-g-t v3 3/4] lib/igt_kms: Switch to verbose assert robert.foss
@ 2016-04-25 15:05 ` robert.foss
2016-04-26 12:32 ` Tomeu Vizoso
3 siblings, 1 reply; 8+ messages in thread
From: robert.foss @ 2016-04-25 15:05 UTC (permalink / raw)
To: marius.c.vlad, daniel.vetter, tomeu.vizoso; +Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Previously crtc0 was statically used for VBLANK tests, but
that assumption is not valid for the VC4 platform.
Instead we're now explicitly setting the mode.
Also add support for testing all connected connectors during
the same test.
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
tests/kms_vblank.c | 179 +++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 140 insertions(+), 39 deletions(-)
diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 40ab6fd..970fefe 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -44,6 +44,14 @@
IGT_TEST_DESCRIPTION("Test speed of WaitVblank.");
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+ struct igt_fb primary_fb;
+ igt_output_t *output;
+ enum pipe pipe;
+} data_t;
+
static double elapsed(const struct timespec *start,
const struct timespec *end,
int loop)
@@ -51,75 +59,164 @@ static double elapsed(const struct timespec *start,
return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_nsec - start->tv_nsec)/1000)/loop;
}
-static bool crtc0_active(int fd)
+static uint32_t crtc_id_to_flag(uint32_t crtc_id)
{
- union drm_wait_vblank vbl;
+ if (crtc_id == 0)
+ return 0;
+ else if (crtc_id == 1)
+ return 1 | _DRM_VBLANK_SECONDARY;
+ else
+ return crtc_id << 1;
+}
- memset(&vbl, 0, sizeof(vbl));
- vbl.request.type = DRM_VBLANK_RELATIVE;
- return drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0;
+static bool prepare_crtc(data_t *data, igt_output_t *output)
+{
+ drmModeModeInfo *mode;
+ igt_display_t *display = &data->display;
+ igt_plane_t *primary;
+
+ /* select the pipe we want to use */
+ igt_output_set_pipe(output, data->pipe);
+ igt_display_commit(display);
+
+ if (!output->valid) {
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit(display);
+ return false;
+ }
+
+ /* create and set the primary plane fb */
+ mode = igt_output_get_mode(output);
+ igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ LOCAL_DRM_FORMAT_MOD_NONE,
+ 0.0, 0.0, 0.0,
+ &data->primary_fb);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+ igt_plane_set_fb(primary, &data->primary_fb);
+
+ igt_display_commit(display);
+
+ igt_wait_for_vblank(data->drm_fd, data->pipe);
+
+ return true;
+}
+
+static void cleanup_crtc(data_t *data, igt_output_t *output)
+{
+ igt_display_t *display = &data->display;
+ igt_plane_t *primary;
+
+ igt_remove_fb(data->drm_fd, &data->primary_fb);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+ igt_plane_set_fb(primary, NULL);
+
+ igt_output_set_pipe(output, PIPE_ANY);
+ igt_display_commit(display);
}
-static void accuracy(int fd)
+static void run_test(data_t *data, void (*testfunc)(data_t *, bool), bool boolean)
+{
+ igt_display_t *display = &data->display;
+ igt_output_t *output;
+ enum pipe p;
+ int valid_tests = 0;
+
+ for_each_connected_output(display, output) {
+ for_each_pipe(display, p) {
+ data->pipe = p;
+
+ if (!prepare_crtc(data, output))
+ continue;
+
+ valid_tests++;
+
+ igt_info("Beginning %s on pipe %s, connector %s\n",
+ igt_subtest_name(),
+ kmstest_pipe_name(data->pipe),
+ igt_output_name(output));
+
+ testfunc(data, boolean);
+
+ igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
+ igt_subtest_name(),
+ kmstest_pipe_name(data->pipe),
+ igt_output_name(output));
+
+ /* cleanup what prepare_crtc() has done */
+ cleanup_crtc(data, output);
+ }
+ }
+
+ igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+static void accuracy(data_t *data, bool busy)
{
union drm_wait_vblank vbl;
unsigned long target;
+ uint32_t crtc_id_flag;
int n;
memset(&vbl, 0, sizeof(vbl));
+ crtc_id_flag = crtc_id_to_flag(data->pipe);
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 1;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
target = vbl.reply.sequence + 60;
for (n = 0; n < 60; n++) {
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 1;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
- vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
+ vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_id_flag;
vbl.request.sequence = target;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
}
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 0;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
igt_assert_eq(vbl.reply.sequence, target);
for (n = 0; n < 60; n++) {
struct drm_event_vblank ev;
- igt_assert_eq(read(fd, &ev, sizeof(ev)), sizeof(ev));
+ igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
igt_assert_eq(ev.sequence, target);
}
}
-static void vblank_query(int fd, bool busy)
+static void vblank_query(data_t *data, bool busy)
{
union drm_wait_vblank vbl;
struct timespec start, end;
unsigned long sq, count = 0;
struct drm_event_vblank buf;
+ uint32_t crtc_id_flag;
memset(&vbl, 0, sizeof(vbl));
+ crtc_id_flag = crtc_id_to_flag(data->pipe);
if (busy) {
- vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+ vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
vbl.request.sequence = 72;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
}
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 0;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
sq = vbl.reply.sequence;
clock_gettime(CLOCK_MONOTONIC, &start);
do {
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 0;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
count++;
} while ((vbl.reply.sequence - sq) <= 60);
clock_gettime(CLOCK_MONOTONIC, &end);
@@ -128,35 +225,37 @@ static void vblank_query(int fd, bool busy)
busy ? "busy" : "idle", elapsed(&start, &end, count));
if (busy)
- igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+ igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
}
-static void vblank_wait(int fd, bool busy)
+static void vblank_wait(data_t *data, bool busy)
{
union drm_wait_vblank vbl;
struct timespec start, end;
unsigned long sq, count = 0;
struct drm_event_vblank buf;
+ uint32_t crtc_id_flag;
memset(&vbl, 0, sizeof(vbl));
+ crtc_id_flag = crtc_id_to_flag(data->pipe);
if (busy) {
- vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
+ vbl.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
vbl.request.sequence = 72;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
}
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type |= DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 0;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
sq = vbl.reply.sequence;
clock_gettime(CLOCK_MONOTONIC, &start);
do {
- vbl.request.type = DRM_VBLANK_RELATIVE;
+ vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
vbl.request.sequence = 1;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
count++;
} while ((vbl.reply.sequence - sq) <= 60);
clock_gettime(CLOCK_MONOTONIC, &end);
@@ -167,32 +266,34 @@ static void vblank_wait(int fd, bool busy)
elapsed(&start, &end, count));
if (busy)
- igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
+ igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
}
igt_main
{
- int fd;
+ data_t data;
igt_skip_on_simulation();
igt_fixture {
- fd = drm_open_driver(DRIVER_ANY);
- igt_require(crtc0_active(fd));
+ data.drm_fd = drm_open_driver(DRIVER_ANY);
+ kmstest_set_vt_graphics_mode();
+ igt_display_init(&data.display, data.drm_fd);
}
igt_subtest("accuracy")
- accuracy(fd);
+ run_test(&data, accuracy, false);
igt_subtest("query-idle")
- vblank_query(fd, false);
+ run_test(&data, vblank_query, false);
igt_subtest("query-busy")
- vblank_query(fd, true);
+ run_test(&data, vblank_query, true);
igt_subtest("wait-idle")
- vblank_wait(fd, false);
+ run_test(&data, vblank_wait, false);
igt_subtest("wait-busy")
- vblank_wait(fd, true);
+ run_test(&data, vblank_wait, true);
}
+
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes.
2016-04-25 15:05 ` [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes robert.foss
@ 2016-04-26 11:47 ` Tomeu Vizoso
0 siblings, 0 replies; 8+ messages in thread
From: Tomeu Vizoso @ 2016-04-26 11:47 UTC (permalink / raw)
To: robert.foss; +Cc: Intel Graphics Development, Daniel Vetter
On 25 April 2016 at 17:05, <robert.foss@collabora.com> wrote:
> From: Robert Foss <robert.foss@collabora.com>
>
> Increase the number of planes supported to 10.
>
> kmstest_plane_name only previously supported 4 planes,
> this patch adds support for up to 10 planes.
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
> lib/igt_kms.c | 6 ++++++
> lib/igt_kms.h | 11 ++++++++++-
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index ef24a49..36ecd4a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -320,6 +320,12 @@ const char *kmstest_plane_name(enum igt_plane plane)
> [IGT_PLANE_1] = "plane1",
> [IGT_PLANE_2] = "plane2",
> [IGT_PLANE_3] = "plane3",
> + [IGT_PLANE_4] = "plane4",
> + [IGT_PLANE_5] = "plane5",
> + [IGT_PLANE_6] = "plane6",
> + [IGT_PLANE_7] = "plane7",
> + [IGT_PLANE_8] = "plane8",
> + [IGT_PLANE_9] = "plane9",
> [IGT_PLANE_CURSOR] = "cursor",
> };
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5c83401..b3fe1b2 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -48,12 +48,21 @@ enum pipe {
> };
> const char *kmstest_pipe_name(enum pipe pipe);
>
> -/* We namespace this enum to not conflict with the Android i915_drm.h */
> +/**
> + * We namespace this enum to not conflict with the Android i915_drm.h
> + * IGT_PLANE_CURSOR is always the last plane.
> + */
> enum igt_plane {
> IGT_PLANE_1 = 0,
> IGT_PLANE_PRIMARY = IGT_PLANE_1,
> IGT_PLANE_2,
> IGT_PLANE_3,
> + IGT_PLANE_4,
> + IGT_PLANE_5,
> + IGT_PLANE_6,
> + IGT_PLANE_7,
> + IGT_PLANE_8,
> + IGT_PLANE_9,
> IGT_PLANE_CURSOR,
> IGT_MAX_PLANES,
> };
Looks good to me, though I would probably add the comment next to the
IGT_PLANE_CURSOR item.
Thanks,
Tomeu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode.
2016-04-25 15:05 ` [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode robert.foss
@ 2016-04-26 12:32 ` Tomeu Vizoso
2016-04-26 15:46 ` Robert Foss
0 siblings, 1 reply; 8+ messages in thread
From: Tomeu Vizoso @ 2016-04-26 12:32 UTC (permalink / raw)
To: robert.foss; +Cc: Intel Graphics Development, Daniel Vetter
On 25 April 2016 at 17:05, <robert.foss@collabora.com> wrote:
> From: Robert Foss <robert.foss@collabora.com>
>
> Previously crtc0 was statically used for VBLANK tests, but
> that assumption is not valid for the VC4 platform.
> Instead we're now explicitly setting the mode.
>
> Also add support for testing all connected connectors during
> the same test.
Is there any reason why this cannot be done in a subsequent patch?
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
> tests/kms_vblank.c | 179 +++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 140 insertions(+), 39 deletions(-)
>
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 40ab6fd..970fefe 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -44,6 +44,14 @@
>
> IGT_TEST_DESCRIPTION("Test speed of WaitVblank.");
>
> +typedef struct {
> + int drm_fd;
> + igt_display_t display;
> + struct igt_fb primary_fb;
> + igt_output_t *output;
> + enum pipe pipe;
> +} data_t;
> +
> static double elapsed(const struct timespec *start,
> const struct timespec *end,
> int loop)
> @@ -51,75 +59,164 @@ static double elapsed(const struct timespec *start,
> return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_nsec - start->tv_nsec)/1000)/loop;
> }
>
> -static bool crtc0_active(int fd)
> +static uint32_t crtc_id_to_flag(uint32_t crtc_id)
This seems to be pipe id, not the crtc id. Maybe this belongs to the library.
> {
> - union drm_wait_vblank vbl;
> + if (crtc_id == 0)
> + return 0;
> + else if (crtc_id == 1)
> + return 1 | _DRM_VBLANK_SECONDARY;
> + else
> + return crtc_id << 1;
> +}
>
> - memset(&vbl, 0, sizeof(vbl));
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> - return drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0;
> +static bool prepare_crtc(data_t *data, igt_output_t *output)
> +{
> + drmModeModeInfo *mode;
> + igt_display_t *display = &data->display;
> + igt_plane_t *primary;
> +
> + /* select the pipe we want to use */
> + igt_output_set_pipe(output, data->pipe);
> + igt_display_commit(display);
> +
> + if (!output->valid) {
> + igt_output_set_pipe(output, PIPE_ANY);
> + igt_display_commit(display);
> + return false;
> + }
> +
> + /* create and set the primary plane fb */
> + mode = igt_output_get_mode(output);
I thought the plan was to set a mode, instead of relying on one being
already set (eg. by fbcon)? Otherwise, I don't see why this cannot be
in the library?
> + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + LOCAL_DRM_FORMAT_MOD_NONE,
> + 0.0, 0.0, 0.0,
> + &data->primary_fb);
> +
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + igt_plane_set_fb(primary, &data->primary_fb);
> +
> + igt_display_commit(display);
> +
> + igt_wait_for_vblank(data->drm_fd, data->pipe);
> +
> + return true;
> +}
> +
> +static void cleanup_crtc(data_t *data, igt_output_t *output)
> +{
> + igt_display_t *display = &data->display;
> + igt_plane_t *primary;
> +
> + igt_remove_fb(data->drm_fd, &data->primary_fb);
> +
> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> + igt_plane_set_fb(primary, NULL);
> +
> + igt_output_set_pipe(output, PIPE_ANY);
> + igt_display_commit(display);
> }
>
> -static void accuracy(int fd)
> +static void run_test(data_t *data, void (*testfunc)(data_t *, bool), bool boolean)
Probably not a big deal, but I think it's more customary to have a
bitfield in data_t that tells what behavior the test should have,
rather than passing a callback.
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + enum pipe p;
> + int valid_tests = 0;
unsigned
> +
> + for_each_connected_output(display, output) {
> + for_each_pipe(display, p) {
> + data->pipe = p;
> +
> + if (!prepare_crtc(data, output))
> + continue;
> +
> + valid_tests++;
> +
> + igt_info("Beginning %s on pipe %s, connector %s\n",
> + igt_subtest_name(),
> + kmstest_pipe_name(data->pipe),
> + igt_output_name(output));
> +
> + testfunc(data, boolean);
> +
> + igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
> + igt_subtest_name(),
> + kmstest_pipe_name(data->pipe),
> + igt_output_name(output));
> +
> + /* cleanup what prepare_crtc() has done */
> + cleanup_crtc(data, output);
> + }
> + }
> +
> + igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> +}
> +
> +static void accuracy(data_t *data, bool busy)
> {
> union drm_wait_vblank vbl;
> unsigned long target;
> + uint32_t crtc_id_flag;
> int n;
>
> memset(&vbl, 0, sizeof(vbl));
> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 1;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
Don't see much point in moving the drm_fd into the data struct, I
think I would just make fd a global and avoid these changes.
> target = vbl.reply.sequence + 60;
> for (n = 0; n < 60; n++) {
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 1;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>
> - vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
> + vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_id_flag;
> vbl.request.sequence = target;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> }
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 0;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> igt_assert_eq(vbl.reply.sequence, target);
>
> for (n = 0; n < 60; n++) {
> struct drm_event_vblank ev;
> - igt_assert_eq(read(fd, &ev, sizeof(ev)), sizeof(ev));
> + igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
> igt_assert_eq(ev.sequence, target);
> }
> }
>
> -static void vblank_query(int fd, bool busy)
> +static void vblank_query(data_t *data, bool busy)
> {
> union drm_wait_vblank vbl;
> struct timespec start, end;
> unsigned long sq, count = 0;
> struct drm_event_vblank buf;
> + uint32_t crtc_id_flag;
>
> memset(&vbl, 0, sizeof(vbl));
> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>
> if (busy) {
> - vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> + vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
> vbl.request.sequence = 72;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> }
>
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 0;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>
> sq = vbl.reply.sequence;
>
> clock_gettime(CLOCK_MONOTONIC, &start);
> do {
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 0;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> count++;
> } while ((vbl.reply.sequence - sq) <= 60);
> clock_gettime(CLOCK_MONOTONIC, &end);
> @@ -128,35 +225,37 @@ static void vblank_query(int fd, bool busy)
> busy ? "busy" : "idle", elapsed(&start, &end, count));
>
> if (busy)
> - igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> + igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
> }
>
> -static void vblank_wait(int fd, bool busy)
> +static void vblank_wait(data_t *data, bool busy)
> {
> union drm_wait_vblank vbl;
> struct timespec start, end;
> unsigned long sq, count = 0;
> struct drm_event_vblank buf;
> + uint32_t crtc_id_flag;
>
> memset(&vbl, 0, sizeof(vbl));
> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>
> if (busy) {
> - vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
> + vbl.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
I would personally just add such a line afterwards, find it more
readable (here and elsewhere):
vbl.request.type |= crtc_id_flag;
> vbl.request.sequence = 72;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> }
>
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type |= DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 0;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>
> sq = vbl.reply.sequence;
>
> clock_gettime(CLOCK_MONOTONIC, &start);
> do {
> - vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
> vbl.request.sequence = 1;
> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
> count++;
> } while ((vbl.reply.sequence - sq) <= 60);
> clock_gettime(CLOCK_MONOTONIC, &end);
> @@ -167,32 +266,34 @@ static void vblank_wait(int fd, bool busy)
> elapsed(&start, &end, count));
>
> if (busy)
> - igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
> + igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
> }
>
> igt_main
> {
> - int fd;
> + data_t data;
>
> igt_skip_on_simulation();
>
> igt_fixture {
> - fd = drm_open_driver(DRIVER_ANY);
> - igt_require(crtc0_active(fd));
> + data.drm_fd = drm_open_driver(DRIVER_ANY);
> + kmstest_set_vt_graphics_mode();
> + igt_display_init(&data.display, data.drm_fd);
> }
>
> igt_subtest("accuracy")
> - accuracy(fd);
> + run_test(&data, accuracy, false);
>
> igt_subtest("query-idle")
> - vblank_query(fd, false);
> + run_test(&data, vblank_query, false);
>
> igt_subtest("query-busy")
> - vblank_query(fd, true);
> + run_test(&data, vblank_query, true);
>
> igt_subtest("wait-idle")
> - vblank_wait(fd, false);
> + run_test(&data, vblank_wait, false);
>
> igt_subtest("wait-busy")
> - vblank_wait(fd, true);
> + run_test(&data, vblank_wait, true);
> }
> +
Thanks!
Tomeu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode.
2016-04-26 12:32 ` Tomeu Vizoso
@ 2016-04-26 15:46 ` Robert Foss
0 siblings, 0 replies; 8+ messages in thread
From: Robert Foss @ 2016-04-26 15:46 UTC (permalink / raw)
To: Tomeu Vizoso; +Cc: Intel Graphics Development, Daniel Vetter
>> Previously crtc0 was statically used for VBLANK tests, but
>> that assumption is not valid for the VC4 platform.
>> Instead we're now explicitly setting the mode.
>>
>> Also add support for testing all connected connectors during
>> the same test.
>
> Is there any reason why this cannot be done in a subsequent patch?
>
It could be done in a separate patch, but I think it would be
jumping through hoops.
Finding the first valid connector is pretty much the same work as
iterating through all of the connected connectors.
If the consensus is separating the two, I'll happily do so,
but the code for the intermediate step would be somewhat convoluted
I think.
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>> tests/kms_vblank.c | 179 +++++++++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 140 insertions(+), 39 deletions(-)
>>
>> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
>> index 40ab6fd..970fefe 100644
>> --- a/tests/kms_vblank.c
>> +++ b/tests/kms_vblank.c
>> @@ -44,6 +44,14 @@
>>
>> IGT_TEST_DESCRIPTION("Test speed of WaitVblank.");
>>
>> +typedef struct {
>> + int drm_fd;
>> + igt_display_t display;
>> + struct igt_fb primary_fb;
>> + igt_output_t *output;
>> + enum pipe pipe;
>> +} data_t;
>> +
>> static double elapsed(const struct timespec *start,
>> const struct timespec *end,
>> int loop)
>> @@ -51,75 +59,164 @@ static double elapsed(const struct timespec *start,
>> return (1e6*(end->tv_sec - start->tv_sec) + (end->tv_nsec - start->tv_nsec)/1000)/loop;
>> }
>>
>> -static bool crtc0_active(int fd)
>> +static uint32_t crtc_id_to_flag(uint32_t crtc_id)
>
> This seems to be pipe id, not the crtc id. Maybe this belongs to the library.
>
Agreed, fixing name and moving to igt_kms in v4.
>> {
>> - union drm_wait_vblank vbl;
>> + if (crtc_id == 0)
>> + return 0;
>> + else if (crtc_id == 1)
>> + return 1 | _DRM_VBLANK_SECONDARY;
>> + else
>> + return crtc_id << 1;
>> +}
>>
>> - memset(&vbl, 0, sizeof(vbl));
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> - return drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl) == 0;
>> +static bool prepare_crtc(data_t *data, igt_output_t *output)
>> +{
>> + drmModeModeInfo *mode;
>> + igt_display_t *display = &data->display;
>> + igt_plane_t *primary;
>> +
>> + /* select the pipe we want to use */
>> + igt_output_set_pipe(output, data->pipe);
>> + igt_display_commit(display);
>> +
>> + if (!output->valid) {
>> + igt_output_set_pipe(output, PIPE_ANY);
>> + igt_display_commit(display);
>> + return false;
>> + }
>> +
>> + /* create and set the primary plane fb */
>> + mode = igt_output_get_mode(output);
>
> I thought the plan was to set a mode, instead of relying on one being
> already set (eg. by fbcon)? Otherwise, I don't see why this cannot be
> in the library?
>
I'm not sure I understand this comment entirely. The mode (as far as I
understand it) is set during the kmstest_set_vt_graphics_mode() call.
>> + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
>> + DRM_FORMAT_XRGB8888,
>> + LOCAL_DRM_FORMAT_MOD_NONE,
>> + 0.0, 0.0, 0.0,
>> + &data->primary_fb);
>> +
>> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
>> + igt_plane_set_fb(primary, &data->primary_fb);
>> +
>> + igt_display_commit(display);
>> +
>> + igt_wait_for_vblank(data->drm_fd, data->pipe);
>> +
>> + return true;
>> +}
>> +
>> +static void cleanup_crtc(data_t *data, igt_output_t *output)
>> +{
>> + igt_display_t *display = &data->display;
>> + igt_plane_t *primary;
>> +
>> + igt_remove_fb(data->drm_fd, &data->primary_fb);
>> +
>> + primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
>> + igt_plane_set_fb(primary, NULL);
>> +
>> + igt_output_set_pipe(output, PIPE_ANY);
>> + igt_display_commit(display);
>> }
>>
>> -static void accuracy(int fd)
>> +static void run_test(data_t *data, void (*testfunc)(data_t *, bool), bool boolean)
>
> Probably not a big deal, but I think it's more customary to have a
> bitfield in data_t that tells what behavior the test should have,
> rather than passing a callback.
>
Agreed, moving to bitfield in v4.
>> +{
>> + igt_display_t *display = &data->display;
>> + igt_output_t *output;
>> + enum pipe p;
>> + int valid_tests = 0;
>
> unsigned
>
Agreed, fixing in v4.
>> +
>> + for_each_connected_output(display, output) {
>> + for_each_pipe(display, p) {
>> + data->pipe = p;
>> +
>> + if (!prepare_crtc(data, output))
>> + continue;
>> +
>> + valid_tests++;
>> +
>> + igt_info("Beginning %s on pipe %s, connector %s\n",
>> + igt_subtest_name(),
>> + kmstest_pipe_name(data->pipe),
>> + igt_output_name(output));
>> +
>> + testfunc(data, boolean);
>> +
>> + igt_info("\n%s on pipe %s, connector %s: PASSED\n\n",
>> + igt_subtest_name(),
>> + kmstest_pipe_name(data->pipe),
>> + igt_output_name(output));
>> +
>> + /* cleanup what prepare_crtc() has done */
>> + cleanup_crtc(data, output);
>> + }
>> + }
>> +
>> + igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>> +}
>> +
>> +static void accuracy(data_t *data, bool busy)
>> {
>> union drm_wait_vblank vbl;
>> unsigned long target;
>> + uint32_t crtc_id_flag;
>> int n;
>>
>> memset(&vbl, 0, sizeof(vbl));
>> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>>
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 1;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>
> Don't see much point in moving the drm_fd into the data struct, I
> think I would just make fd a global and avoid these changes.
>
Agreed, moving drm_fd out of data struct in v4.
>> target = vbl.reply.sequence + 60;
>> for (n = 0; n < 60; n++) {
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 1;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>>
>> - vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
>> + vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | crtc_id_flag;
>> vbl.request.sequence = target;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> }
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 0;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> igt_assert_eq(vbl.reply.sequence, target);
>>
>> for (n = 0; n < 60; n++) {
>> struct drm_event_vblank ev;
>> - igt_assert_eq(read(fd, &ev, sizeof(ev)), sizeof(ev));
>> + igt_assert_eq(read(data->drm_fd, &ev, sizeof(ev)), sizeof(ev));
>> igt_assert_eq(ev.sequence, target);
>> }
>> }
>>
>> -static void vblank_query(int fd, bool busy)
>> +static void vblank_query(data_t *data, bool busy)
>> {
>> union drm_wait_vblank vbl;
>> struct timespec start, end;
>> unsigned long sq, count = 0;
>> struct drm_event_vblank buf;
>> + uint32_t crtc_id_flag;
>>
>> memset(&vbl, 0, sizeof(vbl));
>> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>>
>> if (busy) {
>> - vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
>> vbl.request.sequence = 72;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> }
>>
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 0;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>>
>> sq = vbl.reply.sequence;
>>
>> clock_gettime(CLOCK_MONOTONIC, &start);
>> do {
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 0;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> count++;
>> } while ((vbl.reply.sequence - sq) <= 60);
>> clock_gettime(CLOCK_MONOTONIC, &end);
>> @@ -128,35 +225,37 @@ static void vblank_query(int fd, bool busy)
>> busy ? "busy" : "idle", elapsed(&start, &end, count));
>>
>> if (busy)
>> - igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
>> + igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
>> }
>>
>> -static void vblank_wait(int fd, bool busy)
>> +static void vblank_wait(data_t *data, bool busy)
>> {
>> union drm_wait_vblank vbl;
>> struct timespec start, end;
>> unsigned long sq, count = 0;
>> struct drm_event_vblank buf;
>> + uint32_t crtc_id_flag;
>>
>> memset(&vbl, 0, sizeof(vbl));
>> + crtc_id_flag = crtc_id_to_flag(data->pipe);
>>
>> if (busy) {
>> - vbl.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT;
>> + vbl.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | crtc_id_flag;
>
> I would personally just add such a line afterwards, find it more
> readable (here and elsewhere):
>
> vbl.request.type |= crtc_id_flag;
>
Agreed, changing in v4.
>> vbl.request.sequence = 72;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> }
>>
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type |= DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 0;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>>
>> sq = vbl.reply.sequence;
>>
>> clock_gettime(CLOCK_MONOTONIC, &start);
>> do {
>> - vbl.request.type = DRM_VBLANK_RELATIVE;
>> + vbl.request.type = DRM_VBLANK_RELATIVE | crtc_id_flag;
>> vbl.request.sequence = 1;
>> - do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> + do_ioctl(data->drm_fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
>> count++;
>> } while ((vbl.reply.sequence - sq) <= 60);
>> clock_gettime(CLOCK_MONOTONIC, &end);
>> @@ -167,32 +266,34 @@ static void vblank_wait(int fd, bool busy)
>> elapsed(&start, &end, count));
>>
>> if (busy)
>> - igt_assert_eq(read(fd, &buf, sizeof(buf)), sizeof(buf));
>> + igt_assert_eq(read(data->drm_fd, &buf, sizeof(buf)), sizeof(buf));
>> }
>>
>> igt_main
>> {
>> - int fd;
>> + data_t data;
>>
>> igt_skip_on_simulation();
>>
>> igt_fixture {
>> - fd = drm_open_driver(DRIVER_ANY);
>> - igt_require(crtc0_active(fd));
>> + data.drm_fd = drm_open_driver(DRIVER_ANY);
>> + kmstest_set_vt_graphics_mode();
>> + igt_display_init(&data.display, data.drm_fd);
>> }
>>
>> igt_subtest("accuracy")
>> - accuracy(fd);
>> + run_test(&data, accuracy, false);
>>
>> igt_subtest("query-idle")
>> - vblank_query(fd, false);
>> + run_test(&data, vblank_query, false);
>>
>> igt_subtest("query-busy")
>> - vblank_query(fd, true);
>> + run_test(&data, vblank_query, true);
>>
>> igt_subtest("wait-idle")
>> - vblank_wait(fd, false);
>> + run_test(&data, vblank_wait, false);
>>
>> igt_subtest("wait-busy")
>> - vblank_wait(fd, true);
>> + run_test(&data, vblank_wait, true);
>> }
>> +
>
> Thanks!
>
> Tomeu
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-04-26 15:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-25 15:05 [PATCH i-g-t v3 0/4] kms_flip_event_leak and kms_vblank fixes for VC4 robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 1/4] lib/igt_kms: Add support for up to 10 planes robert.foss
2016-04-26 11:47 ` Tomeu Vizoso
2016-04-25 15:05 ` [PATCH i-g-t v3 2/4] lib/igt_kms: Fix plane counting in igt_display_init robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 3/4] lib/igt_kms: Switch to verbose assert robert.foss
2016-04-25 15:05 ` [PATCH i-g-t v3 4/4] kms_vblank: Switch from using crtc0 statically to explicitly setting mode robert.foss
2016-04-26 12:32 ` Tomeu Vizoso
2016-04-26 15:46 ` Robert Foss
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.