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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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