[PATCH i-g-t v3 00/11] tests/kms_atomic_transition add fence testing

[PATCH i-g-t v3 00/11] tests/kms_atomic_transition add fence testing

[Robert Foss]

This series adds in/out fence testing to kms_atomic_transition test and makes some minor cleanups.

This series is rebased ontop of the dyn_n_planes_v3 series.

This series can be found here:
https://git.collabora.com/cgit/user/robertfoss/intel-gpu-tools.git/log/?h=fences_$VER


Changes since v1:

  lib/igt_kms:
   - Added gtk-doc for exported symbols
   - Changed integer casting to avoid potential issues
   - Changed out_fence_ptr type to int64_t*
   - Fixed igt_plane_set_fence_fd comment

  tests/:
   - Rework timeout change in commit_display()
   - Extract plane_invalid_params_fence() out plane_invalid_params()
   - Extract crtc_invalid_params_fence() out crtc_invalid_params()
   - Prevent add igt_require_sw_sync to subtests using sw_sync


Changes since v2:
  Rebased on upstream/master

  lib/igt_kms:
    - Reset plane->fence_fd to -1 during igt_atomic_prepare_plane_commit()
    - Rework out_fencs_ptr to be an int64_t named out_fence
    - Add igt_pipe_request_out_fence()
  tests/:
    - Switch to using igt_pipe_request_out_fence()
    - Close out_fence fd
    - Change out_fence to int64_t in run_transition_test()
    - Added comments noting that two testcases are not invalid
    - Added igt_pipe_get_last_out_fence() that wraps pipe->fence_out

Gustavo Padovan (8):
  tests/kms_atomic_transition: use igt timeout instead of blocking
  lib/igt_kms: move igt_kms_get_alt_edid() to the right place
  lib/igt_kms: export properties names
  tests/kms_atomic: use global atomic properties definitions
  lib/igt_kms: Add support for the OUT_FENCE_PTR property
  tests/kms_atomic: stress possible fence settings
  tests/kms_atomic_transition: add fencing parameter to
    run_transition_tests
  tests/kms_atomic_transition: add in_fences tests

Robert Foss (3):
  lib/igt_kms: Added igt_pipe_get_last_out_fence()
  lib/igt_kms: Add support for the IN_FENCE_FD property
  tests/kms_atomic_transition: add out_fences tests

 lib/igt_kms.c                 | 104 +++++++++++---
 lib/igt_kms.h                 |  35 ++++-
 tests/kms_atomic.c            | 310 +++++++++++++++++++++++++++++-------------
 tests/kms_atomic_transition.c | 184 +++++++++++++++++++++++--
 4 files changed, 503 insertions(+), 130 deletions(-)

-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 01/11] tests/kms_atomic_transition: use igt timeout instead of blocking

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

If the event never arrives we can timeout and end the test.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 tests/kms_atomic_transition.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 5fdb6175..095af515 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -383,7 +383,9 @@ static void commit_display(igt_display_t *display, unsigned event_mask, bool non
 		struct drm_event_vblank *vblank = (void *)buf;
 		uint32_t crtc_id, pipe = I915_MAX_PIPES;
 
+		igt_set_timeout(3, "Timed out while reading drm_fd\n");
 		ret = read(display->drm_fd, buf, sizeof(buf));
+		igt_reset_timeout();
 		if (ret < 0 && (errno == EINTR || errno == EAGAIN))
 			continue;
 
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 02/11] lib/igt_kms: move igt_kms_get_alt_edid() to the right place

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 4ba6316d..41acce45 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -157,23 +157,6 @@ const unsigned char* igt_kms_get_base_edid(void)
 #define EDID_NAME alt_edid
 #include "igt_edid_template.h"
 
-/**
- * igt_kms_get_alt_edid:
- *
- * Get an alternate edid block, which includes the following modes:
- *
- *  - 1400x1050 60Hz
- *  - 1920x1080 60Hz
- *  - 1280x720 60Hz
- *  - 1024x768 60Hz
- *  - 800x600 60Hz
- *  - 640x480 60Hz
- *
- * This can be extended with further features using functions such as
- * #kmstest_edid_add_3d.
- *
- * Returns: an alternate edid block
- */
 static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	"SRC_X",
 	"SRC_Y",
@@ -301,6 +284,23 @@ igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
 	drmModeFreeObjectProperties(props);
 }
 
+/**
+ * igt_kms_get_alt_edid:
+ *
+ * Get an alternate edid block, which includes the following modes:
+ *
+ *  - 1400x1050 60Hz
+ *  - 1920x1080 60Hz
+ *  - 1280x720 60Hz
+ *  - 1024x768 60Hz
+ *  - 800x600 60Hz
+ *  - 640x480 60Hz
+ *
+ * This can be extended with further features using functions such as
+ * #kmstest_edid_add_3d.
+ *
+ * Returns: an alternate edid block
+ */
 const unsigned char* igt_kms_get_alt_edid(void)
 {
 	update_edid_csum(alt_edid);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 03/11] lib/igt_kms: export properties names

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c |  6 +++---
 lib/igt_kms.h | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 41acce45..142658a6 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -157,7 +157,7 @@ const unsigned char* igt_kms_get_base_edid(void)
 #define EDID_NAME alt_edid
 #include "igt_edid_template.h"
 
-static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
+const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	"SRC_X",
 	"SRC_Y",
 	"SRC_W",
@@ -172,7 +172,7 @@ static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	"rotation"
 };
 
-static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
+const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
 	"background_color",
 	"CTM",
 	"DEGAMMA_LUT",
@@ -181,7 +181,7 @@ static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
 	"ACTIVE"
 };
 
-static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
+const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
 	"scaling mode",
 	"CRTC_ID"
 };
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 25626187..00e0dc68 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -97,12 +97,28 @@ enum igt_atomic_crtc_properties {
        IGT_NUM_CRTC_PROPS
 };
 
+/**
+ * igt_crtc_prop_names
+ *
+ * igt_crtc_prop_names contains a list of crtc property names,
+ * as indexed by the igt_atomic_crtc_properties enum.
+ */
+extern const char *igt_crtc_prop_names[];
+
 enum igt_atomic_connector_properties {
        IGT_CONNECTOR_SCALING_MODE = 0,
        IGT_CONNECTOR_CRTC_ID,
        IGT_NUM_CONNECTOR_PROPS
 };
 
+/**
+ * igt_connector_prop_names
+ *
+ * igt_connector_prop_names contains a list of crtc property names,
+ * as indexed by the igt_atomic_connector_properties enum.
+ */
+extern const char *igt_connector_prop_names[];
+
 struct kmstest_connector_config {
 	drmModeCrtc *crtc;
 	drmModeConnector *connector;
@@ -237,6 +253,13 @@ enum igt_atomic_plane_properties {
        IGT_NUM_PLANE_PROPS
 };
 
+/**
+ * igt_plane_prop_names
+ *
+ * igt_plane_prop_names contains a list of crtc property names,
+ * as indexed by the igt_atomic_plane_properties enum.
+ */
+extern const char *igt_plane_prop_names[];
 
 typedef struct igt_display igt_display_t;
 typedef struct igt_pipe igt_pipe_t;
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 04/11] tests/kms_atomic: use global atomic properties definitions

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 tests/kms_atomic.c | 123 ++++++++++++++++-------------------------------------
 1 file changed, 37 insertions(+), 86 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index e6d71c31..8df51ccd 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -105,55 +105,6 @@ static const char *plane_type_prop_names[NUM_PLANE_TYPE_PROPS] = {
 	"Cursor"
 };
 
-enum plane_properties {
-	PLANE_SRC_X = 0,
-	PLANE_SRC_Y,
-	PLANE_SRC_W,
-	PLANE_SRC_H,
-	PLANE_CRTC_X,
-	PLANE_CRTC_Y,
-	PLANE_CRTC_W,
-	PLANE_CRTC_H,
-	PLANE_FB_ID,
-	PLANE_CRTC_ID,
-	PLANE_TYPE,
-	NUM_PLANE_PROPS
-};
-
-static const char *plane_prop_names[NUM_PLANE_PROPS] = {
-	"SRC_X",
-	"SRC_Y",
-	"SRC_W",
-	"SRC_H",
-	"CRTC_X",
-	"CRTC_Y",
-	"CRTC_W",
-	"CRTC_H",
-	"FB_ID",
-	"CRTC_ID",
-	"type"
-};
-
-enum crtc_properties {
-	CRTC_MODE_ID = 0,
-	CRTC_ACTIVE,
-	NUM_CRTC_PROPS
-};
-
-static const char *crtc_prop_names[NUM_CRTC_PROPS] = {
-	"MODE_ID",
-	"ACTIVE"
-};
-
-enum connector_properties {
-	CONNECTOR_CRTC_ID = 0,
-	NUM_CONNECTOR_PROPS
-};
-
-static const char *connector_prop_names[NUM_CONNECTOR_PROPS] = {
-	"CRTC_ID"
-};
-
 struct kms_atomic_blob {
 	uint32_t id; /* 0 if not already allocated */
 	size_t len;
@@ -197,9 +148,9 @@ struct kms_atomic_state {
 
 struct kms_atomic_desc {
 	int fd;
-	uint32_t props_connector[NUM_CONNECTOR_PROPS];
-	uint32_t props_crtc[NUM_CRTC_PROPS];
-	uint32_t props_plane[NUM_PLANE_PROPS];
+	uint32_t props_connector[IGT_NUM_CONNECTOR_PROPS];
+	uint32_t props_crtc[IGT_NUM_CRTC_PROPS];
+	uint32_t props_plane[IGT_NUM_PLANE_PROPS];
 	uint64_t props_plane_type[NUM_PLANE_TYPE_PROPS];
 };
 
@@ -280,7 +231,7 @@ connector_get_current_state(struct kms_atomic_connector_state *connector)
 	for (i = 0; i < props->count_props; i++) {
 		uint32_t *prop_ids = connector->state->desc->props_connector;
 
-		if (props->props[i] == prop_ids[CONNECTOR_CRTC_ID])
+		if (props->props[i] == prop_ids[IGT_CONNECTOR_CRTC_ID])
 			connector->crtc_id = props->prop_values[i];
 	}
 	drmModeFreeObjectProperties(props);
@@ -348,16 +299,16 @@ find_connector(struct kms_atomic_state *state,
 static void plane_populate_req(struct kms_atomic_plane_state *plane,
 			       drmModeAtomicReq *req)
 {
-	plane_set_prop(req, plane, PLANE_CRTC_ID, plane->crtc_id);
-	plane_set_prop(req, plane, PLANE_FB_ID, plane->fb_id);
-	plane_set_prop(req, plane, PLANE_SRC_X, plane->src_x);
-	plane_set_prop(req, plane, PLANE_SRC_Y, plane->src_y);
-	plane_set_prop(req, plane, PLANE_SRC_W, plane->src_w);
-	plane_set_prop(req, plane, PLANE_SRC_H, plane->src_h);
-	plane_set_prop(req, plane, PLANE_CRTC_X, plane->crtc_x);
-	plane_set_prop(req, plane, PLANE_CRTC_Y, plane->crtc_y);
-	plane_set_prop(req, plane, PLANE_CRTC_W, plane->crtc_w);
-	plane_set_prop(req, plane, PLANE_CRTC_H, plane->crtc_h);
+	plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id);
+	plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id);
+	plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x);
+	plane_set_prop(req, plane, IGT_PLANE_SRC_Y, plane->src_y);
+	plane_set_prop(req, plane, IGT_PLANE_SRC_W, plane->src_w);
+	plane_set_prop(req, plane, IGT_PLANE_SRC_H, plane->src_h);
+	plane_set_prop(req, plane, IGT_PLANE_CRTC_X, plane->crtc_x);
+	plane_set_prop(req, plane, IGT_PLANE_CRTC_Y, plane->crtc_y);
+	plane_set_prop(req, plane, IGT_PLANE_CRTC_W, plane->crtc_w);
+	plane_set_prop(req, plane, IGT_PLANE_CRTC_H, plane->crtc_h);
 }
 
 static void plane_get_current_state(struct kms_atomic_plane_state *plane)
@@ -373,27 +324,27 @@ static void plane_get_current_state(struct kms_atomic_plane_state *plane)
 	for (i = 0; i < props->count_props; i++) {
 		uint32_t *prop_ids = desc->props_plane;
 
-		if (props->props[i] == prop_ids[PLANE_CRTC_ID])
+		if (props->props[i] == prop_ids[IGT_PLANE_CRTC_ID])
 			plane->crtc_id = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_FB_ID])
+		else if (props->props[i] == prop_ids[IGT_PLANE_FB_ID])
 			plane->fb_id = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_CRTC_X])
+		else if (props->props[i] == prop_ids[IGT_PLANE_CRTC_X])
 			plane->crtc_x = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_CRTC_Y])
+		else if (props->props[i] == prop_ids[IGT_PLANE_CRTC_Y])
 			plane->crtc_y = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_CRTC_W])
+		else if (props->props[i] == prop_ids[IGT_PLANE_CRTC_W])
 			plane->crtc_w = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_CRTC_H])
+		else if (props->props[i] == prop_ids[IGT_PLANE_CRTC_H])
 			plane->crtc_h = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_SRC_X])
+		else if (props->props[i] == prop_ids[IGT_PLANE_SRC_X])
 			plane->src_x = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_SRC_Y])
+		else if (props->props[i] == prop_ids[IGT_PLANE_SRC_Y])
 			plane->src_y = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_SRC_W])
+		else if (props->props[i] == prop_ids[IGT_PLANE_SRC_W])
 			plane->src_w = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_SRC_H])
+		else if (props->props[i] == prop_ids[IGT_PLANE_SRC_H])
 			plane->src_h = props->prop_values[i];
