| Message ID | 20181029193637.13954-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftest: fix 64K alignment in igt_write_huge | expand |
Quoting Matthew Auld (2018-10-29 19:36:37) > When using softpin it's not enough to just pad the vma size, we also > need to ensure the vma offset is at the start of the pt boundary, if we > plan to utilize 64K pages. For testing purposes, we have to assume the worst as well as best cases. Looks like we could extend this to cover both aligned and unaligned offsets. Or have I missed the purpose of the test? -Chris
On Mon, 29 Oct 2018 at 19:40, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Matthew Auld (2018-10-29 19:36:37) > > When using softpin it's not enough to just pad the vma size, we also > > need to ensure the vma offset is at the start of the pt boundary, if we > > plan to utilize 64K pages. > > For testing purposes, we have to assume the worst as well as best cases. > Looks like we could extend this to cover both aligned and unaligned > offsets. Or have I missed the purpose of the test? Yup, even better. > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Matthew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.19 next-20181029]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-selftest-fix-64K-alignment-in-igt_write_huge/20181030-034107
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/list.h:9:0,
from include/linux/agp_backend.h:33,
from include/drm/drmP.h:35,
from drivers/gpu/drm/i915/i915_gem.c:28:
drivers/gpu/drm/i915/selftests/huge_pages.c: In function 'igt_write_huge':
include/linux/kernel.h:845:29: warning: comparison of distinct pointer types lacks a cast
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
^
include/linux/kernel.h:859:4: note: in expansion of macro '__typecheck'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~
include/linux/kernel.h:869:24: note: in expansion of macro '__safe_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~
include/linux/kernel.h:885:19: note: in expansion of macro '__careful_cmp'
#define max(x, y) __careful_cmp(x, y, >)
^~~~~~~~~~~~~
>> drivers/gpu/drm/i915/selftests/huge_pages.c:1132:15: note: in expansion of macro 'max'
alignment = max(alignment, I915_GTT_PAGE_SIZE_2M);
^~~
vim +/max +1132 drivers/gpu/drm/i915/selftests/huge_pages.c
1106
1107 static int igt_write_huge(struct i915_gem_context *ctx,
1108 struct drm_i915_gem_object *obj)
1109 {
1110 struct drm_i915_private *i915 = to_i915(obj->base.dev);
1111 struct i915_address_space *vm =
1112 ctx->ppgtt ? &ctx->ppgtt->vm : &i915->ggtt.vm;
1113 static struct intel_engine_cs *engines[I915_NUM_ENGINES];
1114 struct intel_engine_cs *engine;
1115 I915_RND_STATE(prng);
1116 IGT_TIMEOUT(end_time);
1117 unsigned int id;
1118 u64 alignment;
1119 u64 max;
1120 u64 num;
1121 u64 size;
1122 int *order;
1123 int i, n;
1124 int err = 0;
1125
1126 GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
1127
1128 size = obj->base.size;
1129 alignment = rounddown_pow_of_two(obj->mm.page_sizes.sg);
1130 if (obj->mm.page_sizes.sg & I915_GTT_PAGE_SIZE_64K) {
1131 size = round_up(size, I915_GTT_PAGE_SIZE_2M);
> 1132 alignment = max(alignment, I915_GTT_PAGE_SIZE_2M);
1133 }
1134
1135 max = div_u64((vm->total - size), alignment);
1136
1137 n = 0;
1138 for_each_engine(engine, i915, id) {
1139 if (!intel_engine_can_store_dword(engine)) {
1140 pr_info("store-dword-imm not supported on engine=%u\n",
1141 id);
1142 continue;
1143 }
1144 engines[n++] = engine;
1145 }
1146
1147 if (!n)
1148 return 0;
1149
1150 /*
1151 * To keep things interesting when alternating between engines in our
1152 * randomized order, lets also make feeding to the same engine a few
1153 * times in succession a possibility by enlarging the permutation array.
1154 */
1155 order = i915_random_order(n * I915_NUM_ENGINES, &prng);
1156 if (!order)
1157 return -ENOMEM;
1158
1159 /*
1160 * Try various offsets in an ascending/descending fashion until we
1161 * timeout -- we want to avoid issues hidden by effectively always using
1162 * offset = 0.
1163 */
1164 i = 0;
1165 for_each_prime_number_from(num, 0, max) {
1166 u64 offset_low = num * alignment;
1167 u64 offset_high = (max - num) * alignment;
1168 u32 dword = offset_in_page(num) / 4;
1169
1170 engine = engines[order[i] % n];
1171 i = (i + 1) % (n * I915_NUM_ENGINES);
1172
1173 err = __igt_write_huge(ctx, engine, obj, size, offset_low,
1174 dword, num + 1);
1175 if (err)
1176 break;
1177
1178 err = __igt_write_huge(ctx, engine, obj, size, offset_high,
1179 dword, num + 1);
1180 if (err)
1181 break;
1182
1183 if (igt_timeout(end_time,
1184 "%s timed out on engine=%u, offset_low=%llx offset_high=%llx, max_page_size=%lx\n",
1185 __func__, engine->id, offset_low, offset_high,
1186 rounddown_pow_of_two(obj->mm.page_sizes.sg)))
1187 break;
1188 }
1189
1190 kfree(order);
1191
1192 return err;
1193 }
1194
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Matthew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.19 next-20181030]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-selftest-fix-64K-alignment-in-igt_write_huge/20181030-034107
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/i915_gem.c:6238:0:
drivers/gpu/drm/i915/selftests/huge_pages.c: In function 'igt_write_huge':
>> drivers/gpu/drm/i915/selftests/huge_pages.c:1132:72: warning: comparison of distinct pointer types lacks a cast
alignment = max(alignment, I915_GTT_PAGE_SIZE_2M);
^
vim +1132 drivers/gpu/drm/i915/selftests/huge_pages.c
1106
1107 static int igt_write_huge(struct i915_gem_context *ctx,
1108 struct drm_i915_gem_object *obj)
1109 {
1110 struct drm_i915_private *i915 = to_i915(obj->base.dev);
1111 struct i915_address_space *vm =
1112 ctx->ppgtt ? &ctx->ppgtt->vm : &i915->ggtt.vm;
1113 static struct intel_engine_cs *engines[I915_NUM_ENGINES];
1114 struct intel_engine_cs *engine;
1115 I915_RND_STATE(prng);
1116 IGT_TIMEOUT(end_time);
1117 unsigned int id;
1118 u64 alignment;
1119 u64 max;
1120 u64 num;
1121 u64 size;
1122 int *order;
1123 int i, n;
1124 int err = 0;
1125
1126 GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
1127
1128 size = obj->base.size;
1129 alignment = rounddown_pow_of_two(obj->mm.page_sizes.sg);
1130 if (obj->mm.page_sizes.sg & I915_GTT_PAGE_SIZE_64K) {
1131 size = round_up(size, I915_GTT_PAGE_SIZE_2M);
> 1132 alignment = max(alignment, I915_GTT_PAGE_SIZE_2M);
1133 }
1134
1135 max = div_u64((vm->total - size), alignment);
1136
1137 n = 0;
1138 for_each_engine(engine, i915, id) {
1139 if (!intel_engine_can_store_dword(engine)) {
1140 pr_info("store-dword-imm not supported on engine=%u\n",
1141 id);
1142 continue;
1143 }
1144 engines[n++] = engine;
1145 }
1146
1147 if (!n)
1148 return 0;
1149
1150 /*
1151 * To keep things interesting when alternating between engines in our
1152 * randomized order, lets also make feeding to the same engine a few
1153 * times in succession a possibility by enlarging the permutation array.
1154 */
1155 order = i915_random_order(n * I915_NUM_ENGINES, &prng);
1156 if (!order)
1157 return -ENOMEM;
1158
1159 /*
1160 * Try various offsets in an ascending/descending fashion until we
1161 * timeout -- we want to avoid issues hidden by effectively always using
1162 * offset = 0.
1163 */
1164 i = 0;
1165 for_each_prime_number_from(num, 0, max) {
1166 u64 offset_low = num * alignment;
1167 u64 offset_high = (max - num) * alignment;
1168 u32 dword = offset_in_page(num) / 4;
1169
1170 engine = engines[order[i] % n];
1171 i = (i + 1) % (n * I915_NUM_ENGINES);
1172
1173 err = __igt_write_huge(ctx, engine, obj, size, offset_low,
1174 dword, num + 1);
1175 if (err)
1176 break;
1177
1178 err = __igt_write_huge(ctx, engine, obj, size, offset_high,
1179 dword, num + 1);
1180 if (err)
1181 break;
1182
1183 if (igt_timeout(end_time,
1184 "%s timed out on engine=%u, offset_low=%llx offset_high=%llx, max_page_size=%lx\n",
1185 __func__, engine->id, offset_low, offset_high,
1186 rounddown_pow_of_two(obj->mm.page_sizes.sg)))
1187 break;
1188 }
1189
1190 kfree(order);
1191
1192 return err;
1193 }
1194
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index 256001b00e32..765e69471958 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -1114,8 +1114,8 @@ static int igt_write_huge(struct i915_gem_context *ctx, struct intel_engine_cs *engine; I915_RND_STATE(prng); IGT_TIMEOUT(end_time); - unsigned int max_page_size; unsigned int id; + u64 alignment; u64 max; u64 num; u64 size; @@ -1126,16 +1126,19 @@ static int igt_write_huge(struct i915_gem_context *ctx, GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj)); size = obj->base.size; - if (obj->mm.page_sizes.sg & I915_GTT_PAGE_SIZE_64K) + alignment = rounddown_pow_of_two(obj->mm.page_sizes.sg); + if (obj->mm.page_sizes.sg & I915_GTT_PAGE_SIZE_64K) { size = round_up(size, I915_GTT_PAGE_SIZE_2M); + alignment = max(alignment, I915_GTT_PAGE_SIZE_2M); + } - max_page_size = rounddown_pow_of_two(obj->mm.page_sizes.sg); - max = div_u64((vm->total - size), max_page_size); + max = div_u64((vm->total - size), alignment); n = 0; for_each_engine(engine, i915, id) { if (!intel_engine_can_store_dword(engine)) { - pr_info("store-dword-imm not supported on engine=%u\n", id); + pr_info("store-dword-imm not supported on engine=%u\n", + id); continue; } engines[n++] = engine; @@ -1160,24 +1163,27 @@ static int igt_write_huge(struct i915_gem_context *ctx, */ i = 0; for_each_prime_number_from(num, 0, max) { - u64 offset_low = num * max_page_size; - u64 offset_high = (max - num) * max_page_size; + u64 offset_low = num * alignment; + u64 offset_high = (max - num) * alignment; u32 dword = offset_in_page(num) / 4; engine = engines[order[i] % n]; i = (i + 1) % (n * I915_NUM_ENGINES); - err = __igt_write_huge(ctx, engine, obj, size, offset_low, dword, num + 1); + err = __igt_write_huge(ctx, engine, obj, size, offset_low, + dword, num + 1); if (err) break; - err = __igt_write_huge(ctx, engine, obj, size, offset_high, dword, num + 1); + err = __igt_write_huge(ctx, engine, obj, size, offset_high, + dword, num + 1); if (err) break; if (igt_timeout(end_time, - "%s timed out on engine=%u, offset_low=%llx offset_high=%llx, max_page_size=%x\n", - __func__, engine->id, offset_low, offset_high, max_page_size)) + "%s timed out on engine=%u, offset_low=%llx offset_high=%llx, max_page_size=%lx\n", + __func__, engine->id, offset_low, offset_high, + rounddown_pow_of_two(obj->mm.page_sizes.sg))) break; }
When using softpin it's not enough to just pad the vma size, we also need to ensure the vma offset is at the start of the pt boundary, if we plan to utilize 64K pages. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/selftests/huge_pages.c | 28 +++++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-)