Re: [igt-dev] [PATCH i-g-t] lib/intel_bufops: Fix regression on 5.4 kernel

["Srinivas, Vidya" <vidya.srinivas@intel.com>]

> -----Original Message-----
> From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> Sent: Monday, September 20, 2021 10:58 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>
> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Joshi, Kunal1
> <kunal1.joshi@intel.com>; igt-dev@lists.freedesktop.org;
> markyacoub@google.com
> Subject: Re: [PATCH i-g-t] lib/intel_bufops: Fix regression on 5.4 kernel
> 
> On Mon, Sep 20, 2021 at 12:32:03PM +0200, Modem, Bhanuprakash wrote:
> > > From: Srinivas, Vidya <vidya.srinivas@intel.com>
> > > Sent: Monday, September 20, 2021 3:43 PM
> > > To: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>; Modem,
> > > Bhanuprakash <bhanuprakash.modem@intel.com>; Joshi, Kunal1
> > > <kunal1.joshi@intel.com>
> > > Cc: igt-dev@lists.freedesktop.org; markyacoub@google.com
> > > Subject: RE: [PATCH i-g-t] lib/intel_bufops: Fix regression on 5.4
> > > kernel
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kempczynski, Zbigniew <zbigniew.kempczynski@intel.com>
> > > > Sent: Monday, September 20, 2021 3:17 PM
> > > > To: Srinivas, Vidya <vidya.srinivas@intel.com>
> > > > Cc: igt-dev@lists.freedesktop.org; Modem, Bhanuprakash
> > > > <bhanuprakash.modem@intel.com>; markyacoub@google.com
> > > > Subject: Re: [PATCH i-g-t] lib/intel_bufops: Fix regression on 5.4
> > > > kernel
> > > >
> > > > On Mon, Sep 20, 2021 at 02:54:55PM +0530, Vidya Srinivas wrote:
> > > > > Starting commit 8759c4a3020ce4 "Add intel_buf_init_in_region"
> > > > > __intel_buf_init uses gem_create_in_memory_regions instead of
> > > > > gem_create. Older kernels like 5.4 still dont have support for
> > > > > I915_GEM_CREATE_EXT_MEMORY_REGIONS
> (i915_gem_create_ext_ioctl)
> > > > from
> > > > > kernel commit
> > > > > (https://patchwork.freedesktop.org/patch/431581/?series=89648&re
> > > > > v=1) Due to this, the flip-vs-fences tests are failing on kernel
> > > > > 5.4 Patch adds fall back to gem_create when
> > > > > __gem_create_in_memory_region_list
> > > > fails.
> > > > >
> > > > > v2 - Addressed review comments from Zbigniew and Mark Added the
> > > > > fall back in __intel_buf_init
> > > > >
> > > > > v3 - Fixed space issue - review comment from Zbigniew
> > > > >
> > > > > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > > > > ---
> > > > >  lib/intel_bufops.c | 8 ++++++--
> > > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c index
> > > > > f5f67eddabd7..0236e4d919d5 100644
> > > > > --- a/lib/intel_bufops.c
> > > > > +++ b/lib/intel_bufops.c
> > > > > @@ -818,8 +818,12 @@ static void __intel_buf_init(struct buf_ops
> > > > > *bops,
> > > > >
> > > > >   if (handle)
> > > > >           buf->handle = handle;
> > > > > - else
> > > > > -         buf->handle = gem_create_in_memory_regions(bops->fd,
> > > > size, region);
> > > > > + else {
> > > > > +         if (__gem_create_in_memory_regions(bops->fd, &handle,
> > > > size, region) != 0)
> > > > > +                 buf->handle = gem_create(bops->fd, size);
> > > > > +         else
> > > > > +                 buf->handle = handle;
> >
> > There is nothing wrong with this code, but I personally feels to
> > update as below can be more user understandable (Just throw
> > legacy/incompatible stuff to else condition)
> >
> > if (!__gem_create_in_memory_regions(bops->fd, &handle, size, region))
> >         buf->handle = handle;
> > else
> >         buf->handle = gem_create(bops->fd, size);
> >
> > Acked-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> >
> > - Bhanu
> 
> Hmm, after looking at wider context we could even do
> 
> if (__gem_create_in_memory_regions(bops->fd, &buf->handle, size, region))
>          buf->handle = gem_create(bops->fd, size);
> 
> But this will likely obscure the intention.

Hello Zbigniew,
Thank you. Yes I had planned to do the way you have mentioned initially.
I will let you and Bhanu decide on the same.

Regards
Vidya


> 
> --
> Zbigniew
> 
> >
> > > > > + }
> > > >
> > > > Patchwork didn't get my previous rb, so for easier merge:
> > > >
> > > > Reviewed-by: Zbigniew Kempczyński
> <zbigniew.kempczynski@intel.com>
> > >
> > > Hello Zbigniew,
> > >
> > > Thank you so much. That is extremely kind of you.
> > >
> > > @Modem, Bhanuprakash, @Joshi, Kunal1 kindly help with merge if it
> passes CI.
> > >
> > > Regards
> > > Vidya
> > >
> > > >
> > > > --
> > > > Zbigniew
> > > >
> > > > >
> > > > >   set_hw_tiled(bops, buf);
> > > > >  }
> > > > > --
> > > > > 2.33.0
> > > > >