[Intel-gfx] [PATCH 09/12] tests/kms_psr_sink_crc: Fix all testcases.
Rodrigo Vivi
rodrigo.vivi at gmail.com
Thu Sep 4 22:24:37 CEST 2014
On Thu, Sep 4, 2014 at 2:04 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Sep 03, 2014 at 09:30:03PM -0400, Rodrigo Vivi wrote:
> > In order to get all test cases fixed and the matrix planes-operations
> working
> > it was needed to use the common new igt kms functions for all cases.
> > Previously only sprite testcase was using it.
> >
> > Fixed the fb colors in a way to make tests more clear and be impossible
> to see
> > black screen during the tests.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> Ok, everything changed ;-) So a bit hard to review just as a patch and so
> just a few comments on top:
>
I'm sorry about that.... I got surprised I could split all in 9 patches,
but I know this one staid ugly.
> - Looks good overall, but the gold standard is whether it'll catch bugs.
> So if you remove some of the frontbuffer tracking (as little as possible
> in each test) and the new testcase catches it all, then I think we're
> good.
>
It is already catching something, what is good! :)
and not broken anymore!
>
> - I think we should have a residency check before we grab a new crc, to
> make sure that we're really again (or if the kernel is buggy, still) in
> psr mode.
>
yeah, but residency check is bad for vlv/chv.
>
> - Checking for mismatching crc is risky, see the comment in a different
> reply in this thread.
>
> But overall looks good so probably best to just push these patches and
> then fixup anything missing afterwards. I'll read through the entire test
> again once it all landed to double-check we haven't missed anything really
> important.
>
cool. Thanks!
>
> Aside: A suspend/resume testcase might be useful, if it can reliably
> reproduce the issue you're working on. But again, follow-up.
>
For sure. I just saw we have igt_system_suspend_autoresume. I'll definetely
add another test using it.
> -Daniel
>
> > ---
> > tests/kms_psr_sink_crc.c | 269
> ++++++++++++++++++-----------------------------
> > 1 file changed, 101 insertions(+), 168 deletions(-)
> >
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 4358889..27f3df9 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -27,8 +27,6 @@
> > #include <stdio.h>
> > #include <string.h>
> >
> > -#include "drm_fourcc.h"
> > -
> > #include "ioctl_wrappers.h"
> > #include "drmtest.h"
> > #include "intel_bufmgr.h"
> > @@ -79,88 +77,37 @@ typedef struct {
> > int drm_fd;
> > enum planes test_plane;
> > enum operations op;
> > - drmModeRes *resources;
> > - drm_intel_bufmgr *bufmgr;
> > uint32_t devid;
> > - uint32_t handle[2];
> > uint32_t crtc_id;
> > - uint32_t crtc_idx;
> > - uint32_t fb_id[3];
> > - struct kmstest_connector_config config;
> > igt_display_t display;
> > - struct igt_fb fb[2];
> > - igt_plane_t *plane[2];
> > + drm_intel_bufmgr *bufmgr;
> > + struct igt_fb fb_green, fb_white;
> > + igt_plane_t *primary, *sprite, *cursor;
> > } data_t;
> >
> > -static uint32_t create_fb(data_t *data,
> > - int w, int h,
> > - double r, double g, double b,
> > - struct igt_fb *fb)
> > +static void create_cursor_fb(data_t *data)
> > {
> > - uint32_t fb_id;
> > cairo_t *cr;
> > + uint32_t fb_id;
> >
> > - fb_id = igt_create_fb(data->drm_fd, w, h,
> > - DRM_FORMAT_XRGB8888, I915_TILING_X, fb);
> > + fb_id = igt_create_fb(data->drm_fd, 64, 64,
> > + DRM_FORMAT_ARGB8888, I915_TILING_NONE,
> > + &data->fb_white);
> > igt_assert(fb_id);
> >
> > - cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > - igt_paint_color(cr, 0, 0, w, h, r, g, b);
> > - igt_assert(cairo_status(cr) == 0);
> > - cairo_destroy(cr);
> > -
> > - return fb_id;
> > -}
> > -
> > -static void create_cursor_fb(data_t *data, struct igt_fb *fb)
> > -{
> > - cairo_t *cr;
> > -
> > - data->fb_id[2] = igt_create_fb(data->drm_fd, 64, 64,
> > - DRM_FORMAT_ARGB8888,
> I915_TILING_NONE,
> > - fb);
> > - igt_assert(data->fb_id[2]);
> > -
> > - cr = igt_get_cairo_ctx(data->drm_fd, fb);
> > + cr = igt_get_cairo_ctx(data->drm_fd, &data->fb_white);
> > igt_paint_color_alpha(cr, 0, 0, 64, 64, 1.0, 1.0, 1.0, 1.0);
> > igt_assert(cairo_status(cr) == 0);
> > }
> >
> > -static bool
> > -connector_set_mode(data_t *data, drmModeModeInfo *mode, uint32_t fb_id)
> > -{
> > - struct kmstest_connector_config *config = &data->config;
> > - int ret;
> > -
> > -#if 0
> > - fprintf(stdout, "Using pipe %s, %dx%d\n",
> kmstest_pipe_name(config->pipe),
> > - mode->hdisplay, mode->vdisplay);
> > -#endif
> > -
> > - ret = drmModeSetCrtc(data->drm_fd,
> > - config->crtc->crtc_id,
> > - fb_id,
> > - 0, 0, /* x, y */
> > - &config->connector->connector_id,
> > - 1,
> > - mode);
> > - igt_assert(ret == 0);
> > -
> > - return 0;
> > -}
> > -
> > static void display_init(data_t *data)
> > {
> > igt_display_init(&data->display, data->drm_fd);
> > - data->resources = drmModeGetResources(data->drm_fd);
> > - igt_assert(data->resources);
> > }
> >
> > static void display_fini(data_t *data)
> > {
> > igt_display_fini(&data->display);
> > - drmModeSetCursor(data->drm_fd, data->crtc_id, 0, 0, 0);
> > - drmModeFreeResources(data->resources);
> > }
> >
> > static void fill_blt(data_t *data, uint32_t handle, unsigned char color)
> > @@ -316,112 +263,105 @@ static void get_sink_crc(data_t *data, char
> *crc) {
> >
> > static void test_crc(data_t *data)
> > {
> > - uint32_t handle = data->handle[0];
> > + uint32_t handle = data->fb_white.gem_handle;
> > + igt_plane_t *test_plane;
> > + void *ptr;
> > char ref_crc[12];
> > char crc[12];
> >
> > - if (data->op == PLANE_MOVE) {
> > - igt_assert(drmModeSetCursor(data->drm_fd, data->crtc_id,
> > - handle, 64, 64) == 0);
> > - igt_assert(drmModeMoveCursor(data->drm_fd, data->crtc_id,
> > - 1, 1) == 0);
> > + igt_plane_set_fb(data->primary, &data->fb_green);
> > + igt_display_commit(&data->display);
> > +
> > + /* Setting a secondary fb/plane */
> > + switch (data->test_plane) {
> > + case PRIMARY: default: test_plane = data->primary; break;
> > + case SPRITE: test_plane = data->sprite; break;
> > + case CURSOR: test_plane = data->cursor; break;
> > }
> > + igt_plane_set_fb(test_plane, &data->fb_white);
> > + igt_display_commit(&data->display);
> >
> > igt_assert(wait_psr_entry(data, 10));
> > get_sink_crc(data, ref_crc);
> >
> > switch (data->op) {
> > - void *ptr;
> > case PAGE_FLIP:
> > + /* Only in use when testing primary plane */
> > igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
> > - data->fb_id[1], 0, NULL) == 0);
> > + data->fb_green.fb_id, 0, NULL)
> == 0);
> > break;
> > - case MMAP_CPU:
> > - ptr = gem_mmap__cpu(data->drm_fd, handle, 4096,
> PROT_WRITE);
> > + case MMAP_GTT:
> > + ptr = gem_mmap__gtt(data->drm_fd, handle, 4096,
> PROT_WRITE);
> > gem_set_domain(data->drm_fd, handle,
> > - I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> > - sleep(1);
> > + I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > memset(ptr, 0, 4);
> > munmap(ptr, 4096);
> > - sleep(1);
> > - gem_sw_finish(data->drm_fd, handle);
> > break;
> > - case MMAP_GTT:
> > case MMAP_GTT_WAITING:
> > ptr = gem_mmap__gtt(data->drm_fd, handle, 4096,
> PROT_WRITE);
> > gem_set_domain(data->drm_fd, handle,
> > I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > +
> > + /* Printing white on white so the screen shouldn't change
> */
> > memset(ptr, 0xff, 4);
> > + get_sink_crc(data, crc);
> > + igt_assert(strcmp(ref_crc, crc) == 0);
> > +
> > + fprintf(stdout, "Waiting 10s...\n");
> > + sleep(10);
> > +
> > + /* Now lets print black to change the screen */
> > + memset(ptr, 0, 4);
> > munmap(ptr, 4096);
> > - gem_bo_busy(data->drm_fd, handle);
> > + break;
> > + case MMAP_CPU:
> > + ptr = gem_mmap__cpu(data->drm_fd, handle, 4096,
> PROT_WRITE);
> > + gem_set_domain(data->drm_fd, handle,
> > + I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> > + memset(ptr, 0, 4);
> > + munmap(ptr, 4096);
> > + gem_sw_finish(data->drm_fd, handle);
> > break;
> > case BLT:
> > - fill_blt(data, handle, 0xff);
> > + fill_blt(data, handle, 0);
> > break;
> > case RENDER:
> > - fill_render(data, handle, 0xff);
> > + fill_render(data, handle, 0);
> > break;
> > case PLANE_MOVE:
> > - igt_assert(drmModeMoveCursor(data->drm_fd, data->crtc_id,
> 1, 2) == 0);
> > + /* Only in use when testing Sprite and Cursor */
> > + igt_plane_set_position(test_plane, 1, 1);
> > + igt_display_commit(&data->display);
> > break;
> > case PLANE_ONOFF:
> > - igt_plane_set_fb(data->plane[0], &data->fb[0]);
> > - igt_display_commit(&data->display);
> > - igt_plane_set_fb(data->plane[1], &data->fb[1]);
> > + /* Only in use when testing Sprite and Cursor */
> > + igt_plane_set_fb(test_plane, NULL);
> > igt_display_commit(&data->display);
> > break;
> > }
> > -
> > - igt_wait_for_vblank(data->drm_fd, data->crtc_idx);
> > -
> > get_sink_crc(data, crc);
> > igt_assert(strcmp(ref_crc, crc) != 0);
> > }
> >
> > -static bool prepare_crtc(data_t *data, uint32_t connector_id)
> > -{
> > - if (!kmstest_get_connector_config(data->drm_fd,
> > - connector_id,
> > - 1 << data->crtc_idx,
> > - &data->config))
> > - return false;
> > -
> > - data->fb_id[0] = create_fb(data,
> > - data->config.default_mode.hdisplay,
> > - data->config.default_mode.vdisplay,
> > - 0.0, 1.0, 0.0, &data->fb[0]);
> > - igt_assert(data->fb_id[0]);
> > -
> > - if (data->op == PLANE_MOVE)
> > - create_cursor_fb(data, &data->fb[0]);
> > -
> > - data->fb_id[1] = create_fb(data,
> > - data->config.default_mode.hdisplay,
> > - data->config.default_mode.vdisplay,
> > - 1.0, 0.0, 0.0, &data->fb[1]);
> > - igt_assert(data->fb_id[1]);
> > -
> > - data->handle[0] = data->fb[0].gem_handle;
> > - data->handle[1] = data->fb[1].gem_handle;
> > -
> > - /* scanout = fb[1] */
> > - connector_set_mode(data, &data->config.default_mode,
> > - data->fb_id[1]);
> > +static void test_cleanup(data_t *data) {
> > + igt_plane_set_fb(data->primary, NULL);
> > + if (data->test_plane == SPRITE)
> > + igt_plane_set_fb(data->sprite, NULL);
> > + if (data->test_plane == CURSOR)
> > + igt_plane_set_fb(data->cursor, NULL);
> >
> > - /* scanout = fb[0] */
> > - connector_set_mode(data, &data->config.default_mode,
> > - data->fb_id[0]);
> > + igt_display_commit(&data->display);
> >
> > - kmstest_free_connector_config(&data->config);
> > -
> > - return true;
> > + igt_remove_fb(data->drm_fd, &data->fb_green);
> > + igt_remove_fb(data->drm_fd, &data->fb_white);
> > }
> >
> > -static void test_sprite(data_t *data)
> > +static void run_test(data_t *data)
> > {
> > igt_display_t *display = &data->display;
> > igt_output_t *output;
> > drmModeModeInfo *mode;
> > + uint32_t white_h, white_v;
> >
> > for_each_connected_output(display, output) {
> > drmModeConnectorPtr c = output->config.connector;
> > @@ -431,6 +371,7 @@ static void test_sprite(data_t *data)
> > continue;
> >
> > igt_output_set_pipe(output, PIPE_ANY);
> > + data->crtc_id = output->config.crtc->crtc_id;
> >
> > mode = igt_output_get_mode(output);
> >
> > @@ -438,51 +379,45 @@ static void test_sprite(data_t *data)
> > mode->hdisplay, mode->vdisplay,
> > DRM_FORMAT_XRGB8888, I915_TILING_X,
> > 0.0, 1.0, 0.0,
> > - &data->fb[0]);
> > -
> > - igt_create_color_fb(data->drm_fd,
> > - mode->hdisplay/2, mode->vdisplay/2,
> > - DRM_FORMAT_XRGB8888, I915_TILING_X,
> > - 1.0, 0.0, 0.0,
> > - &data->fb[1]);
> > + &data->fb_green);
> > +
> > + data->primary = igt_output_get_plane(output,
> IGT_PLANE_PRIMARY);
> > + igt_plane_set_fb(data->primary, NULL);
> > +
> > + white_h = mode->hdisplay;
> > + white_v = mode->vdisplay;
> > +
> > + switch (data->test_plane) {
> > + case SPRITE:
> > + data->sprite = igt_output_get_plane(output,
> > + IGT_PLANE_2);
> > + igt_plane_set_fb(data->sprite, NULL);
> > + /* To make it different for human eyes let's make
> > + * sprite visible in only one quarter of the
> primary
> > + */
> > + white_h = white_h/2;
> > + white_v = white_v/2;
> > + case PRIMARY:
> > + igt_create_color_fb(data->drm_fd,
> > + white_h, white_v,
> > + DRM_FORMAT_XRGB8888,
> I915_TILING_X,
> > + 1.0, 1.0, 1.0,
> > + &data->fb_white);
> > + break;
> > + case CURSOR:
> > + data->cursor = igt_output_get_plane(output,
> > +
> IGT_PLANE_CURSOR);
> > + igt_plane_set_fb(data->cursor, NULL);
> > + create_cursor_fb(data);
> > + igt_plane_set_position(data->cursor, 0, 0);
> > + break;
> > + }
> >
> > - data->plane[0] = igt_output_get_plane(output, 0);
> > - data->plane[1] = igt_output_get_plane(output, 1);
> > + igt_display_commit(&data->display);
> >
> > test_crc(data);
> > - }
> > -}
> > -
> > -static void run_test(data_t *data)
> > -{
> > - int i, n;
> > - drmModeConnectorPtr c;
> > - /* Baytrail supports per-pipe PSR configuration, however PSR on
> > - * PIPE_B isn't working properly. So let's keep it disabled for
> now.
> > - * crtcs = IS_VALLEYVIEW(data->devid)? 2 : 1; */
> > - int crtcs = 1;
> > -
> > - if (data->op == PLANE_ONOFF) {
> > - test_sprite(data);
> > - return;
> > - }
> >
> > - for (i = 0; i < data->resources->count_connectors; i++) {
> > - uint32_t connector_id = data->resources->connectors[i];
> > - c = drmModeGetConnector(data->drm_fd, connector_id);
> > -
> > - if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> > - c->connection != DRM_MODE_CONNECTED)
> > - continue;
> > - for (n = 0; n < crtcs; n++) {
> > - data->crtc_idx = n;
> > - data->crtc_id = data->resources->crtcs[n];
> > -
> > - if (!prepare_crtc(data, connector_id))
> > - continue;
> > -
> > - test_crc(data);
> > - }
> > + test_cleanup(data);
> > }
> > }
> >
> > @@ -496,7 +431,6 @@ igt_main
> > igt_fixture {
> > data.drm_fd = drm_open_any();
> > kmstest_set_vt_graphics_mode();
> > -
> > data.devid = intel_get_drm_devid(data.drm_fd);
> >
> > igt_skip_on(!psr_enabled(&data));
> > @@ -508,7 +442,6 @@ igt_main
> > display_init(&data);
> > }
> >
> > -
> > for (op = PAGE_FLIP; op <= RENDER; op++) {
> > igt_subtest_f("primary_%s", op_str(op)) {
> > data.test_plane = PRIMARY;
> > @@ -517,7 +450,7 @@ igt_main
> > }
> > }
> >
> > - for (op = PLANE_ONOFF; op <= PLANE_ONOFF; op++) {
> > + for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > igt_subtest_f("sprite_%s", op_str(op)) {
> > data.test_plane = SPRITE;
> > data.op = op;
> > @@ -525,7 +458,7 @@ igt_main
> > }
> > }
> >
> > - for (op = PLANE_MOVE; op <= PLANE_MOVE; op++) {
> > + for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > igt_subtest_f("cursor_%s", op_str(op)) {
> > data.test_plane = CURSOR;
> > data.op = op;
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140904/5b9f5567/attachment.html>
More information about the Intel-gfx
mailing list