|
[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?
/* 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, > |
