Re: [PATCH v2] drm/amd/pm: Remove redundant and incorrect multi-vf check

[Lazar, Lijo]

On 04-Feb-26 3:46 PM, Lazar, Lijo wrote:


On 04-Feb-26 3:35 PM, Wang, Yang(Kevin) wrote:
[AMD Official Use Only - AMD Internal Distribution Only]

Remove redundant and incorrect multi-vf check for pp clock setting, as
the code path will never be taken. Mask check take care of same.

I suspect there might be a misunderstanding of the code logic here. 
This is not redundant code.
Currently, AMDGPU does not support the multi-vf case, so no sysfs node 
will be created for it.
However, in one-vf mode, for security reasons, setting frequency 
information from the vf side is not allowed.

/* setting should not be allowed from VF if not in one VF mode */

This comment is not inline to this.

The logic before this change "drm/amd/pm: centralize all pp_dpm_xxx 
attribute nodes update cb") also supported set frequency  in 1VF mode.


-       /* setting should not be allowed from VF if not in one VF mode */
-       if (amdgpu_sriov_vf(adev) && (!amdgpu_sriov_is_pp_one_vf(adev) ||
-               DEVICE_ATTR_IS(pp_dpm_sclk) ||
-               DEVICE_ATTR_IS(pp_dpm_mclk) ||
-               DEVICE_ATTR_IS(pp_dpm_socclk) ||
-               DEVICE_ATTR_IS(pp_dpm_fclk) ||
-               DEVICE_ATTR_IS(pp_dpm_vclk) ||
-               DEVICE_ATTR_IS(pp_dpm_vclk1) ||
-               DEVICE_ATTR_IS(pp_dpm_dclk) ||
-               DEVICE_ATTR_IS(pp_dpm_dclk1))) {
-               dev_attr->attr.mode &= ~S_IWUGO;
-               dev_attr->store = NULL;


Ignore this. It is "(!amdgpu_sriov_is_pp_one_vf(adev) ||". Set is not 
allowed for these clocks in VF mode.

Thanks,
Lijo


Thanks,
Lijo

Therefore, the .store interface is removed and the node permissions 
are configured as read-only.
In conclusion, this segment of code serves a clear purpose and is by 
no means redundant.

Best Regards,
Kevin
-----Original Message-----
From: Lazar, Lijo <lijo.la...@amd.com>
Sent: Wednesday, February 4, 2026 5:31 PM
To: Kamal, Asad <asad.ka...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <hawking.zh...@amd.com>; Ma, Le <le...@amd.com>; 
Zhang, Morris <shiwu.zh...@amd.com>; Deucher, Alexander 
<alexander.deuc...@amd.com>; Wang, Yang(Kevin) <kevinyang.w...@amd.com>
Subject: Re: [PATCH v2] drm/amd/pm: Remove redundant and incorrect 
multi-vf check



On 04-Feb-26 1:55 PM, Asad Kamal wrote:
Remove redundant and incorrect multi-vf check for pp clock setting, as
the code path will never be taken. Mask check take care of same.

v2: Update patch title, Remove the check (Kevin)

Fixes: 166a3c735c95 ("drm/amd/pm: centralize all pp_dpm_xxx attribute
nodes update cb")

Signed-off-by: Asad Kamal <asad.ka...@amd.com>

You may drop any redundant multivf check in other attr_update calls also.

Thanks,
Lijo

---
   drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 ------
   1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 07641c9413d2..81bef5c5aae9 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2057,12 +2057,6 @@ static int 
pp_dpm_clk_default_attr_update(struct amdgpu_device *adev, struct amd
               break;
       }

-     /* setting should not be allowed from VF if not in one VF mode */
-     if (amdgpu_sriov_vf(adev) && amdgpu_sriov_is_pp_one_vf(adev)) {
-             dev_attr->attr.mode &= ~S_IWUGO;
-             dev_attr->store = NULL;
-     }
-
       return 0;
   }