All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors
@ 2019-01-15 17:47 ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

We're trying to introduce support for writeback connectors, a way to
expose in DRM the hardware functionality from display engines that
allows to write back into memory the result of the DE's composition
of supported planes.

Although this is a rebase of v4 with all the comments addressed, I'm not
expecting people to remember any of the previous versions, please review
this as if it is a new series.

Patches have been originally implemented by Brian, I've done the v3 and v4
updates to them.

Best regards,
Liviu

Changelog:
 - v5: Addressed comments from Brian Starkey. Old v4 changes are here:
   https://lists.freedesktop.org/archives/igt-dev/2018-November/006806.html
 
 - v4: Rebased on the latest i-g-t and switched to the igt_output_xxx()
   call as suggested by Maarten. v3 is here:
   https://lists.freedesktop.org/archives/intel-gfx/2018-March/157394.html
   Maarten's comments came a couple of months later :)
   https://lists.freedesktop.org/archives/intel-gfx/2018-June/169027.html
   
 - v3: I've now dropped all the changes that were trying to split the CRC
   functionality out of lib/igt_debugfs.c. v2 is here:
   https://lists.freedesktop.org/archives/intel-gfx/2017-July/133154.html
   
 - Added meson support for builting the kms_writeback test


Brian Starkey (6):
  lib/igt_kms: Add writeback support
  kms_writeback: Add initial writeback tests
  lib: Add function to hash a framebuffer
  kms_writeback: Add writeback-check-output
  lib/igt_kms: Add igt_output_clone_pipe for cloning
  kms_writeback: Add tests using a cloned output

 lib/igt_fb.c           |  66 ++++++
 lib/igt_fb.h           |   3 +
 lib/igt_kms.c          | 157 +++++++++----
 lib/igt_kms.h          |  11 +
 tests/Makefile.sources |   1 +
 tests/kms_writeback.c  | 492 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 7 files changed, 693 insertions(+), 38 deletions(-)
 create mode 100644 tests/kms_writeback.c

-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors
@ 2019-01-15 17:47 ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

We're trying to introduce support for writeback connectors, a way to
expose in DRM the hardware functionality from display engines that
allows to write back into memory the result of the DE's composition
of supported planes.

Although this is a rebase of v4 with all the comments addressed, I'm not
expecting people to remember any of the previous versions, please review
this as if it is a new series.

Patches have been originally implemented by Brian, I've done the v3 and v4
updates to them.

Best regards,
Liviu

Changelog:
 - v5: Addressed comments from Brian Starkey. Old v4 changes are here:
   https://lists.freedesktop.org/archives/igt-dev/2018-November/006806.html
 
 - v4: Rebased on the latest i-g-t and switched to the igt_output_xxx()
   call as suggested by Maarten. v3 is here:
   https://lists.freedesktop.org/archives/intel-gfx/2018-March/157394.html
   Maarten's comments came a couple of months later :)
   https://lists.freedesktop.org/archives/intel-gfx/2018-June/169027.html
   
 - v3: I've now dropped all the changes that were trying to split the CRC
   functionality out of lib/igt_debugfs.c. v2 is here:
   https://lists.freedesktop.org/archives/intel-gfx/2017-July/133154.html
   
 - Added meson support for builting the kms_writeback test


Brian Starkey (6):
  lib/igt_kms: Add writeback support
  kms_writeback: Add initial writeback tests
  lib: Add function to hash a framebuffer
  kms_writeback: Add writeback-check-output
  lib/igt_kms: Add igt_output_clone_pipe for cloning
  kms_writeback: Add tests using a cloned output

 lib/igt_fb.c           |  66 ++++++
 lib/igt_fb.h           |   3 +
 lib/igt_kms.c          | 157 +++++++++----
 lib/igt_kms.h          |  11 +
 tests/Makefile.sources |   1 +
 tests/kms_writeback.c  | 492 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 7 files changed, 693 insertions(+), 38 deletions(-)
 create mode 100644 tests/kms_writeback.c

-- 
2.20.1

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

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

