[PATCH 1/2] lib/batchbuffer: Fix COLOR_BLIT_COPY_BATCH_START

[PATCH 1/2] lib/batchbuffer: Fix COLOR_BLIT_COPY_BATCH_START

[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

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

[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

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

[Paulo Zanoni]

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

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

[Ville Syrjälä]

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

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

[Daniel Vetter]

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

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

[Daniel Vetter]

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

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

[Ville Syrjälä]

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

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

[Daniel Vetter]

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

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

[Ville Syrjälä]

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