[PATCH v3] drm/amd/pm: add the interface to set and save the bootup workload type

[Lazar, Lijo]

On 1/8/2025 12:17 PM, Kenneth Feng wrote:
> add the interface to set and save the bootup workload type
> v2: add is_support_sw_smu check and pm mutex lock.
> v3: return before the scoreboard is set.
> 
> Signed-off-by: Kenneth Feng <kenneth.feng at amd.com>
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 44 +++++++++++++++++++
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |  3 ++
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  1 +
>  drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  1 +
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 6a9e26905edf..e262c972d0f7 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -349,6 +349,50 @@ int amdgpu_dpm_switch_power_profile(struct amdgpu_device *adev,
>  	return ret;
>  }
>  
> +int amdgpu_dpm_set_and_save_bootup_power_profile(struct amdgpu_device *adev,
> +						bool en)
> +{
> +	if (!is_support_sw_smu(adev))
> +		return -EOPNOTSUPP;
> +
> +	struct smu_context *smu = (struct smu_context*)(adev->powerplay.pp_handle);
> +	int ret = 0;
> +	int current_profile_mode;
> +	int saved_profile_mode;
> +
> +	mutex_lock(&adev->pm.mutex);
> +	current_profile_mode = smu->power_profile_mode;
> +	saved_profile_mode = smu->saved_power_profile_mode;
> +	mutex_unlock(&adev->pm.mutex);
> +
> +	if (en && current_profile_mode == PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return 0;
> +
> +	if (!en && current_profile_mode != PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT)
> +		return 0;

Does this mean that user may be allowed to set a different profile after
function is called with en?

> +
> +	if (en) {
> +		ret = amdgpu_dpm_switch_power_profile(adev, current_profile_mode, !en);
> +		if (ret)
> +			goto out;
> +		mutex_lock(&adev->pm.mutex);
> +		smu->saved_power_profile_mode = current_profile_mode;
> +		smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;


With the second patch combined, the whole logic is applied in generic
layer and may not work for all SOCs.

The default power profile mode of most SOCs is now set to
PP_SMC_POWER_PROFILE_FULLSCREEN3D if supported.

If driver internally has set compute/video profile, then disabling them
reverts only to default init mode which could be 3D mode. So the
assumption that power profile mode is back to bootup default is wrong in
such cases.

If no profile is set and when passed with !en, it will disable the
default init profile mode and workload mask will be 0. If user has set
another profile, it's the same behavior where workload mask is cleared.
Not sure if that's the desired behavior for SOCs where 3D mode is set as
default profile on init.

> +		mutex_unlock(&adev->pm.mutex);
> +	} else {
> +		ret = amdgpu_dpm_switch_power_profile(adev, saved_profile_mode, !en);

I guess the intention here is to enable the saved profile back and !en
doesn't do that.

> +		if (ret)
> +			goto out;
> +		mutex_lock(&adev->pm.mutex);
> +		smu->power_profile_mode = saved_profile_mode;
> +		smu->saved_power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;

If it's always assumed to be bootup default, what is the purpose of
saving this? Again, why is the assumption that boot_up default is the
saved one?

Thanks,
Lijo

> +		mutex_unlock(&adev->pm.mutex);
> +	}
> +
> +out:
> +	return ret;
> +}
> +
>  int amdgpu_dpm_set_xgmi_pstate(struct amdgpu_device *adev,
>  			       uint32_t pstate)
>  {
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 1f5ac7e0230d..5fe404bbb033 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -410,6 +410,9 @@ int amdgpu_dpm_switch_power_profile(struct amdgpu_device *adev,
>  				    enum PP_SMC_POWER_PROFILE type,
>  				    bool en);
>  
> +int amdgpu_dpm_set_and_save_bootup_power_profile(struct amdgpu_device *adev,
> +				    bool en);
> +
>  int amdgpu_dpm_baco_reset(struct amdgpu_device *adev);
>  
>  int amdgpu_dpm_mode2_reset(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 8ca793c222ff..a6f748361198 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1272,6 +1272,7 @@ static void smu_init_power_profile(struct smu_context *smu)
>  				PP_SMC_POWER_PROFILE_FULLSCREEN3D;
>  	}
>  	smu_power_profile_mode_get(smu, smu->power_profile_mode);
> +	smu->saved_power_profile_mode = smu->power_profile_mode;
>  }
>  
>  static int smu_sw_init(struct amdgpu_ip_block *ip_block)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 3630593bce61..c58fc31fb1d7 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -560,6 +560,7 @@ struct smu_context {
>  	uint32_t workload_mask;
>  	/* default/user workload preference */
>  	uint32_t power_profile_mode;
> +	uint32_t saved_power_profile_mode;
>  	uint32_t workload_refcount[PP_SMC_POWER_PROFILE_COUNT];
>  	/* backend specific custom workload settings */
>  	long *custom_profile_params;