[PATCH i-g-t v2 07/10] lib/intel_cmds_info: Define tiling macros
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Tue May 7 05:46:22 UTC 2024
On Fri, Apr 26, 2024 at 03:14:07PM +0200, Karolina Stolarek wrote:
> 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 at 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)
I think above tiling definitions make code more readable and easier to
maintain, especially in render part where tilings for uncompressed and
compressed buffers varies.
If your intention is to have L/X/Y/Yf/4/64/others... predefined first
then glued with bitwise or operator, I agree this will look better than
all lines with BIT(tiling) glued together.
>
> 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.
Right, I was lazy and packed everything to one bag here. I'll split this
according to usage.
Thank you for the review.
--
Zbigniew
>
> 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,
More information about the igt-dev
mailing list