Re: [PATCH] drm/amd/pm: update smartshift powerboost calc for smu13

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 5/11/2022 5:16 PM, Sathishkumar S wrote:
> smartshift apu and dgpu power boost are reported as percentage
> with respect to their power limits. adjust the units of power before
> calculating the percentage of boost.
> 
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju@amd.com>
> ---
>   .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  | 60 ++++++++++++++-----
>   1 file changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> index e2d099409123..35fbeb72c05c 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
> @@ -276,6 +276,40 @@ static int yellow_carp_mode2_reset(struct smu_context *smu)
>   	return yellow_carp_mode_reset(smu, SMU_RESET_MODE_2);
>   }
>   
> +
> +static void yellow_carp_get_ss_power_percent(SmuMetrics_t *metrics,
> +					uint32_t *apu_percent, uint32_t *dgpu_percent)
> +{
> +	uint32_t apu_boost = 0;
> +	uint32_t dgpu_boost = 0;
> +	uint16_t apu_limit = 0;
> +	uint16_t dgpu_limit = 0;
> +	uint16_t apu_power = 0;
> +	uint16_t dgpu_power = 0;
> +
> +	apu_power = metrics->ApuPower/1000;
> +	apu_limit = metrics->StapmOpnLimit;
> +	if (apu_power > apu_limit && apu_limit != 0)
> +		apu_boost =  ((apu_power - apu_limit) * 100) / apu_limit;
> +	apu_boost = (apu_boost > 100) ? 100 : apu_boost;
> +
> +	dgpu_power = metrics->dGpuPower/1000;

Before submitting (not expecting another version here), may add a 
comment that APU/dGPU power values are reported in milli Watts and STAPM 
power limits in Watts.

Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo

> +	if (metrics->StapmCurrentLimit > metrics->StapmOpnLimit)
> +		dgpu_limit = metrics->StapmCurrentLimit - metrics->StapmOpnLimit;
> +	if (dgpu_power > dgpu_limit && dgpu_limit != 0)
> +		dgpu_boost = ((dgpu_power - dgpu_limit) * 100) / dgpu_limit;
> +	dgpu_boost = (dgpu_boost > 100) ? 100 : dgpu_boost;
> +
> +	if (dgpu_boost >= apu_boost)
> +		apu_boost = 0;
> +	else
> +		dgpu_boost = 0;
> +
> +	*apu_percent = apu_boost;
> +	*dgpu_percent = dgpu_boost;
> +
> +}
> +
>   static int yellow_carp_get_smu_metrics_data(struct smu_context *smu,
>   							MetricsMember_t member,
>   							uint32_t *value)
> @@ -284,6 +318,8 @@ static int yellow_carp_get_smu_metrics_data(struct smu_context *smu,
>   
>   	SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table->metrics_table;
>   	int ret = 0;
> +	uint32_t apu_percent = 0;
> +	uint32_t dgpu_percent = 0;
>   
>   	ret = smu_cmn_get_metrics_table(smu, NULL, false);
>   	if (ret)
> @@ -332,26 +368,18 @@ static int yellow_carp_get_smu_metrics_data(struct smu_context *smu,
>   		*value = metrics->Voltage[1];
>   		break;
>   	case METRICS_SS_APU_SHARE:
> -		/* return the percentage of APU power with respect to APU's power limit.
> -		 * percentage is reported, this isn't boost value. Smartshift power
> -		 * boost/shift is only when the percentage is more than 100.
> +		/* return the percentage of APU power boost
> +		 * with respect to APU's power limit.
>   		 */
> -		if (metrics->StapmOpnLimit > 0)
> -			*value =  (metrics->ApuPower * 100) / metrics->StapmOpnLimit;
> -		else
> -			*value = 0;
> +		yellow_carp_get_ss_power_percent(metrics, &apu_percent, &dgpu_percent);
> +		*value = apu_percent;
>   		break;
>   	case METRICS_SS_DGPU_SHARE:
> -		/* return the percentage of dGPU power with respect to dGPU's power limit.
> -		 * percentage is reported, this isn't boost value. Smartshift power
> -		 * boost/shift is only when the percentage is more than 100.
> +		/* return the percentage of dGPU power boost
> +		 * with respect to dGPU's power limit.
>   		 */
> -		if ((metrics->dGpuPower > 0) &&
> -		    (metrics->StapmCurrentLimit > metrics->StapmOpnLimit))
> -			*value = (metrics->dGpuPower * 100) /
> -				  (metrics->StapmCurrentLimit - metrics->StapmOpnLimit);
> -		else
> -			*value = 0;
> +		yellow_carp_get_ss_power_percent(metrics, &apu_percent, &dgpu_percent);
> +		*value = dgpu_percent;
>   		break;
>   	default:
>   		*value = UINT_MAX;
>