All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2.
@ 2018-05-07 13:14 Maarten Lankhorst
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2 Maarten Lankhorst
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-07 13:14 UTC (permalink / raw)
  To: igt-dev; +Cc: Laurent Pinchart, Daniel Vetter

Now actually with testing!

This doesn't solve the API issues yet from having reassignable planes.
It's only a preparation patch that will allow us to get rid of the
duplicated plane definitions in the future when we decide on an api
and fix the tests to use it.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Daniel Vetter <daniel@ffwll.ch>

Maarten Lankhorst (4):
  lib/igt_kms: Remove plane->pipe backpointer, v2.
  tests: Remove kms_mmio_vs_cs_flip
  lib/igt_kms: Remove igt_output_get_plane(), v2.
  lib/igt_kms: Move planes from pipe to display, v2.

 lib/igt_kms.c                     | 206 ++++++------
 lib/igt_kms.h                     |  23 +-
 tests/Makefile.sources            |   1 -
 tests/kms_atomic.c                |  10 +-
 tests/kms_atomic_interruptible.c  |  19 +-
 tests/kms_atomic_transition.c     |  74 +++--
 tests/kms_chamelium.c             |  35 +-
 tests/kms_color.c                 | 157 ++++-----
 tests/kms_concurrent.c            |  48 ++-
 tests/kms_crtc_background_color.c |  23 +-
 tests/kms_cursor_legacy.c         |   4 +-
 tests/kms_flip_tiling.c           |   2 +-
 tests/kms_mmio_vs_cs_flip.c       | 523 ------------------------------
 tests/kms_pipe_b_c_ivb.c          |   4 +-
 tests/kms_pipe_crc_basic.c        |   2 +-
 tests/kms_plane.c                 |  44 ++-
 tests/kms_plane_lowres.c          |   4 +-
 tests/kms_plane_multiple.c        |  75 ++---
 tests/kms_plane_scaling.c         |  47 ++-
 tests/kms_rotation_crc.c          |  12 +-
 tests/kms_universal_plane.c       |   9 +-
 tests/kms_vblank.c                |   2 +-
 tests/meson.build                 |   1 -
 23 files changed, 401 insertions(+), 924 deletions(-)
 delete mode 100644 tests/kms_mmio_vs_cs_flip.c

-- 
2.17.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2.
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
@ 2018-05-07 13:14 ` Maarten Lankhorst
  2018-05-07 14:51   ` Daniel Vetter
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip Maarten Lankhorst
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-07 13:14 UTC (permalink / raw)
  To: igt-dev; +Cc: Laurent Pinchart, Daniel Vetter

We want to get rid of the pipe <-> plane mapping. The first thing we
have to do is decoupling the plane array from the pipe, so do that here.

Changes since v1:
- Disable limited-range tests again, like they were before this commit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_kms.c                     |  52 +++++-----
 lib/igt_kms.h                     |   5 +-
 tests/kms_atomic.c                |  10 +-
 tests/kms_atomic_interruptible.c  |  19 ++--
 tests/kms_chamelium.c             |  35 ++++---
 tests/kms_color.c                 | 155 +++++++++++++++---------------
 tests/kms_concurrent.c            |   2 +-
 tests/kms_crtc_background_color.c |  23 ++---
 tests/kms_plane_multiple.c        |   4 +-
 9 files changed, 154 insertions(+), 151 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 35c77da7b996..7281c71607a3 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1678,8 +1678,7 @@ static void igt_output_refresh(igt_output_t *output)
 static int
 igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	return drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
 				 DRM_MODE_OBJECT_PLANE, prop_id, value);
@@ -1914,7 +1913,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 
 			igt_assert_f(plane->index < n_planes, "n_planes < plane->index failed\n");
 			plane->type = type;
-			plane->pipe = pipe;
+			plane->display = display;
+			plane->bound_pipe = i;
 			plane->drm_plane = drm_plane;
 			plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
 
@@ -2548,7 +2548,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
 					   igt_pipe_t *pipe,
 					   bool fail_on_error)
 {
-	struct igt_display *display = primary->pipe->display;
+	struct igt_display *display = primary->display;
 	igt_output_t *output = igt_pipe_get_output(pipe);
 	drmModeModeInfo *mode;
 	uint32_t fb_id, crtc_id;
@@ -2558,12 +2558,12 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
 	igt_assert((primary->values[IGT_PLANE_CRTC_X] == 0 && primary->values[IGT_PLANE_CRTC_Y] == 0));
 
 	/* nor rotated */
-	if (!pipe->display->first_commit)
+	if (!display->first_commit)
 		igt_assert(!igt_plane_is_prop_changed(primary, IGT_PLANE_ROTATION));
 
 	if (!igt_plane_is_prop_changed(primary, IGT_PLANE_FB_ID) &&
 	    !(primary->changed & IGT_PLANE_COORD_CHANGED_MASK) &&
-	    !igt_pipe_obj_is_prop_changed(primary->pipe, IGT_CRTC_MODE_ID))
+	    !igt_pipe_obj_is_prop_changed(pipe, IGT_CRTC_MODE_ID))
 		return 0;
 
 	crtc_id = pipe->crtc_id;
@@ -2803,7 +2803,7 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties
 {
 	igt_assert(igt_plane_has_prop(plane, prop));
 
-	return igt_mode_object_get_prop(plane->pipe->display, DRM_MODE_OBJECT_PLANE,
+	return igt_mode_object_get_prop(plane->display, DRM_MODE_OBJECT_PLANE,
 					plane->drm_plane->plane_id, plane->props[prop]);
 }
 
@@ -2823,7 +2823,7 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties
 void
 igt_plane_replace_prop_blob(igt_plane_t *plane, enum igt_atomic_plane_properties prop, const void *ptr, size_t length)
 {
-	igt_display_t *display = plane->pipe->display;
+	igt_display_t *display = plane->display;
 	uint64_t *blob = &plane->values[prop];
 	uint32_t blob_id = 0;
 
@@ -3524,13 +3524,16 @@ igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type)
  */
 void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
-	LOG(display, "%s.%d: plane_set_fb(%d)\n", kmstest_pipe_name(pipe->pipe),
+	LOG(display, "%s.%d: plane_set_fb(%d)\n", kmstest_pipe_name(plane->bound_pipe),
 	    plane->index, fb ? fb->fb_id : 0);
 
-	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_ID, fb ? pipe->crtc_id : 0);
+	if (plane->bound_pipe != PIPE_NONE) {
+		igt_pipe_t *pipe = &display->pipes[plane->bound_pipe];
+
+		igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_ID, fb ? pipe->crtc_id : 0);
+	}
 	igt_plane_set_prop_value(plane, IGT_PLANE_FB_ID, fb ? fb->fb_id : 0);
 
 	if (plane->type == DRM_PLANE_TYPE_CURSOR && fb)
@@ -3591,11 +3594,10 @@ void igt_plane_set_fence_fd(igt_plane_t *plane, int fence_fd)
  */
 void igt_plane_set_position(igt_plane_t *plane, int x, int y)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	LOG(display, "%s.%d: plane_set_position(%d,%d)\n",
-	    kmstest_pipe_name(pipe->pipe), plane->index, x, y);
+	    kmstest_pipe_name(plane->bound_pipe), plane->index, x, y);
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_X, x);
 	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_Y, y);
@@ -3613,11 +3615,10 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y)
  */
 void igt_plane_set_size(igt_plane_t *plane, int w, int h)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	LOG(display, "%s.%d: plane_set_size (%dx%d)\n",
-	    kmstest_pipe_name(pipe->pipe), plane->index, w, h);
+	    kmstest_pipe_name(plane->bound_pipe), plane->index, w, h);
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_W, w);
 	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_H, h);
@@ -3636,11 +3637,10 @@ void igt_plane_set_size(igt_plane_t *plane, int w, int h)
 void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t x, uint32_t y)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	LOG(display, "%s.%d: fb_set_position(%d,%d)\n",
-	    kmstest_pipe_name(pipe->pipe), plane->index, x, y);
+	    kmstest_pipe_name(plane->bound_pipe), plane->index, x, y);
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_X, IGT_FIXED(x, 0));
 	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_Y, IGT_FIXED(y, 0));
@@ -3660,11 +3660,10 @@ void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
 void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
 	uint32_t w, uint32_t h)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	LOG(display, "%s.%d: fb_set_size(%dx%d)\n",
-	    kmstest_pipe_name(pipe->pipe), plane->index, w, h);
+	    kmstest_pipe_name(plane->bound_pipe), plane->index, w, h);
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_W, IGT_FIXED(w, 0));
 	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_H, IGT_FIXED(h, 0));
@@ -3697,11 +3696,10 @@ static const char *rotation_name(igt_rotation_t rotation)
  */
 void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
 {
-	igt_pipe_t *pipe = plane->pipe;
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 
 	LOG(display, "%s.%d: plane_set_rotation(%s)\n",
-	    kmstest_pipe_name(pipe->pipe),
+	    kmstest_pipe_name(plane->bound_pipe),
 	    plane->index, rotation_name(rotation));
 
 	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 0e75d0c9e6b9..5e7db0a263fc 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -293,7 +293,7 @@ typedef enum {
 
 typedef struct {
 	/*< private >*/
-	igt_pipe_t *pipe;
+	igt_display_t *display;
 	int index;
 	/* capabilities */
 	int type;
@@ -307,6 +307,9 @@ typedef struct {
 	/* gem handle for fb */
 	uint32_t gem_handle;
 
+	/* Default pipe, override with igt_plane_bind() */
+	enum pipe bound_pipe;
+
 	uint64_t changed;
 	uint32_t props[IGT_NUM_PLANE_PROPS];
 	uint64_t values[IGT_NUM_PLANE_PROPS];
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index ac02baf0170b..71a34c51f8ef 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -98,7 +98,7 @@ static void plane_check_current_state(igt_plane_t *plane, const uint64_t *values
 	uint64_t current_values[IGT_NUM_PLANE_PROPS];
 	int i;
 
-	legacy = drmModeGetPlane(plane->pipe->display->drm_fd, plane->drm_plane->plane_id);
+	legacy = drmModeGetPlane(plane->display->drm_fd, plane->drm_plane->plane_id);
 	igt_assert(legacy);
 
 	igt_assert_eq_u32(legacy->crtc_id, values[IGT_PLANE_CRTC_ID]);
@@ -123,7 +123,7 @@ static void plane_check_current_state(igt_plane_t *plane, const uint64_t *values
 static void plane_commit(igt_plane_t *plane, enum igt_commit_style s,
                                 enum kms_atomic_check_relax relax)
 {
-	igt_display_commit2(plane->pipe->display, s);
+	igt_display_commit2(plane->display, s);
 	plane_check_current_state(plane, plane->values, relax);
 }
 
@@ -135,7 +135,7 @@ static void plane_commit_atomic_err(igt_plane_t *plane,
 
 	plane_get_current_state(plane, current_values);
 
-	igt_assert_eq(-err, igt_display_try_commit2(plane->pipe->display, COMMIT_ATOMIC));
+	igt_assert_eq(-err, igt_display_try_commit2(plane->display, COMMIT_ATOMIC));
 
 	plane_check_current_state(plane, current_values, relax);
 }
@@ -840,7 +840,7 @@ static void atomic_setup(igt_display_t *display, enum pipe pipe, igt_output_t *o
 	igt_output_set_pipe(output, pipe);
 	igt_plane_set_fb(primary, fb);
 
-	crtc_commit(primary->pipe, primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+	crtc_commit(&display->pipes[pipe], primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
 }
 
 static void atomic_clear(igt_display_t *display, enum pipe pipe, igt_plane_t *primary, igt_output_t *output)
@@ -853,7 +853,7 @@ static void atomic_clear(igt_display_t *display, enum pipe pipe, igt_plane_t *pr
 	}
 
 	igt_output_set_pipe(output, PIPE_NONE);
-	crtc_commit(primary->pipe, primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+	crtc_commit(&display->pipes[pipe], primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
 }
 
 igt_main
diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
index 64a005597a21..a4d7d3abbd5a 100644
--- a/tests/kms_atomic_interruptible.c
+++ b/tests/kms_atomic_interruptible.c
@@ -81,6 +81,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 	drmModeModeInfo *mode;
 	igt_fb_t fb, fb2;
 	igt_plane_t *primary, *plane;
+	igt_pipe_t *pipe_obj = &display->pipes[pipe];
 	int block;
 
 	/*
@@ -140,7 +141,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 				struct drm_mode_crtc crtc = {
 					.set_connectors_ptr = (uint64_t)(uintptr_t)&output->id,
 					.count_connectors = 1,
-					.crtc_id = primary->pipe->crtc_id,
+					.crtc_id = pipe_obj->crtc_id,
 					.fb_id = fb2.fb_id,
 					.mode_valid = 1,
 					.mode = *(struct drm_mode_modeinfo*)mode,
@@ -151,15 +152,15 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 			}
 			case test_atomic_modeset: {
 				uint32_t objs[3] = {
-					plane->pipe->crtc_id,
+					pipe_obj->crtc_id,
 					output->id,
 					plane->drm_plane->plane_id
 				};
 				uint32_t count_props[3] = { 2, 1, 6 };
 				uint32_t props[] = {
 					/* crtc: 2 props */
-					plane->pipe->props[IGT_CRTC_MODE_ID],
-					plane->pipe->props[IGT_CRTC_ACTIVE],
+					pipe_obj->props[IGT_CRTC_MODE_ID],
+					pipe_obj->props[IGT_CRTC_ACTIVE],
 					/* connector: 1 prop */
 					output->props[IGT_CONNECTOR_CRTC_ID],
 					/* plane: remainder props */
@@ -175,9 +176,9 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 					0, /* mode_id, filled in below */
 					true,
 					/* connector */
-					plane->pipe->crtc_id,
+					pipe_obj->crtc_id,
 					/* plane */
-					plane->pipe->crtc_id,
+					pipe_obj->crtc_id,
 					fb2.fb_id,
 					IGT_FIXED(fb2.width, 0),
 					IGT_FIXED(fb2.height, 0),
@@ -216,7 +217,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 			case test_setcursor: {
 				struct drm_mode_cursor cur = {
 					.flags = DRM_MODE_CURSOR_BO,
-					.crtc_id = plane->pipe->crtc_id,
+					.crtc_id = pipe_obj->crtc_id,
 					.width = fb2.width,
 					.height = fb2.height,
 					.handle = fb2.gem_handle,
@@ -227,7 +228,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 			case test_setplane: {
 				struct drm_mode_set_plane setplane = {
 					.plane_id = plane->drm_plane->plane_id,
-					.crtc_id = plane->pipe->crtc_id,
+					.crtc_id = pipe_obj->crtc_id,
 					.fb_id = fb2.fb_id,
 					.crtc_w = fb2.width,
 					.crtc_h = fb2.height,
@@ -240,7 +241,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
 			}
 			case test_pageflip: {
 				struct drm_mode_crtc_page_flip pageflip = {
-					.crtc_id = plane->pipe->crtc_id,
+					.crtc_id = pipe_obj->crtc_id,
 					.fb_id = fb2.fb_id,
 					.flags = DRM_MODE_PAGE_FLIP_EVENT,
 				};
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2bc34d07788d..6e9e54ed7f5b 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -410,14 +410,15 @@ test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
 
 static igt_output_t *
 prepare_output(data_t *data,
-	       struct chamelium_port *port)
+	       struct chamelium_port *port,
+	       enum pipe *ret_pipe)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
 	drmModeRes *res;
 	drmModeConnector *connector =
 		chamelium_port_get_connector(data->chamelium, port, false);
-	enum pipe pipe;
+	enum pipe pipe = PIPE_NONE;
 	bool found = false;
 
 	igt_assert(res = drmModeGetResources(data->drm_fd));
@@ -449,6 +450,7 @@ prepare_output(data_t *data,
 	drmModeFreeConnector(connector);
 	drmModeFreeResources(res);
 
+	*ret_pipe = pipe;
 	return output;
 }
 
@@ -456,10 +458,12 @@ static void
 enable_output(data_t *data,
 	      struct chamelium_port *port,
 	      igt_output_t *output,
+	      enum pipe pipe,
 	      drmModeModeInfo *mode,
 	      struct igt_fb *fb)
 {
 	igt_display_t *display = output->display;
+	igt_pipe_t *pipe_obj = &display->pipes[pipe];
 	igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	drmModeConnector *connector = chamelium_port_get_connector(
 	    data->chamelium, port, false);
@@ -471,12 +475,12 @@ enable_output(data_t *data,
 	igt_output_override_mode(output, mode);
 
 	/* Clear any color correction values that might be enabled */
-	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
-		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
-	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT))
-		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_GAMMA_LUT, NULL, 0);
-	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
-		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_CTM, NULL, 0);
+	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_DEGAMMA_LUT))
+		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
+	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT))
+		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, NULL, 0);
+	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_CTM))
+		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_CTM, NULL, 0);
 
 	igt_display_commit2(display, COMMIT_ATOMIC);
 
@@ -500,10 +504,11 @@ test_display_crc(data_t *data, struct chamelium_port *port, int count,
 	drmModeConnector *connector;
 	int fb_id, i, j, captured_frame_count;
 	int count_modes;
+	enum pipe pipe;
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port);
+	output = prepare_output(data, port, &pipe);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -523,7 +528,7 @@ test_display_crc(data_t *data, struct chamelium_port *port, int count,
 		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
 								&fb);
 
-		enable_output(data, port, output, mode, &fb);
+		enable_output(data, port, output, pipe, mode, &fb);
 
 		/* We want to keep the display running for a little bit, since
 		 * there's always the potential the driver isn't able to keep
@@ -563,10 +568,11 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 	drmModeModeInfo *mode;
 	drmModeConnector *connector;
 	int fb_id, i, j;
+	enum pipe pipe;
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port);
+	output = prepare_output(data, port, &pipe);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -580,7 +586,7 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 						    0, 0, 0, &fb);
 		igt_assert(fb_id > 0);
 
-		enable_output(data, port, output, mode, &fb);
+		enable_output(data, port, output, pipe, mode, &fb);
 
 		igt_debug("Reading frame dumps from Chamelium...\n");
 		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 5);
@@ -608,10 +614,11 @@ test_analog_frame_dump(data_t *data, struct chamelium_port *port)
 	drmModeConnector *connector;
 	int fb_id, i;
 	bool bridge;
+	enum pipe pipe;
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port);
+	output = prepare_output(data, port, &pipe);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -631,7 +638,7 @@ test_analog_frame_dump(data_t *data, struct chamelium_port *port)
 						    0, 0, 0, &fb);
 		igt_assert(fb_id > 0);
 
-		enable_output(data, port, output, mode, &fb);
+		enable_output(data, port, output, pipe, mode, &fb);
 
 		igt_debug("Reading frame dumps from Chamelium...\n");
 
diff --git a/tests/kms_color.c b/tests/kms_color.c
index bb106dd00565..ce364713f665 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -258,6 +258,7 @@ static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
  * rectangles with linear degamma LUT.
  */
 static void test_pipe_degamma(data_t *data,
+			      igt_pipe_t *pipe,
 			      igt_plane_t *primary)
 {
 	igt_output_t *output;
@@ -274,13 +275,13 @@ static void test_pipe_degamma(data_t *data,
 
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
 		drmModeModeInfo *mode;
 		struct igt_fb fb_modeset, fb;
 		igt_crc_t crc_fullgamma, crc_fullcolors;
 		int fb_id, fb_modeset_id;
 
-		igt_output_set_pipe(output, primary->pipe->pipe);
+		igt_output_set_pipe(output, pipe->pipe);
 		mode = igt_output_get_mode(output);
 
 		/* Create a framebuffer at the size of the output. */
@@ -301,16 +302,16 @@ static void test_pipe_degamma(data_t *data,
 		igt_assert(fb_modeset_id);
 
 		igt_plane_set_fb(primary, &fb_modeset);
-		disable_ctm(primary->pipe);
-		disable_degamma(primary->pipe);
-		set_gamma(data, primary->pipe, gamma_linear);
+		disable_ctm(pipe);
+		disable_degamma(pipe);
+		set_gamma(data, pipe, gamma_linear);
 		igt_display_commit(&data->display);
 
 		/* Draw solid colors with no degamma transformation. */
 		paint_rectangles(data, mode, red_green_blue, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
 		/* Draw a gradient with degamma LUT to remap all
@@ -318,9 +319,9 @@ static void test_pipe_degamma(data_t *data,
 		 */
 		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
 		igt_plane_set_fb(primary, &fb);
-		set_degamma(data, primary->pipe, degamma_full);
+		set_degamma(data, pipe, degamma_full);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
 		/* Verify that the CRC of the software computed output is
@@ -342,6 +343,7 @@ static void test_pipe_degamma(data_t *data,
  * LUT and verify we have the same CRC as drawing solid color rectangles.
  */
 static void test_pipe_gamma(data_t *data,
+			    igt_pipe_t *pipe,
 			    igt_plane_t *primary)
 {
 	igt_output_t *output;
@@ -354,13 +356,13 @@ static void test_pipe_gamma(data_t *data,
 
 	gamma_full = generate_table_max(data->gamma_lut_size);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
 		drmModeModeInfo *mode;
 		struct igt_fb fb_modeset, fb;
 		igt_crc_t crc_fullgamma, crc_fullcolors;
 		int fb_id, fb_modeset_id;
 
-		igt_output_set_pipe(output, primary->pipe->pipe);
+		igt_output_set_pipe(output, pipe->pipe);
 		mode = igt_output_get_mode(output);
 
 		/* Create a framebuffer at the size of the output. */
@@ -381,16 +383,16 @@ static void test_pipe_gamma(data_t *data,
 		igt_assert(fb_modeset_id);
 
 		igt_plane_set_fb(primary, &fb_modeset);
-		disable_ctm(primary->pipe);
-		disable_degamma(primary->pipe);
-		set_gamma(data, primary->pipe, gamma_full);
+		disable_ctm(pipe);
+		disable_degamma(pipe);
+		set_gamma(data, pipe, gamma_full);
 		igt_display_commit(&data->display);
 
 		/* Draw solid colors with no gamma transformation. */
 		paint_rectangles(data, mode, red_green_blue, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
 		/* Draw a gradient with gamma LUT to remap all values
@@ -399,7 +401,7 @@ static void test_pipe_gamma(data_t *data,
 		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
 		/* Verify that the CRC of the software computed output is
@@ -420,6 +422,7 @@ static void test_pipe_gamma(data_t *data,
  * with linear legacy gamma LUT.
  */
 static void test_pipe_legacy_gamma(data_t *data,
+				   igt_pipe_t *pipe,
 				   igt_plane_t *primary)
 {
 	igt_output_t *output;
@@ -432,7 +435,7 @@ static void test_pipe_legacy_gamma(data_t *data,
 	uint32_t i, legacy_lut_size;
 	uint16_t *red_lut, *green_lut, *blue_lut;
 
-	kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
+	kms_crtc = drmModeGetCrtc(data->drm_fd, pipe->crtc_id);
 	legacy_lut_size = kms_crtc->gamma_size;
 	drmModeFreeCrtc(kms_crtc);
 
@@ -440,13 +443,13 @@ static void test_pipe_legacy_gamma(data_t *data,
 	green_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
 	blue_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
 		drmModeModeInfo *mode;
 		struct igt_fb fb_modeset, fb;
 		igt_crc_t crc_fullgamma, crc_fullcolors;
 		int fb_id, fb_modeset_id;
 
-		igt_output_set_pipe(output, primary->pipe->pipe);
+		igt_output_set_pipe(output, pipe->pipe);
 		mode = igt_output_get_mode(output);
 
 		/* Create a framebuffer at the size of the output. */
@@ -467,16 +470,16 @@ static void test_pipe_legacy_gamma(data_t *data,
 		igt_assert(fb_modeset_id);
 
 		igt_plane_set_fb(primary, &fb_modeset);
-		disable_degamma(primary->pipe);
-		disable_gamma(primary->pipe);
-		disable_ctm(primary->pipe);
+		disable_degamma(pipe);
+		disable_gamma(pipe);
+		disable_ctm(pipe);
 		igt_display_commit(&data->display);
 
 		/* Draw solid colors with no gamma transformation. */
 		paint_rectangles(data, mode, red_green_blue, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
 
 		/* Draw a gradient with gamma LUT to remap all values
@@ -488,10 +491,10 @@ static void test_pipe_legacy_gamma(data_t *data,
 		red_lut[0] = green_lut[0] = blue_lut[0] = 0;
 		for (i = 1; i < legacy_lut_size; i++)
 			red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
-		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, primary->pipe->crtc_id,
+		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, pipe->crtc_id,
 						  legacy_lut_size, red_lut, green_lut, blue_lut), 0);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
 
 		/* Verify that the CRC of the software computed output is
@@ -503,7 +506,7 @@ static void test_pipe_legacy_gamma(data_t *data,
 		for (i = 1; i < legacy_lut_size; i++)
 			red_lut[i] = green_lut[i] = blue_lut[i] = i << 8;
 
-		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, primary->pipe->crtc_id,
+		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, pipe->crtc_id,
 						  legacy_lut_size, red_lut, green_lut, blue_lut), 0);
 		igt_display_commit(&data->display);
 
@@ -534,6 +537,7 @@ get_blob(data_t *data, igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop)
  * through the GAMMA_LUT property.
  */
 static void test_pipe_legacy_gamma_reset(data_t *data,
+					 igt_pipe_t *pipe,
 					 igt_plane_t *primary)
 {
 	const double ctm_identity[] = {
@@ -552,35 +556,35 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_zero = generate_table_zero(data->gamma_lut_size);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
-		igt_output_set_pipe(output, primary->pipe->pipe);
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
+		igt_output_set_pipe(output, pipe->pipe);
 
 		/* Ensure we have a clean state to start with. */
-		disable_degamma(primary->pipe);
-		disable_ctm(primary->pipe);
-		disable_gamma(primary->pipe);
+		disable_degamma(pipe);
+		disable_ctm(pipe);
+		disable_gamma(pipe);
 		igt_display_commit(&data->display);
 
 		/* Set a degama & gamma LUT and a CTM using the
 		 * properties and verify the content of the
 		 * properties. */
-		set_degamma(data, primary->pipe, degamma_linear);
-		set_ctm(primary->pipe, ctm_identity);
-		set_gamma(data, primary->pipe, gamma_zero);
+		set_degamma(data, pipe, degamma_linear);
+		set_ctm(pipe, ctm_identity);
+		set_gamma(data, pipe, gamma_zero);
 		igt_display_commit(&data->display);
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
+		blob = get_blob(data, pipe, IGT_CRTC_DEGAMMA_LUT);
 		igt_assert(blob &&
 			   blob->length == (sizeof(struct _drm_color_lut) *
 					    data->degamma_lut_size));
 		drmModeFreePropertyBlob(blob);
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
+		blob = get_blob(data, pipe, IGT_CRTC_CTM);
 		igt_assert(blob &&
 			   blob->length == sizeof(struct _drm_color_ctm));
 		drmModeFreePropertyBlob(blob);
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
+		blob = get_blob(data, pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
 			   blob->length == (sizeof(struct _drm_color_lut) *
 					    data->gamma_lut_size));
@@ -594,7 +598,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 		/* Set a gamma LUT using the legacy ioctl and verify
 		 * the content of the GAMMA_LUT property is changed
 		 * and that CTM and DEGAMMA_LUT are empty. */
-		kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
+		kms_crtc = drmModeGetCrtc(data->drm_fd, pipe->crtc_id);
 		legacy_lut_size = kms_crtc->gamma_size;
 		drmModeFreeCrtc(kms_crtc);
 
@@ -606,17 +610,17 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
 			red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
 
 		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd,
-						  primary->pipe->crtc_id,
+						  pipe->crtc_id,
 						  legacy_lut_size,
 						  red_lut, green_lut, blue_lut),
 			      0);
 		igt_display_commit(&data->display);
 
-		igt_assert(get_blob(data, primary->pipe,
+		igt_assert(get_blob(data, pipe,
 				    IGT_CRTC_DEGAMMA_LUT) == NULL);
-		igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
+		igt_assert(get_blob(data, pipe, IGT_CRTC_CTM) == NULL);
 
-		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
+		blob = get_blob(data, pipe, IGT_CRTC_GAMMA_LUT);
 		igt_assert(blob &&
 			   blob->length == (sizeof(struct _drm_color_lut) *
 					    legacy_lut_size));
@@ -645,6 +649,7 @@ static bool crc_equal(igt_crc_t *a, igt_crc_t *b)
  * the CRC is equal to using after colors with an identify ctm matrix.
  */
 static bool test_pipe_ctm(data_t *data,
+			  igt_pipe_t *pipe,
 			  igt_plane_t *primary,
 			  color_t *before,
 			  color_t *after,
@@ -662,13 +667,13 @@ static bool test_pipe_ctm(data_t *data,
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
 		drmModeModeInfo *mode;
 		struct igt_fb fb_modeset, fb;
 		igt_crc_t crc_software, crc_hardware;
 		int fb_id, fb_modeset_id;
 
-		igt_output_set_pipe(output, primary->pipe->pipe);
+		igt_output_set_pipe(output, pipe->pipe);
 		mode = igt_output_get_mode(output);
 
 		/* Create a framebuffer at the size of the output. */
@@ -689,24 +694,24 @@ static bool test_pipe_ctm(data_t *data,
 		igt_assert(fb_modeset_id);
 		igt_plane_set_fb(primary, &fb_modeset);
 
-		set_degamma(data, primary->pipe, degamma_linear);
-		set_gamma(data, primary->pipe, gamma_linear);
-		disable_ctm(primary->pipe);
+		set_degamma(data, pipe, degamma_linear);
+		set_gamma(data, pipe, gamma_linear);
+		disable_ctm(pipe);
 		igt_display_commit(&data->display);
 
 		paint_rectangles(data, mode, after, &fb);
 		igt_plane_set_fb(primary, &fb);
-		set_ctm(primary->pipe, ctm_identity);
+		set_ctm(pipe, ctm_identity);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_software);
 
 		/* With CTM transformation. */
 		paint_rectangles(data, mode, before, &fb);
 		igt_plane_set_fb(primary, &fb);
-		set_ctm(primary->pipe, ctm_matrix);
+		set_ctm(pipe, ctm_matrix);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_hardware);
 
 		/* Verify that the CRC of the software computed output is
@@ -738,6 +743,7 @@ static bool test_pipe_ctm(data_t *data,
  */
 #if 0
 static void test_pipe_limited_range_ctm(data_t *data,
+					igt_pipe_t *pipe,
 					igt_plane_t *primary)
 {
 	double limited_result = 235.0 / 255.0;
@@ -761,7 +767,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
 	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
 	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
 
-	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
+	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
 		drmModeModeInfo *mode;
 		struct igt_fb fb_modeset, fb;
 		igt_crc_t crc_full, crc_limited;
@@ -772,7 +778,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
 
 		has_broadcast_rgb_output = true;
 
-		igt_output_set_pipe(output, primary->pipe->pipe);
+		igt_output_set_pipe(output, pipe->pipe);
 		mode = igt_output_get_mode(output);
 
 		/* Create a framebuffer at the size of the output. */
@@ -793,15 +799,15 @@ static void test_pipe_limited_range_ctm(data_t *data,
 		igt_assert(fb_modeset_id);
 		igt_plane_set_fb(primary, &fb_modeset);
 
-		set_degamma(data, primary->pipe, degamma_linear);
-		set_gamma(data, primary->pipe, gamma_linear);
-		set_ctm(primary->pipe, ctm);
+		set_degamma(data, pipe, degamma_linear);
+		set_gamma(data, pipe, gamma_linear);
+		set_ctm(pipe, ctm);
 
 		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
 		paint_rectangles(data, mode, red_green_blue_limited, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
 
 		/* Set the output into limited range. */
@@ -809,7 +815,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
 		paint_rectangles(data, mode, red_green_blue_full, &fb);
 		igt_plane_set_fb(primary, &fb);
 		igt_display_commit(&data->display);
-		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
+		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
 		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
 
 		/* And reset.. */
@@ -853,8 +859,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 
 		primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
 
-		data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
-						  primary->pipe->pipe,
+		data->pipe_crc = igt_pipe_crc_new(data->drm_fd, p,
 						  INTEL_PIPE_CRC_SOURCE_AUTO);
 
 		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE));
@@ -885,7 +890,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		double ctm[] = { 0.0, 0.0, 0.0,
 				0.0, 1.0, 0.0,
 				1.0, 0.0, 1.0 };
-		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
 					 blue_green_blue, ctm));
 	}
 
@@ -898,7 +903,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		double ctm[] = { 1.0, 1.0, 0.0,
 				0.0, 0.0, 0.0,
 				0.0, 0.0, 1.0 };
-		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
 					 red_red_blue, ctm));
 	}
 
@@ -911,7 +916,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		double ctm[] = { 1.0, 0.0, 1.0,
 				0.0, 1.0, 0.0,
 				0.0, 0.0, 0.0 };
-		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
 					 red_green_red, ctm));
 	}
 
@@ -933,7 +938,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 				expected_colors[1].g =
 				expected_colors[2].b =
 				0.25 + delta * (i - 2);
-			success |= test_pipe_ctm(data, primary,
+			success |= test_pipe_ctm(data, pipe, primary,
 						 red_green_blue,
 						 expected_colors, ctm);
 		}
@@ -954,7 +959,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 				expected_colors[1].g =
 				expected_colors[2].b =
 				0.5 + delta * (i - 2);
-			success |= test_pipe_ctm(data, primary,
+			success |= test_pipe_ctm(data, pipe, primary,
 						 red_green_blue,
 						 expected_colors, ctm);
 		}
@@ -975,7 +980,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 				expected_colors[1].g =
 				expected_colors[2].b =
 				0.75 + delta * (i - 3);
-			success |= test_pipe_ctm(data, primary,
+			success |= test_pipe_ctm(data, pipe, primary,
 						 red_green_blue,
 						 expected_colors, ctm);
 		}
@@ -996,7 +1001,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		 * produce with an 8 bits per color framebuffer. */
 		igt_require(!IS_CHERRYVIEW(data->devid));
 
-		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
 					 full_rgb, ctm));
 	}
 
@@ -1009,31 +1014,31 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		double ctm[] = { -1.0,  0.0,  0.0,
 				 0.0, -1.0,  0.0,
 				 0.0,  0.0, -1.0 };
-		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
+		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
 					 all_black, ctm));
 	}
 
 #if 0
 	igt_subtest_f("pipe-%s-ctm-limited-range", kmstest_pipe_name(p))
-		test_pipe_limited_range_ctm(data, primary);
+		test_pipe_limited_range_ctm(data, pipe, primary);
 #endif
 
 	igt_subtest_f("pipe-%s-degamma", kmstest_pipe_name(p))
-		test_pipe_degamma(data, primary);
+		test_pipe_degamma(data, pipe, primary);
 
 	igt_subtest_f("pipe-%s-gamma", kmstest_pipe_name(p))
-		test_pipe_gamma(data, primary);
+		test_pipe_gamma(data, pipe, primary);
 
 	igt_subtest_f("pipe-%s-legacy-gamma", kmstest_pipe_name(p))
-		test_pipe_legacy_gamma(data, primary);
+		test_pipe_legacy_gamma(data, pipe, primary);
 
 	igt_subtest_f("pipe-%s-legacy-gamma-reset", kmstest_pipe_name(p))
-		test_pipe_legacy_gamma_reset(data, primary);
+		test_pipe_legacy_gamma_reset(data, pipe, primary);
 
 	igt_fixture {
-		disable_degamma(primary->pipe);
-		disable_gamma(primary->pipe);
-		disable_ctm(primary->pipe);
+		disable_degamma(pipe);
+		disable_gamma(pipe);
+		disable_ctm(pipe);
 		igt_display_commit(&data->display);
 
 		igt_pipe_crc_free(data->pipe_crc);
diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
index 283acf8c8a7e..695b14822c9a 100644
--- a/tests/kms_concurrent.c
+++ b/tests/kms_concurrent.c
@@ -166,7 +166,7 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
 	igt_output_set_pipe(output, pipe);
 
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	p = primary->pipe;
+	p = &data->display.pipes[pipe];
 
 	x = malloc(p->n_planes * sizeof(*x));
 	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (p->n_planes * sizeof(*x)));
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 6407e19bafce..694d8affa516 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -81,6 +81,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	int fb_id;
 	double alpha;
 
+	igt_display_reset(display);
 	igt_output_set_pipe(output, pipe);
 
 	/* create the pipe_crc object for this pipe */
@@ -89,6 +90,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	mode = igt_output_get_mode(output);
 
+	igt_remove_fb(data->gfx_fd, &data->fb);
 	fb_id = igt_create_fb(data->gfx_fd,
 			mode->hdisplay, mode->vdisplay,
 			DRM_FORMAT_XRGB8888,
@@ -108,22 +110,6 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
 	igt_display_commit2(display, COMMIT_UNIVERSAL);
 }
 
-static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
-{
-	igt_display_t *display = &data->display;
-
-	igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = NULL;
-
-	igt_remove_fb(data->gfx_fd, &data->fb);
-
-	igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND, BLACK64);
-	igt_plane_set_fb(plane, NULL);
-	igt_output_set_pipe(output, PIPE_ANY);
-
-	igt_display_commit2(display, COMMIT_UNIVERSAL);
-}
-
 static void test_crtc_background(data_t *data)
 {
 	igt_display_t *display = &data->display;
@@ -166,8 +152,11 @@ static void test_crtc_background(data_t *data)
 		igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE64);
 		igt_display_commit2(display, COMMIT_UNIVERSAL);
 
+		/* And reset to default */
+		igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+		igt_display_commit2(display, COMMIT_UNIVERSAL);
+
 		valid_tests++;
-		cleanup_crtc(data, output, plane);
 	}
 	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
 }
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index e61bc84624b3..366293b3f9ee 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -185,7 +185,8 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 	       uint64_t tiling, int max_planes, igt_output_t *output)
 {
 	drmModeModeInfo *mode;
-	igt_pipe_t *pipe;
+	igt_display_t *display = output->display;
+	igt_pipe_t *pipe = &display->pipes[pipe_id];
 	igt_plane_t *primary;
 	int *x;
 	int *y;
@@ -194,7 +195,6 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 
 	igt_output_set_pipe(output, pipe_id);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	pipe = primary->pipe;
 
 	x = malloc(pipe->n_planes * sizeof(*x));
 	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (pipe->n_planes * sizeof(*x)));
