[PATCH 16/30] drm/amd/powerplay: correct Navi10 VCN powergate control

[Kevin Wang]

On 7/30/19 4:14 AM, Alex Deucher wrote:
> From: Evan Quan <evan.quan at amd.com>
>
> No VCN DPM bit check as that's different from VCN PG. Also
> no extra check for possible double enablement/disablement
> as that's already done by VCN.
>
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> Reviewed-by: Kenneth Feng <kenneth.feng at amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 26 ++++++++--------------
>   1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 9dd96d8b8dd5..01d534c8442e 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -591,27 +591,19 @@ static int navi10_set_default_dpm_table(struct smu_context *smu)
>   static int navi10_dpm_set_uvd_enable(struct smu_context *smu, bool enable)
>   {
>   	int ret = 0;
> -	struct smu_power_context *smu_power = &smu->smu_power;
> -	struct smu_power_gate *power_gate = &smu_power->power_gate;
>   
> -	if (enable && power_gate->uvd_gated) {
> -		if (smu_feature_is_enabled(smu, SMU_FEATURE_DPM_UVD_BIT)) {
> -			ret = smu_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn, 1);
> -			if (ret)
> -				return ret;
> -		}
> -		power_gate->uvd_gated = false;
> +	if (enable) {
> +		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn, 1);
> +		if (ret)
> +			return ret;
>   	} else {
> -		if (!enable && !power_gate->uvd_gated) {
> -			if (smu_feature_is_enabled(smu, SMU_FEATURE_DPM_UVD_BIT)) {
> -				ret = smu_send_smc_msg(smu, SMU_MSG_PowerDownVcn);
> -				if (ret)
> -					return ret;
> -			}
> -			power_gate->uvd_gated = true;
> -		}
> +		ret = smu_send_smc_msg(smu, SMU_MSG_PowerDownVcn);
> +		if (ret)
> +			return ret;
>   	}
>   
> +	smu_feature_set_enabled(smu, SMU_FEATURE_VCN_PG_BIT, enable);

[kevin]:

you should check return value, this should not be based on some assumptions.

> +
>   	return 0;
>   }
>