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

[Ville Syrjälä]

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?

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