Re: [igt-dev] [PATCH i-g-t v4 2/3] lib/igt_fb.c: Update tile sizes for GEN12 CCS

From: Imre Deak <imre.deak@intel.com>
To: Mika Kahola <mika.kahola@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t v4 2/3] lib/igt_fb.c: Update tile sizes for GEN12 CCS
Date: Mon, 11 Nov 2019 17:51:33 +0200	[thread overview]
Message-ID: <20191111155133.GB19246@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20191111152232.GA19246@ideak-desk.fi.intel.com>

On Mon, Nov 11, 2019 at 05:22:32PM +0200, Imre Deak wrote:
> Hi,
> 
> On Mon, Nov 11, 2019 at 01:00:48PM +0200, Mika Kahola wrote:
> > Update tile sizes for GEN12 CCS.
> > 
> > BSpec: 47709
> > 
> > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  lib/igt_fb.c | 34 ++++++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 4adca967..77030d49 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -413,6 +413,33 @@ void igt_get_fb_tile_size(int fd, uint64_t modifier, int fb_bpp,
> >  			*height_ret = 32;
> >  		}
> >  		break;
> > +	case LOCAL_I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> > +		igt_require_intel(fd);
> > +		switch (fb_bpp) {
> > +		case 8:
> > +			*width_ret = 512;
> > +			*height_ret = 32;
> > +			break;
> > +		case 16:
> > +			*width_ret = 256;
> > +			*height_ret = 32;
> > +			break;
> > +		case 32:
> > +			*width_ret = 128;
> > +			*height_ret = 32;
> > +			break;
> > +		case 64:
> > +			*width_ret = 64;
> > +			*height_ret = 32;
> > +			break;
> > +		case 128:
> > +			*width_ret = 32;
> > +			*height_ret = 32;
> > +			break;
> > +		default:
> > +			igt_assert(false);
> > +		}
> > +		break;
> 
> The above look wrong to me, tiles are always 4kB. So the GEN12 y-tiled
> tile width, height (CCS or not) are the same as on other platforms.
> 
> OTOH, we should align both the main surface and AUX surface width and
> height for CCS. Those are setup in igt_init_fb()->fb_plane_height() and
> ->fb_plane_width(). The main surface (plane 0) should be aligned to 4x4
> tiles (128x128 pixels).

and 128x128 is the case for 32bpp. For all bpps:

bpp	width	height
8	512	128
16	256	128
32	128	128
64	64	128
128	32	128

> The AUX surface (plane 1) width/height should match this, so for that
> in fb_plane_width():
> 
> 	return DIV_ROUND_UP(fb->width, 128) * 64;

and here the divisor is based on the bpp as above.

> 
> and in fb_plane_height():
> 
> 	return DIV_ROUND_UP(fb->height, 128) * 4;
> 
> 
> >  	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> >  	case LOCAL_I915_FORMAT_MOD_Yf_TILED_CCS:
> >  		igt_require_intel(fd);
> > @@ -467,8 +494,10 @@ void igt_get_fb_tile_size(int fd, uint64_t modifier, int fb_bpp,
> >  
> >  static bool is_ccs_modifier(uint64_t modifier)
> >  {
> > -	return modifier == LOCAL_I915_FORMAT_MOD_Y_TILED_CCS ||
> > -		modifier == LOCAL_I915_FORMAT_MOD_Yf_TILED_CCS;
> > +
> > +	return modifier == I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS ||
> > +		modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > +		modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
> >  }
> >  
> >  static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
> > @@ -687,6 +716,7 @@ uint64_t igt_fb_mod_to_tiling(uint64_t modifier)
> >  		return I915_TILING_X;
> >  	case LOCAL_I915_FORMAT_MOD_Y_TILED:
> >  	case LOCAL_I915_FORMAT_MOD_Y_TILED_CCS:
> > +	case LOCAL_I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
> >  		return I915_TILING_Y;
> >  	case LOCAL_I915_FORMAT_MOD_Yf_TILED:
> >  	case LOCAL_I915_FORMAT_MOD_Yf_TILED_CCS:
> > -- 
> > 2.17.1
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev