Re: [PATCH i-g-t v4 2/5] lib/intel_blt: Change surface size calculation

[Karolina Stolarek <karolina.stolarek@intel.com>]

On 1.02.2024 11:07, Zbigniew Kempczyński wrote:
> For tiled surfaces we need to ensure stride and height are valid
> from the blit operation perspective. Calculate required surface
> size according to tiling constraints.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Karolina Drobnik <karolina.drobnik@intel.com>

I'm looking at blt_set_geom() function. We still pass normal height and
width to it. Is this correct? Or should we change height to
aligned_height? Because if we should, this patch should be updated to
reflect that.

If that's not a problem, then:

Reviewed-by: Karolina Stolarek <karolina.stolarek@intel.com>

> ---
>   lib/intel_blt.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/intel_blt.c b/lib/intel_blt.c
> index 13b1dbba4f..00208fc243 100644
> --- a/lib/intel_blt.c
> +++ b/lib/intel_blt.c
> @@ -1858,13 +1858,21 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>   		  bool create_mapping)
>   {
>   	struct blt_copy_object *obj;
> -	uint64_t size = width * height * bpp / 8;
> -	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
> +	uint32_t stride, aligned_height;
> +	uint64_t size;
>   	uint32_t handle;
>   	uint8_t pat_index = DEFAULT_PAT_INDEX;
>   
>   	igt_assert_f(blt->driver, "Driver isn't set, have you called blt_copy_init()?\n");
>   
> +	stride = blt_get_min_stride(width, bpp, tiling);
> +	aligned_height = blt_get_aligned_height(height, bpp, tiling);
> +	size = stride * aligned_height;
> +
> +	/* blitter command expects stride in dwords on tiled surfaces */
> +	if (tiling)
> +		stride /= 4;
> +
>   	obj = calloc(1, sizeof(*obj));
>   
>   	obj->size = size;