[PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function

Deucher, Alexander Alexander.Deucher at amd.com
Tue Feb 9 17:05:54 UTC 2021


[AMD Official Use Only - Internal Distribution Only]

We should try and the behavior as consistent as possible.

Thanks!

Alex

________________________________
From: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Sent: Tuesday, February 9, 2021 12:27 AM
To: Huang, Ray <Ray.Huang at amd.com>; Du, Xiaojian <Xiaojian.Du at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher at amd.com>
Cc: Quan, Evan <Evan.Quan at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>
Subject: Re: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function


[AMD Official Use Only - Internal Distribution Only]

If it is not in manual mode, we should give the user a hint.
if possible, we should move this check into amdgpu_pm.c , it is also can be used swsmu backend.

the patch is
Acked-by: Kevin Wang <kevin1.wang at amd.com>

Best Regards,
Kevin
________________________________
From: Huang, Ray <Ray.Huang at amd.com>
Sent: Tuesday, February 9, 2021 12:10 PM
To: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>; Du, Xiaojian <Xiaojian.Du at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher at amd.com>
Cc: Quan, Evan <Evan.Quan at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>
Subject: RE: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function


[AMD Official Use Only - Internal Distribution Only]


+ Alex



Hi all,



Recently, many users reported the issue to us that fine grain not enabled. Actually, most of them are just caused by not switching to “manual” mode.



         if (!(smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)) {
-               dev_warn(smu->adev->dev, "Fine grain is not enabled!\n");
+               dev_warn(smu->adev->dev,
+                       "pp_od_clk_voltage is not accessible if power_dpm_force_perfomance_level is not in manual mode!\n");



We have to need reminder in the warning message to tell the user where they are.



Any objection for this patch? I found Navi series actually didn’t need this operation to update max/min clock levels. Would you clarify whether dGPU still needs this before we move the prints into amdgpu_pm.c?



However, in APU fine grain design, patch looks good for me.

Acked-by: Huang Rui <ray.huang at amd.com<mailto:ray.huang at amd.com>>



Thanks,

Ray



From: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
Sent: Wednesday, January 20, 2021 12:10 PM
To: Du, Xiaojian <Xiaojian.Du at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Huang, Ray <Ray.Huang at amd.com>; Quan, Evan <Evan.Quan at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>
Subject: Re: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function



[AMD Official Use Only - Internal Distribution Only]





________________________________

From: Du, Xiaojian <Xiaojian.Du at amd.com<mailto:Xiaojian.Du at amd.com>>
Sent: Wednesday, January 20, 2021 11:48 AM
To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>>
Cc: Huang, Ray <Ray.Huang at amd.com<mailto:Ray.Huang at amd.com>>; Quan, Evan <Evan.Quan at amd.com<mailto:Evan.Quan at amd.com>>; Wang, Kevin(Yang) <Kevin1.Wang at amd.com<mailto:Kevin1.Wang at amd.com>>; Lazar, Lijo <Lijo.Lazar at amd.com<mailto:Lijo.Lazar at amd.com>>; Du, Xiaojian <Xiaojian.Du at amd.com<mailto:Xiaojian.Du at amd.com>>; Du, Xiaojian <Xiaojian.Du at amd.com<mailto:Xiaojian.Du at amd.com>>
Subject: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function



From: Xiaojian Du <xiaojian.du at amd.com<mailto:xiaojian.du at amd.com>>

From: Xiaojian Du <Xiaojian.Du at amd.com<mailto:Xiaojian.Du at amd.com>>

This patch is to make the error log more clear for fine grian tuning
function, it covers Raven/Raven2/Picasso/Renoir/Vangogh.
The fine grain tuning function uses the sysfs file -- pp_od_clk_voltage,
but only when another sysfs file -- power_dpm_force_performance_level is
switched to "manual" mode, it is allowd to access "pp_od_clk_voltage".

Signed-off-by: Xiaojian Du <Xiaojian.Du at amd.com<mailto:Xiaojian.Du at amd.com>>
---
 drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 2 +-
 drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c     | 3 ++-
 drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c      | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index 88322781e447..ed05a30d1139 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1487,7 +1487,7 @@ static int smu10_set_fine_grain_clk_vol(struct pp_hwmgr *hwmgr,
         }

         if (!smu10_data->fine_grain_enabled) {
-               pr_err("Fine grain not started\n");
+               pr_err("pp_od_clk_voltage is not accessible if power_dpm_force_perfomance_level is not in manual mode!\n");

[kevin]:

for above codes, the old one looks better for me, i prefer to keep current design.



Best Regards,

Kevin
                 return -EINVAL;
         }

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
index 6d3c556dbe6b..a847fa66797e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
@@ -1452,7 +1452,8 @@ static int vangogh_od_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_TAB
         struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);

         if (!(smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)) {
-               dev_warn(smu->adev->dev, "Fine grain is not enabled!\n");
+               dev_warn(smu->adev->dev,
+                       "pp_od_clk_voltage is not accessible if power_dpm_force_perfomance_level is not in manual mode!\n");
                 return -EINVAL;
         }

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
index ab15570305f7..4ce8fb1d5ce9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
@@ -350,7 +350,8 @@ static int renoir_od_edit_dpm_table(struct smu_context *smu,
         struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);

         if (!(smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL)) {
-               dev_warn(smu->adev->dev, "Fine grain is not enabled!\n");
+               dev_warn(smu->adev->dev,
+                       "pp_od_clk_voltage is not accessible if power_dpm_force_perfomance_level is not in manual mode!\n");
                 return -EINVAL;

[Kevin]:

Just tell the User what's going on, not why.

and we'd better make a function to check manual mode , then embed it to every sysfs node in amdgpu_pm.c

using a unify interface to return result to user.



Best Regards,

Kevin
         }

--
2.17.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210209/7aa0ea44/attachment-0001.htm>


More information about the amd-gfx mailing list