[PATCH v6 3/4] drm/xe/guc: Introduce enum with offsets for multi-LRC register H2G

[Michal Wajdeczko]

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 <tomasz.lis at intel.com>
> ---
>  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,
	...

> +};
> +
>  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?