Message ID | 1429783715-12921-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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 --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); }