<!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> Hi Kamil, <div class="moz-cite-prefix">On 6/28/2024 7:02 PM, Kamil Konieczny wrote: </div> <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap="">Hi Nirmoy, On 2024-06-25 at 15:08:16 +0200, Nirmoy Das wrote: test names should use '-' as separator, you also used other name so: [PATCH i-g-t v3] tests/intel/xe_exec_store: Add basic_inst_benchmark should be: [PATCH i-g-t v3] tests/intel/xe_exec_store: Add basic-store-benchmark</pre> </blockquote> Looks like I misunderstood your last comment. Will fix it. <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Add basic_inst_benchmark to benchmark this basic operation </pre> </blockquote> <pre class="moz-quote-pre" wrap="">---------- ^----^ Same here, use '-' as separator, s/_inst_/-store-/ </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">for BO sizes to get basic understanding how long it takes bind a BO and run simple GPU command on it. This not a CI test but rather for developer to identify various bottleneck/regression in BO binding. Signed-off-by: Nirmoy Das <a class="moz-txt-link-rfc2396E" href="mailto:nirmoy.das@intel.com"><nirmoy.das@intel.com></a> --- tests/intel/xe_exec_store.c | 112 ++++++++++++++++++++++++++++++------ 1 file changed, 94 insertions(+), 18 deletions(-) diff --git a/tests/intel/xe_exec_store.c b/tests/intel/xe_exec_store.c index c872c22d5..aaabdbec3 100644 --- a/tests/intel/xe_exec_store.c +++ b/tests/intel/xe_exec_store.c @@ -93,15 +93,10 @@ static void persistance_batch(struct data *data, uint64_t addr) data->addr = batch_addr; } -/** - * SUBTEST: basic-store - * Description: Basic test to verify store dword. - * SUBTEST: basic-cond-batch - * Description: Basic test to verify cond batch end instruction. - * SUBTEST: basic-all - * Description: Test to verify store dword on all available engines. - */ -static void basic_inst(int fd, int inst_type, struct drm_xe_engine_class_instance *eci) + +static void basic_inst_size(int fd, int inst_type, + struct drm_xe_engine_class_instance *eci, + uint16_t cpu_caching, size_t bo_size) { struct drm_xe_sync sync[2] = { { .type = DRM_XE_SYNC_TYPE_SYNCOBJ, .flags = DRM_XE_SYNC_FLAG_SIGNAL, }, @@ -117,7 +112,6 @@ static void basic_inst(int fd, int inst_type, struct drm_xe_engine_class_instanc uint32_t exec_queue; uint32_t bind_engine; uint32_t syncobj; - size_t bo_size; int value = 0x123456; uint64_t addr = 0x100000; uint32_t bo = 0; @@ -127,12 +121,16 @@ static void basic_inst(int fd, int inst_type, struct drm_xe_engine_class_instanc sync[1].handle = syncobj; vm = xe_vm_create(fd, 0, 0); - bo_size = sizeof(*data); - bo_size = xe_bb_size(fd, bo_size); - bo = xe_bo_create(fd, vm, bo_size, - vram_if_possible(fd, eci->gt_id), - DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM); + if (cpu_caching) + bo = xe_bo_create_caching(fd, vm, bo_size, + vram_if_possible(fd, eci->gt_id), + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, + cpu_caching); + else + bo = xe_bo_create(fd, vm, bo_size, + vram_if_possible(fd, eci->gt_id), + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM); exec_queue = xe_exec_queue_create(fd, vm, eci, 0); bind_engine = xe_bind_exec_queue_create(fd, vm, 0); @@ -167,6 +165,66 @@ static void basic_inst(int fd, int inst_type, struct drm_xe_engine_class_instanc xe_vm_destroy(fd, vm); } + +/** + * SUBTEST: basic-store + * Description: Basic test to verify store dword. + * SUBTEST: basic-cond-batch + * Description: Basic test to verify cond batch end instruction. + * SUBTEST: basic-all + * Description: Test to verify store dword on all available engines. + */ +static void basic_inst(int fd, int inst_type, + struct drm_xe_engine_class_instance *eci, + uint16_t cpu_caching) +{ + size_t bo_size; + + bo_size = sizeof(struct data); + bo_size = xe_bb_size(fd, bo_size); + + basic_inst_size(fd, inst_type, eci, cpu_caching, bo_size); +} + +/** + * SUBTEST: basic-store-benchmark + * Description: Basic test to verify time taken for doing store dword with various size. + */ +static void basic_inst_benchmark(int fd, int inst_type, + struct drm_xe_engine_class_instance *eci, + uint16_t cpu_caching) +{ + struct { + size_t size; + const char *name; + } sizes[] = { + {SZ_4K, "SZ_4K"}, + {SZ_2M, "SZ_2M"}, + {SZ_64M, "SZ_64M"}, + {SZ_128M, "SZ_128M"}, + {SZ_256M, "SZ_256M"}, + {SZ_1G, "SZ_1G"} </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Could you use more human-friendly strings here? 4KB, 2MB, ...1GB</pre> </blockquote> Sure, will do that. <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ }; + + struct timeval start, end; + long seconds, useconds, utime; + + for (size_t i = 0; i < ARRAY_SIZE(sizes); ++i) { + size_t bo_size = sizes[i].size; + const char *size_name = sizes[i].name; + + gettimeofday(&start, NULL); + basic_inst_size(fd, inst_type, eci, cpu_caching, bo_size); + gettimeofday(&end, NULL); + + seconds = end.tv_sec - start.tv_sec; + useconds = end.tv_usec - start.tv_usec; + utime = (seconds * 1000000) + useconds; </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> imho there is igt function for such time measure.</pre> </blockquote> Do you mean igt_nsec_elapsed() <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ + igt_info("Time taken for size %s: %ld us\n", size_name, utime); + } +} + #define PAGES 1 #define NCACHELINES (4096/64) /** @@ -342,12 +400,30 @@ igt_main igt_subtest("basic-store") { engine = xe_engine(fd, 1); - basic_inst(fd, STORE, &engine->instance); + basic_inst(fd, COND_BATCH, &engine->instance, 0); + } + + igt_subtest_with_dynamic("basic-store-benchmark") { + struct dyn { + const char *name; + int cache; + } tests[] = { + {"WC", DRM_XE_GEM_CPU_CACHING_WC}, + {"WB", DRM_XE_GEM_CPU_CACHING_WB} + }; + /* Enable for iGFX only for now */ + igt_require(! xe_has_vram(fd)); </pre> </blockquote> <pre class="moz-quote-pre" wrap="">-------------------- ^ Please use checkpatch.pl for similar hints.</pre> </blockquote> Took me a while find the issue :D. I tend to do that but I guess I have to add a alias that will do a check patch before sending. <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> Could you test with 0 for dGFX?</pre> </blockquote> with fd == 0 ? I think DRM_XE_GEM_CPU_CACHING_WB doesn't work for dGPU. I have to check that on live machine. <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ + for (int i = 0; i < ARRAY_SIZE(tests); i++) { + igt_dynamic_f("%s", tests[i].name); </pre> </blockquote> <pre class="moz-quote-pre" wrap="">----------------------------------------------^ This is the reason you didn't see dynamic tests running with --dyn WC, it should be:</pre> </blockquote> it should be what ? You are keep a secret :) <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ engine = xe_engine(fd, 1); + basic_inst_benchmark(fd, STORE, &engine->instance, tests[i].cache); + } </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> igt_dynamic_f("%s", tests[i].name) { engine = xe_engine(fd, 1); basic_inst_benchmark(fd, STORE, &engine->instance, tests[i].cache); } Btw should you add a skip if engine == NULL?</pre> </blockquote> Yes, that should be safer. Thanks, Nirmoy <blockquote type="cite" cite="mid:20240628170230.6wqbesoeoz7ezpsl@kamilkon-DESK.igk.intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ } } igt_subtest("basic-cond-batch") { engine = xe_engine(fd, 1); - basic_inst(fd, COND_BATCH, &engine->instance); + basic_inst(fd, COND_BATCH, &engine->instance, 0); } igt_subtest_with_dynamic("basic-all") { @@ -356,7 +432,7 @@ igt_main xe_engine_class_string(hwe->engine_class), hwe->engine_instance, hwe->gt_id); - basic_inst(fd, STORE, hwe); + basic_inst(fd, STORE, hwe, 0); } } -- 2.42.0 </pre> </blockquote> </blockquote> </body> </html>