diff mbox

[i-g-t] kms_flip_tiling: Test flips between same tiling

Message ID 1429783715-12921-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin April 23, 2015, 10:08 a.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Or in other words tests that display programming remains
correct when page flipping between tiled frame buffers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 tests/kms_flip_tiling.c | 107 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 95 insertions(+), 12 deletions(-)

Comments

Chris Wilson April 23, 2015, 10:39 a.m. UTC | #1 
On Thu, Apr 23, 2015 at 11:08:35AM +0100, Tvrtko Ursulin wrote:
> +/*
> + * Test that tiled page flips work.

Worth saying that we don't always expect linear->tiled flips to work, so
the mo is to first set the CRTC with a tiled fb, then flip to the
second.

> + */
> +static void
> +test_tiled_flip(data_t *data, igt_output_t *output, uint64_t tiling)
> +{
> +	drmModeModeInfo *mode;
> +	igt_plane_t *primary;
> +	igt_pipe_crc_t *pipe_crc;
> +	igt_crc_t reference_crc, crc;
> +	struct igt_fb fb[2];
> +	int fb_id[2], pipe, ret, width, height;
> +
> +	pipe = output->config.pipe;
> +	pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> +	igt_output_set_pipe(output, pipe);
> +
> +	mode = igt_output_get_mode(output);
> +	primary = igt_output_get_plane(output, 0);
> +
> +	width = mode->hdisplay;
> +	height = mode->vdisplay;
> +
> +	/* create two identical fbs */
> +	fb_id[0] = igt_create_fb(data->drm_fd, width, height,
> +				 DRM_FORMAT_XRGB8888, tiling, &fb[0]);
> +	igt_assert(fb_id[0]);
> +
> +	fb_id[1] = igt_create_fb(data->drm_fd, width, height,
> +				 DRM_FORMAT_XRGB8888, tiling, &fb[1]);
> +	igt_assert(fb_id[1]);
> +
> +	fill_fb(&fb[0], data, mode);
> +	fill_fb(&fb[1], data, mode);

Pretty please can we have two different patterns?

> +	/* set the crtc and generate a reference crc */
> +	igt_plane_set_fb(primary, &fb[0]);
> +	igt_display_commit(&data->display);
> +	igt_pipe_crc_collect_crc(pipe_crc, &reference_crc);
> +
> +	/* flip to the other fb */
> +	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
> +			      fb_id[1], 0, NULL);
> +	igt_assert_eq(ret, 0);
> +
> +	igt_wait_for_vblank(data->drm_fd, pipe);

I'd be more comfortable waiting for the flip-completion event (and then
a vblank for the CRC to be computed? idk)

> +	/* get a crc and compare with the reference */
> +	igt_pipe_crc_collect_crc(pipe_crc, &crc);
> +	igt_assert_crc_equal(&reference_crc, &crc);

With 2 patterns you can even test the second flip back again!

