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 <[email protected]> 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/[email protected]?part=1