-		else if (props->props[i] == prop_ids[PLANE_TYPE]) {
+		else if (props->props[i] == prop_ids[IGT_PLANE_TYPE]) {
 			int j;
 
 			for (j = 0; j < ARRAY_SIZE(desc->props_plane_type); j++) {
@@ -482,8 +433,8 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
 static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
 			      drmModeAtomicReq *req)
 {
-	crtc_set_prop(req, crtc, CRTC_MODE_ID, crtc->mode.id);
-	crtc_set_prop(req, crtc, CRTC_ACTIVE, crtc->active);
+	crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id);
+	crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
 }
 
 static void crtc_get_current_state(struct kms_atomic_crtc_state *crtc)
@@ -498,7 +449,7 @@ static void crtc_get_current_state(struct kms_atomic_crtc_state *crtc)
 	for (i = 0; i < props->count_props; i++) {
 		uint32_t *prop_ids = crtc->state->desc->props_crtc;
 
-		if (props->props[i] == prop_ids[CRTC_MODE_ID]) {
+		if (props->props[i] == prop_ids[IGT_CRTC_MODE_ID]) {
 			drmModePropertyBlobPtr blob;
 
 			crtc->mode.id = props->prop_values[i];
@@ -518,7 +469,7 @@ static void crtc_get_current_state(struct kms_atomic_crtc_state *crtc)
 				crtc->mode.data = blob->data;
 			crtc->mode.len = blob->length;
 		}
-		else if (props->props[i] == prop_ids[CRTC_ACTIVE]) {
+		else if (props->props[i] == prop_ids[IGT_CRTC_ACTIVE]) {
 			crtc->active = props->prop_values[i];
 		}
 	}
@@ -627,7 +578,7 @@ static void crtc_commit_legacy(struct kms_atomic_crtc_state *crtc,
 
 	for (i = 0; i < props->count_props; i++) {
 		if (props->props[i] !=
-		    crtc->state->desc->props_crtc[CRTC_MODE_ID])
+		    crtc->state->desc->props_crtc[IGT_CRTC_MODE_ID])
 			continue;
 		crtc->mode.id = props->prop_values[i];
 		break;
@@ -756,20 +707,20 @@ static void atomic_setup(struct kms_atomic_state *state)
 	igt_assert(state->connectors);
 
 	fill_obj_props(desc->fd, res->crtcs[0],
-		       DRM_MODE_OBJECT_CRTC, NUM_CRTC_PROPS,
-		       crtc_prop_names, desc->props_crtc);
+		       DRM_MODE_OBJECT_CRTC, IGT_NUM_CRTC_PROPS,
+		       igt_crtc_prop_names, desc->props_crtc);
 
 	fill_obj_props(desc->fd, res_plane->planes[0],
-		       DRM_MODE_OBJECT_PLANE, NUM_PLANE_PROPS,
-		       plane_prop_names, desc->props_plane);
+		       DRM_MODE_OBJECT_PLANE, IGT_NUM_PLANE_PROPS,
+		       igt_plane_prop_names, desc->props_plane);
 	fill_obj_prop_map(desc->fd, res_plane->planes[0],
 			  DRM_MODE_OBJECT_PLANE, "type",
 			  NUM_PLANE_TYPE_PROPS, plane_type_prop_names,
 			  desc->props_plane_type);
 
 	fill_obj_props(desc->fd, res->connectors[0],
-		       DRM_MODE_OBJECT_CONNECTOR, NUM_CONNECTOR_PROPS,
-		       connector_prop_names, desc->props_connector);
+		       DRM_MODE_OBJECT_CONNECTOR, IGT_NUM_CONNECTOR_PROPS,
+		       igt_connector_prop_names, desc->props_connector);
 
 	for (i = 0; i < state->num_crtcs; i++) {
 		struct kms_atomic_crtc_state *crtc = &state->crtcs[i];
@@ -1245,7 +1196,7 @@ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
 
 	/* Valid property, valid value. */
 	for (i = 0; i < ARRAY_SIZE(props_raw); i++) {
-		props_raw[i] = desc->props_crtc[CRTC_MODE_ID];
+		props_raw[i] = desc->props_crtc[IGT_CRTC_MODE_ID];
 		values_raw[i] = crtc->mode.id;
 	}
 	do_ioctl(desc->fd, DRM_IOCTL_MODE_ATOMIC, &ioc);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 05/11] lib/igt_kms: Added igt_pipe_get_last_out_fence()

[Robert Foss]

Added the igt_pipe_get_last_out_fence() helper function
that wraps accesses to pipe->fence_out.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c | 8 ++++++++
 lib/igt_kms.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 142658a6..f0e38b75 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1934,6 +1934,14 @@ static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe)
 	return NULL;
 }
 
+int igt_pipe_get_last_out_fence(igt_pipe_t *pipe)
+{
+	int fd = (int) pipe->out_fence;
+	pipe->out_fence = -1;
+
+	return fd;
+}
+
 bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
 			   uint32_t *prop_id, uint64_t *value,
 			   drmModePropertyPtr *prop)
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 00e0dc68..94ff27bb 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -382,6 +382,7 @@ 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);
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
+int igt_pipe_get_last_out_fence(igt_pipe_t *pipe);
 bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
 			   uint32_t *prop_id, uint64_t *value,
 			   drmModePropertyPtr *prop);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 05/11] lib/igt_kms: Added igt_pipe_get_last_out_fence()

[Brian Starkey]

On Mon, Jan 30, 2017 at 08:58:41PM -0500, Robert Foss wrote:
>Added the igt_pipe_get_last_out_fence() helper function
>that wraps accesses to pipe->fence_out.
>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> lib/igt_kms.c | 8 ++++++++
> lib/igt_kms.h | 1 +
> 2 files changed, 9 insertions(+)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index 142658a6..f0e38b75 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -1934,6 +1934,14 @@ static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe)
> 	return NULL;
> }
>
>+int igt_pipe_get_last_out_fence(igt_pipe_t *pipe)
>+{
>+	int fd = (int) pipe->out_fence;
>+	pipe->out_fence = -1;
>+
>+	return fd;

If this wasn't the compile error you already found, then "out_fence"
doesn't seem to have been added to igt_pipe_t yet.

-Brian

>+}
>+
> bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
> 			   uint32_t *prop_id, uint64_t *value,
> 			   drmModePropertyPtr *prop)
>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>index 00e0dc68..94ff27bb 100644
>--- a/lib/igt_kms.h
>+++ b/lib/igt_kms.h
>@@ -382,6 +382,7 @@ 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);
> igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
>+int igt_pipe_get_last_out_fence(igt_pipe_t *pipe);
> bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
> 			   uint32_t *prop_id, uint64_t *value,
> 			   drmModePropertyPtr *prop);
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 06/11] lib/igt_kms: Add support for the IN_FENCE_FD property

[Robert Foss]

Add support dor the IN_FENCE_FD property to enable setting in fences for atomic
commits.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c | 20 ++++++++++++++++++++
 lib/igt_kms.h |  5 +++++
 2 files changed, 25 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f0e38b75..b79d2867 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -168,6 +168,7 @@ const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	"CRTC_H",
 	"FB_ID",
 	"CRTC_ID",
+	"IN_FENCE_FD",
 	"type",
 	"rotation"
 };
@@ -1667,6 +1668,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
 			plane->type = type;
 			plane->pipe = pipe;
 			plane->drm_plane = drm_plane;
+			plane->fence_fd = -1;
 
 			if (is_atomic == 0) {
 				display->is_atomic = 1;
@@ -2002,6 +2004,11 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
 	    plane->index,
 	    fb_id);
 
+	if (plane->fence_fd >= 0) {
+		uint64_t fence_fd = (int64_t) plane->fence_fd;
+		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd);
+	}
+
 	if (plane->fb_changed) {
 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
@@ -2823,6 +2830,19 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
 	plane->size_changed = true;
 }
 
+/**
+ * igt_plane_set_fence_fd:
+ * @plane: plane
+ * @fence_fd: fence fd, disable fence_fd by setting it to -1
+ *
+ * This function sets a fence fd to enable a commit to wait for some event to
+ * occur before completing.
+ */
+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd)
+{
+	plane->fence_fd = fence_fd;
+}
+
 void igt_plane_set_position(igt_plane_t *plane, int x, int y)
 {
 	igt_pipe_t *pipe = plane->pipe;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 94ff27bb..85688853 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -248,6 +248,7 @@ enum igt_atomic_plane_properties {
 
        IGT_PLANE_FB_ID,
        IGT_PLANE_CRTC_ID,
+       IGT_PLANE_IN_FENCE_FD,
        IGT_PLANE_TYPE,
        IGT_PLANE_ROTATION,
        IGT_NUM_PLANE_PROPS
@@ -306,6 +307,9 @@ typedef struct {
 	uint32_t src_h;
 
 	igt_rotation_t rotation;
+
+	/* in fence fd */
+	int32_t fence_fd;
 	uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
 } igt_plane_t;
 
@@ -397,6 +401,7 @@ void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
 void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
 
 void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd);
 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);
 void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 06/11] lib/igt_kms: Add support for the IN_FENCE_FD property

[Brian Starkey]

On Mon, Jan 30, 2017 at 08:58:42PM -0500, Robert Foss wrote:
>Add support dor the IN_FENCE_FD property to enable setting in fences for atomic
>commits.
>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> lib/igt_kms.c | 20 ++++++++++++++++++++
> lib/igt_kms.h |  5 +++++
> 2 files changed, 25 insertions(+)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index f0e38b75..b79d2867 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -168,6 +168,7 @@ const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> 	"CRTC_H",
> 	"FB_ID",
> 	"CRTC_ID",
>+	"IN_FENCE_FD",
> 	"type",
> 	"rotation"
> };
>@@ -1667,6 +1668,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> 			plane->type = type;
> 			plane->pipe = pipe;
> 			plane->drm_plane = drm_plane;
>+			plane->fence_fd = -1;
>
> 			if (is_atomic == 0) {
> 				display->is_atomic = 1;
>@@ -2002,6 +2004,11 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
> 	    plane->index,
> 	    fb_id);
>
>+	if (plane->fence_fd >= 0) {
>+		uint64_t fence_fd = (int64_t) plane->fence_fd;
>+		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd);
>+	}
>+
> 	if (plane->fb_changed) {
> 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
> 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
>@@ -2823,6 +2830,19 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
> 	plane->size_changed = true;
> }
>
>+/**
>+ * igt_plane_set_fence_fd:
>+ * @plane: plane
>+ * @fence_fd: fence fd, disable fence_fd by setting it to -1
>+ *
>+ * This function sets a fence fd to enable a commit to wait for some event to
>+ * occur before completing.
>+ */
>+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd)

nitpick, but should that be "int fence_fd" ?

Also have some comments later in the series around the fd ownership.
If nothing else, the comment here could say what's expected.

-Brian

>+{
>+	plane->fence_fd = fence_fd;
>+}
>+
> void igt_plane_set_position(igt_plane_t *plane, int x, int y)
> {
> 	igt_pipe_t *pipe = plane->pipe;
>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>index 94ff27bb..85688853 100644
>--- a/lib/igt_kms.h
>+++ b/lib/igt_kms.h
>@@ -248,6 +248,7 @@ enum igt_atomic_plane_properties {
>
>        IGT_PLANE_FB_ID,
>        IGT_PLANE_CRTC_ID,
>+       IGT_PLANE_IN_FENCE_FD,
>        IGT_PLANE_TYPE,
>        IGT_PLANE_ROTATION,
>        IGT_NUM_PLANE_PROPS
>@@ -306,6 +307,9 @@ typedef struct {
> 	uint32_t src_h;
>
> 	igt_rotation_t rotation;
>+
>+	/* in fence fd */
>+	int32_t fence_fd;
> 	uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
>@@ -397,6 +401,7 @@ void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
> void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
>
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>+void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd);
> 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);
> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 07/11] lib/igt_kms: Add support for the OUT_FENCE_PTR property

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Add support for the OUT_FENCE_PTR property to enable setting out fences for
atomic commits.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c | 26 +++++++++++++++++++++++++-
 lib/igt_kms.h |  6 +++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index b79d2867..f14496dd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -179,7 +179,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
 	"DEGAMMA_LUT",
 	"GAMMA_LUT",
 	"MODE_ID",
-	"ACTIVE"
+	"ACTIVE",
+	"OUT_FENCE_PTR"
 };
 
 const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
@@ -2393,6 +2394,15 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
 		igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE, !!output);
 	}
 
+	pipe_obj->out_fence = -1;
+	if (pipe_obj->out_fence_requested)
+	{
+		pipe_obj->out_fence_requested = false;
+		igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_OUT_FENCE_PTR,
+		    (uint64_t)(uintptr_t) &pipe_obj->out_fence);
+		igt_assert_f(pipe_obj->out_fence != -1, "Unable to get fence, received -1 fd\n");
+	}
+
 	/*
 	 *	TODO: Add all crtc level properties here
 	 */
@@ -2984,6 +2994,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 }
 
 /**
+ * igt_pipe_set_out_fence_ptr:
+ * @pipe: pipe pointer to which background color to be set
+ * @fence_ptr: out fence pointer
+ *
+ * Sets the out fence pointer that will be passed to the kernel in
+ * the atomic ioctl. When the kernel returns the out fence pointer
+ * will contain the fd number of the out fence created by KMS.
+ */
+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
+{
+	pipe->out_fence_ptr = fence_ptr;
+}
+
+/**
  * igt_crtc_set_background:
  * @pipe: pipe pointer to which background color to be set
  * @background: background color value in BGR 16bpc
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 85688853..9672dadc 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -94,6 +94,7 @@ enum igt_atomic_crtc_properties {
        IGT_CRTC_GAMMA_LUT,
        IGT_CRTC_MODE_ID,
        IGT_CRTC_ACTIVE,
+       IGT_CRTC_OUT_FENCE_PTR,
        IGT_NUM_CRTC_PROPS
 };
 
@@ -341,6 +342,9 @@ struct igt_pipe {
 
 	uint64_t mode_blob;
 	bool mode_changed;
+
+	int64_t out_fence;
+	bool out_fence_requested;
 };
 
 typedef struct {
@@ -395,7 +399,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
 {
 	return plane->rotation_property != 0;
 }
-
+void igt_pipe_request_out_fence(igt_pipe_t *pipe);
 void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
 void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
 void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 07/11] lib/igt_kms: Add support for the OUT_FENCE_PTR property

[Brian Starkey]

On Mon, Jan 30, 2017 at 08:58:43PM -0500, Robert Foss wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>Add support for the OUT_FENCE_PTR property to enable setting out fences for
>atomic commits.
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> lib/igt_kms.c | 26 +++++++++++++++++++++++++-
> lib/igt_kms.h |  6 +++++-
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index b79d2867..f14496dd 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -179,7 +179,8 @@ const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> 	"DEGAMMA_LUT",
> 	"GAMMA_LUT",
> 	"MODE_ID",
>-	"ACTIVE"
>+	"ACTIVE",
>+	"OUT_FENCE_PTR"
> };
>
> const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>@@ -2393,6 +2394,15 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
> 		igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE, !!output);
> 	}
>
>+	pipe_obj->out_fence = -1;
>+	if (pipe_obj->out_fence_requested)
>+	{
>+		pipe_obj->out_fence_requested = false;
>+		igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_OUT_FENCE_PTR,
>+		    (uint64_t)(uintptr_t) &pipe_obj->out_fence);
>+		igt_assert_f(pipe_obj->out_fence != -1, "Unable to get fence, received -1 fd\n");

Doesn't this assertion always fail? You just set it to -1

>+	}
>+
> 	/*
> 	 *	TODO: Add all crtc level properties here
> 	 */
>@@ -2984,6 +2994,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> }
>
> /**
>+ * igt_pipe_set_out_fence_ptr:
>+ * @pipe: pipe pointer to which background color to be set
>+ * @fence_ptr: out fence pointer
>+ *
>+ * Sets the out fence pointer that will be passed to the kernel in
>+ * the atomic ioctl. When the kernel returns the out fence pointer
>+ * will contain the fd number of the out fence created by KMS.
>+ */
>+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
>+{
>+	pipe->out_fence_ptr = fence_ptr;
>+}

