All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
To: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 1/2] lib/i915_blt: Remove create_mapping flag from blt_create_object
Date: Mon, 30 Jan 2023 20:30:03 +0100	[thread overview]
Message-ID: <20230130193003.e2gyeufe7e3hjznd@zkempczy-mobl2> (raw)
In-Reply-To: <60dfaf80-8a48-d887-8586-40f8250e3def@intel.com>

On Mon, Jan 30, 2023 at 09:10:10AM +0100, Karolina Stolarek wrote:
> 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 <zbigniew.kempczynski@intel.com>
> > > Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
> > > ---
> > >   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.

With this number one more doesn't extend it too much. But this additional
argument allows user to avoid create mapping during object creation.
I wouldn't assume client of blitter lib will always require mapping.

--
Zbigniew

> 
> 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
> > > 

  reply	other threads:[~2023-01-30 19:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-23 10:52 [igt-dev] [PATCH i-g-t 0/2] lib/i915_blt: Minor blt_create_object refactoring Karolina Stolarek
2023-01-23 10:52 ` [igt-dev] [PATCH i-g-t 1/2] lib/i915_blt: Remove create_mapping flag from blt_create_object Karolina Stolarek
2023-01-26 12:00   ` Zbigniew Kempczyński
2023-01-30  8:10     ` Karolina Stolarek
2023-01-30 19:30       ` Zbigniew Kempczyński [this message]
2023-01-23 10:52 ` [igt-dev] [PATCH i-g-t 2/2] lib/i915_blt: Split up blt_copy_object functions Karolina Stolarek
2023-01-26 12:13   ` Zbigniew Kempczyński
2023-01-30  8:21     ` Karolina Stolarek
2023-01-30 19:26       ` Zbigniew Kempczyński
2023-01-23 12:52 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/i915_blt: Minor blt_create_object refactoring Patchwork
2023-01-24  1:55 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230130193003.e2gyeufe7e3hjznd@zkempczy-mobl2 \
    --to=zbigniew.kempczynski@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=karolina.stolarek@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.