[igt-dev] [PATCH i-g-t] tests/kms_cursor_crc: Fix user space read too slow error
Kahola, Mika
mika.kahola at intel.com
Wed Dec 18 11:44:14 UTC 2019
On Wed, 2019-12-11 at 22:09 +0200, Juha-Pekka Heikkila wrote:
> Having crc running continuously cause this test sometime
> fill crc buffer, fix this problem as well as do some generic
> cleanups.
>
> v2: Take out gem_sync()
>
> v3: Use rendercopy if available, otherwise Cairo surface for
> restoring
> test image
>
> CC: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
> Chris, about the comments on the previous patches for this test. On
> this
> patch there's two paths, if you want to try see those random crc
> failures
> I was getting disable on test fixture rendercopy part so this test
> will use
> Cairo to restore test image. I'm testing this with ICL where
> rendercopy path
> never fail but Cairo path fail roughly 1/10 run on random round for
> example
> on pipe-A-cursor-128x128-onscreen test (and basically any similar
> test).
>
> I also tried two fb version with flipping but using Cairo those crc
> errors were happening.
>
> Difference with those paths is rendercopy will have test image stored
> on
> FB created during setup, Cairo path has Cairo surface. At
> restore_image()
> then is used either to restore test image.
>
> When using Cairo path if I wait for two vblanks instead of one after
> restoring test image those random crc failures seem to go away.
>
> Anyway, I'm thinking both Cairo and rendercopy path should result in
> same
> outcome and most of the time they do. Then I have no idea where on
> Cairo
> path those failures are coming from.
>
> /Juha-Pekka
>
> tests/kms_cursor_crc.c | 204 +++++++++++++++++++++++++++++++++----
> ------------
> 1 file changed, 138 insertions(+), 66 deletions(-)
>
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 6475dea..76a2ef6 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -48,11 +48,10 @@ IGT_TEST_DESCRIPTION(
> typedef struct {
> int drm_fd;
> igt_display_t display;
> - struct igt_fb primary_fb;
> + struct igt_fb primary_fb[2];
> struct igt_fb fb;
> igt_output_t *output;
> enum pipe pipe;
> - igt_crc_t ref_crc;
> int left, right, top, bottom;
> int screenw, screenh;
> int refresh;
> @@ -60,11 +59,20 @@ typedef struct {
> int cursor_max_w, cursor_max_h;
> igt_pipe_crc_t *pipe_crc;
> unsigned flags;
> + igt_plane_t *primary;
> + igt_plane_t *cursor;
> + cairo_surface_t *surface;
> + uint32_t devid;
> + drm_intel_bufmgr *bufmgr;
> + igt_render_copyfunc_t rendercopy;
> } data_t;
>
> #define TEST_DPMS (1<<0)
> #define TEST_SUSPEND (1<<1)
>
> +#define FRONTBUFFER 0
> +#define RESTOREBUFFER 1
> +
> static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch,
> double a)
> {
> int wl, wr, ht, hb;
> @@ -89,23 +97,15 @@ static void draw_cursor(cairo_t *cr, int x, int
> y, int cw, int ch, double a)
>
> static void cursor_enable(data_t *data)
> {
> - igt_output_t *output = data->output;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
> -
> - igt_plane_set_fb(cursor, &data->fb);
> - igt_plane_set_size(cursor, data->curw, data->curh);
> - igt_fb_set_size(&data->fb, cursor, data->curw, data->curh);
> + igt_plane_set_fb(data->cursor, &data->fb);
> + igt_plane_set_size(data->cursor, data->curw, data->curh);
> + igt_fb_set_size(&data->fb, data->cursor, data->curw, data-
> >curh);
> }
>
> static void cursor_disable(data_t *data)
> {
> - igt_output_t *output = data->output;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
> -
> - igt_plane_set_fb(cursor, NULL);
> - igt_plane_set_position(cursor, 0, 0);
> + igt_plane_set_fb(data->cursor, NULL);
> + igt_plane_set_position(data->cursor, 0, 0);
> }
>
> static bool chv_cursor_broken(data_t *data, int x)
> @@ -144,36 +144,81 @@ static bool cursor_visible(data_t *data, int x,
> int y)
> return true;
> }
>
> +static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo,
> + struct igt_fb* fb)
> +{
> + buf->bo = bo;
> + buf->stride = fb->strides[0];
> + buf->tiling = fb->modifier;
> + buf->size = fb->size;
> + buf->bpp = fb->plane_bpp[0];
> +}
> +
> +static void restore_image(data_t *data)
> +{
> + cairo_t *cr;
> + drm_intel_bo *src, *dst;
> + struct intel_batchbuffer *batch;
> + struct igt_buf src_buf = {0}, dst_buf = {0};
> +
> + if (data->rendercopy != NULL) {
> + /* use rendercopy if available */
> + dst = gem_handle_to_libdrm_bo(data->bufmgr, data-
> >drm_fd, "", data->primary_fb[FRONTBUFFER].gem_handle);
> + igt_assert(dst);
> +
> + src = gem_handle_to_libdrm_bo(data->bufmgr, data-
> >drm_fd, "", data->primary_fb[RESTOREBUFFER].gem_handle);
> + igt_assert(src);
> +
> + scratch_buf_init(&src_buf, src, &data-
> >primary_fb[RESTOREBUFFER]);
> + scratch_buf_init(&dst_buf, dst, &data-
> >primary_fb[FRONTBUFFER]);
> +
> + batch = intel_batchbuffer_alloc(data->bufmgr, data-
> >devid);
> + igt_assert(batch);
> +
> + data->rendercopy(batch, NULL,
> + &src_buf, 0, 0, data-
> >primary_fb[RESTOREBUFFER].width, data-
> >primary_fb[RESTOREBUFFER].height,
> + &dst_buf, 0, 0);
> +
> + intel_batchbuffer_free(batch);
> + } else {
> + /* if rendercopy not implemented in igt use cairo */
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> + cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> + cairo_set_source_surface(cr, data->surface, 0, 0);
> + cairo_rectangle(cr, 0, 0, data->screenw, data-
> >screenh);
> + cairo_fill(cr);
> + igt_put_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER], cr);
> + }
> + igt_dirty_fb(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
> +}
> +
> static void do_single_test(data_t *data, int x, int y)
> {
> igt_display_t *display = &data->display;
> igt_pipe_crc_t *pipe_crc = data->pipe_crc;
> igt_crc_t crc, ref_crc;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
> cairo_t *cr;
> int ret = 0;
>
> igt_print_activity();
>
> /* Hardware test */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> - igt_paint_test_pattern(cr, data->screenw, data->screenh);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + restore_image(data);
>
> + igt_plane_set_position(data->cursor, x, y);
> cursor_enable(data);
> - igt_plane_set_position(cursor, x, y);
>
> if (chv_cursor_broken(data, x) && cursor_visible(data, x, y)) {
> ret = igt_display_try_commit2(display, COMMIT_LEGACY);
> igt_assert_eq(ret, -EINVAL);
> - igt_plane_set_position(cursor, 0, y);
> + igt_plane_set_position(data->cursor, 0, y);
>
> return;
> }
>
> igt_display_commit(display);
>
> + /* Extra vblank wait is because nonblocking cursor ioctl */
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc);
>
> @@ -205,45 +250,35 @@ static void do_single_test(data_t *data, int x,
> int y)
> }
>
> cursor_disable(data);
> - igt_display_commit(display);
>
> /* Now render the same in software and collect crc */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> draw_cursor(cr, x, y, data->curw, data->curh, 1.0);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER],
> cr);
> igt_display_commit(display);
> -
> + igt_dirty_fb(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
> + /* Extra vblank wait is because nonblocking cursor ioctl */
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> - igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
>
> + igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
> igt_assert_crc_equal(&crc, &ref_crc);
> -
> - /* Clear screen afterwards */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> - igt_paint_color(cr, 0, 0, data->screenw, data->screenh, 0.0,
> 0.0, 0.0);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> }
>
> static void do_fail_test(data_t *data, int x, int y, int expect)
> {
> igt_display_t *display = &data->display;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
> - cairo_t *cr;
> int ret;
>
> igt_print_activity();
>
> /* Hardware test */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> - igt_paint_test_pattern(cr, data->screenw, data->screenh);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + restore_image(data);
>
> cursor_enable(data);
> - igt_plane_set_position(cursor, x, y);
> + igt_plane_set_position(data->cursor, x, y);
> ret = igt_display_try_commit2(display, COMMIT_LEGACY);
>
> - igt_plane_set_position(cursor, 0, 0);
> + igt_plane_set_position(data->cursor, 0, 0);
> cursor_disable(data);
> igt_display_commit(display);
>
> @@ -355,7 +390,13 @@ static void cleanup_crtc(data_t *data)
> igt_pipe_crc_free(data->pipe_crc);
> data->pipe_crc = NULL;
>
> - igt_remove_fb(data->drm_fd, &data->primary_fb);
> + cairo_surface_destroy(data->surface);
> +
> + igt_plane_set_fb(data->primary, NULL);
> + igt_display_commit(display);
> +
> + igt_remove_fb(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
> + igt_remove_fb(data->drm_fd, &data->primary_fb[RESTOREBUFFER]);
>
> igt_display_reset(display);
> }
> @@ -365,21 +406,29 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output,
> {
> drmModeModeInfo *mode;
> igt_display_t *display = &data->display;
> - igt_plane_t *primary;
> + cairo_t *cr;
>
> /* select the pipe we want to use */
> igt_output_set_pipe(output, data->pipe);
>
> - /* create and set the primary plane fb */
> + /* create and set the primary plane fbs */
> mode = igt_output_get_mode(output);
> igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> DRM_FORMAT_XRGB8888,
> LOCAL_DRM_FORMAT_MOD_NONE,
> 0.0, 0.0, 0.0,
> - &data->primary_fb);
> + &data->primary_fb[FRONTBUFFER]);
> +
> + igt_create_color_fb(data->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> + DRM_FORMAT_XRGB8888,
> + LOCAL_DRM_FORMAT_MOD_NONE,
> + 0.0, 0.0, 0.0,
> + &data->primary_fb[RESTOREBUFFER]);
> +
> + data->primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> + data->cursor = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_CURSOR);
>
> - primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> - igt_plane_set_fb(primary, &data->primary_fb);
> + igt_plane_set_fb(data->primary, &data-
> >primary_fb[FRONTBUFFER]);
>
> igt_display_commit(display);
>
> @@ -398,9 +447,23 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output,
> data->curh = cursor_h;
> data->refresh = mode->vrefresh;
>
> - /* get reference crc w/o cursor */
> + data->surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24,
> data->screenw, data->screenh);
> +
> + if (data->rendercopy == NULL) {
> + /* store test image as cairo surface */
> + cr = cairo_create(data->surface);
> + cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> + igt_paint_test_pattern(cr, data->screenw, data-
> >screenh);
> + cairo_destroy(cr);
> + } else {
> + /* store test image as fb if rendercopy is available */
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[RESTOREBUFFER]);
> + cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> + igt_paint_test_pattern(cr, data->screenw, data-
> >screenh);
> + igt_put_cairo_ctx(data->drm_fd, &data-
> >primary_fb[RESTOREBUFFER], cr);
> + }
> +
> igt_pipe_crc_start(data->pipe_crc);
> - igt_pipe_crc_get_current(data->drm_fd, data->pipe_crc, &data-
> >ref_crc);
> }
>
> static void test_cursor_alpha(data_t *data, double a)
> @@ -432,9 +495,9 @@ static void test_cursor_alpha(data_t *data,
> double a)
> igt_remove_fb(data->drm_fd, &data->fb);
>
> /*Software Test*/
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER],
> cr);
>
> igt_display_commit(display);
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> @@ -442,10 +505,10 @@ static void test_cursor_alpha(data_t *data,
> double a)
> igt_assert_crc_equal(&crc, &ref_crc);
>
> /*Clear Screen*/
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
> 0.0, 0.0, 0.0);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER],
> cr);
> }
>
> static void test_cursor_transparent(data_t *data)
> @@ -521,8 +584,6 @@ static void test_cursor_size(data_t *data)
> uint32_t fb_id;
> int i, size;
> int cursor_max_size = data->cursor_max_w;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
>
> /* Create a maximum size cursor, then change the size in flight
> to
> * smaller ones to see that the size is applied correctly
> @@ -541,8 +602,8 @@ static void test_cursor_size(data_t *data)
> cursor_enable(data);
> for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++)
> {
> /* Change size in flight: */
> - igt_plane_set_size(cursor, size, size);
> - igt_fb_set_size(&data->fb, cursor, size, size);
> + igt_plane_set_size(data->cursor, size, size);
> + igt_fb_set_size(&data->fb, data->cursor, size, size);
> igt_display_commit(display);
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> igt_pipe_crc_get_current(data->drm_fd, pipe_crc,
> &crc[i]);
> @@ -553,18 +614,18 @@ static void test_cursor_size(data_t *data)
> /* Software test loop */
> for (i = 0, size = cursor_max_size; size >= 64; size /= 2, i++)
> {
> /* Now render the same in software and collect crc */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> igt_paint_color_alpha(cr, 0, 0, size, size, 1.0, 1.0,
> 1.0, 1.0);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + igt_put_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER], cr);
>
> igt_display_commit(display);
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> igt_pipe_crc_get_current(data->drm_fd, pipe_crc,
> &ref_crc);
> /* Clear screen afterwards */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb);
> + cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER]);
> igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
> 0.0, 0.0, 0.0);
> - igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> + igt_put_cairo_ctx(data->drm_fd, &data-
> >primary_fb[FRONTBUFFER], cr);
> igt_assert_crc_equal(&crc[i], &ref_crc);
> }
> }
> @@ -575,26 +636,24 @@ static void test_rapid_movement(data_t *data)
> int x = 0, y = 0;
> long usec;
> igt_display_t *display = &data->display;
> - igt_plane_t *cursor =
> - igt_output_get_plane_type(data->output,
> DRM_PLANE_TYPE_CURSOR);
>
> cursor_enable(data);
>
> gettimeofday(&start, NULL);
> for ( ; x < 100; x++) {
> - igt_plane_set_position(cursor, x, y);
> + igt_plane_set_position(data->cursor, x, y);
> igt_display_commit(display);
> }
> for ( ; y < 100; y++) {
> - igt_plane_set_position(cursor, x, y);
> + igt_plane_set_position(data->cursor, x, y);
> igt_display_commit(display);
> }
> for ( ; x > 0; x--) {
> - igt_plane_set_position(cursor, x, y);
> + igt_plane_set_position(data->cursor, x, y);
> igt_display_commit(display);
> }
> for ( ; y > 0; y--) {
> - igt_plane_set_position(cursor, x, y);
> + igt_plane_set_position(data->cursor, x, y);
> igt_display_commit(display);
> }
> gettimeofday(&end, NULL);
> @@ -736,6 +795,16 @@ igt_main
> igt_require_pipe_crc(data.drm_fd);
>
> igt_display_require(&data.display, data.drm_fd);
> +
> + if (is_i915_device(data.drm_fd)) {
> + data.bufmgr =
> drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> + igt_assert(data.bufmgr);
> + drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> +
> + data.devid = intel_get_drm_devid(data.drm_fd);
> + data.rendercopy =
> igt_get_render_copyfunc(data.devid);
> + igt_assert(data.rendercopy != NULL);
It seems that this assert is unnecessary. In the patch we use
rendercopy, if available, otherwise Cairo is in use.
-Mika-
> + }
> }
>
> data.cursor_max_w = cursor_width;
> @@ -746,6 +815,9 @@ igt_main
> run_tests_on_pipe(&data, pipe);
>
> igt_fixture {
> + if (data.bufmgr != NULL)
> + drm_intel_bufmgr_destroy(data.bufmgr);
> +
> igt_display_fini(&data.display);
> }
> }
More information about the igt-dev
mailing list