[PATCH i-g-t 4/7] lib/intel_bufops: Drop tilings restrictions

[Zbigniew Kempczyński]

On Fri, Apr 26, 2024 at 01:14:21PM +0300, Juha-Pekka Heikkila wrote:
> On 25.4.2024 18.52, Zbigniew Kempczyński wrote:
> > On Thu, Apr 25, 2024 at 04:08:23PM +0300, Juha-Pekka Heikkila wrote:
> > > On 25.4.2024 13.47, Zbigniew Kempczyński wrote:
> > > > Different platforms supports different tilings so instead of asserting
> > > > on buffer creation path move responsibility of passing valid data
> > > > to the test. It can use intel_cmds_info to iterate over supported
> > > > tilings so this is better extendible and requires fewer changes.
> > > > 
> > > > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > > > ---
> > > >    lib/intel_bufops.c | 3 ---
> > > >    1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> > > > index 007ccdb953..b96275e485 100644
> > > > --- a/lib/intel_bufops.c
> > > > +++ b/lib/intel_bufops.c
> > > > @@ -898,9 +898,6 @@ static void __intel_buf_init(struct buf_ops *bops,
> > > >    	if (compression) {
> > > >    		igt_require(bops->intel_gen >= 9);
> > > > -		igt_assert(req_tiling == I915_TILING_Y ||
> > > > -			   req_tiling == I915_TILING_Yf ||
> > > > -			   req_tiling == I915_TILING_4);
> > > 
> > > I'm wondering if it would make sense to flip this assert around instead of
> > > removing it? I mean would check here will not arrive linear and x-tile? I
> > > didn't check but I assume this compression flag will not be enabled for xe2?
> > 
> > I'm using this path to create compressed on xe2 and render-copy works
> > fine on linear/x/4/64 there.
> 
> But do you hit this assert with xe2? I was thinking you wouldn't hit that
> assert because compression is handled through pat index and from that would
> follow with xe2 you'd never have compression flag set. Then for dg2 you'd
> come here with also with tile64 so you could just check no x-tile and linear
> are coming here.
> 
> Then again, looking at what's inside that if(){..} maybe you could just
> change that "if (compression)" to something like "if
> (!HAS_FLATCCS(intel_get_drm_devid(bops->fd))" and remove the inner if doing
> the same thing?

Agree, it makes sense to join these if's keeping asserts in the code.
I'll send v3 with this change.

--
Zbigniew

> 
> /Juha-Pekka