RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug

2019-05-14 Thread Zhang, Hawking
Reviewed-by: Hawking Zhang 

Regards,
Hawking
_
From: Gui, Jack 
Sent: 2019年5月13日 11:32
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to 
support swSMU debug


 << File: 0001-drm-amd-powerplay-Enable-disable-dpm-feature-to-supp.patch >>
Hi Hawking,

V2 patch attached: Return 0 directly and merge some check code.

Please help review again, thanks.

BR,
Jack Gui

_
From: Zhang, Hawking 
Sent: Sunday, May 12, 2019 11:18 AM
To: Gui, Jack ; amd-gfx@lists.freedesktop.org
Cc: Gui, Jack 
Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to 
support swSMU debug


Please check my comments inline.

Regards,
Hawking
-Original Message-
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Chengming Gui
Sent: 2019年5月10日 16:49
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Gui, Jack mailto:jack@amd.com>>
Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support 
swSMU debug

[CAUTION: External Email]

add pm_enabled to control the dpm off/on.

Signed-off-by: Chengming Gui mailto:jack@amd.com>>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++---
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  1 +
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c  | 34 ++
 3 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 52d919a..99b2082 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void 
*buf, size_t size)
ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf;
int ret = 0;

+   if (!smu->pm_enabled)
+   return -EINVAL;
if (header->usStructureSize != size) {
pr_err("pp table size not matched !\n");
return -EIO;
@@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu)
int ret = 0;
uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32];

+   if (!smu->pm_enabled)
+   return ret;
mutex_lock(&feature->mutex);
bitmap_fill(feature->allowed, SMU_FEATURE_MAX);
mutex_unlock(&feature->mutex);
@@ -344,6 +348,7 @@ static int smu_early_init(void *handle)
struct smu_context *smu = &adev->smu;

smu->adev = adev;
+   smu->pm_enabled = amdgpu_dpm;
mutex_init(&smu->mutex);

return smu_set_funcs(adev);
@@ -353,6 +358,9 @@ static int smu_late_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;
+
+   if (!smu->pm_enabled)
+   return 0;
mutex_lock(&smu->mutex);
smu_handle_task(&adev->smu,
smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int 
smu_smc_table_hw_init(struct smu_context *smu,
 */
ret = smu_set_tool_table_location(smu);

+   if (!smu_is_dpm_running(smu))
+   pr_info("dpm has been disabled\n");
+
return ret;
 }

@@ -861,7 +872,10 @@ static int smu_hw_init(void *handle)

mutex_unlock(&smu->mutex);

-   adev->pm.dpm_enabled = true;
+   if (!smu->pm_enabled)
+   adev->pm.dpm_enabled = false;
+   else
+   adev->pm.dpm_enabled = true;

pr_info("SMU is initialized successfully!\n");

@@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle)
struct smu_table_context *table_context = &smu->smu_table;
int ret = 0;

+   if (!smu->pm_enabled)
+   return ret;
if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu)

 static int smu_suspend(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+   return ret;
[Hawking]: Just return 0 directly if dpm was disabled. Don't change the default 
return value to 0 which means resume complete successfully

if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -950,10 +968,12 @@ static int smu_suspend(void *handle)

 static int smu_resume(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+  

RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug

2019-05-12 Thread Gui, Jack

Hi Hawking,

V2 patch attached: Return 0 directly and merge some check code.

Please help review again, thanks.

BR,
Jack Gui

_
From: Zhang, Hawking 
Sent: Sunday, May 12, 2019 11:18 AM
To: Gui, Jack ; amd-gfx@lists.freedesktop.org
Cc: Gui, Jack 
Subject: RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to 
support swSMU debug


Please check my comments inline.

Regards,
Hawking
-Original Message-
From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Chengming Gui
Sent: 2019年5月10日 16:49
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Gui, Jack mailto:jack@amd.com>>
Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support 
swSMU debug

[CAUTION: External Email]

add pm_enabled to control the dpm off/on.

Signed-off-by: Chengming Gui mailto:jack@amd.com>>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++---
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  1 +
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c  | 34 ++
 3 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 52d919a..99b2082 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void 
*buf, size_t size)
ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf;
int ret = 0;

+   if (!smu->pm_enabled)
+   return -EINVAL;
if (header->usStructureSize != size) {
pr_err("pp table size not matched !\n");
return -EIO;
@@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu)
int ret = 0;
uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32];

+   if (!smu->pm_enabled)
+   return ret;
mutex_lock(&feature->mutex);
bitmap_fill(feature->allowed, SMU_FEATURE_MAX);
mutex_unlock(&feature->mutex);
@@ -344,6 +348,7 @@ static int smu_early_init(void *handle)
struct smu_context *smu = &adev->smu;

smu->adev = adev;
+   smu->pm_enabled = amdgpu_dpm;
mutex_init(&smu->mutex);

return smu_set_funcs(adev);
@@ -353,6 +358,9 @@ static int smu_late_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;
+
+   if (!smu->pm_enabled)
+   return 0;
mutex_lock(&smu->mutex);
smu_handle_task(&adev->smu,
smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int 
smu_smc_table_hw_init(struct smu_context *smu,
 */
ret = smu_set_tool_table_location(smu);

+   if (!smu_is_dpm_running(smu))
+   pr_info("dpm has been disabled\n");
+
return ret;
 }

@@ -861,7 +872,10 @@ static int smu_hw_init(void *handle)

mutex_unlock(&smu->mutex);

-   adev->pm.dpm_enabled = true;
+   if (!smu->pm_enabled)
+   adev->pm.dpm_enabled = false;
+   else
+   adev->pm.dpm_enabled = true;

pr_info("SMU is initialized successfully!\n");

@@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle)
struct smu_table_context *table_context = &smu->smu_table;
int ret = 0;

