From 833e3f7e322e5c5c9ea219823e421e5d0c4b7fcd Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:16 -0300 Subject: [PATCH 01/17] drm/xe/rtp: Write kunit test cases specific for rule matching The kunit test cases for the RTP framework are currently separated into those that validate xe_rtp_process_to_sr() and those that validate xe_rtp_process(). In both of them, we also have mixed stuff to validate rule matching functionality, which should rather be done in a separate test case group. Let's create such a group, specific for validating rule matching, and also add an initial set of cases. In an upcoming change, we will do a cleanup of the other groups by migrating those cases intended for rule matching to this new group. v2: - s/no-yes-or-no-yes/no-yes-or-yes-no/ (Matt) - Drop leftover include of . Reviewed-by: Matt Roper Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-1-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp.c | 38 +++++++ drivers/gpu/drm/xe/tests/xe_rtp_test.c | 141 +++++++++++++++++++++++++ drivers/gpu/drm/xe/tests/xe_rtp_test.h | 23 ++++ drivers/gpu/drm/xe/xe_rtp.c | 57 +++++++--- 4 files changed, 242 insertions(+), 17 deletions(-) create mode 100644 drivers/gpu/drm/xe/tests/xe_rtp.c create mode 100644 drivers/gpu/drm/xe/tests/xe_rtp_test.h diff --git a/drivers/gpu/drm/xe/tests/xe_rtp.c b/drivers/gpu/drm/xe/tests/xe_rtp.c new file mode 100644 index 000000000000..b3a8b75936d1 --- /dev/null +++ b/drivers/gpu/drm/xe/tests/xe_rtp.c @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0 AND MIT +/* + * Copyright © 2026 Intel Corporation + */ + +#include "tests/xe_rtp_test.h" + +#include + +/** + * xe_rtp_rule_matches - Check if a set of RTP rule set match against the + * device/GT/hwe + * @xe: The xe device + * @gt: The GT struct (may be NULL) + * @hwe: The hw_engine (may be NULL) + * @rules: The array of rules to match against + * @n_rules: Number of items in @rules + * @err: Pointer (may be NULL) to set error number. + * + * This parses the set of rules and check if they match against the passed + * parameters. + * + * If passed, @err is updated with a non-zero negative error number or zero if + * no errors were found during the parsing/evaluation of rules. + * + * Returns true if there is a match and false if there is no match or if an + * error was found. + */ +bool xe_rtp_rule_matches(const struct xe_device *xe, + struct xe_gt *gt, + struct xe_hw_engine *hwe, + const struct xe_rtp_rule *rules, + unsigned int n_rules, + int *err) +{ + return rule_matches_with_err(xe, gt, hwe, rules, n_rules, err); +} +EXPORT_SYMBOL_IF_KUNIT(xe_rtp_rule_matches); diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index 5d78f2283df9..c98d85a15652 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -21,6 +21,7 @@ #include "xe_pci_test.h" #include "xe_reg_sr.h" #include "xe_rtp.h" +#include "xe_rtp_test.h" #define REGULAR_REG1 XE_REG(1) #define REGULAR_REG2 XE_REG(2) @@ -37,6 +38,14 @@ #undef XE_REG_MCR #define XE_REG_MCR(...) XE_REG(__VA_ARGS__, .mcr = 1) +struct rtp_rules_test_case { + const char *name; + bool expected_match; + int expected_err; + const struct xe_rtp_rule *rules; + u8 n_rules; +}; + struct rtp_to_sr_test_case { const char *name; struct xe_reg expected_reg; @@ -83,6 +92,130 @@ static bool match_no(const struct xe_device *xe, const struct xe_gt *gt, return false; } +static const struct rtp_rules_test_case rtp_rules_cases[] = { + /* + * Single rules. + * + * TODO: Include other types of rules as well: GRAPHICS_VERSION(), + * MEDIA_VERSION(), etc. + */ + { + .name = "no", + .expected_match = false, + XE_RTP_RULES(FUNC(match_no)), + }, + { + .name = "yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes)), + }, + + /* Conjunctions with 2 operands. */ + { + .name = "no-and-no", + .expected_match = false, + XE_RTP_RULES(FUNC(match_no), FUNC(match_no)), + }, + { + .name = "no-and-yes", + .expected_match = false, + XE_RTP_RULES(FUNC(match_no), FUNC(match_yes)), + }, + { + .name = "yes-and-no", + .expected_match = false, + XE_RTP_RULES(FUNC(match_yes), FUNC(match_no)), + }, + { + .name = "yes-and-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes), FUNC(match_yes)), + }, + + /* Disjunctions with 2 operands. */ + { + .name = "no-or-no", + .expected_match = false, + XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_no)), + }, + { + .name = "no-or-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_yes)), + }, + { + .name = "yes-or-no", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes), OR, FUNC(match_no)), + }, + { + .name = "yes-or-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes), OR, FUNC(match_yes)), + }, + + /* Conjunction and disjunctions. */ + { + .name = "no-yes-or-yes-no", + .expected_match = false, + XE_RTP_RULES(FUNC(match_no), FUNC(match_yes), OR, + FUNC(match_yes), FUNC(match_no)), + }, + { + .name = "no-yes-or-yes-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_no), FUNC(match_yes), OR, + FUNC(match_yes), FUNC(match_yes)), + }, + { + .name = "yes-yes-or-no-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes), FUNC(match_yes), OR, + FUNC(match_no), FUNC(match_yes)), + }, + { + .name = "yes-yes-or-yes-yes", + .expected_match = true, + XE_RTP_RULES(FUNC(match_yes), FUNC(match_yes), OR, + FUNC(match_yes), FUNC(match_yes)), + }, + { + .name = "no-no-or-yes-or-no", + .expected_match = true, + XE_RTP_RULES(FUNC(match_no), FUNC(match_no), OR, + FUNC(match_yes), OR, + FUNC(match_no)), + }, + + /* Syntax errors. */ + { + .name = "or", + .expected_match = false, + .expected_err = -EINVAL, + XE_RTP_RULES(OR), + }, + { + .name = "or-anything", + .expected_match = false, + .expected_err = -EINVAL, + XE_RTP_RULES(OR, FUNC(match_yes)), + }, +}; + +static void xe_rtp_rules_tests(struct kunit *test) +{ + const struct rtp_rules_test_case *param = test->param_value; + struct xe_device *xe = test->priv; + struct xe_gt *gt = xe_device_get_root_tile(xe)->primary_gt; + int err; + bool match; + + match = xe_rtp_rule_matches(xe, gt, NULL, param->rules, param->n_rules, &err); + + KUNIT_EXPECT_EQ(test, match, param->expected_match); + KUNIT_EXPECT_EQ(test, err, param->expected_err); +} + static const struct rtp_to_sr_test_case rtp_to_sr_cases[] = { { .name = "coalesce-same-reg", @@ -544,6 +677,13 @@ static void xe_rtp_process_tests(struct kunit *test) KUNIT_EXPECT_EQ(test, active, param->expected_active); } +static void rtp_rules_desc(const struct rtp_rules_test_case *t, char *desc) +{ + strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE); +} + +KUNIT_ARRAY_PARAM(rtp_rules, rtp_rules_cases, rtp_rules_desc); + static void rtp_to_sr_desc(const struct rtp_to_sr_test_case *t, char *desc) { strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE); @@ -591,6 +731,7 @@ static void xe_rtp_test_exit(struct kunit *test) } static struct kunit_case xe_rtp_tests[] = { + KUNIT_CASE_PARAM(xe_rtp_rules_tests, rtp_rules_gen_params), KUNIT_CASE_PARAM(xe_rtp_process_to_sr_tests, rtp_to_sr_gen_params), KUNIT_CASE_PARAM(xe_rtp_process_tests, rtp_gen_params), {} diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.h b/drivers/gpu/drm/xe/tests/xe_rtp_test.h new file mode 100644 index 000000000000..26becc1e2af0 --- /dev/null +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 AND MIT */ +/* + * Copyright © 2026 Intel Corporation + */ + +#ifndef _XE_RTP_TEST_H_ +#define _XE_RTP_TEST_H_ + +#include + +struct xe_device; +struct xe_gt; +struct xe_hw_engine; +struct xe_rtp_rule; + +bool xe_rtp_rule_matches(const struct xe_device *xe, + struct xe_gt *gt, + struct xe_hw_engine *hwe, + const struct xe_rtp_rule *rules, + unsigned int n_rules, + int *err); + +#endif diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index 1a4dcbbbc176..0e1adf07e4e7 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -30,16 +30,20 @@ static bool has_samedia(const struct xe_device *xe) return xe->info.media_verx100 >= 1300; } -static bool rule_matches(const struct xe_device *xe, - struct xe_gt *gt, - struct xe_hw_engine *hwe, - const struct xe_rtp_rule *rules, - unsigned int n_rules) +static bool rule_matches_with_err(const struct xe_device *xe, + struct xe_gt *gt, + struct xe_hw_engine *hwe, + const struct xe_rtp_rule *rules, + unsigned int n_rules, + int *err) { const struct xe_rtp_rule *r; unsigned int i, rcount = 0; bool match; + if (err) + *err = 0; + for (r = rules, i = 0; i < n_rules; r = &rules[++i]) { switch (r->match_type) { case XE_RTP_MATCH_OR: @@ -58,21 +62,21 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_PLATFORM_STEP: if (drm_WARN_ON(&xe->drm, xe->info.step.platform == STEP_NONE)) - return false; + goto error; match = xe->info.step.platform >= r->step_start && xe->info.step.platform < r->step_end; break; case XE_RTP_MATCH_GRAPHICS_VERSION: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.graphics_verx100 == r->ver_start && (!has_samedia(xe) || !xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_GRAPHICS_VERSION_RANGE: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.graphics_verx100 >= r->ver_start && xe->info.graphics_verx100 <= r->ver_end && @@ -80,13 +84,13 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_GRAPHICS_VERSION_ANY_GT: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.graphics_verx100 == r->ver_start; break; case XE_RTP_MATCH_GRAPHICS_STEP: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.step.graphics >= r->step_start && xe->info.step.graphics < r->step_end && @@ -94,14 +98,14 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_MEDIA_VERSION: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.media_verx100 == r->ver_start && (!has_samedia(xe) || xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_MEDIA_VERSION_RANGE: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.media_verx100 >= r->ver_start && xe->info.media_verx100 <= r->ver_end && @@ -109,7 +113,7 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_MEDIA_STEP: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.step.media >= r->step_start && xe->info.step.media < r->step_end && @@ -117,7 +121,7 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_MEDIA_VERSION_ANY_GT: if (drm_WARN_ON(&xe->drm, !gt)) - return false; + goto error; match = xe->info.media_verx100 == r->ver_start; break; @@ -129,13 +133,13 @@ static bool rule_matches(const struct xe_device *xe, break; case XE_RTP_MATCH_ENGINE_CLASS: if (drm_WARN_ON(&xe->drm, !hwe)) - return false; + goto error; match = hwe->class == r->engine_class; break; case XE_RTP_MATCH_NOT_ENGINE_CLASS: if (drm_WARN_ON(&xe->drm, !hwe)) - return false; + goto error; match = hwe->class != r->engine_class; break; @@ -167,9 +171,24 @@ static bool rule_matches(const struct xe_device *xe, done: if (drm_WARN_ON(&xe->drm, !rcount)) - return false; + goto error; return true; + +error: + if (err) + *err = -EINVAL; + + return false; +} + +static bool rule_matches(const struct xe_device *xe, + struct xe_gt *gt, + struct xe_hw_engine *hwe, + const struct xe_rtp_rule *rules, + unsigned int n_rules) +{ + return rule_matches_with_err(xe, gt, hwe, rules, n_rules, NULL); } static void rtp_add_sr_entry(const struct xe_rtp_action *action, @@ -412,3 +431,7 @@ bool xe_rtp_match_has_msix(const struct xe_device *xe, { return xe_device_has_msix(xe); } + +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) +#include "tests/xe_rtp.c" +#endif From ff7746336b2325087aa7958d40f327bcc50185f8 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:17 -0300 Subject: [PATCH 02/17] drm/xe/rtp: Drop rule matching cases from rtp_to_sr_cases and rtp_cases The kunit test cases for the RTP framework are currently separated into three groups: (1) rtp_rules_cases: Those to verify rule matching logic. (2) rtp_to_sr_cases: Those to verify generation of save/restore tables from RTP tables. (3) rtp_cases Those to verify processing of RTP tables without save/restore action associated, which are used for OOB workarounds. Today we have some cases in (2) and (3) that are actually meant to verify rule matching logic. Now that we have (1), let's cleanup (2) and (3) so that they become focused on their main objectives. Reviewed-by: Matt Roper Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-2-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp_test.c | 126 ++----------------------- 1 file changed, 8 insertions(+), 118 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index c98d85a15652..b3eab1337b0c 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -257,80 +257,6 @@ static const struct rtp_to_sr_test_case rtp_to_sr_cases[] = { {} }, }, - { - .name = "match-or", - .expected_reg = REGULAR_REG1, - .expected_set_bits = REG_BIT(0) | REG_BIT(1) | REG_BIT(2), - .expected_clr_bits = REG_BIT(0) | REG_BIT(1) | REG_BIT(2), - .expected_active = BIT(0) | BIT(1) | BIT(2), - .expected_count_sr_entries = 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))) - }, - { XE_RTP_NAME("middle"), - XE_RTP_RULES(FUNC(match_no), FUNC(match_no), OR, - FUNC(match_yes), OR, - FUNC(match_no)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(1))) - }, - { XE_RTP_NAME("last"), - XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_yes)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(2))) - }, - { XE_RTP_NAME("no-match"), - XE_RTP_RULES(FUNC(match_no), OR, FUNC(match_no)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(3))) - }, - {} - }, - }, - { - .name = "match-or-xfail", - .expected_reg = REGULAR_REG1, - .expected_count_sr_entries = 0, - .entries = (const struct xe_rtp_entry_sr[]) { - { XE_RTP_NAME("leading-or"), - XE_RTP_RULES(OR, FUNC(match_yes)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0))) - }, - { XE_RTP_NAME("trailing-or"), - /* - * First condition is match_no, otherwise the failure - * wouldn't really trigger as RTP stops processing as - * soon as it has a matching set of rules - */ - XE_RTP_RULES(FUNC(match_no), OR), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(1))) - }, - { XE_RTP_NAME("no-or-or-yes"), - XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_yes)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(2))) - }, - {} - }, - }, - { - .name = "no-match-no-add-multiple-rules", - .expected_reg = REGULAR_REG1, - .expected_set_bits = REG_BIT(0), - .expected_clr_bits = REG_BIT(0), - .expected_active = BIT(0), - .expected_count_sr_entries = 1, - /* Don't coalesce second entry due to one of the rules */ - .entries = (const struct xe_rtp_entry_sr[]) { - { XE_RTP_NAME("basic-1"), - XE_RTP_RULES(FUNC(match_yes)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(0))) - }, - { XE_RTP_NAME("basic-2"), - XE_RTP_RULES(FUNC(match_yes), FUNC(match_no)), - XE_RTP_ACTIONS(SET(REGULAR_REG1, REG_BIT(1))) - }, - {} - }, - }, { .name = "two-regs-two-entries", .expected_reg = REGULAR_REG1, @@ -591,16 +517,15 @@ static const struct rtp_test_case rtp_cases[] = { }, }, { - .name = "inactive-1st_or_active-inactive", + .name = "inactive-active-inactive", .expected_active = BIT(1), .entries = (const struct xe_rtp_entry[]) { { XE_RTP_NAME("r1"), XE_RTP_RULES(FUNC(match_no)), }, - { XE_RTP_NAME("r2_or_conditions"), - XE_RTP_RULES(FUNC(match_yes), OR, - FUNC(match_no), OR, - FUNC(match_no)) }, + { XE_RTP_NAME("r2"), + XE_RTP_RULES(FUNC(match_yes)), + }, { XE_RTP_NAME("r3"), XE_RTP_RULES(FUNC(match_no)), }, @@ -608,50 +533,15 @@ static const struct rtp_test_case rtp_cases[] = { }, }, { - .name = "inactive-2nd_or_active-inactive", - .expected_active = BIT(1), - .entries = (const struct xe_rtp_entry[]) { - { XE_RTP_NAME("r1"), - XE_RTP_RULES(FUNC(match_no)), - }, - { XE_RTP_NAME("r2_or_conditions"), - XE_RTP_RULES(FUNC(match_no), OR, - FUNC(match_yes), OR, - FUNC(match_no)) }, - { XE_RTP_NAME("r3"), - XE_RTP_RULES(FUNC(match_no)), - }, - {} - }, - }, - { - .name = "inactive-last_or_active-inactive", - .expected_active = BIT(1), - .entries = (const struct xe_rtp_entry[]) { - { XE_RTP_NAME("r1"), - XE_RTP_RULES(FUNC(match_no)), - }, - { XE_RTP_NAME("r2_or_conditions"), - XE_RTP_RULES(FUNC(match_no), OR, - FUNC(match_no), OR, - FUNC(match_yes)) }, - { XE_RTP_NAME("r3"), - XE_RTP_RULES(FUNC(match_no)), - }, - {} - }, - }, - { - .name = "inactive-no_or_active-inactive", + .name = "inactive-inactive-inactive", .expected_active = 0, .entries = (const struct xe_rtp_entry[]) { { XE_RTP_NAME("r1"), XE_RTP_RULES(FUNC(match_no)), }, - { XE_RTP_NAME("r2_or_conditions"), - XE_RTP_RULES(FUNC(match_no), OR, - FUNC(match_no), OR, - FUNC(match_no)) }, + { XE_RTP_NAME("r2"), + XE_RTP_RULES(FUNC(match_no)), + }, { XE_RTP_NAME("r3"), XE_RTP_RULES(FUNC(match_no)), }, From f6d460e33e537b576d74c67bb9b6352511f5bb96 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:18 -0300 Subject: [PATCH 03/17] drm/xe/rtp: Don't short-circuit to false in or-yes case While RTP processing evaluates true on the "yes-or" case (i.e. a conjunction of rules that evaluate to true followed by an "OR" without the right hand operand), it does not on the "or-yes" one. Both cases are considered malformed and could be a result of someone dropping checks deemed not necessary anymore and forgetting to drop the superfluous "OR". Nevertheless, we should aim for consistency, and having the "or-yes" case also evaluating to true while also causing a warning seems reasonable. So let's do that. The "or-yes" pattern being evaluated to false comes from the fact that that we unconditionally short-circuit upon finding XE_RTP_MATCH_OR on the outer loop. We should only do that if the preceding conjunction of rules evaluated to true (meaning that rcount must be non-zero) and continue the evaluation otherwise. Do that and also add extra test cases to validate the short-circuiting behavior. Notice that some of the new test cases have a "FIXME" comment, which comes from the fact that we are unable to detect syntax errors after the short-circuit point. That is going to be fixed in a follow-up change. Link: https://lore.kernel.org/intel-xe/871pfw4lo9.fsf@intel.com/ Reviewed-by: Matt Roper Reviewed-by: Violet Monti Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-3-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp_test.c | 42 ++++++++++++++++++++++++-- drivers/gpu/drm/xe/xe_rtp.c | 15 ++++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index b3eab1337b0c..f56f005dbd98 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -195,11 +195,49 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = { XE_RTP_RULES(OR), }, { - .name = "or-anything", - .expected_match = false, + .name = "or-yes", + .expected_match = true, .expected_err = -EINVAL, XE_RTP_RULES(OR, FUNC(match_yes)), }, + { + .name = "or-no", + .expected_match = false, + .expected_err = -EINVAL, + XE_RTP_RULES(OR, FUNC(match_no)), + }, + { + .name = "yes-or", + .expected_match = true, + /* FIXME: The parser should raise an error here. */ + .expected_err = 0, + XE_RTP_RULES(FUNC(match_yes), OR), + }, + { + .name = "no-or", + .expected_match = false, + .expected_err = -EINVAL, + XE_RTP_RULES(FUNC(match_no), OR), + }, + { + .name = "no-or-or-yes", + .expected_match = true, + .expected_err = -EINVAL, + XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_yes)), + }, + { + .name = "yes-or-or-no", + .expected_match = true, + /* FIXME: The parser should raise an error here. */ + .expected_err = 0, + XE_RTP_RULES(FUNC(match_yes), OR, OR, FUNC(match_no)), + }, + { + .name = "no-or-or-no", + .expected_match = false, + .expected_err = -EINVAL, + XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_no)), + }, }; static void xe_rtp_rules_tests(struct kunit *test) diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index 0e1adf07e4e7..299fa4209a26 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -47,12 +47,18 @@ static bool rule_matches_with_err(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: + if (drm_WARN_ON(&xe->drm, !rcount)) { + if (err) + *err = -EINVAL; + continue; + } + /* - * This is only reached if a complete set of - * rules passed or none were evaluated. For both cases, - * shortcut the other rules and return the proper value. + * This is only reached if a complete conjunction of + * rules passed, in which case we shortcut the other + * rules and return true. */ - goto done; + return true; case XE_RTP_MATCH_PLATFORM: match = xe->info.platform == r->platform; break; @@ -169,7 +175,6 @@ static bool rule_matches_with_err(const struct xe_device *xe, } } -done: if (drm_WARN_ON(&xe->drm, !rcount)) goto error; From 6dc07632134c10dc82abe3ced8316618b1d61c72 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:19 -0300 Subject: [PATCH 04/17] drm/xe/rtp: Do not break parsing when missing context With the current implementation, the RTP framework will cause parsing of the rule set to be interrupted if one rule requires a context item (gt or hwe) that is missing (i.e. when the value is NULL). This is arguably a semantic error instead of a syntactic one, meaning that RTP should not interrupt parsing the rules. With the current behavior, we would miss detecting other errors that could appear in the remaining rules and could also prevent valid rules joined by "OR" from being evaluated. Make sure that we do not stop parsing the rule set when detecting missing context and let's add rtp_rules_test_cases to reflect that. v2: - Add "missing-context" in the test case names to indicate that those are about rules that are missing the necessary context. (Matt) - Rebase: treat the new match type XE_RTP_MATCH_PLATFORM_STEP in the same way when the platform is missing step information. Reviewed-by: Matt Roper # v1 Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-4-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp_test.c | 28 +++++++++++ drivers/gpu/drm/xe/xe_rtp.c | 66 +++++++++++++++++--------- 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index f56f005dbd98..dfb15d81d799 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -238,6 +238,34 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = { .expected_err = -EINVAL, XE_RTP_RULES(FUNC(match_no), OR, OR, FUNC(match_no)), }, + + /* No match because hwe is NULL. */ + { + .name = "missing-context-engine-class", + .expected_match = false, + XE_RTP_RULES(ENGINE_CLASS(RENDER)), + }, + + /* + * Missing context (hwe==NULL) does not cause parsing to stop, hence we + * expect a match. + */ + { + .name = "missing-context-engine-class-or-yes", + .expected_match = true, + XE_RTP_RULES(ENGINE_CLASS(RENDER), OR, FUNC(match_yes)), + }, + + /* + * Missing context (hwe==NULL) does not cause parsing to stop, hence we + * expect a syntax error. + */ + { + .name = "missing-context-engine-class-or-or-yes", + .expected_match = true, + .expected_err = -EINVAL, + XE_RTP_RULES(ENGINE_CLASS(RENDER), OR, OR, FUNC(match_yes)), + }, }; static void xe_rtp_rules_tests(struct kunit *test) diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index 299fa4209a26..df72b456ab16 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -67,67 +67,85 @@ static bool rule_matches_with_err(const struct xe_device *xe, xe->info.subplatform == r->subplatform; break; case XE_RTP_MATCH_PLATFORM_STEP: - if (drm_WARN_ON(&xe->drm, xe->info.step.platform == STEP_NONE)) - goto error; + if (drm_WARN_ON(&xe->drm, xe->info.step.platform == STEP_NONE)) { + match = false; + break; + } match = xe->info.step.platform >= r->step_start && xe->info.step.platform < r->step_end; break; case XE_RTP_MATCH_GRAPHICS_VERSION: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.graphics_verx100 == r->ver_start && (!has_samedia(xe) || !xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_GRAPHICS_VERSION_RANGE: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.graphics_verx100 >= r->ver_start && xe->info.graphics_verx100 <= r->ver_end && (!has_samedia(xe) || !xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_GRAPHICS_VERSION_ANY_GT: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.graphics_verx100 == r->ver_start; break; case XE_RTP_MATCH_GRAPHICS_STEP: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.step.graphics >= r->step_start && xe->info.step.graphics < r->step_end && (!has_samedia(xe) || !xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_MEDIA_VERSION: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.media_verx100 == r->ver_start && (!has_samedia(xe) || xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_MEDIA_VERSION_RANGE: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.media_verx100 >= r->ver_start && xe->info.media_verx100 <= r->ver_end && (!has_samedia(xe) || xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_MEDIA_STEP: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.step.media >= r->step_start && xe->info.step.media < r->step_end && (!has_samedia(xe) || xe_gt_is_media_type(gt)); break; case XE_RTP_MATCH_MEDIA_VERSION_ANY_GT: - if (drm_WARN_ON(&xe->drm, !gt)) - goto error; + if (drm_WARN_ON(&xe->drm, !gt)) { + match = false; + break; + } match = xe->info.media_verx100 == r->ver_start; break; @@ -138,14 +156,18 @@ static bool rule_matches_with_err(const struct xe_device *xe, match = xe->info.is_dgfx; break; case XE_RTP_MATCH_ENGINE_CLASS: - if (drm_WARN_ON(&xe->drm, !hwe)) - goto error; + if (drm_WARN_ON(&xe->drm, !hwe)) { + match = false; + break; + } match = hwe->class == r->engine_class; break; case XE_RTP_MATCH_NOT_ENGINE_CLASS: - if (drm_WARN_ON(&xe->drm, !hwe)) - goto error; + if (drm_WARN_ON(&xe->drm, !hwe)) { + match = false; + break; + } match = hwe->class != r->engine_class; break; From 3f1b68f370ce1c35014f9c04f781b497fb0a7dd2 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:20 -0300 Subject: [PATCH 05/17] drm/xe/rtp: Extract rule_match_item() The current logic in rule_matches() mixes individual rule matching with the logic necessary for handling OR operations. Let's simplify rule_matches() to focus on the latter by extracting individual rule matching into a separate function called rule_match_item(). Reviewed-by: Matt Roper Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-5-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/xe_rtp.c | 219 ++++++++++++++++-------------------- 1 file changed, 94 insertions(+), 125 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index df72b456ab16..c49f80f398af 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -30,6 +30,96 @@ static bool has_samedia(const struct xe_device *xe) return xe->info.media_verx100 >= 1300; } +static bool rule_match_item(const struct xe_device *xe, + struct xe_gt *gt, + struct xe_hw_engine *hwe, + const struct xe_rtp_rule *r) +{ + switch (r->match_type) { + case XE_RTP_MATCH_PLATFORM: + return xe->info.platform == r->platform; + case XE_RTP_MATCH_SUBPLATFORM: + return xe->info.platform == r->platform && + xe->info.subplatform == r->subplatform; + case XE_RTP_MATCH_PLATFORM_STEP: + if (drm_WARN_ON(&xe->drm, xe->info.step.platform == STEP_NONE)) + return false; + + return xe->info.step.platform >= r->step_start && + xe->info.step.platform < r->step_end; + case XE_RTP_MATCH_GRAPHICS_VERSION: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.graphics_verx100 == r->ver_start && + (!has_samedia(xe) || !xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_GRAPHICS_VERSION_RANGE: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.graphics_verx100 >= r->ver_start && + xe->info.graphics_verx100 <= r->ver_end && + (!has_samedia(xe) || !xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_GRAPHICS_VERSION_ANY_GT: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.graphics_verx100 == r->ver_start; + case XE_RTP_MATCH_GRAPHICS_STEP: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.step.graphics >= r->step_start && + xe->info.step.graphics < r->step_end && + (!has_samedia(xe) || !xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_MEDIA_VERSION: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.media_verx100 == r->ver_start && + (!has_samedia(xe) || xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_MEDIA_VERSION_RANGE: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.media_verx100 >= r->ver_start && + xe->info.media_verx100 <= r->ver_end && + (!has_samedia(xe) || xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_MEDIA_STEP: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.step.media >= r->step_start && + xe->info.step.media < r->step_end && + (!has_samedia(xe) || xe_gt_is_media_type(gt)); + case XE_RTP_MATCH_MEDIA_VERSION_ANY_GT: + if (drm_WARN_ON(&xe->drm, !gt)) + return false; + + return xe->info.media_verx100 == r->ver_start; + case XE_RTP_MATCH_INTEGRATED: + return !xe->info.is_dgfx; + case XE_RTP_MATCH_DISCRETE: + return xe->info.is_dgfx; + case XE_RTP_MATCH_ENGINE_CLASS: + if (drm_WARN_ON(&xe->drm, !hwe)) + return false; + + return hwe->class == r->engine_class; + case XE_RTP_MATCH_NOT_ENGINE_CLASS: + if (drm_WARN_ON(&xe->drm, !hwe)) + return false; + + return hwe->class != r->engine_class; + case XE_RTP_MATCH_FUNC: + return r->match_func(xe, gt, hwe); + default: + drm_warn(&xe->drm, "Invalid RTP match %u\n", + r->match_type); + return false; + } +} + static bool rule_matches_with_err(const struct xe_device *xe, struct xe_gt *gt, struct xe_hw_engine *hwe, @@ -39,14 +129,12 @@ static bool rule_matches_with_err(const struct xe_device *xe, { const struct xe_rtp_rule *r; unsigned int i, rcount = 0; - bool match; if (err) *err = 0; for (r = rules, i = 0; i < n_rules; r = &rules[++i]) { - switch (r->match_type) { - case XE_RTP_MATCH_OR: + if (r->match_type == XE_RTP_MATCH_OR) { if (drm_WARN_ON(&xe->drm, !rcount)) { if (err) *err = -EINVAL; @@ -59,128 +147,11 @@ static bool rule_matches_with_err(const struct xe_device *xe, * rules and return true. */ return true; - case XE_RTP_MATCH_PLATFORM: - match = xe->info.platform == r->platform; - break; - case XE_RTP_MATCH_SUBPLATFORM: - match = xe->info.platform == r->platform && - xe->info.subplatform == r->subplatform; - break; - case XE_RTP_MATCH_PLATFORM_STEP: - if (drm_WARN_ON(&xe->drm, xe->info.step.platform == STEP_NONE)) { - match = false; - break; - } - - match = xe->info.step.platform >= r->step_start && - xe->info.step.platform < r->step_end; - break; - case XE_RTP_MATCH_GRAPHICS_VERSION: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.graphics_verx100 == r->ver_start && - (!has_samedia(xe) || !xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_GRAPHICS_VERSION_RANGE: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.graphics_verx100 >= r->ver_start && - xe->info.graphics_verx100 <= r->ver_end && - (!has_samedia(xe) || !xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_GRAPHICS_VERSION_ANY_GT: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.graphics_verx100 == r->ver_start; - break; - case XE_RTP_MATCH_GRAPHICS_STEP: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.step.graphics >= r->step_start && - xe->info.step.graphics < r->step_end && - (!has_samedia(xe) || !xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_MEDIA_VERSION: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.media_verx100 == r->ver_start && - (!has_samedia(xe) || xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_MEDIA_VERSION_RANGE: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.media_verx100 >= r->ver_start && - xe->info.media_verx100 <= r->ver_end && - (!has_samedia(xe) || xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_MEDIA_STEP: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.step.media >= r->step_start && - xe->info.step.media < r->step_end && - (!has_samedia(xe) || xe_gt_is_media_type(gt)); - break; - case XE_RTP_MATCH_MEDIA_VERSION_ANY_GT: - if (drm_WARN_ON(&xe->drm, !gt)) { - match = false; - break; - } - - match = xe->info.media_verx100 == r->ver_start; - break; - case XE_RTP_MATCH_INTEGRATED: - match = !xe->info.is_dgfx; - break; - case XE_RTP_MATCH_DISCRETE: - match = xe->info.is_dgfx; - break; - case XE_RTP_MATCH_ENGINE_CLASS: - if (drm_WARN_ON(&xe->drm, !hwe)) { - match = false; - break; - } - - match = hwe->class == r->engine_class; - break; - case XE_RTP_MATCH_NOT_ENGINE_CLASS: - if (drm_WARN_ON(&xe->drm, !hwe)) { - match = false; - break; - } - - match = hwe->class != r->engine_class; - break; - case XE_RTP_MATCH_FUNC: - match = r->match_func(xe, gt, hwe); - break; - default: - drm_warn(&xe->drm, "Invalid RTP match %u\n", - r->match_type); - match = false; } - if (!match) { + if (rule_match_item(xe, gt, hwe, r)) { + rcount++; + } else { /* * Advance rules until we find XE_RTP_MATCH_OR to check * if there's another set of conditions to check @@ -192,8 +163,6 @@ static bool rule_matches_with_err(const struct xe_device *xe, return false; rcount = 0; - } else { - rcount++; } } From 0da9ab6c4e12e66dbd2e8f54481662225a79b7f5 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:21 -0300 Subject: [PATCH 06/17] drm/xe/rtp: Fully parse the ruleset The function rule_matches() short-circuits evaluation of the implicit conjunctions (each substring of rules not containing OR) and the explicit disjunctions (implicit conjunctions joined by OR). In other words: - in a conjunction, once a rule evaluate to false, we skip to the next OR (if any) to evaluate the next conjunction; - in a disjunction, once a conjunction evaluates to true, we return true and skip evaluating all the remaining rules. While this behavior results in a correct logical value, due to how the "OR" short-circuiting is implemented, it has the side-effect that rule set does not get fully "parsed", allowing incomplete constructs like (rule1, OR) to evaluate to true when rule1 is true. We should treat such constructs as invalid and treat them the same way we do for stuff like (OR, rule1). As such, update rule_matches() to "parse" the whole rule set, and that while keeping the short-circuit aspect of evaluation. With that, we can fix the FIXME test cases that cover that behavior. v2: - Do not change short-circuit *evaluation* behavior. (Matt) Reviewed-by: Matt Roper Reviewed-by: Violet Monti Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-6-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/tests/xe_rtp_test.c | 6 ++---- drivers/gpu/drm/xe/xe_rtp.c | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c index dfb15d81d799..642f6e090ad0 100644 --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c @@ -209,8 +209,7 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = { { .name = "yes-or", .expected_match = true, - /* FIXME: The parser should raise an error here. */ - .expected_err = 0, + .expected_err = -EINVAL, XE_RTP_RULES(FUNC(match_yes), OR), }, { @@ -228,8 +227,7 @@ static const struct rtp_rules_test_case rtp_rules_cases[] = { { .name = "yes-or-or-no", .expected_match = true, - /* FIXME: The parser should raise an error here. */ - .expected_err = 0, + .expected_err = -EINVAL, XE_RTP_RULES(FUNC(match_yes), OR, OR, FUNC(match_no)), }, { diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index c49f80f398af..976a2e1f5592 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -129,6 +129,7 @@ static bool rule_matches_with_err(const struct xe_device *xe, { const struct xe_rtp_rule *r; unsigned int i, rcount = 0; + bool short_circuit_or = false; if (err) *err = 0; @@ -143,13 +144,16 @@ static bool rule_matches_with_err(const struct xe_device *xe, /* * This is only reached if a complete conjunction of - * rules passed, in which case we shortcut the other - * rules and return true. + * rules passed, in which case we short-circuit rule + * evaluation, but still keep parsing to find any syntax + * errors. */ - return true; + short_circuit_or = true; + rcount = 0; + continue; } - if (rule_match_item(xe, gt, hwe, r)) { + if (short_circuit_or || rule_match_item(xe, gt, hwe, r)) { rcount++; } else { /* @@ -166,16 +170,12 @@ static bool rule_matches_with_err(const struct xe_device *xe, } } - if (drm_WARN_ON(&xe->drm, !rcount)) - goto error; + if (drm_WARN_ON(&xe->drm, !rcount)) { + if (err) + *err = -EINVAL; + } - return true; - -error: - if (err) - *err = -EINVAL; - - return false; + return short_circuit_or || rcount; } static bool rule_matches(const struct xe_device *xe, From df07911cb72dabebec3dd18d94c33f9ced74c3d6 Mon Sep 17 00:00:00 2001 From: Gustavo Sousa Date: Fri, 22 May 2026 05:45:22 -0300 Subject: [PATCH 07/17] drm/xe/rtp: Implement a structured parser for rule matching The current unwritten grammar for RTP rules is as follows: rules = disjunction; disjunction = conjunction, { "OR", conjunction }; conjunction = single_rule, { single_rule }; (* the AND operator is implicit *) single_rule = ? GRAPHICS_VERSION(...), MEDIA_VERSION(...), FUNC(...), etc ? While rule_matches() currently works for the grammar above, it doesn't easily resemble it. Let's replace it with an implementation that is structured in a way to resemble the grammar. Such a new implementation, although a bit more verbose, is arguably easier to reason about and to adapt to any extension we do to the grammer in the future. Also take this opportunity to update the kernel-doc for XE_RTP_RULES() to include the grammar, so that it is not unwritten anymore. v2: - Include the grammar in the code documentation. (Matt) Reviewed-by: Matt Roper Reviewed-by: Violet Monti Link: https://patch.msgid.link/20260522-rtp-rule-parser-v3-7-0c51039899f4@intel.com Signed-off-by: Gustavo Sousa --- drivers/gpu/drm/xe/xe_rtp.c | 138 +++++++++++++++++++++++------------- drivers/gpu/drm/xe/xe_rtp.h | 29 ++++++-- 2 files changed, 113 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c index 976a2e1f5592..dec9d94e6fb0 100644 --- a/drivers/gpu/drm/xe/xe_rtp.c +++ b/drivers/gpu/drm/xe/xe_rtp.c @@ -30,11 +30,28 @@ static bool has_samedia(const struct xe_device *xe) return xe->info.media_verx100 >= 1300; } -static bool rule_match_item(const struct xe_device *xe, - struct xe_gt *gt, - struct xe_hw_engine *hwe, - const struct xe_rtp_rule *r) +struct rule_match_ctx { + const struct xe_device *xe; + struct xe_gt *gt; + struct xe_hw_engine *hwe; + const struct xe_rtp_rule *rules; + const unsigned int n_rules; + unsigned int head; + int err; +}; + +static bool rule_is_item(const struct xe_rtp_rule *r) { + return r->match_type != XE_RTP_MATCH_OR; +} + +static bool rule_match_item(struct rule_match_ctx *match_ctx) +{ + const struct xe_device *xe = match_ctx->xe; + struct xe_gt *gt = match_ctx->gt; + struct xe_hw_engine *hwe = match_ctx->hwe; + const struct xe_rtp_rule *r = &match_ctx->rules[match_ctx->head]; + switch (r->match_type) { case XE_RTP_MATCH_PLATFORM: return xe->info.platform == r->platform; @@ -120,6 +137,63 @@ static bool rule_match_item(const struct xe_device *xe, } } +/* + * Match a conjunctive set of rules (rules joined by an implicit "AND"). + * + * Once one item evaluates to false, the remaining items are not evaluated + * anymore. Nevetheless, all rules are consumed to allow detecting syntax + * errors. + */ +static bool rule_match_and(struct rule_match_ctx *match_ctx, bool parse_only) +{ + bool match = true; + unsigned int count = 0; + + while (match_ctx->head < match_ctx->n_rules && + rule_is_item(&match_ctx->rules[match_ctx->head])) { + if (!parse_only) + match = rule_match_item(match_ctx); + + if (!match) + parse_only = true; + + match_ctx->head++; + count++; + } + + if (drm_WARN_ON(&match_ctx->xe->drm, !count)) { + match_ctx->err = -EINVAL; + + if (!parse_only) + match = false; + } + + return match; +} + +/* + * Match a disjunctive set of rules (subset of rules joined by + * "XE_RTP_MATCH_OR"). + * + * Once one subset evaluates to true, the remaining items are not evaluated + * anymore. Nevetheless, all rules are consumed to allow detecting syntax + * errors. + */ +static bool rule_match_or(struct rule_match_ctx *match_ctx) +{ + bool match = rule_match_and(match_ctx, false); + + while (match_ctx->head < match_ctx->n_rules && + match_ctx->rules[match_ctx->head].match_type == XE_RTP_MATCH_OR) { + /* Consume XE_RTP_MATCH_OR. */ + match_ctx->head++; + + match = rule_match_and(match_ctx, match); + } + + return match; +} + static bool rule_matches_with_err(const struct xe_device *xe, struct xe_gt *gt, struct xe_hw_engine *hwe, @@ -127,55 +201,19 @@ static bool rule_matches_with_err(const struct xe_device *xe, unsigned int n_rules, int *err) { - const struct xe_rtp_rule *r; - unsigned int i, rcount = 0; - bool short_circuit_or = false; + struct rule_match_ctx match_ctx = { + .xe = xe, + .gt = gt, + .hwe = hwe, + .rules = rules, + .n_rules = n_rules, + }; + bool match = rule_match_or(&match_ctx); if (err) - *err = 0; + *err = match_ctx.err; - for (r = rules, i = 0; i < n_rules; r = &rules[++i]) { - if (r->match_type == XE_RTP_MATCH_OR) { - if (drm_WARN_ON(&xe->drm, !rcount)) { - if (err) - *err = -EINVAL; - continue; - } - - /* - * This is only reached if a complete conjunction of - * rules passed, in which case we short-circuit rule - * evaluation, but still keep parsing to find any syntax - * errors. - */ - short_circuit_or = true; - rcount = 0; - continue; - } - - if (short_circuit_or || rule_match_item(xe, gt, hwe, r)) { - rcount++; - } else { - /* - * 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; - - rcount = 0; - } - } - - if (drm_WARN_ON(&xe->drm, !rcount)) { - if (err) - *err = -EINVAL; - } - - return short_circuit_or || rcount; + return match; } static bool rule_matches(const struct xe_device *xe, diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h index 562082b18d7b..e4f1930ca1c3 100644 --- a/drivers/gpu/drm/xe/xe_rtp.h +++ b/drivers/gpu/drm/xe/xe_rtp.h @@ -394,18 +394,39 @@ struct xe_reg_sr; * XE_RTP_RULES - Helper to set multiple rules to a struct xe_rtp_entry_sr entry * @...: Rules * - * At least one rule is needed and up to 12 are supported. Multiple rules are - * AND'ed together, i.e. all the rules must evaluate to true for the entry to - * be processed. See XE_RTP_MATCH_* for the possible match rules. Example: + * When an RTP table is being processed, the rules of each entry are evaluated + * to check if they match the target entity (platform, gt or hwe, depending on + * the specific RTP table). + * + * The sequence of arguments of this macro must follow the following eBNF + * grammar:: + * + * rules = disjunction; + * disjunction = conjunction, { "OR", conjunction }; + * conjunction = single_rule, { single_rule }; + * (* the AND operator is implicit *) + * single_rule = ? GRAPHICS_VERSION(...), MEDIA_VERSION(...), + * FUNC(...), etc ? + * + * Examples: * * .. code-block:: c * * const struct xe_rtp_entry_sr wa_entries[] = { * ... - * { XE_RTP_NAME("test-entry"), + * { XE_RTP_NAME("entry-a"), + * // Match DG2-G10 with graphics steppings A0 up-to B0 + * // (exclusive). * XE_RTP_RULES(SUBPLATFORM(DG2, G10), GRAPHICS_STEP(A0, B0)), * ... * }, + * { XE_RTP_NAME("entry-b"), + * // Match graphics version 20 (all steppings) or graphics + * // version 30 steppings A0 up-to B0 (exclusive). + * XE_RTP_RULES(GRAPHICS_VERSION(2000), OR, + * GRAPHICS_VERSION(3000), GRAPHICS_STEP(A0, B0)) + * ... + * }, * ... * }; */ From 24f60b8e9f44b10614b43dbc4ba4b029f8ede3b6 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Sat, 23 May 2026 11:34:18 +0100 Subject: [PATCH 08/17] drm/xe: Assign queue name in time for drm_sched_init Currently the queue name is only assigned after the drm scheduler instance has been created. This loses information with all logging or debug workqueue facilities so lets re-order things a bit so the name gets assigned in time. To be able to assign a GuC ID early we split the allocation into reservation and publish phases. First, with the submission state lock held, we reserve the ID in the GuC ID manager, which serves as an authoritative source of truth. Then we can drop the lock and reserve entries in the exec queue lookup XArray. This can be lockless since the NULL entries are invisible both to the kernel and userspace. Only after the queue has been fully created we replace the reserved entries with the queue pointer, which can be done locklessly for single width queues. Signed-off-by: Tvrtko Ursulin Cc: Matthew Brost Cc: Rodrigo Vivi Cc: Thomas Hellstrom Reviewed-by: Rodrigo Vivi Link: https://patch.msgid.link/20260523103418.61832-1-tvrtko.ursulin@igalia.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_guc_submit.c | 72 +++++++++++++++++------------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index afd8cc7bd231..ab501513d806 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -408,46 +408,43 @@ void xe_guc_submit_disable(struct xe_guc *guc) guc->submission_state.enabled = false; } -static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q, u32 xa_count) +static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q, + int count) { int i; - lockdep_assert_held(&guc->submission_state.lock); + mutex_lock(&guc->submission_state.lock); - for (i = 0; i < xa_count; ++i) - xa_erase(&guc->submission_state.exec_queue_lookup, q->guc->id + i); + for (i = 0; i < count; ++i) + xa_erase(&guc->submission_state.exec_queue_lookup, + q->guc->id + i); xe_guc_id_mgr_release_locked(&guc->submission_state.idm, q->guc->id, q->width); if (xa_empty(&guc->submission_state.exec_queue_lookup)) wake_up(&guc->submission_state.fini_wq); + + mutex_unlock(&guc->submission_state.lock); } static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) { - int ret; - int i; - - /* - * Must use GFP_NOWAIT as this lock is in the dma fence signalling path, - * worse case user gets -ENOMEM on engine create and has to try again. - * - * FIXME: Have caller pre-alloc or post-alloc /w GFP_KERNEL to prevent - * failure. - */ - lockdep_assert_held(&guc->submission_state.lock); + int ret, i; + mutex_lock(&guc->submission_state.lock); ret = xe_guc_id_mgr_reserve_locked(&guc->submission_state.idm, q->width); + mutex_unlock(&guc->submission_state.lock); if (ret < 0) return ret; q->guc->id = ret; + /* Reserve empty slots. */ for (i = 0; i < q->width; ++i) { - ret = xa_err(xa_store(&guc->submission_state.exec_queue_lookup, - q->guc->id + i, q, GFP_NOWAIT)); + ret = xa_insert(&guc->submission_state.exec_queue_lookup, + q->guc->id + i, NULL, GFP_KERNEL); if (ret) goto err_release; } @@ -460,11 +457,24 @@ static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) return ret; } +static void publish_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) +{ + int i; + + lockdep_assert_held(&guc->submission_state.lock); + + for (i = 0; i < q->width; ++i) { + void *old; + + old = xa_store(&guc->submission_state.exec_queue_lookup, + q->guc->id + i, q, GFP_NOWAIT); + XE_WARN_ON(old || xa_is_err(old)); + } +} + static void release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q) { - mutex_lock(&guc->submission_state.lock); __release_guc_id(guc, q, q->width); - mutex_unlock(&guc->submission_state.lock); } struct exec_queue_policy { @@ -1961,6 +1971,12 @@ static int guc_exec_queue_init(struct xe_exec_queue *q) timeout = (q->vm && xe_vm_in_lr_mode(q->vm)) ? MAX_SCHEDULE_TIMEOUT : msecs_to_jiffies(q->sched_props.job_timeout_ms); + err = alloc_guc_id(guc, q); + if (err) + goto err_free; + + xe_exec_queue_assign_name(q, q->guc->id); + /* * Use primary queue's submit_wq for all secondary queues of a * multi queue group. This serialization avoids any locking around @@ -1977,28 +1993,21 @@ static int guc_exec_queue_init(struct xe_exec_queue *q) timeout, guc_to_gt(guc)->ordered_wq, NULL, q->name, gt_to_xe(q->gt)->drm.dev); if (err) - goto err_free; + goto err_release_id; sched = &ge->sched; err = xe_sched_entity_init(&ge->entity, sched); if (err) goto err_sched; - mutex_lock(&guc->submission_state.lock); - - err = alloc_guc_id(guc, q); - if (err) - goto err_entity; - q->entity = &ge->entity; + mutex_lock(&guc->submission_state.lock); if (xe_guc_read_stopped(guc) || vf_recovery(guc)) xe_sched_stop(sched); - + publish_guc_id(guc, q); mutex_unlock(&guc->submission_state.lock); - xe_exec_queue_assign_name(q, q->guc->id); - /* * Maintain secondary queues of the multi queue group in a list * for handling dependencies across the queues in the group. @@ -2021,11 +2030,10 @@ static int guc_exec_queue_init(struct xe_exec_queue *q) return 0; -err_entity: - mutex_unlock(&guc->submission_state.lock); - xe_sched_entity_fini(&ge->entity); err_sched: xe_sched_fini(&ge->sched); +err_release_id: + release_guc_id(guc, q); err_free: kfree(ge); From e20b4833f4883f187abd48b0f54dc26f25f2408b Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:46 +0200 Subject: [PATCH 09/17] drm/xe: Use raw device ID to find sub-platform descriptor We don't need the partially initialized xe_device pointer to find the sub-platform descriptor, as for the descriptor lookup only the device ID is required and it can be obtained directly from the pci_dev. Signed-off-by: Michal Wajdeczko Reviewed-by: Raag Jadav Link: https://patch.msgid.link/20260526195452.20545-2-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_pci.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index 1243c7d8ed10..0095a2d7efa4 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -575,14 +575,14 @@ static bool id_blocked(u16 device_id) } static const struct xe_subplatform_desc * -find_subplatform(const struct xe_device *xe, const struct xe_device_desc *desc) +find_subplatform(const struct xe_device_desc *desc, u16 devid) { const struct xe_subplatform_desc *sp; const u16 *id; for (sp = desc->subplatforms; sp && sp->subplatform; sp++) for (id = sp->pciidlist; *id; id++) - if (*id == xe->info.devid) + if (*id == devid) return sp; return NULL; @@ -1064,6 +1064,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) struct xe_device *xe; int err; + subplatform_desc = find_subplatform(desc, pdev->device); + xe_configfs_check_device(pdev); if (desc->require_force_probe && !id_forced(pdev->device)) { @@ -1098,7 +1100,6 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) pci_set_drvdata(pdev, &xe->drm); xe_pm_assert_unbounded_bridge(xe); - subplatform_desc = find_subplatform(xe, desc); pci_set_master(pdev); From 8a09097b11ca4d436691f64c3cc0958006f36e33 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:47 +0200 Subject: [PATCH 10/17] drm/xe: Drop unused param from xe_device_create() We never used or need anything from the struct pci_device_id there. And while around, add simple kernel-doc for this function. Signed-off-by: Michal Wajdeczko Reviewed-by: Raag Jadav Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-3-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_device.c | 11 +++++++++-- drivers/gpu/drm/xe/xe_device.h | 3 +-- drivers/gpu/drm/xe/xe_pci.c | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 576095cf0952..3f063e5530bb 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -475,8 +475,15 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy) ttm_device_fini(&xe->ttm); } -struct xe_device *xe_device_create(struct pci_dev *pdev, - const struct pci_device_id *ent) +/** + * xe_device_create() - Create a new &xe_device instance + * @pdev: the parent &pci_dev + * + * Allocate and initialize a device managed Xe device structure. + * + * Return: pointer to new &xe_device on success, or ERR_PTR on failure. + */ +struct xe_device *xe_device_create(struct pci_dev *pdev) { const struct drm_driver *driver = ®ular_driver; struct xe_device *xe; diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h index 355d69dc8f54..27cd2329b99f 100644 --- a/drivers/gpu/drm/xe/xe_device.h +++ b/drivers/gpu/drm/xe/xe_device.h @@ -43,8 +43,7 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm) return container_of(ttm, struct xe_device, ttm); } -struct xe_device *xe_device_create(struct pci_dev *pdev, - const struct pci_device_id *ent); +struct xe_device *xe_device_create(struct pci_dev *pdev); int xe_device_probe_early(struct xe_device *xe); int xe_device_probe(struct xe_device *xe); void xe_device_remove(struct xe_device *xe); diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index 0095a2d7efa4..b368bb190fc4 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -1093,7 +1093,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (err) return err; - xe = xe_device_create(pdev, ent); + xe = xe_device_create(pdev); if (IS_ERR(xe)) return PTR_ERR(xe); From f1d581bb50ed54b71a8121d3d46fe2627fddd147 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:48 +0200 Subject: [PATCH 11/17] drm/xe: Move xe->info.force_execlist initialization The xe_info_init_early() is a place where we initialize those of the xe->info fields that do not require any additional hardware probes. Move the initialization of the force_execlist flag there. Signed-off-by: Michal Wajdeczko Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-4-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_device.c | 1 - drivers/gpu/drm/xe/xe_pci.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 3f063e5530bb..cdc7e0935c13 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -523,7 +523,6 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) xe->info.devid = pdev->device; xe->info.revid = pdev->revision; - xe->info.force_execlist = xe_modparam.force_execlist; xe->atomic_svm_timeslice_ms = 5; xe->min_run_period_lr_ms = 5; diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index b368bb190fc4..ed932254f16f 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -777,6 +777,7 @@ static int xe_info_init_early(struct xe_device *xe, xe->info.probe_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) && xe_modparam.probe_display && desc->has_display; + xe->info.force_execlist = xe_modparam.force_execlist; xe_assert(xe, desc->max_gt_per_tile > 0); xe_assert(xe, desc->max_gt_per_tile <= XE_MAX_GT_PER_TILE); From c03d9fbe77ec5002a2345b9be464683e0b157709 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:49 +0200 Subject: [PATCH 12/17] drm/xe: Move xe->info.devid|revid initialization The xe_info_init_early() is a place where we initialize those of the xe->info fields that do not require any additional hardware probes. Move the initialization of the devid/revid also there, but to avoid breaking the kunit helper, which also calls this function, keep their initialization separate in sub-function so we can easily stub it when running the kunit test. Signed-off-by: Michal Wajdeczko Cc: Gustavo Sousa Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-5-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/tests/xe_pci.c | 6 ++++++ drivers/gpu/drm/xe/xe_device.c | 2 -- drivers/gpu/drm/xe/xe_pci.c | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c index 860409c579f8..9240aff779da 100644 --- a/drivers/gpu/drm/xe/tests/xe_pci.c +++ b/drivers/gpu/drm/xe/tests/xe_pci.c @@ -311,6 +311,11 @@ const void *xe_pci_id_gen_param(struct kunit *test, const void *prev, char *desc } EXPORT_SYMBOL_IF_KUNIT(xe_pci_id_gen_param); +static void fake_init_devid(struct xe_device *xe) +{ + /* Nothing to do, just keep zero. */ +} + static int fake_read_gmdid(struct xe_device *xe, enum xe_gmdid_type type, u32 *ver, u32 *revid) { @@ -369,6 +374,7 @@ int xe_pci_fake_device_init(struct xe_device *xe) xe->sriov.__mode = data && data->sriov_mode ? data->sriov_mode : XE_SRIOV_MODE_NONE; + kunit_activate_static_stub(test, init_devid, fake_init_devid); kunit_activate_static_stub(test, read_gmdid, fake_read_gmdid); kunit_activate_static_stub(test, xe_info_probe_tile_count, fake_xe_info_probe_tile_count); diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index cdc7e0935c13..b498147dcf61 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -521,8 +521,6 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) if (err) return ERR_PTR(err); - xe->info.devid = pdev->device; - xe->info.revid = pdev->revision; xe->atomic_svm_timeslice_ms = 5; xe->min_run_period_lr_ms = 5; diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index ed932254f16f..c05cd2ba3509 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -726,6 +726,16 @@ static int handle_gmdid(struct xe_device *xe, return 0; } +static void init_devid(struct xe_device *xe) +{ + struct pci_dev *pdev = to_pci_dev(xe->drm.dev); + + KUNIT_STATIC_STUB_REDIRECT(init_devid, xe); + + xe->info.devid = pdev->device; + xe->info.revid = pdev->revision; +} + /* * Initialize device info content that only depends on static driver_data * passed to the driver at probe time from PCI ID table. @@ -741,6 +751,8 @@ static int xe_info_init_early(struct xe_device *xe, xe->info.subplatform = subplatform_desc ? subplatform_desc->subplatform : XE_SUBPLATFORM_NONE; + init_devid(xe); + xe->info.dma_mask_size = desc->dma_mask_size; xe->info.va_bits = desc->va_bits; xe->info.vm_max_level = desc->vm_max_level; From 2841cea001b983db79dc1fdf160e65318dfda3cd Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:50 +0200 Subject: [PATCH 13/17] drm/xe: Separate early xe_device initialization We would like to initialize more of the xe_device struct also from the kunit code, as it should be safe to use most of the generic drm or xe components without doing any additional tweaks. Separate early xe initialization code to a new function, so it can be reused. Signed-off-by: Michal Wajdeczko Reviewed-by: Raag Jadav Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-6-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_device.c | 39 ++++++++++++++++++++++++---------- drivers/gpu/drm/xe/xe_device.h | 1 + 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index b498147dcf61..7ba407f73a02 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -506,27 +506,45 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) if (IS_ERR(xe)) return xe; + err = xe_device_init_early(xe); + if (err) + return ERR_PTR(err); + + return xe; +} +ALLOW_ERROR_INJECTION(xe_device_create, ERRNO); /* See xe_pci_probe() */ + +/** + * xe_device_init_early() - Initialize a new &xe_device instance + * @xe: the &xe_device to initialize + * + * Return: 0 on success or a negative error code on failure. + */ +int xe_device_init_early(struct xe_device *xe) +{ + int err; + err = ttm_device_init(&xe->ttm, &xe_ttm_funcs, xe->drm.dev, xe->drm.anon_inode->i_mapping, xe->drm.vma_offset_manager, 0); - if (WARN_ON(err)) - return ERR_PTR(err); + if (err) + return err; xe_bo_dev_init(&xe->bo_device); err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy, NULL); if (err) - return ERR_PTR(err); + return err; err = xe_shrinker_create(xe); if (err) - return ERR_PTR(err); + return err; xe->atomic_svm_timeslice_ms = 5; xe->min_run_period_lr_ms = 5; err = xe_irq_init(xe); if (err) - return ERR_PTR(err); + return err; xe_validation_device_init(&xe->val); @@ -536,7 +554,7 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) err = xe_pagemap_shrinker_create(xe); if (err) - return ERR_PTR(err); + return err; xa_init_flags(&xe->usm.asid_to_vm, XA_FLAGS_ALLOC); @@ -555,7 +573,7 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) err = xe_bo_pinned_init(xe); if (err) - return ERR_PTR(err); + return err; xe->preempt_fence_wq = alloc_ordered_workqueue("xe-preempt-fence-wq", WQ_MEM_RECLAIM); @@ -569,16 +587,15 @@ struct xe_device *xe_device_create(struct pci_dev *pdev) * drmm_add_action_or_reset register above */ drm_err(&xe->drm, "Failed to allocate xe workqueues\n"); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } err = drmm_mutex_init(&xe->drm, &xe->pmt.lock); if (err) - return ERR_PTR(err); + return err; - return xe; + return 0; } -ALLOW_ERROR_INJECTION(xe_device_create, ERRNO); /* See xe_pci_probe() */ static bool xe_driver_flr_disabled(struct xe_device *xe) { diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h index 27cd2329b99f..975768a6a9c8 100644 --- a/drivers/gpu/drm/xe/xe_device.h +++ b/drivers/gpu/drm/xe/xe_device.h @@ -44,6 +44,7 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm) } struct xe_device *xe_device_create(struct pci_dev *pdev); +int xe_device_init_early(struct xe_device *xe); int xe_device_probe_early(struct xe_device *xe); int xe_device_probe(struct xe_device *xe); void xe_device_remove(struct xe_device *xe); From 6c766f8d2206823093a91fcf4b57a35627281287 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:51 +0200 Subject: [PATCH 14/17] drm/xe/pm: Don't access device in init_early() We should separate software-only state initialization from anything else that requires access to the device's hardware. Extract d3cold capability detection into a new function. Add simple kernel-doc for updated functions here. Signed-off-by: Michal Wajdeczko Reviewed-by: Raag Jadav Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-7-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_device.c | 4 ++++ drivers/gpu/drm/xe/xe_pci.c | 5 +---- drivers/gpu/drm/xe/xe_pm.c | 27 ++++++++++++++++++++++++++- drivers/gpu/drm/xe/xe_pm.h | 1 + 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 7ba407f73a02..d224861b6f6f 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -594,6 +594,10 @@ int xe_device_init_early(struct xe_device *xe) if (err) return err; + err = xe_pm_init_early(xe); + if (err) + return err; + return 0; } diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index c05cd2ba3509..3165686e3e04 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -1167,7 +1167,7 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) str_yes_no(xe_device_has_sriov(xe)), xe_sriov_mode_to_string(xe_device_sriov_mode(xe))); - err = xe_pm_init_early(xe); + err = xe_pm_probe(xe); if (err) return err; @@ -1179,9 +1179,6 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (err) goto err_driver_cleanup; - drm_dbg(&xe->drm, "d3cold: capable=%s\n", - str_yes_no(xe->d3cold.capable)); - return 0; err_driver_cleanup: diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index d4672eb07476..5d1a3a26cb6e 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -24,6 +24,7 @@ #include "xe_irq.h" #include "xe_late_bind_fw.h" #include "xe_pcode.h" +#include "xe_printk.h" #include "xe_pxp.h" #include "xe_sriov_vf_ccs.h" #include "xe_sysctrl.h" @@ -349,6 +350,15 @@ static void xe_pm_runtime_init(struct xe_device *xe) pm_runtime_put(dev); } +/** + * xe_pm_init_early() - Initialize Xe Power Management + * @xe: the &xe_device instance + * + * Initialize everything that is a "software-only" state that does not + * require access to any of the device's hardware data. + * + * Return: 0 on success or a negative error code on failure. + */ int xe_pm_init_early(struct xe_device *xe) { int err; @@ -363,11 +373,26 @@ int xe_pm_init_early(struct xe_device *xe) if (err) return err; - xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe); return 0; } ALLOW_ERROR_INJECTION(xe_pm_init_early, ERRNO); /* See xe_pci_probe() */ +/** + * xe_pm_probe() - Initialize Xe Power Management + * @xe: the &xe_device instance + * + * Check d3cold capability. + * + * Return: 0 on success or a negative error code on failure. + */ +int xe_pm_probe(struct xe_device *xe) +{ + xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe); + xe_dbg(xe, "d3cold: capable=%s\n", str_yes_no(xe->d3cold.capable)); + + return 0; +} + static u32 vram_threshold_value(struct xe_device *xe) { if (xe->info.platform == XE_BATTLEMAGE) { diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h index 6b27039e7b2d..6d5ab09cb769 100644 --- a/drivers/gpu/drm/xe/xe_pm.h +++ b/drivers/gpu/drm/xe/xe_pm.h @@ -17,6 +17,7 @@ int xe_pm_suspend(struct xe_device *xe); int xe_pm_resume(struct xe_device *xe); int xe_pm_init_early(struct xe_device *xe); +int xe_pm_probe(struct xe_device *xe); int xe_pm_init(struct xe_device *xe); void xe_pm_fini(struct xe_device *xe); bool xe_pm_runtime_suspended(struct xe_device *xe); From 9462f2b677506d8d698e81bfa378bbfd65a19187 Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Tue, 26 May 2026 21:54:52 +0200 Subject: [PATCH 15/17] drm/xe/pm: Do early initialization in init_early() There is no need nor gain in splitting mutex or list initializations between two init functions as all of this is just pure software state and all this could be done at once. Signed-off-by: Michal Wajdeczko Reviewed-by: Gustavo Sousa Link: https://patch.msgid.link/20260526195452.20545-8-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_pm.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c index 5d1a3a26cb6e..99562f691080 100644 --- a/drivers/gpu/drm/xe/xe_pm.c +++ b/drivers/gpu/drm/xe/xe_pm.c @@ -363,6 +363,9 @@ int xe_pm_init_early(struct xe_device *xe) { int err; + init_completion(&xe->pm_block); + complete_all(&xe->pm_block); + INIT_LIST_HEAD(&xe->rebind_resume_list); INIT_LIST_HEAD(&xe->mem_access.vram_userfault.list); err = drmm_mutex_init(&xe->drm, &xe->mem_access.vram_userfault.lock); @@ -373,6 +376,10 @@ int xe_pm_init_early(struct xe_device *xe) if (err) return err; + err = drmm_mutex_init(&xe->drm, &xe->rebind_resume_lock); + if (err) + return err; + return 0; } ALLOW_ERROR_INJECTION(xe_pm_init_early, ERRNO); /* See xe_pci_probe() */ @@ -484,14 +491,6 @@ int xe_pm_init(struct xe_device *xe) if (err) return err; - err = drmm_mutex_init(&xe->drm, &xe->rebind_resume_lock); - if (err) - goto err_unregister; - - init_completion(&xe->pm_block); - complete_all(&xe->pm_block); - INIT_LIST_HEAD(&xe->rebind_resume_list); - /* For now suspend/resume is only allowed with GuC */ if (!xe_device_uc_enabled(xe)) return 0; From df1cfe24743a93b71eab27687e148ab8ae9b69e3 Mon Sep 17 00:00:00 2001 From: Balasubramani Vivekanandan Date: Fri, 22 May 2026 22:05:32 +0530 Subject: [PATCH 16/17] drm/xe: Restore IDLEDLY regiter on engine reset Wa_16023105232 programs the register IDLEDLY. The register is reset whenever the engine is reset. Therefore it should be added to the GuC save-restore register list for it to be restored after reset. Fixes: 7c53ff050ba8 ("drm/xe: Apply Wa_16023105232") Reviewed-by: Matt Roper Link: https://patch.msgid.link/20260522163531.1365540-2-balasubramani.vivekanandan@intel.com Signed-off-by: Balasubramani Vivekanandan --- drivers/gpu/drm/xe/xe_guc_ads.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index b9bca6084a4f..c98454545a85 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -768,6 +768,11 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, } } + if (XE_GT_WA(hwe->gt, 16023105232)) + guc_mmio_regset_write_one(ads, regset_map, + RING_IDLEDLY(hwe->mmio_base), + count++); + return count; } From 637c8e3a525b73ac3cf765831119b02a927d11cd Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Wed, 27 May 2026 13:26:07 +0200 Subject: [PATCH 17/17] drm/xe: Move xe_uc_fw_abi.h to abi/ We aim to keep all pure ABI headers in the abi/ folder, but somehow we missed this file. Signed-off-by: Michal Wajdeczko Reviewed-by: Rodrigo Vivi Link: https://patch.msgid.link/20260527112608.22448-1-michal.wajdeczko@intel.com --- Documentation/gpu/xe/xe_firmware.rst | 4 ++-- drivers/gpu/drm/xe/{xe_uc_fw_abi.h => abi/uc_fw_abi.h} | 4 ++-- drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++- drivers/gpu/drm/xe/xe_uc_fw.h | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) rename drivers/gpu/drm/xe/{xe_uc_fw_abi.h => abi/uc_fw_abi.h} (99%) diff --git a/Documentation/gpu/xe/xe_firmware.rst b/Documentation/gpu/xe/xe_firmware.rst index 9c15a300bc62..d3030d1c9a84 100644 --- a/Documentation/gpu/xe/xe_firmware.rst +++ b/Documentation/gpu/xe/xe_firmware.rst @@ -7,10 +7,10 @@ Firmware Firmware Layout =============== -.. kernel-doc:: drivers/gpu/drm/xe/xe_uc_fw_abi.h +.. kernel-doc:: drivers/gpu/drm/xe/abi/uc_fw_abi.h :doc: CSS-based Firmware Layout -.. kernel-doc:: drivers/gpu/drm/xe/xe_uc_fw_abi.h +.. kernel-doc:: drivers/gpu/drm/xe/abi/uc_fw_abi.h :doc: GSC-based Firmware Layout Write Once Protected Content Memory (WOPCM) Layout diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h b/drivers/gpu/drm/xe/abi/uc_fw_abi.h similarity index 99% rename from drivers/gpu/drm/xe/xe_uc_fw_abi.h rename to drivers/gpu/drm/xe/abi/uc_fw_abi.h index 74b888904fdc..198e949660e0 100644 --- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h +++ b/drivers/gpu/drm/xe/abi/uc_fw_abi.h @@ -3,8 +3,8 @@ * Copyright © 2022 Intel Corporation */ -#ifndef _XE_UC_FW_ABI_H_ -#define _XE_UC_FW_ABI_H_ +#ifndef _ABI_UC_FW_ABI_H +#define _ABI_UC_FW_ABI_H #include #include diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h index 7fdb24e810b3..ee5efe60774e 100644 --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h @@ -10,7 +10,8 @@ #include #include #include -#include "xe_uc_fw_abi.h" + +#include "abi/uc_fw_abi.h" #define XE_LB_MAX_PAYLOAD_SIZE SZ_4K diff --git a/drivers/gpu/drm/xe/xe_uc_fw.h b/drivers/gpu/drm/xe/xe_uc_fw.h index bb281b72a677..f2d3a3e7208b 100644 --- a/drivers/gpu/drm/xe/xe_uc_fw.h +++ b/drivers/gpu/drm/xe/xe_uc_fw.h @@ -8,8 +8,8 @@ #include +#include "abi/uc_fw_abi.h" #include "xe_macros.h" -#include "xe_uc_fw_abi.h" #include "xe_uc_fw_types.h" struct drm_printer;