From: Karolina Stolarek <karolina.stolarek@intel.com>
To: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>,
igt-dev@lists.freedesktop.org
Cc: Karolina Drobnik <karolina.drobnik@intel.com>
Subject: Re: [PATCH i-g-t v4 3/5] lib/intel_blt: Use object pitch and aligned height on png write
Date: Thu, 1 Feb 2024 14:51:32 +0100 [thread overview]
Message-ID: <2fae2b34-03e0-4ea6-8023-af943b9dcf8e@intel.com> (raw)
In-Reply-To: <20240201100724.257845-4-zbigniew.kempczynski@intel.com>
On 1.02.2024 11:07, Zbigniew Kempczyński wrote:
> Pitch and buffer height on tiled surfaces might be bigger than
> image width and height so dump memory to png using pitch size
> instead width and aligned height instead height.
>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Karolina Drobnik <karolina.drobnik@intel.com>
> ---
> v4: Use original width/height in filename instead stride/aligned
> height to avoid png overwriting (Zbigniew)
Reviewed-by: Karolina Stolarek <karolina.stolarek@intel.com>
> ---
> lib/intel_blt.c | 13 ++++++++++---
> lib/intel_blt.h | 2 +-
> tests/intel/gem_ccs.c | 23 ++++++++++++-----------
> tests/intel/gem_exercise_blt.c | 16 ++++++++--------
> tests/intel/xe_ccs.c | 23 ++++++++++++-----------
> tests/intel/xe_exercise_blt.c | 16 ++++++++--------
> 6 files changed, 51 insertions(+), 42 deletions(-)
>
> diff --git a/lib/intel_blt.c b/lib/intel_blt.c
> index 00208fc243..1cb5e68763 100644
> --- a/lib/intel_blt.c
> +++ b/lib/intel_blt.c
> @@ -2076,21 +2076,28 @@ void blt_surface_info(const char *info, const struct blt_copy_object *obj)
> * @obj: blitter copy object (@blt_copy_object) to save to png
> * @width: width
> * @height: height
> + * @bpp: bits per pixel
> *
> * Function save surface to png file. Assumes ARGB format where A == 0xff.
> */
> void blt_surface_to_png(int fd, uint32_t run_id, const char *fileid,
> const struct blt_copy_object *obj,
> - uint32_t width, uint32_t height)
> + uint32_t width, uint32_t height, uint32_t bpp)
> {
> cairo_surface_t *surface;
> cairo_status_t ret;
> + uint32_t dump_width = width, dump_height = height;
> uint8_t *map = (uint8_t *) obj->ptr;
> int format;
> int stride = obj->tiling ? obj->pitch * 4 : obj->pitch;
> char filename[FILENAME_MAX];
> bool is_xe = is_xe_device(fd);
>
> + if (obj->tiling) {
> + dump_width = obj->pitch;
> + dump_height = blt_get_aligned_height(height, bpp, obj->tiling);
> + }
> +
> snprintf(filename, FILENAME_MAX-1, "%d-%s-%s-%ux%u-%s.png",
> run_id, fileid, blt_tiling_name(obj->tiling), width, height,
> obj->compression ? "compressed" : "uncompressed");
> @@ -2103,8 +2110,8 @@ void blt_surface_to_png(int fd, uint32_t run_id, const char *fileid,
> obj->size, PROT_READ);
> }
> format = CAIRO_FORMAT_RGB24;
> - surface = cairo_image_surface_create_for_data(map,
> - format, width, height,
> + surface = cairo_image_surface_create_for_data(map, format,
> + dump_width, dump_height,
> stride);
> ret = cairo_surface_write_to_png(surface, filename);
> if (ret)
> diff --git a/lib/intel_blt.h b/lib/intel_blt.h
> index e3084dc0cd..077283a088 100644
> --- a/lib/intel_blt.h
> +++ b/lib/intel_blt.h
> @@ -315,7 +315,7 @@ void blt_surface_fill_rect(int fd, const struct blt_copy_object *obj,
> uint32_t width, uint32_t height);
> void blt_surface_to_png(int fd, uint32_t run_id, const char *fileid,
> const struct blt_copy_object *obj,
> - uint32_t width, uint32_t height);
> + uint32_t width, uint32_t height, uint32_t bpp);
> void blt_dump_corruption_info_32b(const struct blt_copy_object *surf1,
> const struct blt_copy_object *surf2);
>
> diff --git a/tests/intel/gem_ccs.c b/tests/intel/gem_ccs.c
> index 80a29ecab7..e8f16d7d88 100644
> --- a/tests/intel/gem_ccs.c
> +++ b/tests/intel/gem_ccs.c
> @@ -82,9 +82,9 @@ struct test_config {
> if (param.print_surface_info) \
> blt_surface_info((name), (obj)); } while (0)
>
> -#define WRITE_PNG(fd, id, name, obj, w, h) do { \
> +#define WRITE_PNG(fd, id, name, obj, w, h, bpp) do { \
> if (param.write_png) \
> - blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
> + blt_surface_to_png((fd), (id), (name), (obj), (w), (h), (bpp)); } while (0)
>
> static void surf_copy(int i915,
> const intel_ctx_t *ctx,
> @@ -98,6 +98,7 @@ static void surf_copy(int i915,
> struct blt_copy_data blt = {};
> struct blt_block_copy_data_ext ext = {};
> struct blt_ctrl_surf_copy_data surf = {};
> + const uint32_t bpp = 32;
> uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> uint64_t bb_size, ccssize = mid->size / CCS_RATIO(i915);
> uint32_t *ccscopy;
> @@ -172,7 +173,7 @@ static void surf_copy(int i915,
> blt_set_batch(&blt.bb, bb2, bb_size, REGION_SMEM);
> blt_block_copy(i915, ctx, e, ahnd, &blt, &ext);
> gem_sync(i915, blt.dst.handle);
> - WRITE_PNG(i915, run_id, "corrupted", &blt.dst, dst->x2, dst->y2);
> + WRITE_PNG(i915, run_id, "corrupted", &blt.dst, dst->x2, dst->y2, bpp);
> result = memcmp(src->ptr, dst->ptr, src->size);
> igt_assert(result != 0);
>
> @@ -182,7 +183,7 @@ static void surf_copy(int i915,
>
> blt_block_copy(i915, ctx, e, ahnd, &blt, &ext);
> gem_sync(i915, blt.dst.handle);
> - WRITE_PNG(i915, run_id, "corrected", &blt.dst, dst->x2, dst->y2);
> + WRITE_PNG(i915, run_id, "corrected", &blt.dst, dst->x2, dst->y2, bpp);
> result = memcmp(src->ptr, dst->ptr, src->size);
> if (result)
> blt_dump_corruption_info_32b(src, dst);
> @@ -346,7 +347,7 @@ static void block_copy(int i915,
> PRINT_SURFACE_INFO("dst", dst);
>
> blt_surface_fill_rect(i915, src, width, height);
> - WRITE_PNG(i915, run_id, "src", src, width, height);
> + WRITE_PNG(i915, run_id, "src", src, width, height, bpp);
>
> blt.color_depth = CD_32bit;
> blt.print_bb = param.print_bb;
> @@ -363,7 +364,7 @@ static void block_copy(int i915,
> if (mid->compression)
> igt_assert(memcmp(src->ptr, mid->ptr, src->size) != 0);
>
> - WRITE_PNG(i915, run_id, "mid", &blt.dst, width, height);
> + WRITE_PNG(i915, run_id, "mid", &blt.dst, width, height, bpp);
>
> if (config->surfcopy && pext) {
> const intel_ctx_t *surf_ctx = ctx;
> @@ -408,7 +409,7 @@ static void block_copy(int i915,
> blt_set_batch(&blt.bb, bb, bb_size, region1);
> blt_block_copy(i915, ctx, e, ahnd, &blt, pext);
> gem_sync(i915, blt.dst.handle);
> - WRITE_PNG(i915, run_id, "dst", &blt.dst, width, height);
> + WRITE_PNG(i915, run_id, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
> @@ -491,11 +492,11 @@ static void block_multicopy(int i915,
> blt_block_copy3(i915, ctx, e, ahnd, &blt3, pext3);
> gem_sync(i915, blt3.final.handle);
>
> - WRITE_PNG(i915, run_id, "src", &blt3.src, width, height);
> + WRITE_PNG(i915, run_id, "src", &blt3.src, width, height, bpp);
> if (!config->inplace)
> - WRITE_PNG(i915, run_id, "mid", &blt3.mid, width, height);
> - WRITE_PNG(i915, run_id, "dst", &blt3.dst, width, height);
> - WRITE_PNG(i915, run_id, "final", &blt3.final, width, height);
> + WRITE_PNG(i915, run_id, "mid", &blt3.mid, width, height, bpp);
> + WRITE_PNG(i915, run_id, "dst", &blt3.dst, width, height, bpp);
> + WRITE_PNG(i915, run_id, "final", &blt3.final, width, height, bpp);
>
> result = memcmp(src->ptr, blt3.final.ptr, src->size);
>
> diff --git a/tests/intel/gem_exercise_blt.c b/tests/intel/gem_exercise_blt.c
> index 7355eabbe9..4a9bc37298 100644
> --- a/tests/intel/gem_exercise_blt.c
> +++ b/tests/intel/gem_exercise_blt.c
> @@ -50,9 +50,9 @@ static struct param {
> if (param.print_surface_info) \
> blt_surface_info((name), (obj)); } while (0)
>
> -#define WRITE_PNG(fd, id, name, obj, w, h) do { \
> +#define WRITE_PNG(fd, id, name, obj, w, h, bpp) do { \
> if (param.write_png) \
> - blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
> + blt_surface_to_png((fd), (id), (name), (obj), (w), (h), (bpp)); } while (0)
>
> struct blt_fast_copy_data {
> int i915;
> @@ -167,7 +167,7 @@ static void fast_copy_emit(int i915, const intel_ctx_t *ctx,
> PRINT_SURFACE_INFO("dst", dst);
>
> blt_surface_fill_rect(i915, src, width, height);
> - WRITE_PNG(i915, mid_tiling, "src", src, width, height);
> + WRITE_PNG(i915, mid_tiling, "src", src, width, height, bpp);
>
> memset(&blt, 0, sizeof(blt));
> blt.color_depth = CD_32bit;
> @@ -180,8 +180,8 @@ static void fast_copy_emit(int i915, const intel_ctx_t *ctx,
> fast_copy_one_bb(i915, ctx, e, ahnd, &blt);
> gem_sync(i915, blt.dst.handle);
>
> - WRITE_PNG(i915, mid_tiling, "mid", &blt.mid, width, height);
> - WRITE_PNG(i915, mid_tiling, "dst", &blt.dst, width, height);
> + WRITE_PNG(i915, mid_tiling, "mid", &blt.mid, width, height, bpp);
> + WRITE_PNG(i915, mid_tiling, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
> @@ -234,8 +234,8 @@ static void fast_copy(int i915, const intel_ctx_t *ctx,
> blt_fast_copy(i915, ctx, e, ahnd, &blt);
> gem_sync(i915, mid->handle);
>
> - WRITE_PNG(i915, mid_tiling, "src", &blt.src, width, height);
> - WRITE_PNG(i915, mid_tiling, "mid", &blt.dst, width, height);
> + WRITE_PNG(i915, mid_tiling, "src", &blt.src, width, height, bpp);
> + WRITE_PNG(i915, mid_tiling, "mid", &blt.dst, width, height, bpp);
>
> blt_copy_init(i915, &blt);
> blt.color_depth = CD_32bit;
> @@ -247,7 +247,7 @@ static void fast_copy(int i915, const intel_ctx_t *ctx,
> blt_fast_copy(i915, ctx, e, ahnd, &blt);
> gem_sync(i915, blt.dst.handle);
>
> - WRITE_PNG(i915, mid_tiling, "dst", &blt.dst, width, height);
> + WRITE_PNG(i915, mid_tiling, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
> diff --git a/tests/intel/xe_ccs.c b/tests/intel/xe_ccs.c
> index 7d0e8ed7a5..a7785edcb1 100644
> --- a/tests/intel/xe_ccs.c
> +++ b/tests/intel/xe_ccs.c
> @@ -79,9 +79,9 @@ struct test_config {
> if (param.print_surface_info) \
> blt_surface_info((name), (obj)); } while (0)
>
> -#define WRITE_PNG(fd, id, name, obj, w, h) do { \
> +#define WRITE_PNG(fd, id, name, obj, w, h, bpp) do { \
> if (param.write_png) \
> - blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
> + blt_surface_to_png((fd), (id), (name), (obj), (w), (h), (bpp)); } while (0)
>
> static void surf_copy(int xe,
> intel_ctx_t *ctx,
> @@ -94,6 +94,7 @@ static void surf_copy(int xe,
> struct blt_copy_data blt = {};
> struct blt_block_copy_data_ext ext = {};
> struct blt_ctrl_surf_copy_data surf = {};
> + const uint32_t bpp = 32;
> uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
> uint64_t bb_size, ccssize = mid->size / CCS_RATIO(xe);
> uint64_t ccs_bo_size = xe_get_default_alignment(xe);
> @@ -179,7 +180,7 @@ static void surf_copy(int xe,
> blt_set_batch(&blt.bb, bb2, bb_size, sysmem);
> blt_block_copy(xe, ctx, NULL, ahnd, &blt, &ext);
> intel_ctx_xe_sync(ctx, true);
> - WRITE_PNG(xe, run_id, "corrupted", &blt.dst, dst->x2, dst->y2);
> + WRITE_PNG(xe, run_id, "corrupted", &blt.dst, dst->x2, dst->y2, bpp);
> result = memcmp(src->ptr, dst->ptr, src->size);
> igt_assert(result != 0);
>
> @@ -189,7 +190,7 @@ static void surf_copy(int xe,
>
> blt_block_copy(xe, ctx, NULL, ahnd, &blt, &ext);
> intel_ctx_xe_sync(ctx, true);
> - WRITE_PNG(xe, run_id, "corrected", &blt.dst, dst->x2, dst->y2);
> + WRITE_PNG(xe, run_id, "corrected", &blt.dst, dst->x2, dst->y2, bpp);
> result = memcmp(src->ptr, dst->ptr, src->size);
> if (result)
> blt_dump_corruption_info_32b(src, dst);
> @@ -326,7 +327,7 @@ static void block_copy(int xe,
> PRINT_SURFACE_INFO("dst", dst);
>
> blt_surface_fill_rect(xe, src, width, height);
> - WRITE_PNG(xe, run_id, "src", src, width, height);
> + WRITE_PNG(xe, run_id, "src", src, width, height, bpp);
>
> blt.color_depth = CD_32bit;
> blt.print_bb = param.print_bb;
> @@ -342,7 +343,7 @@ static void block_copy(int xe,
> if (mid->compression)
> igt_assert(memcmp(src->ptr, mid->ptr, src->size) != 0);
>
> - WRITE_PNG(xe, run_id, "mid", &blt.dst, width, height);
> + WRITE_PNG(xe, run_id, "mid", &blt.dst, width, height, bpp);
>
> if (config->surfcopy && pext) {
> struct drm_xe_engine_class_instance inst = {
> @@ -393,7 +394,7 @@ static void block_copy(int xe,
> blt_block_copy(xe, ctx, NULL, ahnd, &blt, pext);
> intel_ctx_xe_sync(ctx, true);
>
> - WRITE_PNG(xe, run_id, "dst", &blt.dst, width, height);
> + WRITE_PNG(xe, run_id, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
> @@ -486,11 +487,11 @@ static void block_multicopy(int xe,
> blt_block_copy3(xe, ctx, ahnd, &blt3, pext3);
> intel_ctx_xe_sync(ctx, true);
>
> - WRITE_PNG(xe, run_id, "src", &blt3.src, width, height);
> + WRITE_PNG(xe, run_id, "src", &blt3.src, width, height, bpp);
> if (!config->inplace)
> - WRITE_PNG(xe, run_id, "mid", &blt3.mid, width, height);
> - WRITE_PNG(xe, run_id, "dst", &blt3.dst, width, height);
> - WRITE_PNG(xe, run_id, "final", &blt3.final, width, height);
> + WRITE_PNG(xe, run_id, "mid", &blt3.mid, width, height, bpp);
> + WRITE_PNG(xe, run_id, "dst", &blt3.dst, width, height, bpp);
> + WRITE_PNG(xe, run_id, "final", &blt3.final, width, height, bpp);
>
> result = memcmp(src->ptr, blt3.final.ptr, src->size);
>
> diff --git a/tests/intel/xe_exercise_blt.c b/tests/intel/xe_exercise_blt.c
> index c908800cf4..ddf9d188a7 100644
> --- a/tests/intel/xe_exercise_blt.c
> +++ b/tests/intel/xe_exercise_blt.c
> @@ -53,9 +53,9 @@ static struct param {
> if (param.print_surface_info) \
> blt_surface_info((name), (obj)); } while (0)
>
> -#define WRITE_PNG(fd, id, name, obj, w, h) do { \
> +#define WRITE_PNG(fd, id, name, obj, w, h, bpp) do { \
> if (param.write_png) \
> - blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
> + blt_surface_to_png((fd), (id), (name), (obj), (w), (h), (bpp)); } while (0)
>
> struct blt_fast_copy_data {
> int xe;
> @@ -141,7 +141,7 @@ static void fast_copy_emit(int xe, const intel_ctx_t *ctx,
> PRINT_SURFACE_INFO("dst", dst);
>
> blt_surface_fill_rect(xe, src, width, height);
> - WRITE_PNG(xe, mid_tiling, "src", src, width, height);
> + WRITE_PNG(xe, mid_tiling, "src", src, width, height, bpp);
>
> memset(&blt, 0, sizeof(blt));
> blt.color_depth = CD_32bit;
> @@ -153,8 +153,8 @@ static void fast_copy_emit(int xe, const intel_ctx_t *ctx,
>
> fast_copy_one_bb(xe, ctx, ahnd, &blt);
>
> - WRITE_PNG(xe, mid_tiling, "mid", &blt.mid, width, height);
> - WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height);
> + WRITE_PNG(xe, mid_tiling, "mid", &blt.mid, width, height, bpp);
> + WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
> @@ -205,8 +205,8 @@ static void fast_copy(int xe, const intel_ctx_t *ctx,
>
> blt_fast_copy(xe, ctx, NULL, ahnd, &blt);
>
> - WRITE_PNG(xe, mid_tiling, "src", &blt.src, width, height);
> - WRITE_PNG(xe, mid_tiling, "mid", &blt.dst, width, height);
> + WRITE_PNG(xe, mid_tiling, "src", &blt.src, width, height, bpp);
> + WRITE_PNG(xe, mid_tiling, "mid", &blt.dst, width, height, bpp);
>
> blt_copy_init(xe, &blt);
> blt.color_depth = CD_32bit;
> @@ -217,7 +217,7 @@ static void fast_copy(int xe, const intel_ctx_t *ctx,
>
> blt_fast_copy(xe, ctx, NULL, ahnd, &blt);
>
> - WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height);
> + WRITE_PNG(xe, mid_tiling, "dst", &blt.dst, width, height, bpp);
>
> result = memcmp(src->ptr, blt.dst.ptr, src->size);
>
next prev parent reply other threads:[~2024-02-01 13:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-01 10:07 [PATCH i-g-t v4 0/5] Fill block-copy test gap for unaligned sizes Zbigniew Kempczyński
2024-02-01 10:07 ` [PATCH i-g-t v4 1/5] lib/intel_blt: Add helpers for calculating stride and aligned height Zbigniew Kempczyński
2024-02-01 12:05 ` Karolina Stolarek
2024-02-01 19:07 ` Zbigniew Kempczyński
2024-02-01 10:07 ` [PATCH i-g-t v4 2/5] lib/intel_blt: Change surface size calculation Zbigniew Kempczyński
2024-02-01 13:44 ` Karolina Stolarek
2024-02-01 19:27 ` Zbigniew Kempczyński
2024-02-01 10:07 ` [PATCH i-g-t v4 3/5] lib/intel_blt: Use object pitch and aligned height on png write Zbigniew Kempczyński
2024-02-01 13:51 ` Karolina Stolarek [this message]
2024-02-01 10:07 ` [PATCH i-g-t v4 4/5] tests/xe-ccs: Add tests which exercise small to large blit sizes Zbigniew Kempczyński
2024-02-01 14:09 ` Karolina Stolarek
2024-02-01 19:32 ` Zbigniew Kempczyński
2024-02-01 14:10 ` Karolina Stolarek
2024-02-01 19:54 ` Zbigniew Kempczyński
2024-02-01 10:07 ` [PATCH i-g-t v4 5/5] tests/xe_exercise_blt: Exercise small to large fast-copy blits Zbigniew Kempczyński
2024-02-01 14:18 ` Karolina Stolarek
2024-02-01 19:57 ` Zbigniew Kempczyński
2024-02-01 14:23 ` Karolina Stolarek
2024-02-01 19:58 ` Zbigniew Kempczyński
2024-02-01 11:43 ` ✓ CI.xeBAT: success for Fill block-copy test gap for unaligned sizes (rev4) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2fae2b34-03e0-4ea6-8023-af943b9dcf8e@intel.com \
--to=karolina.stolarek@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=karolina.drobnik@intel.com \
--cc=zbigniew.kempczynski@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox