[igt-dev] [PATCH i-g-t 1/8] lib/igt_fb: Add igt_put_cairo_ctx as counter to igt_get_cairo_ctx
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Jan 31 17:03:52 UTC 2018
On Tue, Jan 23, 2018 at 01:56:35PM +0100, Maarten Lankhorst wrote:
> This will allow support for NV12 in the future, where igt_get_cairo_ctx
> will return a RGB image to draw with, which will be converted in
> igt_put_cairo_ctx so tests don't have to add special support for NV12.
>
> This is the same as cairo_destroy + checking for errors, but not all
> tests use this correctly so it's better to have a single handler for it.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
> lib/igt_fb.c | 34 ++++++++++++++++++++++++----------
> lib/igt_fb.h | 1 +
> tests/kms_chv_cursor_fail.c | 3 +--
> tests/kms_color.c | 4 ++--
> tests/kms_concurrent.c | 3 +--
> tests/kms_crtc_background_color.c | 2 +-
> tests/kms_cursor_crc.c | 23 +++++++++++++++++++----
> tests/kms_flip.c | 3 +--
> tests/kms_mmap_write_crc.c | 4 ++--
> tests/kms_plane.c | 9 +++------
> tests/kms_plane_multiple.c | 3 +--
> tests/kms_plane_scaling.c | 3 +--
> tests/kms_psr_sink_crc.c | 2 +-
> tests/kms_render.c | 2 +-
> tests/kms_rotation_crc.c | 2 +-
> tests/kms_setmode.c | 2 +-
> tests/testdisplay.c | 8 ++------
> 17 files changed, 63 insertions(+), 45 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ded639e833f1..39a83bae178a 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -771,8 +771,7 @@ unsigned int igt_create_color_fb(int fd, int width, int height,
>
> cr = igt_get_cairo_ctx(fd, fb);
> igt_paint_color(cr, 0, 0, width, height, r, g, b);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(fd, fb, cr);
>
> return fb_id;
> }
> @@ -809,8 +808,7 @@ unsigned int igt_create_pattern_fb(int fd, int width, int height,
>
> cr = igt_get_cairo_ctx(fd, fb);
> igt_paint_test_pattern(cr, width, height);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(fd, fb, cr);
>
> return fb_id;
> }
> @@ -853,8 +851,7 @@ unsigned int igt_create_color_pattern_fb(int fd, int width, int height,
> cr = igt_get_cairo_ctx(fd, fb);
> igt_paint_color(cr, 0, 0, width, height, r, g, b);
> igt_paint_test_pattern(cr, width, height);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(fd, fb, cr);
>
> return fb_id;
> }
> @@ -897,8 +894,7 @@ unsigned int igt_create_image_fb(int fd, int width, int height,
>
> cr = igt_get_cairo_ctx(fd, fb);
> igt_paint_image(cr, filename, 0, 0, width, height);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(fd, fb, cr);
>
> return fb_id;
> }
> @@ -998,7 +994,7 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
> layout.right.x, layout.right.y,
> layout.right.width, layout.right.height);
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, &fb, cr);
>
> return fb_id;
> }
> @@ -1206,7 +1202,7 @@ cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb)
> *
> * This initializes a cairo surface for @fb and then allocates a drawing context
> * for it. The return cairo drawing context should be released by calling
> - * cairo_destroy(). This also sets a default font for drawing text on
> + * igt_put_cairo_ctx(). This also sets a default font for drawing text on
> * framebuffers.
> *
> * Returns:
> @@ -1229,6 +1225,24 @@ cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb)
> return cr;
> }
>
> +/**
> + * igt_put_cairo_ctx:
> + * @fd: open i915 drm file descriptor
> + * @fb: pointer to an #igt_fb structure
> + * @cr: the cairo context returned by igt_get_cairo_ctx.
> + *
> + * This releases the cairo surface @cr returned by igt_get_cairo_ctx()
> + * for @fb, and writes the changes out to the framebuffer if cairo doesn't
> + * have native support for the format.
> + */
> +void igt_put_cairo_ctx(int fd, struct igt_fb *fb, cairo_t *cr)
> +{
> + cairo_status_t ret = cairo_status(cr);
> + igt_assert_f(ret == CAIRO_STATUS_SUCCESS, "Cairo failed to draw with %s\n", cairo_status_to_string(ret));
> +
> + cairo_destroy(cr);
I was looking at the whole cairo surface stuff, and it looks to me like
igt_get_cairo_surface() is missing a cairo_surface_reference() for the
case where already have fb->cairo_surface.
Maybe we should also add some asserts into eg. igt_plane_set_fb() to
make sure there is no fb->cairo_surface around anymore, to guarantee
that we've actually copied the contents from the temp surface back
to the real bo?
Anyway, this patch seems fine to me so
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> +}
> +
> /**
> * igt_remove_fb:
> * @fd: open i915 drm file descriptor
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index d30a7340c76e..3004f0656029 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -131,6 +131,7 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling);
> cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb);
> cairo_surface_t *igt_cairo_image_surface_create_from_png(const char *filename);
> cairo_t *igt_get_cairo_ctx(int fd, struct igt_fb *fb);
> +void igt_put_cairo_ctx(int fd, struct igt_fb *fb, cairo_t *cr);
> void igt_paint_color(cairo_t *cr, int x, int y, int w, int h,
> double r, double g, double b);
> void igt_paint_color_alpha(cairo_t *cr, int x, int y, int w, int h,
> diff --git a/tests/kms_chv_cursor_fail.c b/tests/kms_chv_cursor_fail.c
> index 3e74df1142da..1bcf8469088e 100644
> --- a/tests/kms_chv_cursor_fail.c
> +++ b/tests/kms_chv_cursor_fail.c
> @@ -87,8 +87,7 @@ static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
> else
> igt_paint_color_alpha(cr, 0, 0, data->fb.width, data->fb.height,
> 0.0, 0.0, 0.0, 0.0);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
> }
>
> static void cursor_move(data_t *data, int x, int y, int i)
> diff --git a/tests/kms_color.c b/tests/kms_color.c
> index 3b0a88802c93..dc4fcce241be 100644
> --- a/tests/kms_color.c
> +++ b/tests/kms_color.c
> @@ -86,7 +86,7 @@ static void paint_gradient_rectangles(data_t *data,
> colors[i].b);
> }
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
> }
>
> static void paint_rectangles(data_t *data,
> @@ -103,7 +103,7 @@ static void paint_rectangles(data_t *data,
> colors[i].r, colors[i].g, colors[i].b);
> }
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
> }
>
> static double *generate_table(uint32_t lut_size, double exp)
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
> index 3b2e601505c4..283acf8c8a7e 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -148,8 +148,7 @@ create_fb_for_mode_position(data_t *data, drmModeModeInfo *mode,
> rect_w[i], rect_h[i], 0.0, 0.0, 0.0);
> }
>
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb[primary->index], cr);
> }
>
> static void
> diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
> index e99c67bd185e..6407e19bafce 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -69,7 +69,7 @@ paint_background(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
> b = (double) ((background & 0xFF0000) >> 16) / 255.0;
> igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, alpha);
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->gfx_fd, &data->fb, cr);
> }
>
> static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 4c5e00c074a5..a164839ee2cc 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -141,13 +141,16 @@ static void do_single_test(data_t *data, int x, int y)
> igt_pipe_crc_t *pipe_crc = data->pipe_crc;
> igt_crc_t crc, ref_crc;
> igt_plane_t *cursor;
> - cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + 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);
> +
> cursor_enable(data);
> cursor = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_CURSOR);
> igt_plane_set_position(cursor, x, y);
> @@ -190,7 +193,9 @@ static void do_single_test(data_t *data, int x, int y)
> igt_display_commit(display);
>
> /* Now render the same in software and collect crc */
> + cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> draw_cursor(cr, x, y, data->curw, data->curh);
> + igt_put_cairo_ctx(data->drm_fd, &data->primary_fb, cr);
> igt_display_commit(display);
>
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> @@ -198,20 +203,25 @@ static void do_single_test(data_t *data, int x, int y)
> 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;
> - cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> + 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);
> +
> cursor_enable(data);
> cursor = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_CURSOR);
> igt_plane_set_position(cursor, x, y);
> @@ -446,7 +456,7 @@ static void create_cursor_fb(data_t *data, int cur_w, int cur_h)
>
> cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> draw_cursor(cr, 0, 0, cur_w, cur_h);
> - igt_assert(cairo_status(cr) == 0);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
> }
>
> static bool has_nonsquare_cursors(uint32_t devid)
> @@ -486,6 +496,7 @@ static void test_cursor_size(data_t *data)
> /* Use a solid white rectangle as the cursor */
> cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
> igt_paint_color_alpha(cr, 0, 0, cursor_max_size, cursor_max_size, 1.0, 1.0, 1.0, 1.0);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb, cr);
>
> /* Hardware test loop */
> cursor_enable(data);
> @@ -501,16 +512,20 @@ static void test_cursor_size(data_t *data)
> }
> cursor_disable(data);
> /* Software test loop */
> - cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
> 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);
> 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_display_commit(display);
> igt_wait_for_vblank(data->drm_fd, data->pipe);
> igt_pipe_crc_collect_crc(pipe_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);
> igt_assert_crc_equal(&crc[i], &ref_crc);
> }
> }
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 710ea52b4a8a..b43e77123e7f 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -976,8 +976,7 @@ static void paint_flip_mode(struct igt_fb *fb, bool odd_frame)
> cairo_set_source_rgb(cr, 1, 1, 1);
> cairo_fill(cr);
>
> - igt_assert(!cairo_status(cr));
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, fb, cr);
> }
>
> static int
> diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
> index dd44ce97238a..279d0f67dd53 100644
> --- a/tests/kms_mmap_write_crc.c
> +++ b/tests/kms_mmap_write_crc.c
> @@ -92,7 +92,7 @@ static void test(data_t *data)
>
> cr = igt_get_cairo_ctx(data->drm_fd, fb);
> igt_paint_test_pattern(cr, fb->width, fb->height);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
>
> /* flip to it to make it UC/WC and fully flushed */
> igt_plane_set_fb(data->primary, fb);
> @@ -135,7 +135,7 @@ static void test(data_t *data)
> * fully flushed */
> cr = igt_get_cairo_ctx(data->drm_fd, fb);
> igt_paint_test_pattern(cr, fb->width, fb->height);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
>
> igt_plane_set_fb(data->primary, fb);
> igt_display_commit(display);
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 9672763fe619..54bcffc16f4f 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -138,8 +138,7 @@ create_fb_for_mode__position(data_t *data, drmModeModeInfo *mode,
> igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay,
> 0.0, 1.0, 0.0);
> igt_paint_color(cr, rect_x, rect_y, rect_w, rect_h, 0.0, 0.0, 0.0);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
> }
>
> enum {
> @@ -279,8 +278,7 @@ create_fb_for_mode__panning(data_t *data, drmModeModeInfo *mode,
> mode->hdisplay, mode->vdisplay,
> 0.0, 0.0, 1.0);
>
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, fb, cr);
> }
>
> enum {
> @@ -436,8 +434,7 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> 0.0, 1.0, 0.0);
> if (width >= 164 && height >= 164)
> igt_paint_color(cr, 100, 100, 64, 64, 0.0, 0.0, 0.0);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, &fb, cr);
>
> igt_plane_set_fb(plane, &fb);
> igt_display_commit2(&data->display, COMMIT_UNIVERSAL);
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index aea59df84d76..95b713858edc 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -173,8 +173,7 @@ create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo
> rect_w[i], rect_h[i], 0.0, 0.0, 0.0);
> }
>
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb[primary->index], cr);
> }
>
>
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index c8283a3b23d1..d32f3441e8c9 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -120,8 +120,7 @@ static void paint_fb(data_t *d, struct igt_fb *fb)
>
> cr = igt_get_cairo_ctx(d->drm_fd, fb);
> igt_paint_color(cr, 0, 0, fb->width, fb->height, 0.0, 1.0, 0.0);
> - igt_assert(cairo_status(cr) == 0);
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(d->drm_fd, fb, cr);
> }
>
> static void check_scaling_pipe_plane_rot(data_t *d, igt_plane_t *plane,
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 26cf434af64a..2b60acf34e77 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -91,7 +91,7 @@ static void create_cursor_fb(data_t *data)
>
> 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);
> + igt_put_cairo_ctx(data->drm_fd, &data->fb_white, cr);
> }
>
>
> diff --git a/tests/kms_render.c b/tests/kms_render.c
> index d2208e38f84e..25a0c05b547c 100644
> --- a/tests/kms_render.c
> +++ b/tests/kms_render.c
> @@ -55,7 +55,7 @@ static int paint_fb(struct igt_fb *fb, const char *test_name,
> igt_cairo_printf_line(cr, align_hcenter, 10, "%s", mode_format_str);
> igt_cairo_printf_line(cr, align_hcenter, 10, "%s", cconf_str);
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, fb, cr);
>
> return 0;
> }
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 799cf1142579..5b190a0d03cb 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -120,7 +120,7 @@ paint_squares(data_t *data, igt_rotation_t rotation,
> igt_paint_color(cr, 0, h / 2, w / 2, h / 2, RGB_COLOR(bl));
> igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, RGB_COLOR(br));
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(data->gfx_fd, fb, cr);
> }
>
> static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 206d360607bb..06241b191a0f 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -161,7 +161,7 @@ static int paint_fb(struct igt_fb *fb, const char *test_name,
> crtc_str[i]);
> }
>
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, fb, cr);
>
> return 0;
> }
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index b0156c5cf0e1..0ff98a2b705e 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -227,9 +227,7 @@ paint_color_key(struct igt_fb *fb_info)
> cairo_set_source_rgb(cr, .8, .8, .8);
> cairo_fill(cr);
>
> - igt_assert(!cairo_status(cr));
> -
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, fb_info, cr);
> }
>
> static void paint_image(cairo_t *cr, const char *file)
> @@ -294,9 +292,7 @@ static void paint_output_info(struct connector *c, struct igt_fb *fb)
> if (qr_code)
> paint_image(cr, "pass.png");
>
> - igt_assert(!cairo_status(cr));
> -
> - cairo_destroy(cr);
> + igt_put_cairo_ctx(drm_fd, fb, cr);
> }
>
> static void sighandler(int signo)
> --
> 2.15.1
>
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Ville Syrjälä
Intel OTC
More information about the igt-dev
mailing list