Is this ever used? You seem to use &pipe_obj->out_fence
unconditionally above (and igt_pipe has no member named
out_fence_ptr).

I guess is left over from the previous API and needs to be removed?

>+
>+/**
>  * igt_crtc_set_background:
>  * @pipe: pipe pointer to which background color to be set
>  * @background: background color value in BGR 16bpc
>diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>index 85688853..9672dadc 100644
>--- a/lib/igt_kms.h
>+++ b/lib/igt_kms.h
>@@ -94,6 +94,7 @@ enum igt_atomic_crtc_properties {
>        IGT_CRTC_GAMMA_LUT,
>        IGT_CRTC_MODE_ID,
>        IGT_CRTC_ACTIVE,
>+       IGT_CRTC_OUT_FENCE_PTR,
>        IGT_NUM_CRTC_PROPS
> };
>
>@@ -341,6 +342,9 @@ struct igt_pipe {
>
> 	uint64_t mode_blob;
> 	bool mode_changed;
>+
>+	int64_t out_fence;
>+	bool out_fence_requested;
> };
>
> typedef struct {
>@@ -395,7 +399,7 @@ static inline bool igt_plane_supports_rotation(igt_plane_t *plane)
> {
> 	return plane->rotation_property != 0;
> }
>-
>+void igt_pipe_request_out_fence(igt_pipe_t *pipe);

Not implemented? I think this patch got confused somewhere :-(

-Brian

> void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
> void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
> void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 07/11] lib/igt_kms: Add support for the OUT_FENCE_PTR property

[Robert Foss]

On 2017-01-31 11:49 AM, Brian Starkey wrote:
> On Mon, Jan 30, 2017 at 08:58:43PM -0500, Robert Foss wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> Add support for the OUT_FENCE_PTR property to enable setting out
>> fences for
>> atomic commits.
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>> lib/igt_kms.c | 26 +++++++++++++++++++++++++-
>> lib/igt_kms.h |  6 +++++-
>> 2 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index b79d2867..f14496dd 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -179,7 +179,8 @@ const char
>> *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>>     "DEGAMMA_LUT",
>>     "GAMMA_LUT",
>>     "MODE_ID",
>> -    "ACTIVE"
>> +    "ACTIVE",
>> +    "OUT_FENCE_PTR"
>> };
>>
>> const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> @@ -2393,6 +2394,15 @@ static void
>> igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
>>         igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
>> !!output);
>>     }
>>
>> +    pipe_obj->out_fence = -1;
>> +    if (pipe_obj->out_fence_requested)
>> +    {
>> +        pipe_obj->out_fence_requested = false;
>> +        igt_atomic_populate_crtc_req(req, pipe_obj,
>> IGT_CRTC_OUT_FENCE_PTR,
>> +            (uint64_t)(uintptr_t) &pipe_obj->out_fence);
>> +        igt_assert_f(pipe_obj->out_fence != -1, "Unable to get fence,
>> received -1 fd\n");
>
> Doesn't this assertion always fail? You just set it to -1

Ack, fixed in v4

>> +    }
>> +
>>     /*
>>      *    TODO: Add all crtc level properties here
>>      */
>> @@ -2984,6 +2994,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
>> *ptr, size_t length)
>> }
>>
>> /**
>> + * igt_pipe_set_out_fence_ptr:
>> + * @pipe: pipe pointer to which background color to be set
>> + * @fence_ptr: out fence pointer
>> + *
>> + * Sets the out fence pointer that will be passed to the kernel in
>> + * the atomic ioctl. When the kernel returns the out fence pointer
>> + * will contain the fd number of the out fence created by KMS.
>> + */
>> +void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
>> +{
>> +    pipe->out_fence_ptr = fence_ptr;
>> +}
>
> Is this ever used? You seem to use &pipe_obj->out_fence
> unconditionally above (and igt_pipe has no member named
> out_fence_ptr).
>
> I guess is left over from the previous API and needs to be removed?

That's exactly what happened. Removed in v4.

>
>> +
>> +/**
>>  * igt_crtc_set_background:
>>  * @pipe: pipe pointer to which background color to be set
>>  * @background: background color value in BGR 16bpc
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 85688853..9672dadc 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -94,6 +94,7 @@ enum igt_atomic_crtc_properties {
>>        IGT_CRTC_GAMMA_LUT,
>>        IGT_CRTC_MODE_ID,
>>        IGT_CRTC_ACTIVE,
>> +       IGT_CRTC_OUT_FENCE_PTR,
>>        IGT_NUM_CRTC_PROPS
>> };
>>
>> @@ -341,6 +342,9 @@ struct igt_pipe {
>>
>>     uint64_t mode_blob;
>>     bool mode_changed;
>> +
>> +    int64_t out_fence;
>> +    bool out_fence_requested;
>> };
>>
>> typedef struct {
>> @@ -395,7 +399,7 @@ static inline bool
>> igt_plane_supports_rotation(igt_plane_t *plane)
>> {
>>     return plane->rotation_property != 0;
>> }
>> -
>> +void igt_pipe_request_out_fence(igt_pipe_t *pipe);
>
> Not implemented? I think this patch got confused somewhere :-(
>
> -Brian

Yes, some code has been moved around and this piece was not properly
moved. Fixed in v4.

Rob.
>
>> void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t
>> length);
>> void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
>> void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
>> --
>> 2.11.0.453.g787f75f05
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 08/11] tests/kms_atomic: stress possible fence settings

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 tests/kms_atomic.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 177 insertions(+), 10 deletions(-)

diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index 8df51ccd..09605e38 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -44,6 +44,7 @@
 #include "drmtest.h"
 #include "igt.h"
 #include "igt_aux.h"
+#include "sw_sync.h"
 
 #ifndef DRM_CLIENT_CAP_ATOMIC
 #define DRM_CLIENT_CAP_ATOMIC 3
@@ -126,6 +127,7 @@ struct kms_atomic_plane_state {
 	uint32_t fb_id; /* 0 to disable */
 	uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */
 	uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */
+	int32_t fence_fd;
 };
 
 struct kms_atomic_crtc_state {
@@ -133,6 +135,7 @@ struct kms_atomic_crtc_state {
 	uint32_t obj;
 	int idx;
 	bool active;
+	uint64_t out_fence_ptr;
 	struct kms_atomic_blob mode;
 };
 
@@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
 	crtc_populate_req(crtc, req); \
 	plane_populate_req(plane, req); \
 	do_atomic_commit((crtc)->state->desc->fd, req, flags); \
-	crtc_check_current_state(crtc, plane, relax); \
-	plane_check_current_state(plane, relax); \
+	if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \
+		crtc_check_current_state(crtc, plane, relax); \
+		plane_check_current_state(plane, relax); \
+	} \
 }
 
-#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \
+#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags, relax, e) { \
 	drmModeAtomicSetCursor(req, 0); \
 	crtc_populate_req(crtc, req); \
 	plane_populate_req(plane, req); \
@@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state,
 static void plane_populate_req(struct kms_atomic_plane_state *plane,
 			       drmModeAtomicReq *req)
 {
+	if (plane->fence_fd)
+		plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, plane->fence_fd);
+
 	plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id);
 	plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id);
 	plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x);
@@ -433,6 +441,10 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
 static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
 			      drmModeAtomicReq *req)
 {
+	if (crtc->out_fence_ptr)
+		crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR,
+			      crtc->out_fence_ptr);
+
 	crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id);
 	crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
 }
@@ -1061,6 +1073,37 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
 	drmModeAtomicFree(req);
 }
 
+static void plane_invalid_params_fence(struct kms_atomic_crtc_state *crtc,
+				struct kms_atomic_plane_state *plane_old,
+				struct kms_atomic_connector_state *conn)
+{
+	struct kms_atomic_plane_state plane = *plane_old;
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	int timeline, fence_fd;
+
+	igt_require_sw_sync();
+
+	/* invalid fence fd */
+	plane.fence_fd = plane.state->desc->fd;
+	plane.crtc_id = plane_old->crtc_id;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	/* Valid fence_fd but invalid CRTC */
+	timeline = sw_sync_timeline_create();
+	fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
+	plane.fence_fd = fence_fd;
+	plane.crtc_id = ~0U;
+	plane_commit_atomic_err(&plane, plane_old, req,
+	                        ATOMIC_RELAX_NONE, EINVAL);
+
+	plane.fence_fd = -1;
+	close(fence_fd);
+	close(timeline);
+
+	drmModeAtomicFree(req);
+}
+
 static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
 				struct kms_atomic_plane_state *plane,
 				struct kms_atomic_connector_state *conn)
@@ -1072,30 +1115,32 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
 
 	/* Pass a series of invalid object IDs for the mode ID. */
 	crtc.mode.id = plane->obj;
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 	crtc.mode.id = crtc.obj;
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 	crtc.mode.id = conn->obj;
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 	crtc.mode.id = plane->fb_id;
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
+	/* successful TEST_ONLY with fences set */
 	crtc.mode.id = crtc_old->mode.id;
-	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
+			   DRM_MODE_ATOMIC_TEST_ONLY);
 
 	/* Create a blob which is the wrong size to be a valid mode. */
 	do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
 					    crtc.mode.data,
 					    sizeof(struct drm_mode_modeinfo) - 1,
 					    &crtc.mode.id));
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
 
@@ -1103,15 +1148,108 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
 					    crtc.mode.data,
 					    sizeof(struct drm_mode_modeinfo) + 1,
 					    &crtc.mode.id));
-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
 	                       ATOMIC_RELAX_NONE, EINVAL);
 
+
 	/* Restore the CRTC and check the state matches the old. */
 	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
 
 	drmModeAtomicFree(req);
 }
 
+static void crtc_invalid_params_fence(struct kms_atomic_crtc_state *crtc_old,
+				struct kms_atomic_plane_state *plane,
+				struct kms_atomic_connector_state *conn)
+{
+	struct kms_atomic_crtc_state crtc = *crtc_old;
+	drmModeAtomicReq *req = drmModeAtomicAlloc();
+	int timeline, fence_fd, *out_fence;
+
+	igt_require_sw_sync();
+
+	/* invalid out_fence_ptr */
+	crtc.mode.id = crtc_old->mode.id;
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
+	                       ATOMIC_RELAX_NONE, EFAULT);
+
+	/* invalid out_fence_ptr */
+	crtc.mode.id = crtc_old->mode.id;
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
+	                       ATOMIC_RELAX_NONE, EFAULT);
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
+
+	/* valid in fence but not allowed prop on crtc */
+	timeline = sw_sync_timeline_create();
+	fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
+	plane->fence_fd = fence_fd;
+	crtc.active = false;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	out_fence = malloc(sizeof(uint64_t));
+	igt_assert(out_fence);
+
+
+	/* valid out fence ptr and flip event but not allowed prop on crtc */
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* valid out fence ptr and flip event but not allowed prop on crtc */
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+			       DRM_MODE_PAGE_FLIP_EVENT,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* valid page flip event but not allowed prop on crtc */
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+			       DRM_MODE_PAGE_FLIP_EVENT,
+			       ATOMIC_RELAX_NONE, EINVAL);
+	crtc.active = true;
+
+	/* valid out fence  ptr and flip event but invalid prop on crtc */
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
+	crtc.mode.id = plane->fb_id;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* valid out fence ptr and flip event but invalid prop on crtc */
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+			       DRM_MODE_PAGE_FLIP_EVENT,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* valid page flip event but invalid prop on crtc */
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
+			       DRM_MODE_PAGE_FLIP_EVENT,
+			       ATOMIC_RELAX_NONE, EINVAL);
+
+	/* successful TEST_ONLY with fences set */
+	plane->fence_fd = fence_fd;
+	crtc.mode.id = crtc_old->mode.id;
+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
+			   DRM_MODE_ATOMIC_TEST_ONLY);
+	igt_assert(*out_fence == -1);
+	close(fence_fd);
+	close(timeline);
+
+	/* reset fences */
+	plane->fence_fd = -1;
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
+
+	/* out fence ptr but not page flip event */
+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
+	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
+
+	close(*out_fence);
+	free(out_fence);
+	drmModeAtomicFree(req);
+}
+
 /* Abuse the atomic ioctl directly in order to test various invalid conditions,
  * which the libdrm wrapper won't allow us to create. */
 static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
@@ -1315,6 +1453,20 @@ igt_main
 		atomic_state_free(scratch);
 	}
 
+	igt_subtest("plane_invalid_params_fence") {
+		struct kms_atomic_state *scratch = atomic_state_dup(current);
+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(current, PLANE_TYPE_PRIMARY, crtc);
+		struct kms_atomic_connector_state *conn =
+			find_connector(scratch, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		plane_invalid_params_fence(crtc, plane, conn);
+		atomic_state_free(scratch);
+	}
+
 	igt_subtest("crtc_invalid_params") {
 		struct kms_atomic_state *scratch = atomic_state_dup(current);
 		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
@@ -1330,6 +1482,21 @@ igt_main
 		atomic_state_free(scratch);
 	}
 
+	igt_subtest("crtc_invalid_params_fence") {
+		struct kms_atomic_state *scratch = atomic_state_dup(current);
+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
+		struct kms_atomic_plane_state *plane =
+			find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc);
+		struct kms_atomic_connector_state *conn =
+			find_connector(scratch, crtc);
+
+		igt_require(crtc);
+		igt_require(plane);
+		igt_require(conn);
+		crtc_invalid_params_fence(crtc, plane, conn);
+		atomic_state_free(scratch);
+	}
+
 	igt_subtest("atomic_invalid_params") {
 		struct kms_atomic_state *scratch = atomic_state_dup(current);
 		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 08/11] tests/kms_atomic: stress possible fence settings

[Brian Starkey]

This one lgtm, just need to swap all the uint64_t out_fence_ptrs for
int32_t.

-Brian

On Mon, Jan 30, 2017 at 08:58:44PM -0500, Robert Foss wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> tests/kms_atomic.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 177 insertions(+), 10 deletions(-)
>
>diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
>index 8df51ccd..09605e38 100644
>--- a/tests/kms_atomic.c
>+++ b/tests/kms_atomic.c
>@@ -44,6 +44,7 @@
> #include "drmtest.h"
> #include "igt.h"
> #include "igt_aux.h"
>+#include "sw_sync.h"
>
> #ifndef DRM_CLIENT_CAP_ATOMIC
> #define DRM_CLIENT_CAP_ATOMIC 3
>@@ -126,6 +127,7 @@ struct kms_atomic_plane_state {
> 	uint32_t fb_id; /* 0 to disable */
> 	uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */
> 	uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */
>+	int32_t fence_fd;
> };
>
> struct kms_atomic_crtc_state {
>@@ -133,6 +135,7 @@ struct kms_atomic_crtc_state {
> 	uint32_t obj;
> 	int idx;
> 	bool active;
>+	uint64_t out_fence_ptr;
> 	struct kms_atomic_blob mode;
> };
>
>@@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig)
> 	crtc_populate_req(crtc, req); \
> 	plane_populate_req(plane, req); \
> 	do_atomic_commit((crtc)->state->desc->fd, req, flags); \
>-	crtc_check_current_state(crtc, plane, relax); \
>-	plane_check_current_state(plane, relax); \
>+	if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \
>+		crtc_check_current_state(crtc, plane, relax); \
>+		plane_check_current_state(plane, relax); \
>+	} \
> }
>
>-#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \
>+#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags, relax, e) { \
> 	drmModeAtomicSetCursor(req, 0); \
> 	crtc_populate_req(crtc, req); \
> 	plane_populate_req(plane, req); \
>@@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state,
> static void plane_populate_req(struct kms_atomic_plane_state *plane,
> 			       drmModeAtomicReq *req)
> {
>+	if (plane->fence_fd)
>+		plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, plane->fence_fd);
>+
> 	plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id);
> 	plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id);
> 	plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x);
>@@ -433,6 +441,10 @@ find_plane(struct kms_atomic_state *state, enum plane_type type,
> static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
> 			      drmModeAtomicReq *req)
> {
>+	if (crtc->out_fence_ptr)
>+		crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR,
>+			      crtc->out_fence_ptr);
>+
> 	crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id);
> 	crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
> }
>@@ -1061,6 +1073,37 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc,
> 	drmModeAtomicFree(req);
> }
>
>+static void plane_invalid_params_fence(struct kms_atomic_crtc_state *crtc,
>+				struct kms_atomic_plane_state *plane_old,
>+				struct kms_atomic_connector_state *conn)
>+{
>+	struct kms_atomic_plane_state plane = *plane_old;
>+	drmModeAtomicReq *req = drmModeAtomicAlloc();
>+	int timeline, fence_fd;
>+
>+	igt_require_sw_sync();
>+
>+	/* invalid fence fd */
>+	plane.fence_fd = plane.state->desc->fd;
>+	plane.crtc_id = plane_old->crtc_id;
>+	plane_commit_atomic_err(&plane, plane_old, req,
>+	                        ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* Valid fence_fd but invalid CRTC */
>+	timeline = sw_sync_timeline_create();
>+	fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
>+	plane.fence_fd = fence_fd;
>+	plane.crtc_id = ~0U;
>+	plane_commit_atomic_err(&plane, plane_old, req,
>+	                        ATOMIC_RELAX_NONE, EINVAL);
>+
>+	plane.fence_fd = -1;
>+	close(fence_fd);
>+	close(timeline);
>+
>+	drmModeAtomicFree(req);
>+}
>+
> static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
> 				struct kms_atomic_plane_state *plane,
> 				struct kms_atomic_connector_state *conn)
>@@ -1072,30 +1115,32 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
>
> 	/* Pass a series of invalid object IDs for the mode ID. */
> 	crtc.mode.id = plane->obj;
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
> 	crtc.mode.id = crtc.obj;
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
> 	crtc.mode.id = conn->obj;
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
> 	crtc.mode.id = plane->fb_id;
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
>+	/* successful TEST_ONLY with fences set */
> 	crtc.mode.id = crtc_old->mode.id;
>-	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
>+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
>+			   DRM_MODE_ATOMIC_TEST_ONLY);
>
> 	/* Create a blob which is the wrong size to be a valid mode. */
> 	do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
> 					    crtc.mode.data,
> 					    sizeof(struct drm_mode_modeinfo) - 1,
> 					    &crtc.mode.id));
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
>
>@@ -1103,15 +1148,108 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
> 					    crtc.mode.data,
> 					    sizeof(struct drm_mode_modeinfo) + 1,
> 					    &crtc.mode.id));
>-	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
> 	                       ATOMIC_RELAX_NONE, EINVAL);
>
>+
> 	/* Restore the CRTC and check the state matches the old. */
> 	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
>
> 	drmModeAtomicFree(req);
> }
>
>+static void crtc_invalid_params_fence(struct kms_atomic_crtc_state *crtc_old,
>+				struct kms_atomic_plane_state *plane,
>+				struct kms_atomic_connector_state *conn)
>+{
>+	struct kms_atomic_crtc_state crtc = *crtc_old;
>+	drmModeAtomicReq *req = drmModeAtomicAlloc();
>+	int timeline, fence_fd, *out_fence;
>+
>+	igt_require_sw_sync();
>+
>+	/* invalid out_fence_ptr */
>+	crtc.mode.id = crtc_old->mode.id;
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>+	                       ATOMIC_RELAX_NONE, EFAULT);
>+
>+	/* invalid out_fence_ptr */
>+	crtc.mode.id = crtc_old->mode.id;
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>+	                       ATOMIC_RELAX_NONE, EFAULT);
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>+
>+	/* valid in fence but not allowed prop on crtc */
>+	timeline = sw_sync_timeline_create();
>+	fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
>+	plane->fence_fd = fence_fd;
>+	crtc.active = false;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	out_fence = malloc(sizeof(uint64_t));
>+	igt_assert(out_fence);
>+
>+
>+	/* valid out fence ptr and flip event but not allowed prop on crtc */
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* valid out fence ptr and flip event but not allowed prop on crtc */
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+			       DRM_MODE_PAGE_FLIP_EVENT,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* valid page flip event but not allowed prop on crtc */
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+			       DRM_MODE_PAGE_FLIP_EVENT,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+	crtc.active = true;
>+
>+	/* valid out fence  ptr and flip event but invalid prop on crtc */
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>+	crtc.mode.id = plane->fb_id;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* valid out fence ptr and flip event but invalid prop on crtc */
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+			       DRM_MODE_PAGE_FLIP_EVENT,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* valid page flip event but invalid prop on crtc */
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>+	crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>+			       DRM_MODE_PAGE_FLIP_EVENT,
>+			       ATOMIC_RELAX_NONE, EINVAL);
>+
>+	/* successful TEST_ONLY with fences set */
>+	plane->fence_fd = fence_fd;
>+	crtc.mode.id = crtc_old->mode.id;
>+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
>+			   DRM_MODE_ATOMIC_TEST_ONLY);
>+	igt_assert(*out_fence == -1);
>+	close(fence_fd);
>+	close(timeline);
>+
>+	/* reset fences */
>+	plane->fence_fd = -1;
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>+	crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
>+
>+	/* out fence ptr but not page flip event */
>+	crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>+	crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
>+
>+	close(*out_fence);
>+	free(out_fence);
>+	drmModeAtomicFree(req);
>+}
>+
> /* Abuse the atomic ioctl directly in order to test various invalid conditions,
>  * which the libdrm wrapper won't allow us to create. */
> static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
>@@ -1315,6 +1453,20 @@ igt_main
> 		atomic_state_free(scratch);
> 	}
>
>+	igt_subtest("plane_invalid_params_fence") {
>+		struct kms_atomic_state *scratch = atomic_state_dup(current);
>+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>+		struct kms_atomic_plane_state *plane =
>+			find_plane(current, PLANE_TYPE_PRIMARY, crtc);
>+		struct kms_atomic_connector_state *conn =
>+			find_connector(scratch, crtc);
>+
>+		igt_require(crtc);
>+		igt_require(plane);
>+		plane_invalid_params_fence(crtc, plane, conn);
>+		atomic_state_free(scratch);
>+	}
>+
> 	igt_subtest("crtc_invalid_params") {
> 		struct kms_atomic_state *scratch = atomic_state_dup(current);
> 		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>@@ -1330,6 +1482,21 @@ igt_main
> 		atomic_state_free(scratch);
> 	}
>
>+	igt_subtest("crtc_invalid_params_fence") {
>+		struct kms_atomic_state *scratch = atomic_state_dup(current);
>+		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>+		struct kms_atomic_plane_state *plane =
>+			find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc);
>+		struct kms_atomic_connector_state *conn =
>+			find_connector(scratch, crtc);
>+
>+		igt_require(crtc);
>+		igt_require(plane);
>+		igt_require(conn);
>+		crtc_invalid_params_fence(crtc, plane, conn);
>+		atomic_state_free(scratch);
>+	}
>+
> 	igt_subtest("atomic_invalid_params") {
> 		struct kms_atomic_state *scratch = atomic_state_dup(current);
> 		struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 08/11] tests/kms_atomic: stress possible fence settings

[Robert Foss]

On 2017-01-31 11:50 AM, Brian Starkey wrote:
> This one lgtm, just need to swap all the uint64_t out_fence_ptrs for
> int32_t.
>
> -Brian

Fixed in v4.

Rob.

