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

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 12/17/2025 2:37 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)
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_pc.c | 46 ++++++++++++++++------------------
>  drivers/gpu/drm/xe/xe_guc_pc.h |  2 ++
>  drivers/gpu/drm/xe/xe_guc_rc.c | 26 +++++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc_rc.h |  3 +++
>  drivers/gpu/drm/xe/xe_oa.c     |  9 +++----
>  5 files changed, 56 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index f8b683189f6c..03d7d3d2346e 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -253,6 +253,27 @@ 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

nit:

    * xe_guc_pc_action_set_param() - Set ...

> + * @pc: Xe_GuC_PC instance
> + * @id: Param id
> + * @value: Value to set
> + */
> +int xe_guc_pc_action_set_param(struct xe_guc_pc *pc, u8 id, u32 value)
> +{

maybe to make these helpers more useful,
take a rpm ref here instead of forcing callers to do that:

	guard(xe_pm_runtime)(guc_to_xe(guc));
or
	guard(xe_pm_runtime_noresume)(guc_to_xe(guc));

or at least assert that rpm is already taken?

> +	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
> + */
> +int xe_guc_pc_action_unset_param(struct xe_guc_pc *pc, u8 id)
> +{
> +	return pc_action_unset_param(pc, id);
> +}
> +
>  static u32 decode_freq(u32 raw)
>  {
>  	return DIV_ROUND_CLOSEST(raw * GT_FREQUENCY_MULTIPLIER,
> @@ -1034,31 +1055,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..00182a02a49e 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.h
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
> @@ -18,6 +18,8 @@ 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 a526405b686b..ba98e5db996f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_rc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_rc.c
> @@ -16,6 +16,7 @@
>  #include "xe_gt_idle.h"
>  #include "xe_gt_printk.h"
>  #include "xe_guc_ct.h"
> +#include "xe_guc_pc.h"
>  #include "xe_pm.h"
>  
>  /**
> @@ -131,3 +132,28 @@ int xe_guc_rc_start(struct xe_guc *guc)
>  
>  	return xe_guc_rc_enable(guc);
>  }
> +
> +/**
> + * xe_guc_rc_override_mode() - override GUCRC mode
> + * @guc: Xe GuC instance
> + * @mode: new value of the mode.
> + *
> + * Return: 0 on success, negative error code on error
> + */
> +int xe_guc_rc_override_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
> + *
> + * Return: 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 2500b0d1c151..c77fad9278f0 100644
> --- a/drivers/gpu/drm/xe/xe_guc_rc.h
> +++ b/drivers/gpu/drm/xe/xe_guc_rc.h
> @@ -12,9 +12,12 @@
>  
>  struct xe_gt;
>  struct xe_guc;
> +enum slpc_gucrc_mode;
>  
>  int xe_guc_rc_disable(struct xe_guc *guc);
>  int xe_guc_rc_start(struct xe_guc *guc);
>  int xe_guc_rc_init(struct xe_guc *guc);
> +int xe_guc_rc_override_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 cc48663c2b48..a474f086162e 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);
> @@ -1761,8 +1761,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_override_mode(&gt->uc.guc, SLPC_GUCRC_MODE_GUCRC_NO_RC6);
>  		if (ret)
>  			goto err_free_configs;
>  
> @@ -1820,7 +1819,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: