Re: [PATCH i-g-t v3 07/11] lib/intel_cmds_info: Define tiling macros

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

Hi Zbigniew,

On 7.5.2024 10.58, Zbigniew Kempczyński wrote:
> Blitter tilings don't always matches supported render tilings so
> it is necessary to add separate fields for this purpose. To avoid
> multiple lines where supported tiling is glued with BIT(tiling)
> it is worth to predefine them, especially they will be used in next
> patch related to supported render copy tilings.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> ---
> v3: Predefine single tiling first, then complex (Karolina)
> ---
>   lib/intel_cmds_info.c | 110 +++++++++++++++++-------------------------
>   1 file changed, 45 insertions(+), 65 deletions(-)
> 
> diff --git a/lib/intel_cmds_info.c b/lib/intel_cmds_info.c
> index 669d3e5006..e7aabf6bfb 100644
> --- a/lib/intel_cmds_info.c
> +++ b/lib/intel_cmds_info.c
> @@ -20,75 +20,59 @@
>   		.flags = _flags, \
>   	}
>   
> -static const struct blt_cmd_info src_copy = BLT_INFO(SRC_COPY, BIT(T_LINEAR));
> -static const struct blt_cmd_info
> -		pre_gen6_xy_src_copy = BLT_INFO(XY_SRC_COPY,
> -						BIT(T_LINEAR) |
> -						BIT(T_XMAJOR));
> -static const struct blt_cmd_info
> -		gen6_xy_src_copy = BLT_INFO(XY_SRC_COPY,
> -					    BIT(T_LINEAR) |
> -					    BIT(T_XMAJOR) |
> -					    BIT(T_YMAJOR));
> -static const struct blt_cmd_info
> -		gen11_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> -					      BIT(T_LINEAR)  |
> -					      BIT(T_YMAJOR)  |
> -					      BIT(T_YFMAJOR) |
> -					      BIT(T_TILE64));
> -static const struct blt_cmd_info
> -		gen12_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> -					      BIT(T_LINEAR) |
> -					      BIT(T_YMAJOR) |
> -					      BIT(T_TILE4)  |
> -					      BIT(T_TILE64));
> -static const struct blt_cmd_info
> -		dg2_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> -					    BIT(T_LINEAR) |
> -					    BIT(T_XMAJOR) |
> -					    BIT(T_TILE4)  |
> -					    BIT(T_TILE64));
> -static const struct blt_cmd_info
> -		pvc_xy_fast_copy = BLT_INFO(XY_FAST_COPY,
> -					    BIT(T_LINEAR) |
> -					    BIT(T_TILE4)  |
> -					    BIT(T_TILE64));
> -
> -static const struct blt_cmd_info
> -		gen12_xy_block_copy = BLT_INFO(XY_BLOCK_COPY,
> -					       BIT(T_LINEAR) |
> -					       BIT(T_YMAJOR));
> -static const struct blt_cmd_info
> -		dg2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_XMAJOR) |
> -						 BIT(T_TILE4)  |
> -						 BIT(T_TILE64),
> +#define TILE_4		BIT(T_TILE4)
> +#define TILE_64		BIT(T_TILE64)
> +#define TILE_L		BIT(T_LINEAR)
> +#define TILE_X		BIT(T_XMAJOR)
> +#define TILE_Y		BIT(T_YMAJOR)
> +#define TILE_Yf		BIT(T_YFMAJOR)
> +
> +#define TILE_L_4_64	(TILE_L | TILE_4 | TILE_64)
> +#define TILE_L_X	(TILE_L | TILE_X)
> +#define TILE_L_X_Y	(TILE_L | TILE_X | TILE_Y)
> +#define TILE_L_X_4_64	(TILE_L | TILE_X | TILE_4 | TILE_64)
> +#define TILE_L_Y	(TILE_L | TILE_Y)
> +#define TILE_L_Y_4_64	(TILE_L | TILE_Y | TILE_4 | TILE_64)

I was wondering if this is intentional or a bug? I see this was already 
so in previous implementation so I guess it work.. but on above line is 
set bits for linear, y, 4 and 64tile. I think those y and 4 will never 
exist in same device? Should here 4 be x instead?

I see this is used for gen12_xy_fast_copy, could this play part in those 
failures I had with blitter on adlp w/ x-tile?

> +#define TILE_L_Y_Yf_64	(TILE_L | TILE_Y | TILE_Yf | TILE_64)
> +
> +static const struct blt_cmd_info src_copy = BLT_INFO(SRC_COPY, TILE_L);
> +static const struct blt_cmd_info
> +		pre_gen6_xy_src_copy = BLT_INFO(XY_SRC_COPY, TILE_L_X);
> +
> +static const struct blt_cmd_info
> +		gen6_xy_src_copy = BLT_INFO(XY_SRC_COPY, TILE_L_X_Y);
> +
> +static const struct blt_cmd_info
> +		gen11_xy_fast_copy = BLT_INFO(XY_FAST_COPY, TILE_L_Y_Yf_64);
> +
> +static const struct blt_cmd_info
> +		gen12_xy_fast_copy = BLT_INFO(XY_FAST_COPY, TILE_L_Y_4_64);
> +
> +static const struct blt_cmd_info
> +		dg2_xy_fast_copy = BLT_INFO(XY_FAST_COPY, TILE_L_X_4_64);
> +
> +static const struct blt_cmd_info
> +		pvc_xy_fast_copy = BLT_INFO(XY_FAST_COPY, TILE_L_4_64);
> +
> +static const struct blt_cmd_info
> +		gen12_xy_block_copy = BLT_INFO(XY_BLOCK_COPY, TILE_L_Y);
> +
> +static const struct blt_cmd_info
> +		dg2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY, TILE_L_X_4_64,
>   						 BLT_CMD_EXTENDED |
>   						 BLT_CMD_SUPPORTS_COMPRESSION);
>   
>   static const struct blt_cmd_info
> -		xe2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_XMAJOR) |
> -						 BIT(T_TILE4)  |
> -						 BIT(T_TILE64),
> +		xe2_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY, TILE_L_X_4_64,
>   						 BLT_CMD_EXTENDED |
>   						 BLT_CMD_SUPPORTS_COMPRESSION);
>   
>   static const struct blt_cmd_info
> -		mtl_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_XMAJOR) |
> -						 BIT(T_TILE4)  |
> -						 BIT(T_TILE64),
> +		mtl_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY, TILE_L_X_4_64,
>   						 BLT_CMD_EXTENDED);
>   
>   static const struct blt_cmd_info
> -		pvc_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_TILE4)  |
> -						 BIT(T_TILE64),
> +		pvc_xy_block_copy = BLT_INFO_EXT(XY_BLOCK_COPY, TILE_L_4_64,
>   						 BLT_CMD_EXTENDED);
>   
>   static const struct blt_cmd_info
> @@ -102,17 +86,13 @@ static const struct blt_cmd_info
>   				       BIT(M_MATRIX));
>   
>   static const struct blt_cmd_info
> -		pre_gen6_xy_color_blt = BLT_INFO(XY_COLOR_BLT,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_XMAJOR));
> +		pre_gen6_xy_color_blt = BLT_INFO(XY_COLOR_BLT, TILE_L_X);
>   
>   static const struct blt_cmd_info
> -		gen6_xy_color_blt = BLT_INFO_EXT(XY_COLOR_BLT,
> -						 BIT(T_LINEAR) |
> -						 BIT(T_YMAJOR) |
> -						 BIT(T_XMAJOR),
> +		gen6_xy_color_blt = BLT_INFO_EXT(XY_COLOR_BLT, TILE_L_X_Y,
>   						 BLT_CMD_EXTENDED);
>   
> +
>   const struct intel_cmds_info pre_gen6_cmds_info = {
>   	.blt_cmds = {
>   		[SRC_COPY] = &src_copy,