[Intel-xe] [PATCH v2 12/17] drm/xe/rtp: Improve magic macros for RTP tables
Matt Roper
matthew.d.roper at intel.com
Mon Apr 24 19:21:28 UTC 2023
On Fri, Apr 21, 2023 at 03:32:53PM -0700, Lucas De Marchi wrote:
> The macros for writing the rtp tables have some downsides:
>
> 1) They are fragile, and when making a typo it's hard to know
> where it's failing for someone not used to them
> 2) They polute the global scope. The CONCAT/COUNT_ARGS/etc only
> work because they are defined exactly the same as the ones in
> kernel.h.
> 3) They are not compatible with rules/actions with registers
> passing additional options
>
> This new implementation fixes (2) and (3).
>
> For (2), prefix everything with "_XE", so there is less chance of
> clashes. Also, the maximum number of rules and actions is 4, so there is
> no need to make the helper macros go until 12: reduce that to 5, so if
> more arguments is passed, the compiler gives an error that there is not
> definition with _5 suffix.
>
> For (3), change the implementation to concentrate on "pasting a prefix
> to each argument" rather than the more general "call any macro for each
> argument". Hopefully this will avoid trying to extending this infra and
> making it more complex. The new changes to how the arguments are
> constructed using tuples will allow extending the interface where it's
> needed: passing additional register fields to allow creating our own
> xe_reg_t.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
Sill complicated, but looks correct to me.
Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> ---
> drivers/gpu/drm/xe/xe_rtp.h | 68 ++++++++++++++++++-------------------
> 1 file changed, 33 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h
> index a3be7c77753a..53650f09efe9 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.h
> +++ b/drivers/gpu/drm/xe/xe_rtp.h
> @@ -22,43 +22,42 @@ struct xe_reg_sr;
> /*
> * Helper macros - not to be used outside this header.
> */
> -/* This counts to 12. Any more, it will return 13th argument. */
> -#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> -#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> +#define _XE_ESC(...) __VA_ARGS__
> +#define _XE_COUNT_ARGS(...) _XE_ESC(__XE_COUNT_ARGS(__VA_ARGS__,5,4,3,2,1,))
> +#define __XE_COUNT_ARGS(_,_5,_4,_3,_2,X_,...) X_
>
> -#define __CONCAT(a, b) a ## b
> -#define CONCATENATE(a, b) __CONCAT(a, b)
> +#define _XE_CONCAT(a, b) __XE_CONCAT(a, b)
> +#define __XE_CONCAT(a, b) a ## b
>
> -#define __CALL_FOR_EACH_1(MACRO_, x, ...) MACRO_(x)
> -#define __CALL_FOR_EACH_2(MACRO_, x, ...) \
> - MACRO_(x) __CALL_FOR_EACH_1(MACRO_, ##__VA_ARGS__)
> -#define __CALL_FOR_EACH_3(MACRO_, x, ...) \
> - MACRO_(x) __CALL_FOR_EACH_2(MACRO_, ##__VA_ARGS__)
> -#define __CALL_FOR_EACH_4(MACRO_, x, ...) \
> - MACRO_(x) __CALL_FOR_EACH_3(MACRO_, ##__VA_ARGS__)
> +#define _XE_FIRST(...) _XE_ESC(__XE_FIRST(__VA_ARGS__,))
> +#define __XE_FIRST(x_,...) x_
> +#define _XE_TUPLE_TAIL(...) _XE_ESC(__XE_TUPLE_TAIL(__VA_ARGS__))
> +#define __XE_TUPLE_TAIL(x_,...) (__VA_ARGS__)
>
> -#define _CALL_FOR_EACH(NARGS_, MACRO_, x, ...) \
> - CONCATENATE(__CALL_FOR_EACH_, NARGS_)(MACRO_, x, ##__VA_ARGS__)
> -#define CALL_FOR_EACH(MACRO_, x, ...) \
> - _CALL_FOR_EACH(COUNT_ARGS(x, ##__VA_ARGS__), MACRO_, x, ##__VA_ARGS__)
> +#define __XE_PASTE_SEP_COMMA ,
> +#define __XE_PASTE_SEP_BITWISE_OR |
> +#define __XE_PASTE(prefix_, sep_, args_) _XE_ESC(_XE_CONCAT(__XE_PASTE_,_XE_COUNT_ARGS args_)(prefix_, sep_, args_))
> +#define __XE_PASTE_1(prefix_, sep_, args_) prefix_ args_
> +#define __XE_PASTE_2(prefix_, sep_, args_) prefix_(_XE_FIRST args_) __XE_PASTE_SEP_ ## sep_ __XE_PASTE_1(prefix_, sep_, _XE_TUPLE_TAIL args_)
> +#define __XE_PASTE_3(prefix_, sep_, args_) prefix_(_XE_FIRST args_) __XE_PASTE_SEP_ ## sep_ __XE_PASTE_2(prefix_, sep_, _XE_TUPLE_TAIL args_)
> +#define __XE_PASTE_4(prefix_, sep_, args_) prefix_(_XE_FIRST args_) __XE_PASTE_SEP_ ## sep_ __XE_PASTE_3(prefix_, sep_, _XE_TUPLE_TAIL args_)
>
> -#define _XE_RTP_REG(x_) (x_), XE_RTP_REG_REGULAR
> -#define _XE_RTP_MCR_REG(x_) (x_), XE_RTP_REG_MCR
> +#define _XE_RTP_REG(x_) (x_), XE_RTP_REG_REGULAR
> +#define _XE_RTP_MCR_REG(x_) (x_), XE_RTP_REG_MCR
>
> /*
> * Helper macros for concatenating prefix - do not use them directly outside
> * this header
> */
> -#define __ADD_XE_RTP_ENTRY_FLAG_PREFIX(x) CONCATENATE(XE_RTP_ENTRY_FLAG_, x) |
> -#define __ADD_XE_RTP_ACTION_FLAG_PREFIX(x) CONCATENATE(XE_RTP_ACTION_FLAG_, x) |
> -#define __ADD_XE_RTP_RULE_PREFIX(x) CONCATENATE(XE_RTP_RULE_, x) ,
> -#define __ADD_XE_RTP_ACTION_PREFIX(x) CONCATENATE(XE_RTP_ACTION_, x) ,
> +#define __XE_PASTE_XE_RTP_ENTRY_FLAG_(x_) _XE_CONCAT(XE_RTP_ENTRY_FLAG_, x_)
> +#define __XE_PASTE_XE_RTP_ACTION_FLAG_(x_) _XE_CONCAT(XE_RTP_ACTION_FLAG_, x_)
> +#define __XE_PASTE_XE_RTP_ACTION_(x_) _XE_CONCAT(XE_RTP_ACTION_, x_)
> +#define __XE_PASTE_XE_RTP_RULE_(x_) _XE_CONCAT(XE_RTP_RULE_, x_)
>
> /*
> * Macros to encode rules to match against platform, IP version, stepping, etc.
> * Shouldn't be used directly - see XE_RTP_RULES()
> */
> -
> #define _XE_RTP_RULE_PLATFORM(plat__) \
> { .match_type = XE_RTP_MATCH_PLATFORM, .platform = plat__ }
>
> @@ -315,8 +314,8 @@ struct xe_reg_sr;
> * ...
> * };
> */
> -#define XE_RTP_ENTRY_FLAG(f1_, ...) \
> - .flags = (CALL_FOR_EACH(__ADD_XE_RTP_ENTRY_FLAG_PREFIX, f1_, ##__VA_ARGS__) 0)
> +#define XE_RTP_ENTRY_FLAG(...) \
> + .flags = (__XE_PASTE(__XE_PASTE_XE_RTP_ENTRY_FLAG_, BITWISE_OR, (__VA_ARGS__)))
>
> /**
> * XE_RTP_ACTION_FLAG - Helper to add multiple flags to a struct xe_rtp_action
> @@ -338,8 +337,8 @@ struct xe_reg_sr;
> * ...
> * };
> */
> -#define XE_RTP_ACTION_FLAG(f1_, ...) \
> - .flags = (CALL_FOR_EACH(__ADD_XE_RTP_ACTION_FLAG_PREFIX, f1_, ##__VA_ARGS__) 0)
> +#define XE_RTP_ACTION_FLAG(...) \
> + .flags = (__XE_PASTE(__XE_PASTE_XE_RTP_ACTION_FLAG_, BITWISE_OR, (__VA_ARGS__)))
>
> /**
> * XE_RTP_RULES - Helper to set multiple rules to a struct xe_rtp_entry entry
> @@ -361,16 +360,15 @@ struct xe_reg_sr;
> * ...
> * };
> */
> -#define XE_RTP_RULES(r1, ...) \
> - .n_rules = COUNT_ARGS(r1, ##__VA_ARGS__), \
> +#define XE_RTP_RULES(...) \
> + .n_rules = _XE_COUNT_ARGS(__VA_ARGS__), \
> .rules = (const struct xe_rtp_rule[]) { \
> - CALL_FOR_EACH(__ADD_XE_RTP_RULE_PREFIX, r1, ##__VA_ARGS__) \
> + __XE_PASTE(__XE_PASTE_XE_RTP_RULE_, COMMA, (__VA_ARGS__)) \
> }
>
> /**
> * XE_RTP_ACTIONS - Helper to set multiple actions to a struct xe_rtp_entry
> - * @a1: Action to take. Last part of XE_RTP_ACTION_*
> - * @...: Additional rules, defined like @r1
> + * @...: Actions to be taken
> *
> * At least one rule is needed and up to 4 are supported. Multiple rules are
> * AND'ed together, i.e. all the rules must evaluate to true for the entry to
> @@ -388,10 +386,10 @@ struct xe_reg_sr;
> * ...
> * };
> */
> -#define XE_RTP_ACTIONS(a1, ...) \
> - .n_actions = COUNT_ARGS(a1, ##__VA_ARGS__), \
> +#define XE_RTP_ACTIONS(...) \
> + .n_actions = _XE_COUNT_ARGS(__VA_ARGS__), \
> .actions = (const struct xe_rtp_action[]) { \
> - CALL_FOR_EACH(__ADD_XE_RTP_ACTION_PREFIX, a1, ##__VA_ARGS__) \
> + __XE_PASTE(__XE_PASTE_XE_RTP_ACTION_, COMMA, (__VA_ARGS__)) \
> }
>
> void xe_rtp_process(const struct xe_rtp_entry *entries, struct xe_reg_sr *sr,
> --
> 2.39.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list