So for detecting the current breakage, we should just a
test_tiled_change that if PageFlip returns 0, the crc matches the
reference? And iterate over the modes to find one that causes an
underrun?
-Chris
Tvrtko Ursulin April 23, 2015, 11:33 a.m. UTC | #2 
On 04/23/2015 11:39 AM, Chris Wilson wrote:
> On Thu, Apr 23, 2015 at 11:08:35AM +0100, Tvrtko Ursulin wrote:
>> +/*
>> + * Test that tiled page flips work.
>
> Worth saying that we don't always expect linear->tiled flips to work, so
> the mo is to first set the CRTC with a tiled fb, then flip to the
> second.

I didn't have that in mind for this test - here it only concerns itself 
with skl_do_mmio_flip bug (2ebef630fd283642a11c48c0e0f054c3c5c59e86).

>> + */
>> +static void
>> +test_tiled_flip(data_t *data, igt_output_t *output, uint64_t tiling)
>> +{
>> +	drmModeModeInfo *mode;
>> +	igt_plane_t *primary;
>> +	igt_pipe_crc_t *pipe_crc;
>> +	igt_crc_t reference_crc, crc;
>> +	struct igt_fb fb[2];
>> +	int fb_id[2], pipe, ret, width, height;
>> +
>> +	pipe = output->config.pipe;
>> +	pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
>> +	igt_output_set_pipe(output, pipe);
>> +
>> +	mode = igt_output_get_mode(output);
>> +	primary = igt_output_get_plane(output, 0);
>> +
>> +	width = mode->hdisplay;
>> +	height = mode->vdisplay;
>> +
>> +	/* create two identical fbs */
>> +	fb_id[0] = igt_create_fb(data->drm_fd, width, height,
>> +				 DRM_FORMAT_XRGB8888, tiling, &fb[0]);
>> +	igt_assert(fb_id[0]);
>> +
>> +	fb_id[1] = igt_create_fb(data->drm_fd, width, height,
>> +				 DRM_FORMAT_XRGB8888, tiling, &fb[1]);
>> +	igt_assert(fb_id[1]);
>> +
>> +	fill_fb(&fb[0], data, mode);
>> +	fill_fb(&fb[1], data, mode);
>
> Pretty please can we have two different patterns?
>
>> +	/* set the crtc and generate a reference crc */
>> +	igt_plane_set_fb(primary, &fb[0]);
>> +	igt_display_commit(&data->display);
>> +	igt_pipe_crc_collect_crc(pipe_crc, &reference_crc);
>> +
>> +	/* flip to the other fb */
>> +	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
>> +			      fb_id[1], 0, NULL);
>> +	igt_assert_eq(ret, 0);
>> +
>> +	igt_wait_for_vblank(data->drm_fd, pipe);
>
> I'd be more comfortable waiting for the flip-completion event (and then
> a vblank for the CRC to be computed? idk)

I can look into that but shame on you for ruining my copy&paste party! ;)

>> +	/* get a crc and compare with the reference */
>> +	igt_pipe_crc_collect_crc(pipe_crc, &crc);
>> +	igt_assert_crc_equal(&reference_crc, &crc);
>
> With 2 patterns you can even test the second flip back again!
>
> So for detecting the current breakage, we should just a
> test_tiled_change that if PageFlip returns 0, the crc matches the
> reference? And iterate over the modes to find one that causes an
> underrun?

Underruns and watermarks I intended for another test case. This one is a 
really simple one - flip with the same framebuffer and check that it has 
really remained the same.

And the pre-existent test_flip_changes_tiling works in the opposite 
direction so won't cause an underrun. But will start failing if we merge 
(drm/i915/skl: Disallow tiling changes during page flip).

So third test case could be what you say, flip from linear to tiled and 
check for underrun if it works.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index 8345505..909e3fc 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -35,8 +35,7 @@ 
 #include "ioctl_wrappers.h"
 #include "intel_chipset.h"
 
-IGT_TEST_DESCRIPTION("Test that a page flip from a tiled buffer to a linear"
-		     " one works correctly.");
+IGT_TEST_DESCRIPTION("Test page flips and tiling scenarios");
 
 typedef struct {
 	int drm_fd;
@@ -44,16 +43,8 @@  typedef struct {
 	int gen;
 } data_t;
 
-/*
- * Test that a page flip from a tiled buffer to a linear one works
- * correctly. First, it sets the crtc with the linear buffer and generate
- * a reference crc for the pipe. Then, the crtc is set with the tiled one
- * and page flip to the linear one issued. A new crc is generated and
- * compared to the rerence one.
- */
-
 static void
-fill_linear_fb(struct igt_fb *fb, data_t *data, drmModeModeInfo *mode)
+fill_fb(struct igt_fb *fb, data_t *data, drmModeModeInfo *mode)
 {
 	cairo_t *cr;
 
@@ -62,6 +53,13 @@  fill_linear_fb(struct igt_fb *fb, data_t *data, drmModeModeInfo *mode)
 	cairo_destroy(cr);
 }
 
+/*
+ * Test that a page flip from a tiled buffer to a linear one works
+ * correctly. First, it sets the crtc with the linear buffer and generate
+ * a reference crc for the pipe. Then, the crtc is set with the tiled one
+ * and page flip to the linear one issued. A new crc is generated and
+ * compared to the rerence one.
+ */
 static void
 test_flip_changes_tiling(data_t *data, igt_output_t *output, uint64_t tiling)
 {
@@ -90,7 +88,7 @@  test_flip_changes_tiling(data_t *data, igt_output_t *output, uint64_t tiling)
 			      &linear);
 
 	/* fill it with a pattern that will look wrong if tiling is wrong */
-	fill_linear_fb(&linear, data, mode);
+	fill_fb(&linear, data, mode);
 
 	/* set the crtc and generate a reference crc */
 	igt_plane_set_fb(primary, &linear);
@@ -125,6 +123,67 @@  test_flip_changes_tiling(data_t *data, igt_output_t *output, uint64_t tiling)
 	igt_remove_fb(data->drm_fd, &linear);
 }
 