-- 
2.17.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2 Maarten Lankhorst
@ 2018-05-07 13:14 ` Maarten Lankhorst
  2018-05-07 14:21   ` Daniel Vetter
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2 Maarten Lankhorst
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-07 13:14 UTC (permalink / raw)
  To: igt-dev; +Cc: Laurent Pinchart, Daniel Vetter

CS flips no longer exist, so the test has become useless.
Other tests like kms_busy already perform some testing
that's gpu agnostic.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/Makefile.sources      |   1 -
 tests/kms_mmio_vs_cs_flip.c | 523 ------------------------------------
 tests/meson.build           |   1 -
 3 files changed, 525 deletions(-)
 delete mode 100644 tests/kms_mmio_vs_cs_flip.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 791e4f83d648..8066fb1a7a46 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -194,7 +194,6 @@ TESTS_progs = \
 	kms_invalid_dotclock \
 	kms_legacy_colorkey \
 	kms_mmap_write_crc \
-	kms_mmio_vs_cs_flip \
 	kms_panel_fitting \
 	kms_pipe_b_c_ivb \
 	kms_pipe_crc_basic \
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
deleted file mode 100644
index fa947d9cd7f4..000000000000
--- a/tests/kms_mmio_vs_cs_flip.c
+++ /dev/null
@@ -1,523 +0,0 @@
-/*
- * Copyright © 2014 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#include "igt.h"
-#include <errno.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <time.h>
-
-
-typedef struct {
-	int drm_fd;
-	igt_display_t display;
-	igt_pipe_crc_t *pipe_crc;
-	drm_intel_bufmgr *bufmgr;
-	drm_intel_bo *busy_bo;
-	uint32_t devid;
-	bool flip_done;
-} data_t;
-
-static void exec_nop(data_t *data, uint32_t handle, unsigned int ring)
-{
-	struct intel_batchbuffer *batch;
-	drm_intel_bo *bo;
-
-	batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
-	igt_assert(batch);
-
-	bo = gem_handle_to_libdrm_bo(data->bufmgr, data->drm_fd, "", handle);
-	igt_assert(bo);
-
-	/* add relocs to make sure the kernel will think we write to dst */
-	BEGIN_BATCH(4, 1);
-	OUT_BATCH(MI_BATCH_BUFFER_END);
-	OUT_BATCH(MI_NOOP);
-	OUT_RELOC(bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-	OUT_BATCH(MI_NOOP);
-	ADVANCE_BATCH();
-
-	intel_batchbuffer_flush_on_ring(batch, ring);
-	intel_batchbuffer_free(batch);
-
-	drm_intel_bo_unreference(bo);
-}
-
-static void exec_blt(data_t *data)
-{
-	struct intel_batchbuffer *batch;
-	int w, h, pitch, i;
-
-	batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
-	igt_assert(batch);
-
-	w = 8192;
-	h = data->busy_bo->size / (8192 * 4);
-	pitch = w * 4;
-
-	for (i = 0; i < 40; i++) {
-		BLIT_COPY_BATCH_START(0);
-		OUT_BATCH((3 << 24) | /* 32 bits */
-			  (0xcc << 16) | /* copy ROP */
-			  pitch);
-		OUT_BATCH(0 << 16 | 0);
-		OUT_BATCH(h << 16 | w);
-		OUT_RELOC_FENCED(data->busy_bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-		OUT_BATCH(0 << 16 | 0);
-		OUT_BATCH(pitch);
-		OUT_RELOC_FENCED(data->busy_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
-		ADVANCE_BATCH();
-	}
-
-	intel_batchbuffer_flush(batch);
-	intel_batchbuffer_free(batch);
-}
-
-static void page_flip_handler(int fd, unsigned int frame, unsigned int sec,
-			      unsigned int usec, void *_data)
-{
-	data_t *data = _data;
-
-	data->flip_done = true;
-}
-
-static void wait_for_flip(data_t *data, uint32_t flip_handle)
-{
-	struct timeval timeout = {
-		.tv_sec = 3,
-		.tv_usec = 0,
-	};
-	drmEventContext evctx = {
-		.version = 2,
-		.page_flip_handler = page_flip_handler,
-	};
-	fd_set fds;
-
-	FD_ZERO(&fds);
-	FD_SET(data->drm_fd, &fds);
-
-	while (!data->flip_done) {
-		int ret = select(data->drm_fd + 1, &fds, NULL, NULL, &timeout);
-
-		if (ret < 0 && errno == EINTR)
-			continue;
-
-		igt_assert_lte(0, ret);
-
-		do_or_die(drmHandleEvent(data->drm_fd, &evctx));
-	}
-
-	/*
-	 * The flip completion may have been signalled prematurely, so
-	 * also submit another nop batch and wait for it to make sure
-	 * the ring has really been drained.
-	 */
-	if (IS_GEN7(data->devid) || IS_GEN8(data->devid))
-		exec_nop(data, flip_handle, I915_EXEC_BLT);
-	else
-		exec_nop(data, flip_handle, I915_EXEC_RENDER);
-	gem_sync(data->drm_fd, flip_handle);
-}
-
-static void make_gpu_busy(data_t *data, uint32_t flip_handle)
-{
-	/*
-	 * Make sure flip_handle has been used on the blt ring.
-	 * This should make the flip use the same ring on gen7+.
-	 */
-	if (IS_GEN7(data->devid) || IS_GEN8(data->devid))
-		exec_nop(data, flip_handle, I915_EXEC_BLT);
-
-	/*
-	 * Add a pile commands to the ring.  The flip will be
-	 * stuck behing these commands and hence gets delayed
-	 * significantly.
-	 */
-	exec_blt(data);
-
-	/*
-	 * Make sure the render ring will block until the blt ring is clear.
-	 * This is in case the flip will execute on the render ring and the
-	 * blits were on the blt ring (this will be the case on gen6 at least).
-	 *
-	 * We can't add an explicit dependency between flip_handle and the
-	 * blits since that would cause the driver to block until the blits
-	 * have completed before it will perform a subsequent mmio flip,
-	 * and so the test would fail to exercise the mmio vs. CS flip race.
-	 */
-	if (HAS_BLT_RING(data->devid))
-		exec_nop(data, data->busy_bo->handle, I915_EXEC_RENDER);
-}
-
-/*
- * 1. set primary plane to full red
- * 2. grab a reference crc
- * 3. set primary plane to full blue
- * 4. queue lots of GPU activity to delay the subsequent page flip
- * 5. queue a page flip to the same blue fb
- * 6. toggle a fullscreen sprite (green) on and back off again
- * 7. set primary plane to red fb
- * 8. wait for GPU to finish
- * 9. compare current crc with reference crc
- *
- * We expect the primary plane to display full red at the end.
- * If the sprite operations have interfered with the page flip,
- * the driver may have mistakenly completed the flip before
- * it was executed by the CS, and hence the subsequent mmio
- * flips may have overtaken it. So once we've finished everything
- * the CS flip may have been the last thing to occur, which means
- * the primary plane may be full blue instead of the red it's
- * supposed to be.
- */
-static void
-test_plane(data_t *data, igt_output_t *output, enum pipe pipe, int plane)
-{
-	struct igt_fb red_fb, green_fb, blue_fb;
-	drmModeModeInfo *mode;
-	igt_plane_t *primary, *sprite;
-	igt_crc_t ref_crc, crc;
-	int ret;
-
-	igt_output_set_pipe(output, pipe);
-
-	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	sprite = igt_output_get_plane(output, plane);
-
-	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,
-			    1.0, 0.0, 0.0,
-			    &red_fb);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    0.0, 1.0, 0.0,
-			    &green_fb);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    0.0, 0.0, 1.0,
-			    &blue_fb);
-
-	/*
-	 * Make sure these buffers are suited for display use
-	 * because most of the modeset operations must be fast
-	 * later on.
-	 */
-	igt_plane_set_fb(primary, &blue_fb);
-	igt_display_commit(&data->display);
-	igt_plane_set_fb(sprite, &green_fb);
-	igt_display_commit(&data->display);
-	igt_plane_set_fb(sprite, NULL);
-	igt_display_commit(&data->display);
-
-	if (data->pipe_crc)
-		igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-
-	/* set red fb and grab reference crc */
-	igt_plane_set_fb(primary, &red_fb);
-	igt_display_commit(&data->display);
-	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
-
-	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
-			     blue_fb.fb_id, 0, 0, &output->id, 1,
-			     mode);
-	igt_assert_eq(ret, 0);
-
-	make_gpu_busy(data, blue_fb.gem_handle);
-
-	data->flip_done = false;
-	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
-			      blue_fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, data);
-	igt_assert_eq(ret, 0);
-
-	/*
-	 * Toggle a fullscreen sprite on and back off. This will result
-	 * in the primary plane getting disabled and re-enbled, and that
-	 * leads to mmio flips. The driver may then mistake the flip done
-	 * interrupts from the mmio flips as the flip done interrupts for
-	 * the CS flip, and hence subsequent mmio flips won't wait for the
-	 * CS flips like they should.
-	 */
-	ret = drmModeSetPlane(data->drm_fd,
-			      sprite->drm_plane->plane_id,
-			      output->config.crtc->crtc_id,
-			      green_fb.fb_id, 0,
-			      0, 0, mode->hdisplay, mode->vdisplay,
-			      0, 0, mode->hdisplay << 16, mode->vdisplay << 16);
-	igt_assert_eq(ret, 0);
-	ret = drmModeSetPlane(data->drm_fd,
-			      sprite->drm_plane->plane_id,
-			      output->config.crtc->crtc_id,
-			      0, 0,
-			      0, 0, 0, 0,
-			      0, 0, 0, 0);
-	igt_assert_eq(ret, 0);
-
-	/*
-	 * Set primary plane to red fb. This should wait for the CS flip
-	 * to complete. But if the kernel mistook the flip done interrupt
-	 * from the mmio flip as the flip done from the CS flip, this will
-	 * not wait for anything. And hence the the CS flip will actually
-	 * occur after this mmio flip.
-	 */
-	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
-			     red_fb.fb_id, 0, 0, &output->id, 1,
-			     mode);
-	igt_assert_eq(ret, 0);
-
-	/* Make sure the flip has been executed */
-	wait_for_flip(data, blue_fb.gem_handle);
-
-	/* Grab crc and compare with the extected result */
-	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
-
-	igt_plane_set_fb(primary, NULL);
-	igt_display_commit(&data->display);
-
-	igt_remove_fb(data->drm_fd, &red_fb);
-	igt_remove_fb(data->drm_fd, &green_fb);
-	igt_remove_fb(data->drm_fd, &blue_fb);
-
-	igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = NULL;
-
-	igt_output_set_pipe(output, PIPE_ANY);
-	igt_display_commit(&data->display);
-
-	igt_assert_crc_equal(&ref_crc, &crc);
-}
-
-/*
- * 1. set primary plane to full red
- * 2. grab a reference crc
- * 3. set primary plane to full green
- * 4. wait for vblank
- * 5. pan primary plane a bit (to cause a mmio flip w/o vblank wait)
- * 6. queue lots of GPU activity to delay the subsequent page flip
- * 6. queue a page flip to a blue fb
- * 7. set primary plane to red fb
- * 8. wait for GPU to finish
- * 9. compare current crc with reference crc
- *
- * We expect the primary plane to display full red at the end.
- * If the previously schedule primary plane pan operation has interfered
- * with the following page flip, the driver may have mistakenly completed
- * the flip before it was executed by the CS, and hence the subsequent mmio
- * flips may have overtaken it. So once we've finished everything
- * the CS flip may have been the last thing to occur, which means
- * the primary plane may be full blue instead of the red it's
- * supposed to be.
- */
-static void
-test_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
-{
-	struct igt_fb red_fb, green_fb, blue_fb;
-	drmModeModeInfo *mode;
-	igt_plane_t *primary;
-	igt_crc_t ref_crc, crc;
-	int ret;
-
-	igt_output_set_pipe(output, pipe);
-
-	primary = igt_output_get_plane(output, 0);
-
-	mode = igt_output_get_mode(output);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    1.0, 0.0, 0.0,
-			    &red_fb);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    0.0, 0.0, 1.0,
-			    &blue_fb);
-	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
-			    DRM_FORMAT_XRGB8888,
-			    LOCAL_DRM_FORMAT_MOD_NONE,
-			    0.0, 1.0, 0.0,
-			    &green_fb);
-
-	/*
-	 * Make sure these buffers are suited for display use
-	 * because most of the modeset operations must be fast
-	 * later on.
-	 */
-	igt_plane_set_fb(primary, &green_fb);
-	igt_display_commit(&data->display);
-	igt_plane_set_fb(primary, &blue_fb);
-	igt_display_commit(&data->display);
-
-	if (data->pipe_crc)
-		igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-
-	/* set red fb and grab reference crc */
-	igt_plane_set_fb(primary, &red_fb);
-	igt_display_commit(&data->display);
-	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
-
-	/*
-	 * Further down we need to issue an mmio flip w/o the kernel
-	 * waiting for vblank. The easiest way is to just pan within
-	 * the same FB. So pan away a bit here, and later we undo this
-	 * with another pan which will result in the desired mmio flip.
-	 */
-	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
-			     green_fb.fb_id, 0, 1, &output->id, 1,
-			     mode);
-	igt_assert_eq(ret, 0);
-
-	/*
-	 * Make it more likely that the CS flip has been submitted into the
-	 * ring by the time the mmio flip from the drmModeSetCrtc() below
-	 * completes. The driver will then mistake the flip done interrupt
-	 * from the mmio flip as the flip done interrupt from the CS flip.
-	 */
-	igt_wait_for_vblank(data->drm_fd, pipe);
-
-	/* now issue the mmio flip w/o vblank waits in the kernel, ie. pan a bit */
-	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
-			     green_fb.fb_id, 0, 0, &output->id, 1,
-			     mode);
-	igt_assert_eq(ret, 0);
-
-	make_gpu_busy(data, blue_fb.gem_handle);
-
-	/*
-	 * Submit the CS flip. The commands must be emitted into the ring
-	 * before the mmio flip from the panning operation completes.
-	 */
-	data->flip_done = false;
-	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
-			      blue_fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, data);
-	igt_assert_eq(ret, 0);
-
-	/*
-	 * Set primary plane to red fb. This should wait for the CS flip
-	 * to complete. But if the kernel mistook the flip done interrupt
-	 * from the mmio flip as the flip done from the CS flip, this will
-	 * not wait for anything. And hence the the CS flip will actually
-	 * occur after this mmio flip.
-	 */
-	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
-			     red_fb.fb_id, 0, 0, &output->id, 1,
-			     mode);
-	igt_assert_eq(ret, 0);
-
-	/* Make sure the flip has been executed */
-	wait_for_flip(data, blue_fb.gem_handle);
-
-	/* Grab crc and compare with the extected result */
-	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
-
-	igt_plane_set_fb(primary, NULL);
-	igt_display_commit(&data->display);
-
-	igt_remove_fb(data->drm_fd, &red_fb);
-	igt_remove_fb(data->drm_fd, &green_fb);
-	igt_remove_fb(data->drm_fd, &blue_fb);
-
-	igt_pipe_crc_free(data->pipe_crc);
-	data->pipe_crc = NULL;
-
-	igt_output_set_pipe(output, PIPE_ANY);
-	igt_display_commit(&data->display);
-
-	igt_assert_crc_equal(&ref_crc, &crc);
-}
-
-static void
-run_plane_test(data_t *data)
-{
-	igt_output_t *output;
-	int plane = 1; /* testing with one sprite is enough */
-	int valid_tests = 0;
-	enum pipe pipe;
-
-	for_each_pipe_with_valid_output(&data->display, pipe, output) {
-		igt_require(data->display.pipes[pipe].n_planes > 2);
-
-		test_plane(data, output, pipe, plane);
-		valid_tests++;
-	}
-
-	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-}
-
-static void
-run_crtc_test(data_t *data)
-{
-	igt_output_t *output;
-	int valid_tests = 0;
-	enum pipe pipe;
-
-	for_each_pipe_with_valid_output(&data->display, pipe, output) {
-		test_crtc(data, output, pipe);
-		valid_tests++;
-	}
-
-	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-}
-
-static data_t data;
-
-igt_main
-{
-	igt_skip_on_simulation();
-
-	igt_fixture {
-		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
-
-		kmstest_set_vt_graphics_mode();
-
-		data.devid = intel_get_drm_devid(data.drm_fd);
-
-		igt_require_pipe_crc(data.drm_fd);
-		igt_display_init(&data.display, data.drm_fd);
-
-		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
-		igt_assert(data.bufmgr);
-		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
-
-		data.busy_bo = drm_intel_bo_alloc(data.bufmgr, "bo",
-						  64*1024*1024, 4096);
-		gem_set_tiling(data.drm_fd, data.busy_bo->handle, 0, 4096);
-	}
-
-	igt_subtest_f("setplane_vs_cs_flip")
-		run_plane_test(&data);
-
-	igt_subtest_f("setcrtc_vs_cs_flip")
-		run_crtc_test(&data);
-
-	igt_fixture {
-		drm_intel_bo_unreference(data.busy_bo);
-		drm_intel_bufmgr_destroy(data.bufmgr);
-		igt_display_fini(&data.display);
-	}
-}
diff --git a/tests/meson.build b/tests/meson.build
index 015afa4742c8..1f00fc4fd983 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -170,7 +170,6 @@ test_progs = [
 	'kms_invalid_dotclock',
 	'kms_legacy_colorkey',
 	'kms_mmap_write_crc',
-	'kms_mmio_vs_cs_flip',
 	'kms_panel_fitting',
 	'kms_pipe_b_c_ivb',
 	'kms_pipe_crc_basic',
-- 
2.17.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2.
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2 Maarten Lankhorst
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip Maarten Lankhorst
@ 2018-05-07 13:14 ` Maarten Lankhorst
  2018-05-07 14:57   ` Daniel Vetter
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 4/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-07 13:14 UTC (permalink / raw)
  To: igt-dev; +Cc: Laurent Pinchart, Daniel Vetter

Because planes will no longer be fixed to a pipe, the index becomes
useless. Use igt_output_get_plane_type() instead to get a plane,
or for_each_plane_on_pipe().

Changes since v1:
- Only test overlay planes in the plane-position tests,
  just like before this commit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_kms.c              | 19 -------------
 lib/igt_kms.h              |  1 -
 tests/kms_concurrent.c     | 22 ++++++++-------
 tests/kms_flip_tiling.c    |  2 +-
 tests/kms_pipe_b_c_ivb.c   |  4 +--
 tests/kms_pipe_crc_basic.c |  2 +-
 tests/kms_plane.c          | 40 ++++++++++++++------------
 tests/kms_plane_multiple.c | 57 +++++++++++++++-----------------------
 tests/kms_plane_scaling.c  | 47 +++++++++++++++++++++++--------
 tests/kms_vblank.c         |  2 +-
 10 files changed, 99 insertions(+), 97 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7281c71607a3..e9b8c4319a61 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2168,15 +2168,6 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 	return &display->pipes[pipe];
 }
 