>
> On Mon, Jan 30, 2017 at 08:58:44PM -0500, Robert Foss wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>> tests/kms_atomic.c | 187
>> ++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 177 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
>> index 8df51ccd..09605e38 100644
>> --- a/tests/kms_atomic.c
>> +++ b/tests/kms_atomic.c
>> @@ -44,6 +44,7 @@
>> #include "drmtest.h"
>> #include "igt.h"
>> #include "igt_aux.h"
>> +#include "sw_sync.h"
>>
>> #ifndef DRM_CLIENT_CAP_ATOMIC
>> #define DRM_CLIENT_CAP_ATOMIC 3
>> @@ -126,6 +127,7 @@ struct kms_atomic_plane_state {
>>     uint32_t fb_id; /* 0 to disable */
>>     uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */
>>     uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */
>> +    int32_t fence_fd;
>> };
>>
>> struct kms_atomic_crtc_state {
>> @@ -133,6 +135,7 @@ struct kms_atomic_crtc_state {
>>     uint32_t obj;
>>     int idx;
>>     bool active;
>> +    uint64_t out_fence_ptr;
>>     struct kms_atomic_blob mode;
>> };
>>
>> @@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t
>> id_orig)
>>     crtc_populate_req(crtc, req); \
>>     plane_populate_req(plane, req); \
>>     do_atomic_commit((crtc)->state->desc->fd, req, flags); \
>> -    crtc_check_current_state(crtc, plane, relax); \
>> -    plane_check_current_state(plane, relax); \
>> +    if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \
>> +        crtc_check_current_state(crtc, plane, relax); \
>> +        plane_check_current_state(plane, relax); \
>> +    } \
>> }
>>
>> -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req,
>> relax, e) { \
>> +#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req,
>> flags, relax, e) { \
>>     drmModeAtomicSetCursor(req, 0); \
>>     crtc_populate_req(crtc, req); \
>>     plane_populate_req(plane, req); \
>> @@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state,
>> static void plane_populate_req(struct kms_atomic_plane_state *plane,
>>                    drmModeAtomicReq *req)
>> {
>> +    if (plane->fence_fd)
>> +        plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD,
>> plane->fence_fd);
>> +
>>     plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id);
>>     plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id);
>>     plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x);
>> @@ -433,6 +441,10 @@ find_plane(struct kms_atomic_state *state, enum
>> plane_type type,
>> static void crtc_populate_req(struct kms_atomic_crtc_state *crtc,
>>                   drmModeAtomicReq *req)
>> {
>> +    if (crtc->out_fence_ptr)
>> +        crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR,
>> +                  crtc->out_fence_ptr);
>> +
>>     crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id);
>>     crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active);
>> }
>> @@ -1061,6 +1073,37 @@ static void plane_invalid_params(struct
>> kms_atomic_crtc_state *crtc,
>>     drmModeAtomicFree(req);
>> }
>>
>> +static void plane_invalid_params_fence(struct kms_atomic_crtc_state
>> *crtc,
>> +                struct kms_atomic_plane_state *plane_old,
>> +                struct kms_atomic_connector_state *conn)
>> +{
>> +    struct kms_atomic_plane_state plane = *plane_old;
>> +    drmModeAtomicReq *req = drmModeAtomicAlloc();
>> +    int timeline, fence_fd;
>> +
>> +    igt_require_sw_sync();
>> +
>> +    /* invalid fence fd */
>> +    plane.fence_fd = plane.state->desc->fd;
>> +    plane.crtc_id = plane_old->crtc_id;
>> +    plane_commit_atomic_err(&plane, plane_old, req,
>> +                            ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* Valid fence_fd but invalid CRTC */
>> +    timeline = sw_sync_timeline_create();
>> +    fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
>> +    plane.fence_fd = fence_fd;
>> +    plane.crtc_id = ~0U;
>> +    plane_commit_atomic_err(&plane, plane_old, req,
>> +                            ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    plane.fence_fd = -1;
>> +    close(fence_fd);
>> +    close(timeline);
>> +
>> +    drmModeAtomicFree(req);
>> +}
>> +
>> static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old,
>>                 struct kms_atomic_plane_state *plane,
>>                 struct kms_atomic_connector_state *conn)
>> @@ -1072,30 +1115,32 @@ static void crtc_invalid_params(struct
>> kms_atomic_crtc_state *crtc_old,
>>
>>     /* Pass a series of invalid object IDs for the mode ID. */
>>     crtc.mode.id = plane->obj;
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>>     crtc.mode.id = crtc.obj;
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>>     crtc.mode.id = conn->obj;
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>>     crtc.mode.id = plane->fb_id;
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>> +    /* successful TEST_ONLY with fences set */
>>     crtc.mode.id = crtc_old->mode.id;
>> -    crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
>> +    crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
>> +               DRM_MODE_ATOMIC_TEST_ONLY);
>>
>>     /* Create a blob which is the wrong size to be a valid mode. */
>>     do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd,
>>                         crtc.mode.data,
>>                         sizeof(struct drm_mode_modeinfo) - 1,
>>                         &crtc.mode.id));
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>>
>> @@ -1103,15 +1148,108 @@ static void crtc_invalid_params(struct
>> kms_atomic_crtc_state *crtc_old,
>>                         crtc.mode.data,
>>                         sizeof(struct drm_mode_modeinfo) + 1,
>>                         &crtc.mode.id));
>> -    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>>                            ATOMIC_RELAX_NONE, EINVAL);
>>
>> +
>>     /* Restore the CRTC and check the state matches the old. */
>>     crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
>>
>>     drmModeAtomicFree(req);
>> }
>>
>> +static void crtc_invalid_params_fence(struct kms_atomic_crtc_state
>> *crtc_old,
>> +                struct kms_atomic_plane_state *plane,
>> +                struct kms_atomic_connector_state *conn)
>> +{
>> +    struct kms_atomic_crtc_state crtc = *crtc_old;
>> +    drmModeAtomicReq *req = drmModeAtomicAlloc();
>> +    int timeline, fence_fd, *out_fence;
>> +
>> +    igt_require_sw_sync();
>> +
>> +    /* invalid out_fence_ptr */
>> +    crtc.mode.id = crtc_old->mode.id;
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>> +                           ATOMIC_RELAX_NONE, EFAULT);
>> +
>> +    /* invalid out_fence_ptr */
>> +    crtc.mode.id = crtc_old->mode.id;
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>> +                           ATOMIC_RELAX_NONE, EFAULT);
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>> +
>> +    /* valid in fence but not allowed prop on crtc */
>> +    timeline = sw_sync_timeline_create();
>> +    fence_fd =  sw_sync_timeline_create_fence(timeline, 1);
>> +    plane->fence_fd = fence_fd;
>> +    crtc.active = false;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    out_fence = malloc(sizeof(uint64_t));
>> +    igt_assert(out_fence);
>> +
>> +
>> +    /* valid out fence ptr and flip event but not allowed prop on
>> crtc */
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* valid out fence ptr and flip event but not allowed prop on
>> crtc */
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +                   DRM_MODE_PAGE_FLIP_EVENT,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* valid page flip event but not allowed prop on crtc */
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +                   DRM_MODE_PAGE_FLIP_EVENT,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +    crtc.active = true;
>> +
>> +    /* valid out fence  ptr and flip event but invalid prop on crtc */
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>> +    crtc.mode.id = plane->fb_id;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* valid out fence ptr and flip event but invalid prop on crtc */
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +                   DRM_MODE_PAGE_FLIP_EVENT,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* valid page flip event but invalid prop on crtc */
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>> +    crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req,
>> +                   DRM_MODE_PAGE_FLIP_EVENT,
>> +                   ATOMIC_RELAX_NONE, EINVAL);
>> +
>> +    /* successful TEST_ONLY with fences set */
>> +    plane->fence_fd = fence_fd;
>> +    crtc.mode.id = crtc_old->mode.id;
>> +    crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE,
>> +               DRM_MODE_ATOMIC_TEST_ONLY);
>> +    igt_assert(*out_fence == -1);
>> +    close(fence_fd);
>> +    close(timeline);
>> +
>> +    /* reset fences */
>> +    plane->fence_fd = -1;
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0;
>> +    crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0);
>> +
>> +    /* out fence ptr but not page flip event */
>> +    crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence;
>> +    crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0);
>> +
>> +    close(*out_fence);
>> +    free(out_fence);
>> +    drmModeAtomicFree(req);
>> +}
>> +
>> /* Abuse the atomic ioctl directly in order to test various invalid
>> conditions,
>>  * which the libdrm wrapper won't allow us to create. */
>> static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc,
>> @@ -1315,6 +1453,20 @@ igt_main
>>         atomic_state_free(scratch);
>>     }
>>
>> +    igt_subtest("plane_invalid_params_fence") {
>> +        struct kms_atomic_state *scratch = atomic_state_dup(current);
>> +        struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>> +        struct kms_atomic_plane_state *plane =
>> +            find_plane(current, PLANE_TYPE_PRIMARY, crtc);
>> +        struct kms_atomic_connector_state *conn =
>> +            find_connector(scratch, crtc);
>> +
>> +        igt_require(crtc);
>> +        igt_require(plane);
>> +        plane_invalid_params_fence(crtc, plane, conn);
>> +        atomic_state_free(scratch);
>> +    }
>> +
>>     igt_subtest("crtc_invalid_params") {
>>         struct kms_atomic_state *scratch = atomic_state_dup(current);
>>         struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>> @@ -1330,6 +1482,21 @@ igt_main
>>         atomic_state_free(scratch);
>>     }
>>
>> +    igt_subtest("crtc_invalid_params_fence") {
>> +        struct kms_atomic_state *scratch = atomic_state_dup(current);
>> +        struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>> +        struct kms_atomic_plane_state *plane =
>> +            find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc);
>> +        struct kms_atomic_connector_state *conn =
>> +            find_connector(scratch, crtc);
>> +
>> +        igt_require(crtc);
>> +        igt_require(plane);
>> +        igt_require(conn);
>> +        crtc_invalid_params_fence(crtc, plane, conn);
>> +        atomic_state_free(scratch);
>> +    }
>> +
>>     igt_subtest("atomic_invalid_params") {
>>         struct kms_atomic_state *scratch = atomic_state_dup(current);
>>         struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
>> --
>> 2.11.0.453.g787f75f05
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 09/11] tests/kms_atomic_transition: add fencing parameter to run_transition_tests

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 tests/kms_atomic_transition.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 095af515..72429759 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -264,7 +264,7 @@ retry:
  */
 static void
 run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output,
-		    enum transition_type type, bool nonblocking)
+		enum transition_type type, bool nonblocking, bool fencing)
 {
 	struct igt_fb fb, argb_fb, sprite_fb;
 	drmModeModeInfo *mode, override_mode;
@@ -674,19 +674,19 @@ igt_main
 
 	igt_subtest("plane-all-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output)
-			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false);
+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false);
 
 	igt_subtest("plane-all-transition-nonblocking")
 		for_each_pipe_with_valid_output(&display, pipe, output)
-			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true);
+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false);
 
 	igt_subtest("plane-all-modeset-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output)
-			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false);
+			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false);
 
 	igt_subtest("plane-toggle-modeset-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output)
-			run_transition_test(&display, pipe, output, TRANSITION_MODESET_DISABLE, false);
+			run_transition_test(&display, pipe, output, TRANSITION_MODESET_DISABLE, false, false);
 
 	for (i = 1; i <= I915_MAX_PIPES; i++) {
 		igt_subtest_f("%ix-modeset-transitions", i)
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 10/11] tests/kms_atomic_transition: add out_fences tests

[Robert Foss]

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c                 |  35 ++++++----
 tests/kms_atomic_transition.c | 148 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+), 14 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f14496dd..523f3f57 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -53,6 +53,7 @@
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
 #include "igt_sysfs.h"
+#include "sw_sync.h"
 
 /**
  * SECTION:igt_kms
@@ -2479,6 +2480,21 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
 	}
 
 	ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
+	if (!ret) {
+
+		for_each_pipe(display, pipe) {
+			igt_pipe_t *pipe_obj = &display->pipes[pipe];
+
+			if (pipe_obj->out_fence != -1)
+				continue;
+
+			igt_assert(pipe_obj->out_fence >= 0);
+			ret = sync_fence_wait(pipe_obj->out_fence, 1000);
+			igt_assert(ret == 0);
+			close(pipe_obj->out_fence);
+		}
+	}
+
 	drmModeAtomicFree(req);
 	return ret;
 
@@ -2972,6 +2988,11 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
 	plane->rotation_changed = true;
 }
 
+void igt_pipe_request_out_fence(igt_pipe_t *pipe)
+{
+	pipe->out_fence_requested = true;
+}
+
 void
 igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 {
@@ -2994,20 +3015,6 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
 }
 
 /**
- * igt_pipe_set_out_fence_ptr:
- * @pipe: pipe pointer to which background color to be set
- * @fence_ptr: out fence pointer
- *
- * Sets the out fence pointer that will be passed to the kernel in
- * the atomic ioctl. When the kernel returns the out fence pointer
- * will contain the fd number of the out fence created by KMS.
- */
-void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
-{
-	pipe->out_fence_ptr = fence_ptr;
-}
-
-/**
  * igt_crtc_set_background:
  * @pipe: pipe pointer to which background color to be set
  * @background: background color value in BGR 16bpc
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 72429759..eebb5d66 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -253,6 +253,93 @@ retry:
 		 sprite_width, sprite_height, alpha);
 }
 
+int *timeline;
+pthread_t *thread;
+int *seqno;
+
+static void prepare_fencing(igt_display_t *display, enum pipe pipe)
+{
+	igt_plane_t *plane;
+	int n_planes;
+
+	igt_require_sw_sync();
+
+	n_planes = display->pipes[pipe].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);
+        igt_assert_f(thread != NULL, "Failed to allocate memory for thread\n");
+        seqno = calloc(sizeof(*seqno), n_planes);
+        igt_assert_f(seqno != NULL, "Failed to allocate memory for seqno\n");
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		timeline[plane->index] = sw_sync_timeline_create();
+}
+
+static void unprepare_fencing(igt_display_t *display, enum pipe pipe)
+{
+	igt_plane_t *plane;
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		close(timeline[plane->index]);
+
+	free(timeline);
+	free(thread);
+	free(seqno);
+}
+
+static void *fence_inc_thread(void *arg)
+{
+	int t = *((int *) arg);
+
+	pthread_detach(pthread_self());
+
+	usleep(5000);
+	sw_sync_timeline_inc(t, 1);
+	return NULL;
+}
+
+static void configure_fencing(igt_display_t *display, enum pipe pipe)
+{
+	igt_plane_t *plane;
+	int i, fd, ret;
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+
+		if (!plane->fb)
+			continue;
+
+		i = plane->index;
+
+		seqno[i]++;
+		fd = sw_sync_timeline_create_fence(timeline[i], seqno[i]);
+		igt_plane_set_fence_fd(plane, fd);
+		ret = pthread_create(&thread[i], NULL, fence_inc_thread, &timeline[i]);
+		igt_assert_eq(ret, 0);
+	}
+}
+
+static void clear_fencing(igt_display_t *display, enum pipe pipe)
+{
+	igt_plane_t *plane;
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		igt_plane_set_fence_fd(plane, -1);
+}
+
+static void atomic_commit(igt_display_t *display, enum pipe pipe, unsigned int flags, void *data, bool fencing)
+{
+	if (fencing) {
+		configure_fencing(display, pipe);
+		igt_pipe_request_out_fence(&display->pipes[pipe]);
+	}
+
+	igt_display_commit_atomic(display, flags, data);
+
+	if (fencing)
+		clear_fencing(display, pipe);
+}
+
 /*
  * 1. Set primary plane to a known fb.
  * 2. Make sure getcrtc returns the correct fb id.
@@ -273,6 +360,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 	struct plane_parms parms[display->pipes[pipe].n_planes];
 	bool skip_test = false;
 	unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
+	int ret;
 
 	if (nonblocking)
 		flags |= DRM_MODE_ATOMIC_NONBLOCK;
@@ -307,11 +395,50 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 	setup_parms(display, pipe, mode, &argb_fb, &sprite_fb, parms);
 
+	/*
+	 * In some configurations the tests may not run to completion with all
+	 * sprite planes lit up at 4k resolution, try decreasing width/size of secondary
+	 * planes to fix this
+	 */
+	while (1) {
+		wm_setup_plane(display, pipe, iter_max - 1, parms);
+
+		if (fencing)
+			igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
+			    (int64_t *) &out_fence);
+		ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+		if (ret != -EINVAL || n_planes < 3)
+			break;
+
+		ret = 0;
+		for_each_plane_on_pipe(display, pipe, plane) {
+			i = plane->index;
+
+			if (plane->is_primary || plane->is_cursor)
+				continue;
+
+			if (parms[i].width <= 512)
+				continue;
+
+			parms[i].width /= 2;
+			ret = 1;
+			igt_info("Reducing sprite %i to %ux%u\n", i - 1, parms[i].width, parms[i].height);
+			break;
+		}
+
+		if (!ret)
+			igt_skip("Cannot run tests without proper size sprite planes\n");
+	}
+
 	for (i = 0; i < iter_max; i++) {
 		igt_output_set_pipe(output, pipe);
 
 		wm_setup_plane(display, pipe, i, parms);
 
+		if (fencing)
+			igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
+
 		igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
 		drmHandleEvent(display->drm_fd, &drm_events);
 
@@ -320,6 +447,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 			wm_setup_plane(display, pipe, 0, parms);
 
+			if (fencing)
+				igt_pipe_request_out_fence(&display->pipes[pipe]);
+
 			igt_display_commit_atomic(display, flags, (void *)0UL);
 
 			drmHandleEvent(display->drm_fd, &drm_events);
@@ -333,6 +463,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 				if (type == TRANSITION_MODESET)
 					igt_output_override_mode(output, &override_mode);
 
+				if (fencing)
+					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
+
 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)j);
 				drmHandleEvent(display->drm_fd, &drm_events);
 
@@ -340,6 +473,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 				if (type == TRANSITION_MODESET)
 					igt_output_override_mode(output, NULL);
 
+				if (fencing)
+					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
+
 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
 				drmHandleEvent(display->drm_fd, &drm_events);
 			}
@@ -676,14 +812,26 @@ igt_main
 		for_each_pipe_with_valid_output(&display, pipe, output)
 			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false);
 
+	igt_subtest("plane-all-transition-fencing")
+		for_each_pipe_with_valid_output(&display, pipe, output)
+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true);
+
 	igt_subtest("plane-all-transition-nonblocking")
 		for_each_pipe_with_valid_output(&display, pipe, output)
 			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false);
 
+	igt_subtest("plane-all-transition-nonblocking-fencing")
+		for_each_pipe_with_valid_output(&display, pipe, output)
+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true);
+
 	igt_subtest("plane-all-modeset-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output)
 			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false);
 
+	igt_subtest("plane-all-modeset-transition-fencing")
+		for_each_pipe_with_valid_output(&display, pipe, output)
+			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, true);
+
 	igt_subtest("plane-toggle-modeset-transition")
 		for_each_pipe_with_valid_output(&display, pipe, output)
 			run_transition_test(&display, pipe, output, TRANSITION_MODESET_DISABLE, false, false);
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 10/11] tests/kms_atomic_transition: add out_fences tests

