Re: [PATCH 3/3] drm/amd/pm: explicitly initialize cached power limits in smu struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[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,
>

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux