igt-dev.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc.
@ 2018-07-24 13:59 Maarten Lankhorst
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current() Maarten Lankhorst
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2018-07-24 13:59 UTC (permalink / raw)
  To: igt-dev

A pair of igt_pipe_crc_drain() and igt_pipe_crc_get_single() has a bug in which
you're not sure whether the new CRC is captured because the vblank irq may race
against the CRC irq and delivered to userspace too soon.

When receiving a vblank event, igt_pipe_crc_drain() will then drain all crc's,
but not the new (stale) crc, which is then delivered and immediately returned
by igt_pipe_crc_get_single().

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 lib/igt_debugfs.c | 32 +++++++++++++++++++++++++++++++-
 lib/igt_debugfs.h |  2 ++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index f3196f4341f3..dc2c7df9da30 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -910,7 +910,7 @@ void igt_pipe_crc_drain(igt_pipe_crc_t *pipe_crc)
  * look at igt_pipe_crc_collect_crc().
  *
  * If capturing has been going on for a while and a fresh crc is required,
- * you will need to call igt_pipe_crc_drain() first to remove stale entries.
+ * you should use igt_pipe_crc_get_current() instead.
  *
  * Returns:
  * Whether a crc is captured, only false in non-blocking mode.
@@ -933,6 +933,36 @@ igt_pipe_crc_get_single(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc)
 	return found;
 }
 
+/**
+ * igt_pipe_crc_get_current:
+ * @drm_fd: Pointer to drm fd for vblank counter
+ * @pipe_crc: pipe CRC object
+ * @crc: buffer pointer for the captured CRC value
+ *
+ * Same as igt_pipe_crc_get_single(), but will wait until a new CRC can be captured.
+ * This is useful for retrieving the current CRC in a more race free way than
+ * igt_pipe_crc_drain() + igt_pipe_crc_get_single().
+ */
+void
+igt_pipe_crc_get_current(int drm_fd, igt_pipe_crc_t *pipe_crc, igt_crc_t *crc)
+{
+	unsigned vblank = kmstest_get_vblank(drm_fd, pipe_crc->pipe, 0);
+
+	igt_assert(!(pipe_crc->flags & O_NONBLOCK));
+	do {
+		read_one_crc(pipe_crc, crc);
+
+		/* Only works with valid frame counter */
+		if (!crc->has_valid_frame) {
+			igt_pipe_crc_drain(pipe_crc);
+			igt_pipe_crc_get_single(pipe_crc, crc);
+			return;
+		}
+	} while (crc->frame <= vblank);
+
+	crc_sanity_checks(crc);
+}
+
 /*
  * Drop caches
  */
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 8d25abfe43c1..db634a827290 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -132,6 +132,8 @@ int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
 			  igt_crc_t **out_crcs);
 void igt_pipe_crc_drain(igt_pipe_crc_t *pipe_crc);
 bool igt_pipe_crc_get_single(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
+void igt_pipe_crc_get_current(int drm_fd, igt_pipe_crc_t *pipe_crc, igt_crc_t *crc);
+
 void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
 
 void igt_hpd_storm_set_threshold(int fd, unsigned int threshold);
-- 
2.18.0

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

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

* [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current()
  2018-07-24 13:59 [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Maarten Lankhorst
@ 2018-07-24 13:59 ` Maarten Lankhorst
  2018-08-14 15:18   ` Daniel Vetter
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests Maarten Lankhorst
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-07-24 13:59 UTC (permalink / raw)
  To: igt-dev

This is a more race free of accomplishing the same.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/kms_available_modes_crc.c |  3 +--
 tests/kms_cursor_legacy.c       |  3 +--
 tests/kms_plane.c               |  6 +-----
 tests/kms_plane_lowres.c        |  6 ++----
 tests/kms_plane_multiple.c      |  3 +--
 tests/kms_rotation_crc.c        | 13 +++++--------
 6 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
index b70ef5d7d4c0..69a6a37d8ebd 100644
--- a/tests/kms_available_modes_crc.c
+++ b/tests/kms_available_modes_crc.c
@@ -400,8 +400,7 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
 		igt_display_commit2(&data->display, data->commit);
 
 		if (do_crc) {
-			igt_pipe_crc_drain(data->pipe_crc);
-			igt_pipe_crc_get_single(data->pipe_crc, &current_crc);
+			igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &current_crc);
 
 			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
 				if (!igt_check_crc_equal(&current_crc,
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 85340d43e5b6..82ceebcb0809 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -1334,8 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 
 		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
 
-		igt_pipe_crc_drain(pipe_crc);
-		igt_pipe_crc_get_single(pipe_crc, &test_crc);
+		igt_pipe_crc_get_current(display->drm_fd, pipe_crc, &test_crc);
 
 		igt_spin_batch_free(display->drm_fd, spin);
 
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index f9e123f0d1ea..3999dde8f694 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -419,11 +419,7 @@ static void test_format_plane_color(data_t *data, enum pipe pipe,
 	igt_plane_set_fb(plane, fb);
 
 	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_UNIVERSAL);
-
-	/* make sure the crc we get is for the new fb */
-	igt_wait_for_vblank(data->drm_fd, pipe);
-	igt_pipe_crc_drain(data->pipe_crc);
-	igt_pipe_crc_get_single(data->pipe_crc, crc);
+	igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, crc);
 
 	igt_remove_fb(data->drm_fd, &old_fb);
 }
diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index d1e4b3ca536e..5e83ef5630b5 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -221,8 +221,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 	check_mode(&mode_lowres, mode2);
 
 	igt_display_commit2(&data->display, COMMIT_ATOMIC);
-	igt_pipe_crc_drain(pipe_crc);
-	igt_pipe_crc_get_single(pipe_crc, &crc_lowres);
+	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_lowres);
 
 	igt_assert_plane_visible(data->drm_fd, pipe, false);
 
@@ -236,8 +235,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 
 	igt_display_commit2(&data->display, COMMIT_ATOMIC);
 
-	igt_pipe_crc_drain(pipe_crc);
-	igt_pipe_crc_get_single(pipe_crc, &crc_hires2);
+	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_hires2);
 
 	igt_assert_plane_visible(data->drm_fd, pipe, true);
 
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index e61bc84624b3..a53be00141f0 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -276,8 +276,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 
 		igt_display_commit2(&data->display, COMMIT_ATOMIC);
 
-		igt_pipe_crc_drain(data->pipe_crc);
-		igt_pipe_crc_get_single(data->pipe_crc, &crc);
+		igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, &crc);
 
 		igt_assert_crc_equal(&data->ref_crc, &crc);
 
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 6cb5858adb0f..b994cc5d24fe 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -235,8 +235,8 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
 	if (plane->type != DRM_PLANE_TYPE_CURSOR)
 		igt_plane_set_position(plane, data->pos_x, data->pos_y);
 	igt_display_commit2(display, COMMIT_ATOMIC);
-	igt_pipe_crc_drain(data->pipe_crc);
-	igt_pipe_crc_get_single(data->pipe_crc, &data->flip_crc);
+
+	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->flip_crc);
 
 	/*
 	  * Prepare the non-rotated flip fb.
@@ -259,8 +259,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
 		igt_plane_set_position(plane, data->pos_x, data->pos_y);
 	igt_display_commit2(display, COMMIT_ATOMIC);
 
-	igt_pipe_crc_drain(data->pipe_crc);
-	igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
+	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->ref_crc);
 
 	/*
 	 * Prepare the non-rotated reference fb.
@@ -310,8 +309,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
 	igt_assert_eq(ret, 0);
 
 	/* Check CRC */
