| blob | 56436865714fcbdee6817d0b96a3f3791e0d1e0e |
1 From aa16e81259f59734230d441905b9d0f605e4a4b5 Mon Sep 17 00:00:00 2001
2 From: =?UTF-8?q?Zbigniew=20Kempczy=C5=84ski?=
3 <zbigniew.kempczynski@intel.com>
4 Date: Sat, 14 Jan 2023 20:49:10 +0100
5 Subject: [PATCH] i915/gem_ccs: Remove race in creating batch buffers
6 MIME-Version: 1.0
7 Content-Type: text/plain; charset=UTF-8
8 Content-Transfer-Encoding: 8bit
10 Creating buffers from pool allows achieve pipelined execution easier -
11 returned buffer is not active from driver point of view. Unfortunately
12 buffer should be created before use, otherwise we can hit the race.
14 In the test in rare cases batch created for ctrl-surf from pool was
15 immediate executed and not active so batch created for block-copy had
16 same handle. Problem occurred when block-copy was executed immediately
17 after surf-copy without stalls and both batches used same handle,
18 so latter just overwrites instructions of previous one.
20 Instead of using buffer pool we create two separate batches, one for
21 surf-copy and second for block-copy. This will remove the buffer
22 creation race from the pool. When src and dst buffer are not equal
23 visual ascii dump of differrences grouped by 8x8 blocks of pixels
24 are dump to stdout.
26 Fixes: https://gitlab.freedesktop.org/drm/intel/-/issues/6683
28 Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
29 Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
30 Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
31 ---
32 tests/i915/gem_ccs.c | 71 +++++++++++++++++++++++++++++++++++++++-----
33 1 file changed, 63 insertions(+), 8 deletions(-)
35 diff --git a/tests/i915/gem_ccs.c b/tests/i915/gem_ccs.c
36 index 751f65e6..ff28c6d8 100644
37 --- a/tests/i915/gem_ccs.c
38 +++ b/tests/i915/gem_ccs.c
39 @@ -158,6 +158,54 @@ static void set_blt_object(struct blt_copy_object *obj,
40 if (param.write_png) \
41 blt_surface_to_png((fd), (id), (name), (obj), (w), (h)); } while (0)
43 +static int compare_nxn(const struct blt_copy_object *surf1,
44 + const struct blt_copy_object *surf2,
45 + int xsize, int ysize, int bx, int by)
46 +{
47 + int x, y, corrupted;
48 + uint32_t pos, px1, px2;
49 +
50 + corrupted = 0;
51 + for (y = 0; y < ysize; y++) {
52 + for (x = 0; x < xsize; x++) {
53 + pos = bx * xsize + by * ysize * surf1->pitch / 4;
54 + pos += x + y * surf1->pitch / 4;
55 + px1 = surf1->ptr[pos];
56 + px2 = surf2->ptr[pos];
57 + if (px1 != px2)
58 + corrupted++;
59 + }
60 + }
61 +
62 + return corrupted;
63 +}
64 +
65 +static void dump_corruption_info(const struct blt_copy_object *surf1,
66 + const struct blt_copy_object *surf2)
67 +{
68 + const int xsize = 8, ysize = 8;
69 + int w, h, bx, by, corrupted;
70 +
71 + igt_assert(surf1->x1 == surf2->x1 && surf1->x2 == surf2->x2);
72 + igt_assert(surf1->y1 == surf2->y1 && surf1->y2 == surf2->y2);
73 + w = surf1->x2;
74 + h = surf1->y2;
75 +
76 + igt_info("dump corruption - width: %d, height: %d, sizex: %x, sizey: %x\n",
77 + surf1->x2, surf1->y2, xsize, ysize);
78 +
79 + for (by = 0; by < h / ysize; by++) {
80 + for (bx = 0; bx < w / xsize; bx++) {
81 + corrupted = compare_nxn(surf1, surf2, xsize, ysize, bx, by);
82 + if (corrupted == 0)
83 + igt_info(".");
84 + else
85 + igt_info("%c", '0' + corrupted);
86 + }
87 + igt_info("\n");
88 + }
89 +}
90 +
91 static void surf_copy(int i915,
92 const intel_ctx_t *ctx,
93 const struct intel_execution_engine2 *e,
94 @@ -170,7 +218,7 @@ static void surf_copy(int i915,
95 struct blt_copy_data blt = {};
96 struct blt_block_copy_data_ext ext = {};
97 struct blt_ctrl_surf_copy_data surf = {};
98 - uint32_t bb, ccs, ccs2, *ccsmap, *ccsmap2;
99 + uint32_t bb1, bb2, ccs, ccs2, *ccsmap, *ccsmap2;
100 uint64_t bb_size, ccssize = mid->size / CCS_RATIO;
101 uint32_t *ccscopy;
102 uint8_t uc_mocs = intel_get_uc_mocs(i915);
103 @@ -178,8 +226,6 @@ static void surf_copy(int i915,
105 igt_assert(mid->compression);
106 ccscopy = (uint32_t *) malloc(ccssize);
107 - bb_size = 4096;
108 - bb = gem_create_from_pool(i915, &bb_size, REGION_SMEM);
109 ccs = gem_create(i915, ccssize);
110 ccs2 = gem_create(i915, ccssize);
112 @@ -189,7 +235,9 @@ static void surf_copy(int i915,
113 uc_mocs, INDIRECT_ACCESS);
114 set_surf_object(&surf.dst, ccs, REGION_SMEM, ccssize,
115 uc_mocs, DIRECT_ACCESS);
116 - set_batch(&surf.bb, bb, bb_size, REGION_SMEM);
117 + bb_size = 4096;
118 + igt_assert_eq(__gem_create(i915, &bb_size, &bb1), 0);
119 + set_batch(&surf.bb, bb1, bb_size, REGION_SMEM);
120 blt_ctrl_surf_copy(i915, ctx, e, ahnd, &surf);
121 gem_sync(i915, surf.dst.handle);
123 @@ -240,9 +288,8 @@ static void surf_copy(int i915,
124 set_blt_object(&blt.dst, dst);
125 set_object_ext(&ext.src, mid->compression_type, mid->x2, mid->y2, SURFACE_TYPE_2D);
126 set_object_ext(&ext.dst, 0, dst->x2, dst->y2, SURFACE_TYPE_2D);
127 - bb_size = 4096;
128 - bb = gem_create_from_pool(i915, &bb_size, REGION_SMEM);
129 - set_batch(&blt.bb, bb, bb_size, REGION_SMEM);
130 + igt_assert_eq(__gem_create(i915, &bb_size, &bb2), 0);
131 + set_batch(&blt.bb, bb2, bb_size, REGION_SMEM);
132 blt_block_copy(i915, ctx, e, ahnd, &blt, &ext);
133 gem_sync(i915, blt.dst.handle);
134 WRITE_PNG(i915, run_id, "corrupted", &blt.dst, dst->x2, dst->y2);
135 @@ -257,10 +304,18 @@ static void surf_copy(int i915,
136 gem_sync(i915, blt.dst.handle);
137 WRITE_PNG(i915, run_id, "corrected", &blt.dst, dst->x2, dst->y2);
138 result = memcmp(src->ptr, dst->ptr, src->size);
139 - igt_assert(result == 0);
140 + if (result)
141 + dump_corruption_info(src, dst);
143 munmap(ccsmap, ccssize);
144 gem_close(i915, ccs);
145 + gem_close(i915, ccs2);
146 + gem_close(i915, bb1);
147 + gem_close(i915, bb2);
148 +
149 + igt_assert_f(result == 0,
150 + "Source and destination surfaces are different after "
151 + "restoring source ccs data\n");
152 }
154 struct blt_copy3_data {
155 --
156 GitLab