diff mbox

[09/12] tests/kms_psr_sink_crc: Fix all testcases.

Message ID 1409794206-2126-9-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Sept. 4, 2014, 1:30 a.m. UTC 
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@intel.com>
---
 tests/kms_psr_sink_crc.c | 269 ++++++++++++++++++-----------------------------
 1 file changed, 101 insertions(+), 168 deletions(-)

Comments

Daniel Vetter Sept. 4, 2014, 9:04 a.m. UTC | #1 
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@intel.com>

Ok, everything changed ;-) So a bit hard to review just as a patch and so
just a few comments on top:
- 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.

- 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.

- 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.

Aside: A suspend/resume testcase might be useful, if it can reliably
reproduce the issue you're working on. But again, follow-up.
-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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Sept. 4, 2014, 8:24 p.m. UTC | #2 
On Thu, Sep 4, 2014 at 2:04 AM, Daniel Vetter <daniel@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@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Rodrigo Vivi Sept. 5, 2014, 12:55 a.m. UTC | #3 
adding suspend_autoresume on primary tests like this:
@ -470,6 +472,8 @@ igt_main
                        data.test_plane = PRIMARY;
                        data.op = op;
                        run_test(&data);
+                       igt_system_suspend_autoresume();
+                       run_test(&data);

on BDW I got these results:


vivijim rdvivi-seattle tests$ sudo ./kms_psr_sink_crc
IGT-Version: 1.7-gd4b43f0 (x86_64) (Linux: 3.17.0-rc2+ x86_64)
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:44:03 2014
Subtest primary_page_flip: SUCCESS
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:44:40 2014
Subtest primary_mmap_gtt: SUCCESS
Waiting 10s...
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:45:27 2014
Waiting 10s...
Subtest primary_mmap_gtt_waiting: SUCCESS
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:46:13 2014
Subtest primary_mmap_cpu: SUCCESS
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:46:50 2014
Subtest primary_blt: SUCCESS
rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:47:27 2014
Subtest primary_render: SUCCESS

on HSW I couldn't test because suspend/resume breaks even with psr disabled.
I'm going to check more tomorrow..

But regarding the suspend resume test, how do you suggest to organize it?
Extra loops for all current cases?
suspend_{primary, sprite, cursor}_{page_flip, mmap_gtt, etc}? I believe the
test will take so long to finish on this case what is bad for qa alghouth
it is the complete one. What do you think?




















On Thu, Sep 4, 2014 at 1:24 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:

>
>
>
> On Thu, Sep 4, 2014 at 2:04 AM, Daniel Vetter <daniel@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@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@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
>
>
Daniel Vetter Sept. 5, 2014, 8:32 a.m. UTC | #4 
On Thu, Sep 04, 2014 at 05:55:24PM -0700, Rodrigo Vivi wrote:
> adding suspend_autoresume on primary tests like this:
> @ -470,6 +472,8 @@ igt_main
>                         data.test_plane = PRIMARY;
>                         data.op = op;
>                         run_test(&data);
> +                       igt_system_suspend_autoresume();
> +                       run_test(&data);
> 
> on BDW I got these results:
> 
> 
> vivijim rdvivi-seattle tests$ sudo ./kms_psr_sink_crc
> IGT-Version: 1.7-gd4b43f0 (x86_64) (Linux: 3.17.0-rc2+ x86_64)
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:44:03 2014
> Subtest primary_page_flip: SUCCESS
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:44:40 2014
> Subtest primary_mmap_gtt: SUCCESS
> Waiting 10s...
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:45:27 2014
> Waiting 10s...
> Subtest primary_mmap_gtt_waiting: SUCCESS
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:46:13 2014
> Subtest primary_mmap_cpu: SUCCESS
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:46:50 2014
> Subtest primary_blt: SUCCESS
> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Sep  5 00:47:27 2014
> Subtest primary_render: SUCCESS
> 
> on HSW I couldn't test because suspend/resume breaks even with psr disabled.
> I'm going to check more tomorrow..
> 
> But regarding the suspend resume test, how do you suggest to organize it?
> Extra loops for all current cases?
> suspend_{primary, sprite, cursor}_{page_flip, mmap_gtt, etc}? I believe the
> test will take so long to finish on this case what is bad for qa alghouth
> it is the complete one. What do you think?

I think we don't need the full set of tests also with system suspend. I
think just one test which catches the current bug is good enough, after
all if psr is set up correctly it should work the same at runtime than
over s/r. And since this is a test I'd just copypaste the relevant subtest
(if it doesn't integrate quickly into the existing code), not worth at all
to make a big fuzz.

And we have lots of resume tests already, they "only" take about
30 second. Only important to have "suspend" somewhere in the subtest name
so that all system suspend tests can easily be filtered out/selected.
-Daniel
diff mbox

Patch

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;