Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES

Paulo Zanoni
2015-03-25T21:47:21Z

2015-03-25 17:15 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> And use the same colors for both flip and fills so that we can reuse
> crcs. Some details:
> - For the flip_and_foo tests flip twice so that we again start with
> the black framebuffer and hence have a real change when painting it
> white.

But you don't really check the CRC after the first flip, so if it's
completely ignored by the display engine, we won't know. That still
looks like a regression from the previous code we had.

> - The upload for the rendercopy source isn't fast, but hey I'm lazy.

You're also changing everything form single-pixel write to full-fb
write, which is not necessarily a bad thing (although slower), but
could at least be on the changelog. Rodrigo sent a patch a while ago
to transform these into visible rectangles and add
igt_debug_wait_for_keypress(). I also have in my plans to add a small
library for these draw operations, so we don't need to reimplement
"write a colored rectangle using blt/render/mmap/etc" in every test.
I'll send some patches for you to take a look.

>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> tests/kms_fbc_crc.c | 52 +++++++++++++++++++++++++++-------------------------
> 1 file changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 4256fede0fb5..314f94d879d4 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -91,7 +91,7 @@ static const char *test_mode_str(enum test_mode mode)
> static void fill_blt(data_t *data,
> uint32_t handle,
> struct igt_fb *fb,
> - unsigned char color)
> + uint32_t color)
> {
> drm_intel_bo *dst = gem_handle_to_libdrm_bo(data->bufmgr,
> data->drm_fd,
> @@ -125,12 +125,13 @@ static void fill_blt(data_t *data,
> gem_bo_busy(data->drm_fd, handle);
> }
>
> -static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
> +static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo,
> + struct igt_fb *fb)
> {
> buf->bo = bo;
> - buf->stride = 4096;
> + buf->stride = fb->stride;
> buf->tiling = I915_TILING_X;
> - buf->size = 4096;
> + buf->size = fb->size;
> }
>
> static void exec_nop(data_t *data, uint32_t handle, drm_intel_context *context)
> @@ -170,19 +171,21 @@ static void fill_render(data_t *data, uint32_t handle,
> dst = gem_handle_to_libdrm_bo(data->bufmgr, data->drm_fd, "", handle);
> igt_assert(dst);
>
> - src = drm_intel_bo_alloc(data->bufmgr, "", 4096, 4096);
> + src = drm_intel_bo_alloc(data->bufmgr, "", data->fb[0].size, 4096);
> igt_assert(src);
>
> - gem_write(data->drm_fd, src->handle, 0, buf, 4);
> + for (int i = 0; i < data->fb[0].size/4; i++)
> + gem_write(data->drm_fd, src->handle, i*4, buf, 4);
>
> - scratch_buf_init(&src_buf, src);
> - scratch_buf_init(&dst_buf, dst);
> + scratch_buf_init(&src_buf, src, data->fb);
> + scratch_buf_init(&dst_buf, dst, data->fb);
>
> batch = intel_batchbuffer_alloc(data->bufmgr, data->devid);
> igt_assert(batch);
>
> rendercopy(batch, context,
> - &src_buf, 0, 0, 1, 1,
> + &src_buf, 0, 0,
> + data->fb[0].width, data->fb[0].height,
> &dst_buf, 0, 0);
>
> intel_batchbuffer_free(batch);
> @@ -213,12 +216,17 @@ static void test_crc(data_t *data, enum test_mode mode)
> igt_assert(fbc_enabled(data));
>
> if (mode >= TEST_PAGE_FLIP_AND_MMAP_CPU) {
> - handle = data->handle[1];
> igt_assert(drmModePageFlip(data->drm_fd, crtc_id,
> data->fb_id[1], 0, NULL) == 0);
> usleep(300000);
>
> igt_assert(fbc_enabled(data));
> +
> + igt_assert(drmModePageFlip(data->drm_fd, crtc_id,
> + data->fb_id[0], 0, NULL) == 0);
> + usleep(300000);
> +
> + igt_assert(fbc_enabled(data));
> }
>
> switch (mode) {
> @@ -229,18 +237,18 @@ static void test_crc(data_t *data, enum test_mode mode)
> break;
> case TEST_MMAP_CPU:
> case TEST_PAGE_FLIP_AND_MMAP_CPU:
> - ptr = gem_mmap__cpu(data->drm_fd, handle, 0, 4096, PROT_WRITE);
> + ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->fb[0].size, PROT_WRITE);
> gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> - memset(ptr, 0xff, 4);
> - munmap(ptr, 4096);
> + memset(ptr, 0xff, data->fb[0].size);
> + munmap(ptr, data->fb[0].size);
> gem_sw_finish(data->drm_fd, handle);
> break;
> case TEST_MMAP_GTT:
> case TEST_PAGE_FLIP_AND_MMAP_GTT:
> - ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
> + ptr = gem_mmap__gtt(data->drm_fd, handle, data->fb[0].size, PROT_WRITE);
> gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> - memset(ptr, 0xff, 4);
> - munmap(ptr, 4096);
> + memset(ptr, 0xff, data->fb[0].size);
> + munmap(ptr, data->fb[0].size);
> break;
> case TEST_BLT:
> case TEST_PAGE_FLIP_AND_BLT:
> @@ -267,10 +275,7 @@ static void test_crc(data_t *data, enum test_mode mode)
> igt_pipe_crc_start(pipe_crc);
> igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
> igt_pipe_crc_stop(pipe_crc);
> - if (mode == TEST_PAGE_FLIP)
> - igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
> - else
> - ;/* FIXME: missing reference CRCs */
> + igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
> free(crcs);
>
> /*
> @@ -284,10 +289,7 @@ static void test_crc(data_t *data, enum test_mode mode)
> igt_pipe_crc_start(pipe_crc);
> igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
> igt_pipe_crc_stop(pipe_crc);
> - if (mode == TEST_PAGE_FLIP)
> - igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
> - else
> - ;/* FIXME: missing reference CRCs */
> + igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
> free(crcs);
> }
>
> @@ -328,7 +330,7 @@ static bool prepare_test(data_t *data, enum test_mode test_mode)
> data->fb_id[1] = igt_create_color_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
> DRM_FORMAT_XRGB8888,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> - 0.1, 0.1, 0.1,
> + 1.0, 1.0, 1.0,
> &data->fb[1]);
> igt_assert(data->fb_id[1]);
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni