[igt-dev] [PATCH i-g-t 5/6] lib/i915: Add ADL-P stride restrictions for non-linear buffers

[Imre Deak]

On Mon, Jun 07, 2021 at 08:34:45PM -0700, Matt Roper wrote:
> On Mon, Jun 07, 2021 at 10:58:21AM +0300, Imre Deak wrote:
> > On Fri, Jun 04, 2021 at 11:51:51PM +0300, Souza, Jose wrote:
> > > Hum we can drop this change, Imre landed changes to make framebuffers
> > > strides power of two for adl_p, so for userspace it should be
> > > transparent.
> > 
> > Yes, it should be transparent for CCS framebuffers too, however that's a
> > TODO item. I suspect that kernel would need to recreate the CCS plane if
> > the main surface stride changes (since the layout of each CCS page would
> > change, so we can't just remap the CCS pages), hence left this part for
> > a follow-up.
> > 
> > So this patch is needed.
> 
> Are there any changes required, or can we consider this r-b?

No changes are needed:

Reviewed-by: Imre Deak <imre.deak at intel.com>

> 
> Thanks.
> 
> 
> Matt
> 
> > 
> > > On Fri, 2021-06-04 at 13:39 -0700, Matt Roper wrote:
> > > > From: José Roberto de Souza <jose.souza at intel.com>
> > > > 
> > > > ADL-P tiled framebuffer strides must be power-of-two aligned and has a
> > > > minimum of 8 tiles. For non-CCS framebuffers the driver supports FBs not
> > > > meeting this requirement by remapping the framebuffer and padding the
> > > > stride as required, but for CCS FBs userspace must ensure the alignment.
> > > > Adding remap support for CCS FBs to the driver is to be done as a
> > > > follow-up.
> > > > 
> > > > Cc: Juha-Pekka Heikkilä <juha-pekka.heikkila at intel.com>
> > > > Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > > Cc: Clinton Taylor <Clinton.A.Taylor at intel.com>
> > > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > > > Cc: Imre Deak <imre.deak at intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > > > ---
> > > >  lib/igt_fb.c | 26 +++++++++++++++++++++-----
> > > >  1 file changed, 21 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > index 585ede38..ce2aa6ce 100644
> > > > --- a/lib/igt_fb.c
> > > > +++ b/lib/igt_fb.c
> > > > @@ -729,8 +729,16 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
> > > >  		/* clear color always fixed to 64 bytes */
> > > >  		return 64;
> > > >  	} else if (is_gen12_ccs_plane(fb, plane)) {
> > > > -		/* A main surface using a CCS AUX surface must be 4x4 tiles aligned. */
> > > > -		return ALIGN(min_stride, 64);
> > > > +		/*
> > > > +		 * A main surface using a CCS AUX surface must be 4x4 tiles
> > > > +		 * aligned.  On ADL_P the minimum main surface stride is 8
> > > > +		 * tiles (2 * 64 byte on CCS surface) and it has to be POT
> > > > +		 * aligned.
> > > > +		 */
> > > > +		if (IS_ALDERLAKE_P(intel_get_drm_devid(fb->fd)))
> > > > +			return roundup_power_of_two(max(min_stride, 128u));
> > > > +		else
> > > > +			return ALIGN(min_stride, 64);
> > > >  	} else if (!fb->modifier && is_nouveau_device(fb->fd)) {
> > > >  		int align;
> > > >  
> > > > @@ -743,14 +751,22 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
> > > >  		return ALIGN(min_stride, align);
> > > >  	} else {
> > > >  		unsigned int tile_width, tile_height;
> > > > +		uint32_t stride;
> > > >  
> > > >  		igt_get_fb_tile_size(fb->fd, fb->modifier, fb->plane_bpp[plane],
> > > >  				     &tile_width, &tile_height);
> > > >  
> > > > -		if (is_gen12_ccs_modifier(fb->modifier))
> > > > -			tile_width *= 4;
> > > > +		if (is_gen12_ccs_modifier(fb->modifier)) {
> > > > +			stride = ALIGN(min_stride, tile_width * 4);
> > > >  
> > > > -		return ALIGN(min_stride, tile_width);
> > > > +			/* TODO: add support to kernel to POT align CCS format strides */
> > > > +			if (IS_ALDERLAKE_P(intel_get_drm_devid(fb->fd)))
> > > > +				stride = roundup_power_of_two(max(stride, tile_width * 8));
> > > > +		} else {
> > > > +			stride = ALIGN(min_stride, tile_width);
> > > > +		}
> > > > +
> > > > +		return stride;
> > > >  	}
> > > >  }
> > > >  
> > > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795