From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id 473386EAB7 for ; Mon, 4 Oct 2021 23:35:06 +0000 (UTC) Date: Mon, 04 Oct 2021 16:20:24 -0700 Message-ID: <87bl44z8av.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20211004054056.24346-3-zbigniew.kempczynski@intel.com> References: <20211004054056.24346-1-zbigniew.kempczynski@intel.com> <20211004054056.24346-3-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 2/2] lib/intel_bufops: Store gem bo size 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 Sun, 03 Oct 2021 22:40:56 -0700, Zbigniew Kempczy=F1ski wrote: > > intel_buf is keeping its size which may differ to underlying gem bo size. > Introduce keeping bo_size field which is used along with softpin mode > - like in intel_bb. > > Patch also should remove previous discrepancy where intel_buf_bo_size() > returned requested (not gem bo size). > > From now on user has an access to: > 1. raw buffer size - intel_buf_size() - function returns how buffer data > really takes in the memory Not sure what we mean by this since intel_buf_size() can return 0 even with a non-zero handle. See below. > 2. gem bo buffer size - intel_buf_bo_size() - function returns how big > underlying gem object is > diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c > index 7199723bb..80c5bb80b 100644 > --- a/lib/intel_bufops.c > +++ b/lib/intel_bufops.c > @@ -813,17 +813,16 @@ static void __intel_buf_init(struct buf_ops *bops, > size =3D bo_size; > } > > - /* Store real bo size to avoid mistakes in calculating it again */ > + /* Store buffer size to avoid mistakes in calculating it again */ > buf->size =3D size; > + buf->handle =3D handle; > > - if (handle) > - buf->handle =3D handle; > - else { > - if (!__gem_create_in_memory_regions(bops->fd, &handle, &size, region)) > - buf->handle =3D handle; > - else > - buf->handle =3D gem_create(bops->fd, size); > - } > + if (!handle) > + if (__gem_create_in_memory_regions(bops->fd, &buf->handle, &size, regi= on)) > + igt_assert_eq(__gem_create(bops->fd, &size, &buf->handle), 0); > + > + /* Store gem bo size */ > + buf->bo_size =3D size; The code after the above changes is like this: if (bo_size > 0) { igt_assert(bo_size >=3D size); size =3D bo_size; } /* Store buffer size to avoid mistakes in calculating it again */ buf->size =3D size; buf->handle =3D handle; if (!handle) if (__gem_create_in_memory_regions(bops->fd, &buf->handle, = &size, region)) igt_assert_eq(__gem_create(bops->fd, &size, &buf->h= andle), 0); /* Store gem bo size */ buf->bo_size =3D size; The function is called with: a. handle =3D=3D 0 or !=3D 0 b. bo_size =3D=3D 0 or !=3D 0 As seen in __intel_buf_init callers: *** lib/intel_bufops.c: __intel_buf_init[728] static void __intel_buf_init(struct buf_ops = *bops, intel_buf_init[851] __intel_buf_init(bops, 0, buf, width, height= , bpp, alignment, intel_buf_init_in_region[868] __intel_buf_init(bops, 0, buf, width, height= , bpp, alignment, intel_buf_init_using_handle[927] __intel_buf_init(bops, handle, buf, width,= height, bpp, alignment, intel_buf_create_using_handle_and_size[1013] __intel_buf_init(bops, handle,= buf, width, height, bpp, alignment, So when handle !=3D 0 and bo_size =3D=3D 0, we end with both buf->size =3D= =3D buf->bo_size =3D=3D 0. When handle =3D=3D 0 and bo_size =3D=3D 0, we end up with buf->size =3D=3D = 0 and buf->bo_size !=3D 0. So this is not a new issue, maybe it's ok, but I just wanted to check with you if you think all these scenarios work out ok even after introducing separate buf->size and buf->bo_size. Thanks.