-static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int plane_idx)
-{
-	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
-		      "Valid pipe->planes plane_idx not found, plane_idx=%d n_planes=%d",
-		      plane_idx, pipe->n_planes);
-
-	return &pipe->planes[plane_idx];
-}
-
 /**
  * igt_pipe_get_plane_type:
  * @pipe: Target pipe
@@ -3482,16 +3473,6 @@ void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
 		igt_pipe_obj_set_prop_changed(pipe_obj, IGT_CRTC_MODE_ID);
 }
 
-igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx)
-{
-	igt_pipe_t *pipe;
-
-	pipe = igt_output_get_driving_pipe(output);
-	igt_assert(pipe);
-
-	return igt_pipe_get_plane(pipe, plane_idx);
-}
-
 /**
  * igt_output_get_plane_type:
  * @output: Target output
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 5e7db0a263fc..fd43e8c86a85 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -388,7 +388,6 @@ const char *igt_output_name(igt_output_t *output);
 drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
 void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
-igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
index 695b14822c9a..976ba677b935 100644
--- a/tests/kms_concurrent.c
+++ b/tests/kms_concurrent.c
@@ -119,12 +119,13 @@ static void
 create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
 			    int *rect_x, int *rect_y,
 			    int *rect_w, int *rect_h,
-			    uint64_t tiling, int max_planes,
+			    uint64_t tiling, enum pipe pipe,
 			    igt_output_t *output)
 {
 	unsigned int fb_id;
 	cairo_t *cr;
 	igt_plane_t *primary;
+	igt_plane_t *plane;
 
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
@@ -140,8 +141,10 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
 			mode->hdisplay, mode->vdisplay,
 			0.0f, 0.0f, 1.0f);
 
-	for (int i = 0; i < max_planes; i++) {
-		if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
+	for_each_plane_on_pipe(&data->display, pipe, plane) {
+		int i = plane->index;
+
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
 
 		igt_paint_color(cr, rect_x[i], rect_y[i],
@@ -152,16 +155,16 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
 }
 
 static void
-prepare_planes(data_t *data, enum pipe pipe, int max_planes,
+prepare_planes(data_t *data, enum pipe pipe,
 	       igt_output_t *output)
 {
+	igt_plane_t *plane;
 	drmModeModeInfo *mode;
 	igt_pipe_t *p;
 	igt_plane_t *primary;
 	int *x;
 	int *y;
 	int *size;
-	int i;
 
 	igt_output_set_pipe(output, pipe);
 
@@ -182,8 +185,8 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
 	/* planes with random positions */
 	x[primary->index] = 0;
 	y[primary->index] = 0;
-	for (i = 0; i < max_planes; i++) {
-		igt_plane_t *plane = igt_output_get_plane(output, i);
+	for_each_plane_on_pipe(&data->display, pipe, plane) {
+		int i = plane->index;
 
 		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
@@ -212,7 +215,7 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
 	data->plane[primary->index] = primary;
 	create_fb_for_mode_position(data, mode, x, y, size, size,
 				    LOCAL_I915_FORMAT_MOD_X_TILED,
-				    max_planes, output);
+				    pipe, output);
 
 	igt_plane_set_fb(data->plane[primary->index], &data->fb[primary->index]);
 }
@@ -223,13 +226,12 @@ test_plane_position_with_output(data_t *data, enum pipe pipe, igt_output_t *outp
 	int i;
 	int iterations = opt.iterations < 1 ? 1 : opt.iterations;
 	bool loop_forever = opt.iterations == LOOP_FOREVER ? true : false;
-	int max_planes = data->display.pipes[pipe].n_planes;
 
 	igt_pipe_refresh(&data->display, pipe, true);
 
 	i = 0;
 	while (i < iterations || loop_forever) {
-		prepare_planes(data, pipe, max_planes, output);
+		prepare_planes(data, pipe, output);
 		igt_display_commit2(&data->display, COMMIT_ATOMIC);
 
 		i++;
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index 5aae29a8d56b..ca486367fdec 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -93,7 +93,7 @@ test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t ti
 	igt_output_set_pipe(output, pipe);
 
 	mode = igt_output_get_mode(output);
-	primary = igt_output_get_plane(output, 0);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 	width = mode->hdisplay;
 
diff --git a/tests/kms_pipe_b_c_ivb.c b/tests/kms_pipe_b_c_ivb.c
index 64086915e8e0..14988bb55b6c 100644
--- a/tests/kms_pipe_b_c_ivb.c
+++ b/tests/kms_pipe_b_c_ivb.c
@@ -71,7 +71,7 @@ disable_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_plane_t *primary;
 
 	igt_output_set_pipe(output, pipe);
-	primary = igt_output_get_plane(output, 0);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(primary, NULL);
 	return igt_display_commit(&data->display);
 }
@@ -88,7 +88,7 @@ set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 
 	mode = igt_output_get_mode(output);
 
-	primary = igt_output_get_plane(output, 0);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 	fb_id = igt_create_color_fb(data->drm_fd,
 				    mode->hdisplay, mode->vdisplay,
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 235fdc386ba2..f5f59015c55d 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -86,7 +86,7 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 					colors[c].b,
 					&data->fb);
 
-		primary = igt_output_get_plane(output, 0);
+		primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 		igt_plane_set_fb(primary, &data->fb);
 
 		igt_display_commit(display);
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 23173b966eab..d5027b25548d 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -72,7 +72,7 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 
 	igt_output_set_pipe(output, pipe);
 
-	primary = igt_output_get_plane(output, 0);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 	mode = igt_output_get_mode(output);
 	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
@@ -149,43 +149,45 @@ enum {
 static void
 test_plane_position_with_output(data_t *data,
 				enum pipe pipe,
-				int plane,
+				igt_plane_t *plane,
 				igt_output_t *output,
 				igt_crc_t *reference_crc,
 				unsigned int flags)
 {
-	igt_plane_t *primary, *sprite;
+	igt_plane_t *primary;
 	struct igt_fb primary_fb, sprite_fb;
 	drmModeModeInfo *mode;
 	igt_crc_t crc, crc2;
 
 	igt_info("Testing connector %s using pipe %s plane %d\n",
-		 igt_output_name(output), kmstest_pipe_name(pipe), plane);
+		 igt_output_name(output), kmstest_pipe_name(pipe), plane->index);
 
 	igt_output_set_pipe(output, pipe);
 
 	mode = igt_output_get_mode(output);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	sprite = igt_output_get_plane(output, plane);
 
 	create_fb_for_mode__position(data, mode, 100, 100, 64, 64,
 				     &primary_fb);
 	igt_plane_set_fb(primary, &primary_fb);
 
+	if (primary == plane && !data->display.is_atomic)
+		igt_display_commit(&data->display);
+
 	igt_create_color_fb(data->drm_fd,
 				64, 64, /* width, height */
 				DRM_FORMAT_XRGB8888,
 				LOCAL_DRM_FORMAT_MOD_NONE,
 				0.0, 1.0, 0.0,
 				&sprite_fb);
-	igt_plane_set_fb(sprite, &sprite_fb);
+	igt_plane_set_fb(plane, &sprite_fb);
 
 	if (flags & TEST_POSITION_FULLY_COVERED)
-		igt_plane_set_position(sprite, 100, 100);
+		igt_plane_set_position(plane, 100, 100);
 	else
-		igt_plane_set_position(sprite, 132, 132);
+		igt_plane_set_position(plane, 132, 132);
 
-	igt_display_commit(&data->display);
+	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
 
 	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
 
@@ -209,7 +211,7 @@ test_plane_position_with_output(data_t *data,
 	igt_assert_crc_equal(&crc, &crc2);
 
 	igt_plane_set_fb(primary, NULL);
-	igt_plane_set_fb(sprite, NULL);
+	igt_plane_set_fb(plane, NULL);
 
 	/* reset the constraint on the pipe */
 	igt_output_set_pipe(output, PIPE_ANY);
@@ -222,17 +224,21 @@ test_plane_position(data_t *data, enum pipe pipe, unsigned int flags)
 	int connected_outs = 0;
 
 	for_each_valid_output_on_pipe(&data->display, pipe, output) {
-		int n_planes = data->display.pipes[pipe].n_planes;
+		igt_plane_t *plane;
 		igt_crc_t reference_crc;
 
 		test_init(data, pipe);
 
 		test_grab_crc(data, output, pipe, &green, &reference_crc);
 
-		for (int plane = 1; plane < n_planes; plane++)
+		for_each_plane_on_pipe(&data->display, pipe, plane) {
+			if (plane->type != DRM_PLANE_TYPE_OVERLAY)
+				continue;
+
 			test_plane_position_with_output(data, pipe, plane,
 							output, &reference_crc,
 							flags);
+		}
 
 		test_fini(data);
 
@@ -290,7 +296,7 @@ enum {
 static void
 test_plane_panning_with_output(data_t *data,
 			       enum pipe pipe,
-			       int plane,
+			       igt_plane_t *plane,
 			       igt_output_t *output,
 			       igt_crc_t *red_crc, igt_crc_t *blue_crc,
 			       unsigned int flags)
@@ -301,12 +307,12 @@ test_plane_panning_with_output(data_t *data,
 	igt_crc_t crc;
 
 	igt_info("Testing connector %s using pipe %s plane %d\n",
-		 igt_output_name(output), kmstest_pipe_name(pipe), plane);
+		 igt_output_name(output), kmstest_pipe_name(pipe), plane->index);
 
 	igt_output_set_pipe(output, pipe);
 
 	mode = igt_output_get_mode(output);
-	primary = igt_output_get_plane(output, 0);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 	create_fb_for_mode__panning(data, mode, &primary_fb);
 	igt_plane_set_fb(primary, &primary_fb);
@@ -343,7 +349,7 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
 	int connected_outs = 0;
 
 	for_each_valid_output_on_pipe(&data->display, pipe, output) {
-		int n_planes = data->display.pipes[pipe].n_planes;
+		igt_plane_t *plane;
 		igt_crc_t red_crc;
 		igt_crc_t blue_crc;
 
@@ -352,7 +358,7 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
 		test_grab_crc(data, output, pipe, &red, &red_crc);
 		test_grab_crc(data, output, pipe, &blue, &blue_crc);
 
-		for (int plane = 1; plane < n_planes; plane++)
+		for_each_plane_on_pipe(&data->display, pipe, plane)
 			test_plane_panning_with_output(data, pipe, plane,
 						       output,
 						       &red_crc, &blue_crc,
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index 366293b3f9ee..45e0fd31fce3 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -65,8 +65,10 @@ struct {
 /*
  * Common code across all tests, acting on data_t
  */
-static void test_init(data_t *data, enum pipe pipe, int n_planes)
+static void test_init(data_t *data, enum pipe pipe)
 {
+	int n_planes = data->display.pipes[pipe].n_planes;
+
 	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 
 	data->plane = calloc(n_planes, sizeof(data->plane));
@@ -76,23 +78,10 @@ static void test_init(data_t *data, enum pipe pipe, int n_planes)
 	igt_assert_f(data->fb != NULL, "Failed to allocate memory for FBs\n");
 }
 
-static void test_fini(data_t *data, igt_output_t *output, int n_planes)
+static void test_fini(data_t *data, igt_output_t *output)
 {
 	igt_pipe_crc_stop(data->pipe_crc);
 
-	for (int i = 0; i < n_planes; i++) {
-		igt_plane_t *plane = data->plane[i];
-		if (!plane)
-			continue;
-		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-			continue;
-		igt_plane_set_fb(plane, NULL);
-		data->plane[i] = NULL;
-	}
-
-	/* reset the constraint on the pipe */
-	igt_output_set_pipe(output, PIPE_ANY);
-
 	igt_pipe_crc_free(data->pipe_crc);
 	data->pipe_crc = NULL;
 
@@ -149,11 +138,11 @@ static void
 create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo *mode,
 			    color_t *color, int *rect_x, int *rect_y,
 			    int *rect_w, int *rect_h, uint64_t tiling,
-			    int max_planes)
+			    enum pipe pipe)
 {
 	unsigned int fb_id;
 	cairo_t *cr;
-	igt_plane_t *primary;
+	igt_plane_t *primary, *plane;
 
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
@@ -169,12 +158,15 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
 			mode->hdisplay, mode->vdisplay,
 			color->red, color->green, color->blue);
 
-	for (int i = 0; i < max_planes; i++) {
-		if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
+	for_each_plane_on_pipe(&data->display, pipe, plane) {
+		int i = plane->index;
+
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
+
 		igt_paint_color(cr, rect_x[i], rect_y[i],
 				rect_w[i], rect_h[i], 0.0, 0.0, 0.0);
-		}
+	}
 
 	igt_put_cairo_ctx(data->drm_fd, &data->fb[primary->index], cr);
 }
@@ -182,16 +174,15 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
 
 static void
 prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
-	       uint64_t tiling, int max_planes, igt_output_t *output)
+	       uint64_t tiling, igt_output_t *output)
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = output->display;
 	igt_pipe_t *pipe = &display->pipes[pipe_id];
-	igt_plane_t *primary;
+	igt_plane_t *primary, *plane;
 	int *x;
 	int *y;
 	int *size;
-	int i;
 
 	igt_output_set_pipe(output, pipe_id);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
@@ -208,8 +199,8 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 	/* planes with random positions */
 	x[primary->index] = 0;
 	y[primary->index] = 0;
-	for (i = 0; i < max_planes; i++) {
-		igt_plane_t *plane = igt_output_get_plane(output, i);
+	for_each_plane_on_pipe(display, pipe_id, plane) {
+		int i = plane->index;
 
 		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 			continue;
@@ -237,13 +228,13 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 	/* primary plane */
 	data->plane[primary->index] = primary;
 	create_fb_for_mode_position(data, output, mode, color, x, y,
-				    size, size, tiling, max_planes);
+				    size, size, tiling, pipe_id);
 	igt_plane_set_fb(data->plane[primary->index], &data->fb[primary->index]);
 }
 
 static void
 test_plane_position_with_output(data_t *data, enum pipe pipe,
-				igt_output_t *output, int n_planes,
+				igt_output_t *output,
 				uint64_t tiling)
 {
 	color_t blue  = { 0.0f, 0.0f, 1.0f };
@@ -262,17 +253,17 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 			iterations, iterations > 1 ? "iterations" : "iteration");
 	}
 
-	igt_info("Testing connector %s using pipe %s with %d planes %s with seed %d\n",
-		 igt_output_name(output), kmstest_pipe_name(pipe), n_planes,
+	igt_info("Testing connector %s using pipe %s %s with seed %d\n",
+		 igt_output_name(output), kmstest_pipe_name(pipe),
 		 info, opt.seed);
 
-	test_init(data, pipe, n_planes);
+	test_init(data, pipe);
 
 	test_grab_crc(data, output, pipe, &blue, tiling);
 
 	i = 0;
 	while (i < iterations || loop_forever) {
-		prepare_planes(data, pipe, &blue, tiling, n_planes, output);
+		prepare_planes(data, pipe, &blue, tiling, output);
 
 		igt_display_commit2(&data->display, COMMIT_ATOMIC);
 
@@ -284,7 +275,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 		i++;
 	}
 
-	test_fini(data, output, n_planes);
+	test_fini(data, output);
 }
 
 static void
@@ -293,7 +284,6 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t tiling)
 	igt_output_t *output;
 	int connected_outs;
 	int devid = intel_get_drm_devid(data->drm_fd);
-	int n_planes = data->display.pipes[pipe].n_planes;
 
 	if ((tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
 	     tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED))
@@ -308,7 +298,6 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t tiling)
 	for_each_valid_output_on_pipe(&data->display, pipe, output) {
 		test_plane_position_with_output(data, pipe,
 						output,
-						n_planes,
 						tiling);
 		connected_outs++;
 	}
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index a8454205dbd8..984594753db8 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -257,6 +257,29 @@ static void iterate_plane_scaling(data_t *d, drmModeModeInfo *mode)
 	}
 }
 
+static void set_plane_mappings(data_t *d, igt_output_t *output, enum pipe pipe)
+{
+	igt_plane_t *plane;
+
+	igt_output_set_pipe(output, pipe);
+
+	/* Set up display with plane 1 */
+	d->plane1 = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+
+	d->plane2 = d->plane3 = NULL;
+
+	for_each_plane_on_pipe(&d->display, pipe, plane) {
+		if (plane->type != DRM_PLANE_TYPE_OVERLAY)
+			continue;
+
+		if (!d->plane2)
+			d->plane2 = plane;
+		else if (!d->plane3)
+			d->plane3 = plane;
+	}
+
+}
+
 static void
 test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 {
@@ -266,8 +289,7 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 
 	mode = igt_output_get_mode(output);
 
-	/* Set up display with plane 1 */
-	d->plane1 = &display->pipes[pipe].planes[0];
+	set_plane_mappings(d, output, pipe);
 	prepare_crtc(d, output, pipe, d->plane1, mode);
 
 	igt_create_color_pattern_fb(display->drm_fd, 600, 600,
@@ -298,7 +320,6 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 	}
 
 	/* Set up fb[1]->plane2 mapping. */
-	d->plane2 = igt_output_get_plane(output, 1);
 	igt_plane_set_fb(d->plane2, &d->fb[1]);
 
 	/* 2nd plane windowed */
@@ -336,10 +357,9 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
 	}
 
 	/* Set up fb[2]->plane3 mapping. */
-	d->plane3 = igt_output_get_plane(output, 2);
 	igt_plane_set_fb(d->plane3, &d->fb[2]);
 
-	if(d->plane3->type == DRM_PLANE_TYPE_CURSOR) {
+	if(!d->plane3) {
 		igt_debug("Plane-3 doesnt exist on pipe %s\n", kmstest_pipe_name(pipe));
 		return;
 	}
@@ -418,8 +438,7 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe, igt_outpu
 	igt_require(get_num_scalers(d->devid, pipe) >= 2);
 
 	mode = igt_output_get_mode(output);
-	d->plane1 = &d->display.pipes[pipe].planes[0];
-	d->plane2 = &d->display.pipes[pipe].planes[1];
+	set_plane_mappings(d, output, pipe);
 	prepare_crtc(d, output, pipe, d->plane1, mode);
 
 	for (int i = 0; i < d->plane1->drm_plane->count_formats; i++) {
@@ -481,10 +500,16 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
 	igt_output_set_pipe(output1, pipe1);
 	igt_output_set_pipe(output2, pipe2);
 
-	d->plane1 = igt_output_get_plane(output1, 0);
-	d->plane2 = get_num_scalers(d->devid, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
-	d->plane3 = igt_output_get_plane(output2, 0);
-	d->plane4 = get_num_scalers(d->devid, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
+	d->plane1 = igt_output_get_plane_type(output1, DRM_PLANE_TYPE_PRIMARY);
+	if (get_num_scalers(d->devid, pipe1) >= 2)
+		d->plane2 = igt_output_get_plane_type(output1, DRM_PLANE_TYPE_OVERLAY);
+	else
+		d->plane2 = NULL;
+	d->plane3 = igt_output_get_plane_type(output2, DRM_PLANE_TYPE_PRIMARY);
+	if (get_num_scalers(d->devid, pipe2) >= 2)
+		d->plane4 = igt_output_get_plane_type(output2, DRM_PLANE_TYPE_OVERLAY);
+	else
+		d->plane4 = NULL;
 
 	mode1 = igt_output_get_mode(output1);
 	mode2 = igt_output_get_mode(output2);
diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 2bee49de55eb..3f9ab34d35bc 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -208,7 +208,7 @@ static void crtc_id_subtest(data_t *data, int fd)
 		igt_assert_eq(buf.crtc_id, expected_crtc_id);
 
 		if (display->is_atomic) {
-			igt_plane_t *primary = igt_output_get_plane(output, 0);
+			igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 			igt_plane_set_fb(primary, &data->primary_fb);
 			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
-- 
2.17.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [igt-dev] [PATCH i-g-t 4/4] lib/igt_kms: Move planes from pipe to display, v2.
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2 Maarten Lankhorst
@ 2018-05-07 13:14 ` Maarten Lankhorst
  2018-05-07 14:15 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
  2018-05-07 15:40 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-07 13:14 UTC (permalink / raw)
  To: igt-dev; +Cc: Laurent Pinchart, Daniel Vetter

This doesn't get rid of the separate copies yet, just moving the
planes from pipe to display.

Changes since v1:
- Remove crash in kms_atomic_transition by using igt_display_reset instead.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_kms.c                 | 135 +++++++++++++++++-----------------
 lib/igt_kms.h                 |  17 +++--
 tests/kms_atomic_transition.c |  74 ++++++++++---------
 tests/kms_color.c             |   2 +-
 tests/kms_concurrent.c        |  26 +++----
 tests/kms_cursor_legacy.c     |   4 +-
 tests/kms_plane.c             |   4 +-
 tests/kms_plane_lowres.c      |   4 +-
 tests/kms_plane_multiple.c    |  18 ++---
 tests/kms_rotation_crc.c      |  12 +--
 tests/kms_universal_plane.c   |   9 +--
 11 files changed, 151 insertions(+), 154 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e9b8c4319a61..35c6b1f6b4de 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1846,19 +1846,24 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 	plane_resources = drmModeGetPlaneResources(display->drm_fd);
 	igt_assert(plane_resources);
 
+	/*
+	 * Until we fix the API, each pipe currently get its own copy
+	 * of the same plane.
+	 */
+	display->planes = calloc(sizeof(igt_plane_t), display->n_pipes *
+				 (2 + plane_resources->count_planes));
+	igt_assert_f(display->planes, "Failed to allocate memory for planes\n");
+
 	for_each_pipe(display, i) {
 		igt_pipe_t *pipe = &display->pipes[i];
 		igt_plane_t *plane;
-		int p = 1;
 		int j, type;
-		uint8_t last_plane = 0, n_planes = 0;
+		uint8_t first_plane = display->n_planes, last_plane, n_planes = 0;
+		int p = first_plane + 1;
 
 		pipe->crtc_id = resources->crtcs[i];
 		pipe->display = display;
 		pipe->pipe = i;
-		pipe->plane_cursor = -1;
-		pipe->plane_primary = -1;
-		pipe->planes = NULL;
 
 		igt_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
 
@@ -1876,10 +1881,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 			drmModeFreePlane(drm_plane);
 		}
 
-		igt_assert_lte(0, n_planes);
-		pipe->planes = calloc(sizeof(igt_plane_t), n_planes);
-		igt_assert_f(pipe->planes, "Failed to allocate memory for %d planes\n", n_planes);
-		last_plane = n_planes - 1;
+		last_plane = first_plane + n_planes - 1;
 
 		/* add the planes that can be used with that pipe */
 		for (j = 0; j < plane_resources->count_planes; j++) {
@@ -1897,21 +1899,23 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 			type = get_drm_plane_type(display->drm_fd,
 						  plane_resources->planes[j]);
 
-			if (type == DRM_PLANE_TYPE_PRIMARY && pipe->plane_primary == -1) {
-				plane = &pipe->planes[0];
-				plane->index = 0;
-				pipe->plane_primary = 0;
-			} else if (type == DRM_PLANE_TYPE_CURSOR && pipe->plane_cursor == -1) {
-				plane = &pipe->planes[last_plane];
+			if (type == DRM_PLANE_TYPE_PRIMARY &&
+			    !display->planes[first_plane].drm_plane) {
+				plane = &display->planes[first_plane];
+				plane->index = first_plane;
+			} else if (type == DRM_PLANE_TYPE_CURSOR &&
+				   !display->planes[last_plane].drm_plane) {
+				plane = &display->planes[last_plane];
 				plane->index = last_plane;
-				pipe->plane_cursor = last_plane;
 				display->has_cursor_plane = true;
 			} else {
-				plane = &pipe->planes[p];
+				plane = &display->planes[p];
 				plane->index = p++;
+
+				/* override type */
+				type = DRM_PLANE_TYPE_OVERLAY;
 			}
 
-			igt_assert_f(plane->index < n_planes, "n_planes < plane->index failed\n");
 			plane->type = type;
 			plane->display = display;
 			plane->bound_pipe = i;
@@ -1925,9 +1929,9 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 
 		/*
 		 * At the bare minimum, we should expect to have a primary
-		 * plane, and it must be in slot 0.
+		 * plane, and it must be in the first slot.
 		 */
-		igt_assert_eq(pipe->plane_primary, 0);
+		igt_assert(display->planes[first_plane].drm_plane);
 
 		/* Check that we filled every slot exactly once */
 		if (display->has_cursor_plane)
@@ -1935,7 +1939,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 		else
 			igt_assert_eq(p, n_planes);
 
-		pipe->n_planes = n_planes;
+		display->n_planes += n_planes;
 	}
 
 	igt_fill_display_format_mod(display);
@@ -2060,24 +2064,20 @@ igt_output_t *igt_output_from_connector(igt_display_t *display,
 	return found;
 }
 
-static void igt_pipe_fini(igt_pipe_t *pipe)
+static void igt_plane_fini(igt_plane_t *plane)
 {
-	int i;
-
-	for (i = 0; i < pipe->n_planes; i++) {
-		igt_plane_t *plane = &pipe->planes[i];
-
-		if (plane->drm_plane) {
-			drmModeFreePlane(plane->drm_plane);
-			plane->drm_plane = NULL;
-		}
+	if (plane->drm_plane) {
+		drmModeFreePlane(plane->drm_plane);
+		plane->drm_plane = NULL;
 	}
+}
 
-	free(pipe->planes);
-	pipe->planes = NULL;
-
-	if (pipe->out_fence_fd != -1)
+static void igt_pipe_fini(igt_pipe_t *pipe)
+{
+	if (pipe->out_fence_fd != -1) {
 		close(pipe->out_fence_fd);
+		pipe->out_fence_fd = -1;
+	}
 }
 
 static void igt_output_fini(igt_output_t *output)
@@ -2098,15 +2098,23 @@ void igt_display_fini(igt_display_t *display)
 {
 	int i;
 
+	for (i = 0; i < display->n_planes; i++)
+		igt_plane_fini(&display->planes[i]);
+	free(display->planes);
+	display->planes = NULL;
+
+
 	for (i = 0; i < display->n_pipes; i++)
 		igt_pipe_fini(&display->pipes[i]);
 
+	free(display->pipes);
+	display->pipes = NULL;
+
 	for (i = 0; i < display->n_outputs; i++)
 		igt_output_fini(&display->outputs[i]);
+
 	free(display->outputs);
 	display->outputs = NULL;
-	free(display->pipes);
-	display->pipes = NULL;
 }
 
 static void igt_display_refresh(igt_display_t *display)
@@ -2180,29 +2188,18 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
  */
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type)
 {
-	int i, plane_idx = -1;
+	igt_plane_t *plane;
 
-	switch(plane_type) {
-	case DRM_PLANE_TYPE_CURSOR:
-		plane_idx = pipe->plane_cursor;
-		break;
-	case DRM_PLANE_TYPE_PRIMARY:
-		plane_idx = pipe->plane_primary;
-		break;
-	case DRM_PLANE_TYPE_OVERLAY:
-		for(i = 0; i < pipe->n_planes; i++)
-			if (pipe->planes[i].type == DRM_PLANE_TYPE_OVERLAY)
-			    plane_idx = i;
-		break;
-	default:
-		break;
-	}
+	for_each_plane_on_pipe(pipe->display, pipe->pipe, plane)
+		if (plane->type == plane_type)
+			return plane;
 
-	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
-		      "Valid pipe->planes idx not found. plane_idx=%d plane_type=%d n_planes=%d\n",
-		      plane_idx, plane_type, pipe->n_planes);
+	plane = NULL;
+	igt_require_f(plane,
+		      "Valid plane type not found. plane_type=%d, pipe=%s\n",
+		      plane_type, kmstest_pipe_name(pipe->pipe));
 
-	return &pipe->planes[plane_idx];
+	return plane;
 }
 
 static bool output_is_internal_panel(igt_output_t *output)
@@ -2328,17 +2325,17 @@ static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
  * Add position and fb changes of a plane to the atomic property set
  */
 static void
-igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
+igt_atomic_prepare_plane_commit(igt_plane_t *plane,
 	drmModeAtomicReq *req)
 {
-	igt_display_t *display = pipe->display;
+	igt_display_t *display = plane->display;
 	int i;
 
 	igt_assert(plane->drm_plane);
 
 	LOG(display,
 	    "populating plane data: %s.%d, fb %u\n",
-	    kmstest_pipe_name(pipe->pipe),
+	    kmstest_pipe_name(plane->bound_pipe),
 	    plane->index,
 	    igt_plane_get_fb_id(plane));
 
@@ -2350,7 +2347,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
 		igt_assert(plane->props[i]);
 
 		igt_debug("plane %s.%d: Setting property \"%s\" to 0x%"PRIx64"/%"PRIi64"\n",
-			kmstest_pipe_name(pipe->pipe), plane->index, igt_plane_prop_names[i],
+			kmstest_pipe_name(plane->bound_pipe), plane->index, igt_plane_prop_names[i],
 			plane->values[i], plane->values[i]);
 
 		igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,
@@ -2697,6 +2694,7 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
 			   enum igt_commit_style s,
 			   bool fail_on_error)
 {
+	igt_plane_t *plane;
 	int i;
 	int ret;
 
@@ -2712,9 +2710,7 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
 			CHECK_RETURN(ret, fail_on_error);
 		}
 
-	for (i = 0; i < pipe->n_planes; i++) {
-		igt_plane_t *plane = &pipe->planes[i];
-
+	for_each_plane_on_pipe(pipe->display, pipe->pipe, plane) {
 		ret = igt_plane_commit(plane, pipe, s, fail_on_error);
 		CHECK_RETURN(ret, fail_on_error);
 	}
@@ -3004,7 +3000,6 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
 
 	for_each_pipe(display, pipe) {
 		igt_pipe_t *pipe_obj = &display->pipes[pipe];
-		igt_plane_t *plane;
 
 		/*
 		 * Add CRTC Properties to the property set
@@ -3012,11 +3007,13 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
 		if (pipe_obj->changed)
 			igt_atomic_prepare_crtc_commit(pipe_obj, req);
 
-		for_each_plane_on_pipe(display, pipe, plane) {
-			if (plane->changed)
-				igt_atomic_prepare_plane_commit(plane, pipe_obj, req);
-		}
+	}
+
+	for (i = 0; i < display->n_planes; i++) {
+		igt_plane_t *plane = &display->planes[i];
 
+		if (plane->changed)
+			igt_atomic_prepare_plane_commit(plane, req);
 	}
 
 	for (i = 0; i < display->n_outputs; i++) {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index fd43e8c86a85..b7a3bb156e71 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -323,11 +323,6 @@ struct igt_pipe {
 	igt_display_t *display;
 	enum pipe pipe;
 
-	int n_planes;
-	int plane_cursor;
-	int plane_primary;
-	igt_plane_t *planes;
-
 	uint64_t changed;
 	uint32_t props[IGT_NUM_CRTC_PROPS];
 	uint64_t values[IGT_NUM_CRTC_PROPS];
@@ -358,10 +353,15 @@ typedef struct {
 struct igt_display {
 	int drm_fd;
 	int log_shift;
+
+	int n_planes;
 	int n_pipes;
 	int n_outputs;
-	igt_output_t *outputs;
+
+	igt_plane_t *planes;
 	igt_pipe_t *pipes;
+	igt_output_t *outputs;
+
 	bool has_cursor_plane;
 	bool is_atomic;
 	bool first_commit;
@@ -529,8 +529,9 @@ igt_output_t **__igt_pipe_populate_outputs(igt_display_t *display,
 		for_each_if (igt_pipe_connector_valid((pipe), (output)))
 
 #define for_each_plane_on_pipe(display, pipe, plane)			\
-	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->pipes[(pipe)].planes[j__], \
-		     j__ < (display)->pipes[(pipe)].n_planes; j__++)
+	for (int j__ = 0; assert(igt_can_fail()), (plane) = &(display)->planes[j__], \
+		     j__ < (display)->n_planes; j__++) \
+		for_each_if ((plane)->bound_pipe == pipe)
 
 #define IGT_FIXED(i,f)	((i) << 16 | (f))
 
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 2fbd94bd2c57..5080087007eb 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -40,6 +40,7 @@
 #endif
 
 struct plane_parms {
+	igt_plane_t *plane;
 	struct igt_fb *fb;
 	uint32_t width, height;
 };
@@ -119,19 +120,13 @@ static void configure_fencing(igt_plane_t *plane)
 
 static void
 wm_setup_plane(igt_display_t *display, enum pipe pipe,
-	       uint32_t mask, struct plane_parms *parms, bool fencing)
+	       uint64_t mask, struct plane_parms *parms,
+	       bool fencing)
 {
-	igt_plane_t *plane;
+	for (int i = 0; parms[i].plane; i++) {
+		igt_plane_t *plane = parms[i].plane;
 
-	/*
-	* Make sure these buffers are suited for display use
-	* because most of the modeset operations must be fast
-	* later on.
-	*/
-	for_each_plane_on_pipe(display, pipe, plane) {
-		int i = plane->index;
-
-		if (!((1 << plane->index) & mask)) {
+		if (!((1 << i) & mask)) {
 			if (plane->values[IGT_PLANE_FB_ID])
 				igt_plane_set_fb(plane, NULL);
 			continue;
@@ -168,10 +163,10 @@ static void set_sprite_wh(igt_display_t *display, enum pipe pipe,
 			  struct plane_parms *parms, struct igt_fb *sprite_fb,
 			  bool alpha, unsigned w, unsigned h)
 {
-	igt_plane_t *plane;
+	int i;
 
-	for_each_plane_on_pipe(display, pipe, plane) {
-		int i = plane->index;
+	for (i = 0; parms[i].plane; i++) {
+		igt_plane_t *plane = parms[i].plane;
 
 		if (plane->type == DRM_PLANE_TYPE_PRIMARY ||
 		    plane->type == DRM_PLANE_TYPE_CURSOR)
@@ -187,17 +182,17 @@ static void set_sprite_wh(igt_display_t *display, enum pipe pipe,
 		      LOCAL_DRM_FORMAT_MOD_NONE, sprite_fb);
 }
 
-static void setup_parms(igt_display_t *display, enum pipe pipe,
-			const drmModeModeInfo *mode,
-			struct igt_fb *primary_fb,
-			struct igt_fb *argb_fb,
-			struct igt_fb *sprite_fb,
-			struct plane_parms *parms)
+static int setup_parms(igt_display_t *display, enum pipe pipe,
+		       const drmModeModeInfo *mode,
+		       struct igt_fb *primary_fb,
+		       struct igt_fb *argb_fb,
+		       struct igt_fb *sprite_fb,
+		       struct plane_parms *parms)
 {
 	uint64_t cursor_width, cursor_height;
 	unsigned sprite_width, sprite_height, prev_w, prev_h;
 	bool max_sprite_width, max_sprite_height, alpha = true;
-	uint32_t n_planes = display->pipes[pipe].n_planes;
+	uint32_t n_planes = 0;
 	igt_plane_t *plane;
 
 	do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
@@ -209,8 +204,9 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 		cursor_height = mode->vdisplay;
 
 	for_each_plane_on_pipe(display, pipe, plane) {
-		int i = plane->index;
+		int i = n_planes++;
 
+		parms[i].plane = plane;
 		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
 			parms[i].fb = primary_fb;
 			parms[i].width = mode->hdisplay;
@@ -223,6 +219,9 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 			parms[i].fb = sprite_fb;
 	}
 
+	/* Set the sentinel */
+	parms[n_planes].plane = NULL;
+
 	igt_create_fb(display->drm_fd, cursor_width, cursor_height,
 		      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, argb_fb);
 
@@ -230,7 +229,7 @@ static void setup_parms(igt_display_t *display, enum pipe pipe,
 		      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, sprite_fb);
 
 	if (n_planes < 3)
-		return;
+		return n_planes;
 
 	/*
 	 * Pre gen9 not all sizes are supported, find the biggest possible
@@ -305,6 +304,8 @@ retry:
 	igt_info("Running test on pipe %s with resolution %dx%d and sprite size %dx%d alpha %i\n",
 		 kmstest_pipe_name(pipe), mode->hdisplay, mode->vdisplay,
 		 sprite_width, sprite_height, alpha);
+
+	return n_planes;
 }
 
 static void prepare_fencing(igt_display_t *display, enum pipe pipe)
@@ -314,7 +315,7 @@ static void prepare_fencing(igt_display_t *display, enum pipe pipe)
 
 	igt_require_sw_sync();
 
-	n_planes = display->pipes[pipe].n_planes;
+	n_planes = display->n_planes;
 	timeline = calloc(sizeof(*timeline), n_planes);
 	igt_assert_f(timeline != NULL, "Failed to allocate memory for timelines\n");
 	thread = calloc(sizeof(*thread), n_planes);
@@ -390,10 +391,10 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 	drmModeModeInfo *mode, override_mode;
 	igt_plane_t *plane;
 	igt_pipe_t *pipe_obj = &display->pipes[pipe];
-	uint32_t iter_max = 1 << pipe_obj->n_planes, i;
-	struct plane_parms parms[pipe_obj->n_planes];
+	uint64_t iter_max, i;
+	struct plane_parms parms[display->n_planes + 1];
 	unsigned flags = 0;
-	int ret;
+	int ret, n_planes;
 
 	if (fencing)
 		prepare_fencing(display, pipe);
@@ -414,10 +415,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 	igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
 		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
 
+	igt_display_reset(display);
 	igt_output_set_pipe(output, pipe);
 
-	wm_setup_plane(display, pipe, 0, NULL, false);
-
 	if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
 		igt_output_set_pipe(output, PIPE_NONE);
 
@@ -428,7 +428,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 	igt_display_commit2(display, COMMIT_ATOMIC);
 
-	setup_parms(display, pipe, mode, &fb, &argb_fb, &sprite_fb, parms);
+	n_planes = setup_parms(display, pipe, mode, &fb, &argb_fb, &sprite_fb, parms);
+	iter_max = (1 << n_planes) - 1;
 
 	/*
 	 * In some configurations the tests may not run to completion with all
@@ -442,11 +443,12 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 			igt_pipe_request_out_fence(pipe_obj);
 
 		ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
-		if (ret != -EINVAL || pipe_obj->n_planes < 3)
+		if (ret != -EINVAL || !igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_OVERLAY))
 			break;
 
 		ret = 0;
-		for_each_plane_on_pipe(display, pipe, plane) {
+		for (i = 0; parms[i].plane; i++) {
+			plane = parms[i].plane;
 			i = plane->index;
 
 			if (plane->type == DRM_PLANE_TYPE_PRIMARY ||
@@ -458,7 +460,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 			parms[i].width /= 2;
 			ret = 1;
-			igt_info("Reducing sprite %i to %ux%u\n", i - 1, parms[i].width, parms[i].height);
+			igt_info("Reducing sprite %"PRIi64" to %ux%u\n", i - 1, parms[i].width, parms[i].height);
 			break;
 		}
 
@@ -500,7 +502,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 		if (type == TRANSITION_MODESET_FAST &&
 		    n_enable_planes > 1 &&
-		    n_enable_planes < pipe_obj->n_planes)
+		    n_enable_planes < display->n_planes)
 			continue;
 
 		igt_output_set_pipe(output, pipe);
@@ -526,7 +528,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 				if (type == TRANSITION_MODESET_FAST &&
 				    n_enable_planes > 1 &&
-				    n_enable_planes < pipe_obj->n_planes)
+				    n_enable_planes < n_planes)
 					continue;
 
 				wm_setup_plane(display, pipe, j, parms, fencing);
@@ -676,6 +678,8 @@ static void collect_crcs_mask(igt_pipe_crc_t **pipe_crcs, unsigned mask, igt_crc
 	}
 }
 
+
+
 static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing)
 {
 	struct igt_fb fbs[2];
diff --git a/tests/kms_color.c b/tests/kms_color.c
index ce364713f665..b3794b34f257 100644
--- a/tests/kms_color.c
+++ b/tests/kms_color.c
@@ -855,9 +855,9 @@ run_tests_for_pipe(data_t *data, enum pipe p)
 		igt_require(p < data->display.n_pipes);
 
 		pipe = &data->display.pipes[p];
-		igt_require(pipe->n_planes >= 0);
 
 		primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(primary);
 
 		data->pipe_crc = igt_pipe_crc_new(data->drm_fd, p,
 						  INTEL_PIPE_CRC_SOURCE_AUTO);
diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
index 976ba677b935..f12b3a626a0b 100644
--- a/tests/kms_concurrent.c
+++ b/tests/kms_concurrent.c
@@ -158,9 +158,9 @@ static void
 prepare_planes(data_t *data, enum pipe pipe,
 	       igt_output_t *output)
 {
+	igt_display_t *display = &data->display;
 	igt_plane_t *plane;
 	drmModeModeInfo *mode;
-	igt_pipe_t *p;
 	igt_plane_t *primary;
 	int *x;
 	int *y;
@@ -169,16 +169,15 @@ prepare_planes(data_t *data, enum pipe pipe,
 	igt_output_set_pipe(output, pipe);
 
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-	p = &data->display.pipes[pipe];
 
-	x = malloc(p->n_planes * sizeof(*x));
-	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (p->n_planes * sizeof(*x)));
+	x = malloc(display->n_planes * sizeof(*x));
+	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (display->n_planes * sizeof(*x)));
 
-	y = malloc(p->n_planes * sizeof(*y));
-	igt_assert_f(y, "Failed to allocate %ld bytes for variable y\n", (long int) (p->n_planes * sizeof(*y)));
+	y = malloc(display->n_planes * sizeof(*y));
+	igt_assert_f(y, "Failed to allocate %ld bytes for variable y\n", (long int) (display->n_planes * sizeof(*y)));
 
-	size = malloc(p->n_planes * sizeof(*size));
-	igt_assert_f(size, "Failed to allocate %ld bytes for variable size\n", (long int) (p->n_planes * sizeof(*size)));
+	size = malloc(display->n_planes * sizeof(*size));
+	igt_assert_f(size, "Failed to allocate %ld bytes for variable size\n", (long int) (display->n_planes * sizeof(*size)));
 
 	mode = igt_output_get_mode(output);
 
@@ -313,7 +312,7 @@ static void
 run_test(data_t *data, enum pipe pipe, igt_output_t *output)
 {
 	int connected_outs;
-	int n_planes = data->display.pipes[pipe].n_planes;
+	int n_planes = data->display.n_planes;
 
 	if (!opt.user_seed)
 		opt.seed = time(NULL);
@@ -347,15 +346,8 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
 	igt_output_t *output;
 
 	igt_fixture {
-		int valid_tests = 0;
-
 		igt_skip_on(pipe >= data->display.n_pipes);
-		igt_require(data->display.pipes[pipe].n_planes > 0);
-
-		for_each_valid_output_on_pipe(&data->display, pipe, output)
-			valid_tests++;
-
-		igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+		igt_display_require_output_on_pipe(&data->display, pipe);
 	}
 
 	igt_subtest_f("pipe-%s", kmstest_pipe_name(pipe))
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index d0a28b3c442c..1b2d90e29cca 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -371,8 +371,8 @@ static void prepare_flip_test(igt_display_t *display,
 
 	if (mode == flip_test_atomic_transitions ||
 	    mode == flip_test_atomic_transitions_varying_size) {
-		igt_require(display->pipes[flip_pipe].n_planes > 1 &&
-		            display->pipes[flip_pipe].planes[1].type != DRM_PLANE_TYPE_CURSOR);
+		igt_pipe_t *flip_pipe_obj = &display->pipes[flip_pipe];
+		igt_require(igt_pipe_get_plane_type(flip_pipe_obj, DRM_PLANE_TYPE_OVERLAY));
 
 		igt_create_color_pattern_fb(display->drm_fd, prim_fb->width, prim_fb->height,
 					    DRM_FORMAT_ARGB8888, 0, .1, .1, .1, argb_fb);
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index d5027b25548d..7a10725cad95 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -460,8 +460,10 @@ static void
 run_tests_for_pipe_plane(data_t *data, enum pipe pipe)
 {
 	igt_fixture {
+		igt_pipe_t *pipe_obj = &data->display.pipes[pipe];
+
 		igt_skip_on(pipe >= data->display.n_pipes);
-		igt_require(data->display.pipes[pipe].n_planes > 0);
+		igt_require(igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_PRIMARY));
 	}
 
 	igt_subtest_f("pixel-format-pipe-%s-planes",
diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index d1e4b3ca536e..38188e4b6b86 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -146,9 +146,9 @@ test_setup(data_t *data, enum pipe pipe, uint64_t modifier,
 
 	mode = igt_output_get_mode(output);
 
-	data->fb = calloc(data->display.pipes[pipe].n_planes, sizeof(struct igt_fb));
+	data->fb = calloc(data->display.n_planes, sizeof(struct igt_fb));
 	igt_assert_f(data->fb, "Failed to allocate memory for %d FBs\n",
-	             data->display.pipes[pipe].n_planes);
+	             data->display.n_planes);
 
 	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
 			    DRM_FORMAT_XRGB8888,
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index 45e0fd31fce3..439c6c3fe2b1 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -67,7 +67,7 @@ struct {
  */
 static void test_init(data_t *data, enum pipe pipe)
 {
-	int n_planes = data->display.pipes[pipe].n_planes;
+	int n_planes = data->display.n_planes;
 
 	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
 
@@ -178,7 +178,6 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 {
 	drmModeModeInfo *mode;
 	igt_display_t *display = output->display;
-	igt_pipe_t *pipe = &display->pipes[pipe_id];
 	igt_plane_t *primary, *plane;
 	int *x;
 	int *y;
@@ -187,12 +186,12 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 	igt_output_set_pipe(output, pipe_id);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
-	x = malloc(pipe->n_planes * sizeof(*x));
-	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (pipe->n_planes * sizeof(*x)));
-	y = malloc(pipe->n_planes * sizeof(*y));
-	igt_assert_f(y, "Failed to allocate %ld bytes for variable y\n", (long int) (pipe->n_planes * sizeof(*y)));
-	size = malloc(pipe->n_planes * sizeof(*size));
-	igt_assert_f(size, "Failed to allocate %ld bytes for variable size\n", (long int) (pipe->n_planes * sizeof(*size)));
+	x = malloc(display->n_planes * sizeof(*x));
+	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (display->n_planes * sizeof(*x)));
+	y = malloc(display->n_planes * sizeof(*y));
+	igt_assert_f(y, "Failed to allocate %ld bytes for variable y\n", (long int) (display->n_planes * sizeof(*y)));
+	size = malloc(display->n_planes * sizeof(*size));
+	igt_assert_f(size, "Failed to allocate %ld bytes for variable size\n", (long int) (display->n_planes * sizeof(*size)));
 
 	mode = igt_output_get_mode(output);
 
@@ -320,7 +319,8 @@ run_tests_for_pipe(data_t *data, enum pipe pipe)
 			valid_tests++;
 
 		igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
-		igt_require(data->display.pipes[pipe].n_planes > 0);
+		igt_require(igt_pipe_get_plane_type(&data->display.pipes[pipe],
+						    DRM_PLANE_TYPE_PRIMARY));
 	}
 
 	igt_subtest_f("atomic-pipe-%s-tiling-x", kmstest_pipe_name(pipe))
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 0cd5c6e52b57..c84b0dd75a84 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -414,8 +414,7 @@ static void test_plane_rotation(data_t *data, int plane_type, bool test_bad_form
 
 static void test_plane_rotation_exhaust_fences(data_t *data,
 					       enum pipe pipe,
-					       igt_output_t *output,
-					       igt_plane_t *plane)
+					       igt_output_t *output)
 {
 	igt_display_t *display = &data->display;
 	uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
@@ -426,6 +425,11 @@ static void test_plane_rotation_exhaust_fences(data_t *data,
 	unsigned int stride, size, w, h;
 	uint64_t total_aperture_size, total_fbs_size;
 	int i;
+	igt_plane_t *plane;
+
+	igt_output_set_pipe(output, pipe);
+
+	plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 
 	igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
 
@@ -617,9 +621,7 @@ igt_main
 		igt_display_require_output(&data.display);
 
 		for_each_pipe_with_valid_output(&data.display, pipe, output) {
-			igt_plane_t *primary = &data.display.pipes[pipe].planes[0];
-
-			test_plane_rotation_exhaust_fences(&data, pipe, output, primary);
+			test_plane_rotation_exhaust_fences(&data, pipe, output);
 			break;
 		}
 	}
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 58f329e684c4..e07cc7e2ce6f 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -131,9 +131,8 @@ functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 {
 	functional_test_t test = { .data = data };
 	igt_display_t *display = &data->display;
-	igt_plane_t *primary, *sprite;
+	igt_plane_t *primary, *sprite, *plane;
 	int num_primary = 0, num_cursor = 0;
-	int i;
 
 	igt_skip_on(pipe >= display->n_pipes);
 
@@ -148,10 +147,10 @@ functional_test_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
 	 * drm_universal_plane_init(), the type enum can get interpreted as a
 	 * boolean and show up in userspace as the wrong type.
 	 */
-	for (i = 0; i < display->pipes[pipe].n_planes; i++)
-		if (display->pipes[pipe].planes[i].type == DRM_PLANE_TYPE_PRIMARY)
+	for_each_plane_on_pipe(display, pipe, plane)
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 			num_primary++;
-		else if (display->pipes[pipe].planes[i].type == DRM_PLANE_TYPE_CURSOR)
+		else if (plane->type == DRM_PLANE_TYPE_CURSOR)
 			num_cursor++;
 
 	igt_assert_eq(num_primary, 1);
-- 
2.17.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for lib/igt_kms: Move planes from pipe to display, v2.
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 4/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
@ 2018-05-07 14:15 ` Patchwork
  2018-05-07 15:40 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-05-07 14:15 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

== Series Details ==

Series: lib/igt_kms: Move planes from pipe to display, v2.
URL   : https://patchwork.freedesktop.org/series/42814/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4150 -> IGTPW_1325 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42814/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1325 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    
    ==== Possible fixes ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-skl-guc:         FAIL (fdo#103928) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       DMESG-FAIL (fdo#106103, fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-skl-6700k2:      FAIL (fdo#103191, fdo#104724) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    
  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (38 -> 37) ==

  Additional (2): fi-kbl-7560u fi-pnv-d510 
  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4463 -> IGTPW_1325

  CI_DRM_4150: 93d32416ba4b1dae9451fec28aaa71915d770f51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1325: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1325/
  IGT_4463: 91b5a3ef5516b29584ea4567b0f5ffa18219b29f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4463: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit



== Testlist changes ==

-igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip
-igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1325/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip Maarten Lankhorst
@ 2018-05-07 14:21   ` Daniel Vetter
  2018-05-08 14:21     ` Maarten Lankhorst
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2018-05-07 14:21 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev, Laurent Pinchart, Daniel Vetter

On Mon, May 07, 2018 at 03:14:28PM +0200, Maarten Lankhorst wrote:
> CS flips no longer exist, so the test has become useless.
> Other tests like kms_busy already perform some testing
> that's gpu agnostic.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  tests/Makefile.sources      |   1 -
>  tests/kms_mmio_vs_cs_flip.c | 523 ------------------------------------
>  tests/meson.build           |   1 -
>  3 files changed, 525 deletions(-)
>  delete mode 100644 tests/kms_mmio_vs_cs_flip.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 791e4f83d648..8066fb1a7a46 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -194,7 +194,6 @@ TESTS_progs = \
>  	kms_invalid_dotclock \
>  	kms_legacy_colorkey \
>  	kms_mmap_write_crc \
> -	kms_mmio_vs_cs_flip \
>  	kms_panel_fitting \
>  	kms_pipe_b_c_ivb \
>  	kms_pipe_crc_basic \
> diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
> deleted file mode 100644
> index fa947d9cd7f4..000000000000
> --- a/tests/kms_mmio_vs_cs_flip.c
> +++ /dev/null
> @@ -1,523 +0,0 @@
> -/*
> - * Copyright © 2014 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> - * IN THE SOFTWARE.
> - */
> -
> -#include "igt.h"
> -#include <errno.h>
> -#include <stdbool.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <time.h>
> -
> -
> -typedef struct {
> -	int drm_fd;
> -	igt_display_t display;
> -	igt_pipe_crc_t *pipe_crc;
> -	drm_intel_bufmgr *bufmgr;
> -	drm_intel_bo *busy_bo;
> -	uint32_t devid;
> -	bool flip_done;
> -} data_t;
> -
> -static void exec_nop(data_t *data, uint32_t handle, unsigned int ring)
> -{
> -	struct intel_batchbuffer *batch;
> -	drm_intel_bo *bo;
> -
> -	batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
> -	igt_assert(batch);
> -
> -	bo = gem_handle_to_libdrm_bo(data->bufmgr, data->drm_fd, "", handle);
> -	igt_assert(bo);
> -
> -	/* add relocs to make sure the kernel will think we write to dst */
> -	BEGIN_BATCH(4, 1);
> -	OUT_BATCH(MI_BATCH_BUFFER_END);
> -	OUT_BATCH(MI_NOOP);
> -	OUT_RELOC(bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> -	OUT_BATCH(MI_NOOP);
> -	ADVANCE_BATCH();
> -
> -	intel_batchbuffer_flush_on_ring(batch, ring);
> -	intel_batchbuffer_free(batch);
> -
> -	drm_intel_bo_unreference(bo);
> -}
> -
> -static void exec_blt(data_t *data)
> -{
> -	struct intel_batchbuffer *batch;
> -	int w, h, pitch, i;
> -
> -	batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
> -	igt_assert(batch);
> -
> -	w = 8192;
> -	h = data->busy_bo->size / (8192 * 4);
> -	pitch = w * 4;
> -
> -	for (i = 0; i < 40; i++) {
> -		BLIT_COPY_BATCH_START(0);
> -		OUT_BATCH((3 << 24) | /* 32 bits */
> -			  (0xcc << 16) | /* copy ROP */
> -			  pitch);
> -		OUT_BATCH(0 << 16 | 0);
> -		OUT_BATCH(h << 16 | w);
> -		OUT_RELOC_FENCED(data->busy_bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> -		OUT_BATCH(0 << 16 | 0);
> -		OUT_BATCH(pitch);
> -		OUT_RELOC_FENCED(data->busy_bo, I915_GEM_DOMAIN_RENDER, 0, 0);
> -		ADVANCE_BATCH();
> -	}
> -
> -	intel_batchbuffer_flush(batch);
> -	intel_batchbuffer_free(batch);
> -}
> -
> -static void page_flip_handler(int fd, unsigned int frame, unsigned int sec,
> -			      unsigned int usec, void *_data)
> -{
> -	data_t *data = _data;
> -
> -	data->flip_done = true;
> -}
> -
> -static void wait_for_flip(data_t *data, uint32_t flip_handle)
> -{
> -	struct timeval timeout = {
> -		.tv_sec = 3,
> -		.tv_usec = 0,
> -	};
> -	drmEventContext evctx = {
> -		.version = 2,
> -		.page_flip_handler = page_flip_handler,
> -	};
> -	fd_set fds;
> -
> -	FD_ZERO(&fds);
> -	FD_SET(data->drm_fd, &fds);
> -
> -	while (!data->flip_done) {
> -		int ret = select(data->drm_fd + 1, &fds, NULL, NULL, &timeout);
> -
> -		if (ret < 0 && errno == EINTR)
> -			continue;
> -
> -		igt_assert_lte(0, ret);
> -
> -		do_or_die(drmHandleEvent(data->drm_fd, &evctx));
> -	}
> -
> -	/*
> -	 * The flip completion may have been signalled prematurely, so
> -	 * also submit another nop batch and wait for it to make sure
> -	 * the ring has really been drained.
> -	 */
> -	if (IS_GEN7(data->devid) || IS_GEN8(data->devid))
> -		exec_nop(data, flip_handle, I915_EXEC_BLT);
> -	else
> -		exec_nop(data, flip_handle, I915_EXEC_RENDER);
> -	gem_sync(data->drm_fd, flip_handle);
> -}
> -
> -static void make_gpu_busy(data_t *data, uint32_t flip_handle)
> -{
> -	/*
> -	 * Make sure flip_handle has been used on the blt ring.
> -	 * This should make the flip use the same ring on gen7+.
> -	 */
> -	if (IS_GEN7(data->devid) || IS_GEN8(data->devid))
> -		exec_nop(data, flip_handle, I915_EXEC_BLT);
> -
> -	/*
> -	 * Add a pile commands to the ring.  The flip will be
> -	 * stuck behing these commands and hence gets delayed
> -	 * significantly.
> -	 */
> -	exec_blt(data);
> -
> -	/*
> -	 * Make sure the render ring will block until the blt ring is clear.
> -	 * This is in case the flip will execute on the render ring and the
> -	 * blits were on the blt ring (this will be the case on gen6 at least).
> -	 *
> -	 * We can't add an explicit dependency between flip_handle and the
> -	 * blits since that would cause the driver to block until the blits
> -	 * have completed before it will perform a subsequent mmio flip,
> -	 * and so the test would fail to exercise the mmio vs. CS flip race.
> -	 */
> -	if (HAS_BLT_RING(data->devid))
> -		exec_nop(data, data->busy_bo->handle, I915_EXEC_RENDER);
> -}
> -
> -/*
> - * 1. set primary plane to full red
> - * 2. grab a reference crc
> - * 3. set primary plane to full blue
> - * 4. queue lots of GPU activity to delay the subsequent page flip
> - * 5. queue a page flip to the same blue fb
> - * 6. toggle a fullscreen sprite (green) on and back off again
> - * 7. set primary plane to red fb
> - * 8. wait for GPU to finish
> - * 9. compare current crc with reference crc
> - *
> - * We expect the primary plane to display full red at the end.
> - * If the sprite operations have interfered with the page flip,
> - * the driver may have mistakenly completed the flip before
> - * it was executed by the CS, and hence the subsequent mmio
> - * flips may have overtaken it. So once we've finished everything
> - * the CS flip may have been the last thing to occur, which means
> - * the primary plane may be full blue instead of the red it's
> - * supposed to be.
> - */
> -static void
> -test_plane(data_t *data, igt_output_t *output, enum pipe pipe, int plane)
> -{
> -	struct igt_fb red_fb, green_fb, blue_fb;
> -	drmModeModeInfo *mode;
> -	igt_plane_t *primary, *sprite;
> -	igt_crc_t ref_crc, crc;
> -	int ret;
> -
> -	igt_output_set_pipe(output, pipe);
> -
> -	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	sprite = igt_output_get_plane(output, plane);
> -
> -	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,
> -			    1.0, 0.0, 0.0,
> -			    &red_fb);
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -			    DRM_FORMAT_XRGB8888,
> -			    LOCAL_DRM_FORMAT_MOD_NONE,
> -			    0.0, 1.0, 0.0,
> -			    &green_fb);
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> -			    DRM_FORMAT_XRGB8888,
> -			    LOCAL_DRM_FORMAT_MOD_NONE,
> -			    0.0, 0.0, 1.0,
> -			    &blue_fb);
> -
> -	/*
> -	 * Make sure these buffers are suited for display use
> -	 * because most of the modeset operations must be fast
> -	 * later on.
> -	 */
> -	igt_plane_set_fb(primary, &blue_fb);
> -	igt_display_commit(&data->display);
> -	igt_plane_set_fb(sprite, &green_fb);
> -	igt_display_commit(&data->display);
> -	igt_plane_set_fb(sprite, NULL);
> -	igt_display_commit(&data->display);
> -
> -	if (data->pipe_crc)
> -		igt_pipe_crc_free(data->pipe_crc);
> -	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> -
> -	/* set red fb and grab reference crc */
> -	igt_plane_set_fb(primary, &red_fb);
> -	igt_display_commit(&data->display);
> -	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
> -
> -	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
> -			     blue_fb.fb_id, 0, 0, &output->id, 1,
> -			     mode);
> -	igt_assert_eq(ret, 0);
> -
> -	make_gpu_busy(data, blue_fb.gem_handle);
> -
> -	data->flip_done = false;
> -	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
> -			      blue_fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, data);
> -	igt_assert_eq(ret, 0);
> -
> -	/*
> -	 * Toggle a fullscreen sprite on and back off. This will result
> -	 * in the primary plane getting disabled and re-enbled, and that
> -	 * leads to mmio flips. The driver may then mistake the flip done
> -	 * interrupts from the mmio flips as the flip done interrupts for
> -	 * the CS flip, and hence subsequent mmio flips won't wait for the
> -	 * CS flips like they should.
> -	 */
> -	ret = drmModeSetPlane(data->drm_fd,
> -			      sprite->drm_plane->plane_id,
> -			      output->config.crtc->crtc_id,
> -			      green_fb.fb_id, 0,
> -			      0, 0, mode->hdisplay, mode->vdisplay,
> -			      0, 0, mode->hdisplay << 16, mode->vdisplay << 16);
> -	igt_assert_eq(ret, 0);
> -	ret = drmModeSetPlane(data->drm_fd,
> -			      sprite->drm_plane->plane_id,
> -			      output->config.crtc->crtc_id,
> -			      0, 0,
> -			      0, 0, 0, 0,
> -			      0, 0, 0, 0);
> -	igt_assert_eq(ret, 0);
> -
> -	/*
> -	 * Set primary plane to red fb. This should wait for the CS flip
> -	 * to complete. But if the kernel mistook the flip done interrupt
> -	 * from the mmio flip as the flip done from the CS flip, this will
> -	 * not wait for anything. And hence the the CS flip will actually
> -	 * occur after this mmio flip.
> -	 */
> -	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
> -			     red_fb.fb_id, 0, 0, &output->id, 1,
> -			     mode);
> -	igt_assert_eq(ret, 0);
> -
> -	/* Make sure the flip has been executed */
> -	wait_for_flip(data, blue_fb.gem_handle);
> -
> -	/* Grab crc and compare with the extected result */
> -	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> -
> -	igt_plane_set_fb(primary, NULL);
> -	igt_display_commit(&data->display);
> -
> -	igt_remove_fb(data->drm_fd, &red_fb);
> -	igt_remove_fb(data->drm_fd, &green_fb);
> -	igt_remove_fb(data->drm_fd, &blue_fb);
> -
> -	igt_pipe_crc_free(data->pipe_crc);
> -	data->pipe_crc = NULL;
> -
> -	igt_output_set_pipe(output, PIPE_ANY);
> -	igt_display_commit(&data->display);
> -
> -	igt_assert_crc_equal(&ref_crc, &crc);
> -}
> -
> -/*
> - * 1. set primary plane to full red
> - * 2. grab a reference crc
> - * 3. set primary plane to full green
> - * 4. wait for vblank
> - * 5. pan primary plane a bit (to cause a mmio flip w/o vblank wait)
> - * 6. queue lots of GPU activity to delay the subsequent page flip
> - * 6. queue a page flip to a blue fb
> - * 7. set primary plane to red fb
> - * 8. wait for GPU to finish
> - * 9. compare current crc with reference crc
> - *
> - * We expect the primary plane to display full red at the end.
> - * If the previously schedule primary plane pan operation has interfered
> - * with the following page flip, the driver may have mistakenly completed
> - * the flip before it was executed by the CS, and hence the subsequent mmio
> - * flips may have overtaken it. So once we've finished everything
> - * the CS flip may have been the last thing to occur, which means
> - * the primary plane may be full blue instead of the red it's
> - * supposed to be.
> - */
> -static void
> -test_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
> -{
> -	struct igt_fb red_fb, green_fb, blue_fb;
> -	drmModeModeInfo *mode;
> -	igt_plane_t *primary;
> -	igt_crc_t ref_crc, crc;
> -	int ret;
> -
> -	igt_output_set_pipe(output, pipe);
> -
> -	primary = igt_output_get_plane(output, 0);
> -
> -	mode = igt_output_get_mode(output);
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
> -			    DRM_FORMAT_XRGB8888,
> -			    LOCAL_DRM_FORMAT_MOD_NONE,
> -			    1.0, 0.0, 0.0,
> -			    &red_fb);
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
> -			    DRM_FORMAT_XRGB8888,
> -			    LOCAL_DRM_FORMAT_MOD_NONE,
> -			    0.0, 0.0, 1.0,
> -			    &blue_fb);
> -	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay+1,
> -			    DRM_FORMAT_XRGB8888,
> -			    LOCAL_DRM_FORMAT_MOD_NONE,
> -			    0.0, 1.0, 0.0,
> -			    &green_fb);
> -
> -	/*
> -	 * Make sure these buffers are suited for display use
> -	 * because most of the modeset operations must be fast
> -	 * later on.
> -	 */
> -	igt_plane_set_fb(primary, &green_fb);
> -	igt_display_commit(&data->display);
> -	igt_plane_set_fb(primary, &blue_fb);
> -	igt_display_commit(&data->display);
> -
> -	if (data->pipe_crc)
> -		igt_pipe_crc_free(data->pipe_crc);
> -	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> -
> -	/* set red fb and grab reference crc */
> -	igt_plane_set_fb(primary, &red_fb);
> -	igt_display_commit(&data->display);
> -	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
> -
> -	/*
> -	 * Further down we need to issue an mmio flip w/o the kernel
> -	 * waiting for vblank. The easiest way is to just pan within
> -	 * the same FB. So pan away a bit here, and later we undo this
> -	 * with another pan which will result in the desired mmio flip.
> -	 */
> -	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
> -			     green_fb.fb_id, 0, 1, &output->id, 1,
> -			     mode);
> -	igt_assert_eq(ret, 0);
> -
> -	/*
> -	 * Make it more likely that the CS flip has been submitted into the
> -	 * ring by the time the mmio flip from the drmModeSetCrtc() below
> -	 * completes. The driver will then mistake the flip done interrupt
> -	 * from the mmio flip as the flip done interrupt from the CS flip.
> -	 */
> -	igt_wait_for_vblank(data->drm_fd, pipe);
> -
> -	/* now issue the mmio flip w/o vblank waits in the kernel, ie. pan a bit */
> -	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
> -			     green_fb.fb_id, 0, 0, &output->id, 1,
> -			     mode);
> -	igt_assert_eq(ret, 0);
> -
> -	make_gpu_busy(data, blue_fb.gem_handle);
> -
> -	/*
> -	 * Submit the CS flip. The commands must be emitted into the ring
> -	 * before the mmio flip from the panning operation completes.
> -	 */
> -	data->flip_done = false;
> -	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
> -			      blue_fb.fb_id, DRM_MODE_PAGE_FLIP_EVENT, data);
> -	igt_assert_eq(ret, 0);
> -
> -	/*
> -	 * Set primary plane to red fb. This should wait for the CS flip
> -	 * to complete. But if the kernel mistook the flip done interrupt
> -	 * from the mmio flip as the flip done from the CS flip, this will
> -	 * not wait for anything. And hence the the CS flip will actually
> -	 * occur after this mmio flip.
> -	 */
> -	ret = drmModeSetCrtc(data->drm_fd, output->config.crtc->crtc_id,
> -			     red_fb.fb_id, 0, 0, &output->id, 1,
> -			     mode);
> -	igt_assert_eq(ret, 0);
> -
> -	/* Make sure the flip has been executed */
> -	wait_for_flip(data, blue_fb.gem_handle);
> -
> -	/* Grab crc and compare with the extected result */
> -	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> -
> -	igt_plane_set_fb(primary, NULL);
> -	igt_display_commit(&data->display);
> -
> -	igt_remove_fb(data->drm_fd, &red_fb);
> -	igt_remove_fb(data->drm_fd, &green_fb);
> -	igt_remove_fb(data->drm_fd, &blue_fb);
> -
> -	igt_pipe_crc_free(data->pipe_crc);
> -	data->pipe_crc = NULL;
> -
> -	igt_output_set_pipe(output, PIPE_ANY);
> -	igt_display_commit(&data->display);
> -
> -	igt_assert_crc_equal(&ref_crc, &crc);
> -}
> -
> -static void
> -run_plane_test(data_t *data)
> -{
> -	igt_output_t *output;
> -	int plane = 1; /* testing with one sprite is enough */
> -	int valid_tests = 0;
> -	enum pipe pipe;
> -
> -	for_each_pipe_with_valid_output(&data->display, pipe, output) {
> -		igt_require(data->display.pipes[pipe].n_planes > 2);
> -
> -		test_plane(data, output, pipe, plane);
> -		valid_tests++;
> -	}
> -
> -	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> -}
> -
> -static void
> -run_crtc_test(data_t *data)
> -{
> -	igt_output_t *output;
> -	int valid_tests = 0;
> -	enum pipe pipe;
> -
> -	for_each_pipe_with_valid_output(&data->display, pipe, output) {
> -		test_crtc(data, output, pipe);
> -		valid_tests++;
> -	}
> -
> -	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
> -}
> -
> -static data_t data;
> -
> -igt_main
> -{
> -	igt_skip_on_simulation();
> -
> -	igt_fixture {
> -		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> -
> -		kmstest_set_vt_graphics_mode();
> -
> -		data.devid = intel_get_drm_devid(data.drm_fd);
> -
> -		igt_require_pipe_crc(data.drm_fd);
> -		igt_display_init(&data.display, data.drm_fd);
> -
> -		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> -		igt_assert(data.bufmgr);
> -		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> -
> -		data.busy_bo = drm_intel_bo_alloc(data.bufmgr, "bo",
> -						  64*1024*1024, 4096);
> -		gem_set_tiling(data.drm_fd, data.busy_bo->handle, 0, 4096);
> -	}
> -
> -	igt_subtest_f("setplane_vs_cs_flip")
> -		run_plane_test(&data);
> -
> -	igt_subtest_f("setcrtc_vs_cs_flip")
> -		run_crtc_test(&data);
> -
> -	igt_fixture {
> -		drm_intel_bo_unreference(data.busy_bo);
> -		drm_intel_bufmgr_destroy(data.bufmgr);
> -		igt_display_fini(&data.display);
> -	}
> -}
> diff --git a/tests/meson.build b/tests/meson.build
> index 015afa4742c8..1f00fc4fd983 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -170,7 +170,6 @@ test_progs = [
>  	'kms_invalid_dotclock',
>  	'kms_legacy_colorkey',
>  	'kms_mmap_write_crc',
> -	'kms_mmio_vs_cs_flip',
>  	'kms_panel_fitting',
>  	'kms_pipe_b_c_ivb',
>  	'kms_pipe_crc_basic',
> -- 
> 2.17.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2.
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2 Maarten Lankhorst
@ 2018-05-07 14:51   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-05-07 14:51 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev, Laurent Pinchart, Daniel Vetter

On Mon, May 07, 2018 at 03:14:27PM +0200, Maarten Lankhorst wrote:
> We want to get rid of the pipe <-> plane mapping. The first thing we
> have to do is decoupling the plane array from the pipe, so do that here.
> 
> Changes since v1:
> - Disable limited-range tests again, like they were before this commit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  lib/igt_kms.c                     |  52 +++++-----
>  lib/igt_kms.h                     |   5 +-
>  tests/kms_atomic.c                |  10 +-
>  tests/kms_atomic_interruptible.c  |  19 ++--
>  tests/kms_chamelium.c             |  35 ++++---
>  tests/kms_color.c                 | 155 +++++++++++++++---------------
>  tests/kms_concurrent.c            |   2 +-
>  tests/kms_crtc_background_color.c |  23 ++---
>  tests/kms_plane_multiple.c        |   4 +-
>  9 files changed, 154 insertions(+), 151 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 35c77da7b996..7281c71607a3 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1678,8 +1678,7 @@ static void igt_output_refresh(igt_output_t *output)
>  static int
>  igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	return drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
>  				 DRM_MODE_OBJECT_PLANE, prop_id, value);
> @@ -1914,7 +1913,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>  
>  			igt_assert_f(plane->index < n_planes, "n_planes < plane->index failed\n");
>  			plane->type = type;
> -			plane->pipe = pipe;
> +			plane->display = display;
> +			plane->bound_pipe = i;
>  			plane->drm_plane = drm_plane;
>  			plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
>  
> @@ -2548,7 +2548,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  					   igt_pipe_t *pipe,
>  					   bool fail_on_error)
>  {
> -	struct igt_display *display = primary->pipe->display;
> +	struct igt_display *display = primary->display;
>  	igt_output_t *output = igt_pipe_get_output(pipe);
>  	drmModeModeInfo *mode;
>  	uint32_t fb_id, crtc_id;
> @@ -2558,12 +2558,12 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
>  	igt_assert((primary->values[IGT_PLANE_CRTC_X] == 0 && primary->values[IGT_PLANE_CRTC_Y] == 0));
>  
>  	/* nor rotated */
> -	if (!pipe->display->first_commit)
> +	if (!display->first_commit)
>  		igt_assert(!igt_plane_is_prop_changed(primary, IGT_PLANE_ROTATION));
>  
>  	if (!igt_plane_is_prop_changed(primary, IGT_PLANE_FB_ID) &&
>  	    !(primary->changed & IGT_PLANE_COORD_CHANGED_MASK) &&
> -	    !igt_pipe_obj_is_prop_changed(primary->pipe, IGT_CRTC_MODE_ID))
> +	    !igt_pipe_obj_is_prop_changed(pipe, IGT_CRTC_MODE_ID))
>  		return 0;
>  
>  	crtc_id = pipe->crtc_id;
> @@ -2803,7 +2803,7 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties
>  {
>  	igt_assert(igt_plane_has_prop(plane, prop));
>  
> -	return igt_mode_object_get_prop(plane->pipe->display, DRM_MODE_OBJECT_PLANE,
> +	return igt_mode_object_get_prop(plane->display, DRM_MODE_OBJECT_PLANE,
>  					plane->drm_plane->plane_id, plane->props[prop]);
>  }
>  
> @@ -2823,7 +2823,7 @@ uint64_t igt_plane_get_prop(igt_plane_t *plane, enum igt_atomic_plane_properties
>  void
>  igt_plane_replace_prop_blob(igt_plane_t *plane, enum igt_atomic_plane_properties prop, const void *ptr, size_t length)
>  {
> -	igt_display_t *display = plane->pipe->display;
> +	igt_display_t *display = plane->display;
>  	uint64_t *blob = &plane->values[prop];
>  	uint32_t blob_id = 0;
>  
> @@ -3524,13 +3524,16 @@ igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type)
>   */
>  void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
> -	LOG(display, "%s.%d: plane_set_fb(%d)\n", kmstest_pipe_name(pipe->pipe),
> +	LOG(display, "%s.%d: plane_set_fb(%d)\n", kmstest_pipe_name(plane->bound_pipe),
>  	    plane->index, fb ? fb->fb_id : 0);
>  
> -	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_ID, fb ? pipe->crtc_id : 0);
> +	if (plane->bound_pipe != PIPE_NONE) {
> +		igt_pipe_t *pipe = &display->pipes[plane->bound_pipe];
> +
> +		igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_ID, fb ? pipe->crtc_id : 0);
> +	}
>  	igt_plane_set_prop_value(plane, IGT_PLANE_FB_ID, fb ? fb->fb_id : 0);
>  
>  	if (plane->type == DRM_PLANE_TYPE_CURSOR && fb)
> @@ -3591,11 +3594,10 @@ void igt_plane_set_fence_fd(igt_plane_t *plane, int fence_fd)
>   */
>  void igt_plane_set_position(igt_plane_t *plane, int x, int y)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	LOG(display, "%s.%d: plane_set_position(%d,%d)\n",
> -	    kmstest_pipe_name(pipe->pipe), plane->index, x, y);
> +	    kmstest_pipe_name(plane->bound_pipe), plane->index, x, y);
>  
>  	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_X, x);
>  	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_Y, y);
> @@ -3613,11 +3615,10 @@ void igt_plane_set_position(igt_plane_t *plane, int x, int y)
>   */
>  void igt_plane_set_size(igt_plane_t *plane, int w, int h)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	LOG(display, "%s.%d: plane_set_size (%dx%d)\n",
> -	    kmstest_pipe_name(pipe->pipe), plane->index, w, h);
> +	    kmstest_pipe_name(plane->bound_pipe), plane->index, w, h);
>  
>  	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_W, w);
>  	igt_plane_set_prop_value(plane, IGT_PLANE_CRTC_H, h);
> @@ -3636,11 +3637,10 @@ void igt_plane_set_size(igt_plane_t *plane, int w, int h)
>  void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>  	uint32_t x, uint32_t y)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	LOG(display, "%s.%d: fb_set_position(%d,%d)\n",
> -	    kmstest_pipe_name(pipe->pipe), plane->index, x, y);
> +	    kmstest_pipe_name(plane->bound_pipe), plane->index, x, y);
>  
>  	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_X, IGT_FIXED(x, 0));
>  	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_Y, IGT_FIXED(y, 0));
> @@ -3660,11 +3660,10 @@ void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>  void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
>  	uint32_t w, uint32_t h)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	LOG(display, "%s.%d: fb_set_size(%dx%d)\n",
> -	    kmstest_pipe_name(pipe->pipe), plane->index, w, h);
> +	    kmstest_pipe_name(plane->bound_pipe), plane->index, w, h);
>  
>  	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_W, IGT_FIXED(w, 0));
>  	igt_plane_set_prop_value(plane, IGT_PLANE_SRC_H, IGT_FIXED(h, 0));
> @@ -3697,11 +3696,10 @@ static const char *rotation_name(igt_rotation_t rotation)
>   */
>  void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
>  {
> -	igt_pipe_t *pipe = plane->pipe;
> -	igt_display_t *display = pipe->display;
> +	igt_display_t *display = plane->display;
>  
>  	LOG(display, "%s.%d: plane_set_rotation(%s)\n",
> -	    kmstest_pipe_name(pipe->pipe),
> +	    kmstest_pipe_name(plane->bound_pipe),
>  	    plane->index, rotation_name(rotation));
>  
>  	igt_plane_set_prop_value(plane, IGT_PLANE_ROTATION, rotation);
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0e75d0c9e6b9..5e7db0a263fc 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -293,7 +293,7 @@ typedef enum {
>  
>  typedef struct {
>  	/*< private >*/
> -	igt_pipe_t *pipe;
> +	igt_display_t *display;
>  	int index;
>  	/* capabilities */
>  	int type;
> @@ -307,6 +307,9 @@ typedef struct {
>  	/* gem handle for fb */
>  	uint32_t gem_handle;
>  
> +	/* Default pipe, override with igt_plane_bind() */
> +	enum pipe bound_pipe;

igt docs for all the kms_ stuff would be really neat ...

More to the point, igt_plane_bind() doesn't yet exist here.

Also if the goal is to make this stuff dynamic, I'm voting for
->pending_pipe, to be consistent with igt_output_t.

> +
>  	uint64_t changed;
>  	uint32_t props[IGT_NUM_PLANE_PROPS];
>  	uint64_t values[IGT_NUM_PLANE_PROPS];
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index ac02baf0170b..71a34c51f8ef 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -98,7 +98,7 @@ static void plane_check_current_state(igt_plane_t *plane, const uint64_t *values
>  	uint64_t current_values[IGT_NUM_PLANE_PROPS];
>  	int i;
>  
> -	legacy = drmModeGetPlane(plane->pipe->display->drm_fd, plane->drm_plane->plane_id);
> +	legacy = drmModeGetPlane(plane->display->drm_fd, plane->drm_plane->plane_id);
>  	igt_assert(legacy);
>  
>  	igt_assert_eq_u32(legacy->crtc_id, values[IGT_PLANE_CRTC_ID]);
> @@ -123,7 +123,7 @@ static void plane_check_current_state(igt_plane_t *plane, const uint64_t *values
>  static void plane_commit(igt_plane_t *plane, enum igt_commit_style s,
>                                  enum kms_atomic_check_relax relax)
>  {
> -	igt_display_commit2(plane->pipe->display, s);
> +	igt_display_commit2(plane->display, s);
>  	plane_check_current_state(plane, plane->values, relax);
>  }
>  
> @@ -135,7 +135,7 @@ static void plane_commit_atomic_err(igt_plane_t *plane,
>  
>  	plane_get_current_state(plane, current_values);
>  
> -	igt_assert_eq(-err, igt_display_try_commit2(plane->pipe->display, COMMIT_ATOMIC));
> +	igt_assert_eq(-err, igt_display_try_commit2(plane->display, COMMIT_ATOMIC));
>  
>  	plane_check_current_state(plane, current_values, relax);
>  }
> @@ -840,7 +840,7 @@ static void atomic_setup(igt_display_t *display, enum pipe pipe, igt_output_t *o
>  	igt_output_set_pipe(output, pipe);
>  	igt_plane_set_fb(primary, fb);
>  
> -	crtc_commit(primary->pipe, primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
> +	crtc_commit(&display->pipes[pipe], primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>  }
>  
>  static void atomic_clear(igt_display_t *display, enum pipe pipe, igt_plane_t *primary, igt_output_t *output)
> @@ -853,7 +853,7 @@ static void atomic_clear(igt_display_t *display, enum pipe pipe, igt_plane_t *pr
>  	}
>  
>  	igt_output_set_pipe(output, PIPE_NONE);
> -	crtc_commit(primary->pipe, primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
> +	crtc_commit(&display->pipes[pipe], primary, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
>  }
>  
>  igt_main
> diff --git a/tests/kms_atomic_interruptible.c b/tests/kms_atomic_interruptible.c
> index 64a005597a21..a4d7d3abbd5a 100644
> --- a/tests/kms_atomic_interruptible.c
> +++ b/tests/kms_atomic_interruptible.c
> @@ -81,6 +81,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  	drmModeModeInfo *mode;
>  	igt_fb_t fb, fb2;
>  	igt_plane_t *primary, *plane;
> +	igt_pipe_t *pipe_obj = &display->pipes[pipe];
>  	int block;
>  
>  	/*
> @@ -140,7 +141,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  				struct drm_mode_crtc crtc = {
>  					.set_connectors_ptr = (uint64_t)(uintptr_t)&output->id,
>  					.count_connectors = 1,
> -					.crtc_id = primary->pipe->crtc_id,
> +					.crtc_id = pipe_obj->crtc_id,
>  					.fb_id = fb2.fb_id,
>  					.mode_valid = 1,
>  					.mode = *(struct drm_mode_modeinfo*)mode,
> @@ -151,15 +152,15 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  			}
>  			case test_atomic_modeset: {
>  				uint32_t objs[3] = {
> -					plane->pipe->crtc_id,
> +					pipe_obj->crtc_id,
>  					output->id,
>  					plane->drm_plane->plane_id
>  				};
>  				uint32_t count_props[3] = { 2, 1, 6 };
>  				uint32_t props[] = {
>  					/* crtc: 2 props */
> -					plane->pipe->props[IGT_CRTC_MODE_ID],
> -					plane->pipe->props[IGT_CRTC_ACTIVE],
> +					pipe_obj->props[IGT_CRTC_MODE_ID],
> +					pipe_obj->props[IGT_CRTC_ACTIVE],
>  					/* connector: 1 prop */
>  					output->props[IGT_CONNECTOR_CRTC_ID],
>  					/* plane: remainder props */
> @@ -175,9 +176,9 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  					0, /* mode_id, filled in below */
>  					true,
>  					/* connector */
> -					plane->pipe->crtc_id,
> +					pipe_obj->crtc_id,
>  					/* plane */
> -					plane->pipe->crtc_id,
> +					pipe_obj->crtc_id,
>  					fb2.fb_id,
>  					IGT_FIXED(fb2.width, 0),
>  					IGT_FIXED(fb2.height, 0),
> @@ -216,7 +217,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  			case test_setcursor: {
>  				struct drm_mode_cursor cur = {
>  					.flags = DRM_MODE_CURSOR_BO,
> -					.crtc_id = plane->pipe->crtc_id,
> +					.crtc_id = pipe_obj->crtc_id,
>  					.width = fb2.width,
>  					.height = fb2.height,
>  					.handle = fb2.gem_handle,
> @@ -227,7 +228,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  			case test_setplane: {
>  				struct drm_mode_set_plane setplane = {
>  					.plane_id = plane->drm_plane->plane_id,
> -					.crtc_id = plane->pipe->crtc_id,
> +					.crtc_id = pipe_obj->crtc_id,
>  					.fb_id = fb2.fb_id,
>  					.crtc_w = fb2.width,
>  					.crtc_h = fb2.height,
> @@ -240,7 +241,7 @@ static void run_plane_test(igt_display_t *display, enum pipe pipe, igt_output_t
>  			}
>  			case test_pageflip: {
>  				struct drm_mode_crtc_page_flip pageflip = {
> -					.crtc_id = plane->pipe->crtc_id,
> +					.crtc_id = pipe_obj->crtc_id,
>  					.fb_id = fb2.fb_id,
>  					.flags = DRM_MODE_PAGE_FLIP_EVENT,
>  				};
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 2bc34d07788d..6e9e54ed7f5b 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -410,14 +410,15 @@ test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
>  
>  static igt_output_t *
>  prepare_output(data_t *data,
> -	       struct chamelium_port *port)
> +	       struct chamelium_port *port,
> +	       enum pipe *ret_pipe)
>  {
>  	igt_display_t *display = &data->display;
>  	igt_output_t *output;
>  	drmModeRes *res;
>  	drmModeConnector *connector =
>  		chamelium_port_get_connector(data->chamelium, port, false);
> -	enum pipe pipe;
> +	enum pipe pipe = PIPE_NONE;
>  	bool found = false;
>  
>  	igt_assert(res = drmModeGetResources(data->drm_fd));
> @@ -449,6 +450,7 @@ prepare_output(data_t *data,
>  	drmModeFreeConnector(connector);
>  	drmModeFreeResources(res);
>  
> +	*ret_pipe = pipe;
>  	return output;
>  }
>  
> @@ -456,10 +458,12 @@ static void
>  enable_output(data_t *data,
>  	      struct chamelium_port *port,
>  	      igt_output_t *output,
> +	      enum pipe pipe,
>  	      drmModeModeInfo *mode,
>  	      struct igt_fb *fb)
>  {
>  	igt_display_t *display = output->display;
> +	igt_pipe_t *pipe_obj = &display->pipes[pipe];
>  	igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	drmModeConnector *connector = chamelium_port_get_connector(
>  	    data->chamelium, port, false);
> @@ -471,12 +475,12 @@ enable_output(data_t *data,
>  	igt_output_override_mode(output, mode);
>  
>  	/* Clear any color correction values that might be enabled */
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_DEGAMMA_LUT))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_GAMMA_LUT))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_GAMMA_LUT, NULL, 0);
> -	if (igt_pipe_obj_has_prop(primary->pipe, IGT_CRTC_CTM))
> -		igt_pipe_obj_replace_prop_blob(primary->pipe, IGT_CRTC_CTM, NULL, 0);
> +	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_DEGAMMA_LUT))
> +		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_DEGAMMA_LUT, NULL, 0);
> +	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT))
> +		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_GAMMA_LUT, NULL, 0);
> +	if (igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_CTM))
> +		igt_pipe_obj_replace_prop_blob(pipe_obj, IGT_CRTC_CTM, NULL, 0);
>  
>  	igt_display_commit2(display, COMMIT_ATOMIC);
>  
> @@ -500,10 +504,11 @@ test_display_crc(data_t *data, struct chamelium_port *port, int count,
>  	drmModeConnector *connector;
>  	int fb_id, i, j, captured_frame_count;
>  	int count_modes;
> +	enum pipe pipe;

A bit a bikeshed, but I think we should pass around more igt_pipe_t *
pointers and less enum pipe. Will make it easier when we're going to do
the s/igt_pipe_t/igt_crtc_t/ bikeshed.

But oh well.
>  
>  	reset_state(data, port);
>  
> -	output = prepare_output(data, port);
> +	output = prepare_output(data, port, &pipe);
>  	connector = chamelium_port_get_connector(data->chamelium, port, false);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	igt_assert(primary);
> @@ -523,7 +528,7 @@ test_display_crc(data_t *data, struct chamelium_port *port, int count,
>  		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
>  								&fb);
>  
> -		enable_output(data, port, output, mode, &fb);
> +		enable_output(data, port, output, pipe, mode, &fb);
>  
>  		/* We want to keep the display running for a little bit, since
>  		 * there's always the potential the driver isn't able to keep
> @@ -563,10 +568,11 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>  	drmModeModeInfo *mode;
>  	drmModeConnector *connector;
>  	int fb_id, i, j;
> +	enum pipe pipe;
>  
>  	reset_state(data, port);
>  
> -	output = prepare_output(data, port);
> +	output = prepare_output(data, port, &pipe);
>  	connector = chamelium_port_get_connector(data->chamelium, port, false);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	igt_assert(primary);
> @@ -580,7 +586,7 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>  						    0, 0, 0, &fb);
>  		igt_assert(fb_id > 0);
>  
> -		enable_output(data, port, output, mode, &fb);
> +		enable_output(data, port, output, pipe, mode, &fb);
>  
>  		igt_debug("Reading frame dumps from Chamelium...\n");
>  		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 5);
> @@ -608,10 +614,11 @@ test_analog_frame_dump(data_t *data, struct chamelium_port *port)
>  	drmModeConnector *connector;
>  	int fb_id, i;
>  	bool bridge;
> +	enum pipe pipe;
>  
>  	reset_state(data, port);
>  
> -	output = prepare_output(data, port);
> +	output = prepare_output(data, port, &pipe);
>  	connector = chamelium_port_get_connector(data->chamelium, port, false);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	igt_assert(primary);
> @@ -631,7 +638,7 @@ test_analog_frame_dump(data_t *data, struct chamelium_port *port)
>  						    0, 0, 0, &fb);
>  		igt_assert(fb_id > 0);
>  
> -		enable_output(data, port, output, mode, &fb);
> +		enable_output(data, port, output, pipe, mode, &fb);
>  
>  		igt_debug("Reading frame dumps from Chamelium...\n");
>  
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index bb106dd00565..ce364713f665 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -258,6 +258,7 @@ static void set_ctm(igt_pipe_t *pipe, const double *coefficients)
>   * rectangles with linear degamma LUT.
>   */
>  static void test_pipe_degamma(data_t *data,
> +			      igt_pipe_t *pipe,
>  			      igt_plane_t *primary)
>  {
>  	igt_output_t *output;
> @@ -274,13 +275,13 @@ static void test_pipe_degamma(data_t *data,
>  
>  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
>  		drmModeModeInfo *mode;
>  		struct igt_fb fb_modeset, fb;
>  		igt_crc_t crc_fullgamma, crc_fullcolors;
>  		int fb_id, fb_modeset_id;
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +		igt_output_set_pipe(output, pipe->pipe);
>  		mode = igt_output_get_mode(output);
>  
>  		/* Create a framebuffer at the size of the output. */
> @@ -301,16 +302,16 @@ static void test_pipe_degamma(data_t *data,
>  		igt_assert(fb_modeset_id);
>  
>  		igt_plane_set_fb(primary, &fb_modeset);
> -		disable_ctm(primary->pipe);
> -		disable_degamma(primary->pipe);
> -		set_gamma(data, primary->pipe, gamma_linear);
> +		disable_ctm(pipe);
> +		disable_degamma(pipe);
> +		set_gamma(data, pipe, gamma_linear);
>  		igt_display_commit(&data->display);
>  
>  		/* Draw solid colors with no degamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
>  		/* Draw a gradient with degamma LUT to remap all
> @@ -318,9 +319,9 @@ static void test_pipe_degamma(data_t *data,
>  		 */
>  		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		set_degamma(data, primary->pipe, degamma_full);
> +		set_degamma(data, pipe, degamma_full);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
>  		/* Verify that the CRC of the software computed output is
> @@ -342,6 +343,7 @@ static void test_pipe_degamma(data_t *data,
>   * LUT and verify we have the same CRC as drawing solid color rectangles.
>   */
>  static void test_pipe_gamma(data_t *data,
> +			    igt_pipe_t *pipe,
>  			    igt_plane_t *primary)
>  {
>  	igt_output_t *output;
> @@ -354,13 +356,13 @@ static void test_pipe_gamma(data_t *data,
>  
>  	gamma_full = generate_table_max(data->gamma_lut_size);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
>  		drmModeModeInfo *mode;
>  		struct igt_fb fb_modeset, fb;
>  		igt_crc_t crc_fullgamma, crc_fullcolors;
>  		int fb_id, fb_modeset_id;
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +		igt_output_set_pipe(output, pipe->pipe);
>  		mode = igt_output_get_mode(output);
>  
>  		/* Create a framebuffer at the size of the output. */
> @@ -381,16 +383,16 @@ static void test_pipe_gamma(data_t *data,
>  		igt_assert(fb_modeset_id);
>  
>  		igt_plane_set_fb(primary, &fb_modeset);
> -		disable_ctm(primary->pipe);
> -		disable_degamma(primary->pipe);
> -		set_gamma(data, primary->pipe, gamma_full);
> +		disable_ctm(pipe);
> +		disable_degamma(pipe);
> +		set_gamma(data, pipe, gamma_full);
>  		igt_display_commit(&data->display);
>  
>  		/* Draw solid colors with no gamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
>  		/* Draw a gradient with gamma LUT to remap all values
> @@ -399,7 +401,7 @@ static void test_pipe_gamma(data_t *data,
>  		paint_gradient_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
>  		/* Verify that the CRC of the software computed output is
> @@ -420,6 +422,7 @@ static void test_pipe_gamma(data_t *data,
>   * with linear legacy gamma LUT.
>   */
>  static void test_pipe_legacy_gamma(data_t *data,
> +				   igt_pipe_t *pipe,
>  				   igt_plane_t *primary)
>  {
>  	igt_output_t *output;
> @@ -432,7 +435,7 @@ static void test_pipe_legacy_gamma(data_t *data,
>  	uint32_t i, legacy_lut_size;
>  	uint16_t *red_lut, *green_lut, *blue_lut;
>  
> -	kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
> +	kms_crtc = drmModeGetCrtc(data->drm_fd, pipe->crtc_id);
>  	legacy_lut_size = kms_crtc->gamma_size;
>  	drmModeFreeCrtc(kms_crtc);
>  
> @@ -440,13 +443,13 @@ static void test_pipe_legacy_gamma(data_t *data,
>  	green_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
>  	blue_lut = malloc(sizeof(uint16_t) * legacy_lut_size);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
>  		drmModeModeInfo *mode;
>  		struct igt_fb fb_modeset, fb;
>  		igt_crc_t crc_fullgamma, crc_fullcolors;
>  		int fb_id, fb_modeset_id;
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +		igt_output_set_pipe(output, pipe->pipe);
>  		mode = igt_output_get_mode(output);
>  
>  		/* Create a framebuffer at the size of the output. */
> @@ -467,16 +470,16 @@ static void test_pipe_legacy_gamma(data_t *data,
>  		igt_assert(fb_modeset_id);
>  
>  		igt_plane_set_fb(primary, &fb_modeset);
> -		disable_degamma(primary->pipe);
> -		disable_gamma(primary->pipe);
> -		disable_ctm(primary->pipe);
> +		disable_degamma(pipe);
> +		disable_gamma(pipe);
> +		disable_ctm(pipe);
>  		igt_display_commit(&data->display);
>  
>  		/* Draw solid colors with no gamma transformation. */
>  		paint_rectangles(data, mode, red_green_blue, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullcolors);
>  
>  		/* Draw a gradient with gamma LUT to remap all values
> @@ -488,10 +491,10 @@ static void test_pipe_legacy_gamma(data_t *data,
>  		red_lut[0] = green_lut[0] = blue_lut[0] = 0;
>  		for (i = 1; i < legacy_lut_size; i++)
>  			red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
> -		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, primary->pipe->crtc_id,
> +		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, pipe->crtc_id,
>  						  legacy_lut_size, red_lut, green_lut, blue_lut), 0);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_fullgamma);
>  
>  		/* Verify that the CRC of the software computed output is
> @@ -503,7 +506,7 @@ static void test_pipe_legacy_gamma(data_t *data,
>  		for (i = 1; i < legacy_lut_size; i++)
>  			red_lut[i] = green_lut[i] = blue_lut[i] = i << 8;
>  
> -		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, primary->pipe->crtc_id,
> +		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd, pipe->crtc_id,
>  						  legacy_lut_size, red_lut, green_lut, blue_lut), 0);
>  		igt_display_commit(&data->display);
>  
> @@ -534,6 +537,7 @@ get_blob(data_t *data, igt_pipe_t *pipe, enum igt_atomic_crtc_properties prop)
>   * through the GAMMA_LUT property.
>   */
>  static void test_pipe_legacy_gamma_reset(data_t *data,
> +					 igt_pipe_t *pipe,
>  					 igt_plane_t *primary)
>  {
>  	const double ctm_identity[] = {
> @@ -552,35 +556,35 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_zero = generate_table_zero(data->gamma_lut_size);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
> +		igt_output_set_pipe(output, pipe->pipe);
>  
>  		/* Ensure we have a clean state to start with. */
> -		disable_degamma(primary->pipe);
> -		disable_ctm(primary->pipe);
> -		disable_gamma(primary->pipe);
> +		disable_degamma(pipe);
> +		disable_ctm(pipe);
> +		disable_gamma(pipe);
>  		igt_display_commit(&data->display);
>  
>  		/* Set a degama & gamma LUT and a CTM using the
>  		 * properties and verify the content of the
>  		 * properties. */
> -		set_degamma(data, primary->pipe, degamma_linear);
> -		set_ctm(primary->pipe, ctm_identity);
> -		set_gamma(data, primary->pipe, gamma_zero);
> +		set_degamma(data, pipe, degamma_linear);
> +		set_ctm(pipe, ctm_identity);
> +		set_gamma(data, pipe, gamma_zero);
>  		igt_display_commit(&data->display);
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_DEGAMMA_LUT);
> +		blob = get_blob(data, pipe, IGT_CRTC_DEGAMMA_LUT);
>  		igt_assert(blob &&
>  			   blob->length == (sizeof(struct _drm_color_lut) *
>  					    data->degamma_lut_size));
>  		drmModeFreePropertyBlob(blob);
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_CTM);
> +		blob = get_blob(data, pipe, IGT_CRTC_CTM);
>  		igt_assert(blob &&
>  			   blob->length == sizeof(struct _drm_color_ctm));
>  		drmModeFreePropertyBlob(blob);
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
> +		blob = get_blob(data, pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
>  			   blob->length == (sizeof(struct _drm_color_lut) *
>  					    data->gamma_lut_size));
> @@ -594,7 +598,7 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  		/* Set a gamma LUT using the legacy ioctl and verify
>  		 * the content of the GAMMA_LUT property is changed
>  		 * and that CTM and DEGAMMA_LUT are empty. */
> -		kms_crtc = drmModeGetCrtc(data->drm_fd, primary->pipe->crtc_id);
> +		kms_crtc = drmModeGetCrtc(data->drm_fd, pipe->crtc_id);
>  		legacy_lut_size = kms_crtc->gamma_size;
>  		drmModeFreeCrtc(kms_crtc);
>  
> @@ -606,17 +610,17 @@ static void test_pipe_legacy_gamma_reset(data_t *data,
>  			red_lut[i] = green_lut[i] = blue_lut[i] = 0xffff;
>  
>  		igt_assert_eq(drmModeCrtcSetGamma(data->drm_fd,
> -						  primary->pipe->crtc_id,
> +						  pipe->crtc_id,
>  						  legacy_lut_size,
>  						  red_lut, green_lut, blue_lut),
>  			      0);
>  		igt_display_commit(&data->display);
>  
> -		igt_assert(get_blob(data, primary->pipe,
> +		igt_assert(get_blob(data, pipe,
>  				    IGT_CRTC_DEGAMMA_LUT) == NULL);
> -		igt_assert(get_blob(data, primary->pipe, IGT_CRTC_CTM) == NULL);
> +		igt_assert(get_blob(data, pipe, IGT_CRTC_CTM) == NULL);
>  
> -		blob = get_blob(data, primary->pipe, IGT_CRTC_GAMMA_LUT);
> +		blob = get_blob(data, pipe, IGT_CRTC_GAMMA_LUT);
>  		igt_assert(blob &&
>  			   blob->length == (sizeof(struct _drm_color_lut) *
>  					    legacy_lut_size));
> @@ -645,6 +649,7 @@ static bool crc_equal(igt_crc_t *a, igt_crc_t *b)
>   * the CRC is equal to using after colors with an identify ctm matrix.
>   */
>  static bool test_pipe_ctm(data_t *data,
> +			  igt_pipe_t *pipe,
>  			  igt_plane_t *primary,
>  			  color_t *before,
>  			  color_t *after,
> @@ -662,13 +667,13 @@ static bool test_pipe_ctm(data_t *data,
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
>  		drmModeModeInfo *mode;
>  		struct igt_fb fb_modeset, fb;
>  		igt_crc_t crc_software, crc_hardware;
>  		int fb_id, fb_modeset_id;
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +		igt_output_set_pipe(output, pipe->pipe);
>  		mode = igt_output_get_mode(output);
>  
>  		/* Create a framebuffer at the size of the output. */
> @@ -689,24 +694,24 @@ static bool test_pipe_ctm(data_t *data,
>  		igt_assert(fb_modeset_id);
>  		igt_plane_set_fb(primary, &fb_modeset);
>  
> -		set_degamma(data, primary->pipe, degamma_linear);
> -		set_gamma(data, primary->pipe, gamma_linear);
> -		disable_ctm(primary->pipe);
> +		set_degamma(data, pipe, degamma_linear);
> +		set_gamma(data, pipe, gamma_linear);
> +		disable_ctm(pipe);
>  		igt_display_commit(&data->display);
>  
>  		paint_rectangles(data, mode, after, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		set_ctm(primary->pipe, ctm_identity);
> +		set_ctm(pipe, ctm_identity);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_software);
>  
>  		/* With CTM transformation. */
>  		paint_rectangles(data, mode, before, &fb);
>  		igt_plane_set_fb(primary, &fb);
> -		set_ctm(primary->pipe, ctm_matrix);
> +		set_ctm(pipe, ctm_matrix);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_hardware);
>  
>  		/* Verify that the CRC of the software computed output is
> @@ -738,6 +743,7 @@ static bool test_pipe_ctm(data_t *data,
>   */
>  #if 0
>  static void test_pipe_limited_range_ctm(data_t *data,
> +					igt_pipe_t *pipe,
>  					igt_plane_t *primary)
>  {
>  	double limited_result = 235.0 / 255.0;
> @@ -761,7 +767,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  	degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>  	gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>  
> -	for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
> +	for_each_valid_output_on_pipe(&data->display, pipe->pipe, output) {
>  		drmModeModeInfo *mode;
>  		struct igt_fb fb_modeset, fb;
>  		igt_crc_t crc_full, crc_limited;
> @@ -772,7 +778,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  
>  		has_broadcast_rgb_output = true;
>  
> -		igt_output_set_pipe(output, primary->pipe->pipe);
> +		igt_output_set_pipe(output, pipe->pipe);
>  		mode = igt_output_get_mode(output);
>  
>  		/* Create a framebuffer at the size of the output. */
> @@ -793,15 +799,15 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  		igt_assert(fb_modeset_id);
>  		igt_plane_set_fb(primary, &fb_modeset);
>  
> -		set_degamma(data, primary->pipe, degamma_linear);
> -		set_gamma(data, primary->pipe, gamma_linear);
> -		set_ctm(primary->pipe, ctm);
> +		set_degamma(data, pipe, degamma_linear);
> +		set_gamma(data, pipe, gamma_linear);
> +		set_ctm(pipe, ctm);
>  
>  		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
>  		paint_rectangles(data, mode, red_green_blue_limited, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
>  
>  		/* Set the output into limited range. */
> @@ -809,7 +815,7 @@ static void test_pipe_limited_range_ctm(data_t *data,
>  		paint_rectangles(data, mode, red_green_blue_full, &fb);
>  		igt_plane_set_fb(primary, &fb);
>  		igt_display_commit(&data->display);
> -		igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> +		igt_wait_for_vblank(data->drm_fd, pipe->pipe);
>  		igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
>  
>  		/* And reset.. */
> @@ -853,8 +859,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  
>  		primary = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
>  
> -		data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
> -						  primary->pipe->pipe,
> +		data->pipe_crc = igt_pipe_crc_new(data->drm_fd, p,
>  						  INTEL_PIPE_CRC_SOURCE_AUTO);
>  
>  		igt_require(igt_pipe_obj_has_prop(&data->display.pipes[p], IGT_CRTC_DEGAMMA_LUT_SIZE));
> @@ -885,7 +890,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		double ctm[] = { 0.0, 0.0, 0.0,
>  				0.0, 1.0, 0.0,
>  				1.0, 0.0, 1.0 };
> -		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
> +		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
>  					 blue_green_blue, ctm));
>  	}
>  
> @@ -898,7 +903,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		double ctm[] = { 1.0, 1.0, 0.0,
>  				0.0, 0.0, 0.0,
>  				0.0, 0.0, 1.0 };
> -		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
> +		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
>  					 red_red_blue, ctm));
>  	}
>  
> @@ -911,7 +916,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		double ctm[] = { 1.0, 0.0, 1.0,
>  				0.0, 1.0, 0.0,
>  				0.0, 0.0, 0.0 };
> -		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
> +		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
>  					 red_green_red, ctm));
>  	}
>  
> @@ -933,7 +938,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  				expected_colors[1].g =
>  				expected_colors[2].b =
>  				0.25 + delta * (i - 2);
> -			success |= test_pipe_ctm(data, primary,
> +			success |= test_pipe_ctm(data, pipe, primary,
>  						 red_green_blue,
>  						 expected_colors, ctm);
>  		}
> @@ -954,7 +959,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  				expected_colors[1].g =
>  				expected_colors[2].b =
>  				0.5 + delta * (i - 2);
> -			success |= test_pipe_ctm(data, primary,
> +			success |= test_pipe_ctm(data, pipe, primary,
>  						 red_green_blue,
>  						 expected_colors, ctm);
>  		}
> @@ -975,7 +980,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  				expected_colors[1].g =
>  				expected_colors[2].b =
>  				0.75 + delta * (i - 3);
> -			success |= test_pipe_ctm(data, primary,
> +			success |= test_pipe_ctm(data, pipe, primary,
>  						 red_green_blue,
>  						 expected_colors, ctm);
>  		}
> @@ -996,7 +1001,7 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		 * produce with an 8 bits per color framebuffer. */
>  		igt_require(!IS_CHERRYVIEW(data->devid));
>  
> -		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
> +		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
>  					 full_rgb, ctm));
>  	}
>  
> @@ -1009,31 +1014,31 @@ run_tests_for_pipe(data_t *data, enum pipe p)
>  		double ctm[] = { -1.0,  0.0,  0.0,
>  				 0.0, -1.0,  0.0,
>  				 0.0,  0.0, -1.0 };
> -		igt_assert(test_pipe_ctm(data, primary, red_green_blue,
> +		igt_assert(test_pipe_ctm(data, pipe, primary, red_green_blue,
>  					 all_black, ctm));
>  	}
>  
>  #if 0
>  	igt_subtest_f("pipe-%s-ctm-limited-range", kmstest_pipe_name(p))
> -		test_pipe_limited_range_ctm(data, primary);
> +		test_pipe_limited_range_ctm(data, pipe, primary);
>  #endif
>  
>  	igt_subtest_f("pipe-%s-degamma", kmstest_pipe_name(p))
> -		test_pipe_degamma(data, primary);
> +		test_pipe_degamma(data, pipe, primary);
>  
>  	igt_subtest_f("pipe-%s-gamma", kmstest_pipe_name(p))
> -		test_pipe_gamma(data, primary);
> +		test_pipe_gamma(data, pipe, primary);
>  
>  	igt_subtest_f("pipe-%s-legacy-gamma", kmstest_pipe_name(p))
> -		test_pipe_legacy_gamma(data, primary);
> +		test_pipe_legacy_gamma(data, pipe, primary);
>  
>  	igt_subtest_f("pipe-%s-legacy-gamma-reset", kmstest_pipe_name(p))
> -		test_pipe_legacy_gamma_reset(data, primary);
> +		test_pipe_legacy_gamma_reset(data, pipe, primary);
>  
>  	igt_fixture {
> -		disable_degamma(primary->pipe);
> -		disable_gamma(primary->pipe);
> -		disable_ctm(primary->pipe);
> +		disable_degamma(pipe);
> +		disable_gamma(pipe);
> +		disable_ctm(pipe);
>  		igt_display_commit(&data->display);
>  
>  		igt_pipe_crc_free(data->pipe_crc);
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
> index 283acf8c8a7e..695b14822c9a 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -166,7 +166,7 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
>  	igt_output_set_pipe(output, pipe);
>  
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	p = primary->pipe;
> +	p = &data->display.pipes[pipe];
>  
>  	x = malloc(p->n_planes * sizeof(*x));
>  	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (p->n_planes * sizeof(*x)));
> diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
> index 6407e19bafce..694d8affa516 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -81,6 +81,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  	int fb_id;
>  	double alpha;
>  
> +	igt_display_reset(display);
>  	igt_output_set_pipe(output, pipe);

Please remove the redundant igt_output_set_pipe in test_crtc_background.

Aside: The kernel implementation never landed for lack of userspace. Yay
for pushing stuff without the entire thing being done :-/
>  
>  	/* create the pipe_crc object for this pipe */
> @@ -89,6 +90,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  
>  	mode = igt_output_get_mode(output);
>  
> +	igt_remove_fb(data->gfx_fd, &data->fb);
>  	fb_id = igt_create_fb(data->gfx_fd,
>  			mode->hdisplay, mode->vdisplay,
>  			DRM_FORMAT_XRGB8888,
> @@ -108,22 +110,6 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>  	igt_display_commit2(display, COMMIT_UNIVERSAL);
>  }
>  
> -static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
> -{
> -	igt_display_t *display = &data->display;
> -
> -	igt_pipe_crc_free(data->pipe_crc);
> -	data->pipe_crc = NULL;
> -
> -	igt_remove_fb(data->gfx_fd, &data->fb);
> -
> -	igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND, BLACK64);
> -	igt_plane_set_fb(plane, NULL);
> -	igt_output_set_pipe(output, PIPE_ANY);
> -
> -	igt_display_commit2(display, COMMIT_UNIVERSAL);
> -}
> -
>  static void test_crtc_background(data_t *data)
>  {
>  	igt_display_t *display = &data->display;
> @@ -166,8 +152,11 @@ static void test_crtc_background(data_t *data)
>  		igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE64);
>  		igt_display_commit2(display, COMMIT_UNIVERSAL);
>  
> +		/* And reset to default */
> +		igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
> +		igt_display_commit2(display, COMMIT_UNIVERSAL);
> +
>  		valid_tests++;
> -		cleanup_crtc(data, output, plane);
>  	}
>  	igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
>  }
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index e61bc84624b3..366293b3f9ee 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -185,7 +185,8 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>  	       uint64_t tiling, int max_planes, igt_output_t *output)
>  {
>  	drmModeModeInfo *mode;
> -	igt_pipe_t *pipe;
> +	igt_display_t *display = output->display;
> +	igt_pipe_t *pipe = &display->pipes[pipe_id];
>  	igt_plane_t *primary;
>  	int *x;
>  	int *y;
> @@ -194,7 +195,6 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>  
>  	igt_output_set_pipe(output, pipe_id);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	pipe = primary->pipe;
>  
>  	x = malloc(pipe->n_planes * sizeof(*x));
>  	igt_assert_f(x, "Failed to allocate %ld bytes for variable x\n", (long int) (pipe->n_planes * sizeof(*x)));

Bunch of bikesheds, with or without them:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> -- 
> 2.17.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2.
  2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2 Maarten Lankhorst
@ 2018-05-07 14:57   ` Daniel Vetter
  2018-05-08 10:10     ` Maarten Lankhorst
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2018-05-07 14:57 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev, Laurent Pinchart, Daniel Vetter

On Mon, May 07, 2018 at 03:14:29PM +0200, Maarten Lankhorst wrote:
> Because planes will no longer be fixed to a pipe, the index becomes
> useless. Use igt_output_get_plane_type() instead to get a plane,
> or for_each_plane_on_pipe().
> 
> Changes since v1:
> - Only test overlay planes in the plane-position tests,
>   just like before this commit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Hm, igt_pipe_get_plane_type is super verbose. Can't we just mass-switch to
a simple pipe->plane_primary? That also has the upshot of being much more
aligned with kernel code ...

On that note, igt_output_get_driving_pipe is also a verbose beast. I think
storing the pending_pipe as a full igt_pipe_t * pointer would result in
plenty cleaner code.
-Daniel


> ---
>  lib/igt_kms.c              | 19 -------------
>  lib/igt_kms.h              |  1 -
>  tests/kms_concurrent.c     | 22 ++++++++-------
>  tests/kms_flip_tiling.c    |  2 +-
>  tests/kms_pipe_b_c_ivb.c   |  4 +--
>  tests/kms_pipe_crc_basic.c |  2 +-
>  tests/kms_plane.c          | 40 ++++++++++++++------------
>  tests/kms_plane_multiple.c | 57 +++++++++++++++-----------------------
>  tests/kms_plane_scaling.c  | 47 +++++++++++++++++++++++--------
>  tests/kms_vblank.c         |  2 +-
>  10 files changed, 99 insertions(+), 97 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 7281c71607a3..e9b8c4319a61 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2168,15 +2168,6 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
>  	return &display->pipes[pipe];
>  }
>  
> -static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int plane_idx)
> -{
> -	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
> -		      "Valid pipe->planes plane_idx not found, plane_idx=%d n_planes=%d",
> -		      plane_idx, pipe->n_planes);
> -
> -	return &pipe->planes[plane_idx];
> -}
> -
>  /**
>   * igt_pipe_get_plane_type:
>   * @pipe: Target pipe
> @@ -3482,16 +3473,6 @@ void igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
>  		igt_pipe_obj_set_prop_changed(pipe_obj, IGT_CRTC_MODE_ID);
>  }
>  
> -igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx)
> -{
> -	igt_pipe_t *pipe;
> -
> -	pipe = igt_output_get_driving_pipe(output);
> -	igt_assert(pipe);
> -
> -	return igt_pipe_get_plane(pipe, plane_idx);
> -}
> -
>  /**
>   * igt_output_get_plane_type:
>   * @output: Target output
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5e7db0a263fc..fd43e8c86a85 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -388,7 +388,6 @@ const char *igt_output_name(igt_output_t *output);
>  drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
>  void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
>  void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
> -igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
>  igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
>  igt_output_t *igt_output_from_connector(igt_display_t *display,
>      drmModeConnector *connector);
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
> index 695b14822c9a..976ba677b935 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -119,12 +119,13 @@ static void
>  create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
>  			    int *rect_x, int *rect_y,
>  			    int *rect_w, int *rect_h,
> -			    uint64_t tiling, int max_planes,
> +			    uint64_t tiling, enum pipe pipe,
>  			    igt_output_t *output)
>  {
>  	unsigned int fb_id;
>  	cairo_t *cr;
>  	igt_plane_t *primary;
> +	igt_plane_t *plane;
>  
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
> @@ -140,8 +141,10 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
>  			mode->hdisplay, mode->vdisplay,
>  			0.0f, 0.0f, 1.0f);
>  
> -	for (int i = 0; i < max_planes; i++) {
> -		if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
> +	for_each_plane_on_pipe(&data->display, pipe, plane) {
> +		int i = plane->index;
> +
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
>  
>  		igt_paint_color(cr, rect_x[i], rect_y[i],
> @@ -152,16 +155,16 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
>  }
>  
>  static void
> -prepare_planes(data_t *data, enum pipe pipe, int max_planes,
> +prepare_planes(data_t *data, enum pipe pipe,
>  	       igt_output_t *output)
>  {
> +	igt_plane_t *plane;
>  	drmModeModeInfo *mode;
>  	igt_pipe_t *p;
>  	igt_plane_t *primary;
>  	int *x;
>  	int *y;
>  	int *size;
> -	int i;
>  
>  	igt_output_set_pipe(output, pipe);
>  
> @@ -182,8 +185,8 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
>  	/* planes with random positions */
>  	x[primary->index] = 0;
>  	y[primary->index] = 0;
> -	for (i = 0; i < max_planes; i++) {
> -		igt_plane_t *plane = igt_output_get_plane(output, i);
> +	for_each_plane_on_pipe(&data->display, pipe, plane) {
> +		int i = plane->index;
>  
>  		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
> @@ -212,7 +215,7 @@ prepare_planes(data_t *data, enum pipe pipe, int max_planes,
>  	data->plane[primary->index] = primary;
>  	create_fb_for_mode_position(data, mode, x, y, size, size,
>  				    LOCAL_I915_FORMAT_MOD_X_TILED,
> -				    max_planes, output);
> +				    pipe, output);
>  
>  	igt_plane_set_fb(data->plane[primary->index], &data->fb[primary->index]);
>  }
> @@ -223,13 +226,12 @@ test_plane_position_with_output(data_t *data, enum pipe pipe, igt_output_t *outp
>  	int i;
>  	int iterations = opt.iterations < 1 ? 1 : opt.iterations;
>  	bool loop_forever = opt.iterations == LOOP_FOREVER ? true : false;
> -	int max_planes = data->display.pipes[pipe].n_planes;
>  
>  	igt_pipe_refresh(&data->display, pipe, true);
>  
>  	i = 0;
>  	while (i < iterations || loop_forever) {
> -		prepare_planes(data, pipe, max_planes, output);
> +		prepare_planes(data, pipe, output);
>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
>  		i++;
> diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
> index 5aae29a8d56b..ca486367fdec 100644
> --- a/tests/kms_flip_tiling.c
> +++ b/tests/kms_flip_tiling.c
> @@ -93,7 +93,7 @@ test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t ti
>  	igt_output_set_pipe(output, pipe);
>  
>  	mode = igt_output_get_mode(output);
> -	primary = igt_output_get_plane(output, 0);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
>  	width = mode->hdisplay;
>  
> diff --git a/tests/kms_pipe_b_c_ivb.c b/tests/kms_pipe_b_c_ivb.c
> index 64086915e8e0..14988bb55b6c 100644
> --- a/tests/kms_pipe_b_c_ivb.c
> +++ b/tests/kms_pipe_b_c_ivb.c
> @@ -71,7 +71,7 @@ disable_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
>  	igt_plane_t *primary;
>  
>  	igt_output_set_pipe(output, pipe);
> -	primary = igt_output_get_plane(output, 0);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(primary, NULL);
>  	return igt_display_commit(&data->display);
>  }
> @@ -88,7 +88,7 @@ set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
>  
>  	mode = igt_output_get_mode(output);
>  
> -	primary = igt_output_get_plane(output, 0);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
>  	fb_id = igt_create_color_fb(data->drm_fd,
>  				    mode->hdisplay, mode->vdisplay,
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 235fdc386ba2..f5f59015c55d 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -86,7 +86,7 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
>  					colors[c].b,
>  					&data->fb);
>  
> -		primary = igt_output_get_plane(output, 0);
> +		primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  		igt_plane_set_fb(primary, &data->fb);
>  
>  		igt_display_commit(display);
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 23173b966eab..d5027b25548d 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -72,7 +72,7 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
>  
>  	igt_output_set_pipe(output, pipe);
>  
> -	primary = igt_output_get_plane(output, 0);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
>  	mode = igt_output_get_mode(output);
>  	igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> @@ -149,43 +149,45 @@ enum {
>  static void
>  test_plane_position_with_output(data_t *data,
>  				enum pipe pipe,
> -				int plane,
> +				igt_plane_t *plane,
>  				igt_output_t *output,
>  				igt_crc_t *reference_crc,
>  				unsigned int flags)
>  {
> -	igt_plane_t *primary, *sprite;
> +	igt_plane_t *primary;
>  	struct igt_fb primary_fb, sprite_fb;
>  	drmModeModeInfo *mode;
>  	igt_crc_t crc, crc2;
>  
>  	igt_info("Testing connector %s using pipe %s plane %d\n",
> -		 igt_output_name(output), kmstest_pipe_name(pipe), plane);
> +		 igt_output_name(output), kmstest_pipe_name(pipe), plane->index);
>  
>  	igt_output_set_pipe(output, pipe);
>  
>  	mode = igt_output_get_mode(output);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> -	sprite = igt_output_get_plane(output, plane);
>  
>  	create_fb_for_mode__position(data, mode, 100, 100, 64, 64,
>  				     &primary_fb);
>  	igt_plane_set_fb(primary, &primary_fb);
>  
> +	if (primary == plane && !data->display.is_atomic)
> +		igt_display_commit(&data->display);
> +
>  	igt_create_color_fb(data->drm_fd,
>  				64, 64, /* width, height */
>  				DRM_FORMAT_XRGB8888,
>  				LOCAL_DRM_FORMAT_MOD_NONE,
>  				0.0, 1.0, 0.0,
>  				&sprite_fb);
> -	igt_plane_set_fb(sprite, &sprite_fb);
> +	igt_plane_set_fb(plane, &sprite_fb);
>  
>  	if (flags & TEST_POSITION_FULLY_COVERED)
> -		igt_plane_set_position(sprite, 100, 100);
> +		igt_plane_set_position(plane, 100, 100);
>  	else
> -		igt_plane_set_position(sprite, 132, 132);
> +		igt_plane_set_position(plane, 132, 132);
>  
> -	igt_display_commit(&data->display);
> +	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
>  
>  	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
>  
> @@ -209,7 +211,7 @@ test_plane_position_with_output(data_t *data,
>  	igt_assert_crc_equal(&crc, &crc2);
>  
>  	igt_plane_set_fb(primary, NULL);
> -	igt_plane_set_fb(sprite, NULL);
> +	igt_plane_set_fb(plane, NULL);
>  
>  	/* reset the constraint on the pipe */
>  	igt_output_set_pipe(output, PIPE_ANY);
> @@ -222,17 +224,21 @@ test_plane_position(data_t *data, enum pipe pipe, unsigned int flags)
>  	int connected_outs = 0;
>  
>  	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> -		int n_planes = data->display.pipes[pipe].n_planes;
> +		igt_plane_t *plane;
>  		igt_crc_t reference_crc;
>  
>  		test_init(data, pipe);
>  
>  		test_grab_crc(data, output, pipe, &green, &reference_crc);
>  
> -		for (int plane = 1; plane < n_planes; plane++)
> +		for_each_plane_on_pipe(&data->display, pipe, plane) {
> +			if (plane->type != DRM_PLANE_TYPE_OVERLAY)
> +				continue;
> +
>  			test_plane_position_with_output(data, pipe, plane,
>  							output, &reference_crc,
>  							flags);
> +		}
>  
>  		test_fini(data);
>  
> @@ -290,7 +296,7 @@ enum {
>  static void
>  test_plane_panning_with_output(data_t *data,
>  			       enum pipe pipe,
> -			       int plane,
> +			       igt_plane_t *plane,
>  			       igt_output_t *output,
>  			       igt_crc_t *red_crc, igt_crc_t *blue_crc,
>  			       unsigned int flags)
> @@ -301,12 +307,12 @@ test_plane_panning_with_output(data_t *data,
>  	igt_crc_t crc;
>  
>  	igt_info("Testing connector %s using pipe %s plane %d\n",
> -		 igt_output_name(output), kmstest_pipe_name(pipe), plane);
> +		 igt_output_name(output), kmstest_pipe_name(pipe), plane->index);
>  
>  	igt_output_set_pipe(output, pipe);
>  
>  	mode = igt_output_get_mode(output);
> -	primary = igt_output_get_plane(output, 0);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
>  	create_fb_for_mode__panning(data, mode, &primary_fb);
>  	igt_plane_set_fb(primary, &primary_fb);
> @@ -343,7 +349,7 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
>  	int connected_outs = 0;
>  
>  	for_each_valid_output_on_pipe(&data->display, pipe, output) {
> -		int n_planes = data->display.pipes[pipe].n_planes;
> +		igt_plane_t *plane;
>  		igt_crc_t red_crc;
>  		igt_crc_t blue_crc;
>  
> @@ -352,7 +358,7 @@ test_plane_panning(data_t *data, enum pipe pipe, unsigned int flags)
>  		test_grab_crc(data, output, pipe, &red, &red_crc);
>  		test_grab_crc(data, output, pipe, &blue, &blue_crc);
>  
> -		for (int plane = 1; plane < n_planes; plane++)
> +		for_each_plane_on_pipe(&data->display, pipe, plane)
>  			test_plane_panning_with_output(data, pipe, plane,
>  						       output,
>  						       &red_crc, &blue_crc,
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 366293b3f9ee..45e0fd31fce3 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -65,8 +65,10 @@ struct {
>  /*
>   * Common code across all tests, acting on data_t
>   */
> -static void test_init(data_t *data, enum pipe pipe, int n_planes)
> +static void test_init(data_t *data, enum pipe pipe)
>  {
> +	int n_planes = data->display.pipes[pipe].n_planes;
> +
>  	data->pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
>  
>  	data->plane = calloc(n_planes, sizeof(data->plane));
> @@ -76,23 +78,10 @@ static void test_init(data_t *data, enum pipe pipe, int n_planes)
>  	igt_assert_f(data->fb != NULL, "Failed to allocate memory for FBs\n");
>  }
>  
> -static void test_fini(data_t *data, igt_output_t *output, int n_planes)
> +static void test_fini(data_t *data, igt_output_t *output)
>  {
>  	igt_pipe_crc_stop(data->pipe_crc);
>  
> -	for (int i = 0; i < n_planes; i++) {
> -		igt_plane_t *plane = data->plane[i];
> -		if (!plane)
> -			continue;
> -		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> -			continue;
> -		igt_plane_set_fb(plane, NULL);
> -		data->plane[i] = NULL;
> -	}
> -
> -	/* reset the constraint on the pipe */
> -	igt_output_set_pipe(output, PIPE_ANY);
> -
>  	igt_pipe_crc_free(data->pipe_crc);
>  	data->pipe_crc = NULL;
>  
> @@ -149,11 +138,11 @@ static void
>  create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo *mode,
>  			    color_t *color, int *rect_x, int *rect_y,
>  			    int *rect_w, int *rect_h, uint64_t tiling,
> -			    int max_planes)
> +			    enum pipe pipe)
>  {
>  	unsigned int fb_id;
>  	cairo_t *cr;
> -	igt_plane_t *primary;
> +	igt_plane_t *primary, *plane;
>  
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
> @@ -169,12 +158,15 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
>  			mode->hdisplay, mode->vdisplay,
>  			color->red, color->green, color->blue);
>  
> -	for (int i = 0; i < max_planes; i++) {
> -		if (data->plane[i]->type == DRM_PLANE_TYPE_PRIMARY)
> +	for_each_plane_on_pipe(&data->display, pipe, plane) {
> +		int i = plane->index;
> +
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
> +
>  		igt_paint_color(cr, rect_x[i], rect_y[i],
>  				rect_w[i], rect_h[i], 0.0, 0.0, 0.0);
> -		}
> +	}
>  
>  	igt_put_cairo_ctx(data->drm_fd, &data->fb[primary->index], cr);
>  }
> @@ -182,16 +174,15 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
>  
>  static void
>  prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
> -	       uint64_t tiling, int max_planes, igt_output_t *output)
> +	       uint64_t tiling, igt_output_t *output)
>  {
>  	drmModeModeInfo *mode;
>  	igt_display_t *display = output->display;
>  	igt_pipe_t *pipe = &display->pipes[pipe_id];
> -	igt_plane_t *primary;
> +	igt_plane_t *primary, *plane;
>  	int *x;
>  	int *y;
>  	int *size;
> -	int i;
>  
>  	igt_output_set_pipe(output, pipe_id);
>  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> @@ -208,8 +199,8 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>  	/* planes with random positions */
>  	x[primary->index] = 0;
>  	y[primary->index] = 0;
> -	for (i = 0; i < max_planes; i++) {
> -		igt_plane_t *plane = igt_output_get_plane(output, i);
> +	for_each_plane_on_pipe(display, pipe_id, plane) {
> +		int i = plane->index;
>  
>  		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>  			continue;
> @@ -237,13 +228,13 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
>  	/* primary plane */
>  	data->plane[primary->index] = primary;
>  	create_fb_for_mode_position(data, output, mode, color, x, y,
> -				    size, size, tiling, max_planes);
> +				    size, size, tiling, pipe_id);
>  	igt_plane_set_fb(data->plane[primary->index], &data->fb[primary->index]);
>  }
>  
>  static void
>  test_plane_position_with_output(data_t *data, enum pipe pipe,
> -				igt_output_t *output, int n_planes,
> +				igt_output_t *output,
>  				uint64_t tiling)
>  {
>  	color_t blue  = { 0.0f, 0.0f, 1.0f };
> @@ -262,17 +253,17 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>  			iterations, iterations > 1 ? "iterations" : "iteration");
>  	}
>  
> -	igt_info("Testing connector %s using pipe %s with %d planes %s with seed %d\n",
> -		 igt_output_name(output), kmstest_pipe_name(pipe), n_planes,
> +	igt_info("Testing connector %s using pipe %s %s with seed %d\n",
> +		 igt_output_name(output), kmstest_pipe_name(pipe),
>  		 info, opt.seed);
>  
> -	test_init(data, pipe, n_planes);
> +	test_init(data, pipe);
>  
>  	test_grab_crc(data, output, pipe, &blue, tiling);
>  
>  	i = 0;
>  	while (i < iterations || loop_forever) {
> -		prepare_planes(data, pipe, &blue, tiling, n_planes, output);
> +		prepare_planes(data, pipe, &blue, tiling, output);
>  
>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
> @@ -284,7 +275,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>  		i++;
>  	}
>  
> -	test_fini(data, output, n_planes);
> +	test_fini(data, output);
>  }
>  
>  static void
> @@ -293,7 +284,6 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t tiling)
>  	igt_output_t *output;
>  	int connected_outs;
>  	int devid = intel_get_drm_devid(data->drm_fd);
> -	int n_planes = data->display.pipes[pipe].n_planes;
>  
>  	if ((tiling == LOCAL_I915_FORMAT_MOD_Y_TILED ||
>  	     tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED))
> @@ -308,7 +298,6 @@ test_plane_position(data_t *data, enum pipe pipe, uint64_t tiling)
>  	for_each_valid_output_on_pipe(&data->display, pipe, output) {
>  		test_plane_position_with_output(data, pipe,
>  						output,
> -						n_planes,
>  						tiling);
>  		connected_outs++;
>  	}
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index a8454205dbd8..984594753db8 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -257,6 +257,29 @@ static void iterate_plane_scaling(data_t *d, drmModeModeInfo *mode)
>  	}
>  }
>  
> +static void set_plane_mappings(data_t *d, igt_output_t *output, enum pipe pipe)
> +{
> +	igt_plane_t *plane;
> +
> +	igt_output_set_pipe(output, pipe);
> +
> +	/* Set up display with plane 1 */
> +	d->plane1 = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +	d->plane2 = d->plane3 = NULL;
> +
> +	for_each_plane_on_pipe(&d->display, pipe, plane) {
> +		if (plane->type != DRM_PLANE_TYPE_OVERLAY)
> +			continue;
> +
> +		if (!d->plane2)
> +			d->plane2 = plane;
> +		else if (!d->plane3)
> +			d->plane3 = plane;
> +	}
> +
> +}
> +
>  static void
>  test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  {
> @@ -266,8 +289,7 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  
>  	mode = igt_output_get_mode(output);
>  
> -	/* Set up display with plane 1 */
> -	d->plane1 = &display->pipes[pipe].planes[0];
> +	set_plane_mappings(d, output, pipe);
>  	prepare_crtc(d, output, pipe, d->plane1, mode);
>  
>  	igt_create_color_pattern_fb(display->drm_fd, 600, 600,
> @@ -298,7 +320,6 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  	}
>  
>  	/* Set up fb[1]->plane2 mapping. */
> -	d->plane2 = igt_output_get_plane(output, 1);
>  	igt_plane_set_fb(d->plane2, &d->fb[1]);
>  
>  	/* 2nd plane windowed */
> @@ -336,10 +357,9 @@ test_plane_scaling_on_pipe(data_t *d, enum pipe pipe, igt_output_t *output)
>  	}
>  
>  	/* Set up fb[2]->plane3 mapping. */
> -	d->plane3 = igt_output_get_plane(output, 2);
>  	igt_plane_set_fb(d->plane3, &d->fb[2]);
>  
> -	if(d->plane3->type == DRM_PLANE_TYPE_CURSOR) {
> +	if(!d->plane3) {
>  		igt_debug("Plane-3 doesnt exist on pipe %s\n", kmstest_pipe_name(pipe));
>  		return;
>  	}
> @@ -418,8 +438,7 @@ test_scaler_with_clipping_clamping_scenario(data_t *d, enum pipe pipe, igt_outpu
>  	igt_require(get_num_scalers(d->devid, pipe) >= 2);
>  
>  	mode = igt_output_get_mode(output);
> -	d->plane1 = &d->display.pipes[pipe].planes[0];
> -	d->plane2 = &d->display.pipes[pipe].planes[1];
> +	set_plane_mappings(d, output, pipe);
>  	prepare_crtc(d, output, pipe, d->plane1, mode);
>  
>  	for (int i = 0; i < d->plane1->drm_plane->count_formats; i++) {
> @@ -481,10 +500,16 @@ static void test_scaler_with_multi_pipe_plane(data_t *d)
>  	igt_output_set_pipe(output1, pipe1);
>  	igt_output_set_pipe(output2, pipe2);
>  
> -	d->plane1 = igt_output_get_plane(output1, 0);
> -	d->plane2 = get_num_scalers(d->devid, pipe1) >= 2 ? igt_output_get_plane(output1, 1) : NULL;
> -	d->plane3 = igt_output_get_plane(output2, 0);
> -	d->plane4 = get_num_scalers(d->devid, pipe2) >= 2 ? igt_output_get_plane(output2, 1) : NULL;
> +	d->plane1 = igt_output_get_plane_type(output1, DRM_PLANE_TYPE_PRIMARY);
> +	if (get_num_scalers(d->devid, pipe1) >= 2)
> +		d->plane2 = igt_output_get_plane_type(output1, DRM_PLANE_TYPE_OVERLAY);
> +	else
> +		d->plane2 = NULL;
> +	d->plane3 = igt_output_get_plane_type(output2, DRM_PLANE_TYPE_PRIMARY);
> +	if (get_num_scalers(d->devid, pipe2) >= 2)
> +		d->plane4 = igt_output_get_plane_type(output2, DRM_PLANE_TYPE_OVERLAY);
> +	else
> +		d->plane4 = NULL;
>  
>  	mode1 = igt_output_get_mode(output1);
>  	mode2 = igt_output_get_mode(output2);
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 2bee49de55eb..3f9ab34d35bc 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -208,7 +208,7 @@ static void crtc_id_subtest(data_t *data, int fd)
>  		igt_assert_eq(buf.crtc_id, expected_crtc_id);
>  
>  		if (display->is_atomic) {
> -			igt_plane_t *primary = igt_output_get_plane(output, 0);
> +			igt_plane_t *primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>  
>  			igt_plane_set_fb(primary, &data->primary_fb);
>  			igt_display_commit_atomic(display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
> -- 
> 2.17.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for lib/igt_kms: Move planes from pipe to display, v2.
  2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
                   ` (4 preceding siblings ...)
  2018-05-07 14:15 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-05-07 15:40 ` Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-05-07 15:40 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

== Series Details ==

Series: lib/igt_kms: Move planes from pipe to display, v2.
URL   : https://patchwork.freedesktop.org/series/42814/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4463_full -> IGTPW_1325_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1325_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1325_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42814/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1325_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_plane_scaling@pipe-c-plane-scaling:
      shard-apl:          PASS -> CRASH
      shard-kbl:          PASS -> CRASH +2

    
== Known issues ==

  Here are the changes found in IGTPW_1325_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-vebox:
      shard-kbl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-hsw:          PASS -> FAIL (fdo#102887)

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +2

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-snb:          PASS -> FAIL (fdo#104724, fdo#103166)

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103166) +5

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      shard-apl:          NOTRUN -> FAIL (fdo#104724, fdo#103166)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
      shard-hsw:          PASS -> FAIL (fdo#104724, fdo#103166) +1

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      shard-apl:          PASS -> FAIL (fdo#104724, fdo#103166) +4
      shard-kbl:          PASS -> FAIL (fdo#104724, fdo#103166) +5

    igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
      shard-apl:          PASS -> FAIL (fdo#103925, fdo#104724)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS +2

    igt@gem_exec_store@cachelines-bsd:
      shard-hsw:          FAIL (fdo#100007) -> PASS

    igt@gem_tiled_blits@interruptible:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@blocking-absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#106134) -> PASS

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank:
      shard-glk:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#102887) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-apl:          FAIL (fdo#104724, fdo#103167) -> PASS

    
  fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#106134 https://bugs.freedesktop.org/show_bug.cgi?id=106134


== Participating hosts (6 -> 5) ==

  Missing    (1): pig-snb-2600 


== Build changes ==

    * IGT: IGT_4463 -> IGTPW_1325
    * Linux: CI_DRM_4149 -> CI_DRM_4150

  CI_DRM_4149: 6c2ec0dee7d19b798a1de1101175f5a076549cd9 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4150: 93d32416ba4b1dae9451fec28aaa71915d770f51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1325: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1325/
  IGT_4463: 91b5a3ef5516b29584ea4567b0f5ffa18219b29f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4463: 33e58d5583eb7ed3966a1b905f875a1dfa959f6b @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1325/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2.
  2018-05-07 14:57   ` Daniel Vetter
@ 2018-05-08 10:10     ` Maarten Lankhorst
  2018-05-09  9:46       ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-08 10:10 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev, Laurent Pinchart

Op 07-05-18 om 16:57 schreef Daniel Vetter:
> On Mon, May 07, 2018 at 03:14:29PM +0200, Maarten Lankhorst wrote:
>> Because planes will no longer be fixed to a pipe, the index becomes
>> useless. Use igt_output_get_plane_type() instead to get a plane,
>> or for_each_plane_on_pipe().
>>
>> Changes since v1:
>> - Only test overlay planes in the plane-position tests,
>>   just like before this commit.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Hm, igt_pipe_get_plane_type is super verbose. Can't we just mass-switch to
> a simple pipe->plane_primary? That also has the upshot of being much more
> aligned with kernel code ...
>
> On that note, igt_output_get_driving_pipe is also a verbose beast. I think
> storing the pending_pipe as a full igt_pipe_t * pointer would result in
> plenty cleaner code.

With the existing api we have no way to know what pipe has what plane as primary
currently so we only end up with guesses. When the mapping is exposed in the future,
we could hook it up again to igt_pipe_get_plane_type again.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip
  2018-05-07 14:21   ` Daniel Vetter
@ 2018-05-08 14:21     ` Maarten Lankhorst
  0 siblings, 0 replies; 13+ messages in thread
From: Maarten Lankhorst @ 2018-05-08 14:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev, Laurent Pinchart

Op 07-05-18 om 16:21 schreef Daniel Vetter:
> On Mon, May 07, 2018 at 03:14:28PM +0200, Maarten Lankhorst wrote:
>> CS flips no longer exist, so the test has become useless.
>> Other tests like kms_busy already perform some testing
>> that's gpu agnostic.
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks, pushed just this patch. :)

~Maarten
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2.
  2018-05-08 10:10     ` Maarten Lankhorst
@ 2018-05-09  9:46       ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-05-09  9:46 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev, Laurent Pinchart, Daniel Vetter

On Tue, May 08, 2018 at 12:10:17PM +0200, Maarten Lankhorst wrote:
> Op 07-05-18 om 16:57 schreef Daniel Vetter:
> > On Mon, May 07, 2018 at 03:14:29PM +0200, Maarten Lankhorst wrote:
> >> Because planes will no longer be fixed to a pipe, the index becomes
> >> useless. Use igt_output_get_plane_type() instead to get a plane,
> >> or for_each_plane_on_pipe().
> >>
> >> Changes since v1:
> >> - Only test overlay planes in the plane-position tests,
> >>   just like before this commit.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Hm, igt_pipe_get_plane_type is super verbose. Can't we just mass-switch to
> > a simple pipe->plane_primary? That also has the upshot of being much more
> > aligned with kernel code ...
> >
> > On that note, igt_output_get_driving_pipe is also a verbose beast. I think
> > storing the pending_pipe as a full igt_pipe_t * pointer would result in
> > plenty cleaner code.
> 
> With the existing api we have no way to know what pipe has what plane as primary
> currently so we only end up with guesses. When the mapping is exposed in the future,
> we could hook it up again to igt_pipe_get_plane_type again.

That's not what I meant ...

Current state: our best guess for primary and cursor plane is encoded in
the igt_pipe_get_plane_type function, resulting in very verbose code. And
code that doesn't look like kernel code.

My suggestion: Encode that best guess into new igt_pipe_t->primary|cursor
pointers. It's still just a best guess, but the resulting code looks much
better. And since you're touching all these places we might as well do it
pretty.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-05-09  9:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 13:14 [igt-dev] [PATCH i-g-t 0/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 1/4] lib/igt_kms: Remove plane->pipe backpointer, v2 Maarten Lankhorst
2018-05-07 14:51   ` Daniel Vetter
2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 2/4] tests: Remove kms_mmio_vs_cs_flip Maarten Lankhorst
2018-05-07 14:21   ` Daniel Vetter
2018-05-08 14:21     ` Maarten Lankhorst
2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 3/4] lib/igt_kms: Remove igt_output_get_plane(), v2 Maarten Lankhorst
2018-05-07 14:57   ` Daniel Vetter
2018-05-08 10:10     ` Maarten Lankhorst
2018-05-09  9:46       ` Daniel Vetter
2018-05-07 13:14 ` [igt-dev] [PATCH i-g-t 4/4] lib/igt_kms: Move planes from pipe to display, v2 Maarten Lankhorst
2018-05-07 14:15 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2018-05-07 15:40 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork

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.