-	igt_pipe_crc_drain(data->pipe_crc);
-	igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
+	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
 	igt_assert_crc_equal(&data->ref_crc, &crc_output);
 
 	/*
@@ -334,8 +332,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
 			igt_assert_eq(ret, 0);
 		}
 		kmstest_wait_for_pageflip(data->gfx_fd);
-		igt_pipe_crc_drain(data->pipe_crc);
-		igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
+		igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
 		igt_assert_crc_equal(&data->flip_crc,
 				     &crc_output);
 	}
-- 
2.18.0

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

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

* [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests.
  2018-07-24 13:59 [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Maarten Lankhorst
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current() Maarten Lankhorst
@ 2018-07-24 13:59 ` Maarten Lankhorst
  2018-07-26  6:55   ` Petri Latvala
  2018-07-24 15:17 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Patchwork
  2018-07-24 18:21 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-07-24 13:59 UTC (permalink / raw)
  To: igt-dev

Add tests excercising switching between various scaled/unscaled
transitions to and from NV12, since some of the workarounds mention
this may fail.

We also add NV12 specific clipping/scaling tests, to make sure
that NV12 src coordinates are always programmed as a multiple of 4
correctly, and verify scaling/clipping works with CRC tests.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 tests/Makefile.sources |   1 +
 tests/kms_nv12.c       | 743 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 3 files changed, 745 insertions(+)
 create mode 100644 tests/kms_nv12.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c84933f1d971..dab9260f584d 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -190,6 +190,7 @@ TESTS_progs = \
 	kms_invalid_dotclock \
 	kms_legacy_colorkey \
 	kms_mmap_write_crc \
+	kms_nv12 \
 	kms_panel_fitting \
 	kms_pipe_b_c_ivb \
 	kms_pipe_crc_basic \
diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c
new file mode 100644
index 000000000000..05ae83e53dda
--- /dev/null
+++ b/tests/kms_nv12.c
@@ -0,0 +1,743 @@
+/*
+ * 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 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.
+ *
+ * Authors:
+ *    Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
+ */
+#include "config.h"
+
+#include "igt.h"
+#include <cairo.h>
+#include <errno.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <sys/time.h>
+
+typedef struct {
+	igt_display_t display;
+
+	igt_pipe_crc_t *pipe_crc;
+	struct igt_fb fb[4];
+} data_t;
+
+static bool plane_supports_format(igt_plane_t *plane, uint32_t format)
+{
+	int i;
+
+	if (!igt_fb_supported_format(format))
+		return false;
+
+	for (i = 0; i < plane->drm_plane->count_formats; i++)
+		if (plane->drm_plane->formats[i] == format)
+			return true;
+
+	return false;
+}
+
+static bool pipe_supports_format(igt_display_t *display, enum pipe pipe, uint32_t format)
+{
+	igt_plane_t *plane;
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		if (plane_supports_format(plane, format))
+			return true;
+
+	return false;
+}
+
+static void remove_fbs(data_t *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(data->fb); i++)
+		igt_remove_fb(data->display.drm_fd, &data->fb[i]);
+}
+
+static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+	igt_display_t *display = &data->display;
+
+	remove_fbs(data);
+	igt_display_reset(display);
+	igt_output_set_pipe(output, pipe);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_pipe_crc_free(data->pipe_crc);
+	data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+}
+
+static void set_fb(igt_plane_t *plane, struct igt_fb *fb, bool scaled)
+{
+	igt_plane_set_fb(plane, fb);
+
+	if (scaled && fb)
+		igt_fb_set_size(fb, plane, fb->width, 16);
+
+	if (fb && fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED) {
+		igt_plane_set_rotation(plane, IGT_ROTATION_90);
+		igt_plane_set_size(plane, fb->height, fb->width);
+	} else
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+}
+
+static void nv12_rgb_switch(data_t *data, enum pipe pipe, igt_output_t *output, bool scaled, unsigned format)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane;
+	int i;
+	igt_crc_t ref_crc[4], crc;
+
+	prepare_crtc(data, pipe, output);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      format, LOCAL_I915_FORMAT_MOD_X_TILED,
+			      &data->fb[0]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+			      format, LOCAL_I915_FORMAT_MOD_Y_TILED,
+			      &data->fb[1]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_X_TILED,
+			      &data->fb[2]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_Y_TILED,
+			      &data->fb[3]);
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+		if (!plane_supports_format(plane, DRM_FORMAT_XRGB8888))
+			continue;
+
+		/* We only have few scalers, don't use 1 for unused planes */
+		if (plane->type != DRM_PLANE_TYPE_CURSOR && !scaled)
+			set_fb(plane, &data->fb[3], false);
+	}
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+		const int seq[] = {
+			2, 0, 2, 1, 3, 1, 3
+		};
+
+		if (!plane_supports_format(plane, format))
+			continue;
+
+		/* Collect reference crc with toggle in between. */
+		for (i = 0; i < ARRAY_SIZE(ref_crc); i++) {
+			set_fb(plane, &data->fb[i], scaled);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+
+			igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc[i]);
+
+			set_fb(plane, NULL, scaled);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+		}
+
+		for (i = 0; i < ARRAY_SIZE(seq); i++) {
+			int j = seq[i];
+
+			set_fb(plane, &data->fb[j], scaled);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+
+			igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+			igt_assert_crc_equal(&ref_crc[j], &crc);
+		}
+
+		/* We only have few scalers, don't use 1 for unused planes */
+		if (scaled)
+			igt_plane_set_fb(plane, NULL);
+	}
+}
+
+#define assert_collected_crc_equal(pipe_crc, crc, ref_crc, rotation) \
+	do { \
+		igt_pipe_crc_get_current(display->drm_fd, pipe_crc, crc);	\
+		igt_warn_on_f(!igt_check_crc_equal(ref_crc, crc), "mismatch with %x rotation!\n", rotation);	\
+	} while (0)
+
+static void set_src_coords(igt_plane_t *plane, struct igt_fb *fb,
+			   igt_rotation_t rot, int vis, int hidden)
+{
+	switch (rot) {
+	case IGT_ROTATION_0:
+		igt_fb_set_position(fb, plane, fb->width / 2 - vis, fb->height / 2 - vis);
+		break;
+	case IGT_ROTATION_90:
+		igt_fb_set_position(fb, plane, fb->width / 2 - hidden, fb->height / 2 - vis);
+		break;
+	case IGT_ROTATION_180:
+		igt_fb_set_position(fb, plane, fb->width / 2 - hidden, fb->height / 2 - hidden);
+		break;
+	case IGT_ROTATION_270:
+		igt_fb_set_position(fb, plane, fb->width / 2 - vis, fb->height / 2 - hidden);
+		break;
+	default: igt_assert(0);
+	}
+	igt_fb_set_size(fb, plane, vis + hidden, vis + hidden);
+}
+
+static void nv12_valid_width_plane(data_t *data, drmModeModeInfo *mode,
+				   igt_plane_t *plane, struct igt_fb *fb,
+				   igt_rotation_t rot, igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc;
+	struct igt_fb *clip_fb = &data->fb[2];
+	int i;
+
+	igt_plane_set_fb(plane, fb);
+	igt_plane_set_rotation(plane, rot);
+	if (rot & (IGT_ROTATION_90 | IGT_ROTATION_270))
+		igt_plane_set_size(plane, fb->height, fb->width);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	/* Just clipping.. */
+	igt_plane_set_fb(plane, clip_fb);
+	if (rot & (IGT_ROTATION_90 | IGT_ROTATION_270))
+		igt_plane_set_size(plane, clip_fb->height, clip_fb->width);
+	igt_plane_set_position(plane, mode->hdisplay - clip_fb->width / 2, mode->vdisplay - clip_fb->height / 2);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+
+	/* Clipping and scaling. */
+	igt_fb_set_position(clip_fb, plane, clip_fb->width / 2 - 16, clip_fb->height / 2 - 16);
+	igt_fb_set_size(clip_fb, plane, 32, 32);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+
+	/* Invalid < 16 src visible. */
+	set_src_coords(plane, clip_fb, rot, 8, 8);
+	igt_assert_eq(igt_display_try_commit2(display, COMMIT_ATOMIC), -EINVAL);
+
+	/* Try different alignments for x/y to see if any hit underruns */
+	for (i = 1; i < 4; i++) {
+		set_src_coords(plane, clip_fb, rot, 16 + 2 * i, 16 - 2 * i);
+		igt_display_commit2(display, COMMIT_ATOMIC);
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+
+		/* And also show more of the screen */
+		set_src_coords(plane, clip_fb, rot, 16 + i, 16 + i);
+		if (rot == IGT_ROTATION_270 || rot == (IGT_ROTATION_90 | IGT_REFLECT_X)) {
+			igt_assert_eq(igt_display_try_commit2(display, COMMIT_ATOMIC), -EINVAL);
+			continue;
+		}
+		igt_display_commit2(display, COMMIT_ATOMIC);
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+	}
+
+	/*
+	 * As a final test, try with 16 src visible, and various invisible
+	 * components to check clipping doesn't drop the visible src below 16.
+	 */
+	for (i = 0; i < 16; i += 4) {
+		set_src_coords(plane, clip_fb, rot, 16, i);
+		if (rot & (IGT_ROTATION_90 | IGT_ROTATION_270))
+			igt_plane_set_size(plane, (clip_fb->height / 32) * (16 + i),
+					   (clip_fb->width / 32) * (16 + i));
+		else
+			igt_plane_set_size(plane, (clip_fb->width / 32) * (16 + i),
+					   (clip_fb->height / 32) * (16 + i));
+		igt_display_commit2(display, COMMIT_ATOMIC);
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+	}
+}
+
+static void nv12_valid_width(data_t *data, enum pipe pipe, igt_output_t *output, unsigned format)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane;
+	struct igt_fb *clip_fb = &data->fb[2];
+	struct igt_fb *ref_fb = &data->fb[3];
+
+	prepare_crtc(data, pipe, output);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      format, LOCAL_I915_FORMAT_MOD_X_TILED,
+			      &data->fb[0]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+			      format, LOCAL_I915_FORMAT_MOD_Y_TILED,
+			      &data->fb[1]);
+
+	igt_create_fb(display->drm_fd, 64, 64, format,
+		      LOCAL_I915_FORMAT_MOD_Y_TILED, ref_fb);
+
+	igt_create_fb(display->drm_fd, 2 * ref_fb->width,
+		      2 * ref_fb->height, format,
+		      LOCAL_I915_FORMAT_MOD_Y_TILED, clip_fb);
+
+	igt_pipe_crc_start(data->pipe_crc);
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+		igt_crc_t ref_crc[4];
+		cairo_t *cr;
+		static const double colors[4][3] = {
+			{ 1., 0., 0. },
+			{ 0., 1., 0. },
+			{ 0., 0., 1. },
+			{ 1., 1., 1. }
+		};
+		int i;
+
+		if (!plane_supports_format(plane, format))
+			continue;
+
+		/* Draw the FB that will be used for clipping tests. */
+		cr = igt_get_cairo_ctx(display->drm_fd, clip_fb);
+		igt_paint_color(cr, 0, 0, clip_fb->width / 2, clip_fb->height / 2,
+				colors[0][0], colors[0][1], colors[0][2]);
+
+		igt_paint_color(cr, clip_fb->width / 2, 0,
+				clip_fb->width / 2, clip_fb->height / 2,
+				colors[1][0], colors[1][1], colors[1][2]);
+
+		igt_paint_color(cr, clip_fb->width / 2, clip_fb->height / 2,
+				clip_fb->width / 2, clip_fb->height / 2,
+				colors[2][0], colors[2][1], colors[2][2]);
+
+		igt_paint_color(cr, 0, clip_fb->height / 2,
+				clip_fb->width / 2, clip_fb->height / 2,
+				colors[3][0], colors[3][1], colors[3][2]);
+
+		igt_put_cairo_ctx(display->drm_fd, clip_fb, cr);
+
+		/* Draw all reference FB's to collect the CRC. */
+		for (i = 0; i < 4; i++) {
+			cr = igt_get_cairo_ctx(display->drm_fd, ref_fb);
+			igt_paint_color(cr, 0, 0, ref_fb->width, ref_fb->height,
+					colors[i][0], colors[i][1], colors[i][2]);
+			igt_put_cairo_ctx(display->drm_fd, &data->fb[3], cr);
+
+			if (!i) {
+				igt_plane_set_fb(plane, ref_fb);
+				igt_plane_set_position(plane, mode->hdisplay - ref_fb->width, mode->vdisplay - ref_fb->height);
+				igt_display_commit2(display, COMMIT_ATOMIC);
+			} else {
+				igt_dirty_fb(display->drm_fd, ref_fb);
+				igt_wait_for_vblank(display->drm_fd, pipe);
+			}
+
+			igt_pipe_crc_drain(data->pipe_crc);
+			igt_pipe_crc_get_single(data->pipe_crc, &ref_crc[i]);
+		}
+
+		igt_plane_set_fb(plane, NULL);
+		igt_plane_set_position(plane, 0, 0);
+
+		nv12_valid_width_plane(data, mode, plane, &data->fb[0], IGT_ROTATION_0, &ref_crc[0]);
+		nv12_valid_width_plane(data, mode, plane, &data->fb[1], IGT_ROTATION_90, &ref_crc[1]);
+		nv12_valid_width_plane(data, mode, plane, &data->fb[0], IGT_ROTATION_180, &ref_crc[2]);
+		nv12_valid_width_plane(data, mode, plane, &data->fb[1], IGT_ROTATION_270, &ref_crc[3]);
+
+		igt_plane_set_fb(plane, NULL);
+	}
+}
+
+static bool nv12_commit_rotated(igt_display_t *display, igt_rotation_t rot)
+{
+	if (igt_display_try_commit2(display, COMMIT_ATOMIC) < 0) {
+		igt_assert(rot == IGT_ROTATION_270 || rot == (IGT_ROTATION_90 | IGT_REFLECT_X));
+		igt_debug("Skipping rotated test, alignment is wrong.\n");
+		return false;
+	}
+
+	return true;
+}
+
+static void nv12_crop_scale_bug_plane(data_t *data, drmModeModeInfo *mode,
+				      igt_plane_t *plane, struct igt_fb *fb,
+				      igt_rotation_t rot, igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc;
+
+	igt_plane_set_fb(plane, fb);
+	igt_plane_set_rotation(plane, rot);
+	igt_fb_set_position(fb, plane, fb->width / 4 + 1, fb->height / 4 + 1);
+	igt_fb_set_size(fb, plane, fb->width / 2 + 2, fb->height / 2 + 2);
+
+	igt_plane_set_position(plane, 64, 64);
+	igt_plane_set_size(plane, 256, 256);
+
+	if (nv12_commit_rotated(display, rot))
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rot);
+}
+
+static void nv12_crop_scale_bug(data_t *data, enum pipe pipe, igt_output_t *output, unsigned format)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane;
+	struct igt_fb *clip_fb = &data->fb[2];
+	struct igt_fb *ref_fb = &data->fb[3];
+
+	prepare_crtc(data, pipe, output);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      format, LOCAL_I915_FORMAT_MOD_X_TILED, &data->fb[0]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+		      format, LOCAL_I915_FORMAT_MOD_Y_TILED, &data->fb[1]);
+
+	igt_create_color_fb(display->drm_fd, 256, 256, format,
+			    LOCAL_I915_FORMAT_MOD_Y_TILED, 1., 1., 1., ref_fb);
+
+	igt_create_fb(display->drm_fd, 80, 128, format,
+		      LOCAL_I915_FORMAT_MOD_Y_TILED, clip_fb);
+
+	igt_pipe_crc_start(data->pipe_crc);
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+		igt_crc_t ref_crc;
+		cairo_t *cr;
+
+		if (!plane_supports_format(plane, format))
+			continue;
+
+		/* Draw the FB that will be used for clipping tests. */
+		cr = igt_get_cairo_ctx(display->drm_fd, clip_fb);
+		igt_paint_color(cr, 0, 0, clip_fb->width, clip_fb->height, .5, .5, .5);
+
+		igt_paint_color(cr, clip_fb->width / 4, clip_fb->height / 4,
+				clip_fb->width / 2, clip_fb->height / 2, 1., 1., 1.);
+
+		igt_put_cairo_ctx(display->drm_fd, clip_fb, cr);
+
+		igt_plane_set_fb(plane, ref_fb);
+		igt_plane_set_position(plane, 64, 64);
+		igt_display_commit2(display, COMMIT_ATOMIC);
+
+		igt_pipe_crc_drain(data->pipe_crc);
+		igt_pipe_crc_get_single(data->pipe_crc, &ref_crc);
+
+		nv12_crop_scale_bug_plane(data, mode, plane, clip_fb, IGT_ROTATION_0, &ref_crc);
+		nv12_crop_scale_bug_plane(data, mode, plane, clip_fb, IGT_ROTATION_90, &ref_crc);
+		nv12_crop_scale_bug_plane(data, mode, plane, clip_fb, IGT_ROTATION_180, &ref_crc);
+		nv12_crop_scale_bug_plane(data, mode, plane, clip_fb, IGT_ROTATION_270, &ref_crc);
+
+		igt_plane_set_fb(plane, NULL);
+	}
+}
+
+static void nv12_chroma_subpixel_clip_plane_with_cursor(data_t *data,
+							igt_plane_t *plane,
+							struct igt_fb *clip_fb,
+							igt_plane_t *cursor,
+							struct igt_fb *cursor_fb,
+							igt_crc_t *ref_crc,
+							int cursor_x, int cursor_y)
+{
+	igt_display_t *display = &data->display;
+
+	igt_plane_set_fb(plane, clip_fb);
+	igt_plane_set_position(plane, 64, 64);
+	igt_plane_set_rotation(plane, IGT_ROTATION_0);
+
+	igt_plane_set_fb(cursor, cursor_fb);
+	igt_plane_set_position(cursor, cursor_x, cursor_y);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_pipe_crc_drain(data->pipe_crc);
+	igt_pipe_crc_get_single(data->pipe_crc, ref_crc);
+
+	igt_plane_set_fb(cursor, NULL);
+}
+
+static void nv12_chroma_subpixel_clip_plane_right(data_t *data,
+						  igt_rotation_t rotation,
+						  igt_plane_t *plane,
+						  struct igt_fb *clip_fb,
+						  igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc = {};
+
+	igt_plane_set_fb(plane, clip_fb);
+	igt_plane_set_position(plane, 64, 64);
+	igt_plane_set_rotation(plane, rotation);
+	igt_plane_set_size(plane, clip_fb->width - 1, clip_fb->height);
+
+	switch (rotation) {
+	case IGT_ROTATION_180:
+		igt_fb_set_position(clip_fb, plane, 1, 0);
+	case IGT_ROTATION_0:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width - 1, clip_fb->height);
+		break;
+
+	case IGT_ROTATION_270:
+		igt_fb_set_position(clip_fb, plane, 0, 1);
+	case IGT_ROTATION_90:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width, clip_fb->height - 1);
+		break;
+	default:
+		igt_assert_f(0, "Unhandled rotation %x\n", rotation);
+	}
+
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rotation);
+}
+
+static void nv12_chroma_subpixel_clip_plane_bottom(data_t *data,
+						   igt_rotation_t rotation,
+						   igt_plane_t *plane,
+						   struct igt_fb *clip_fb,
+						   igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc = {};
+
+	igt_plane_set_fb(plane, clip_fb);
+	igt_plane_set_position(plane, 64, 64);
+	igt_plane_set_rotation(plane, rotation);
+	igt_plane_set_size(plane, clip_fb->width, clip_fb->height - 1);
+
+	switch (rotation) {
+	case IGT_ROTATION_180:
+		igt_fb_set_position(clip_fb, plane, 0, 1);
+	case IGT_ROTATION_0:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width, clip_fb->height - 1);
+		break;
+
+	case IGT_ROTATION_270:
+		igt_fb_set_position(clip_fb, plane, 1, 0);
+	case IGT_ROTATION_90:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width - 1, clip_fb->height);
+		break;
+	default:
+		igt_assert_f(0, "Unhandled rotation %x\n", rotation);
+	}
+
+	if (nv12_commit_rotated(display, rotation))
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rotation);
+}
+
+static void nv12_chroma_subpixel_clip_plane_left(data_t *data,
+						 igt_rotation_t rotation,
+						 igt_plane_t *plane,
+						 struct igt_fb *clip_fb,
+						 igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc = {};
+
+	igt_plane_set_fb(plane, clip_fb);
+	igt_plane_set_position(plane, 65, 64);
+	igt_plane_set_rotation(plane, rotation);
+	igt_plane_set_size(plane, clip_fb->width - 1, clip_fb->height);
+
+	switch (rotation) {
+	case IGT_ROTATION_0:
+		igt_fb_set_position(clip_fb, plane, 1, 0);
+	case IGT_ROTATION_180:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width - 1, clip_fb->height);
+		break;
+
+	case IGT_ROTATION_90:
+		igt_fb_set_position(clip_fb, plane, 0, 1);
+	case IGT_ROTATION_270:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width, clip_fb->height - 1);
+		break;
+	default:
+		igt_assert_f(0, "Unhandled rotation %x\n", rotation);
+	}
+
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rotation);
+}
+
+
+static void nv12_chroma_subpixel_clip_plane_top(data_t *data,
+						igt_rotation_t rotation,
+						igt_plane_t *plane,
+						struct igt_fb *clip_fb,
+						igt_crc_t *ref_crc)
+{
+	igt_display_t *display = &data->display;
+	igt_crc_t crc = {};
+
+	igt_plane_set_fb(plane, clip_fb);
+	igt_plane_set_position(plane, 64, 65);
+	igt_plane_set_rotation(plane, rotation);
+	igt_plane_set_size(plane, clip_fb->width, clip_fb->height - 1);
+
+	switch (rotation) {
+	case IGT_ROTATION_0:
+		igt_fb_set_position(clip_fb, plane, 0, 1);
+	case IGT_ROTATION_180:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width, clip_fb->height - 1);
+		break;
+
+	case IGT_ROTATION_90:
+		igt_fb_set_position(clip_fb, plane, 1, 0);
+	case IGT_ROTATION_270:
+		igt_fb_set_size(clip_fb, plane, clip_fb->width - 1, clip_fb->height);
+		break;
+	default:
+		igt_assert_f(0, "Unhandled rotation %x\n", rotation);
+	}
+
+	if (nv12_commit_rotated(display, rotation))
+		assert_collected_crc_equal(data->pipe_crc, &crc, ref_crc, rotation);
+}
+
+static void nv12_chroma_subpixel_clip(data_t *data, enum pipe pipe, igt_output_t *output, unsigned format)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	igt_display_t *display = &data->display;
+	igt_pipe_t *pipe_obj = &display->pipes[pipe];
+	struct igt_fb *bg_fb = &data->fb[0];
+	struct igt_fb *bg_cursor_fb = &data->fb[1];
+	struct igt_fb *black_cursor_fb = &data->fb[2];
+	struct igt_fb *clip_fb = &data->fb[3];
+	igt_plane_t *primary = igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_t *cursor = igt_pipe_get_plane_type(pipe_obj, DRM_PLANE_TYPE_CURSOR);
+	igt_plane_t *plane;
+	cairo_t *cr;
+
+	prepare_crtc(data, pipe, output);
+
+	igt_create_color_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			    DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_X_TILED,
+			    .0, .0, 1., bg_fb);
+
+	igt_create_color_fb(display->drm_fd, 256, 256,
+		            DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+			    .0, .0, 1., bg_cursor_fb);
+
+	igt_create_color_fb(display->drm_fd, 256, 256,
+		            DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+			    .0, .0, .0, black_cursor_fb);
+
+	igt_create_fb(display->drm_fd, 256, 256,
+		      format, LOCAL_I915_FORMAT_MOD_Y_TILED, clip_fb);
+
+	/* Draw the FB that will be used for clipping the chroma subpixel test. */
+	cr = igt_get_cairo_ctx(display->drm_fd, clip_fb);
+
+	igt_paint_color(cr, 0, 0, clip_fb->width, clip_fb->height, 1., 0., 0.);
+	igt_paint_color(cr, 2, 2, clip_fb->width - 4, clip_fb->height - 4, 0., 1., 1.);
+	igt_paint_color(cr, 4, 4, clip_fb->width - 8, clip_fb->height - 8, .75, .75, 1.);
+
+	/* White dots in the corners. */
+	igt_paint_color(cr, 0, 0, 6, 6, 1., 1., 1.);
+	igt_paint_color(cr, clip_fb->width - 6, 0, 6, 6, 1., 1., 1.);
+	igt_paint_color(cr, 0, clip_fb->height - 6, 6, 6, 1., 1., 1.);
+	igt_paint_color(cr, clip_fb->width - 6, clip_fb->height - 6, 6, 6, 1., 1., 1.);
+
+	igt_put_cairo_ctx(display->drm_fd, clip_fb, cr);
+
+	igt_pipe_crc_start(data->pipe_crc);
+
+	igt_plane_set_fb(primary, bg_fb);
+	for_each_plane_on_pipe(display, pipe, plane) {
+		struct igt_fb *cursor_fb = plane == primary ? black_cursor_fb : bg_cursor_fb;
+		igt_crc_t ref_crc;
+
+		if (!plane_supports_format(plane, format) || plane == cursor)
+			continue;
+
+		nv12_chroma_subpixel_clip_plane_with_cursor(data, plane, clip_fb, cursor, cursor_fb, &ref_crc, 63 + clip_fb->width, 64);
+		nv12_chroma_subpixel_clip_plane_right(data, IGT_ROTATION_0, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_right(data, IGT_ROTATION_90, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_right(data, IGT_ROTATION_180, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_right(data, IGT_ROTATION_270, plane, clip_fb, &ref_crc);
+
+		nv12_chroma_subpixel_clip_plane_with_cursor(data, plane, clip_fb, cursor, cursor_fb, &ref_crc, 64, 63 + clip_fb->height);
+		nv12_chroma_subpixel_clip_plane_bottom(data, IGT_ROTATION_0, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_bottom(data, IGT_ROTATION_90, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_bottom(data, IGT_ROTATION_180, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_bottom(data, IGT_ROTATION_270, plane, clip_fb, &ref_crc);
+
+		nv12_chroma_subpixel_clip_plane_with_cursor(data, plane, clip_fb, cursor, cursor_fb, &ref_crc, 65 - cursor_fb->width, 64);
+		nv12_chroma_subpixel_clip_plane_left(data, IGT_ROTATION_0, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_left(data, IGT_ROTATION_90, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_left(data, IGT_ROTATION_180, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_left(data, IGT_ROTATION_270, plane, clip_fb, &ref_crc);
+
+		nv12_chroma_subpixel_clip_plane_with_cursor(data, plane, clip_fb, cursor, cursor_fb, &ref_crc, 64, 65 - cursor_fb->height);
+		nv12_chroma_subpixel_clip_plane_top(data, IGT_ROTATION_0, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_top(data, IGT_ROTATION_90, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_top(data, IGT_ROTATION_180, plane, clip_fb, &ref_crc);
+		nv12_chroma_subpixel_clip_plane_top(data, IGT_ROTATION_270, plane, clip_fb, &ref_crc);
+
+		/* Reset to defaults */
+		igt_plane_set_position(plane, 0, 0);
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+		igt_plane_set_fb(plane, plane == primary ? bg_fb : NULL);
+	}
+}
+
+static void run_tests_for_pipe(data_t *data, enum pipe pipe)
+{
+	igt_output_t *output;
+	igt_display_t *display = &data->display;
+
+	igt_fixture {
+		igt_display_require_output_on_pipe(display, pipe);
+		igt_require(pipe_supports_format(display, pipe, DRM_FORMAT_NV12));
+	}
+
+	igt_subtest_f("pipe-%s-nv12-rgb-switch", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_rgb_switch(data, pipe, output, false, DRM_FORMAT_NV12);
+
+	igt_subtest_f("pipe-%s-nv12-rgb-scaled-switch", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_rgb_switch(data, pipe, output, true, DRM_FORMAT_NV12);
+
+	igt_subtest_f("pipe-%s-nv12-valid-width", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_valid_width(data, pipe, output, DRM_FORMAT_NV12);
+
+	igt_subtest_f("pipe-%s-nv12-crop-scale-bug", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_crop_scale_bug(data, pipe, output, DRM_FORMAT_NV12);
+
+	igt_subtest_f("pipe-%s-nv12-chroma-subpixel-clip", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_chroma_subpixel_clip(data, pipe, output, DRM_FORMAT_NV12);
+}
+
+igt_main
+{
+	data_t data = {};
+	enum pipe pipe;
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		data.display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+
+		kmstest_set_vt_graphics_mode();
+		igt_display_init(&data.display, data.display.drm_fd);
+		igt_require(data.display.is_atomic);
+		igt_require_pipe_crc(data.display.drm_fd);
+	}
+
+	for_each_pipe_static(pipe)
+		igt_subtest_group
+			run_tests_for_pipe(&data, pipe);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+		close(data.display.drm_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 32c2156c6f4d..e5fab06bc2f2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -166,6 +166,7 @@ test_progs = [
 	'kms_invalid_dotclock',
 	'kms_legacy_colorkey',
 	'kms_mmap_write_crc',
+	'kms_nv12',
 	'kms_panel_fitting',
 	'kms_pipe_b_c_ivb',
 	'kms_pipe_crc_basic',
-- 
2.18.0

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

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc.
  2018-07-24 13:59 [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Maarten Lankhorst
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current() Maarten Lankhorst
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests Maarten Lankhorst
@ 2018-07-24 15:17 ` Patchwork
  2018-07-24 18:21 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-07-24 15:17 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc.
URL   : https://patchwork.freedesktop.org/series/47129/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4534 -> IGTPW_1641 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#103841)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      {fi-bsw-kefka}:     PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@gem_exec_flush@basic-wb-ro-default:
      fi-glk-j4005:       DMESG-WARN (fdo#105719) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

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

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719


== Participating hosts (52 -> 44) ==

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-caroline fi-byt-clapper fi-bdw-samus 


== Build changes ==

    * IGT: IGT_4572 -> IGTPW_1641

  CI_DRM_4534: a59bbda34ede6f5685fdc86b58f143bada751617 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1641: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1641/
  IGT_4572: 9b064015df14506b23cd2d7245a73e1b1d16ee1f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_nv12@pipe-a-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-a-nv12-crop-scale-bug
+igt@kms_nv12@pipe-a-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-a-nv12-rgb-switch
+igt@kms_nv12@pipe-a-nv12-valid-width
+igt@kms_nv12@pipe-b-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-b-nv12-crop-scale-bug
+igt@kms_nv12@pipe-b-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-b-nv12-rgb-switch
+igt@kms_nv12@pipe-b-nv12-valid-width
+igt@kms_nv12@pipe-c-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-c-nv12-crop-scale-bug
+igt@kms_nv12@pipe-c-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-c-nv12-rgb-switch
+igt@kms_nv12@pipe-c-nv12-valid-width
+igt@kms_nv12@pipe-d-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-d-nv12-crop-scale-bug
+igt@kms_nv12@pipe-d-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-d-nv12-rgb-switch
+igt@kms_nv12@pipe-d-nv12-valid-width
+igt@kms_nv12@pipe-e-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-e-nv12-crop-scale-bug
+igt@kms_nv12@pipe-e-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-e-nv12-rgb-switch
+igt@kms_nv12@pipe-e-nv12-valid-width
+igt@kms_nv12@pipe-f-nv12-chroma-subpixel-clip
+igt@kms_nv12@pipe-f-nv12-crop-scale-bug
+igt@kms_nv12@pipe-f-nv12-rgb-scaled-switch
+igt@kms_nv12@pipe-f-nv12-rgb-switch
+igt@kms_nv12@pipe-f-nv12-valid-width

== Logs ==

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

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

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc.
  2018-07-24 13:59 [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2018-07-24 15:17 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Patchwork
@ 2018-07-24 18:21 ` Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-07-24 18:21 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc.
URL   : https://patchwork.freedesktop.org/series/47129/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4572_full -> IGTPW_1641_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1641_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1641_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/47129/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    {igt@kms_nv12@pipe-a-nv12-chroma-subpixel-clip}:
      shard-kbl:          NOTRUN -> DMESG-FAIL +3

    {igt@kms_nv12@pipe-a-nv12-crop-scale-bug}:
      shard-glk:          NOTRUN -> WARN +2

    {igt@kms_nv12@pipe-b-nv12-chroma-subpixel-clip}:
      shard-glk:          NOTRUN -> DMESG-FAIL +2

    {igt@kms_nv12@pipe-b-nv12-crop-scale-bug}:
      shard-kbl:          NOTRUN -> WARN +1

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-apl:          SKIP -> PASS +3

    igt@gem_exec_schedule@deep-bsd:
      shard-glk:          SKIP -> PASS +3

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +2

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_parallel@blt:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          PASS -> FAIL (fdo#105767)

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          NOTRUN -> FAIL (fdo#105454, fdo#106509)

    igt@kms_flip@2x-blocking-wf_vblank:
      shard-glk:          PASS -> FAIL (fdo#100368)

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

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

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS +1

    igt@kms_flip@plain-flip-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS +1

    igt@kms_universal_plane@cursor-fb-leak-pipe-a:
      shard-apl:          FAIL (fdo#107241) -> PASS

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-kbl:          FAIL (fdo#105010) -> PASS

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

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#107241 https://bugs.freedesktop.org/show_bug.cgi?id=107241
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4572 -> IGTPW_1641
    * Linux: CI_DRM_4533 -> CI_DRM_4534

  CI_DRM_4533: 712e7047cfc7a129df6d324f59cc5d4eb2879285 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4534: a59bbda34ede6f5685fdc86b58f143bada751617 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1641: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1641/
  IGT_4572: 9b064015df14506b23cd2d7245a73e1b1d16ee1f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests.
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests Maarten Lankhorst
@ 2018-07-26  6:55   ` Petri Latvala
  2018-07-26  8:57     ` Maarten Lankhorst
  0 siblings, 1 reply; 9+ messages in thread
From: Petri Latvala @ 2018-07-26  6:55 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

On Tue, Jul 24, 2018 at 03:59:27PM +0200, Maarten Lankhorst wrote:
> Add tests excercising switching between various scaled/unscaled
> transitions to and from NV12, since some of the workarounds mention
> this may fail.
> 
> We also add NV12 specific clipping/scaling tests, to make sure
> that NV12 src coordinates are always programmed as a multiple of 4
> correctly, and verify scaling/clipping works with CRC tests.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_nv12.c       | 743 +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  3 files changed, 745 insertions(+)
>  create mode 100644 tests/kms_nv12.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index c84933f1d971..dab9260f584d 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -190,6 +190,7 @@ TESTS_progs = \
>  	kms_invalid_dotclock \
>  	kms_legacy_colorkey \
>  	kms_mmap_write_crc \
> +	kms_nv12 \
>  	kms_panel_fitting \
>  	kms_pipe_b_c_ivb \
>  	kms_pipe_crc_basic \
> diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c
> new file mode 100644
> index 000000000000..05ae83e53dda
> --- /dev/null
> +++ b/tests/kms_nv12.c
> @@ -0,0 +1,743 @@
> +/*
> + * Permission is hereby granted, free of charge, to any person obtaining a


Where's the copyright line?



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

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

* Re: [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests.
  2018-07-26  6:55   ` Petri Latvala
@ 2018-07-26  8:57     ` Maarten Lankhorst
  0 siblings, 0 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2018-07-26  8:57 UTC (permalink / raw)
  To: igt-dev

Op 26-07-18 om 08:55 schreef Petri Latvala:
> On Tue, Jul 24, 2018 at 03:59:27PM +0200, Maarten Lankhorst wrote:
>> Add tests excercising switching between various scaled/unscaled
>> transitions to and from NV12, since some of the workarounds mention
>> this may fail.
>>
>> We also add NV12 specific clipping/scaling tests, to make sure
>> that NV12 src coordinates are always programmed as a multiple of 4
>> correctly, and verify scaling/clipping works with CRC tests.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  tests/Makefile.sources |   1 +
>>  tests/kms_nv12.c       | 743 +++++++++++++++++++++++++++++++++++++++++
>>  tests/meson.build      |   1 +
>>  3 files changed, 745 insertions(+)
>>  create mode 100644 tests/kms_nv12.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index c84933f1d971..dab9260f584d 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -190,6 +190,7 @@ TESTS_progs = \
>>  	kms_invalid_dotclock \
>>  	kms_legacy_colorkey \
>>  	kms_mmap_write_crc \
>> +	kms_nv12 \
>>  	kms_panel_fitting \
>>  	kms_pipe_b_c_ivb \
>>  	kms_pipe_crc_basic \
>> diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c
>> new file mode 100644
>> index 000000000000..05ae83e53dda
>> --- /dev/null
>> +++ b/tests/kms_nv12.c
>> @@ -0,0 +1,743 @@
>> +/*
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>
> Where's the copyright line?
>
>
>
Indeed, good catch. :)

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

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

* Re: [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current()
  2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current() Maarten Lankhorst
@ 2018-08-14 15:18   ` Daniel Vetter
  2018-08-14 15:36     ` Maarten Lankhorst
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2018-08-14 15:18 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: igt-dev

On Tue, Jul 24, 2018 at 03:59:26PM +0200, Maarten Lankhorst wrote:
> This is a more race free of accomplishing the same.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Grep says you missed a few. I'd also go even further and entirely remove
igt_pipe_crc_get_single - the only place it's safe to call is right after
_start, and there there's not really a functional difference between
get_single and get_current.

Anyway, on patch 1: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> if you respin
this one here to convert all places where we have drain+get_single.
-Daniel

> ---
>  tests/kms_available_modes_crc.c |  3 +--
>  tests/kms_cursor_legacy.c       |  3 +--
>  tests/kms_plane.c               |  6 +-----
>  tests/kms_plane_lowres.c        |  6 ++----
>  tests/kms_plane_multiple.c      |  3 +--
>  tests/kms_rotation_crc.c        | 13 +++++--------
>  6 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
> index b70ef5d7d4c0..69a6a37d8ebd 100644
> --- a/tests/kms_available_modes_crc.c
> +++ b/tests/kms_available_modes_crc.c
> @@ -400,8 +400,7 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
>  		igt_display_commit2(&data->display, data->commit);
>  
>  		if (do_crc) {
> -			igt_pipe_crc_drain(data->pipe_crc);
> -			igt_pipe_crc_get_single(data->pipe_crc, &current_crc);
> +			igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &current_crc);
>  
>  			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>  				if (!igt_check_crc_equal(&current_crc,
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index 85340d43e5b6..82ceebcb0809 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -1334,8 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>  
>  		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
>  
> -		igt_pipe_crc_drain(pipe_crc);
> -		igt_pipe_crc_get_single(pipe_crc, &test_crc);
> +		igt_pipe_crc_get_current(display->drm_fd, pipe_crc, &test_crc);
>  
>  		igt_spin_batch_free(display->drm_fd, spin);
>  
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index f9e123f0d1ea..3999dde8f694 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -419,11 +419,7 @@ static void test_format_plane_color(data_t *data, enum pipe pipe,
>  	igt_plane_set_fb(plane, fb);
>  
>  	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_UNIVERSAL);
> -
> -	/* make sure the crc we get is for the new fb */
> -	igt_wait_for_vblank(data->drm_fd, pipe);
> -	igt_pipe_crc_drain(data->pipe_crc);
> -	igt_pipe_crc_get_single(data->pipe_crc, crc);
> +	igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, crc);
>  
>  	igt_remove_fb(data->drm_fd, &old_fb);
>  }
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index d1e4b3ca536e..5e83ef5630b5 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -221,8 +221,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>  	check_mode(&mode_lowres, mode2);
>  
>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> -	igt_pipe_crc_drain(pipe_crc);
> -	igt_pipe_crc_get_single(pipe_crc, &crc_lowres);
> +	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_lowres);
>  
>  	igt_assert_plane_visible(data->drm_fd, pipe, false);
>  
> @@ -236,8 +235,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>  
>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
> -	igt_pipe_crc_drain(pipe_crc);
> -	igt_pipe_crc_get_single(pipe_crc, &crc_hires2);
> +	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_hires2);
>  
>  	igt_assert_plane_visible(data->drm_fd, pipe, true);
>  
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index e61bc84624b3..a53be00141f0 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -276,8 +276,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>  
>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>  
> -		igt_pipe_crc_drain(data->pipe_crc);
> -		igt_pipe_crc_get_single(data->pipe_crc, &crc);
> +		igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, &crc);
>  
>  		igt_assert_crc_equal(&data->ref_crc, &crc);
>  
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 6cb5858adb0f..b994cc5d24fe 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -235,8 +235,8 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>  	if (plane->type != DRM_PLANE_TYPE_CURSOR)
>  		igt_plane_set_position(plane, data->pos_x, data->pos_y);
>  	igt_display_commit2(display, COMMIT_ATOMIC);
> -	igt_pipe_crc_drain(data->pipe_crc);
> -	igt_pipe_crc_get_single(data->pipe_crc, &data->flip_crc);
> +
> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->flip_crc);
>  
>  	/*
>  	  * Prepare the non-rotated flip fb.
> @@ -259,8 +259,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>  		igt_plane_set_position(plane, data->pos_x, data->pos_y);
>  	igt_display_commit2(display, COMMIT_ATOMIC);
>  
> -	igt_pipe_crc_drain(data->pipe_crc);
> -	igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->ref_crc);
>  
>  	/*
>  	 * Prepare the non-rotated reference fb.
> @@ -310,8 +309,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
>  	igt_assert_eq(ret, 0);
>  
>  	/* Check CRC */
> -	igt_pipe_crc_drain(data->pipe_crc);
> -	igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
>  	igt_assert_crc_equal(&data->ref_crc, &crc_output);
>  
>  	/*
> @@ -334,8 +332,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
>  			igt_assert_eq(ret, 0);
>  		}
>  		kmstest_wait_for_pageflip(data->gfx_fd);
> -		igt_pipe_crc_drain(data->pipe_crc);
> -		igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
> +		igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
>  		igt_assert_crc_equal(&data->flip_crc,
>  				     &crc_output);
>  	}
> -- 
> 2.18.0
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

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

* Re: [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current()
  2018-08-14 15:18   ` Daniel Vetter
@ 2018-08-14 15:36     ` Maarten Lankhorst
  0 siblings, 0 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2018-08-14 15:36 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev

Op 14-08-18 om 17:18 schreef Daniel Vetter:
> On Tue, Jul 24, 2018 at 03:59:26PM +0200, Maarten Lankhorst wrote:
>> This is a more race free of accomplishing the same.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Grep says you missed a few. I'd also go even further and entirely remove
> igt_pipe_crc_get_single - the only place it's safe to call is right after
> _start, and there there's not really a functional difference between
> get_single and get_current.
>
> Anyway, on patch 1: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Also Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> if you respin
> this one here to convert all places where we have drain+get_single.
> -Daniel
Thanks, pushed both with fixes :)
>> ---
>>  tests/kms_available_modes_crc.c |  3 +--
>>  tests/kms_cursor_legacy.c       |  3 +--
>>  tests/kms_plane.c               |  6 +-----
>>  tests/kms_plane_lowres.c        |  6 ++----
>>  tests/kms_plane_multiple.c      |  3 +--
>>  tests/kms_rotation_crc.c        | 13 +++++--------
>>  6 files changed, 11 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
>> index b70ef5d7d4c0..69a6a37d8ebd 100644
>> --- a/tests/kms_available_modes_crc.c
>> +++ b/tests/kms_available_modes_crc.c
>> @@ -400,8 +400,7 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
>>  		igt_display_commit2(&data->display, data->commit);
>>  
>>  		if (do_crc) {
>> -			igt_pipe_crc_drain(data->pipe_crc);
>> -			igt_pipe_crc_get_single(data->pipe_crc, &current_crc);
>> +			igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &current_crc);
>>  
>>  			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>>  				if (!igt_check_crc_equal(&current_crc,
>> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> index 85340d43e5b6..82ceebcb0809 100644
>> --- a/tests/kms_cursor_legacy.c
>> +++ b/tests/kms_cursor_legacy.c
>> @@ -1334,8 +1334,7 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
>>  
>>  		igt_assert_eq(get_vblank(display->drm_fd, pipe, 0), vblank_start);
>>  
>> -		igt_pipe_crc_drain(pipe_crc);
>> -		igt_pipe_crc_get_single(pipe_crc, &test_crc);
>> +		igt_pipe_crc_get_current(display->drm_fd, pipe_crc, &test_crc);
>>  
>>  		igt_spin_batch_free(display->drm_fd, spin);
>>  
>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>> index f9e123f0d1ea..3999dde8f694 100644
>> --- a/tests/kms_plane.c
>> +++ b/tests/kms_plane.c
>> @@ -419,11 +419,7 @@ static void test_format_plane_color(data_t *data, enum pipe pipe,
>>  	igt_plane_set_fb(plane, fb);
>>  
>>  	igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> -
>> -	/* make sure the crc we get is for the new fb */
>> -	igt_wait_for_vblank(data->drm_fd, pipe);
>> -	igt_pipe_crc_drain(data->pipe_crc);
>> -	igt_pipe_crc_get_single(data->pipe_crc, crc);
>> +	igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, crc);
>>  
>>  	igt_remove_fb(data->drm_fd, &old_fb);
>>  }
>> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
>> index d1e4b3ca536e..5e83ef5630b5 100644
>> --- a/tests/kms_plane_lowres.c
>> +++ b/tests/kms_plane_lowres.c
>> @@ -221,8 +221,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>>  	check_mode(&mode_lowres, mode2);
>>  
>>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>> -	igt_pipe_crc_drain(pipe_crc);
>> -	igt_pipe_crc_get_single(pipe_crc, &crc_lowres);
>> +	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_lowres);
>>  
>>  	igt_assert_plane_visible(data->drm_fd, pipe, false);
>>  
>> @@ -236,8 +235,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>>  
>>  	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>>  
>> -	igt_pipe_crc_drain(pipe_crc);
>> -	igt_pipe_crc_get_single(pipe_crc, &crc_hires2);
>> +	igt_pipe_crc_get_current(data->display.drm_fd, pipe_crc, &crc_hires2);
>>  
>>  	igt_assert_plane_visible(data->drm_fd, pipe, true);
>>  
>> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
>> index e61bc84624b3..a53be00141f0 100644
>> --- a/tests/kms_plane_multiple.c
>> +++ b/tests/kms_plane_multiple.c
>> @@ -276,8 +276,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
>>  
>>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
>>  
>> -		igt_pipe_crc_drain(data->pipe_crc);
>> -		igt_pipe_crc_get_single(data->pipe_crc, &crc);
>> +		igt_pipe_crc_get_current(data->display.drm_fd, data->pipe_crc, &crc);
>>  
>>  		igt_assert_crc_equal(&data->ref_crc, &crc);
>>  
>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>> index 6cb5858adb0f..b994cc5d24fe 100644
>> --- a/tests/kms_rotation_crc.c
>> +++ b/tests/kms_rotation_crc.c
>> @@ -235,8 +235,8 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>>  	if (plane->type != DRM_PLANE_TYPE_CURSOR)
>>  		igt_plane_set_position(plane, data->pos_x, data->pos_y);
>>  	igt_display_commit2(display, COMMIT_ATOMIC);
>> -	igt_pipe_crc_drain(data->pipe_crc);
>> -	igt_pipe_crc_get_single(data->pipe_crc, &data->flip_crc);
>> +
>> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->flip_crc);
>>  
>>  	/*
>>  	  * Prepare the non-rotated flip fb.
>> @@ -259,8 +259,7 @@ static void prepare_fbs(data_t *data, igt_output_t *output,
>>  		igt_plane_set_position(plane, data->pos_x, data->pos_y);
>>  	igt_display_commit2(display, COMMIT_ATOMIC);
>>  
>> -	igt_pipe_crc_drain(data->pipe_crc);
>> -	igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
>> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &data->ref_crc);
>>  
>>  	/*
>>  	 * Prepare the non-rotated reference fb.
>> @@ -310,8 +309,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
>>  	igt_assert_eq(ret, 0);
>>  
>>  	/* Check CRC */
>> -	igt_pipe_crc_drain(data->pipe_crc);
>> -	igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
>> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
>>  	igt_assert_crc_equal(&data->ref_crc, &crc_output);
>>  
>>  	/*
>> @@ -334,8 +332,7 @@ static void test_single_case(data_t *data, enum pipe pipe,
>>  			igt_assert_eq(ret, 0);
>>  		}
>>  		kmstest_wait_for_pageflip(data->gfx_fd);
>> -		igt_pipe_crc_drain(data->pipe_crc);
>> -		igt_pipe_crc_get_single(data->pipe_crc, &crc_output);
>> +		igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc_output);
>>  		igt_assert_crc_equal(&data->flip_crc,
>>  				     &crc_output);
>>  	}
>> -- 
>> 2.18.0
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev


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

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

end of thread, other threads:[~2018-08-14 15:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-24 13:59 [igt-dev] [PATCH i-g-t 1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Maarten Lankhorst
2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 2/3] tests: Replace calls to igt_pipe_crc_drain + get_single with igt_pipe_crc_get_current() Maarten Lankhorst
2018-08-14 15:18   ` Daniel Vetter
2018-08-14 15:36     ` Maarten Lankhorst
2018-07-24 13:59 ` [igt-dev] [PATCH i-g-t 3/3] tests/kms_nv12: Add nv12 specific tests Maarten Lankhorst
2018-07-26  6:55   ` Petri Latvala
2018-07-26  8:57     ` Maarten Lankhorst
2018-07-24 15:17 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/3] lib/igt_debugfs: Add igt_pipe_crc_get_current() to get a crc Patchwork
2018-07-24 18:21 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).