All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function
@ 2021-01-20  3:48 Xiaojian Du
  2021-01-20  4:10 ` Wang, Kevin(Yang)
  0 siblings, 1 reply; 5+ messages in thread
From: Xiaojian Du @ 2021-01-20  3:48 UTC (permalink / raw)
  To: amd-gfx; +Cc: lijo.lazar, kevin1.wang, ray.huang, Xiaojian Du, evan.quan

From: Xiaojian Du <xiaojian.du@amd.com>

From: Xiaojian Du <Xiaojian.Du@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@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");
 		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;
 	}
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function
  2021-01-20  3:48 [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function Xiaojian Du
@ 2021-01-20  4:10 ` Wang, Kevin(Yang)
  2021-02-09  4:10   ` Huang, Ray
  0 siblings, 1 reply; 5+ messages in thread
From: Wang, Kevin(Yang) @ 2021-01-20  4:10 UTC (permalink / raw)
  To: Du, Xiaojian, amd-gfx; +Cc: Lazar, Lijo, Huang, Ray, Quan, Evan


[-- Attachment #1.1: Type: text/plain, Size: 3892 bytes --]

[AMD Official Use Only - Internal Distribution Only]


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

From: Xiaojian Du <xiaojian.du@amd.com>

From: Xiaojian Du <Xiaojian.Du@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@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


[-- Attachment #1.2: Type: text/html, Size: 8854 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function
  2021-01-20  4:10 ` Wang, Kevin(Yang)
@ 2021-02-09  4:10   ` Huang, Ray
  2021-02-09  5:27     ` Wang, Kevin(Yang)
  0 siblings, 1 reply; 5+ messages in thread
From: Huang, Ray @ 2021-02-09  4:10 UTC (permalink / raw)
  To: Wang, Kevin(Yang), Du, Xiaojian, amd-gfx, Deucher, Alexander
  Cc: Lazar, Lijo, Quan, Evan


[-- Attachment #1.1: Type: text/plain, Size: 5601 bytes --]

[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@amd.com<mailto:ray.huang@amd.com>>

Thanks,
Ray

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

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

From: Xiaojian Du <Xiaojian.Du@amd.com<mailto:Xiaojian.Du@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@amd.com<mailto:Xiaojian.Du@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

[-- Attachment #1.2: Type: text/html, Size: 12351 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function
  2021-02-09  4:10   ` Huang, Ray
@ 2021-02-09  5:27     ` Wang, Kevin(Yang)
  2021-02-09 17:05       ` Deucher, Alexander
  0 siblings, 1 reply; 5+ messages in thread
From: Wang, Kevin(Yang) @ 2021-02-09  5:27 UTC (permalink / raw)
  To: Huang, Ray, Du, Xiaojian, amd-gfx, Deucher, Alexander
  Cc: Lazar, Lijo, Quan, Evan


[-- Attachment #1.1: Type: text/plain, Size: 6445 bytes --]

[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@amd.com>

Best Regards,
Kevin
________________________________
From: Huang, Ray <Ray.Huang@amd.com>
Sent: Tuesday, February 9, 2021 12:10 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; Lazar, Lijo <Lijo.Lazar@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@amd.com<mailto:ray.huang@amd.com>>



Thanks,

Ray



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



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

From: Xiaojian Du <Xiaojian.Du@amd.com<mailto:Xiaojian.Du@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@amd.com<mailto:Xiaojian.Du@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

[-- Attachment #1.2: Type: text/html, Size: 14689 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function
  2021-02-09  5:27     ` Wang, Kevin(Yang)
@ 2021-02-09 17:05       ` Deucher, Alexander
  0 siblings, 0 replies; 5+ messages in thread
From: Deucher, Alexander @ 2021-02-09 17:05 UTC (permalink / raw)
  To: Wang, Kevin(Yang), Huang, Ray, Du, Xiaojian, amd-gfx
  Cc: Lazar, Lijo, Quan, Evan


[-- Attachment #1.1: Type: text/plain, Size: 7056 bytes --]

[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@amd.com>
Sent: Tuesday, February 9, 2021 12:27 AM
To: Huang, Ray <Ray.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; Lazar, Lijo <Lijo.Lazar@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@amd.com>

Best Regards,
Kevin
________________________________
From: Huang, Ray <Ray.Huang@amd.com>
Sent: Tuesday, February 9, 2021 12:10 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Quan, Evan <Evan.Quan@amd.com>; Lazar, Lijo <Lijo.Lazar@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@amd.com<mailto:ray.huang@amd.com>>



Thanks,

Ray



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



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

From: Xiaojian Du <Xiaojian.Du@amd.com<mailto:Xiaojian.Du@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@amd.com<mailto:Xiaojian.Du@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

[-- Attachment #1.2: Type: text/html, Size: 16420 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-02-09 17:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20  3:48 [PATCH] drm/amd/pm: make the error log more clear for fine grain tuning function Xiaojian Du
2021-01-20  4:10 ` Wang, Kevin(Yang)
2021-02-09  4:10   ` Huang, Ray
2021-02-09  5:27     ` Wang, Kevin(Yang)
2021-02-09 17:05       ` Deucher, Alexander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.