From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 22C9E6EC21 for ; Thu, 14 Oct 2021 07:42:10 +0000 (UTC) Date: Thu, 14 Oct 2021 00:22:54 -0700 Message-ID: <87wnmggje9.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20211014072529.GB3617@zkempczy-mobl2> References: <20211008065432.15482-1-zbigniew.kempczynski@intel.com> <20211008065432.15482-3-zbigniew.kempczynski@intel.com> <87a6jcib6l.wl-ashutosh.dixit@intel.com> <20211014072529.GB3617@zkempczy-mobl2> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-2 Content-Transfer-Encoding: quoted-printable Subject: Re: [igt-dev] [PATCH i-g-t 2/7] lib/intel_batchbuffer: Detect and use kernel alignment capability List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Zbigniew =?ISO-8859-2?Q?Kempczy=F1ski?= Cc: igt-dev@lists.freedesktop.org, Petri Latvala List-ID: On Thu, 14 Oct 2021 00:25:29 -0700, Zbigniew Kempczy=F1ski wrote: > > On Wed, Oct 13, 2021 at 07:37:22PM -0700, Dixit, Ashutosh wrote: > > On Thu, 07 Oct 2021 23:54:27 -0700, Zbigniew Kempczy=F1ski wrote: > > > > > > diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c > > > index 9c26fe207..ed7c25015 100644 > > > --- a/lib/intel_batchbuffer.c > > > +++ b/lib/intel_batchbuffer.c > > > @@ -1335,6 +1335,7 @@ __intel_bb_create(int i915, uint32_t ctx, uint3= 2_t size, bool do_relocs, > > > > > > igt_assert(ibb); > > > > > > + ibb->allows_passing_alignment =3D gem_allows_passing_alignment(i915= ); > > > ibb->uses_full_ppgtt =3D gem_uses_full_ppgtt(i915); > > > ibb->devid =3D intel_get_drm_devid(i915); > > > ibb->gen =3D intel_gen(ibb->devid); > > > @@ -1783,6 +1784,7 @@ __add_to_cache(struct intel_bb *ibb, uint32_t h= andle) > > > igt_assert(object); > > > > > > object->handle =3D handle; > > > + object->alignment =3D 0; > > > found =3D tsearch((void *) object, &ibb->root, __compare_objects); > > > > > > if (*found =3D=3D object) { > > > @@ -1905,7 +1907,7 @@ intel_bb_add_object(struct intel_bb *ibb, uint3= 2_t handle, uint64_t size, > > > || ALIGN(offset, alignment) =3D=3D offset); > > > > > > object =3D __add_to_cache(ibb, handle); > > > - object->alignment =3D alignment ?: 4096; > > > + alignment =3D alignment ?: 4096; > > > > Can't we do: > > if (ibb->allows_passing_alignment) > > object->alignment =3D alignment ?: 4096; > > So we don't need further changes in this function? Or it breaks somethi= ng? > > I think it will work at this place, object->alignment is touched twice: > 1. first on __add_to_cache() and set to 0 > 2. second on finalizing object settings after acquiring offset > > I added this on the bottom where object->... are filling with established > values. I just wanted to have all those settings in one place, not > object->alignment on the top and all the rest on the bottom. But there's > cosmetic and personal preference. Reviewed-by: Ashutosh Dixit