+   if (!smu->pm_enabled)
+   return ret;
if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu)

 static int smu_suspend(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+   return ret;
[Hawking]: Just return 0 directly if dpm was disabled. Don't change the default 
return value to 0 which means resume complete successfully

if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -950,10 +968,12 @@ static int smu_suspend(void *handle)

 static int smu_resume(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+   return ret;
[Hawking]: similar as suspend, directly return 0 if dpm was disabled.

if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -985,7 +1005,7 @@ int smu_display_configuration_change(struct smu_context 
*smu,
int index = 0;
int num_of_active_display = 0;

-   if (!is_support_sw_smu(smu->adev))
+   if (!smu->p

RE: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support swSMU debug

2019-05-11 Thread Zhang, Hawking
Please check my comments inline.

Regards,
Hawking
-Original Message-
From: amd-gfx  On Behalf Of Chengming Gui
Sent: 2019年5月10日 16:49
To: amd-gfx@lists.freedesktop.org
Cc: Gui, Jack 
Subject: [PATCH 1/2] drm/amd/powerplay: Enable "disable dpm" feature to support 
swSMU debug

[CAUTION: External Email]

add pm_enabled to control the dpm off/on.

Signed-off-by: Chengming Gui mailto:jack@amd.com>>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 34 +++---
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  1 +
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c  | 34 ++
 3 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 52d919a..99b2082 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -198,6 +198,8 @@ int smu_sys_set_pp_table(struct smu_context *smu,  void 
*buf, size_t size)
ATOM_COMMON_TABLE_HEADER *header = (ATOM_COMMON_TABLE_HEADER *)buf;
int ret = 0;

+   if (!smu->pm_enabled)
+   return -EINVAL;
if (header->usStructureSize != size) {
pr_err("pp table size not matched !\n");
return -EIO;
@@ -233,6 +235,8 @@ int smu_feature_init_dpm(struct smu_context *smu)
int ret = 0;
uint32_t unallowed_feature_mask[SMU_FEATURE_MAX/32];

+   if (!smu->pm_enabled)
+   return ret;
mutex_lock(&feature->mutex);
bitmap_fill(feature->allowed, SMU_FEATURE_MAX);
mutex_unlock(&feature->mutex);
@@ -344,6 +348,7 @@ static int smu_early_init(void *handle)
struct smu_context *smu = &adev->smu;

smu->adev = adev;
+   smu->pm_enabled = amdgpu_dpm;
mutex_init(&smu->mutex);

return smu_set_funcs(adev);
@@ -353,6 +358,9 @@ static int smu_late_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;
+
+   if (!smu->pm_enabled)
+   return 0;
mutex_lock(&smu->mutex);
smu_handle_task(&adev->smu,
smu->smu_dpm.dpm_level, @@ -746,6 +754,9 @@ static int 
smu_smc_table_hw_init(struct smu_context *smu,
 */
ret = smu_set_tool_table_location(smu);

+   if (!smu_is_dpm_running(smu))
+   pr_info("dpm has been disabled\n");
+
return ret;
 }

@@ -861,7 +872,10 @@ static int smu_hw_init(void *handle)

mutex_unlock(&smu->mutex);

-   adev->pm.dpm_enabled = true;
+   if (!smu->pm_enabled)
+   adev->pm.dpm_enabled = false;
+   else
+   adev->pm.dpm_enabled = true;

pr_info("SMU is initialized successfully!\n");

@@ -879,6 +893,8 @@ static int smu_hw_fini(void *handle)
struct smu_table_context *table_context = &smu->smu_table;
int ret = 0;

+   if (!smu->pm_enabled)
+   return ret;
if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -932,10 +948,12 @@ int smu_reset(struct smu_context *smu)

 static int smu_suspend(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+   return ret;
[Hawking]: Just return 0 directly if dpm was disabled. Don't change the default 
return value to 0 which means resume complete successfully

if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -950,10 +968,12 @@ static int smu_suspend(void *handle)

 static int smu_resume(void *handle)
 {
-   int ret;
+   int ret = 0;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = &adev->smu;

+   if (!smu->pm_enabled)
+   return ret;
[Hawking]: similar as suspend, directly return 0 if dpm was disabled.

if (!is_support_sw_smu(adev))
return -EINVAL;

@@ -985,7 +1005,7 @@ int smu_display_configuration_change(struct smu_context 
*smu,
int index = 0;
int num_of_active_display = 0;

-   if (!is_support_sw_smu(smu->adev))
+   if (!smu->pm_enabled || !is_support_sw_smu(smu->adev))
return -EINVAL;

if (!display_config)
@@ -1113,6 +1133,8 @@ static int smu_enable_umd_pstate(void *handle,

struct smu_context *smu = (struct smu_context*)(handle);
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
+   if (!smu->pm_enabled)
+   return -EINVAL;
[Hawking]: please merge this with the dpm_context check

if (!smu_dpm_ctx->dpm_context)
return -EINVAL;

@@ -1156,6 +1178,8 @@ int smu_adjust_power_state_dynamic(struct smu_context 
*smu,
long workload;
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);

+   if (!smu->pm_enabled)
+   return