Re: [PATCH RESEND] drm/amd/pm: Use guard(mutex) instead of manual lock+unlock

[Christian König <christian.koenig@amd.com>]

Just two notes, apart from that looks like a nice cleanup to me.

On 5/8/26 04:47, Andre Hirata wrote:
> @@ -46,10 +47,9 @@ int amdgpu_dpm_get_sclk(struct amdgpu_device *adev, bool low)
>         if (!pp_funcs->get_sclk)
>                 return 0;
> 
> -       mutex_lock(&adev->pm.mutex);
> +       guard(mutex)(&adev->pm.mutex);
>         ret = pp_funcs->get_sclk((adev)->powerplay.pp_handle,
>                                  low);
> -       mutex_unlock(&adev->pm.mutex);
> 
>         return ret;

In a lot of cases you can now turn the patter "ret = f(...); return ret;" into just return f(..); and potentially drop the ret variable.

>  }
> 
> @@ -291,9 +265,8 @@ bool amdgpu_dpm_is_mode1_reset_supported(struct amdgpu_device *adev)
>         bool support_mode1_reset = false;
> 
>         if (is_support_sw_smu(adev)) {
> -               mutex_lock(&adev->pm.mutex);
> +               guard(mutex)(&adev->pm.mutex);
>                 support_mode1_reset = smu_mode1_reset_is_support(smu);
> -               mutex_unlock(&adev->pm.mutex);
>         }
> 
>         return support_mode1_reset;

For cases like this here the coding pattern to check the pre-requisites and abort early is usually better.

So this example here would become:

if (!is_support_sw_smu(adev))
	return false;

guard(mutex)(&adev->pm.mutex);
return smu_mode1_reset_is_support(smu);

Which as far as I can see is less code and much easier to read/understand.

But both suggestions could be a separate patch if you want to keep this one as it is.

Regards,
Christian.