Re: [PATCH v6 2/2] drm/xe: Add a wrapper for set/unset params

[Riana Tauro <riana.tauro@intel.com>]

On 1/24/2026 5:20 AM, Vinay Belgaumkar wrote:
> Also, extract out the GuC RC related set/unset param functions
> into xe_guc_rc file. GuC still allows us to override GuC RC mode
> using an SLPC H2G interface. Continue to use that interface, but
> move the related code to the newly created xe_guc_rc file.
> 
> v2: xe_guc_rc functions to use guc pointer instead of gt (Michal W)
> v3: Assert if runtime pm ref is not held (Michal W)
> v4: Review comments (Riana)
> 
> Cc: Riana Tauro <riana.tauro@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

Looks good to me.
Reviewed-by: Riana Tauro <riana.tauro@intel.com>

> ---
>   drivers/gpu/drm/xe/xe_guc_pc.c | 56 +++++++++++++++++++---------------
>   drivers/gpu/drm/xe/xe_guc_pc.h |  5 ++-
>   drivers/gpu/drm/xe/xe_guc_rc.c | 30 ++++++++++++++++++
>   drivers/gpu/drm/xe/xe_guc_rc.h |  3 ++
>   drivers/gpu/drm/xe/xe_oa.c     |  9 +++---
>   5 files changed, 70 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index cf02c53ca3c5..31c6cb111916 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -264,6 +264,37 @@ static int pc_action_unset_param(struct xe_guc_pc *pc, u8 id)
>   	return ret;
>   }
>   
> +/**
> + * xe_guc_pc_action_set_param() - Set value of SLPC param
> + * @pc: Xe_GuC_PC instance
> + * @id: Param id
> + * @value: Value to set
> + *
> + * This function can be used to set any SLPC param.
> + *
> + * Returns: 0 on Success
> + */
> +int xe_guc_pc_action_set_param(struct xe_guc_pc *pc, u8 id, u32 value)
> +{
> +	xe_device_assert_mem_access(pc_to_xe(pc));
> +	return pc_action_set_param(pc, id, value);
> +}
> +
> +/**
> + * xe_guc_pc_action_unset_param() - Revert to default value
> + * @pc: Xe_GuC_PC instance
> + * @id: Param id
> + *
> + * This function can be used revert any SLPC param to its default value.
> + *
> + * Returns: 0 on Success
> + */
> +int xe_guc_pc_action_unset_param(struct xe_guc_pc *pc, u8 id)
> +{
> +	xe_device_assert_mem_access(pc_to_xe(pc));
> +	return pc_action_unset_param(pc, id);
> +}
> +
>   static u32 decode_freq(u32 raw)
>   {
>   	return DIV_ROUND_CLOSEST(raw * GT_FREQUENCY_MULTIPLIER,
> @@ -1045,31 +1076,6 @@ int xe_guc_pc_restore_stashed_freq(struct xe_guc_pc *pc)
>   	return ret;
>   }
>   
> -/**
> - * xe_guc_pc_override_gucrc_mode - override GUCRC mode
> - * @pc: Xe_GuC_PC instance
> - * @mode: new value of the mode.
> - *
> - * Return: 0 on success, negative error code on error
> - */
> -int xe_guc_pc_override_gucrc_mode(struct xe_guc_pc *pc, enum slpc_gucrc_mode mode)
> -{
> -	guard(xe_pm_runtime)(pc_to_xe(pc));
> -	return pc_action_set_param(pc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
> -}
> -
> -/**
> - * xe_guc_pc_unset_gucrc_mode - unset GUCRC mode override
> - * @pc: Xe_GuC_PC instance
> - *
> - * Return: 0 on success, negative error code on error
> - */
> -int xe_guc_pc_unset_gucrc_mode(struct xe_guc_pc *pc)
> -{
> -	guard(xe_pm_runtime)(pc_to_xe(pc));
> -	return pc_action_unset_param(pc, SLPC_PARAM_PWRGATE_RC_MODE);
> -}
> -
>   static void pc_init_pcode_freq(struct xe_guc_pc *pc)
>   {
>   	u32 min = DIV_ROUND_CLOSEST(pc->rpn_freq, GT_FREQUENCY_MULTIPLIER);
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h b/drivers/gpu/drm/xe/xe_guc_pc.h
> index 1b95873b262e..0678a4e787b3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.h
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
> @@ -9,15 +9,14 @@
>   #include <linux/types.h>
>   
>   struct xe_guc_pc;
> -enum slpc_gucrc_mode;
>   struct drm_printer;
>   
>   int xe_guc_pc_init(struct xe_guc_pc *pc);
>   int xe_guc_pc_start(struct xe_guc_pc *pc);
>   int xe_guc_pc_stop(struct xe_guc_pc *pc);
> -int xe_guc_pc_override_gucrc_mode(struct xe_guc_pc *pc, enum slpc_gucrc_mode mode);
> -int xe_guc_pc_unset_gucrc_mode(struct xe_guc_pc *pc);
>   void xe_guc_pc_print(struct xe_guc_pc *pc, struct drm_printer *p);
> +int xe_guc_pc_action_set_param(struct xe_guc_pc *pc, u8 id, u32 value);
> +int xe_guc_pc_action_unset_param(struct xe_guc_pc *pc, u8 id);
>   
>   u32 xe_guc_pc_get_act_freq(struct xe_guc_pc *pc);
>   int xe_guc_pc_get_cur_freq(struct xe_guc_pc *pc, u32 *freq);
> diff --git a/drivers/gpu/drm/xe/xe_guc_rc.c b/drivers/gpu/drm/xe/xe_guc_rc.c
> index fc57e5290635..0a900c309ecf 100644
> --- a/drivers/gpu/drm/xe/xe_guc_rc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_rc.c
> @@ -14,6 +14,7 @@
>   #include "xe_gt_printk.h"
>   #include "xe_guc.h"
>   #include "xe_guc_ct.h"
> +#include "xe_guc_pc.h"
>   #include "xe_guc_rc.h"
>   #include "xe_pm.h"
>   
> @@ -112,3 +113,32 @@ int xe_guc_rc_enable(struct xe_guc *guc)
>   
>   	return guc_action_setup_gucrc(guc, GUCRC_FIRMWARE_CONTROL);
>   }
> +
> +/**
> + * xe_guc_rc_set_mode() - set GUCRC mode
> + * @guc: Xe GuC instance
> + * @mode: new value of the mode.
> + *
> + * Function to set GuC RC mode to one of the enum values.
> + *
> + * Returns: 0 on success, negative error code on error
> + */
> +int xe_guc_rc_set_mode(struct xe_guc *guc, enum slpc_gucrc_mode mode)
> +{
> +	guard(xe_pm_runtime)(guc_to_xe(guc));
> +	return xe_guc_pc_action_set_param(&guc->pc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
> +}
> +
> +/**
> + * xe_guc_rc_unset_mode() - revert to default mode
> + * @guc: Xe GuC instance
> + *
> + * Function to revert GuC RC mode to platform defaults.
> + *
> + * Returns: 0 on success, negative error code on error
> + */
> +int xe_guc_rc_unset_mode(struct xe_guc *guc)
> +{
> +	guard(xe_pm_runtime)(guc_to_xe(guc));
> +	return xe_guc_pc_action_unset_param(&guc->pc, SLPC_PARAM_PWRGATE_RC_MODE);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_guc_rc.h b/drivers/gpu/drm/xe/xe_guc_rc.h
> index c980b0fce472..d76dfad8e26c 100644
> --- a/drivers/gpu/drm/xe/xe_guc_rc.h
> +++ b/drivers/gpu/drm/xe/xe_guc_rc.h
> @@ -7,8 +7,11 @@
>   #define _XE_GUC_RC_H_
>   
>   struct xe_guc;
> +enum slpc_gucrc_mode;
>   
>   void xe_guc_rc_disable(struct xe_guc *guc);
>   int xe_guc_rc_enable(struct xe_guc *guc);
> +int xe_guc_rc_set_mode(struct xe_guc *guc, enum slpc_gucrc_mode mode);
> +int xe_guc_rc_unset_mode(struct xe_guc *guc);
>   
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index abf87fe0b345..f7752719b74f 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -29,7 +29,7 @@
>   #include "xe_gt.h"
>   #include "xe_gt_mcr.h"
>   #include "xe_gt_printk.h"
> -#include "xe_guc_pc.h"
> +#include "xe_guc_rc.h"
>   #include "xe_macros.h"
>   #include "xe_mmio.h"
>   #include "xe_oa.h"
> @@ -875,7 +875,7 @@ static void xe_oa_stream_destroy(struct xe_oa_stream *stream)
>   
>   	/* Wa_1509372804:pvc: Unset the override of GUCRC mode to enable rc6 */
>   	if (stream->override_gucrc)
> -		xe_gt_WARN_ON(gt, xe_guc_pc_unset_gucrc_mode(&gt->uc.guc.pc));
> +		xe_gt_WARN_ON(gt, xe_guc_rc_unset_mode(&gt->uc.guc));
>   
>   	xe_oa_free_configs(stream);
>   	xe_file_put(stream->xef);
> @@ -1765,8 +1765,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream,
>   	 * state. Prevent this by overriding GUCRC mode.
>   	 */
>   	if (XE_GT_WA(stream->gt, 1509372804)) {
> -		ret = xe_guc_pc_override_gucrc_mode(&gt->uc.guc.pc,
> -						    SLPC_GUCRC_MODE_GUCRC_NO_RC6);
> +		ret = xe_guc_rc_set_mode(&gt->uc.guc, SLPC_GUCRC_MODE_GUCRC_NO_RC6);
>   		if (ret)
>   			goto err_free_configs;
>   
> @@ -1824,7 +1823,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream,
>   	xe_force_wake_put(gt_to_fw(gt), stream->fw_ref);
>   	xe_pm_runtime_put(stream->oa->xe);
>   	if (stream->override_gucrc)
> -		xe_gt_WARN_ON(gt, xe_guc_pc_unset_gucrc_mode(&gt->uc.guc.pc));
> +		xe_gt_WARN_ON(gt, xe_guc_rc_unset_mode(&gt->uc.guc));
>   err_free_configs:
>   	xe_oa_free_configs(stream);
>   exit: