* [PATCH 1/2] lib/batchbuffer: Fix COLOR_BLIT_COPY_BATCH_START @ 2015-03-25 20:15 Daniel Vetter 2015-03-25 20:15 ` [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2015-03-25 20:15 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- lib/intel_batchbuffer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h index fa8875b3e472..8b440ca84824 100644 --- a/lib/intel_batchbuffer.h +++ b/lib/intel_batchbuffer.h @@ -175,6 +175,7 @@ intel_batchbuffer_require_space(struct intel_batchbuffer *batch, OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | \ COLOR_BLT_WRITE_ALPHA | \ XY_COLOR_BLT_WRITE_RGB | \ + (flags) | \ (4 + (batch->gen >= 8))); \ } while(0) -- 1.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-25 20:15 [PATCH 1/2] lib/batchbuffer: Fix COLOR_BLIT_COPY_BATCH_START Daniel Vetter @ 2015-03-25 20:15 ` Daniel Vetter 2015-03-25 21:47 ` Paulo Zanoni 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2015-03-25 20:15 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter 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. - The upload for the rendercopy source isn't fast, but hey I'm lazy. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-25 20:15 ` [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES Daniel Vetter @ 2015-03-25 21:47 ` Paulo Zanoni 2015-03-26 7:35 ` Ville Syrjälä 2015-03-26 10:02 ` Daniel Vetter 0 siblings, 2 replies; 9+ messages in thread From: Paulo Zanoni @ 2015-03-25 21:47 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel Graphics Development 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-25 21:47 ` Paulo Zanoni @ 2015-03-26 7:35 ` Ville Syrjälä 2015-03-26 10:03 ` Daniel Vetter 2015-03-26 10:02 ` Daniel Vetter 1 sibling, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2015-03-26 7:35 UTC (permalink / raw) To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > 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. The reason I riginally used very small operations was to make sure the hardware catches precicesly such small operations. If we just blast away the entire thing we can't tell if the hardware would even notice a small change. Although since hardware tracking was declared to be the evil that may not matter so much, except people still want to use the GTT tracking for some reason. > 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 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-26 7:35 ` Ville Syrjälä @ 2015-03-26 10:03 ` Daniel Vetter 2015-03-26 11:20 ` Ville Syrjälä 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2015-03-26 10:03 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development On Thu, Mar 26, 2015 at 09:35:10AM +0200, Ville Syrjälä wrote: > On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > > 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. > > The reason I riginally used very small operations was to make sure the > hardware catches precicesly such small operations. If we just blast > away the entire thing we can't tell if the hardware would even notice > a small change. Although since hardware tracking was declared to be > the evil that may not matter so much, except people still want to use > the GTT tracking for some reason. We're still using the gtt cpu write tracking, which is the only hw tracking bit that works per-line-block. I guess if we see that one fail we can add a specific testcase for the pattern, but for now I kinda trust the hw in that regard actually. At least I haven't seen bugs like that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-26 10:03 ` Daniel Vetter @ 2015-03-26 11:20 ` Ville Syrjälä 2015-03-26 13:14 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Ville Syrjälä @ 2015-03-26 11:20 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development On Thu, Mar 26, 2015 at 11:03:55AM +0100, Daniel Vetter wrote: > On Thu, Mar 26, 2015 at 09:35:10AM +0200, Ville Syrjälä wrote: > > On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > > > 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. > > > > The reason I riginally used very small operations was to make sure the > > hardware catches precicesly such small operations. If we just blast > > away the entire thing we can't tell if the hardware would even notice > > a small change. Although since hardware tracking was declared to be > > the evil that may not matter so much, except people still want to use > > the GTT tracking for some reason. > > We're still using the gtt cpu write tracking, which is the only hw > tracking bit that works per-line-block. I guess if we see that one fail we > can add a specific testcase for the pattern, but for now I kinda trust the > hw in that regard actually. At least I haven't seen bugs like that. We don't even test panning, which is where I think the gtt tracking code is broken. And you can't catch bugs like that if you paint the entire fb. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-26 11:20 ` Ville Syrjälä @ 2015-03-26 13:14 ` Daniel Vetter 2015-03-26 13:27 ` Ville Syrjälä 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2015-03-26 13:14 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development On Thu, Mar 26, 2015 at 01:20:50PM +0200, Ville Syrjälä wrote: > On Thu, Mar 26, 2015 at 11:03:55AM +0100, Daniel Vetter wrote: > > On Thu, Mar 26, 2015 at 09:35:10AM +0200, Ville Syrjälä wrote: > > > On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > > > > 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. > > > > > > The reason I riginally used very small operations was to make sure the > > > hardware catches precicesly such small operations. If we just blast > > > away the entire thing we can't tell if the hardware would even notice > > > a small change. Although since hardware tracking was declared to be > > > the evil that may not matter so much, except people still want to use > > > the GTT tracking for some reason. > > > > We're still using the gtt cpu write tracking, which is the only hw > > tracking bit that works per-line-block. I guess if we see that one fail we > > can add a specific testcase for the pattern, but for now I kinda trust the > > hw in that regard actually. At least I haven't seen bugs like that. > > We don't even test panning, which is where I think the gtt tracking code > is broken. And you can't catch bugs like that if you paint the entire > fb. You could pan an entire width from an all black to an all white framebuffer. At least I don't see a fundamental difference in panning 1 pixel and in panning by 1600. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-26 13:14 ` Daniel Vetter @ 2015-03-26 13:27 ` Ville Syrjälä 0 siblings, 0 replies; 9+ messages in thread From: Ville Syrjälä @ 2015-03-26 13:27 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development On Thu, Mar 26, 2015 at 02:14:39PM +0100, Daniel Vetter wrote: > On Thu, Mar 26, 2015 at 01:20:50PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 26, 2015 at 11:03:55AM +0100, Daniel Vetter wrote: > > > On Thu, Mar 26, 2015 at 09:35:10AM +0200, Ville Syrjälä wrote: > > > > On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > > > > > 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. > > > > > > > > The reason I riginally used very small operations was to make sure the > > > > hardware catches precicesly such small operations. If we just blast > > > > away the entire thing we can't tell if the hardware would even notice > > > > a small change. Although since hardware tracking was declared to be > > > > the evil that may not matter so much, except people still want to use > > > > the GTT tracking for some reason. > > > > > > We're still using the gtt cpu write tracking, which is the only hw > > > tracking bit that works per-line-block. I guess if we see that one fail we > > > can add a specific testcase for the pattern, but for now I kinda trust the > > > hw in that regard actually. At least I haven't seen bugs like that. > > > > We don't even test panning, which is where I think the gtt tracking code > > is broken. And you can't catch bugs like that if you paint the entire > > fb. > > You could pan an entire width from an all black to an all white > framebuffer. At least I don't see a fundamental difference in panning 1 > pixel and in panning by 1600. We don't want to test invalidate due to panning (that's a flip which means the hw will nuke the entire thing anyway), instead we want to test invalidating when scanning out a panned fb and there's a gtt write into the visible part. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES 2015-03-25 21:47 ` Paulo Zanoni 2015-03-26 7:35 ` Ville Syrjälä @ 2015-03-26 10:02 ` Daniel Vetter 1 sibling, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2015-03-26 10:02 UTC (permalink / raw) To: Paulo Zanoni; +Cc: Daniel Vetter, Intel Graphics Development On Wed, Mar 25, 2015 at 06:47:21PM -0300, Paulo Zanoni wrote: > 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 clean fix would be to set up the 2nd (white) fb in the flip tests instead of my lazy solution of simply flipping. And we already have a simple flip test which makes sure that flips themselves work, so imo it's ok to only test the _and_foo part in those tests. If you want I can rework the test to be crazy and select the right starting fb depending upon testcase. > > - 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. Yeah the rectangle is a lot bigger now than a single pixel, I'm not sure whether it really matters - if the hw gtt tracking misses a line we'll still spot it because the crc checksum looks at the entire frame. And if we indeed discover issues where only a few pixels get lost (thus far I haven't seen that happen with fbc) then we can add more subtests with corresponding crc. But for now I'd go with simple and obvious, full-screen black->white transitions is about the most massive we can get. And yeah helpers to write rbgx32 rectangles into buffers would be awesome indeed, especially since I wasted a few hours yesterday debugging the blt issues. btw while you're thinking of making the test more useful for interactive debugging, I wonder whether we should have an igt_debug_wait_for_keypress when an igt_assert fails. Then you could run with --interactive-debug=failure and with most crc testcase we don't need to add specific interactive debug points. Another one is running the testcase without fbc checks. That's useful for debugging the test itself since you can then run it with fbc disabled. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-26 13:27 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-25 20:15 [PATCH 1/2] lib/batchbuffer: Fix COLOR_BLIT_COPY_BATCH_START Daniel Vetter 2015-03-25 20:15 ` [PATCH 2/2] tests/kms_fbc_crc: Fill entire target framebuffer to adds crc FIXMES Daniel Vetter 2015-03-25 21:47 ` Paulo Zanoni 2015-03-26 7:35 ` Ville Syrjälä 2015-03-26 10:03 ` Daniel Vetter 2015-03-26 11:20 ` Ville Syrjälä 2015-03-26 13:14 ` Daniel Vetter 2015-03-26 13:27 ` Ville Syrjälä 2015-03-26 10:02 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.