* [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

Add support in igt_kms for writeback connectors, with the ability
to attach framebuffers.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the latest igt style]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  6 ++++++
 2 files changed, 63 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 684a599ca..0bc2996cd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -197,6 +197,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
 	[IGT_CONNECTOR_DPMS] = "DPMS",
 	[IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
 	[IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
+	[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
+	[IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
+	[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
 };
 
 /*
@@ -453,6 +456,7 @@ static const struct type_name connector_type_names[] = {
 	{ DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
 	{ DRM_MODE_CONNECTOR_DSI, "DSI" },
 	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
+	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
 	{}
 };
 
@@ -1802,6 +1806,12 @@ static void igt_output_reset(igt_output_t *output)
 	if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
 		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
 					  BROADCAST_RGB_FULL);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
+		igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -1814,6 +1824,8 @@ static void igt_output_reset(igt_output_t *output)
  * For outputs:
  * - %IGT_CONNECTOR_CRTC_ID
  * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
+ * - %IGT_CONNECTOR_WRITEBACK_FB_ID
+ * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
  * - igt_output_override_mode() to default.
  *
  * For pipes:
@@ -1899,6 +1911,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
 		display->is_atomic = 1;
 
+	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+
 	plane_resources = drmModeGetPlaneResources(display->drm_fd);
 	igt_assert(plane_resources);
 
@@ -2147,6 +2161,11 @@ static void igt_output_fini(igt_output_t *output)
 	kmstest_free_connector_config(&output->config);
 	free(output->name);
 	output->name = NULL;
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -3144,6 +3163,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
 					  output->props[i],
 					  output->values[i]));
 	}
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /*
@@ -3264,6 +3288,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
 		else
 			/* no modeset in universal commit, no change to crtc. */
 			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
+
+		if (s == COMMIT_ATOMIC) {
+			if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
+				igt_assert(output->writeback_out_fence_fd >= 0);
+
+			output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
+			output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		}
 	}
 
 	if (display->first_commit) {
@@ -3884,6 +3918,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
 	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
 }
 
+/**
+ * igt_output_set_writeback_fb:
+ * @output: Target output
+ * @fb: Target framebuffer
+ *
+ * This function sets the given @fb to be used as the target framebuffer for the
+ * writeback engine at the next atomic commit. It will also request a writeback
+ * out fence that will contain the fd number of the out fence created by KMS if
+ * the given @fb is valid.
+ */
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
+{
+	igt_display_t *display = output->display;
+
+	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
+
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
+	/* only request a writeback out fence if the framebuffer is valid */
+	if (fb)
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
+					  (ptrdiff_t)&output->writeback_out_fence_fd);
+}
+
 /**
  * igt_wait_for_vblank_count:
  * @drm_fd: A drm file descriptor
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4a7c3c979..13d3a9ceb 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -121,6 +121,9 @@ enum igt_atomic_connector_properties {
        IGT_CONNECTOR_DPMS,
        IGT_CONNECTOR_BROADCAST_RGB,
        IGT_CONNECTOR_CONTENT_PROTECTION,
+       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
+       IGT_CONNECTOR_WRITEBACK_FB_ID,
+       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
        IGT_NUM_CONNECTOR_PROPS
 };
 
@@ -359,6 +362,8 @@ typedef struct {
 	bool use_override_mode;
 	drmModeModeInfo override_mode;
 
+	int32_t writeback_out_fence_fd;
+
 	/* bitmask of changed properties */
 	uint64_t changed;
 
@@ -403,6 +408,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
 
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 igt_output_t *igt_get_single_output_for_pipe(igt_display_t *display, enum pipe pipe);
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

Add support in igt_kms for writeback connectors, with the ability
to attach framebuffers.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the latest igt style]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  6 ++++++
 2 files changed, 63 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 684a599ca..0bc2996cd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -197,6 +197,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
 	[IGT_CONNECTOR_DPMS] = "DPMS",
 	[IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
 	[IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
+	[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
+	[IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
+	[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
 };
 
 /*
@@ -453,6 +456,7 @@ static const struct type_name connector_type_names[] = {
 	{ DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
 	{ DRM_MODE_CONNECTOR_DSI, "DSI" },
 	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
+	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
 	{}
 };
 
@@ -1802,6 +1806,12 @@ static void igt_output_reset(igt_output_t *output)
 	if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
 		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
 					  BROADCAST_RGB_FULL);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
+		igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -1814,6 +1824,8 @@ static void igt_output_reset(igt_output_t *output)
  * For outputs:
  * - %IGT_CONNECTOR_CRTC_ID
  * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
+ * - %IGT_CONNECTOR_WRITEBACK_FB_ID
+ * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
  * - igt_output_override_mode() to default.
  *
  * For pipes:
@@ -1899,6 +1911,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
 		display->is_atomic = 1;
 
+	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+
 	plane_resources = drmModeGetPlaneResources(display->drm_fd);
 	igt_assert(plane_resources);
 
@@ -2147,6 +2161,11 @@ static void igt_output_fini(igt_output_t *output)
 	kmstest_free_connector_config(&output->config);
 	free(output->name);
 	output->name = NULL;
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -3144,6 +3163,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
 					  output->props[i],
 					  output->values[i]));
 	}
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /*
@@ -3264,6 +3288,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
 		else
 			/* no modeset in universal commit, no change to crtc. */
 			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
+
+		if (s == COMMIT_ATOMIC) {
+			if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
+				igt_assert(output->writeback_out_fence_fd >= 0);
+
+			output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
+			output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		}
 	}
 
 	if (display->first_commit) {
@@ -3884,6 +3918,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
 	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
 }
 
+/**
+ * igt_output_set_writeback_fb:
+ * @output: Target output
+ * @fb: Target framebuffer
+ *
+ * This function sets the given @fb to be used as the target framebuffer for the
+ * writeback engine at the next atomic commit. It will also request a writeback
+ * out fence that will contain the fd number of the out fence created by KMS if
+ * the given @fb is valid.
+ */
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
+{
+	igt_display_t *display = output->display;
+
+	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
+
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
+	/* only request a writeback out fence if the framebuffer is valid */
+	if (fb)
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
+					  (ptrdiff_t)&output->writeback_out_fence_fd);
+}
+
 /**
  * igt_wait_for_vblank_count:
  * @drm_fd: A drm file descriptor
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4a7c3c979..13d3a9ceb 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -121,6 +121,9 @@ enum igt_atomic_connector_properties {
        IGT_CONNECTOR_DPMS,
        IGT_CONNECTOR_BROADCAST_RGB,
        IGT_CONNECTOR_CONTENT_PROTECTION,
+       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
+       IGT_CONNECTOR_WRITEBACK_FB_ID,
+       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
        IGT_NUM_CONNECTOR_PROPS
 };
 
@@ -359,6 +362,8 @@ typedef struct {
 	bool use_override_mode;
 	drmModeModeInfo override_mode;
 
+	int32_t writeback_out_fence_fd;
+
 	/* bitmask of changed properties */
 	uint64_t changed;
 
@@ -403,6 +408,7 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
 
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 igt_output_t *igt_get_single_output_for_pipe(igt_display_t *display, enum pipe pipe);
-- 
2.20.1

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

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

* [PATCH i-g-t v5 2/6] kms_writeback: Add initial writeback tests
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

Add tests for the WRITEBACK_PIXEL_FORMATS, WRITEBACK_OUT_FENCE_PTR and
WRITEBACK_FB_ID properties on writeback connectors, ensuring their
behaviour is correct.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated do_writeback_test() function to address feedback]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_writeback.c  | 314 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 3 files changed, 316 insertions(+)
 create mode 100644 tests/kms_writeback.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 519eac792..4438d2165 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs = \
 	kms_universal_plane \
 	kms_vblank \
 	kms_sequence \
+	kms_writeback \
 	meta_test \
 	perf \
 	perf_pmu \
diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
new file mode 100644
index 000000000..66ef48a6c
--- /dev/null
+++ b/tests/kms_writeback.c
@@ -0,0 +1,314 @@
+/*
+ * (C) COPYRIGHT 2017 ARM Limited. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "igt.h"
+#include "igt_core.h"
+#include "igt_fb.h"
+
+static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
+{
+	drmModePropertyBlobRes *blob = NULL;
+	uint64_t blob_id;
+	int ret;
+
+	ret = kmstest_get_property(output->display->drm_fd,
+				   output->config.connector->connector_id,
+				   DRM_MODE_OBJECT_CONNECTOR,
+				   igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
+				   NULL, &blob_id, NULL);
+	if (ret)
+		blob = drmModeGetPropertyBlob(output->display->drm_fd, blob_id);
+
+	igt_assert(blob);
+
+	return blob;
+}
+
+static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
+{
+	igt_fb_t input_fb, output_fb;
+	igt_plane_t *plane;
+	uint32_t writeback_format = DRM_FORMAT_XRGB8888;
+	uint64_t tiling = igt_fb_mod_to_tiling(0);
+	int width, height, ret;
+	drmModeModeInfo override_mode = {
+		.clock = 25175,
+		.hdisplay = 640,
+		.hsync_start = 656,
+		.hsync_end = 752,
+		.htotal = 800,
+		.hskew = 0,
+		.vdisplay = 480,
+		.vsync_start = 490,
+		.vsync_end = 492,
+		.vtotal = 525,
+		.vscan = 0,
+		.vrefresh = 60,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.name = {"640x480-60"},
+	};
+	igt_output_override_mode(output, &override_mode);
+
+	width = override_mode.hdisplay;
+	height = override_mode.vdisplay;
+
+	ret = igt_create_fb(display->drm_fd, width, height, DRM_FORMAT_XRGB8888, tiling, &input_fb);
+	igt_assert(ret >= 0);
+
+	ret = igt_create_fb(display->drm_fd, width, height, writeback_format, tiling, &output_fb);
+	igt_assert(ret >= 0);
+
+	plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(plane, &input_fb);
+	igt_output_set_writeback_fb(output, &output_fb);
+
+	ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
+					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	igt_plane_set_fb(plane, NULL);
+	igt_remove_fb(display->drm_fd, &input_fb);
+	igt_remove_fb(display->drm_fd, &output_fb);
+
+	return !ret;
+}
+
+static igt_output_t *kms_writeback_get_output(igt_display_t *display)
+{
+	int i;
+
+	for (i = 0; i < display->n_outputs; i++) {
+		igt_output_t *output = &display->outputs[i];
+		int j;
+
+		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK)
+			continue;
+
+		kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_ON);
+
+		for (j = 0; j < igt_display_get_n_pipes(display); j++) {
+			igt_output_set_pipe(output, j);
+
+			if (check_writeback_config(display, output)) {
+				igt_debug("Using connector %u:%s on pipe %d\n",
+					  output->config.connector->connector_id,
+					  output->name, j);
+				return output;
+			}
+		}
+
+		/* Restore any connectors we don't use, so we don't trip on them later */
+		kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_UNSPECIFIED);
+	}
+
+	return NULL;
+}
+
+static void check_writeback_fb_id(igt_output_t *output)
+{
+	uint64_t check_fb_id;
+
+	check_fb_id = igt_output_get_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
+	igt_assert(check_fb_id == 0);
+}
+
+static int do_writeback_test(igt_output_t *output, uint32_t flags,
+			      uint32_t fb_id, int32_t *out_fence_ptr,
+			      bool ptr_valid)
+{
+	int ret;
+	igt_display_t *display = output->display;
+	struct kmstest_connector_config *config = &output->config;
+
+	igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID, config->crtc->crtc_id);
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb_id);
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR, (uint64_t)out_fence_ptr);
+
+	if (ptr_valid)
+		*out_fence_ptr = 0;
+
+	ret = igt_display_try_commit_atomic(display, flags, NULL);
+
+	if (ptr_valid)
+		igt_assert(*out_fence_ptr == -1);
+
+	/* WRITEBACK_FB_ID must always read as zero */
+	check_writeback_fb_id(output);
+
+	return ret;
+}
+
+static void invalid_out_fence(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *invalid_fb)
+{
+	int i, ret;
+	int32_t out_fence;
+	struct {
+		uint32_t fb_id;
+		bool ptr_valid;
+		int32_t *out_fence_ptr;
+	} invalid_tests[] = {
+		{
+			/* No output buffer, but the WRITEBACK_OUT_FENCE_PTR set. */
+			.fb_id = 0,
+			.ptr_valid = true,
+			.out_fence_ptr = &out_fence,
+		},
+		{
+			/* Invalid output buffer. */
+			.fb_id = invalid_fb->fb_id,
+			.ptr_valid = true,
+			.out_fence_ptr = &out_fence,
+		},
+		{
+			/* Invalid WRITEBACK_OUT_FENCE_PTR. */
+			.fb_id = valid_fb->fb_id,
+			.ptr_valid = false,
+			.out_fence_ptr = (int32_t *)0x8,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(invalid_tests); i++) {
+		ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+					invalid_tests[i].fb_id,
+					invalid_tests[i].out_fence_ptr,
+					invalid_tests[i].ptr_valid);
+		igt_assert(ret != 0);
+	}
+}
+
+static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *invalid_fb)
+{
+
+	int ret;
+
+	/* Valid output buffer */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				valid_fb->fb_id, NULL, false);
+	igt_assert(ret == 0);
+
+	/* Invalid object for WRITEBACK_FB_ID */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				output->id, NULL, false);
+	igt_assert(ret == -EINVAL);
+
+	/* Zero WRITEBACK_FB_ID */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				0, NULL, false);
+	igt_assert(ret == 0);
+}
+
+igt_main
+{
+	igt_display_t display;
+	igt_output_t *output;
+	igt_plane_t *plane;
+	igt_fb_t input_fb;
+	drmModeModeInfo mode;
+	int ret;
+
+	memset(&display, 0, sizeof(display));
+
+	igt_fixture {
+		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+		igt_display_require(&display, display.drm_fd);
+
+		kmstest_set_vt_graphics_mode();
+
+		igt_display_require(&display, display.drm_fd);
+
+		igt_require(display.is_atomic);
+
+		output = kms_writeback_get_output(&display);
+		igt_require(output);
+
+		if (output->use_override_mode)
+			memcpy(&mode, &output->override_mode, sizeof(mode));
+		else
+			memcpy(&mode, &output->config.default_mode, sizeof(mode));
+
+		plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(plane);
+
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay,
+				    mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &input_fb);
+		igt_assert(ret >= 0);
+		igt_plane_set_fb(plane, &input_fb);
+	}
+
+	igt_subtest("writeback-pixel-formats") {
+		drmModePropertyBlobRes *formats_blob = get_writeback_formats_blob(output);
+		const char *valid_chars = "0123456 ABCGNRUVXY";
+		unsigned int i;
+		char *c;
+
+		/*
+		 * We don't have a comprehensive list of formats, so just check
+		 * that the blob length is sensible and that it doesn't contain
+		 * any outlandish characters
+		 */
+		igt_assert(!(formats_blob->length % 4));
+		c = formats_blob->data;
+		for (i = 0; i < formats_blob->length; i++)
+			igt_assert_f(strchr(valid_chars, c[i]),
+				     "Unexpected character %c\n", c[i]);
+	}
+
+	igt_subtest("writeback-invalid-out-fence") {
+		igt_fb_t invalid_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay / 2,
+				    mode.vdisplay / 2,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &invalid_fb);
+		igt_require(ret > 0);
+
+		invalid_out_fence(output, &input_fb, &invalid_fb);
+
+		igt_remove_fb(display.drm_fd, &invalid_fb);
+	}
+
+	igt_subtest("writeback-fb-id") {
+		igt_fb_t output_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		writeback_fb_id(output, &input_fb, &output_fb);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
+	igt_fixture {
+		igt_remove_fb(display.drm_fd, &input_fb);
+		igt_display_fini(&display);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index e14ab2b45..41e78b8ee 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -60,6 +60,7 @@ test_progs = [
 	'kms_tv_load_detect',
 	'kms_universal_plane',
 	'kms_vblank',
+	'kms_writeback',
 	'meta_test',
 	'perf',
 	'pm_backlight',
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 2/6] kms_writeback: Add initial writeback tests
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

Add tests for the WRITEBACK_PIXEL_FORMATS, WRITEBACK_OUT_FENCE_PTR and
WRITEBACK_FB_ID properties on writeback connectors, ensuring their
behaviour is correct.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated do_writeback_test() function to address feedback]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_writeback.c  | 314 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 3 files changed, 316 insertions(+)
 create mode 100644 tests/kms_writeback.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 519eac792..4438d2165 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs = \
 	kms_universal_plane \
 	kms_vblank \
 	kms_sequence \
+	kms_writeback \
 	meta_test \
 	perf \
 	perf_pmu \
diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
new file mode 100644
index 000000000..66ef48a6c
--- /dev/null
+++ b/tests/kms_writeback.c
@@ -0,0 +1,314 @@
+/*
+ * (C) COPYRIGHT 2017 ARM Limited. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "igt.h"
+#include "igt_core.h"
+#include "igt_fb.h"
+
+static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
+{
+	drmModePropertyBlobRes *blob = NULL;
+	uint64_t blob_id;
+	int ret;
+
+	ret = kmstest_get_property(output->display->drm_fd,
+				   output->config.connector->connector_id,
+				   DRM_MODE_OBJECT_CONNECTOR,
+				   igt_connector_prop_names[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS],
+				   NULL, &blob_id, NULL);
+	if (ret)
+		blob = drmModeGetPropertyBlob(output->display->drm_fd, blob_id);
+
+	igt_assert(blob);
+
+	return blob;
+}
+
+static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
+{
+	igt_fb_t input_fb, output_fb;
+	igt_plane_t *plane;
+	uint32_t writeback_format = DRM_FORMAT_XRGB8888;
+	uint64_t tiling = igt_fb_mod_to_tiling(0);
+	int width, height, ret;
+	drmModeModeInfo override_mode = {
+		.clock = 25175,
+		.hdisplay = 640,
+		.hsync_start = 656,
+		.hsync_end = 752,
+		.htotal = 800,
+		.hskew = 0,
+		.vdisplay = 480,
+		.vsync_start = 490,
+		.vsync_end = 492,
+		.vtotal = 525,
+		.vscan = 0,
+		.vrefresh = 60,
+		.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+		.name = {"640x480-60"},
+	};
+	igt_output_override_mode(output, &override_mode);
+
+	width = override_mode.hdisplay;
+	height = override_mode.vdisplay;
+
+	ret = igt_create_fb(display->drm_fd, width, height, DRM_FORMAT_XRGB8888, tiling, &input_fb);
+	igt_assert(ret >= 0);
+
+	ret = igt_create_fb(display->drm_fd, width, height, writeback_format, tiling, &output_fb);
+	igt_assert(ret >= 0);
+
+	plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(plane, &input_fb);
+	igt_output_set_writeback_fb(output, &output_fb);
+
+	ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
+					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	igt_plane_set_fb(plane, NULL);
+	igt_remove_fb(display->drm_fd, &input_fb);
+	igt_remove_fb(display->drm_fd, &output_fb);
+
+	return !ret;
+}
+
+static igt_output_t *kms_writeback_get_output(igt_display_t *display)
+{
+	int i;
+
+	for (i = 0; i < display->n_outputs; i++) {
+		igt_output_t *output = &display->outputs[i];
+		int j;
+
+		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK)
+			continue;
+
+		kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_ON);
+
+		for (j = 0; j < igt_display_get_n_pipes(display); j++) {
+			igt_output_set_pipe(output, j);
+
+			if (check_writeback_config(display, output)) {
+				igt_debug("Using connector %u:%s on pipe %d\n",
+					  output->config.connector->connector_id,
+					  output->name, j);
+				return output;
+			}
+		}
+
+		/* Restore any connectors we don't use, so we don't trip on them later */
+		kmstest_force_connector(display->drm_fd, output->config.connector, FORCE_CONNECTOR_UNSPECIFIED);
+	}
+
+	return NULL;
+}
+
+static void check_writeback_fb_id(igt_output_t *output)
+{
+	uint64_t check_fb_id;
+
+	check_fb_id = igt_output_get_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
+	igt_assert(check_fb_id == 0);
+}
+
+static int do_writeback_test(igt_output_t *output, uint32_t flags,
+			      uint32_t fb_id, int32_t *out_fence_ptr,
+			      bool ptr_valid)
+{
+	int ret;
+	igt_display_t *display = output->display;
+	struct kmstest_connector_config *config = &output->config;
+
+	igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID, config->crtc->crtc_id);
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb_id);
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR, (uint64_t)out_fence_ptr);
+
+	if (ptr_valid)
+		*out_fence_ptr = 0;
+
+	ret = igt_display_try_commit_atomic(display, flags, NULL);
+
+	if (ptr_valid)
+		igt_assert(*out_fence_ptr == -1);
+
+	/* WRITEBACK_FB_ID must always read as zero */
+	check_writeback_fb_id(output);
+
+	return ret;
+}
+
+static void invalid_out_fence(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *invalid_fb)
+{
+	int i, ret;
+	int32_t out_fence;
+	struct {
+		uint32_t fb_id;
+		bool ptr_valid;
+		int32_t *out_fence_ptr;
+	} invalid_tests[] = {
+		{
+			/* No output buffer, but the WRITEBACK_OUT_FENCE_PTR set. */
+			.fb_id = 0,
+			.ptr_valid = true,
+			.out_fence_ptr = &out_fence,
+		},
+		{
+			/* Invalid output buffer. */
+			.fb_id = invalid_fb->fb_id,
+			.ptr_valid = true,
+			.out_fence_ptr = &out_fence,
+		},
+		{
+			/* Invalid WRITEBACK_OUT_FENCE_PTR. */
+			.fb_id = valid_fb->fb_id,
+			.ptr_valid = false,
+			.out_fence_ptr = (int32_t *)0x8,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(invalid_tests); i++) {
+		ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+					invalid_tests[i].fb_id,
+					invalid_tests[i].out_fence_ptr,
+					invalid_tests[i].ptr_valid);
+		igt_assert(ret != 0);
+	}
+}
+
+static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *invalid_fb)
+{
+
+	int ret;
+
+	/* Valid output buffer */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				valid_fb->fb_id, NULL, false);
+	igt_assert(ret == 0);
+
+	/* Invalid object for WRITEBACK_FB_ID */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				output->id, NULL, false);
+	igt_assert(ret == -EINVAL);
+
+	/* Zero WRITEBACK_FB_ID */
+	ret = do_writeback_test(output, DRM_MODE_ATOMIC_ALLOW_MODESET,
+				0, NULL, false);
+	igt_assert(ret == 0);
+}
+
+igt_main
+{
+	igt_display_t display;
+	igt_output_t *output;
+	igt_plane_t *plane;
+	igt_fb_t input_fb;
+	drmModeModeInfo mode;
+	int ret;
+
+	memset(&display, 0, sizeof(display));
+
+	igt_fixture {
+		display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+		igt_display_require(&display, display.drm_fd);
+
+		kmstest_set_vt_graphics_mode();
+
+		igt_display_require(&display, display.drm_fd);
+
+		igt_require(display.is_atomic);
+
+		output = kms_writeback_get_output(&display);
+		igt_require(output);
+
+		if (output->use_override_mode)
+			memcpy(&mode, &output->override_mode, sizeof(mode));
+		else
+			memcpy(&mode, &output->config.default_mode, sizeof(mode));
+
+		plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(plane);
+
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay,
+				    mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &input_fb);
+		igt_assert(ret >= 0);
+		igt_plane_set_fb(plane, &input_fb);
+	}
+
+	igt_subtest("writeback-pixel-formats") {
+		drmModePropertyBlobRes *formats_blob = get_writeback_formats_blob(output);
+		const char *valid_chars = "0123456 ABCGNRUVXY";
+		unsigned int i;
+		char *c;
+
+		/*
+		 * We don't have a comprehensive list of formats, so just check
+		 * that the blob length is sensible and that it doesn't contain
+		 * any outlandish characters
+		 */
+		igt_assert(!(formats_blob->length % 4));
+		c = formats_blob->data;
+		for (i = 0; i < formats_blob->length; i++)
+			igt_assert_f(strchr(valid_chars, c[i]),
+				     "Unexpected character %c\n", c[i]);
+	}
+
+	igt_subtest("writeback-invalid-out-fence") {
+		igt_fb_t invalid_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay / 2,
+				    mode.vdisplay / 2,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &invalid_fb);
+		igt_require(ret > 0);
+
+		invalid_out_fence(output, &input_fb, &invalid_fb);
+
+		igt_remove_fb(display.drm_fd, &invalid_fb);
+	}
+
+	igt_subtest("writeback-fb-id") {
+		igt_fb_t output_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		writeback_fb_id(output, &input_fb, &output_fb);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
+	igt_fixture {
+		igt_remove_fb(display.drm_fd, &input_fb);
+		igt_display_fini(&display);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index e14ab2b45..41e78b8ee 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -60,6 +60,7 @@ test_progs = [
 	'kms_tv_load_detect',
 	'kms_universal_plane',
 	'kms_vblank',
+	'kms_writeback',
 	'meta_test',
 	'perf',
 	'pm_backlight',
-- 
2.20.1

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

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

* [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

To use writeback buffers as a CRC source, we need to be able to hash
them. Implement a simple FVA-1a hashing routine for this purpose.

Doing a bytewise hash on the framebuffer directly can be very slow if
the memory is noncached. By making a copy of each line in the FB first
(which can take advantage of word-access speedup), we can do the hash
on a cached copy, which is much faster (10x speedup on my platform).

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the most recent API]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_fb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  3 +++
 2 files changed, 69 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5cd1829a3..11e200b53 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2379,6 +2379,72 @@ bool igt_fb_supported_format(uint32_t drm_format)
 	return false;
 }
 
+/*
+ * This implements the FNV-1a hashing algorithm instead of CRC, for
+ * simplicity
+ * http://www.isthe.com/chongo/tech/comp/fnv/index.html
+ *
+ * hash = offset_basis
+ * for each octet_of_data to be hashed
+ *         hash = hash xor octet_of_data
+ *         hash = hash * FNV_prime
+ * return hash
+ *
+ * 32 bit offset_basis = 2166136261
+ * 32 bit FNV_prime = 224 + 28 + 0x93 = 16777619
+ */
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
+{
+#define FNV1a_OFFSET_BIAS 2166136261
+#define FNV1a_PRIME 16777619
+	uint32_t hash;
+	void *map;
+	char *ptr, *line = NULL;
+	int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
+	uint32_t stride = calc_plane_stride(fb, 0);
+
+	if (fb->is_dumb)
+		map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
+					      PROT_READ);
+	else
+		map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
+				    PROT_READ);
+	ptr = map;
+
+	/*
+	 * Framebuffers are often uncached, which can make byte-wise accesses
+	 * very slow. We copy each line of the FB into a local buffer to speed
+	 * up the hashing.
+	 */
+	line = malloc(stride);
+	if (!line) {
+		munmap(map, fb->size);
+		return -ENOMEM;
+	}
+
+	hash = FNV1a_OFFSET_BIAS;
+
+	for (y = 0; y < fb->height; y++, ptr += stride) {
+
+		memcpy(line, ptr, stride);
+
+		for (x = 0; x < fb->width * cpp; x++) {
+			hash ^= line[x];
+			hash *= FNV1a_PRIME;
+		}
+	}
+
+	crc->n_words = 1;
+	crc->crc[0] = hash;
+
+	free(line);
+	munmap(map, fb->size);
+
+	return 0;
+#undef FNV1a_OFFSET_BIAS
+#undef FNV1a_PRIME
+}
+
 /**
  * igt_format_is_yuv:
  * @drm_format: drm fourcc
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 9f027deba..948c5380c 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -37,6 +37,7 @@
 #include <i915_drm.h>
 
 #include "igt_color_encoding.h"
+#include "igt_debugfs.h"
 
 /**
  * igt_fb_t:
@@ -173,5 +174,7 @@ const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
 
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc);
+
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

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

* [Intel-gfx] [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

To use writeback buffers as a CRC source, we need to be able to hash
them. Implement a simple FVA-1a hashing routine for this purpose.

Doing a bytewise hash on the framebuffer directly can be very slow if
the memory is noncached. By making a copy of each line in the FB first
(which can take advantage of word-access speedup), we can do the hash
on a cached copy, which is much faster (10x speedup on my platform).

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the most recent API]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_fb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  3 +++
 2 files changed, 69 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5cd1829a3..11e200b53 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2379,6 +2379,72 @@ bool igt_fb_supported_format(uint32_t drm_format)
 	return false;
 }
 
+/*
+ * This implements the FNV-1a hashing algorithm instead of CRC, for
+ * simplicity
+ * http://www.isthe.com/chongo/tech/comp/fnv/index.html
+ *
+ * hash = offset_basis
+ * for each octet_of_data to be hashed
+ *         hash = hash xor octet_of_data
+ *         hash = hash * FNV_prime
+ * return hash
+ *
+ * 32 bit offset_basis = 2166136261
+ * 32 bit FNV_prime = 224 + 28 + 0x93 = 16777619
+ */
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
+{
+#define FNV1a_OFFSET_BIAS 2166136261
+#define FNV1a_PRIME 16777619
+	uint32_t hash;
+	void *map;
+	char *ptr, *line = NULL;
+	int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
+	uint32_t stride = calc_plane_stride(fb, 0);
+
+	if (fb->is_dumb)
+		map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
+					      PROT_READ);
+	else
+		map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
+				    PROT_READ);
+	ptr = map;
+
+	/*
+	 * Framebuffers are often uncached, which can make byte-wise accesses
+	 * very slow. We copy each line of the FB into a local buffer to speed
+	 * up the hashing.
+	 */
+	line = malloc(stride);
+	if (!line) {
+		munmap(map, fb->size);
+		return -ENOMEM;
+	}
+
+	hash = FNV1a_OFFSET_BIAS;
+
+	for (y = 0; y < fb->height; y++, ptr += stride) {
+
+		memcpy(line, ptr, stride);
+
+		for (x = 0; x < fb->width * cpp; x++) {
+			hash ^= line[x];
+			hash *= FNV1a_PRIME;
+		}
+	}
+
+	crc->n_words = 1;
+	crc->crc[0] = hash;
+
+	free(line);
+	munmap(map, fb->size);
+
+	return 0;
+#undef FNV1a_OFFSET_BIAS
+#undef FNV1a_PRIME
+}
+
 /**
  * igt_format_is_yuv:
  * @drm_format: drm fourcc
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 9f027deba..948c5380c 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -37,6 +37,7 @@
 #include <i915_drm.h>
 
 #include "igt_color_encoding.h"
+#include "igt_debugfs.h"
 
 /**
  * igt_fb_t:
@@ -173,5 +174,7 @@ const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
 
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc);
+
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

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

* [PATCH i-g-t v5 4/6] kms_writeback: Add writeback-check-output
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

Add a test which makes commits using the writeback connector, and
checks the output buffer hash to make sure it is/isn't written as
appropriate.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 tests/kms_writeback.c | 124 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 66ef48a6c..0f20dadd2 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -30,6 +30,7 @@
 #include "igt.h"
 #include "igt_core.h"
 #include "igt_fb.h"
+#include "sw_sync.h"
 
 static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
 {
@@ -221,6 +222,116 @@ static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *
 	igt_assert(ret == 0);
 }
 
+static void fill_fb(igt_fb_t *fb, double color[3])
+{
+	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
+	igt_assert(cr);
+
+	igt_paint_color(cr, 0, 0, fb->width, fb->height,
+			color[0], color[1], color[2]);
+}
+
+static void get_and_wait_out_fence(igt_output_t *output)
+{
+	int ret;
+
+	igt_assert(output->writeback_out_fence_fd >= 0);
+
+	ret = sync_fence_wait(output->writeback_out_fence_fd, 1000);
+	igt_assert(ret == 0);
+	close(output->writeback_out_fence_fd);
+	output->writeback_out_fence_fd = -1;
+}
+
+static void writeback_sequence(igt_output_t *output, igt_plane_t *plane,
+				igt_fb_t *in_fb, igt_fb_t *out_fbs[], int n_commits)
+{
+	int i, color_idx = 0;
+	double in_fb_colors[2][3] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+	};
+	double clear_color[3] = { 1.0, 1.0, 1.0 };
+	igt_crc_t cleared_crc, out_expected;
+
+	for (i = 0; i < n_commits; i++, color_idx++) {
+		/* Change the input color each time */
+		fill_fb(in_fb, in_fb_colors[color_idx % 2]);
+
+		if (out_fbs[i]) {
+			igt_crc_t out_before;
+
+			/* Get the expected CRC */
+			fill_fb(out_fbs[i], in_fb_colors[color_idx % 2]);
+			igt_fb_get_crc(out_fbs[i], &out_expected);
+
+			fill_fb(out_fbs[i], clear_color);
+			if (i == 0)
+				igt_fb_get_crc(out_fbs[i], &cleared_crc);
+			igt_fb_get_crc(out_fbs[i], &out_before);
+			igt_assert_crc_equal(&cleared_crc, &out_before);
+		}
+
+		/* Commit */
+		igt_plane_set_fb(plane, in_fb);
+		igt_output_set_writeback_fb(output, out_fbs[i]);
+
+		igt_display_commit_atomic(output->display,
+					  DRM_MODE_ATOMIC_ALLOW_MODESET,
+					  NULL);
+		if (out_fbs[i])
+			get_and_wait_out_fence(output);
+
+		/* Make sure the old output buffer is untouched */
+		if (i > 0 && out_fbs[i - 1] && (out_fbs[i] != out_fbs[i - 1])) {
+			igt_crc_t out_prev;
+			igt_fb_get_crc(out_fbs[i - 1], &out_prev);
+			igt_assert_crc_equal(&cleared_crc, &out_prev);
+		}
+
+		/* Make sure this output buffer is written */
+		if (out_fbs[i]) {
+			igt_crc_t out_after;
+			igt_fb_get_crc(out_fbs[i], &out_after);
+			igt_assert_crc_equal(&out_expected, &out_after);
+
+			/* And clear it, for the next time */
+			fill_fb(out_fbs[i], clear_color);
+		}
+	}
+}
+
+static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
+				   igt_fb_t *input_fb, igt_fb_t *output_fb)
+{
+	igt_fb_t *out_fbs[2] = { 0 };
+	igt_fb_t second_out_fb;
+	int ret;
+
+	/* One commit, with a writeback. */
+	writeback_sequence(output, plane, input_fb, &output_fb, 1);
+
+	/* Two commits, the second with no writeback */
+	out_fbs[0] = output_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	/* Two commits, both with writeback */
+	out_fbs[1] = output_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	ret = igt_create_fb(output_fb->fd, output_fb->width, output_fb->height,
+			    DRM_FORMAT_XRGB8888,
+			    igt_fb_mod_to_tiling(0),
+			    &second_out_fb);
+	igt_require(ret > 0);
+
+	/* Two commits, with different writeback buffers */
+	out_fbs[1] = &second_out_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	igt_remove_fb(output_fb->fd, &second_out_fb);
+}
+
 igt_main
 {
 	igt_display_t display;
@@ -307,6 +418,19 @@ igt_main
 		igt_remove_fb(display.drm_fd, &output_fb);
 	}
 
