On 10/5/2021 10:34 AM, Powell, Darren wrote:
[AMD Official Use Only]
I'm just looking to clarify this code. The macro eventually expands to
look like this
if ((smu)->ppt_funcs)
{
if ((smu)->ppt_funcs->get_power_limit)
(smu)->ppt_funcs->get_power_limit(smu,
&smu->current_power_limit,
&smu->default_power_limit,
&smu->max_power_limit);
else
return 0;
}
else
return -EINVAL;
But you have to dig to realize that it's a macro, and that it makes no
modification if the function is not defined. It's not clear without then
searching and following the function pointers which platforms are using
the saved value. I thought of inserting the following comment or should
I just drop this altogether?
Yes. The limit values are by default initialized to 0. If the function
is not supported, it's not considered an error to fail late_init (it
only reports 0 values in hwmon).
late_init also gets called on other occasions like resume or after a
GPU-only reset. If you want to change the invalid limit value in hwmon
from 0 to something else (to differentiate unsupported vs API returning
0 value), then better to do in sw_init.
Thanks,
Lijo
/* seed the cached smu power limit values iff get_power_limit is
defined, otherwise they remain 0 */
Thanks
Darren
------------------------------------------------------------------------
*From:* Lazar, Lijo <Lijo.Lazar@xxxxxxx>
*Sent:* Monday, October 4, 2021 6:43 AM
*To:* Powell, Darren <Darren.Powell@xxxxxxx>;
amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
*Subject:* Re: [PATCH 3/3] drm/amd/pm: explicitly initialize cached
power limits in smu struct
On 10/3/2021 10:16 AM, Darren Powell wrote:
Code appears to initialize values but macro will exit without error
or initializing value if function is not implmented
Signed-off-by: Darren Powell <darren.powell@xxxxxxx>
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index faa78a048b1f..210f047e136d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -712,6 +712,10 @@ static int smu_late_init(void *handle)
return ret;
}
+ smu->current_power_limit = 0;
+ smu->default_power_limit = 0;
+ smu->max_power_limit = 0;
+
If this is only about first-time init - smu_context is part of adev, it
will be zero initialized when adev is allocated.
Thanks,
Lijo
ret = smu_get_asic_power_limits(smu,
&smu->current_power_limit,
&smu->default_power_limit,
