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

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

On 26.04.2024 11:01, 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>
> ---
>   lib/intel_cmds_info.c | 107 +++++++++++++++++-------------------------
>   1 file changed, 42 insertions(+), 65 deletions(-)
> 
> diff --git a/lib/intel_cmds_info.c b/lib/intel_cmds_info.c
> index 669d3e5006..2baac54c28 100644
> --- a/lib/intel_cmds_info.c
> +++ b/lib/intel_cmds_info.c
> @@ -20,75 +20,56 @@
>   		.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_4_64	(BIT(T_TILE4) | BIT(T_TILE64))
> +#define TILE_L		(BIT(T_LINEAR))
> +#define TILE_L_4_64	(BIT(T_LINEAR) | BIT(T_TILE4) | BIT(T_TILE64))
> +#define TILE_L_X	(BIT(T_LINEAR) | BIT(T_XMAJOR))
> +#define TILE_L_X_4	(BIT(T_LINEAR) | BIT(T_XMAJOR) | BIT(T_TILE4))
> +#define TILE_L_Y	(BIT(T_LINEAR) | BIT(T_YMAJOR))
> +#define TILE_L_X_Y	(BIT(T_LINEAR) | BIT(T_XMAJOR) | BIT(T_YMAJOR))
> +#define TILE_L_X_4_64	(BIT(T_LINEAR) | BIT(T_XMAJOR) | BIT(T_TILE4) | BIT(T_TILE64))
> +#define TILE_L_Y_4_64	(BIT(T_LINEAR) | BIT(T_YMAJOR) | BIT(T_TILE4) | BIT(T_TILE64))
> +#define TILE_L_Y_Yf_64	(BIT(T_LINEAR) | BIT(T_YMAJOR) | BIT(T_YFMAJOR) | BIT(T_TILE64))

Nit: I'd define simplest macros first and then follow with more complex
definitions (i.e., TILE_4, TILE_L, TILE_4_64...)

I'm not sure what to think about these defines. On one hand, they are
nice and tidy, on the other they can't be reused or coupled together.
Ideally, we'd have smaller pairings that come together and are used as
puzzles when defining render/blt info, but probably would take some time
to implement it nicely, so I don't push for it (...too much)

Also:

TILE_4_64 and TILE_L_X_4 are not used until patch 08/10, so I wouldn't
define them here, but later on.

All the best,
Karolina

> +
> +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 +83,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,