<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 08.04.2025 15:34, Michal Wajdeczko wrote: </div> <blockquote type="cite" cite="mid:b999151b-923d-457b-97eb-ef656cd7c1d3@intel.com"> <pre wrap="" class="moz-quote-pre">On 03.04.2025 20:40, Tomasz Lis wrote: </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">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. v2: Renamed enum members, added ones for single context registration, modified asserts to check values rather than indexes. Signed-off-by: Tomasz Lis <a class="moz-txt-link-rfc2396E" href="mailto:tomasz.lis@intel.com"><tomasz.lis@intel.com></a> Suggested-by: Michal Wajdeczko <a class="moz-txt-link-rfc2396E" href="mailto:michal.wajdeczko@intel.com"><michal.wajdeczko@intel.com></a> --- drivers/gpu/drm/xe/abi/guc_actions_abi.h | 29 ++++++++++++++++++++++++ drivers/gpu/drm/xe/xe_guc_submit.c | 22 ++++++++++++++++++ 2 files changed, 51 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..64c71526356e 100644 --- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h @@ -161,6 +161,35 @@ enum xe_guc_preempt_options { XE_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q = 0x8, }; +enum xe_guc_register_context_param_offsets { + XE_GUC_REGISTER_CONTEXT_DATA_0_MBZ = 0, + XE_GUC_REGISTER_CONTEXT_DATA_1_FLAGS, + XE_GUC_REGISTER_CONTEXT_DATA_2_CONTEXT_INDEX, + XE_GUC_REGISTER_CONTEXT_DATA_3_ENGINE_CLASS, + XE_GUC_REGISTER_CONTEXT_DATA_4_ENGINE_SUBMIT_MASK, + XE_GUC_REGISTER_CONTEXT_DATA_5_WQ_DESC_ADDR_LOWER, + XE_GUC_REGISTER_CONTEXT_DATA_6_WQ_DESC_ADDR_UPPER, + XE_GUC_REGISTER_CONTEXT_DATA_7_WQ_BUF_BASE_LOWER, + XE_GUC_REGISTER_CONTEXT_DATA_8_WQ_BUF_BASE_UPPER, + XE_GUC_REGISTER_CONTEXT_DATA_9_WQ_BUF_SIZE, + XE_GUC_REGISTER_CONTEXT_DATA_10_HW_LRC_ADDR, +}; + +enum xe_guc_register_context_multi_lrc_param_offsets { + 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, + XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_7_WQ_BUF_BASE_LOWER, + XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_8_WQ_BUF_BASE_UPPER, + XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_9_WQ_BUF_SIZE, + XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_10_NUM_CTXS, + XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_11_HW_LRC_ADDR, +}; + 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..63ef06d3a28f 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -485,6 +485,18 @@ static void __register_mlrc_exec_queue(struct xe_guc *guc, action[len++] = upper_32_bits(xe_lrc_descriptor(lrc)); } </pre> </blockquote> <pre wrap="" class="moz-quote-pre">nit: if we want to keep these asserts then small comment saying /* explicitly checks some fields that we might fixup later */ will not hurt</pre> </blockquote> will add <blockquote type="cite" cite="mid:b999151b-923d-457b-97eb-ef656cd7c1d3@intel.com"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">+ xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_5_WQ_DESC_ADDR_LOWER] + == info->wq_desc_lo); + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_7_WQ_BUF_BASE_LOWER] + == info->wq_base_lo); + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_10_NUM_CTXS] + == q->width); + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_11_HW_LRC_ADDR] + == info->hwlrca_lo); </pre> </blockquote> <pre wrap="" class="moz-quote-pre">maybe we can spare one line in each assert by: xe_gt_assert(guc_to_gt(guc), info->hwlrca_lo == action[XE_GUC_REGISTER_CONTEXT_MULTI_LRC_DATA_11_HW_LRC_ADDR]); or by introducing: xe_guc_assert(guc, cond) or by ignoring 100 column limit - it will not be first time in xe ;)</pre> </blockquote> will switch order of operands. a macro wouldn't be much better, unless it merges the enum name, ie. `XE_GUC_REGISTER_CONTEXT_ ## part_name`. But I really don't like such techniques, as I then _always_ hear someone can't find the name and is confused. -Tomasz <blockquote type="cite" cite="mid:b999151b-923d-457b-97eb-ef656cd7c1d3@intel.com"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre"> xe_gt_assert(guc_to_gt(guc), len <= MAX_MLRC_REG_SIZE); #undef MAX_MLRC_REG_SIZE @@ -509,6 +521,16 @@ static void __register_exec_queue(struct xe_guc *guc, info->hwlrca_hi, }; + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_DATA_5_WQ_DESC_ADDR_LOWER] + == info->wq_desc_lo); + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_DATA_7_WQ_BUF_BASE_LOWER] + == info->wq_base_lo); + xe_gt_assert(guc_to_gt(guc), + action[XE_GUC_REGISTER_CONTEXT_DATA_10_HW_LRC_ADDR] + == info->hwlrca_lo); + xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0); } </pre> </blockquote> <pre wrap="" class="moz-quote-pre">otherwise, since I don't have better ideas for enum names, LGTM </pre> </blockquote> </body> </html>