+/*
+ * Test that tiled page flips work.
+ */
+static void
+test_tiled_flip(data_t *data, igt_output_t *output, uint64_t tiling)
+{
+	drmModeModeInfo *mode;
+	igt_plane_t *primary;
+	igt_pipe_crc_t *pipe_crc;
+	igt_crc_t reference_crc, crc;
+	struct igt_fb fb[2];
+	int fb_id[2], pipe, ret, width, height;
+
+	pipe = output->config.pipe;
+	pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+	igt_output_set_pipe(output, pipe);
+
+	mode = igt_output_get_mode(output);
+	primary = igt_output_get_plane(output, 0);
+
+	width = mode->hdisplay;
+	height = mode->vdisplay;
+
+	/* create two identical fbs */
+	fb_id[0] = igt_create_fb(data->drm_fd, width, height,
+				 DRM_FORMAT_XRGB8888, tiling, &fb[0]);
+	igt_assert(fb_id[0]);
+
+	fb_id[1] = igt_create_fb(data->drm_fd, width, height,
+				 DRM_FORMAT_XRGB8888, tiling, &fb[1]);
+	igt_assert(fb_id[1]);
+
+	fill_fb(&fb[0], data, mode);
+	fill_fb(&fb[1], data, mode);
+
+	/* set the crtc and generate a reference crc */
+	igt_plane_set_fb(primary, &fb[0]);
+	igt_display_commit(&data->display);
+	igt_pipe_crc_collect_crc(pipe_crc, &reference_crc);
+
+	/* flip to the other fb */
+	ret = drmModePageFlip(data->drm_fd, output->config.crtc->crtc_id,
+			      fb_id[1], 0, NULL);
+	igt_assert_eq(ret, 0);
+
+	igt_wait_for_vblank(data->drm_fd, pipe);
+
+	/* get a crc and compare with the reference */
+	igt_pipe_crc_collect_crc(pipe_crc, &crc);
+	igt_assert_crc_equal(&reference_crc, &crc);
+
+	/* clean up */
+	igt_plane_set_fb(primary, NULL);
+	igt_pipe_crc_free(pipe_crc);
+	igt_output_set_pipe(output, PIPE_ANY);
+	igt_display_commit(&data->display);
+
+	igt_remove_fb(data->drm_fd, &fb[0]);
+	igt_remove_fb(data->drm_fd, &fb[1]);
+}
+
 static data_t data;
 igt_output_t *output;
 
@@ -166,6 +225,30 @@  igt_main
 						LOCAL_I915_FORMAT_MOD_Yf_TILED);
 	}
 
+	igt_subtest_f("flip-X-tiled") {
+		for_each_connected_output(&data.display, output)
+			test_tiled_flip(&data, output,
+					LOCAL_I915_FORMAT_MOD_X_TILED);
+	}
+
+	igt_subtest_f("flip-Y-tiled") {
+		igt_require_fb_modifiers(data.drm_fd);
+		igt_require(data.gen >= 9);
+
+		for_each_connected_output(&data.display, output)
+			test_tiled_flip(&data, output,
+					LOCAL_I915_FORMAT_MOD_Y_TILED);
+	}
+
+	igt_subtest_f("flip-Yf-tiled") {
+		igt_require_fb_modifiers(data.drm_fd);
+		igt_require(data.gen >= 9);
+
+		for_each_connected_output(&data.display, output)
+			test_tiled_flip(&data, output,
+					LOCAL_I915_FORMAT_MOD_Yf_TILED);
+	}
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 	}