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

[Modem, Bhanuprakash]

> From: Srinivas, Vidya <vidya.srinivas at intel.com>
> Sent: Monday, September 20, 2021 3:43 PM
> To: Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>; Modem,
> Bhanuprakash <bhanuprakash.modem at intel.com>; Joshi, Kunal1
> <kunal1.joshi at intel.com>
> Cc: igt-dev at lists.freedesktop.org; markyacoub at google.com
> Subject: RE: [PATCH i-g-t] lib/intel_bufops: Fix regression on 5.4 kernel
> 
> 
> 
> > -----Original Message-----
> > From: Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>
> > Sent: Monday, September 20, 2021 3:17 PM
> > To: Srinivas, Vidya <vidya.srinivas at intel.com>
> > Cc: igt-dev at lists.freedesktop.org; Modem, Bhanuprakash
> > <bhanuprakash.modem at intel.com>; markyacoub at 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&rev=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 at 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 at intel.com>

- Bhanu

> > > +	}
> >
> > Patchwork didn't get my previous rb, so for easier merge:
> >
> > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at 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
> > >