+	igt_subtest("writeback-check-output") {
+		igt_fb_t output_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		writeback_check_output(output, plane, &input_fb, &output_fb);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
 	igt_fixture {
 		igt_remove_fb(display.drm_fd, &input_fb);
 		igt_display_fini(&display);
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 4/6] kms_writeback: Add writeback-check-output
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

Add a test which makes commits using the writeback connector, and
checks the output buffer hash to make sure it is/isn't written as
appropriate.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 tests/kms_writeback.c | 124 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 66ef48a6c..0f20dadd2 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -30,6 +30,7 @@
 #include "igt.h"
 #include "igt_core.h"
 #include "igt_fb.h"
+#include "sw_sync.h"
 
 static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
 {
@@ -221,6 +222,116 @@ static void writeback_fb_id(igt_output_t *output, igt_fb_t *valid_fb, igt_fb_t *
 	igt_assert(ret == 0);
 }
 
+static void fill_fb(igt_fb_t *fb, double color[3])
+{
+	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
+	igt_assert(cr);
+
+	igt_paint_color(cr, 0, 0, fb->width, fb->height,
+			color[0], color[1], color[2]);
+}
+
+static void get_and_wait_out_fence(igt_output_t *output)
+{
+	int ret;
+
+	igt_assert(output->writeback_out_fence_fd >= 0);
+
+	ret = sync_fence_wait(output->writeback_out_fence_fd, 1000);
+	igt_assert(ret == 0);
+	close(output->writeback_out_fence_fd);
+	output->writeback_out_fence_fd = -1;
+}
+
+static void writeback_sequence(igt_output_t *output, igt_plane_t *plane,
+				igt_fb_t *in_fb, igt_fb_t *out_fbs[], int n_commits)
+{
+	int i, color_idx = 0;
+	double in_fb_colors[2][3] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+	};
+	double clear_color[3] = { 1.0, 1.0, 1.0 };
+	igt_crc_t cleared_crc, out_expected;
+
+	for (i = 0; i < n_commits; i++, color_idx++) {
+		/* Change the input color each time */
+		fill_fb(in_fb, in_fb_colors[color_idx % 2]);
+
+		if (out_fbs[i]) {
+			igt_crc_t out_before;
+
+			/* Get the expected CRC */
+			fill_fb(out_fbs[i], in_fb_colors[color_idx % 2]);
+			igt_fb_get_crc(out_fbs[i], &out_expected);
+
+			fill_fb(out_fbs[i], clear_color);
+			if (i == 0)
+				igt_fb_get_crc(out_fbs[i], &cleared_crc);
+			igt_fb_get_crc(out_fbs[i], &out_before);
+			igt_assert_crc_equal(&cleared_crc, &out_before);
+		}
+
+		/* Commit */
+		igt_plane_set_fb(plane, in_fb);
+		igt_output_set_writeback_fb(output, out_fbs[i]);
+
+		igt_display_commit_atomic(output->display,
+					  DRM_MODE_ATOMIC_ALLOW_MODESET,
+					  NULL);
+		if (out_fbs[i])
+			get_and_wait_out_fence(output);
+
+		/* Make sure the old output buffer is untouched */
+		if (i > 0 && out_fbs[i - 1] && (out_fbs[i] != out_fbs[i - 1])) {
+			igt_crc_t out_prev;
+			igt_fb_get_crc(out_fbs[i - 1], &out_prev);
+			igt_assert_crc_equal(&cleared_crc, &out_prev);
+		}
+
+		/* Make sure this output buffer is written */
+		if (out_fbs[i]) {
+			igt_crc_t out_after;
+			igt_fb_get_crc(out_fbs[i], &out_after);
+			igt_assert_crc_equal(&out_expected, &out_after);
+
+			/* And clear it, for the next time */
+			fill_fb(out_fbs[i], clear_color);
+		}
+	}
+}
+
+static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
+				   igt_fb_t *input_fb, igt_fb_t *output_fb)
+{
+	igt_fb_t *out_fbs[2] = { 0 };
+	igt_fb_t second_out_fb;
+	int ret;
+
+	/* One commit, with a writeback. */
+	writeback_sequence(output, plane, input_fb, &output_fb, 1);
+
+	/* Two commits, the second with no writeback */
+	out_fbs[0] = output_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	/* Two commits, both with writeback */
+	out_fbs[1] = output_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	ret = igt_create_fb(output_fb->fd, output_fb->width, output_fb->height,
+			    DRM_FORMAT_XRGB8888,
+			    igt_fb_mod_to_tiling(0),
+			    &second_out_fb);
+	igt_require(ret > 0);
+
+	/* Two commits, with different writeback buffers */
+	out_fbs[1] = &second_out_fb;
+	writeback_sequence(output, plane, input_fb, out_fbs, 2);
+
+	igt_remove_fb(output_fb->fd, &second_out_fb);
+}
+
 igt_main
 {
 	igt_display_t display;
@@ -307,6 +418,19 @@ igt_main
 		igt_remove_fb(display.drm_fd, &output_fb);
 	}
 
