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

["Kahola, Mika" <mika.kahola@intel.com>]

On Mon, 2019-11-11 at 17:51 +0200, Imre Deak wrote:
> 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
Ok, the numbers that I wrote originally above was found from BSpec.
Since the tiles are always 4kB, I could update the patch with the
numbers you provided.

> 
> > 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