On 01.04.2025 19:24, Michal Wajdeczko wrote:
On 31.03.2025 15:21, Tomasz Lis wrote:
Some GuC messages are constructed with incrementing dword counter
rather than referencing specific DWORDs, as described in GuC interface
specification.
This change introduces the definitions of DWORD numbers for parameters
which will need to be referenced in a CTB parser to be added in a
following patch. To ensure correctness of these DWORDs, verification
in form of asserts was added to the message construction code.
Signed-off-by: Tomasz Lis<[email protected]>
---
drivers/gpu/drm/xe/abi/guc_actions_abi.h | 7 +++++++
drivers/gpu/drm/xe/xe_guc_submit.c | 4 ++++
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
index 448afb86e05c..86bd4b092c7c 100644
--- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
@@ -161,6 +161,13 @@ enum xe_guc_preempt_options {
XE_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q = 0x8,
};
+enum xe_guc_register_context_multi_lrc_param_offsets {
+ XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_WQ_DESC = 5,
+ XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_WQ_BASE = 7,
+ XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_N_CHILDREN = 10,
+ XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_HWLRCA = 11
while likely we only need those 4 indices, IMO it would cleaner to
define all of them in ABI to reflect the whole message layout:
enum xe_guc_register_context_multi_lrc_message {
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_0_MBZ = 0,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_1_FLAGS,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_2_PARENT_CONTEXT,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_3_ENGINE_CLASS,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_4_ENGINE_SUBMIT_MASK,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_5_WQ_DESC_ADDR_LOWER,
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_6_WQ_DESC_ADDR_UPPER,
...
Will agree to all the propositions here as this whole patch isn't my idea.
-Tomasz
+};
+
enum xe_guc_report_status {
XE_GUC_REPORT_STATUS_UNKNOWN = 0x0,
XE_GUC_REPORT_STATUS_ACKED = 0x1,
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c
b/drivers/gpu/drm/xe/xe_guc_submit.c
index 31bc2022bfc2..3c8574e2c1f3 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -469,12 +469,16 @@ static void __register_mlrc_exec_queue(struct xe_guc *guc,
action[len++] = info->context_idx;
action[len++] = info->engine_class;
action[len++] = info->engine_submit_mask;
+ xe_gt_assert(guc_to_gt(guc), len ==
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_WQ_DESC);
action[len++] = info->wq_desc_lo;
action[len++] = info->wq_desc_hi;
+ xe_gt_assert(guc_to_gt(guc), len ==
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_WQ_BASE);
action[len++] = info->wq_base_lo;
action[len++] = info->wq_base_hi;
action[len++] = info->wq_size;
+ xe_gt_assert(guc_to_gt(guc), len ==
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_N_CHILDREN);
action[len++] = q->width;
+ xe_gt_assert(guc_to_gt(guc), len ==
XE_GUC_REGISTER_CONTEXT_MULTI_LRC_OFFS_HWLRCA);
action[len++] = info->hwlrca_lo;
action[len++] = info->hwlrca_hi;
hmm, it looks little paranoid, and pollute otherwise clean code,
so maybe either convert completely to index based message encoding:
action[.._FLAGS] = info->flags;
action[.._WQ_DESC_LO] = info->wq_desc_lo;
action[.._WQ_DESC_HI] = info->wq_desc_hi;
...
for (i = 0; i < q->width; ++i)
action[.._HWLRCA_LO + 1 + i] = ..;
action[.._HWLRCA_HI + 1 + i] = ..;
or assert those important entries at the end:
xe_gt_assert(gt, action[..WQ_DESC_LO] == info->wq_desc_lo);
xe_gt_assert(gt, action[..HWLRCA_LO] == info->hwlrca_lo);
@Matt, any other ideas or you don't care?