+	igt_subtest("writeback-check-output") {
+		igt_fb_t output_fb;
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		writeback_check_output(output, plane, &input_fb, &output_fb);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
 	igt_fixture {
 		igt_remove_fb(display.drm_fd, &input_fb);
 		igt_display_fini(&display);
-- 
2.20.1

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

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

* [PATCH i-g-t v5 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

An output can be added as a clone of any other output(s) attached to a
pipe using igt_output_clone_pipe()

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 lib/igt_kms.c | 100 +++++++++++++++++++++++++++++++-------------------
 lib/igt_kms.h |   5 +++
 2 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 0bc2996cd..fe51a6c69 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1694,6 +1694,17 @@ static void igt_display_log_shift(igt_display_t *display, int shift)
 	igt_assert(display->log_shift >= 0);
 }
 
+static int igt_output_idx(igt_output_t *output)
+{
+	int i;
+
+	for (i = 0; i < output->display->n_outputs; i++)
+		if (&output->display->outputs[i] == output)
+			return i;
+
+	return -1;
+}
+
 static void igt_output_refresh(igt_output_t *output)
 {
 	igt_display_t *display = output->display;
@@ -2190,42 +2201,6 @@ void igt_display_fini(igt_display_t *display)
 	display->pipes = NULL;
 }
 
-static void igt_display_refresh(igt_display_t *display)
-{
-	igt_output_t *output;
-	int i;
-
-	unsigned long pipes_in_use = 0;
-
-       /* Check that two outputs aren't trying to use the same pipe */
-	for (i = 0; i < display->n_outputs; i++) {
-		output = &display->outputs[i];
-
-		if (output->pending_pipe != PIPE_NONE) {
-			if (pipes_in_use & (1 << output->pending_pipe))
-				goto report_dup;
-
-			pipes_in_use |= 1 << output->pending_pipe;
-		}
-
-		if (output->force_reprobe)
-			igt_output_refresh(output);
-	}
-
-	return;
-
-report_dup:
-	for (; i > 0; i--) {
-		igt_output_t *b = &display->outputs[i - 1];
-
-		igt_assert_f(output->pending_pipe !=
-			     b->pending_pipe,
-			     "%s and %s are both trying to use pipe %s\n",
-			     igt_output_name(output), igt_output_name(b),
-			     kmstest_pipe_name(output->pending_pipe));
-	}
-}
-
 static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 {
 	igt_display_t *display = output->display;
@@ -2249,6 +2224,40 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 	return &display->pipes[pipe];
 }
 
+static void igt_display_refresh(igt_display_t *display)
+{
+	igt_output_t *output;
+	igt_pipe_t *pipe;
+	int i;
+
+	unsigned long pipes_in_use = 0;
+	unsigned long pending_crtc_idx_mask;
+
+	/* Check that outputs and pipes agree wrt. cloning */
+	for (i = 0; i < display->n_outputs; i++) {
+		output = &display->outputs[i];
+		pending_crtc_idx_mask = 1 << output->pending_pipe;
+
+		pipe = igt_output_get_driving_pipe(output);
+		if (pipe) {
+			igt_assert_f(pipe->outputs & (1 << igt_output_idx(output)),
+				     "Output %s not expected to be using pipe %s\n",
+				     igt_output_name(output),
+				     kmstest_pipe_name(pipe->pipe));
+
+			if (pipes_in_use & pending_crtc_idx_mask)
+				LOG(display, "Output %s clones pipe %s\n",
+				    igt_output_name(output),
+				    kmstest_pipe_name(pipe->pipe));
+		}
+
+		pipes_in_use |= pending_crtc_idx_mask;
+
+		if (output->force_reprobe)
+			igt_output_refresh(output);
+	}
+}
+
 static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int plane_idx)
 {
 	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
@@ -3587,6 +3596,7 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
 	output->use_override_mode = !!mode;
 
 	if (pipe) {
+		igt_debug("overriding pipe mode in %s way\n", output->display->is_atomic ? "atomic" : "legacy");
 		if (output->display->is_atomic)
 			igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
 		else
@@ -3594,6 +3604,16 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
 	}
 }
 
+void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe)
+{
+	igt_display_t *display = output->display;
+	uint32_t current_clones = display->pipes[pipe].outputs;
+
+	igt_output_set_pipe(output, pipe);
+
+	display->pipes[pipe].outputs |= current_clones;
+}
+
 /*
  * igt_output_set_pipe:
  * @output: Target output for which the pipe is being set to
@@ -3610,11 +3630,15 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
 
 	igt_assert(output->name);
 
-	if (output->pending_pipe != PIPE_NONE)
+	if (output->pending_pipe != PIPE_NONE) {
 		old_pipe = igt_output_get_driving_pipe(output);
+		old_pipe->outputs &= ~(1 << igt_output_idx(output));
+	}
 
-	if (pipe != PIPE_NONE)
+	if (pipe != PIPE_NONE) {
 		pipe_obj = &display->pipes[pipe];
+		pipe_obj->outputs = (1 << igt_output_idx(output));
+	}
 
 	LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
 	    kmstest_pipe_name(pipe));
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 13d3a9ceb..acb27982b 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -349,6 +349,9 @@ struct igt_pipe {
 	uint32_t crtc_id;
 
 	int32_t out_fence_fd;
+	bool out_fence_requested;
+
+	uint32_t outputs;
 };
 
 typedef struct {
@@ -404,6 +407,8 @@ const char *igt_output_name(igt_output_t *output);
 drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
 void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
+void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe);
+
 igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

An output can be added as a clone of any other output(s) attached to a
pipe using igt_output_clone_pipe()

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
 lib/igt_kms.c | 100 +++++++++++++++++++++++++++++++-------------------
 lib/igt_kms.h |   5 +++
 2 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 0bc2996cd..fe51a6c69 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1694,6 +1694,17 @@ static void igt_display_log_shift(igt_display_t *display, int shift)
 	igt_assert(display->log_shift >= 0);
 }
 
+static int igt_output_idx(igt_output_t *output)
+{
+	int i;
+
+	for (i = 0; i < output->display->n_outputs; i++)
+		if (&output->display->outputs[i] == output)
+			return i;
+
+	return -1;
+}
+
 static void igt_output_refresh(igt_output_t *output)
 {
 	igt_display_t *display = output->display;
@@ -2190,42 +2201,6 @@ void igt_display_fini(igt_display_t *display)
 	display->pipes = NULL;
 }
 
-static void igt_display_refresh(igt_display_t *display)
-{
-	igt_output_t *output;
-	int i;
-
-	unsigned long pipes_in_use = 0;
-
-       /* Check that two outputs aren't trying to use the same pipe */
-	for (i = 0; i < display->n_outputs; i++) {
-		output = &display->outputs[i];
-
-		if (output->pending_pipe != PIPE_NONE) {
-			if (pipes_in_use & (1 << output->pending_pipe))
-				goto report_dup;
-
-			pipes_in_use |= 1 << output->pending_pipe;
-		}
-
-		if (output->force_reprobe)
-			igt_output_refresh(output);
-	}
-
-	return;
-
-report_dup:
-	for (; i > 0; i--) {
-		igt_output_t *b = &display->outputs[i - 1];
-
-		igt_assert_f(output->pending_pipe !=
-			     b->pending_pipe,
-			     "%s and %s are both trying to use pipe %s\n",
-			     igt_output_name(output), igt_output_name(b),
-			     kmstest_pipe_name(output->pending_pipe));
-	}
-}
-
 static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 {
 	igt_display_t *display = output->display;
@@ -2249,6 +2224,40 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 	return &display->pipes[pipe];
 }
 
+static void igt_display_refresh(igt_display_t *display)
+{
+	igt_output_t *output;
+	igt_pipe_t *pipe;
+	int i;
+
+	unsigned long pipes_in_use = 0;
+	unsigned long pending_crtc_idx_mask;
+
+	/* Check that outputs and pipes agree wrt. cloning */
+	for (i = 0; i < display->n_outputs; i++) {
+		output = &display->outputs[i];
+		pending_crtc_idx_mask = 1 << output->pending_pipe;
+
+		pipe = igt_output_get_driving_pipe(output);
+		if (pipe) {
+			igt_assert_f(pipe->outputs & (1 << igt_output_idx(output)),
+				     "Output %s not expected to be using pipe %s\n",
+				     igt_output_name(output),
+				     kmstest_pipe_name(pipe->pipe));
+
+			if (pipes_in_use & pending_crtc_idx_mask)
+				LOG(display, "Output %s clones pipe %s\n",
+				    igt_output_name(output),
+				    kmstest_pipe_name(pipe->pipe));
+		}
+
+		pipes_in_use |= pending_crtc_idx_mask;
+
+		if (output->force_reprobe)
+			igt_output_refresh(output);
+	}
+}
+
 static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, int plane_idx)
 {
 	igt_require_f(plane_idx >= 0 && plane_idx < pipe->n_planes,
@@ -3587,6 +3596,7 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
 	output->use_override_mode = !!mode;
 
 	if (pipe) {
+		igt_debug("overriding pipe mode in %s way\n", output->display->is_atomic ? "atomic" : "legacy");
 		if (output->display->is_atomic)
 			igt_pipe_obj_replace_prop_blob(pipe, IGT_CRTC_MODE_ID, igt_output_get_mode(output), sizeof(*mode));
 		else
@@ -3594,6 +3604,16 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
 	}
 }
 
+void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe)
+{
+	igt_display_t *display = output->display;
+	uint32_t current_clones = display->pipes[pipe].outputs;
+
+	igt_output_set_pipe(output, pipe);
+
+	display->pipes[pipe].outputs |= current_clones;
+}
+
 /*
  * igt_output_set_pipe:
  * @output: Target output for which the pipe is being set to
@@ -3610,11 +3630,15 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
 
 	igt_assert(output->name);
 
-	if (output->pending_pipe != PIPE_NONE)
+	if (output->pending_pipe != PIPE_NONE) {
 		old_pipe = igt_output_get_driving_pipe(output);
+		old_pipe->outputs &= ~(1 << igt_output_idx(output));
+	}
 
-	if (pipe != PIPE_NONE)
+	if (pipe != PIPE_NONE) {
 		pipe_obj = &display->pipes[pipe];
+		pipe_obj->outputs = (1 << igt_output_idx(output));
+	}
 
 	LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
 	    kmstest_pipe_name(pipe));
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 13d3a9ceb..acb27982b 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -349,6 +349,9 @@ struct igt_pipe {
 	uint32_t crtc_id;
 
 	int32_t out_fence_fd;
+	bool out_fence_requested;
+
+	uint32_t outputs;
 };
 
 typedef struct {
@@ -404,6 +407,8 @@ const char *igt_output_name(igt_output_t *output);
 drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
 void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
+void igt_output_clone_pipe(igt_output_t *output, enum pipe pipe);
+
 igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
-- 
2.20.1

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

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

* [PATCH i-g-t v5 6/6] kms_writeback: Add tests using a cloned output
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-01-15 17:47   ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

Update the connector search to also optionally attempt to find a
non-writeback connector to clone to.

Add a subtest which is the same as writeback-check-output, but also
clones to the second connector.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and addressed comments by Maarten]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 tests/kms_writeback.c | 64 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 0f20dadd2..ae536bbfa 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -51,7 +51,8 @@ static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
 	return blob;
 }
 
-static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
+static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
+				   int pipe, igt_output_t **clone)
 {
 	igt_fb_t input_fb, output_fb;
 	igt_plane_t *plane;
@@ -91,6 +92,30 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
 
 	ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
 					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	if (!ret && clone) {
+		/* Try and find a clone */
+		int i, newret;
+		*clone = NULL;
+
+		for (i = 0; i < display->n_outputs; i++) {
+			igt_output_t *second_output = &display->outputs[i];
+			if (output != second_output &&
+			    igt_pipe_connector_valid(pipe, second_output)) {
+
+				igt_output_clone_pipe(second_output, pipe);
+				igt_output_override_mode(output, &override_mode);
+				newret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
+								       DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+				igt_output_set_pipe(second_output, PIPE_NONE);
+				igt_debug("try_commit_atomic clone returned %d\n", newret);
+				if (!newret) {
+					*clone = second_output;
+					igt_debug("Selected clone %s\n", (*clone)->name);
+					break;
+				}
+			}
+		}
+	}
 	igt_plane_set_fb(plane, NULL);
 	igt_remove_fb(display->drm_fd, &input_fb);
 	igt_remove_fb(display->drm_fd, &output_fb);
@@ -98,7 +123,8 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
 	return !ret;
 }
 
