From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6FAFC10E6B1 for ; Thu, 24 Feb 2022 20:54:05 +0000 (UTC) Date: Thu, 24 Feb 2022 12:50:24 -0800 Message-ID: <87v8x4htv3.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Zbigniew =?ISO-8859-2?Q?Kempczy=F1ski?= In-Reply-To: <20220224075415.18663-2-zbigniew.kempczynski@intel.com> References: <20220224075415.18663-1-zbigniew.kempczynski@intel.com> <20220224075415.18663-2-zbigniew.kempczynski@intel.com> 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 1/2] lib/intel_allocator: Add safe alignment as a default 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: On Wed, 23 Feb 2022 23:54:14 -0800, Zbigniew Kempczy=F1ski wrote: > > diff --git a/lib/intel_allocator.c b/lib/intel_allocator.c > index eabff1f9a..340b88828 100644 > --- a/lib/intel_allocator.c > +++ b/lib/intel_allocator.c > @@ -283,7 +283,8 @@ static bool __allocator_put(struct allocator *al) > static struct intel_allocator *intel_allocator_create(int fd, > uint64_t start, uint64_t end, > uint8_t allocator_type, > - uint8_t allocator_strategy) > + uint8_t allocator_strategy, > + uint64_t default_alignment) > { > struct intel_allocator *ial =3D NULL; > > @@ -321,6 +322,7 @@ static struct intel_allocator *intel_allocator_create= (int fd, > > ial->type =3D allocator_type; > ial->strategy =3D allocator_strategy; > + ial->default_alignment =3D default_alignment; Isn't this a better place to set default_alignment to gem_detect_safe_alignment() rather than in __intel_allocator_open_full() below (though I understand they are roughly equivalent)? > @@ -903,6 +920,9 @@ static uint64_t __intel_allocator_open_full(int fd, u= int32_t ctx, > req.open.end =3D gtt_size; > } > > + if (!default_alignment) > + req.open.default_alignment =3D gem_detect_safe_alignment(fd); > + /snip/ > @@ -948,20 +970,24 @@ static uint64_t __intel_allocator_open_full(int fd,= uint32_t ctx, > uint64_t intel_allocator_open_full(int fd, uint32_t ctx, > uint64_t start, uint64_t end, > uint8_t allocator_type, > - enum allocator_strategy strategy) > + enum allocator_strategy strategy, > + uint64_t default_alignment) So we have added default_alignment to all these API's now and that does provide additional flexibility but I am wondering if the additional flexibility is really needed and will any IGT ever set default_alignment other than 0 (i.e. let allocator choose safe_alignment)? And also does having a different default_alignment improve test coverage in any way? In any case, this is: Reviewed-by: Ashutosh Dixit