[Brian Starkey]

On Mon, Jan 30, 2017 at 08:58:46PM -0500, Robert Foss wrote:
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> lib/igt_kms.c                 |  35 ++++++----
> tests/kms_atomic_transition.c | 148 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 169 insertions(+), 14 deletions(-)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index f14496dd..523f3f57 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -53,6 +53,7 @@
> #include "intel_chipset.h"
> #include "igt_debugfs.h"
> #include "igt_sysfs.h"
>+#include "sw_sync.h"
>
> /**
>  * SECTION:igt_kms
>@@ -2479,6 +2480,21 @@ static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_
> 	}
>
> 	ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
>+	if (!ret) {
>+
>+		for_each_pipe(display, pipe) {
>+			igt_pipe_t *pipe_obj = &display->pipes[pipe];
>+
>+			if (pipe_obj->out_fence != -1)
>+				continue;

Should be "if (pipe_obj->fence == -1)" ? The stuff below seems to be
assuming the fence is valid.

>+
>+			igt_assert(pipe_obj->out_fence >= 0);
>+			ret = sync_fence_wait(pipe_obj->out_fence, 1000);
>+			igt_assert(ret == 0);
>+			close(pipe_obj->out_fence);
>+		}
>+	}
>+
> 	drmModeAtomicFree(req);
> 	return ret;
>
>@@ -2972,6 +2988,11 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
> 	plane->rotation_changed = true;
> }
>
>+void igt_pipe_request_out_fence(igt_pipe_t *pipe)

Oh here it is!

>+{
>+	pipe->out_fence_requested = true;
>+}
>+
> void
> igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> {
>@@ -2994,20 +3015,6 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
> }
>
> /**
>- * igt_pipe_set_out_fence_ptr:
>- * @pipe: pipe pointer to which background color to be set
>- * @fence_ptr: out fence pointer
>- *
>- * Sets the out fence pointer that will be passed to the kernel in
>- * the atomic ioctl. When the kernel returns the out fence pointer
>- * will contain the fd number of the out fence created by KMS.
>- */
>-void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)

... and there this one goes. This deletion and the function above
need to be squashed into the earlier commit I suppose.

>-{
>-	pipe->out_fence_ptr = fence_ptr;
>-}
>-
>-/**
>  * igt_crtc_set_background:
>  * @pipe: pipe pointer to which background color to be set
>  * @background: background color value in BGR 16bpc
>diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>index 72429759..eebb5d66 100644
>--- a/tests/kms_atomic_transition.c
>+++ b/tests/kms_atomic_transition.c
>@@ -253,6 +253,93 @@ retry:
> 		 sprite_width, sprite_height, alpha);
> }
>
>+int *timeline;
>+pthread_t *thread;
>+int *seqno;
>+
>+static void prepare_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+	igt_plane_t *plane;
>+	int n_planes;
>+
>+	igt_require_sw_sync();
>+
>+	n_planes = display->pipes[pipe].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);
>+        igt_assert_f(thread != NULL, "Failed to allocate memory for thread\n");
>+        seqno = calloc(sizeof(*seqno), n_planes);
>+        igt_assert_f(seqno != NULL, "Failed to allocate memory for seqno\n");

The 6 lines above are space-indented

>+
>+	for_each_plane_on_pipe(display, pipe, plane)
>+		timeline[plane->index] = sw_sync_timeline_create();
>+}
>+
>+static void unprepare_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+	igt_plane_t *plane;
>+
>+	for_each_plane_on_pipe(display, pipe, plane)
>+		close(timeline[plane->index]);
>+
>+	free(timeline);
>+	free(thread);
>+	free(seqno);
>+}
>+
>+static void *fence_inc_thread(void *arg)
>+{
>+	int t = *((int *) arg);
>+
>+	pthread_detach(pthread_self());
>+
>+	usleep(5000);
>+	sw_sync_timeline_inc(t, 1);
>+	return NULL;
>+}
>+
>+static void configure_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+	igt_plane_t *plane;
>+	int i, fd, ret;
>+
>+	for_each_plane_on_pipe(display, pipe, plane) {
>+
>+		if (!plane->fb)
>+			continue;
>+
>+		i = plane->index;
>+
>+		seqno[i]++;
>+		fd = sw_sync_timeline_create_fence(timeline[i], seqno[i]);
>+		igt_plane_set_fence_fd(plane, fd);
>+		ret = pthread_create(&thread[i], NULL, fence_inc_thread, &timeline[i]);
>+		igt_assert_eq(ret, 0);
>+	}
>+}
>+
>+static void clear_fencing(igt_display_t *display, enum pipe pipe)
>+{
>+	igt_plane_t *plane;
>+
>+	for_each_plane_on_pipe(display, pipe, plane)
>+		igt_plane_set_fence_fd(plane, -1);

Someone should close the old fence_fd if it's not -1.

I think igt_plane_set_fence_fd() should dup() the passed in fence, and
igt_display_atomic_commit() should set it to -1 after the commit.

That makes it very obvious, and that way igt_plane_set_fence_fd() can
always safely close plane->fence_fd if it gets called while the
current fence_fd is valid, because it always owns that fd. Callers
have their own copy to take care of.

In any case, I think you're leaking fds here.

>+}
>+
>+static void atomic_commit(igt_display_t *display, enum pipe pipe, unsigned int flags, void *data, bool fencing)
>+{
>+	if (fencing) {
>+		configure_fencing(display, pipe);
>+		igt_pipe_request_out_fence(&display->pipes[pipe]);
>+	}
>+
>+	igt_display_commit_atomic(display, flags, data);
>+
>+	if (fencing)
>+		clear_fencing(display, pipe);
>+}
>+
> /*
>  * 1. Set primary plane to a known fb.
>  * 2. Make sure getcrtc returns the correct fb id.
>@@ -273,6 +360,7 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 	struct plane_parms parms[display->pipes[pipe].n_planes];
> 	bool skip_test = false;
> 	unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
>+	int ret;
>
> 	if (nonblocking)
> 		flags |= DRM_MODE_ATOMIC_NONBLOCK;
>@@ -307,11 +395,50 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>
> 	setup_parms(display, pipe, mode, &argb_fb, &sprite_fb, parms);
>
>+	/*
>+	 * In some configurations the tests may not run to completion with all
>+	 * sprite planes lit up at 4k resolution, try decreasing width/size of secondary
>+	 * planes to fix this
>+	 */
>+	while (1) {
>+		wm_setup_plane(display, pipe, iter_max - 1, parms);
>+
>+		if (fencing)
>+			igt_pipe_set_out_fence_ptr(&display->pipes[pipe],

That function was deleted in this patch.

>+			    (int64_t *) &out_fence);
>+		ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>+
>+		if (ret != -EINVAL || n_planes < 3)
>+			break;
>+
>+		ret = 0;
>+		for_each_plane_on_pipe(display, pipe, plane) {
>+			i = plane->index;
>+
>+			if (plane->is_primary || plane->is_cursor)
>+				continue;
>+
>+			if (parms[i].width <= 512)
>+				continue;
>+
>+			parms[i].width /= 2;
>+			ret = 1;
>+			igt_info("Reducing sprite %i to %ux%u\n", i - 1, parms[i].width, parms[i].height);
>+			break;
>+		}
>+
>+		if (!ret)
>+			igt_skip("Cannot run tests without proper size sprite planes\n");
>+	}
>+
> 	for (i = 0; i < iter_max; i++) {
> 		igt_output_set_pipe(output, pipe);
>
> 		wm_setup_plane(display, pipe, i, parms);
>
>+		if (fencing)
>+			igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);

Same again.

>+
> 		igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> 		drmHandleEvent(display->drm_fd, &drm_events);
>
>@@ -320,6 +447,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>
> 			wm_setup_plane(display, pipe, 0, parms);
>
>+			if (fencing)
>+				igt_pipe_request_out_fence(&display->pipes[pipe]);
>+
> 			igt_display_commit_atomic(display, flags, (void *)0UL);
>
> 			drmHandleEvent(display->drm_fd, &drm_events);
>@@ -333,6 +463,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 				if (type == TRANSITION_MODESET)
> 					igt_output_override_mode(output, &override_mode);
>
>+				if (fencing)
>+					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);

etc.

-Brian

>+
> 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)j);
> 				drmHandleEvent(display->drm_fd, &drm_events);
>
>@@ -340,6 +473,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 				if (type == TRANSITION_MODESET)
> 					igt_output_override_mode(output, NULL);
>
>+				if (fencing)
>+					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
>+
> 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> 				drmHandleEvent(display->drm_fd, &drm_events);
> 			}
>@@ -676,14 +812,26 @@ igt_main
> 		for_each_pipe_with_valid_output(&display, pipe, output)
> 			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, false);
>
>+	igt_subtest("plane-all-transition-fencing")
>+		for_each_pipe_with_valid_output(&display, pipe, output)
>+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, false, true);
>+
> 	igt_subtest("plane-all-transition-nonblocking")
> 		for_each_pipe_with_valid_output(&display, pipe, output)
> 			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, false);
>
>+	igt_subtest("plane-all-transition-nonblocking-fencing")
>+		for_each_pipe_with_valid_output(&display, pipe, output)
>+			run_transition_test(&display, pipe, output, TRANSITION_PLANES, true, true);
>+
> 	igt_subtest("plane-all-modeset-transition")
> 		for_each_pipe_with_valid_output(&display, pipe, output)
> 			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, false);
>
>+	igt_subtest("plane-all-modeset-transition-fencing")
>+		for_each_pipe_with_valid_output(&display, pipe, output)
>+			run_transition_test(&display, pipe, output, TRANSITION_MODESET, false, true);
>+
> 	igt_subtest("plane-toggle-modeset-transition")
> 		for_each_pipe_with_valid_output(&display, pipe, output)
> 			run_transition_test(&display, pipe, output, TRANSITION_MODESET_DISABLE, false, false);
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 10/11] tests/kms_atomic_transition: add out_fences tests

[Robert Foss]

On 2017-01-31 11:52 AM, Brian Starkey wrote:
> On Mon, Jan 30, 2017 at 08:58:46PM -0500, Robert Foss wrote:
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>> lib/igt_kms.c                 |  35 ++++++----
>> tests/kms_atomic_transition.c | 148
>> ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 169 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index f14496dd..523f3f57 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -53,6 +53,7 @@
>> #include "intel_chipset.h"
>> #include "igt_debugfs.h"
>> #include "igt_sysfs.h"
>> +#include "sw_sync.h"
>>
>> /**
>>  * SECTION:igt_kms
>> @@ -2479,6 +2480,21 @@ static int igt_atomic_commit(igt_display_t
>> *display, uint32_t flags, void *user_
>>     }
>>
>>     ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
>> +    if (!ret) {
>> +
>> +        for_each_pipe(display, pipe) {
>> +            igt_pipe_t *pipe_obj = &display->pipes[pipe];
>> +
>> +            if (pipe_obj->out_fence != -1)
>> +                continue;
>
> Should be "if (pipe_obj->fence == -1)" ? The stuff below seems to be
> assuming the fence is valid.
>

Correct, fixed in v4.

>> +
>> +            igt_assert(pipe_obj->out_fence >= 0);
>> +            ret = sync_fence_wait(pipe_obj->out_fence, 1000);
>> +            igt_assert(ret == 0);
>> +            close(pipe_obj->out_fence);
>> +        }
>> +    }
>> +
>>     drmModeAtomicFree(req);
>>     return ret;
>>
>> @@ -2972,6 +2988,11 @@ void igt_plane_set_rotation(igt_plane_t *plane,
>> igt_rotation_t rotation)
>>     plane->rotation_changed = true;
>> }
>>
>> +void igt_pipe_request_out_fence(igt_pipe_t *pipe)
>
> Oh here it is!
>
>> +{
>> +    pipe->out_fence_requested = true;
>> +}
>> +
>> void
>> igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
>> {
>> @@ -2994,20 +3015,6 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
>> *ptr, size_t length)
>> }
>>
>> /**
>> - * igt_pipe_set_out_fence_ptr:
>> - * @pipe: pipe pointer to which background color to be set
>> - * @fence_ptr: out fence pointer
>> - *
>> - * Sets the out fence pointer that will be passed to the kernel in
>> - * the atomic ioctl. When the kernel returns the out fence pointer
>> - * will contain the fd number of the out fence created by KMS.
>> - */
>> -void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
>
> ... and there this one goes. This deletion and the function above
> need to be squashed into the earlier commit I suppose.
>

Ack, fixed in v4.

