[PATCH 2/5] drm/xe/rtp: Allow to OR rules
Matt Roper
matthew.d.roper at intel.com
Mon Jun 17 19:09:26 UTC 2024
On Mon, Jun 17, 2024 at 11:25:15AM -0700, Lucas De Marchi wrote:
> Some workarounds started to depend in different set of conditions where
> the action should be applied if any of them match. See e.g.
> commit 24d0d98af1c3 ("drm/xe/xe2lpm: Fixup Wa_14020756599"). Add
> XE_RTP_MATCH_OR that allows to implement a logical OR for the rules.
> Normal precedence applies:
>
> r1, r2, OR, r3
>
> means
>
> (r1 AND r2) OR r3
>
> The check is shortcut as soon as a set of conditions match.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
> drivers/gpu/drm/xe/tests/xe_rtp_test.c | 54 ++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_rtp.c | 20 +++++++++-
> drivers/gpu/drm/xe/xe_rtp.h | 21 ++++++++++
> drivers/gpu/drm/xe/xe_rtp_types.h | 1 +
> 4 files changed, 94 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> index 474a0b222ce1..bf7fee9b20cc 100644
> --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
> @@ -90,6 +90,60 @@ static const struct rtp_test_case cases[] = {
> {}
> },
> },
> + {
> + .name = "match-or-first",
> + .expected_reg = REGULAR_REG1,
> + .expected_set_bits = REG_BIT(0),
> + .expected_clr_bits = REG_BIT(0),
> + .expected_count = 1,
> + .entries = (const struct xe_rtp_entry_sr[]) {
> + { XE_RTP_NAME("first"),
> + XE_RTP_RULES(FUNC(match_yes), OR, FUNC(match_no)),
> + XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0)))
> + },
> + {}
> + },
> + },
> + {
> + .name = "match-or-last",
> + .expected_reg = REGULAR_REG1,
> + .expected_set_bits = REG_BIT(0),
> + .expected_clr_bits = REG_BIT(0),
> + .expected_count = 1,
> + .entries = (const struct xe_rtp_entry_sr[]) {
> + { XE_RTP_NAME("last"),
> + XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_yes)),
> + XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0)))
> + },
> + {}
> + },
> + },
> + {
> + .name = "match-or-empty",
> + .expected_reg = REGULAR_REG1,
> + .expected_set_bits = REG_BIT(0),
> + .expected_clr_bits = REG_BIT(0),
> + .expected_count = 1,
> + .entries = (const struct xe_rtp_entry_sr[]) {
> + { XE_RTP_NAME("empty-matches-ok"),
> + XE_RTP_RULES(OR, FUNC(match_no)),
> + XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0)))
> + },
> + {}
> + },
> + },
Is this a case we actually want to support? See note farther down.
> + {
> + .name = "match-or-none",
> + .expected_reg = REGULAR_REG1,
> + .expected_count = 0,
> + .entries = (const struct xe_rtp_entry_sr[]) {
> + { XE_RTP_NAME("none"),
> + XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_no)),
> + XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0)))
> + },
> + {}
> + },
> + },
> {
> .name = "no-match-no-add-multiple-rules",
> .expected_reg = REGULAR_REG1,
> diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
> index eff1c9c2f5cc..66ca58fa5739 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.c
> +++ b/drivers/gpu/drm/xe/xe_rtp.c
> @@ -40,6 +40,13 @@ static bool rule_matches(const struct xe_device *xe,
>
> for (r = rules, i = 0; i < n_rules; r = &rules[++i]) {
> switch (r->match_type) {
> + case XE_RTP_MATCH_OR:
> + /*
> + * This is only reached if a complete set of required
> + * rules passed, so return success: the remaining rules
> + * do not matter
> + */
> + return true;
Would it be worth adding an xe_assert here to make sure OR isn't the
first/last rule in the list? I can see someone deleting a workaround
condition that's no longer relevant and accidentally leaving a dangling
OR, leading to an unintentional "always pass" condition.
Up to you whether this is something we want to deal with or not. Either
way,
Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> case XE_RTP_MATCH_PLATFORM:
> match = xe->info.platform == r->platform;
> break;
> @@ -102,8 +109,17 @@ static bool rule_matches(const struct xe_device *xe,
> match = false;
> }
>
> - if (!match)
> - return false;
> + if (!match) {
> + /*
> + * Advance rules until we find XE_RTP_MATCH_OR to check
> + * if there's another set of conditions to check
> + */
> + while (i < n_rules && rules[++i].match_type != XE_RTP_MATCH_OR)
> + ;
> +
> + if (i >= n_rules)
> + return false;
> + }
> }
>
> return true;
> diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h
> index 337b1ef1959c..5a6b112a8be6 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.h
> +++ b/drivers/gpu/drm/xe/xe_rtp.h
> @@ -179,6 +179,27 @@ struct xe_reg_sr;
> #define XE_RTP_RULE_IS_DISCRETE \
> { .match_type = XE_RTP_MATCH_DISCRETE }
>
> +/**
> + * XE_RTP_RULE_OR - Create an OR condition for rtp rules
> + *
> + * RTP rules are AND'ed when evaluated and all of them need to match.
> + * XE_RTP_RULE_OR allows to create set of rules where any of them matching is
> + * sufficient for the action to trigger. Example:
> + *
> + * .. code-block:: c
> + *
> + * const struct xe_rtp_entry_sr entries[] = {
> + * ...
> + * { XE_RTP_NAME("test-entry"),
> + * XE_RTP_RULES(PLATFORM(DG2), OR, PLATFORM(TIGERLAKE)),
> + * ...
> + * },
> + * ...
> + * };
> + */
> +#define XE_RTP_RULE_OR \
> + { .match_type = XE_RTP_MATCH_OR }
> +
> /**
> * XE_RTP_ACTION_WR - Helper to write a value to the register, overriding all
> * the bits
> diff --git a/drivers/gpu/drm/xe/xe_rtp_types.h b/drivers/gpu/drm/xe/xe_rtp_types.h
> index 637acc7626a4..10150bc22ccd 100644
> --- a/drivers/gpu/drm/xe/xe_rtp_types.h
> +++ b/drivers/gpu/drm/xe/xe_rtp_types.h
> @@ -51,6 +51,7 @@ enum {
> XE_RTP_MATCH_ENGINE_CLASS,
> XE_RTP_MATCH_NOT_ENGINE_CLASS,
> XE_RTP_MATCH_FUNC,
> + XE_RTP_MATCH_OR,
> };
>
> /** struct xe_rtp_rule - match rule for processing entry */
> --
> 2.43.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list