-static igt_output_t *kms_writeback_get_output(igt_display_t *display)
+static igt_output_t *kms_writeback_get_output(igt_display_t *display, enum pipe *pipe,
+					      igt_output_t **clone)
 {
 	int i;
 
@@ -114,10 +140,16 @@ static igt_output_t *kms_writeback_get_output(igt_display_t *display)
 		for (j = 0; j < igt_display_get_n_pipes(display); j++) {
 			igt_output_set_pipe(output, j);
 
-			if (check_writeback_config(display, output)) {
+			if (check_writeback_config(display, output, j, clone)) {
 				igt_debug("Using connector %u:%s on pipe %d\n",
 					  output->config.connector->connector_id,
 					  output->name, j);
+				if (clone && *clone)
+					igt_debug("Cloning to connector %u:%s\n",
+						  (*clone)->config.connector->connector_id,
+						  (*clone)->name);
+				if (pipe)
+					*pipe = j;
 				return output;
 			}
 		}
@@ -335,10 +367,11 @@ static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
 igt_main
 {
 	igt_display_t display;
-	igt_output_t *output;
+	igt_output_t *output, *clone;
 	igt_plane_t *plane;
 	igt_fb_t input_fb;
 	drmModeModeInfo mode;
+	enum pipe pipe;
 	int ret;
 
 	memset(&display, 0, sizeof(display));
@@ -353,7 +386,7 @@ igt_main
 
 		igt_require(display.is_atomic);
 
-		output = kms_writeback_get_output(&display);
+		output = kms_writeback_get_output(&display, &pipe, &clone);
 		igt_require(output);
 
 		if (output->use_override_mode)
@@ -431,6 +464,27 @@ igt_main
 		igt_remove_fb(display.drm_fd, &output_fb);
 	}
 
+	igt_subtest("writeback-check-output-clone") {
+		igt_fb_t output_fb;
+
+		igt_require(clone);
+
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		igt_output_clone_pipe(clone, pipe);
+		igt_output_override_mode(clone, &mode);
+
+		writeback_check_output(output, plane, &input_fb, &output_fb);
+
+		igt_output_set_pipe(clone, PIPE_NONE);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
 	igt_fixture {
 		igt_remove_fb(display.drm_fd, &input_fb);
 		igt_display_fini(&display);
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v5 6/6] kms_writeback: Add tests using a cloned output
@ 2019-01-15 17:47   ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-15 17:47 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

Update the connector search to also optionally attempt to find a
non-writeback connector to clone to.

Add a subtest which is the same as writeback-check-output, but also
clones to the second connector.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and addressed comments by Maarten]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 tests/kms_writeback.c | 64 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
index 0f20dadd2..ae536bbfa 100644
--- a/tests/kms_writeback.c
+++ b/tests/kms_writeback.c
@@ -51,7 +51,8 @@ static drmModePropertyBlobRes *get_writeback_formats_blob(igt_output_t *output)
 	return blob;
 }
 
-static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
+static bool check_writeback_config(igt_display_t *display, igt_output_t *output,
+				   int pipe, igt_output_t **clone)
 {
 	igt_fb_t input_fb, output_fb;
 	igt_plane_t *plane;
@@ -91,6 +92,30 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
 
 	ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
 					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	if (!ret && clone) {
+		/* Try and find a clone */
+		int i, newret;
+		*clone = NULL;
+
+		for (i = 0; i < display->n_outputs; i++) {
+			igt_output_t *second_output = &display->outputs[i];
+			if (output != second_output &&
+			    igt_pipe_connector_valid(pipe, second_output)) {
+
+				igt_output_clone_pipe(second_output, pipe);
+				igt_output_override_mode(output, &override_mode);
+				newret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY |
+								       DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+				igt_output_set_pipe(second_output, PIPE_NONE);
+				igt_debug("try_commit_atomic clone returned %d\n", newret);
+				if (!newret) {
+					*clone = second_output;
+					igt_debug("Selected clone %s\n", (*clone)->name);
+					break;
+				}
+			}
+		}
+	}
 	igt_plane_set_fb(plane, NULL);
 	igt_remove_fb(display->drm_fd, &input_fb);
 	igt_remove_fb(display->drm_fd, &output_fb);
@@ -98,7 +123,8 @@ static bool check_writeback_config(igt_display_t *display, igt_output_t *output)
 	return !ret;
 }
 
-static igt_output_t *kms_writeback_get_output(igt_display_t *display)
+static igt_output_t *kms_writeback_get_output(igt_display_t *display, enum pipe *pipe,
+					      igt_output_t **clone)
 {
 	int i;
 
@@ -114,10 +140,16 @@ static igt_output_t *kms_writeback_get_output(igt_display_t *display)
 		for (j = 0; j < igt_display_get_n_pipes(display); j++) {
 			igt_output_set_pipe(output, j);
 
-			if (check_writeback_config(display, output)) {
+			if (check_writeback_config(display, output, j, clone)) {
 				igt_debug("Using connector %u:%s on pipe %d\n",
 					  output->config.connector->connector_id,
 					  output->name, j);
+				if (clone && *clone)
+					igt_debug("Cloning to connector %u:%s\n",
+						  (*clone)->config.connector->connector_id,
+						  (*clone)->name);
+				if (pipe)
+					*pipe = j;
 				return output;
 			}
 		}
@@ -335,10 +367,11 @@ static void writeback_check_output(igt_output_t *output, igt_plane_t *plane,
 igt_main
 {
 	igt_display_t display;
-	igt_output_t *output;
+	igt_output_t *output, *clone;
 	igt_plane_t *plane;
 	igt_fb_t input_fb;
 	drmModeModeInfo mode;
+	enum pipe pipe;
 	int ret;
 
 	memset(&display, 0, sizeof(display));
@@ -353,7 +386,7 @@ igt_main
 
 		igt_require(display.is_atomic);
 
-		output = kms_writeback_get_output(&display);
+		output = kms_writeback_get_output(&display, &pipe, &clone);
 		igt_require(output);
 
 		if (output->use_override_mode)
@@ -431,6 +464,27 @@ igt_main
 		igt_remove_fb(display.drm_fd, &output_fb);
 	}
 
+	igt_subtest("writeback-check-output-clone") {
+		igt_fb_t output_fb;
+
+		igt_require(clone);
+
+		ret = igt_create_fb(display.drm_fd, mode.hdisplay, mode.vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    igt_fb_mod_to_tiling(0),
+				    &output_fb);
+		igt_require(ret > 0);
+
+		igt_output_clone_pipe(clone, pipe);
+		igt_output_override_mode(clone, &mode);
+
+		writeback_check_output(output, plane, &input_fb, &output_fb);
+
+		igt_output_set_pipe(clone, PIPE_NONE);
+
+		igt_remove_fb(display.drm_fd, &output_fb);
+	}
+
 	igt_fixture {
 		igt_remove_fb(display.drm_fd, &input_fb);
 		igt_display_fini(&display);
-- 
2.20.1

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

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

* Re: [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
  2019-01-15 17:47   ` [Intel-gfx] " Liviu Dudau
@ 2019-01-15 18:47     ` Chris Wilson
  -1 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2019-01-15 18:47 UTC (permalink / raw)
  To: Brian Starkey, Liviu Dudau; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML

Quoting Liviu Dudau (2019-01-15 17:47:44)
> +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> +{
> +#define FNV1a_OFFSET_BIAS 2166136261
> +#define FNV1a_PRIME 16777619
> +       uint32_t hash;
> +       void *map;
> +       char *ptr, *line = NULL;
> +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> +       uint32_t stride = calc_plane_stride(fb, 0);
> +
> +       if (fb->is_dumb)
> +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> +                                             PROT_READ);
> +       else
> +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> +                                   PROT_READ);
> +       ptr = map;
> +
> +       /*
> +        * Framebuffers are often uncached, which can make byte-wise accesses
> +        * very slow. We copy each line of the FB into a local buffer to speed
> +        * up the hashing.
> +        */
> +       line = malloc(stride);
> +       if (!line) {
> +               munmap(map, fb->size);
> +               return -ENOMEM;
> +       }
> +
> +       hash = FNV1a_OFFSET_BIAS;
> +
> +       for (y = 0; y < fb->height; y++, ptr += stride) {
> +
> +               memcpy(line, ptr, stride);

igt_memcpy_from_wc() for the reasons cited above.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
@ 2019-01-15 18:47     ` Chris Wilson
  0 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2019-01-15 18:47 UTC (permalink / raw)
  To: Brian Starkey, Liviu Dudau; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML

Quoting Liviu Dudau (2019-01-15 17:47:44)
> +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> +{
> +#define FNV1a_OFFSET_BIAS 2166136261
> +#define FNV1a_PRIME 16777619
> +       uint32_t hash;
> +       void *map;
> +       char *ptr, *line = NULL;
> +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> +       uint32_t stride = calc_plane_stride(fb, 0);
> +
> +       if (fb->is_dumb)
> +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> +                                             PROT_READ);
> +       else
> +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> +                                   PROT_READ);
> +       ptr = map;
> +
> +       /*
> +        * Framebuffers are often uncached, which can make byte-wise accesses
> +        * very slow. We copy each line of the FB into a local buffer to speed
> +        * up the hashing.
> +        */
> +       line = malloc(stride);
> +       if (!line) {
> +               munmap(map, fb->size);
> +               return -ENOMEM;
> +       }
> +
> +       hash = FNV1a_OFFSET_BIAS;
> +
> +       for (y = 0; y < fb->height; y++, ptr += stride) {
> +
> +               memcpy(line, ptr, stride);

igt_memcpy_from_wc() for the reasons cited above.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for igt: Add support for testing writeback connectors (rev3)
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
                   ` (6 preceding siblings ...)
  (?)
@ 2019-01-15 19:08 ` Patchwork
  -1 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2019-01-15 19:08 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: igt-dev

== Series Details ==

Series: igt: Add support for testing writeback connectors (rev3)
URL   : https://patchwork.freedesktop.org/series/39229/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5429 -> IGTPW_2241
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/39229/revisions/3/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_prime@amd-to-i915:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#107341]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        DMESG-FAIL [fdo#108735] -> PASS
    - fi-cfl-8109u:       DMESG-FAIL -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          FAIL [fdo#103167] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (46 -> 43)
------------------------------

  Additional (2): fi-kbl-7567u fi-skl-6700hq 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-pnv-d510 


Build changes
-------------

    * IGT: IGT_4773 -> IGTPW_2241

  CI_DRM_5429: b3e3bcfcefbc6f14bb30d54bcadddb63593b559c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2241: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2241/
  IGT_4773: 951e2b1a016b750544d0f42459b13b9c70631c68 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_writeback@writeback-check-output
+igt@kms_writeback@writeback-check-output-clone
+igt@kms_writeback@writeback-fb-id
+igt@kms_writeback@writeback-invalid-out-fence
+igt@kms_writeback@writeback-pixel-formats

== Logs ==

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

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

* [igt-dev] ✗ Fi.CI.IGT: failure for igt: Add support for testing writeback connectors (rev3)
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
                   ` (7 preceding siblings ...)
  (?)
@ 2019-01-16  0:13 ` Patchwork
  -1 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2019-01-16  0:13 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: igt-dev

== Series Details ==

Series: igt: Add support for testing writeback connectors (rev3)
URL   : https://patchwork.freedesktop.org/series/39229/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5429_full -> IGTPW_2241_full
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/39229/revisions/3/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries_display_off:
    - shard-snb:          PASS -> FAIL +1

  * igt@kms_rmfb@close-fd:
    - shard-hsw:          PASS -> FAIL +1
    - shard-glk:          PASS -> FAIL +1
    - shard-apl:          PASS -> FAIL +1
    - shard-kbl:          PASS -> FAIL +1

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-render:
    - shard-glk:          NOTRUN -> FAIL [fdo#103158]
    - shard-apl:          NOTRUN -> FAIL [fdo#103158]
    - shard-kbl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-apl:          NOTRUN -> INCOMPLETE [fdo#103927]
    - shard-kbl:          NOTRUN -> INCOMPLETE [fdo#103665]
    - shard-glk:          NOTRUN -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@i915_suspend@shrink:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#107886] / [fdo#109244]

  * igt@kms_atomic_transition@plane-all-transition-fencing:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] / [fdo#109225]

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_content_protection@atomic:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108597]
    - shard-apl:          NOTRUN -> FAIL [fdo#108597] +1

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-glk:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_legacy@pipe-a-forked-bo:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +18

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167]
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-glk:          PASS -> FAIL [fdo#103167] / [fdo#105682]
    - shard-kbl:          PASS -> FAIL [fdo#103167] / [fdo#105682]
    - shard-apl:          PASS -> DMESG-FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          PASS -> FAIL [fdo#108948] +1

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]
    - shard-apl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +5
    - shard-kbl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@kms_sysfs_edid_timing:
    - shard-apl:          NOTRUN -> FAIL [fdo#100047]

  * igt@prime_vgem@basic-fence-flip:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#103558] / [fdo#105602] +3

  
#### Possible fixes ####

  * igt@gem_userptr_blits@map-fixed-invalidate-gup:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-hsw:          INCOMPLETE [fdo#103540] -> PASS

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-gup:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          FAIL [fdo#106641] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          FAIL [fdo#104873] -> PASS

  * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-kbl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
    - shard-glk:          FAIL [fdo#103167] -> PASS +6

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          FAIL [fdo#103166] -> PASS
    - shard-kbl:          FAIL [fdo#103166] -> PASS +1

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          {SKIP} [fdo#109271] -> PASS
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108597]: https://bugs.freedesktop.org/show_bug.cgi?id=108597
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109225]: https://bugs.freedesktop.org/show_bug.cgi?id=109225
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4773 -> IGTPW_2241
    * Piglit: piglit_4509 -> None

  CI_DRM_5429: b3e3bcfcefbc6f14bb30d54bcadddb63593b559c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2241: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2241/
  IGT_4773: 951e2b1a016b750544d0f42459b13b9c70631c68 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
  2019-01-15 18:47     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-01-16 11:20       ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-16 11:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML

On Tue, Jan 15, 2019 at 06:47:47PM +0000, Chris Wilson wrote:
> Quoting Liviu Dudau (2019-01-15 17:47:44)
> > +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> > +{
> > +#define FNV1a_OFFSET_BIAS 2166136261
> > +#define FNV1a_PRIME 16777619
> > +       uint32_t hash;
> > +       void *map;
> > +       char *ptr, *line = NULL;
> > +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> > +       uint32_t stride = calc_plane_stride(fb, 0);
> > +
> > +       if (fb->is_dumb)
> > +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> > +                                             PROT_READ);
> > +       else
> > +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> > +                                   PROT_READ);
> > +       ptr = map;
> > +
> > +       /*
> > +        * Framebuffers are often uncached, which can make byte-wise accesses
> > +        * very slow. We copy each line of the FB into a local buffer to speed
> > +        * up the hashing.
> > +        */
> > +       line = malloc(stride);
> > +       if (!line) {
> > +               munmap(map, fb->size);
> > +               return -ENOMEM;
> > +       }
> > +
> > +       hash = FNV1a_OFFSET_BIAS;
> > +
> > +       for (y = 0; y < fb->height; y++, ptr += stride) {
> > +
> > +               memcpy(line, ptr, stride);
> 
> igt_memcpy_from_wc() for the reasons cited above.
> -Chris

Hi Chris,

Thanks for pointing out the function, I was not aware of it!

Now, looking at the implementation, and ignoring the fact that it is in a
file called igt_x86.c, it looks to me that it will end up calling memcpy
anyway for Arm drivers. Not being a GCC expert, I am wondering if the
ifunc() wrapper around resolve_memcpy_from_wc() will not actually
prevent the compiler from choosing an optimised version of memcpy for Arm.

I can refresh the patch if you think it is safe to use igt_memcpy_from_wc() for
non-x86 architectures.

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
@ 2019-01-16 11:20       ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-16 11:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML, Brian Starkey

On Tue, Jan 15, 2019 at 06:47:47PM +0000, Chris Wilson wrote:
> Quoting Liviu Dudau (2019-01-15 17:47:44)
> > +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> > +{
> > +#define FNV1a_OFFSET_BIAS 2166136261
> > +#define FNV1a_PRIME 16777619
> > +       uint32_t hash;
> > +       void *map;
> > +       char *ptr, *line = NULL;
> > +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> > +       uint32_t stride = calc_plane_stride(fb, 0);
> > +
> > +       if (fb->is_dumb)
> > +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> > +                                             PROT_READ);
> > +       else
> > +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> > +                                   PROT_READ);
> > +       ptr = map;
> > +
> > +       /*
> > +        * Framebuffers are often uncached, which can make byte-wise accesses
> > +        * very slow. We copy each line of the FB into a local buffer to speed
> > +        * up the hashing.
> > +        */
> > +       line = malloc(stride);
> > +       if (!line) {
> > +               munmap(map, fb->size);
> > +               return -ENOMEM;
> > +       }
> > +
> > +       hash = FNV1a_OFFSET_BIAS;
> > +
> > +       for (y = 0; y < fb->height; y++, ptr += stride) {
> > +
> > +               memcpy(line, ptr, stride);
> 
> igt_memcpy_from_wc() for the reasons cited above.
> -Chris

Hi Chris,

Thanks for pointing out the function, I was not aware of it!

Now, looking at the implementation, and ignoring the fact that it is in a
file called igt_x86.c, it looks to me that it will end up calling memcpy
anyway for Arm drivers. Not being a GCC expert, I am wondering if the
ifunc() wrapper around resolve_memcpy_from_wc() will not actually
prevent the compiler from choosing an optimised version of memcpy for Arm.

I can refresh the patch if you think it is safe to use igt_memcpy_from_wc() for
non-x86 architectures.

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
  2019-01-16 11:20       ` [igt-dev] [Intel-gfx] " Liviu Dudau
@ 2019-01-16 11:50         ` Chris Wilson
  -1 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2019-01-16 11:50 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML

Quoting Liviu Dudau (2019-01-16 11:20:09)
> On Tue, Jan 15, 2019 at 06:47:47PM +0000, Chris Wilson wrote:
> > Quoting Liviu Dudau (2019-01-15 17:47:44)
> > > +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> > > +{
> > > +#define FNV1a_OFFSET_BIAS 2166136261
> > > +#define FNV1a_PRIME 16777619
> > > +       uint32_t hash;
> > > +       void *map;
> > > +       char *ptr, *line = NULL;
> > > +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> > > +       uint32_t stride = calc_plane_stride(fb, 0);
> > > +
> > > +       if (fb->is_dumb)
> > > +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> > > +                                             PROT_READ);
> > > +       else
> > > +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> > > +                                   PROT_READ);
> > > +       ptr = map;
> > > +
> > > +       /*
> > > +        * Framebuffers are often uncached, which can make byte-wise accesses
> > > +        * very slow. We copy each line of the FB into a local buffer to speed
> > > +        * up the hashing.
> > > +        */
> > > +       line = malloc(stride);
> > > +       if (!line) {
> > > +               munmap(map, fb->size);
> > > +               return -ENOMEM;
> > > +       }
> > > +
> > > +       hash = FNV1a_OFFSET_BIAS;
> > > +
> > > +       for (y = 0; y < fb->height; y++, ptr += stride) {
> > > +
> > > +               memcpy(line, ptr, stride);
> > 
> > igt_memcpy_from_wc() for the reasons cited above.
> > -Chris
> 
> Hi Chris,
> 
> Thanks for pointing out the function, I was not aware of it!
> 
> Now, looking at the implementation, and ignoring the fact that it is in a
> file called igt_x86.c, it looks to me that it will end up calling memcpy
> anyway for Arm drivers. Not being a GCC expert, I am wondering if the
> ifunc() wrapper around resolve_memcpy_from_wc() will not actually
> prevent the compiler from choosing an optimised version of memcpy for Arm.
> 
> I can refresh the patch if you think it is safe to use igt_memcpy_from_wc() for
> non-x86 architectures.

You are memcpy'ing from uncached, there is no general purpose optimised
memcpy! :) The compiler builtin (e.g. rep movb for x86) may be the worst
thing you could do ;) For Arm, it will fallback to the general memcpy
routine which will do it own ifunc, just will not be inlined.

It should be safe, and it should be no worse than a call to memcpy with
variable arguments.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer
@ 2019-01-16 11:50         ` Chris Wilson
  0 siblings, 0 replies; 40+ messages in thread
From: Chris Wilson @ 2019-01-16 11:50 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: IGT GPU Tools, Boris Brezillon, Intel GFX ML, Brian Starkey

Quoting Liviu Dudau (2019-01-16 11:20:09)
> On Tue, Jan 15, 2019 at 06:47:47PM +0000, Chris Wilson wrote:
> > Quoting Liviu Dudau (2019-01-15 17:47:44)
> > > +int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
> > > +{
> > > +#define FNV1a_OFFSET_BIAS 2166136261
> > > +#define FNV1a_PRIME 16777619
> > > +       uint32_t hash;
> > > +       void *map;
> > > +       char *ptr, *line = NULL;
> > > +       int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
> > > +       uint32_t stride = calc_plane_stride(fb, 0);
> > > +
> > > +       if (fb->is_dumb)
> > > +               map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
> > > +                                             PROT_READ);
> > > +       else
> > > +               map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
> > > +                                   PROT_READ);
> > > +       ptr = map;
> > > +
> > > +       /*
> > > +        * Framebuffers are often uncached, which can make byte-wise accesses
> > > +        * very slow. We copy each line of the FB into a local buffer to speed
> > > +        * up the hashing.
> > > +        */
> > > +       line = malloc(stride);
> > > +       if (!line) {
> > > +               munmap(map, fb->size);
> > > +               return -ENOMEM;
> > > +       }
> > > +
> > > +       hash = FNV1a_OFFSET_BIAS;
> > > +
> > > +       for (y = 0; y < fb->height; y++, ptr += stride) {
> > > +
> > > +               memcpy(line, ptr, stride);
> > 
> > igt_memcpy_from_wc() for the reasons cited above.
> > -Chris
> 
> Hi Chris,
> 
> Thanks for pointing out the function, I was not aware of it!
> 
> Now, looking at the implementation, and ignoring the fact that it is in a
> file called igt_x86.c, it looks to me that it will end up calling memcpy
> anyway for Arm drivers. Not being a GCC expert, I am wondering if the
> ifunc() wrapper around resolve_memcpy_from_wc() will not actually
> prevent the compiler from choosing an optimised version of memcpy for Arm.
> 
> I can refresh the patch if you think it is safe to use igt_memcpy_from_wc() for
> non-x86 architectures.

You are memcpy'ing from uncached, there is no general purpose optimised
memcpy! :) The compiler builtin (e.g. rep movb for x86) may be the worst
thing you could do ;) For Arm, it will fallback to the general memcpy
routine which will do it own ifunc, just will not be inlined.

It should be safe, and it should be no worse than a call to memcpy with
variable arguments.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [PATCH i-g-t v6] lib: Add function to hash a framebuffer
  2019-01-15 18:47     ` [igt-dev] [Intel-gfx] " Chris Wilson
@ 2019-01-16 12:21       ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-16 12:21 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools

From: Brian Starkey <brian.starkey@arm.com>

To use writeback buffers as a CRC source, we need to be able to hash
them. Implement a simple FVA-1a hashing routine for this purpose.

Doing a bytewise hash on the framebuffer directly can be very slow if
the memory is noncached. By making a copy of each line in the FB first
(which can take advantage of word-access speedup), we can do the hash
on a cached copy, which is much faster (10x speedup on my platform).

v6: use igt_memcpy_from_wc() instead of plain memcpy, as suggested by
    Chris Wilson

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the most recent API]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_fb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  3 +++
 2 files changed, 69 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5cd1829a3..044fd57de 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2379,6 +2379,72 @@ bool igt_fb_supported_format(uint32_t drm_format)
 	return false;
 }
 
+/*
+ * This implements the FNV-1a hashing algorithm instead of CRC, for
+ * simplicity
+ * http://www.isthe.com/chongo/tech/comp/fnv/index.html
+ *
+ * hash = offset_basis
+ * for each octet_of_data to be hashed
+ *         hash = hash xor octet_of_data
+ *         hash = hash * FNV_prime
+ * return hash
+ *
+ * 32 bit offset_basis = 2166136261
+ * 32 bit FNV_prime = 224 + 28 + 0x93 = 16777619
+ */
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
+{
+#define FNV1a_OFFSET_BIAS 2166136261
+#define FNV1a_PRIME 16777619
+	uint32_t hash;
+	void *map;
+	char *ptr, *line = NULL;
+	int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
+	uint32_t stride = calc_plane_stride(fb, 0);
+
+	if (fb->is_dumb)
+		map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
+					      PROT_READ);
+	else
+		map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
+				    PROT_READ);
+	ptr = map;
+
+	/*
+	 * Framebuffers are often uncached, which can make byte-wise accesses
+	 * very slow. We copy each line of the FB into a local buffer to speed
+	 * up the hashing.
+	 */
+	line = malloc(stride);
+	if (!line) {
+		munmap(map, fb->size);
+		return -ENOMEM;
+	}
+
+	hash = FNV1a_OFFSET_BIAS;
+
+	for (y = 0; y < fb->height; y++, ptr += stride) {
+
+		igt_memcpy_from_wc(line, ptr, stride);
+
+		for (x = 0; x < fb->width * cpp; x++) {
+			hash ^= line[x];
+			hash *= FNV1a_PRIME;
+		}
+	}
+
+	crc->n_words = 1;
+	crc->crc[0] = hash;
+
+	free(line);
+	munmap(map, fb->size);
+
+	return 0;
+#undef FNV1a_OFFSET_BIAS
+#undef FNV1a_PRIME
+}
+
 /**
  * igt_format_is_yuv:
  * @drm_format: drm fourcc
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 9f027deba..948c5380c 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -37,6 +37,7 @@
 #include <i915_drm.h>
 
 #include "igt_color_encoding.h"
+#include "igt_debugfs.h"
 
 /**
  * igt_fb_t:
@@ -173,5 +174,7 @@ const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
 
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc);
+
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

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

* [igt-dev] [PATCH i-g-t v6] lib: Add function to hash a framebuffer
@ 2019-01-16 12:21       ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-01-16 12:21 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter

From: Brian Starkey <brian.starkey@arm.com>

To use writeback buffers as a CRC source, we need to be able to hash
them. Implement a simple FVA-1a hashing routine for this purpose.

Doing a bytewise hash on the framebuffer directly can be very slow if
the memory is noncached. By making a copy of each line in the FB first
(which can take advantage of word-access speedup), we can do the hash
on a cached copy, which is much faster (10x speedup on my platform).

v6: use igt_memcpy_from_wc() instead of plain memcpy, as suggested by
    Chris Wilson

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the most recent API]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_fb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  3 +++
 2 files changed, 69 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 5cd1829a3..044fd57de 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2379,6 +2379,72 @@ bool igt_fb_supported_format(uint32_t drm_format)
 	return false;
 }
 
+/*
+ * This implements the FNV-1a hashing algorithm instead of CRC, for
+ * simplicity
+ * http://www.isthe.com/chongo/tech/comp/fnv/index.html
+ *
+ * hash = offset_basis
+ * for each octet_of_data to be hashed
+ *         hash = hash xor octet_of_data
+ *         hash = hash * FNV_prime
+ * return hash
+ *
+ * 32 bit offset_basis = 2166136261
+ * 32 bit FNV_prime = 224 + 28 + 0x93 = 16777619
+ */
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc)
+{
+#define FNV1a_OFFSET_BIAS 2166136261
+#define FNV1a_PRIME 16777619
+	uint32_t hash;
+	void *map;
+	char *ptr, *line = NULL;
+	int x, y, cpp = igt_drm_format_to_bpp(fb->drm_format) / 8;
+	uint32_t stride = calc_plane_stride(fb, 0);
+
+	if (fb->is_dumb)
+		map = kmstest_dumb_map_buffer(fb->fd, fb->gem_handle, fb->size,
+					      PROT_READ);
+	else
+		map = gem_mmap__gtt(fb->fd, fb->gem_handle, fb->size,
+				    PROT_READ);
+	ptr = map;
+
+	/*
+	 * Framebuffers are often uncached, which can make byte-wise accesses
+	 * very slow. We copy each line of the FB into a local buffer to speed
+	 * up the hashing.
+	 */
+	line = malloc(stride);
+	if (!line) {
+		munmap(map, fb->size);
+		return -ENOMEM;
+	}
+
+	hash = FNV1a_OFFSET_BIAS;
+
+	for (y = 0; y < fb->height; y++, ptr += stride) {
+
+		igt_memcpy_from_wc(line, ptr, stride);
+
+		for (x = 0; x < fb->width * cpp; x++) {
+			hash ^= line[x];
+			hash *= FNV1a_PRIME;
+		}
+	}
+
+	crc->n_words = 1;
+	crc->crc[0] = hash;
+
+	free(line);
+	munmap(map, fb->size);
+
+	return 0;
+#undef FNV1a_OFFSET_BIAS
+#undef FNV1a_PRIME
+}
+
 /**
  * igt_format_is_yuv:
  * @drm_format: drm fourcc
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 9f027deba..948c5380c 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -37,6 +37,7 @@
 #include <i915_drm.h>
 
 #include "igt_color_encoding.h"
+#include "igt_debugfs.h"
 
 /**
  * igt_fb_t:
@@ -173,5 +174,7 @@ const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
 
+int igt_fb_get_crc(struct igt_fb *fb, igt_crc_t *crc);
+
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

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

* [igt-dev] ✗ Fi.CI.BAT: failure for igt: Add support for testing writeback connectors (rev4)
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
                   ` (8 preceding siblings ...)
  (?)
@ 2019-01-16 12:41 ` Patchwork
  -1 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2019-01-16 12:41 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: igt-dev

== Series Details ==

Series: igt: Add support for testing writeback connectors (rev4)
URL   : https://patchwork.freedesktop.org/series/39229/
State : failure

== Summary ==

CI Bug Log - changes from IGT_4774 -> IGTPW_2246
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/39229/revisions/4/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       PASS -> WARN

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       PASS -> DMESG-WARN [fdo#102614]

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +2

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (49 -> 44)
------------------------------

  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * IGT: IGT_4774 -> IGTPW_2246

  CI_DRM_5434: f7277432c1bb671177bdef59755cb88c4ad9f75f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2246: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2246/
  IGT_4774: 8477ed9aeeaa8afb491d73a4c53d1b7dc64413c6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_writeback@writeback-check-output
+igt@kms_writeback@writeback-check-output-clone
+igt@kms_writeback@writeback-fb-id
+igt@kms_writeback@writeback-invalid-out-fence
+igt@kms_writeback@writeback-pixel-formats

== Logs ==

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

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

* Re: [PATCH i-g-t v5 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning
  2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
@ 2019-02-04 13:27     ` Brian Starkey
  -1 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:27 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

Hi,

On Tue, Jan 15, 2019 at 05:47:46PM +0000, Liviu Dudau wrote:
> From: Brian Starkey <brian.starkey@arm.com>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 13d3a9ceb..acb27982b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -349,6 +349,9 @@ struct igt_pipe {
>  	uint32_t crtc_id;
>  
>  	int32_t out_fence_fd;
> +	bool out_fence_requested;

I think it looks like this ended up being unused.

Thanks,
-Brian

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

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

* Re: [igt-dev] [PATCH i-g-t v5 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning
@ 2019-02-04 13:27     ` Brian Starkey
  0 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:27 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter, nd

Hi,

On Tue, Jan 15, 2019 at 05:47:46PM +0000, Liviu Dudau wrote:
> From: Brian Starkey <brian.starkey@arm.com>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 13d3a9ceb..acb27982b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -349,6 +349,9 @@ struct igt_pipe {
>  	uint32_t crtc_id;
>  
>  	int32_t out_fence_fd;
> +	bool out_fence_requested;

I think it looks like this ended up being unused.

Thanks,
-Brian

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

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

* Re: [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
@ 2019-02-04 13:31     ` Brian Starkey
  -1 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:31 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

Hi Liviu,

On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> From: Brian Starkey <brian.starkey@arm.com>

[snip]

>  
> +/**
> + * igt_output_set_writeback_fb:
> + * @output: Target output
> + * @fb: Target framebuffer
> + *
> + * This function sets the given @fb to be used as the target framebuffer for the
> + * writeback engine at the next atomic commit. It will also request a writeback
> + * out fence that will contain the fd number of the out fence created by KMS if
> + * the given @fb is valid.
> + */
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> +{
> +	igt_display_t *display = output->display;
> +
> +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> +
> +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> +	/* only request a writeback out fence if the framebuffer is valid */
> +	if (fb)
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> +					  (ptrdiff_t)&output->writeback_out_fence_fd);

I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
seems better.

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

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

* Re: [Intel-gfx] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-02-04 13:31     ` Brian Starkey
  0 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:31 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

Hi Liviu,

On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> From: Brian Starkey <brian.starkey@arm.com>

[snip]

>  
> +/**
> + * igt_output_set_writeback_fb:
> + * @output: Target output
> + * @fb: Target framebuffer
> + *
> + * This function sets the given @fb to be used as the target framebuffer for the
> + * writeback engine at the next atomic commit. It will also request a writeback
> + * out fence that will contain the fd number of the out fence created by KMS if
> + * the given @fb is valid.
> + */
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> +{
> +	igt_display_t *display = output->display;
> +
> +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> +
> +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> +	/* only request a writeback out fence if the framebuffer is valid */
> +	if (fb)
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> +					  (ptrdiff_t)&output->writeback_out_fence_fd);

I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
seems better.

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

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

* Re: [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
@ 2019-02-04 13:36   ` Brian Starkey
  -1 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:36 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

Hi Liviu,

On Tue, Jan 15, 2019 at 05:47:41PM +0000, Liviu Dudau wrote:
> We're trying to introduce support for writeback connectors, a way to
> expose in DRM the hardware functionality from display engines that
> allows to write back into memory the result of the DE's composition
> of supported planes.
> 
> Although this is a rebase of v4 with all the comments addressed, I'm not
> expecting people to remember any of the previous versions, please review
> this as if it is a new series.
> 
> Patches have been originally implemented by Brian, I've done the v3 and v4
> updates to them.
> 
> Best regards,
> Liviu
> 
> Changelog:
>  - v5: Addressed comments from Brian Starkey. Old v4 changes are here:
>    https://lists.freedesktop.org/archives/igt-dev/2018-November/006806.html

I just replied with two super minor things, but otherwise this looks
fine to me.

As we were just discussing, strangely it seems like we both never
received Maarten's v4 emails. His proposed changes sounded fine to me,
though, if you decide to roll them in.

Thanks,
-Brian

>  
>  - v4: Rebased on the latest i-g-t and switched to the igt_output_xxx()
>    call as suggested by Maarten. v3 is here:
>    https://lists.freedesktop.org/archives/intel-gfx/2018-March/157394.html
>    Maarten's comments came a couple of months later :)
>    https://lists.freedesktop.org/archives/intel-gfx/2018-June/169027.html
>    
>  - v3: I've now dropped all the changes that were trying to split the CRC
>    functionality out of lib/igt_debugfs.c. v2 is here:
>    https://lists.freedesktop.org/archives/intel-gfx/2017-July/133154.html
>    
>  - Added meson support for builting the kms_writeback test
> 
> 
> Brian Starkey (6):
>   lib/igt_kms: Add writeback support
>   kms_writeback: Add initial writeback tests
>   lib: Add function to hash a framebuffer
>   kms_writeback: Add writeback-check-output
>   lib/igt_kms: Add igt_output_clone_pipe for cloning
>   kms_writeback: Add tests using a cloned output
> 
>  lib/igt_fb.c           |  66 ++++++
>  lib/igt_fb.h           |   3 +
>  lib/igt_kms.c          | 157 +++++++++----
>  lib/igt_kms.h          |  11 +
>  tests/Makefile.sources |   1 +
>  tests/kms_writeback.c  | 492 +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  7 files changed, 693 insertions(+), 38 deletions(-)
>  create mode 100644 tests/kms_writeback.c
> 
> -- 
> 2.20.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors
@ 2019-02-04 13:36   ` Brian Starkey
  0 siblings, 0 replies; 40+ messages in thread
From: Brian Starkey @ 2019-02-04 13:36 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter, nd

Hi Liviu,

On Tue, Jan 15, 2019 at 05:47:41PM +0000, Liviu Dudau wrote:
> We're trying to introduce support for writeback connectors, a way to
> expose in DRM the hardware functionality from display engines that
> allows to write back into memory the result of the DE's composition
> of supported planes.
> 
> Although this is a rebase of v4 with all the comments addressed, I'm not
> expecting people to remember any of the previous versions, please review
> this as if it is a new series.
> 
> Patches have been originally implemented by Brian, I've done the v3 and v4
> updates to them.
> 
> Best regards,
> Liviu
> 
> Changelog:
>  - v5: Addressed comments from Brian Starkey. Old v4 changes are here:
>    https://lists.freedesktop.org/archives/igt-dev/2018-November/006806.html

I just replied with two super minor things, but otherwise this looks
fine to me.

As we were just discussing, strangely it seems like we both never
received Maarten's v4 emails. His proposed changes sounded fine to me,
though, if you decide to roll them in.

Thanks,
-Brian

>  
>  - v4: Rebased on the latest i-g-t and switched to the igt_output_xxx()
>    call as suggested by Maarten. v3 is here:
>    https://lists.freedesktop.org/archives/intel-gfx/2018-March/157394.html
>    Maarten's comments came a couple of months later :)
>    https://lists.freedesktop.org/archives/intel-gfx/2018-June/169027.html
>    
>  - v3: I've now dropped all the changes that were trying to split the CRC
>    functionality out of lib/igt_debugfs.c. v2 is here:
>    https://lists.freedesktop.org/archives/intel-gfx/2017-July/133154.html
>    
>  - Added meson support for builting the kms_writeback test
> 
> 
> Brian Starkey (6):
>   lib/igt_kms: Add writeback support
>   kms_writeback: Add initial writeback tests
>   lib: Add function to hash a framebuffer
>   kms_writeback: Add writeback-check-output
>   lib/igt_kms: Add igt_output_clone_pipe for cloning
>   kms_writeback: Add tests using a cloned output
> 
>  lib/igt_fb.c           |  66 ++++++
>  lib/igt_fb.h           |   3 +
>  lib/igt_kms.c          | 157 +++++++++----
>  lib/igt_kms.h          |  11 +
>  tests/Makefile.sources |   1 +
>  tests/kms_writeback.c  | 492 +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  7 files changed, 693 insertions(+), 38 deletions(-)
>  create mode 100644 tests/kms_writeback.c
> 
> -- 
> 2.20.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-02-04 13:31     ` [Intel-gfx] " Brian Starkey
@ 2019-03-06 21:30       ` Rodrigo Siqueira
  -1 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Siqueira @ 2019-03-06 21:30 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd


[-- Attachment #1.1: Type: text/plain, Size: 3483 bytes --]

Hi Liviu,

I’m using your patchset to guide my implementation of writeback in the
VKMS, so, first of all, thanks :)

During my work, I noticed that you’re setting the drmSetClientCap()
before drmModeGetResources() which make the writeback capability
‘invisible’ for drmModeGetResources(). I made the following change, and
I could pass the igt_display_require():

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f4ff3a96..bb2d8e06 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 
        display->drm_fd = drm_fd;
 
+       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
+               display->is_atomic = 1;
+
+       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+
        resources = drmModeGetResources(display->drm_fd);
        if (!resources)
                goto out;
@@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
        display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
        igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
 
-       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
-       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
-               display->is_atomic = 1;
-
-       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
-
        plane_resources = drmModeGetPlaneResources(display->drm_fd);
        igt_assert(plane_resources);

I'm not 100% confident about this issue, because of this I will send an
RFC and see if I can get more details about this issue.

Additionally, if you need any help with this patchset I will be glad to
help since I’m using it \o/

Best Regards
Rodrigo Siqueira

On 02/04, Brian Starkey wrote:
> Hi Liviu,
> 
> On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > From: Brian Starkey <brian.starkey@arm.com>
> 
> [snip]
> 
> >  
> > +/**
> > + * igt_output_set_writeback_fb:
> > + * @output: Target output
> > + * @fb: Target framebuffer
> > + *
> > + * This function sets the given @fb to be used as the target framebuffer for the
> > + * writeback engine at the next atomic commit. It will also request a writeback
> > + * out fence that will contain the fd number of the out fence created by KMS if
> > + * the given @fb is valid.
> > + */
> > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > +{
> > +	igt_display_t *display = output->display;
> > +
> > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > +
> > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > +	/* only request a writeback out fence if the framebuffer is valid */
> > +	if (fb)
> > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> 
> I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> seems better.
> 
> Thanks,
> -Brian
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-03-06 21:30       ` Rodrigo Siqueira
  0 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Siqueira @ 2019-03-06 21:30 UTC (permalink / raw)
  To: Brian Starkey; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd


[-- Attachment #1.1: Type: text/plain, Size: 3483 bytes --]

Hi Liviu,

I’m using your patchset to guide my implementation of writeback in the
VKMS, so, first of all, thanks :)

During my work, I noticed that you’re setting the drmSetClientCap()
before drmModeGetResources() which make the writeback capability
‘invisible’ for drmModeGetResources(). I made the following change, and
I could pass the igt_display_require():

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f4ff3a96..bb2d8e06 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 
        display->drm_fd = drm_fd;
 
+       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
+               display->is_atomic = 1;
+
+       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+
        resources = drmModeGetResources(display->drm_fd);
        if (!resources)
                goto out;
@@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
        display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
        igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
 
-       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
-       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
-               display->is_atomic = 1;
-
-       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
-
        plane_resources = drmModeGetPlaneResources(display->drm_fd);
        igt_assert(plane_resources);

I'm not 100% confident about this issue, because of this I will send an
RFC and see if I can get more details about this issue.

Additionally, if you need any help with this patchset I will be glad to
help since I’m using it \o/

Best Regards
Rodrigo Siqueira

On 02/04, Brian Starkey wrote:
> Hi Liviu,
> 
> On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > From: Brian Starkey <brian.starkey@arm.com>
> 
> [snip]
> 
> >  
> > +/**
> > + * igt_output_set_writeback_fb:
> > + * @output: Target output
> > + * @fb: Target framebuffer
> > + *
> > + * This function sets the given @fb to be used as the target framebuffer for the
> > + * writeback engine at the next atomic commit. It will also request a writeback
> > + * out fence that will contain the fd number of the out fence created by KMS if
> > + * the given @fb is valid.
> > + */
> > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > +{
> > +	igt_display_t *display = output->display;
> > +
> > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > +
> > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > +	/* only request a writeback out fence if the framebuffer is valid */
> > +	if (fb)
> > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> 
> I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> seems better.
> 
> Thanks,
> -Brian
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* [igt-dev] ✗ Fi.CI.BAT: failure for igt: Add support for testing writeback connectors (rev5)
  2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
                   ` (10 preceding siblings ...)
  (?)
@ 2019-03-06 21:42 ` Patchwork
  -1 siblings, 0 replies; 40+ messages in thread
From: Patchwork @ 2019-03-06 21:42 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: igt-dev

== Series Details ==

Series: igt: Add support for testing writeback connectors (rev5)
URL   : https://patchwork.freedesktop.org/series/39229/
State : failure

== Summary ==

Applying: lib/igt_kms: Add writeback support
Patch failed at 0001 lib/igt_kms: Add writeback support
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-03-06 21:30       ` [Intel-gfx] " Rodrigo Siqueira
@ 2019-03-18 10:41         ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-03-18 10:41 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> Hi Liviu,
> 
> I’m using your patchset to guide my implementation of writeback in the
> VKMS, so, first of all, thanks :)
> 
> During my work, I noticed that you’re setting the drmSetClientCap()
> before drmModeGetResources() which make the writeback capability

before? I believe your patchset moves it to be before, which makes
sense, as the writeback applies to connectors, not planes (sorry for the
confusion, I think I wrote the code when we were toying with the idea
that writeback was going to be a plane property).

> ‘invisible’ for drmModeGetResources(). I made the following change, and
> I could pass the igt_display_require():
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f4ff3a96..bb2d8e06 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  
>         display->drm_fd = drm_fd;
>  
> +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> +               display->is_atomic = 1;
> +
> +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> +
>         resources = drmModeGetResources(display->drm_fd);
>         if (!resources)
>                 goto out;
> @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
>         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
>  
> -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> -               display->is_atomic = 1;
> -
> -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> -
>         plane_resources = drmModeGetPlaneResources(display->drm_fd);
>         igt_assert(plane_resources);
> 
> I'm not 100% confident about this issue, because of this I will send an
> RFC and see if I can get more details about this issue.

It actually looks correct to me, so 

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

> 
> Additionally, if you need any help with this patchset I will be glad to
> help since I’m using it \o/

Well, I need to send an updated version and if you could review that
then I can merge it into igt.

Best regards,
Liviu

> 
> Best Regards
> Rodrigo Siqueira
> 
> On 02/04, Brian Starkey wrote:
> > Hi Liviu,
> > 
> > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > From: Brian Starkey <brian.starkey@arm.com>
> > 
> > [snip]
> > 
> > >  
> > > +/**
> > > + * igt_output_set_writeback_fb:
> > > + * @output: Target output
> > > + * @fb: Target framebuffer
> > > + *
> > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > + * the given @fb is valid.
> > > + */
> > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > +{
> > > +	igt_display_t *display = output->display;
> > > +
> > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > +
> > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > +	if (fb)
> > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > 
> > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > seems better.
> > 
> > Thanks,
> > -Brian
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-03-18 10:41         ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-03-18 10:41 UTC (permalink / raw)
  To: Rodrigo Siqueira
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter, nd, Brian Starkey

On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> Hi Liviu,
> 
> I’m using your patchset to guide my implementation of writeback in the
> VKMS, so, first of all, thanks :)
> 
> During my work, I noticed that you’re setting the drmSetClientCap()
> before drmModeGetResources() which make the writeback capability

before? I believe your patchset moves it to be before, which makes
sense, as the writeback applies to connectors, not planes (sorry for the
confusion, I think I wrote the code when we were toying with the idea
that writeback was going to be a plane property).

> ‘invisible’ for drmModeGetResources(). I made the following change, and
> I could pass the igt_display_require():
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f4ff3a96..bb2d8e06 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  
>         display->drm_fd = drm_fd;
>  
> +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> +               display->is_atomic = 1;
> +
> +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> +
>         resources = drmModeGetResources(display->drm_fd);
>         if (!resources)
>                 goto out;
> @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
>         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
>  
> -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> -               display->is_atomic = 1;
> -
> -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> -
>         plane_resources = drmModeGetPlaneResources(display->drm_fd);
>         igt_assert(plane_resources);
> 
> I'm not 100% confident about this issue, because of this I will send an
> RFC and see if I can get more details about this issue.

It actually looks correct to me, so 

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

> 
> Additionally, if you need any help with this patchset I will be glad to
> help since I’m using it \o/

Well, I need to send an updated version and if you could review that
then I can merge it into igt.

Best regards,
Liviu

> 
> Best Regards
> Rodrigo Siqueira
> 
> On 02/04, Brian Starkey wrote:
> > Hi Liviu,
> > 
> > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > From: Brian Starkey <brian.starkey@arm.com>
> > 
> > [snip]
> > 
> > >  
> > > +/**
> > > + * igt_output_set_writeback_fb:
> > > + * @output: Target output
> > > + * @fb: Target framebuffer
> > > + *
> > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > + * the given @fb is valid.
> > > + */
> > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > +{
> > > +	igt_display_t *display = output->display;
> > > +
> > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > +
> > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > +	if (fb)
> > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > 
> > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > seems better.
> > 
> > Thanks,
> > -Brian
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-03-18 10:41         ` Liviu Dudau
@ 2019-03-18 22:05           ` Rodrigo Siqueira
  -1 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Siqueira @ 2019-03-18 22:05 UTC (permalink / raw)
  To: Liviu Dudau; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd


[-- Attachment #1.1: Type: text/plain, Size: 5204 bytes --]

On 03/18, Liviu Dudau wrote:
> On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> > Hi Liviu,
> > 
> > I’m using your patchset to guide my implementation of writeback in the
> > VKMS, so, first of all, thanks :)
> > 
> > During my work, I noticed that you’re setting the drmSetClientCap()
> > before drmModeGetResources() which make the writeback capability
> 
> before? I believe your patchset moves it to be before, which makes
> sense, as the writeback applies to connectors, not planes (sorry for the
> confusion, I think I wrote the code when we were toying with the idea
> that writeback was going to be a plane property).
> 
> > ‘invisible’ for drmModeGetResources(). I made the following change, and
> > I could pass the igt_display_require():
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index f4ff3a96..bb2d8e06 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> >  
> >         display->drm_fd = drm_fd;
> >  
> > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > +               display->is_atomic = 1;
> > +
> > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > +
> >         resources = drmModeGetResources(display->drm_fd);
> >         if (!resources)
> >                 goto out;
> > @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> >         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
> >         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
> >  
> > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > -               display->is_atomic = 1;
> > -
> > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > -
> >         plane_resources = drmModeGetPlaneResources(display->drm_fd);
> >         igt_assert(plane_resources);
> > 
> > I'm not 100% confident about this issue, because of this I will send an
> > RFC and see if I can get more details about this issue.
> 
> It actually looks correct to me, so 
> 
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> > 
> > Additionally, if you need any help with this patchset I will be glad to
> > help since I’m using it \o/
> 
> Well, I need to send an updated version and if you could review that
> then I can merge it into igt.

Sure! I will be glad to take some time to review your code :)

I can also try to test it on my current implementation of VKMS.
One question, I have a NanoPc-T1 device [1] which has a Mali GPU, can I
test the writeback feature in this device? All kind of Mali GPU has
support for the writeback feature?

1. http://wiki.friendlyarm.com/wiki/index.php/NanoPC-T1

Thanks!
Best Regards
 
> Best regards,
> Liviu
> 
> > 
> > Best Regards
> > Rodrigo Siqueira
> > 
> > On 02/04, Brian Starkey wrote:
> > > Hi Liviu,
> > > 
> > > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > > From: Brian Starkey <brian.starkey@arm.com>
> > > 
> > > [snip]
> > > 
> > > >  
> > > > +/**
> > > > + * igt_output_set_writeback_fb:
> > > > + * @output: Target output
> > > > + * @fb: Target framebuffer
> > > > + *
> > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > + * the given @fb is valid.
> > > > + */
> > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > +{
> > > > +	igt_display_t *display = output->display;
> > > > +
> > > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > +
> > > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > > +	if (fb)
> > > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > > 
> > > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > > seems better.
> > > 
> > > Thanks,
> > > -Brian
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > -- 
> > Rodrigo Siqueira
> > https://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> 
> 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-03-18 22:05           ` Rodrigo Siqueira
  0 siblings, 0 replies; 40+ messages in thread
From: Rodrigo Siqueira @ 2019-03-18 22:05 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter, nd, Brian Starkey


[-- Attachment #1.1: Type: text/plain, Size: 5204 bytes --]

On 03/18, Liviu Dudau wrote:
> On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> > Hi Liviu,
> > 
> > I’m using your patchset to guide my implementation of writeback in the
> > VKMS, so, first of all, thanks :)
> > 
> > During my work, I noticed that you’re setting the drmSetClientCap()
> > before drmModeGetResources() which make the writeback capability
> 
> before? I believe your patchset moves it to be before, which makes
> sense, as the writeback applies to connectors, not planes (sorry for the
> confusion, I think I wrote the code when we were toying with the idea
> that writeback was going to be a plane property).
> 
> > ‘invisible’ for drmModeGetResources(). I made the following change, and
> > I could pass the igt_display_require():
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index f4ff3a96..bb2d8e06 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> >  
> >         display->drm_fd = drm_fd;
> >  
> > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > +               display->is_atomic = 1;
> > +
> > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > +
> >         resources = drmModeGetResources(display->drm_fd);
> >         if (!resources)
> >                 goto out;
> > @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> >         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
> >         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
> >  
> > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > -               display->is_atomic = 1;
> > -
> > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > -
> >         plane_resources = drmModeGetPlaneResources(display->drm_fd);
> >         igt_assert(plane_resources);
> > 
> > I'm not 100% confident about this issue, because of this I will send an
> > RFC and see if I can get more details about this issue.
> 
> It actually looks correct to me, so 
> 
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> > 
> > Additionally, if you need any help with this patchset I will be glad to
> > help since I’m using it \o/
> 
> Well, I need to send an updated version and if you could review that
> then I can merge it into igt.

Sure! I will be glad to take some time to review your code :)

I can also try to test it on my current implementation of VKMS.
One question, I have a NanoPc-T1 device [1] which has a Mali GPU, can I
test the writeback feature in this device? All kind of Mali GPU has
support for the writeback feature?

1. http://wiki.friendlyarm.com/wiki/index.php/NanoPC-T1

Thanks!
Best Regards
 
> Best regards,
> Liviu
> 
> > 
> > Best Regards
> > Rodrigo Siqueira
> > 
> > On 02/04, Brian Starkey wrote:
> > > Hi Liviu,
> > > 
> > > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > > From: Brian Starkey <brian.starkey@arm.com>
> > > 
> > > [snip]
> > > 
> > > >  
> > > > +/**
> > > > + * igt_output_set_writeback_fb:
> > > > + * @output: Target output
> > > > + * @fb: Target framebuffer
> > > > + *
> > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > + * the given @fb is valid.
> > > > + */
> > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > +{
> > > > +	igt_display_t *display = output->display;
> > > > +
> > > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > +
> > > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > > +	if (fb)
> > > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > > 
> > > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > > seems better.
> > > 
> > > Thanks,
> > > -Brian
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > -- 
> > Rodrigo Siqueira
> > https://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> 
> 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
  2019-03-18 22:05           ` Rodrigo Siqueira
@ 2019-03-20 14:46             ` Liviu Dudau
  -1 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-03-20 14:46 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: Boris Brezillon, Intel GFX ML, IGT GPU Tools, nd

On Mon, Mar 18, 2019 at 07:05:29PM -0300, Rodrigo Siqueira wrote:
> On 03/18, Liviu Dudau wrote:
> > On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> > > Hi Liviu,
> > > 
> > > I’m using your patchset to guide my implementation of writeback in the
> > > VKMS, so, first of all, thanks :)
> > > 
> > > During my work, I noticed that you’re setting the drmSetClientCap()
> > > before drmModeGetResources() which make the writeback capability
> > 
> > before? I believe your patchset moves it to be before, which makes
> > sense, as the writeback applies to connectors, not planes (sorry for the
> > confusion, I think I wrote the code when we were toying with the idea
> > that writeback was going to be a plane property).
> > 
> > > ‘invisible’ for drmModeGetResources(). I made the following change, and
> > > I could pass the igt_display_require():
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index f4ff3a96..bb2d8e06 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > >  
> > >         display->drm_fd = drm_fd;
> > >  
> > > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > > +               display->is_atomic = 1;
> > > +
> > > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > +
> > >         resources = drmModeGetResources(display->drm_fd);
> > >         if (!resources)
> > >                 goto out;
> > > @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > >         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
> > >         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
> > >  
> > > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > > -               display->is_atomic = 1;
> > > -
> > > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > -
> > >         plane_resources = drmModeGetPlaneResources(display->drm_fd);
> > >         igt_assert(plane_resources);
> > > 
> > > I'm not 100% confident about this issue, because of this I will send an
> > > RFC and see if I can get more details about this issue.
> > 
> > It actually looks correct to me, so 
> > 
> > Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > > 
> > > Additionally, if you need any help with this patchset I will be glad to
> > > help since I’m using it \o/
> > 
> > Well, I need to send an updated version and if you could review that
> > then I can merge it into igt.
> 
> Sure! I will be glad to take some time to review your code :)
> 
> I can also try to test it on my current implementation of VKMS.
> One question, I have a NanoPc-T1 device [1] which has a Mali GPU, can I
> test the writeback feature in this device? All kind of Mali GPU has
> support for the writeback feature?
> 
> 1. http://wiki.friendlyarm.com/wiki/index.php/NanoPC-T1

(sorry, email fell through the cracks)

The writeback is a Mali Display Processor feature, not GPU. As such you
need a device that has Mali DP in it. Commercially there will be a
platform later this year, but until it is announced I cannot say more
about it.

Best regards,
Liviu

> 
> Thanks!
> Best Regards
>  
> > Best regards,
> > Liviu
> > 
> > > 
> > > Best Regards
> > > Rodrigo Siqueira
> > > 
> > > On 02/04, Brian Starkey wrote:
> > > > Hi Liviu,
> > > > 
> > > > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > > > From: Brian Starkey <brian.starkey@arm.com>
> > > > 
> > > > [snip]
> > > > 
> > > > >  
> > > > > +/**
> > > > > + * igt_output_set_writeback_fb:
> > > > > + * @output: Target output
> > > > > + * @fb: Target framebuffer
> > > > > + *
> > > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > > + * the given @fb is valid.
> > > > > + */
> > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > > +{
> > > > > +	igt_display_t *display = output->display;
> > > > > +
> > > > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > > +
> > > > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > > > +	if (fb)
> > > > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > > > 
> > > > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > > > seems better.
> > > > 
> > > > Thanks,
> > > > -Brian
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > > 
> > > -- 
> > > Rodrigo Siqueira
> > > https://siqueira.tech
> > > Graduate Student
> > > Department of Computer Science
> > > University of São Paulo
> > 
> > 
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
> -- 
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support
@ 2019-03-20 14:46             ` Liviu Dudau
  0 siblings, 0 replies; 40+ messages in thread
From: Liviu Dudau @ 2019-03-20 14:46 UTC (permalink / raw)
  To: Rodrigo Siqueira
  Cc: Boris Brezillon, Petri Latvala, Intel GFX ML, IGT GPU Tools,
	Daniel Vetter, nd, Brian Starkey

On Mon, Mar 18, 2019 at 07:05:29PM -0300, Rodrigo Siqueira wrote:
> On 03/18, Liviu Dudau wrote:
> > On Wed, Mar 06, 2019 at 06:30:05PM -0300, Rodrigo Siqueira wrote:
> > > Hi Liviu,
> > > 
> > > I’m using your patchset to guide my implementation of writeback in the
> > > VKMS, so, first of all, thanks :)
> > > 
> > > During my work, I noticed that you’re setting the drmSetClientCap()
> > > before drmModeGetResources() which make the writeback capability
> > 
> > before? I believe your patchset moves it to be before, which makes
> > sense, as the writeback applies to connectors, not planes (sorry for the
> > confusion, I think I wrote the code when we were toying with the idea
> > that writeback was going to be a plane property).
> > 
> > > ‘invisible’ for drmModeGetResources(). I made the following change, and
> > > I could pass the igt_display_require():
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index f4ff3a96..bb2d8e06 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -1912,6 +1912,12 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > >  
> > >         display->drm_fd = drm_fd;
> > >  
> > > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > +       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > > +               display->is_atomic = 1;
> > > +
> > > +       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > +
> > >         resources = drmModeGetResources(display->drm_fd);
> > >         if (!resources)
> > >                 goto out;
> > > @@ -1924,12 +1930,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > >         display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
> > >         igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
> > >  
> > > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > -       if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> > > -               display->is_atomic = 1;
> > > -
> > > -       drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > -
> > >         plane_resources = drmModeGetPlaneResources(display->drm_fd);
> > >         igt_assert(plane_resources);
> > > 
> > > I'm not 100% confident about this issue, because of this I will send an
> > > RFC and see if I can get more details about this issue.
> > 
> > It actually looks correct to me, so 
> > 
> > Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > > 
> > > Additionally, if you need any help with this patchset I will be glad to
> > > help since I’m using it \o/
> > 
> > Well, I need to send an updated version and if you could review that
> > then I can merge it into igt.
> 
> Sure! I will be glad to take some time to review your code :)
> 
> I can also try to test it on my current implementation of VKMS.
> One question, I have a NanoPc-T1 device [1] which has a Mali GPU, can I
> test the writeback feature in this device? All kind of Mali GPU has
> support for the writeback feature?
> 
> 1. http://wiki.friendlyarm.com/wiki/index.php/NanoPC-T1

(sorry, email fell through the cracks)

The writeback is a Mali Display Processor feature, not GPU. As such you
need a device that has Mali DP in it. Commercially there will be a
platform later this year, but until it is announced I cannot say more
about it.

Best regards,
Liviu

> 
> Thanks!
> Best Regards
>  
> > Best regards,
> > Liviu
> > 
> > > 
> > > Best Regards
> > > Rodrigo Siqueira
> > > 
> > > On 02/04, Brian Starkey wrote:
> > > > Hi Liviu,
> > > > 
> > > > On Tue, Jan 15, 2019 at 05:47:42PM +0000, Liviu Dudau wrote:
> > > > > From: Brian Starkey <brian.starkey@arm.com>
> > > > 
> > > > [snip]
> > > > 
> > > > >  
> > > > > +/**
> > > > > + * igt_output_set_writeback_fb:
> > > > > + * @output: Target output
> > > > > + * @fb: Target framebuffer
> > > > > + *
> > > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > > + * the given @fb is valid.
> > > > > + */
> > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > > +{
> > > > > +	igt_display_t *display = output->display;
> > > > > +
> > > > > +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > > +
> > > > > +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > > +	/* only request a writeback out fence if the framebuffer is valid */
> > > > > +	if (fb)
> > > > > +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> > > > 
> > > > I'm still not sure (ptrdiff_t) is the right type here, (uintptr_t)
> > > > seems better.
> > > > 
> > > > Thanks,
> > > > -Brian
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > > 
> > > -- 
> > > Rodrigo Siqueira
> > > https://siqueira.tech
> > > Graduate Student
> > > Department of Computer Science
> > > University of São Paulo
> > 
> > 
> > 
> > -- 
> > ====================
> > | I would like to |
> > | fix the world,  |
> > | but they're not |
> > | giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
> -- 
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo



-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2019-03-20 14:46 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 17:47 [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors Liviu Dudau
2019-01-15 17:47 ` [igt-dev] " Liviu Dudau
2019-01-15 17:47 ` [PATCH i-g-t v5 1/6] lib/igt_kms: Add writeback support Liviu Dudau
2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
2019-02-04 13:31   ` Brian Starkey
2019-02-04 13:31     ` [Intel-gfx] " Brian Starkey
2019-03-06 21:30     ` [igt-dev] " Rodrigo Siqueira
2019-03-06 21:30       ` [Intel-gfx] " Rodrigo Siqueira
2019-03-18 10:41       ` Liviu Dudau
2019-03-18 10:41         ` Liviu Dudau
2019-03-18 22:05         ` Rodrigo Siqueira
2019-03-18 22:05           ` Rodrigo Siqueira
2019-03-20 14:46           ` Liviu Dudau
2019-03-20 14:46             ` Liviu Dudau
2019-01-15 17:47 ` [PATCH i-g-t v5 2/6] kms_writeback: Add initial writeback tests Liviu Dudau
2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
2019-01-15 17:47 ` [PATCH i-g-t v5 3/6] lib: Add function to hash a framebuffer Liviu Dudau
2019-01-15 17:47   ` [Intel-gfx] " Liviu Dudau
2019-01-15 18:47   ` Chris Wilson
2019-01-15 18:47     ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-01-16 11:20     ` Liviu Dudau
2019-01-16 11:20       ` [igt-dev] [Intel-gfx] " Liviu Dudau
2019-01-16 11:50       ` Chris Wilson
2019-01-16 11:50         ` [igt-dev] [Intel-gfx] " Chris Wilson
2019-01-16 12:21     ` [PATCH i-g-t v6] " Liviu Dudau
2019-01-16 12:21       ` [igt-dev] " Liviu Dudau
2019-01-15 17:47 ` [PATCH i-g-t v5 4/6] kms_writeback: Add writeback-check-output Liviu Dudau
2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
2019-01-15 17:47 ` [PATCH i-g-t v5 5/6] lib/igt_kms: Add igt_output_clone_pipe for cloning Liviu Dudau
2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
2019-02-04 13:27   ` Brian Starkey
2019-02-04 13:27     ` [igt-dev] " Brian Starkey
2019-01-15 17:47 ` [PATCH i-g-t v5 6/6] kms_writeback: Add tests using a cloned output Liviu Dudau
2019-01-15 17:47   ` [igt-dev] " Liviu Dudau
2019-01-15 19:08 ` [igt-dev] ✓ Fi.CI.BAT: success for igt: Add support for testing writeback connectors (rev3) Patchwork
2019-01-16  0:13 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2019-01-16 12:41 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt: Add support for testing writeback connectors (rev4) Patchwork
2019-02-04 13:36 ` [PATCH i-g-t v5 0/6] igt: Add support for testing writeback connectors Brian Starkey
2019-02-04 13:36   ` [igt-dev] " Brian Starkey
2019-03-06 21:42 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt: Add support for testing writeback connectors (rev5) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.