Re: [PATCH 1/3] drm/xe/guc: Move H2G SETUP_PC_GUCRC definition to SLPC ABI

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

On 6/9/2024 11:19 AM, Michal Wajdeczko wrote:
> We already have a dedicated file for GuC SLPC ABI definitions.
> Move definition of the SETUP_PC_GUCRC action and related enum
> to that file, rename them to match format of other new ABI
> definitions and add simple kernel-doc.

LGTM,

Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>   drivers/gpu/drm/xe/abi/guc_actions_abi.h      |  6 -----
>   drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h | 22 +++++++++++++++++++
>   drivers/gpu/drm/xe/xe_guc_pc.c                |  7 +++---
>   3 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/abi/guc_actions_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> index 79ba98a169f9..43ad4652c2b2 100644
> --- a/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_actions_abi.h
> @@ -128,7 +128,6 @@ enum xe_guc_action {
>   	XE_GUC_ACTION_CONTEXT_RESET_NOTIFICATION = 0x1008,
>   	XE_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
>   	XE_GUC_ACTION_HOST2GUC_UPDATE_CONTEXT_POLICIES = 0x100B,
> -	XE_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
>   	XE_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
>   	XE_GUC_ACTION_GET_HWCONFIG = 0x4100,
>   	XE_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
> @@ -153,11 +152,6 @@ enum xe_guc_action {
>   	XE_GUC_ACTION_LIMIT
>   };
>   
> -enum xe_guc_rc_options {
> -	XE_GUCRC_HOST_CONTROL,
> -	XE_GUCRC_FIRMWARE_CONTROL,
> -};
> -
>   enum xe_guc_preempt_options {
>   	XE_GUC_PREEMPT_OPTION_DROP_WORK_Q = 0x4,
>   	XE_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q = 0x8,
> diff --git a/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> index c165e26c0976..85abe4f09ae2 100644
> --- a/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> @@ -246,4 +246,26 @@ struct slpc_shared_data {
>   #define HOST2GUC_PC_SLPC_REQUEST_MSG_1_EVENT_ARGC	(0xffu << 0)
>   #define HOST2GUC_PC_SLPC_REQUEST_MSG_N_EVENT_DATA_N	GUC_HXG_REQUEST_MSG_n_DATAn
>   
> +/**
> + * DOC: SETUP_PC_GUCRC
> + *
> + *  +---+-------+--------------------------------------------------------------+
> + *  |   | Bits  | Description                                                  |
> + *  +===+=======+==============================================================+
> + *  | 0 |    31 | ORIGIN = GUC_HXG_ORIGIN_HOST_                                |
> + *  |   +-------+--------------------------------------------------------------+
> + *  |   | 30:28 | TYPE = GUC_HXG_TYPE_FAST_REQUEST_                            |
> + *  |   +-------+--------------------------------------------------------------+
> + *  |   | 27:16 | DATA0 = MBZ                                                  |
> + *  |   +-------+--------------------------------------------------------------+
> + *  |   |  15:0 | ACTION = _`GUC_ACTION_HOST2GUC_SETUP_PC_GUCRC` = 0x3004      |
> + *  +---+-------+--------------------------------------------------------------+
> + *  | 1 |  31:0 | **MODE** = GUCRC_HOST_CONTROL(0), GUCRC_FIRMWARE_CONTROL(1)  |
> + *  +---+-------+--------------------------------------------------------------+
> + */
> +
> +#define GUC_ACTION_HOST2GUC_SETUP_PC_GUCRC		0x3004u
> +#define   GUCRC_HOST_CONTROL				0u
> +#define   GUCRC_FIRMWARE_CONTROL			1u
> +
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 508f0d39b4ad..b57207bb1f11 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -9,7 +9,6 @@
>   
>   #include <drm/drm_managed.h>
>   
> -#include "abi/guc_actions_abi.h"
>   #include "abi/guc_actions_slpc_abi.h"
>   #include "regs/xe_gt_regs.h"
>   #include "regs/xe_regs.h"
> @@ -195,7 +194,7 @@ static int pc_action_setup_gucrc(struct xe_guc_pc *pc, u32 mode)
>   {
>   	struct xe_guc_ct *ct = &pc_to_guc(pc)->ct;
>   	u32 action[] = {
> -		XE_GUC_ACTION_SETUP_PC_GUCRC,
> +		GUC_ACTION_HOST2GUC_SETUP_PC_GUCRC,
>   		mode,
>   	};
>   	int ret;
> @@ -758,7 +757,7 @@ int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc)
>   	if (xe->info.skip_guc_pc)
>   		return 0;
>   
> -	ret = pc_action_setup_gucrc(pc, XE_GUCRC_HOST_CONTROL);
> +	ret = pc_action_setup_gucrc(pc, GUCRC_HOST_CONTROL);
>   	if (ret)
>   		return ret;
>   
> @@ -861,7 +860,7 @@ int xe_guc_pc_start(struct xe_guc_pc *pc)
>   		goto out;
>   	}
>   
> -	ret = pc_action_setup_gucrc(pc, XE_GUCRC_FIRMWARE_CONTROL);
> +	ret = pc_action_setup_gucrc(pc, GUCRC_FIRMWARE_CONTROL);
>   
>   out:
>   	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));