[igt-dev] [PATCH i-g-t 2/3] lib/igt_fb: Fix stride for render_copy based FB conversions

Zbigniew Kempczyński zbigniew.kempczynski at intel.com
Fri Apr 30 05:53:02 UTC 2021


On Thu, Apr 29, 2021 at 10:51:15PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 29, 2021 at 10:26:27PM +0300, Imre Deak wrote:
> > When setting up an intel_buf for a render copy operation, the render
> > buffer stride has to match the framebuffer stride. Not ensuring this may
> > lead to a mismatch modeset error or an -EBUSY set_tiling IOCTL failure
> > when trying to change the stride for a framebuffer object.
> 
> The stride needs to always match, otherwise we're going to be
> rendering garbage. What used to be the case is that the igt_buf
> thing was just given the stride and size, and it just set up
> things so that the render surface covers the entire stride,
> whether or not the fb is smaller or not.
> 
> So I guess this is some kind of regression because we're now
> using the fb->width for something?

Yes, writing intel_buf I was wet behind the ears here. I assumed
calculating stride according to passed width was enough. As KMS tests
shows I was wrong and there're cases where stride can be greater than
aligned width.

IMO Imre's patch should solve the missing case, thus:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew

> 
> > 
> > Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  lib/igt_fb.c       |  3 ++-
> >  lib/intel_bufops.c | 25 +++++++++++++++++--------
> >  lib/intel_bufops.h |  3 ++-
> >  3 files changed, 21 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 5c1670410..209bfacf0 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -2257,7 +2257,8 @@ igt_fb_create_intel_buf(int fd, struct buf_ops *bops,
> >  						     fb->width, fb->height,
> >  						     fb->plane_bpp[0], 0,
> >  						     igt_fb_mod_to_tiling(fb->modifier),
> > -						     compression, fb->size);
> > +						     compression, fb->size,
> > +						     fb->strides[0]);
> >  	intel_buf_set_name(buf, name);
> >  
> >  	/* Make sure we close handle on destroy path */
> > diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> > index 3c1cca0cf..5dece5764 100644
> > --- a/lib/intel_bufops.c
> > +++ b/lib/intel_bufops.c
> > @@ -709,7 +709,7 @@ static void __intel_buf_init(struct buf_ops *bops,
> >  			     struct intel_buf *buf,
> >  			     int width, int height, int bpp, int alignment,
> >  			     uint32_t req_tiling, uint32_t compression,
> > -			     uint64_t bo_size)
> > +			     uint64_t bo_size, int bo_stride)
> >  {
> >  	uint32_t tiling = req_tiling;
> >  	uint64_t size;
> > @@ -741,7 +741,9 @@ static void __intel_buf_init(struct buf_ops *bops,
> >  		 * CCS units, that is 4 * 64 bytes. These 4 CCS units are in
> >  		 * turn mapped by one L1 AUX page table entry.
> >  		 */
> > -		if (bops->intel_gen >= 12)
> > +		if (bo_stride)
> > +			buf->surface[0].stride = bo_stride;
> > +		else if (bops->intel_gen >= 12)
> >  			buf->surface[0].stride = ALIGN(width * (bpp / 8), 128 * 4);
> >  		else
> >  			buf->surface[0].stride = ALIGN(width * (bpp / 8), 128);
> > @@ -765,10 +767,16 @@ static void __intel_buf_init(struct buf_ops *bops,
> >  		if (tiling) {
> >  			devid =  intel_get_drm_devid(bops->fd);
> >  			tile_width = get_stride(devid, tiling);
> > -			buf->surface[0].stride = ALIGN(width * (bpp / 8), tile_width);
> > +			if (bo_stride)
> > +				buf->surface[0].stride = bo_stride;
> > +			else
> > +				buf->surface[0].stride = ALIGN(width * (bpp / 8), tile_width);
> >  			align_h = tiling == I915_TILING_X ? 8 : 32;
> >  		} else {
> > -			buf->surface[0].stride = ALIGN(width * (bpp / 8), alignment ?: 1);
> > +			if (bo_stride)
> > +				buf->surface[0].stride = bo_stride;
> > +			else
> > +				buf->surface[0].stride = ALIGN(width * (bpp / 8), alignment ?: 1);
> >  		}
> >  
> >  		buf->surface[0].size = buf->surface[0].stride * height;
> > @@ -816,7 +824,7 @@ void intel_buf_init(struct buf_ops *bops,
> >  		    uint32_t tiling, uint32_t compression)
> >  {
> >  	__intel_buf_init(bops, 0, buf, width, height, bpp, alignment,
> > -			 tiling, compression, 0);
> > +			 tiling, compression, 0, 0);
> >  
> >  	intel_buf_set_ownership(buf, true);
> >  }
> > @@ -875,7 +883,7 @@ void intel_buf_init_using_handle(struct buf_ops *bops,
> >  				 uint32_t req_tiling, uint32_t compression)
> >  {
> >  	__intel_buf_init(bops, handle, buf, width, height, bpp, alignment,
> > -			 req_tiling, compression, 0);
> > +			 req_tiling, compression, 0, 0);
> >  }
> >  
> >  /**
> > @@ -950,7 +958,8 @@ struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
> >  							 int bpp, int alignment,
> >  							 uint32_t req_tiling,
> >  							 uint32_t compression,
> > -							 uint64_t size)
> > +							 uint64_t size,
> > +							 int stride)
> >  {
> >  	struct intel_buf *buf;
> >  
> > @@ -960,7 +969,7 @@ struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
> >  	igt_assert(buf);
> >  
> >  	__intel_buf_init(bops, handle, buf, width, height, bpp, alignment,
> > -			 req_tiling, compression, size);
> > +			 req_tiling, compression, size, stride);
> >  
> >  	return buf;
> >  }
> > diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
> > index 1a3d86925..12c2d5239 100644
> > --- a/lib/intel_bufops.h
> > +++ b/lib/intel_bufops.h
> > @@ -151,7 +151,8 @@ struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
> >  							 int bpp, int alignment,
> >  							 uint32_t req_tiling,
> >  							 uint32_t compression,
> > -							 uint64_t size);
> > +							 uint64_t size,
> > +							 int stride);
> >  void intel_buf_destroy(struct intel_buf *buf);
> >  
> >  void *intel_buf_cpu_map(struct intel_buf *buf, bool write);
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev


More information about the igt-dev mailing list