From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) by gabe.freedesktop.org (Postfix) with ESMTPS id E35E210E163 for ; Thu, 5 Oct 2023 10:52:30 +0000 (UTC) Message-ID: <8bb6299c-2807-3994-cb37-09415bb66798@linux.intel.com> Date: Thu, 5 Oct 2023 12:52:24 +0200 MIME-Version: 1.0 Content-Language: en-US To: Tvrtko Ursulin , igt-dev@lists.freedesktop.org References: <20230928174535.2074462-1-marcin.bernatowicz@linux.intel.com> <20230928174535.2074462-16-marcin.bernatowicz@linux.intel.com> <3d25cb8c-83b6-5bd4-181a-58d8066d4615@linux.intel.com> <06324cce-f5bd-4485-7bc2-25f3339d8575@linux.intel.com> <8cc14715-dac3-8336-e70e-f7fd95fb9be7@linux.intel.com> From: "Bernatowicz, Marcin" In-Reply-To: <8cc14715-dac3-8336-e70e-f7fd95fb9be7@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [igt-dev] [PATCH i-g-t 15/17] benchmarks/gem_wsim: introduce bb_size in w_step List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: chris.p.wilson@linux.intel.com Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 9/29/2023 12:49 PM, Tvrtko Ursulin wrote: > > On 29/09/2023 11:08, Bernatowicz, Marcin wrote: >> >> >> On 9/29/2023 11:35 AM, Tvrtko Ursulin wrote: >>> >>> On 28/09/2023 18:45, Marcin Bernatowicz wrote: >>>> Put it next to bb_handle. >>>> Use it in alloc_step_batch and measure_active_set. >>> >>> Could say why. >>> >>> Like xe might need more than 4k? Might not be able to allocate only >>> 4k? (Guessing only.) >> >> Xe uses following formula: >> >> w->bb_size = ALIGN(sizeof(*w->xe.data) + xe_cs_prefetch_size(fd), >>                 xe_get_default_alignment(fd)); >> >> which equaled 4096 on platform I tested. >> I didn't want to put bb_size inside xe specifics as it is connected >> with bb_handle. > > Hmmm could you dig a bit to figure out if sometimes this can be larger > than 4k and if so why only xe and not i915. Because things like prefetch > and alignment sound like should be more hardware than driver dependent. I got information there may be a case a prefetch size 4096 on some platform, but I did not get a clear answer why/if above calculation is needed/redundant. So I assume it's redundant and I will not copy/paste it from igt tests. There is one concern I have related to DRM_IOCTL_XE_GEM_CREATE ioctl and size field, suggesting a need for bb_size according to description: struct drm_xe_gem_create { ... /** * @size: Requested size for the object * * The (page-aligned) allocated size for the object will be returned. */ __u64 size; It suggests size could be adjusted after a call, but I think there is some discussion ongoing to that, so will wait. (and the xe_bo_create_flags does not take it into account and does not update size param) I will drop the patch, leave 4096 and I'm expecting a xe_bo_create failure if not possible. Regards, Marcin > > Regards, > > Tvrtko > >> >> Regards, >> marcin >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> Signed-off-by: Marcin Bernatowicz >>>> --- >>>>   benchmarks/gem_wsim.c | 6 ++++-- >>>>   1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c >>>> index 4618509ab..d22d66aeb 100644 >>>> --- a/benchmarks/gem_wsim.c >>>> +++ b/benchmarks/gem_wsim.c >>>> @@ -183,6 +183,7 @@ struct w_step { >>>>           } i915; >>>>       }; >>>>       uint32_t bb_handle; >>>> +    size_t bb_size; >>>>   }; >>>>   struct ctx { >>>> @@ -1481,6 +1482,7 @@ alloc_step_batch(struct workload *wrk, struct >>>> w_step *w) >>>>       unsigned int nr_obj = 2 + w->data_deps.nr; >>>>       unsigned int i; >>>> +    w->bb_size = 4096; >>>>       w->i915.obj = calloc(nr_obj, sizeof(*w->i915.obj)); >>>>       igt_assert(w->i915.obj); >>>> @@ -1522,7 +1524,7 @@ alloc_step_batch(struct workload *wrk, struct >>>> w_step *w) >>>>           igt_assert(j < nr_obj); >>>>       } >>>> -    w->bb_handle = w->i915.obj[j].handle = gem_create(fd, 4096); >>>> +    w->bb_handle = w->i915.obj[j].handle = gem_create(fd, w->bb_size); >>>>       w->i915.obj[j].relocation_count = create_bb(w, j); >>>>       igt_assert(w->i915.obj[j].relocation_count <= >>>> ARRAY_SIZE(w->i915.reloc)); >>>>       w->i915.obj[j].relocs_ptr = to_user_pointer(&w->i915.reloc); >>>> @@ -1722,7 +1724,7 @@ static void measure_active_set(struct workload >>>> *wrk) >>>>           if (w->type != BATCH) >>>>               continue; >>>> -        batch_sizes += 4096; >>>> +        batch_sizes += w->bb_size; >>>>           for (j = 0; j < w->data_deps.nr; j++) { >>>>               struct dep_entry *dep = &w->data_deps.list[j];