From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by gabe.freedesktop.org (Postfix) with ESMTPS id D5F1410E0CF for ; Mon, 30 Jan 2023 08:10:21 +0000 (UTC) Message-ID: <60dfaf80-8a48-d887-8586-40f8250e3def@intel.com> Date: Mon, 30 Jan 2023 09:10:10 +0100 Content-Language: en-US To: =?UTF-8?Q?Zbigniew_Kempczy=c5=84ski?= References: <9583a3ed0f8e373e447553726f41313a49e91279.1674469729.git.karolina.stolarek@intel.com> <20230126120049.cwrq4fym3fovgexu@zkempczy-mobl2> From: Karolina Stolarek In-Reply-To: <20230126120049.cwrq4fym3fovgexu@zkempczy-mobl2> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t 1/2] lib/i915_blt: Remove create_mapping flag from blt_create_object List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: Hi Zbigniew, Sorry for my late response, the email got stuck in my inbox. On 26.01.2023 13:00, Zbigniew Kempczyński wrote: > On Mon, Jan 23, 2023 at 11:52:42AM +0100, Karolina Stolarek wrote: >> In the tests we have, we always set create_mapping flag to true, as we >> wish to create memory mappings when creating blitter copy objects. As >> this is the only use case, we can delete the flag and just create a >> mapping. >> >> Cc: Zbigniew Kempczynski >> Signed-off-by: Karolina Stolarek >> --- >> lib/i915/i915_blt.c | 8 +++----- >> lib/i915/i915_blt.h | 3 +-- >> tests/i915/gem_ccs.c | 14 +++++++------- >> tests/i915/gem_exercise_blt.c | 12 ++++++------ >> 4 files changed, 17 insertions(+), 20 deletions(-) >> >> diff --git a/lib/i915/i915_blt.c b/lib/i915/i915_blt.c >> index bbfb6ffc..3e64efeb 100644 >> --- a/lib/i915/i915_blt.c >> +++ b/lib/i915/i915_blt.c >> @@ -1219,8 +1219,7 @@ blt_create_object(int i915, uint32_t region, >> uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs, >> enum blt_tiling_type tiling, >> enum blt_compression compression, >> - enum blt_compression_type compression_type, >> - bool create_mapping) >> + enum blt_compression_type compression_type) > > In this case I would like to keep create_mapping flag. gem_ccs and > gem_exercise_blt use this mapping but if you someone would like > to reuse blt_create_object() without creation additional mapping > lack of this argument enforces creating object in the test. Hm, but in the majority of cases we'd like to create a new object. From the API point of view, we could introduce another function to do it. The list of parameters is already quite long here. Thanks, Karolina > > -- > Zbigniew > >> { >> struct blt_copy_object *obj; >> uint64_t size = width * height * bpp / 8; >> @@ -1237,9 +1236,8 @@ blt_create_object(int i915, uint32_t region, >> compression, compression_type); >> blt_set_geom(obj, stride, 0, 0, width, height, 0, 0); >> >> - if (create_mapping) >> - obj->ptr = gem_mmap__device_coherent(i915, handle, 0, size, >> - PROT_READ | PROT_WRITE); >> + obj->ptr = gem_mmap__device_coherent(i915, handle, 0, size, >> + PROT_READ | PROT_WRITE); >> >> return obj; >> } >> diff --git a/lib/i915/i915_blt.h b/lib/i915/i915_blt.h >> index 299dff8e..eaf4cc1f 100644 >> --- a/lib/i915/i915_blt.h >> +++ b/lib/i915/i915_blt.h >> @@ -218,8 +218,7 @@ blt_create_object(int i915, uint32_t region, >> uint32_t width, uint32_t height, uint32_t bpp, uint8_t mocs, >> enum blt_tiling_type tiling, >> enum blt_compression compression, >> - enum blt_compression_type compression_type, >> - bool create_mapping); >> + enum blt_compression_type compression_type); >> void blt_destroy_object(int i915, struct blt_copy_object *obj); >> void blt_set_object(struct blt_copy_object *obj, >> uint32_t handle, uint64_t size, uint32_t region, >> diff --git a/tests/i915/gem_ccs.c b/tests/i915/gem_ccs.c >> index f629f664..b7a32673 100644 >> --- a/tests/i915/gem_ccs.c >> +++ b/tests/i915/gem_ccs.c >> @@ -361,11 +361,11 @@ static void block_copy(int i915, >> pext = NULL; >> >> src = blt_create_object(i915, region1, width, height, bpp, uc_mocs, >> - T_LINEAR, COMPRESSION_DISABLED, comp_type, true); >> + T_LINEAR, COMPRESSION_DISABLED, comp_type); >> mid = blt_create_object(i915, mid_region, width, height, bpp, uc_mocs, >> - mid_tiling, mid_compression, comp_type, true); >> + mid_tiling, mid_compression, comp_type); >> dst = blt_create_object(i915, region1, width, height, bpp, uc_mocs, >> - T_LINEAR, COMPRESSION_DISABLED, comp_type, true); >> + T_LINEAR, COMPRESSION_DISABLED, comp_type); >> igt_assert(src->size == dst->size); >> PRINT_SURFACE_INFO("src", src); >> PRINT_SURFACE_INFO("mid", mid); >> @@ -477,13 +477,13 @@ static void block_multicopy(int i915, >> pext3 = NULL; >> >> src = blt_create_object(i915, region1, width, height, bpp, uc_mocs, >> - T_LINEAR, COMPRESSION_DISABLED, comp_type, true); >> + T_LINEAR, COMPRESSION_DISABLED, comp_type); >> mid = blt_create_object(i915, mid_region, width, height, bpp, uc_mocs, >> - mid_tiling, mid_compression, comp_type, true); >> + mid_tiling, mid_compression, comp_type); >> dst = blt_create_object(i915, region1, width, height, bpp, uc_mocs, >> - mid_tiling, COMPRESSION_DISABLED, comp_type, true); >> + mid_tiling, COMPRESSION_DISABLED, comp_type); >> final = blt_create_object(i915, region1, width, height, bpp, uc_mocs, >> - T_LINEAR, COMPRESSION_DISABLED, comp_type, true); >> + T_LINEAR, COMPRESSION_DISABLED, comp_type); >> igt_assert(src->size == dst->size); >> PRINT_SURFACE_INFO("src", src); >> PRINT_SURFACE_INFO("mid", mid); >> diff --git a/tests/i915/gem_exercise_blt.c b/tests/i915/gem_exercise_blt.c >> index 02c54f85..b1123356 100644 >> --- a/tests/i915/gem_exercise_blt.c >> +++ b/tests/i915/gem_exercise_blt.c >> @@ -136,11 +136,11 @@ static void fast_copy_emit(int i915, const intel_ctx_t *ctx, >> igt_assert(__gem_create_in_memory_regions(i915, &bb, &bb_size, region1) == 0); >> >> src = blt_create_object(i915, region1, width, height, bpp, 0, >> - T_LINEAR, COMPRESSION_DISABLED, 0, true); >> + T_LINEAR, COMPRESSION_DISABLED, 0); >> mid = blt_create_object(i915, region2, width, height, bpp, 0, >> - mid_tiling, COMPRESSION_DISABLED, 0, true); >> + mid_tiling, COMPRESSION_DISABLED, 0); >> dst = blt_create_object(i915, region1, width, height, bpp, 0, >> - T_LINEAR, COMPRESSION_DISABLED, 0, true); >> + T_LINEAR, COMPRESSION_DISABLED, 0); >> igt_assert(src->size == dst->size); >> >> PRINT_SURFACE_INFO("src", src); >> @@ -196,11 +196,11 @@ static void fast_copy(int i915, const intel_ctx_t *ctx, >> igt_assert(__gem_create_in_memory_regions(i915, &bb, &bb_size, region1) == 0); >> >> src = blt_create_object(i915, region1, width, height, bpp, 0, >> - T_LINEAR, COMPRESSION_DISABLED, 0, true); >> + T_LINEAR, COMPRESSION_DISABLED, 0); >> mid = blt_create_object(i915, region2, width, height, bpp, 0, >> - mid_tiling, COMPRESSION_DISABLED, 0, true); >> + mid_tiling, COMPRESSION_DISABLED, 0); >> dst = blt_create_object(i915, region1, width, height, bpp, 0, >> - T_LINEAR, COMPRESSION_DISABLED, 0, true); >> + T_LINEAR, COMPRESSION_DISABLED, 0); >> igt_assert(src->size == dst->size); >> >> blt_surface_fill_rect(i915, src, width, height); >> -- >> 2.25.1 >>