>> -{
>> -    pipe->out_fence_ptr = fence_ptr;
>> -}
>> -
>> -/**
>>  * igt_crtc_set_background:
>>  * @pipe: pipe pointer to which background color to be set
>>  * @background: background color value in BGR 16bpc
>> diff --git a/tests/kms_atomic_transition.c
>> b/tests/kms_atomic_transition.c
>> index 72429759..eebb5d66 100644
>> --- a/tests/kms_atomic_transition.c
>> +++ b/tests/kms_atomic_transition.c
>> @@ -253,6 +253,93 @@ retry:
>>          sprite_width, sprite_height, alpha);
>> }
>>
>> +int *timeline;
>> +pthread_t *thread;
>> +int *seqno;
>> +
>> +static void prepare_fencing(igt_display_t *display, enum pipe pipe)
>> +{
>> +    igt_plane_t *plane;
>> +    int n_planes;
>> +
>> +    igt_require_sw_sync();
>> +
>> +    n_planes = display->pipes[pipe].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);
>> +        igt_assert_f(thread != NULL, "Failed to allocate memory for
>> thread\n");
>> +        seqno = calloc(sizeof(*seqno), n_planes);
>> +        igt_assert_f(seqno != NULL, "Failed to allocate memory for
>> seqno\n");
>
> The 6 lines above are space-indented
>

Ack, fixed in v4.

>> +
>> +    for_each_plane_on_pipe(display, pipe, plane)
>> +        timeline[plane->index] = sw_sync_timeline_create();
>> +}
>> +
>> +static void unprepare_fencing(igt_display_t *display, enum pipe pipe)
>> +{
>> +    igt_plane_t *plane;
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane)
>> +        close(timeline[plane->index]);
>> +
>> +    free(timeline);
>> +    free(thread);
>> +    free(seqno);
>> +}
>> +
>> +static void *fence_inc_thread(void *arg)
>> +{
>> +    int t = *((int *) arg);
>> +
>> +    pthread_detach(pthread_self());
>> +
>> +    usleep(5000);
>> +    sw_sync_timeline_inc(t, 1);
>> +    return NULL;
>> +}
>> +
>> +static void configure_fencing(igt_display_t *display, enum pipe pipe)
>> +{
>> +    igt_plane_t *plane;
>> +    int i, fd, ret;
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane) {
>> +
>> +        if (!plane->fb)
>> +            continue;
>> +
>> +        i = plane->index;
>> +
>> +        seqno[i]++;
>> +        fd = sw_sync_timeline_create_fence(timeline[i], seqno[i]);
>> +        igt_plane_set_fence_fd(plane, fd);
>> +        ret = pthread_create(&thread[i], NULL, fence_inc_thread,
>> &timeline[i]);
>> +        igt_assert_eq(ret, 0);
>> +    }
>> +}
>> +
>> +static void clear_fencing(igt_display_t *display, enum pipe pipe)
>> +{
>> +    igt_plane_t *plane;
>> +
>> +    for_each_plane_on_pipe(display, pipe, plane)
>> +        igt_plane_set_fence_fd(plane, -1);
>
> Someone should close the old fence_fd if it's not -1.
>
> I think igt_plane_set_fence_fd() should dup() the passed in fence, and
> igt_display_atomic_commit() should set it to -1 after the commit.
>
> That makes it very obvious, and that way igt_plane_set_fence_fd() can
> always safely close plane->fence_fd if it gets called while the
> current fence_fd is valid, because it always owns that fd. Callers
> have their own copy to take care of.
>
> In any case, I think you're leaking fds here.
>

Fixed in v4:
  - Implemented igt_plane_set_fence_fd logic
  - Added close() to callers with valid FDs

>> +}
>> +
>> +static void atomic_commit(igt_display_t *display, enum pipe pipe,
>> unsigned int flags, void *data, bool fencing)
>> +{
>> +    if (fencing) {
>> +        configure_fencing(display, pipe);
>> +        igt_pipe_request_out_fence(&display->pipes[pipe]);
>> +    }
>> +
>> +    igt_display_commit_atomic(display, flags, data);
>> +
>> +    if (fencing)
>> +        clear_fencing(display, pipe);
>> +}
>> +
>> /*
>>  * 1. Set primary plane to a known fb.
>>  * 2. Make sure getcrtc returns the correct fb id.
>> @@ -273,6 +360,7 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>     struct plane_parms parms[display->pipes[pipe].n_planes];
>>     bool skip_test = false;
>>     unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
>> +    int ret;
>>
>>     if (nonblocking)
>>         flags |= DRM_MODE_ATOMIC_NONBLOCK;
>> @@ -307,11 +395,50 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>
>>     setup_parms(display, pipe, mode, &argb_fb, &sprite_fb, parms);
>>
>> +    /*
>> +     * In some configurations the tests may not run to completion
>> with all
>> +     * sprite planes lit up at 4k resolution, try decreasing
>> width/size of secondary
>> +     * planes to fix this
>> +     */
>> +    while (1) {
>> +        wm_setup_plane(display, pipe, iter_max - 1, parms);
>> +
>> +        if (fencing)
>> +            igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>
> That function was deleted in this patch.

Fixed in v4.

>
>> +                (int64_t *) &out_fence);
>> +        ret = igt_display_try_commit_atomic(display,
>> DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> +
>> +        if (ret != -EINVAL || n_planes < 3)
>> +            break;
>> +
>> +        ret = 0;
>> +        for_each_plane_on_pipe(display, pipe, plane) {
>> +            i = plane->index;
>> +
>> +            if (plane->is_primary || plane->is_cursor)
>> +                continue;
>> +
>> +            if (parms[i].width <= 512)
>> +                continue;
>> +
>> +            parms[i].width /= 2;
>> +            ret = 1;
>> +            igt_info("Reducing sprite %i to %ux%u\n", i - 1,
>> parms[i].width, parms[i].height);
>> +            break;
>> +        }
>> +
>> +        if (!ret)
>> +            igt_skip("Cannot run tests without proper size sprite
>> planes\n");
>> +    }
>> +
>>     for (i = 0; i < iter_max; i++) {
>>         igt_output_set_pipe(output, pipe);
>>
>>         wm_setup_plane(display, pipe, i, parms);
>>
>> +        if (fencing)
>> +            igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>
> Same again.

Fixed in v4.

>
>> +
>>         igt_display_commit_atomic(display, flags, (void *)(unsigned
>> long)i);
>>         drmHandleEvent(display->drm_fd, &drm_events);
>>
>> @@ -320,6 +447,9 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>
>>             wm_setup_plane(display, pipe, 0, parms);
>>
>> +            if (fencing)
>> +                igt_pipe_request_out_fence(&display->pipes[pipe]);
>> +
>>             igt_display_commit_atomic(display, flags, (void *)0UL);
>>
>>             drmHandleEvent(display->drm_fd, &drm_events);
>> @@ -333,6 +463,9 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>                 if (type == TRANSITION_MODESET)
>>                     igt_output_override_mode(output, &override_mode);
>>
>> +                if (fencing)
>> +                    igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>
> etc.

Fixed in v4.

>
> -Brian
>
>> +
>>                 igt_display_commit_atomic(display, flags, (void
>> *)(unsigned long)j);
>>                 drmHandleEvent(display->drm_fd, &drm_events);
>>
>> @@ -340,6 +473,9 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>                 if (type == TRANSITION_MODESET)
>>                     igt_output_override_mode(output, NULL);
>>
>> +                if (fencing)
>> +                    igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>> +
>>                 igt_display_commit_atomic(display, flags, (void
>> *)(unsigned long)i);
>>                 drmHandleEvent(display->drm_fd, &drm_events);
>>             }
>> @@ -676,14 +812,26 @@ igt_main
>>         for_each_pipe_with_valid_output(&display, pipe, output)
>>             run_transition_test(&display, pipe, output,
>> TRANSITION_PLANES, false, false);
>>
>> +    igt_subtest("plane-all-transition-fencing")
>> +        for_each_pipe_with_valid_output(&display, pipe, output)
>> +            run_transition_test(&display, pipe, output,
>> TRANSITION_PLANES, false, true);
>> +
>>     igt_subtest("plane-all-transition-nonblocking")
>>         for_each_pipe_with_valid_output(&display, pipe, output)
>>             run_transition_test(&display, pipe, output,
>> TRANSITION_PLANES, true, false);
>>
>> +    igt_subtest("plane-all-transition-nonblocking-fencing")
>> +        for_each_pipe_with_valid_output(&display, pipe, output)
>> +            run_transition_test(&display, pipe, output,
>> TRANSITION_PLANES, true, true);
>> +
>>     igt_subtest("plane-all-modeset-transition")
>>         for_each_pipe_with_valid_output(&display, pipe, output)
>>             run_transition_test(&display, pipe, output,
>> TRANSITION_MODESET, false, false);
>>
>> +    igt_subtest("plane-all-modeset-transition-fencing")
>> +        for_each_pipe_with_valid_output(&display, pipe, output)
>> +            run_transition_test(&display, pipe, output,
>> TRANSITION_MODESET, false, true);
>> +
>>     igt_subtest("plane-toggle-modeset-transition")
>>         for_each_pipe_with_valid_output(&display, pipe, output)
>>             run_transition_test(&display, pipe, output,
>> TRANSITION_MODESET_DISABLE, false, false);
>> --
>> 2.11.0.453.g787f75f05
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3 11/11] tests/kms_atomic_transition: add in_fences tests

[Robert Foss]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 lib/igt_kms.c                 |  3 +++
 tests/kms_atomic_transition.c | 48 ++++++++++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 523f3f57..bc815363 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
 	if (plane->fence_fd >= 0) {
 		uint64_t fence_fd = (int64_t) plane->fence_fd;
 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd);
+
+		/* reset fence_fd to prevent it from being set for the next commit */
+		plane->fence_fd = -1;
 	}
 
 	if (plane->fb_changed) {
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index eebb5d66..0876bbb3 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -23,7 +23,9 @@
 
 #include "igt.h"
 #include "drmtest.h"
+#include "sw_sync.h"
 #include <errno.h>
+#include <pthread.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
@@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 	unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
 	int ret;
 
+	if (fencing)
+		prepare_fencing(display, pipe);
+
 	if (nonblocking)
 		flags |= DRM_MODE_ATOMIC_NONBLOCK;
 
@@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 		wm_setup_plane(display, pipe, iter_max - 1, parms);
 
 		if (fencing)
-			igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
-			    (int64_t *) &out_fence);
+			igt_pipe_request_out_fence(&display->pipes[pipe]);
+
 		ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
 
-		if (ret != -EINVAL || n_planes < 3)
+		if (ret != -EINVAL || display->pipes[pipe].n_planes < 3)
 			break;
 
 		ret = 0;
 		for_each_plane_on_pipe(display, pipe, plane) {
 			i = plane->index;
 
-			if (plane->is_primary || plane->is_cursor)
+			if (plane->type == DRM_PLANE_TYPE_PRIMARY ||
+			    plane->type == DRM_PLANE_TYPE_CURSOR)
 				continue;
 
 			if (parms[i].width <= 512)
@@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 
 		wm_setup_plane(display, pipe, i, parms);
 
-		if (fencing)
-			igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
+		atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
 
-		igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
 		drmHandleEvent(display->drm_fd, &drm_events);
 
 		if (type == TRANSITION_MODESET_DISABLE) {
@@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 				if (type == TRANSITION_MODESET)
 					igt_output_override_mode(output, &override_mode);
 
-				if (fencing)
-					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
-
-				igt_display_commit_atomic(display, flags, (void *)(unsigned long)j);
+				atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
 				drmHandleEvent(display->drm_fd, &drm_events);
 
 				wm_setup_plane(display, pipe, i, parms);
 				if (type == TRANSITION_MODESET)
 					igt_output_override_mode(output, NULL);
 
-				if (fencing)
-					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
-
+				atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
 				drmHandleEvent(display->drm_fd, &drm_events);
 			}
@@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
 	}
 
 cleanup:
+	unprepare_fencing(display, pipe);
+
 	igt_output_set_pipe(output, PIPE_NONE);
 
 	for_each_plane_on_pipe(display, pipe, plane)
@@ -617,7 +618,7 @@ 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)
+static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing)
 {
 	struct igt_fb fbs[2];
 	int i, j;
@@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblock
 			igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
 		} else
 			igt_plane_set_fb(plane, NULL);
+
+		if(fencing)
+			igt_pipe_request_out_fence(&display->pipes[i]);
 	}
 
 	/*
@@ -751,7 +755,7 @@ cleanup:
 
 }
 
-static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking)
+static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking, bool fencing)
 {
 	igt_output_t *outputs[I915_MAX_PIPES] = {};
 	int num_outputs = 0;
@@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t *display, int requested_outputs
 		      "Should have at least %i outputs, found %i\n",
 		      requested_outputs, num_outputs);
 
-	run_modeset_tests(display, requested_outputs, nonblocking);
+	run_modeset_tests(display, requested_outputs, nonblocking, fencing);
 }
 
 igt_main
@@ -838,10 +842,16 @@ igt_main
 
 	for (i = 1; i <= I915_MAX_PIPES; i++) {
 		igt_subtest_f("%ix-modeset-transitions", i)
-			run_modeset_transition(&display, i, false);
+			run_modeset_transition(&display, i, false, false);
 
 		igt_subtest_f("%ix-modeset-transitions-nonblocking", i)
-			run_modeset_transition(&display, i, true);
+			run_modeset_transition(&display, i, true, false);
+
+		igt_subtest_f("%ix-modeset-transitions-fencing", i)
+			run_modeset_transition(&display, i, false, true);
+
+		igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i)
+			run_modeset_transition(&display, i, true, true);
 	}
 
 	igt_fixture {
-- 
2.11.0.453.g787f75f05

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 11/11] tests/kms_atomic_transition: add in_fences tests

[Brian Starkey]

On Mon, Jan 30, 2017 at 08:58:47PM -0500, Robert Foss wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>Signed-off-by: Robert Foss <robert.foss@collabora.com>
>---
> lib/igt_kms.c                 |  3 +++
> tests/kms_atomic_transition.c | 48 ++++++++++++++++++++++++++-----------------
> 2 files changed, 32 insertions(+), 19 deletions(-)
>
>diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>index 523f3f57..bc815363 100644
>--- a/lib/igt_kms.c
>+++ b/lib/igt_kms.c
>@@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
> 	if (plane->fence_fd >= 0) {
> 		uint64_t fence_fd = (int64_t) plane->fence_fd;
> 		igt_atomic_populate_plane_req(req, plane, IGT_PLANE_IN_FENCE_FD, fence_fd);
>+
>+		/* reset fence_fd to prevent it from being set for the next commit */
>+		plane->fence_fd = -1;

Who closes it?

> 	}
>
> 	if (plane->fb_changed) {
>diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>index eebb5d66..0876bbb3 100644
>--- a/tests/kms_atomic_transition.c
>+++ b/tests/kms_atomic_transition.c
>@@ -23,7 +23,9 @@
>
> #include "igt.h"
> #include "drmtest.h"
>+#include "sw_sync.h"
> #include <errno.h>
>+#include <pthread.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <string.h>
>@@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 	unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
> 	int ret;
>
>+	if (fencing)
>+		prepare_fencing(display, pipe);
>+
> 	if (nonblocking)
> 		flags |= DRM_MODE_ATOMIC_NONBLOCK;
>
>@@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 		wm_setup_plane(display, pipe, iter_max - 1, parms);
>
> 		if (fencing)
>-			igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>-			    (int64_t *) &out_fence);
>+			igt_pipe_request_out_fence(&display->pipes[pipe]);
>+

Hopefully this can get rebased away? I'm getting confused about
what's actually being added/changed in this commit.

