Re: [PATCH AUTOSEL 6.18] drm/amd/pm: derive stable PPT limits from PPTable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/amd/pm: aldebaran_get_power_limit() sets non-zero default on secondary die
--

commit e12c0e27cabe98103b51eb24b3ca4cb7b2a3ede8
Author: Yang Wang <kevinyang.wang@amd.com>

drm/amd/pm: derive stable PPT limits from PPTable

This commit updates the power limit query handlers for Sienna Cichlid,
Navi10, Arcturus, and Aldebaran to derive platform default, minimum, and
maximum PPT limits from the driver PPTable rather than mutable PMFW
runtime state.

> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 18d5d07045098..9e67e57d70b75 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1281,8 +1281,12 @@ static int aldebaran_get_power_limit(struct smu_context *smu,
>  
>  	if (current_power_limit)
>  		*current_power_limit = power_limit;
> -	if (default_power_limit)
> -		*default_power_limit = power_limit;
> +	if (default_power_limit) {
> +		if (pptable)
> +			*default_power_limit = pptable->PptLimit;
> +		else
> +			*default_power_limit = power_limit;
> +	}

[Severity: Medium]
On Aldebaran multi-die GPUs, could this cause the secondary die to report a
non-zero default power limit in hwmon (power1_cap_default)?

Earlier in aldebaran_get_power_limit(), both the SMU_MSG_GetPptLimit query
and the pptable->PptLimit fallback are guarded by aldebaran_is_primary(smu)
(from commit 076f55a45e9e ("drm/amd/pm: Only primary die supports power
data")):

	/* Valid power data is available only from primary die.
	 * For secondary die show the value as 0.
	 */
	if (aldebaran_is_primary(smu)) {
		ret = smu_cmn_send_smc_msg(smu, SMU_MSG_GetPptLimit,
					   &power_limit);

		if (ret) {
			/* the last hope to figure out the ppt limit */
			if (!pptable) {
				dev_err(smu->adev->dev,
					"Cannot get PPT limit due to pptable missing!");
				return -EINVAL;
			}
			power_limit = pptable->PptLimit;
		}
	}

Similarly, aldebaran_set_power_limit() rejects changes on the secondary
die:

	/* Power limit can be set only through primary die */
	if (aldebaran_is_primary(smu))
		return smu_v13_0_set_power_limit(smu, limit_type, limit);

	return -EINVAL;

Because smu->smu_table.driver_pptable is allocated and populated on all
dies via aldebaran_init_smc_tables() -> smu_v13_0_init_smc_tables() and
aldebaran_store_powerplay_table(), pptable is non-NULL on the secondary
die.

When smu_late_init() -> smu_get_asic_power_limits() calls
aldebaran_get_power_limit(), *default_power_limit was previously set to
power_limit (0 on the secondary die), whereas now it will be set to
pptable->PptLimit without checking aldebaran_is_primary(smu). Should the
pptable->PptLimit assignment for default_power_limit also be gated on
aldebaran_is_primary(smu)?

>  
>  	if (max_power_limit) {
>  		if (pptable)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-82-sashal@kernel.org?part=1