Hi Kevin, On 10/30/2023 2:23 PM, Wang, Yang(Kevin) wrote: > [AMD Official Use Only - General] > > The driver already has similar code in hwmon_attributes_visible(), > So, what issue you have now ? > > /* under multi-vf mode, the hwmon attributes are all not supported */ > if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) > This function is called for each attribute now, so the vf flags will also be checked multiple times. So move this code to amdgpu_pm_sysfs_init so that the vf flags will only be checked once. Regards, Ma Jun return 0; > > Best Regards, > Kevin > > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ma, Jun > Sent: Monday, October 30, 2023 2:10 PM > To: Ma, Jun <Jun.Ma2@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Feng, Kenneth <Kenneth.Feng@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Subject: Re: [PATCH Resend] drm/amd/pm: only check sriov vf flag once when creating hwmon sysfs > > ping... > > On 10/26/2023 10:50 AM, Ma Jun wrote: >> The current code checks sriov vf flag multiple times when creating >> hwmon sysfs. So fix it. >> >> Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx> >> --- >> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 27 ++++++++++++++------------- >> 1 file changed, 14 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c >> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c >> index 358bb5e485f2..ee46d04549e6 100644 >> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c >> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c >> @@ -3288,10 +3288,6 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj, >> uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0); >> uint32_t tmp; >> >> - /* under multi-vf mode, the hwmon attributes are all not supported */ >> - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) >> - return 0; >> - >> /* under pp one vf mode manage of hwmon attributes is not supported */ >> if (amdgpu_sriov_is_pp_one_vf(adev)) >> effective_mode &= ~S_IWUSR; >> @@ -4162,6 +4158,7 @@ static int amdgpu_od_set_init(struct >> amdgpu_device *adev) >> >> int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) { >> + enum amdgpu_sriov_vf_mode mode; >> uint32_t mask = 0; >> int ret; >> >> @@ -4173,17 +4170,21 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) >> if (adev->pm.dpm_enabled == 0) >> return 0; >> >> - adev->pm.int_hwmon_dev = hwmon_device_register_with_groups(adev->dev, >> - DRIVER_NAME, adev, >> - hwmon_groups); >> - if (IS_ERR(adev->pm.int_hwmon_dev)) { >> - ret = PTR_ERR(adev->pm.int_hwmon_dev); >> - dev_err(adev->dev, >> - "Unable to register hwmon device: %d\n", ret); >> - return ret; >> + mode = amdgpu_virt_get_sriov_vf_mode(adev); >> + >> + /* under multi-vf mode, the hwmon attributes are all not supported */ >> + if (mode != SRIOV_VF_MODE_MULTI_VF) { >> + adev->pm.int_hwmon_dev = hwmon_device_register_with_groups(adev->dev, >> + DRIVER_NAME, adev, >> + hwmon_groups); >> + if (IS_ERR(adev->pm.int_hwmon_dev)) { >> + ret = PTR_ERR(adev->pm.int_hwmon_dev); >> + dev_err(adev->dev, "Unable to register hwmon device: %d\n", ret); >> + return ret; >> + } >> } >> >> - switch (amdgpu_virt_get_sriov_vf_mode(adev)) { >> + switch (mode) { >> case SRIOV_VF_MODE_ONE_VF: >> mask = ATTR_FLAG_ONEVF; >> break;