> 		ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>
>-		if (ret != -EINVAL || n_planes < 3)
>+		if (ret != -EINVAL || display->pipes[pipe].n_planes < 3)
> 			break;
>
> 		ret = 0;
> 		for_each_plane_on_pipe(display, pipe, plane) {
> 			i = plane->index;
>
>-			if (plane->is_primary || plane->is_cursor)
>+			if (plane->type == DRM_PLANE_TYPE_PRIMARY ||
>+			    plane->type == DRM_PLANE_TYPE_CURSOR)
> 				continue;

This seems spurious?

...

A bit more add/remove churn below which can hopefully go away with a
rebase.

Cheers,
-Brian
>
> 			if (parms[i].width <= 512)
>@@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
>
> 		wm_setup_plane(display, pipe, i, parms);
>
>-		if (fencing)
>-			igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
>+		atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
>
>-		igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> 		drmHandleEvent(display->drm_fd, &drm_events);
>
> 		if (type == TRANSITION_MODESET_DISABLE) {
>@@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 				if (type == TRANSITION_MODESET)
> 					igt_output_override_mode(output, &override_mode);
>
>-				if (fencing)
>-					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
>-
>-				igt_display_commit_atomic(display, flags, (void *)(unsigned long)j);
>+				atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
> 				drmHandleEvent(display->drm_fd, &drm_events);
>
> 				wm_setup_plane(display, pipe, i, parms);
> 				if (type == TRANSITION_MODESET)
> 					igt_output_override_mode(output, NULL);
>
>-				if (fencing)
>-					igt_pipe_set_out_fence_ptr(&display->pipes[pipe], &out_fence);
>-
>+				atomic_commit(display, pipe, flags, (void *)(unsigned long)i, fencing);
> 				igt_display_commit_atomic(display, flags, (void *)(unsigned long)i);
> 				drmHandleEvent(display->drm_fd, &drm_events);
> 			}
>@@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output
> 	}
>
> cleanup:
>+	unprepare_fencing(display, pipe);
>+
> 	igt_output_set_pipe(output, PIPE_NONE);
>
> 	for_each_plane_on_pipe(display, pipe, plane)
>@@ -617,7 +618,7 @@ 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)
>+static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblocking, bool fencing)
> {
> 	struct igt_fb fbs[2];
> 	int i, j;
>@@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t *display, int howmany, bool nonblock
> 			igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
> 		} else
> 			igt_plane_set_fb(plane, NULL);
>+
>+		if(fencing)
>+			igt_pipe_request_out_fence(&display->pipes[i]);
> 	}
>
> 	/*
>@@ -751,7 +755,7 @@ cleanup:
>
> }
>
>-static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking)
>+static void run_modeset_transition(igt_display_t *display, int requested_outputs, bool nonblocking, bool fencing)
> {
> 	igt_output_t *outputs[I915_MAX_PIPES] = {};
> 	int num_outputs = 0;
>@@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t *display, int requested_outputs
> 		      "Should have at least %i outputs, found %i\n",
> 		      requested_outputs, num_outputs);
>
>-	run_modeset_tests(display, requested_outputs, nonblocking);
>+	run_modeset_tests(display, requested_outputs, nonblocking, fencing);
> }
>
> igt_main
>@@ -838,10 +842,16 @@ igt_main
>
> 	for (i = 1; i <= I915_MAX_PIPES; i++) {
> 		igt_subtest_f("%ix-modeset-transitions", i)
>-			run_modeset_transition(&display, i, false);
>+			run_modeset_transition(&display, i, false, false);
>
> 		igt_subtest_f("%ix-modeset-transitions-nonblocking", i)
>-			run_modeset_transition(&display, i, true);
>+			run_modeset_transition(&display, i, true, false);
>+
>+		igt_subtest_f("%ix-modeset-transitions-fencing", i)
>+			run_modeset_transition(&display, i, false, true);
>+
>+		igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i)
>+			run_modeset_transition(&display, i, true, true);
> 	}
>
> 	igt_fixture {
>-- 
>2.11.0.453.g787f75f05
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 11/11] tests/kms_atomic_transition: add in_fences tests

[Robert Foss]

On 2017-01-31 11:52 AM, Brian Starkey wrote:
> On Mon, Jan 30, 2017 at 08:58:47PM -0500, Robert Foss wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>> lib/igt_kms.c                 |  3 +++
>> tests/kms_atomic_transition.c | 48
>> ++++++++++++++++++++++++++-----------------
>> 2 files changed, 32 insertions(+), 19 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 523f3f57..bc815363 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -2009,6 +2009,9 @@ igt_atomic_prepare_plane_commit(igt_plane_t
>> *plane, igt_pipe_t *pipe,
>>     if (plane->fence_fd >= 0) {
>>         uint64_t fence_fd = (int64_t) plane->fence_fd;
>>         igt_atomic_populate_plane_req(req, plane,
>> IGT_PLANE_IN_FENCE_FD, fence_fd);
>> +
>> +        /* reset fence_fd to prevent it from being set for the next
>> commit */
>> +        plane->fence_fd = -1;
>
> Who closes it?

Fixed in v4.

>
>>     }
>>
>>     if (plane->fb_changed) {
>> diff --git a/tests/kms_atomic_transition.c
>> b/tests/kms_atomic_transition.c
>> index eebb5d66..0876bbb3 100644
>> --- a/tests/kms_atomic_transition.c
>> +++ b/tests/kms_atomic_transition.c
>> @@ -23,7 +23,9 @@
>>
>> #include "igt.h"
>> #include "drmtest.h"
>> +#include "sw_sync.h"
>> #include <errno.h>
>> +#include <pthread.h>
>> #include <stdbool.h>
>> #include <stdio.h>
>> #include <string.h>
>> @@ -362,6 +364,9 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>     unsigned flags = DRM_MODE_PAGE_FLIP_EVENT;
>>     int ret;
>>
>> +    if (fencing)
>> +        prepare_fencing(display, pipe);
>> +
>>     if (nonblocking)
>>         flags |= DRM_MODE_ATOMIC_NONBLOCK;
>>
>> @@ -404,18 +409,19 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>         wm_setup_plane(display, pipe, iter_max - 1, parms);
>>
>>         if (fencing)
>> -            igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> -                (int64_t *) &out_fence);
>> +            igt_pipe_request_out_fence(&display->pipes[pipe]);
>> +
>
> Hopefully this can get rebased away? I'm getting confused about
> what's actually being added/changed in this commit.

Fixed in v4.
The igt_pipe_set_out_fence_ptr calls were really spread about.

>
>>         ret = igt_display_try_commit_atomic(display,
>> DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>>
>> -        if (ret != -EINVAL || n_planes < 3)
>> +        if (ret != -EINVAL || display->pipes[pipe].n_planes < 3)
>>             break;
>>
>>         ret = 0;
>>         for_each_plane_on_pipe(display, pipe, plane) {
>>             i = plane->index;
>>
>> -            if (plane->is_primary || plane->is_cursor)
>> +            if (plane->type == DRM_PLANE_TYPE_PRIMARY ||
>> +                plane->type == DRM_PLANE_TYPE_CURSOR)
>>                 continue;
>
> This seems spurious?
>
> ...
>
> A bit more add/remove churn below which can hopefully go away with a
> rebase.
>

Ack, removing churn in v4.

> Cheers,
> -Brian
>>
>>             if (parms[i].width <= 512)
>> @@ -436,10 +442,8 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>
>>         wm_setup_plane(display, pipe, i, parms);
>>
>> -        if (fencing)
>> -            igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>> +        atomic_commit(display, pipe, flags, (void *)(unsigned long)i,
>> fencing);
>>
>> -        igt_display_commit_atomic(display, flags, (void *)(unsigned
>> long)i);
>>         drmHandleEvent(display->drm_fd, &drm_events);
>>
>>         if (type == TRANSITION_MODESET_DISABLE) {
>> @@ -463,19 +467,14 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>                 if (type == TRANSITION_MODESET)
>>                     igt_output_override_mode(output, &override_mode);
>>
>> -                if (fencing)
>> -                    igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>> -
>> -                igt_display_commit_atomic(display, flags, (void
>> *)(unsigned long)j);
>> +                atomic_commit(display, pipe, flags, (void *)(unsigned
>> long)i, fencing);
>>                 drmHandleEvent(display->drm_fd, &drm_events);
>>
>>                 wm_setup_plane(display, pipe, i, parms);
>>                 if (type == TRANSITION_MODESET)
>>                     igt_output_override_mode(output, NULL);
>>
>> -                if (fencing)
>> -                    igt_pipe_set_out_fence_ptr(&display->pipes[pipe],
>> &out_fence);
>> -
>> +                atomic_commit(display, pipe, flags, (void *)(unsigned
>> long)i, fencing);
>>                 igt_display_commit_atomic(display, flags, (void
>> *)(unsigned long)i);
>>                 drmHandleEvent(display->drm_fd, &drm_events);
>>             }
>> @@ -483,6 +482,8 @@ run_transition_test(igt_display_t *display, enum
>> pipe pipe, igt_output_t *output
>>     }
>>
>> cleanup:
>> +    unprepare_fencing(display, pipe);
>> +
>>     igt_output_set_pipe(output, PIPE_NONE);
>>
>>     for_each_plane_on_pipe(display, pipe, plane)
>> @@ -617,7 +618,7 @@ 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)
>> +static void run_modeset_tests(igt_display_t *display, int howmany,
>> bool nonblocking, bool fencing)
>> {
>>     struct igt_fb fbs[2];
>>     int i, j;
>> @@ -664,6 +665,9 @@ static void run_modeset_tests(igt_display_t
>> *display, int howmany, bool nonblock
>>             igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
>>         } else
>>             igt_plane_set_fb(plane, NULL);
>> +
>> +        if(fencing)
>> +            igt_pipe_request_out_fence(&display->pipes[i]);
>>     }
>>
>>     /*
>> @@ -751,7 +755,7 @@ cleanup:
>>
>> }
>>
>> -static void run_modeset_transition(igt_display_t *display, int
>> requested_outputs, bool nonblocking)
>> +static void run_modeset_transition(igt_display_t *display, int
>> requested_outputs, bool nonblocking, bool fencing)
>> {
>>     igt_output_t *outputs[I915_MAX_PIPES] = {};
>>     int num_outputs = 0;
>> @@ -779,7 +783,7 @@ static void run_modeset_transition(igt_display_t
>> *display, int requested_outputs
>>               "Should have at least %i outputs, found %i\n",
>>               requested_outputs, num_outputs);
>>
>> -    run_modeset_tests(display, requested_outputs, nonblocking);
>> +    run_modeset_tests(display, requested_outputs, nonblocking, fencing);
>> }
>>
>> igt_main
>> @@ -838,10 +842,16 @@ igt_main
>>
>>     for (i = 1; i <= I915_MAX_PIPES; i++) {
>>         igt_subtest_f("%ix-modeset-transitions", i)
>> -            run_modeset_transition(&display, i, false);
>> +            run_modeset_transition(&display, i, false, false);
>>
>>         igt_subtest_f("%ix-modeset-transitions-nonblocking", i)
>> -            run_modeset_transition(&display, i, true);
>> +            run_modeset_transition(&display, i, true, false);
>> +
>> +        igt_subtest_f("%ix-modeset-transitions-fencing", i)
>> +            run_modeset_transition(&display, i, false, true);
>> +
>> +        igt_subtest_f("%ix-modeset-transitions-nonblocking-fencing", i)
>> +            run_modeset_transition(&display, i, true, true);
>>     }
>>
>>     igt_fixture {
>> --
>> 2.11.0.453.g787f75f05
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 00/11] tests/kms_atomic_transition add fence testing

[Chris Wilson]

On Mon, Jan 30, 2017 at 08:58:36PM -0500, Robert Foss wrote:
> This series adds in/out fence testing to kms_atomic_transition test and makes some minor cleanups.
> 
> This series is rebased ontop of the dyn_n_planes_v3 series.
> 
> This series can be found here:
> https://git.collabora.com/cgit/user/robertfoss/intel-gpu-tools.git/log/?h=fences_$VER
> 
> 
> Changes since v1:
> 
>   lib/igt_kms:
>    - Added gtk-doc for exported symbols
>    - Changed integer casting to avoid potential issues
>    - Changed out_fence_ptr type to int64_t*
>    - Fixed igt_plane_set_fence_fd comment
> 
>   tests/:
>    - Rework timeout change in commit_display()
>    - Extract plane_invalid_params_fence() out plane_invalid_params()
>    - Extract crtc_invalid_params_fence() out crtc_invalid_params()
>    - Prevent add igt_require_sw_sync to subtests using sw_sync
> 
> 
> Changes since v2:
>   Rebased on upstream/master
> 
>   lib/igt_kms:
>     - Reset plane->fence_fd to -1 during igt_atomic_prepare_plane_commit()
>     - Rework out_fencs_ptr to be an int64_t named out_fence
>     - Add igt_pipe_request_out_fence()
>   tests/:
>     - Switch to using igt_pipe_request_out_fence()
>     - Close out_fence fd
>     - Change out_fence to int64_t in run_transition_test()
>     - Added comments noting that two testcases are not invalid
>     - Added igt_pipe_get_last_out_fence() that wraps pipe->fence_out

Looks like this this missing the uabi conversion to s32 (int).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v3 00/11] tests/kms_atomic_transition add fence testing

[Robert Foss]

On 2017-01-31 05:18 AM, Chris Wilson wrote:
> On Mon, Jan 30, 2017 at 08:58:36PM -0500, Robert Foss wrote:
>> This series adds in/out fence testing to kms_atomic_transition test and makes some minor cleanups.
>>
>> This series is rebased ontop of the dyn_n_planes_v3 series.
>>
>> This series can be found here:
>> https://git.collabora.com/cgit/user/robertfoss/intel-gpu-tools.git/log/?h=fences_$VER
>>
>>
>> Changes since v1:
>>
>>   lib/igt_kms:
>>    - Added gtk-doc for exported symbols
>>    - Changed integer casting to avoid potential issues
>>    - Changed out_fence_ptr type to int64_t*
>>    - Fixed igt_plane_set_fence_fd comment
>>
>>   tests/:
>>    - Rework timeout change in commit_display()
>>    - Extract plane_invalid_params_fence() out plane_invalid_params()
>>    - Extract crtc_invalid_params_fence() out crtc_invalid_params()
>>    - Prevent add igt_require_sw_sync to subtests using sw_sync
>>
>>
>> Changes since v2:
>>   Rebased on upstream/master
>>
>>   lib/igt_kms:
>>     - Reset plane->fence_fd to -1 during igt_atomic_prepare_plane_commit()
>>     - Rework out_fencs_ptr to be an int64_t named out_fence
>>     - Add igt_pipe_request_out_fence()
>>   tests/:
>>     - Switch to using igt_pipe_request_out_fence()
>>     - Close out_fence fd
>>     - Change out_fence to int64_t in run_transition_test()
>>     - Added comments noting that two testcases are not invalid
>>     - Added igt_pipe_get_last_out_fence() that wraps pipe->fence_out
>
> Looks like this this missing the uabi conversion to s32 (int).
> -Chris

Correct, I'll submit a v4 with this fix later today if no other major 
issues are reported.

Rob.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx