RE: [PATCH 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20 power functions.
Hi Jack,
Let's hold on patch #2 for now. I'd prefer to gracefully deal with dpm_disabled
in amdgpu_pm interface level (or at least amdgpu_smu level) so that we don't
need to maintain the case for each ASIC since ppt is asic specific ones.
Regards,
Hawking
-Original Message-
From: Gui, Jack
Sent: 2019年5月13日 11:26
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 2/2] drm/amd/powerplay: Enable "disable dpm" feature for
vega20 power functions.
Hi Hawking,
If no other tools (sysfs inferfaces have been controlled by adev->dpm_enabled)
directly use the ppt functs, PATCH[1/2] can cover all the cases now, PATCH[2/2]
can be dropped.
BR,
Jack Gui
-Original Message-
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 2/2] drm/amd/powerplay: Enable "disable dpm" feature for
vega20 power functions.
I'm wondering whether we are able to handle all asic specific ppt function
gracefully. Add pm_enable check into ppt functions would be boring since we
have to do similar jobs for each ASIC. Instead, is it possible to return early
from amdgpu_smu interface level?
Take smu_handle_task for the example, all the amdgpu_smu level interface
invoked from this function could be stopped if we check smu pm_enable flag at
the beginning of smu_handle_task. We probably have better approach to eliminate
this patch.
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 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20
power functions.
[CAUTION: External Email]
use pm_enabled to control all power related functions about vega20.
Signed-off-by: Chengming Gui
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 48 +++---
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 8fafcbd..7faa6a1 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -411,7 +411,8 @@ amd_pm_state_type vega20_get_current_power_state(struct
smu_context *smu)
enum amd_pm_state_type pm_type;
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
- if (!smu_dpm_ctx->dpm_context ||
+ if (!smu->pm_enabled ||
+ !smu_dpm_ctx->dpm_context ||
!smu_dpm_ctx->dpm_current_power_state)
return -EINVAL;
@@ -491,12 +492,14 @@ static void vega20_init_single_dpm_state(struct
vega20_dpm_state *dpm_state)
static int vega20_set_default_dpm_table(struct smu_context *smu) {
- int ret;
+ int ret = 0;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct vega20_dpm_table *dpm_table = NULL;
struct vega20_single_dpm_table *single_dpm_table;
+ if (smu->pm_enabled)
+ return ret;
dpm_table = smu_dpm->dpm_context;
/* socclk */
@@ -738,6 +741,8 @@ static int vega20_print_clk_levels(struct smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
switch (type) {
case PP_SCLK:
ret = smu_get_current_clk_freq(smu, PPCLK_GFXCLK, &now); @@
-969,6 +974,8 @@ static int vega20_upload_dpm_level(struct smu_context *smu,
bool max,
uint32_t freq;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
dpm_table = smu->smu_dpm.dpm_context;
if (smu_feature_is_enabled(smu, FEATURE_DPM_GFXCLK_BIT) && @@ -1058,6
+1065,8 @@ static int vega20_force_clk_levels(struct smu_context *smu,
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
if (smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) {
pr_info("force clock level is for dpm manual mode only.\n");
return -EINVAL;
@@ -1232,8 +1241,9 @@ static int vega20_get_clock_by_type_with_latency(struct
smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
mutex_lock(&smu->mutex);
-
switch (type) {
case amd_pp_sys_clock:
single_dpm_table = &(dpm_table->gfx_table); @@ -1265,6 +1275,8
@@ static int vega20_overdrive_get_gfx_clk_base_voltage(struct smu_context
*smu, {
int ret;
+ if (!smu->pm_enabled)
+ return -EINVAL;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_GetAVFSVoltageByDpm,
((AVFS_CURVE << 24) | (OD8_HOTCURVE_TEMPERATURE << 16
RE: [PATCH 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20 power functions.
Hi Hawking,
If no other tools (sysfs inferfaces have been controlled by adev->dpm_enabled)
directly use the ppt functs, PATCH[1/2] can cover all the cases now, PATCH[2/2]
can be dropped.
BR,
Jack Gui
-Original Message-
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 2/2] drm/amd/powerplay: Enable "disable dpm" feature for
vega20 power functions.
I'm wondering whether we are able to handle all asic specific ppt function
gracefully. Add pm_enable check into ppt functions would be boring since we
have to do similar jobs for each ASIC. Instead, is it possible to return early
from amdgpu_smu interface level?
Take smu_handle_task for the example, all the amdgpu_smu level interface
invoked from this function could be stopped if we check smu pm_enable flag at
the beginning of smu_handle_task. We probably have better approach to eliminate
this patch.
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 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20
power functions.
[CAUTION: External Email]
use pm_enabled to control all power related functions about vega20.
Signed-off-by: Chengming Gui
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 48 +++---
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 8fafcbd..7faa6a1 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -411,7 +411,8 @@ amd_pm_state_type vega20_get_current_power_state(struct
smu_context *smu)
enum amd_pm_state_type pm_type;
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
- if (!smu_dpm_ctx->dpm_context ||
+ if (!smu->pm_enabled ||
+ !smu_dpm_ctx->dpm_context ||
!smu_dpm_ctx->dpm_current_power_state)
return -EINVAL;
@@ -491,12 +492,14 @@ static void vega20_init_single_dpm_state(struct
vega20_dpm_state *dpm_state)
static int vega20_set_default_dpm_table(struct smu_context *smu) {
- int ret;
+ int ret = 0;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct vega20_dpm_table *dpm_table = NULL;
struct vega20_single_dpm_table *single_dpm_table;
+ if (smu->pm_enabled)
+ return ret;
dpm_table = smu_dpm->dpm_context;
/* socclk */
@@ -738,6 +741,8 @@ static int vega20_print_clk_levels(struct smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
switch (type) {
case PP_SCLK:
ret = smu_get_current_clk_freq(smu, PPCLK_GFXCLK, &now); @@
-969,6 +974,8 @@ static int vega20_upload_dpm_level(struct smu_context *smu,
bool max,
uint32_t freq;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
dpm_table = smu->smu_dpm.dpm_context;
if (smu_feature_is_enabled(smu, FEATURE_DPM_GFXCLK_BIT) && @@ -1058,6
+1065,8 @@ static int vega20_force_clk_levels(struct smu_context *smu,
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
if (smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) {
pr_info("force clock level is for dpm manual mode only.\n");
return -EINVAL;
@@ -1232,8 +1241,9 @@ static int vega20_get_clock_by_type_with_latency(struct
smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
mutex_lock(&smu->mutex);
-
switch (type) {
case amd_pp_sys_clock:
single_dpm_table = &(dpm_table->gfx_table); @@ -1265,6 +1275,8
@@ static int vega20_overdrive_get_gfx_clk_base_voltage(struct smu_context
*smu, {
int ret;
+ if (!smu->pm_enabled)
+ return -EINVAL;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_GetAVFSVoltageByDpm,
((AVFS_CURVE << 24) | (OD8_HOTCURVE_TEMPERATURE << 16)
| freq)); @@ -1287,7 +1299,7 @@ static int
vega20_set_default_od8_setttings(struct smu_context *smu)
PPTable_t *smc_pptable = table_context->driver_pptable;
int i, ret;
- if (table_context->od8_settings)
+ if (!smu->pm_enabled || table_context->od8_settings)
return -EINVAL;
table_context->od8_settings = kzalloc(sizeof(struct
vega20_od8_settings), GFP_KERNEL); @@ -1474,6 +1486,8 @@ static int
vega20_get_od_percentage(struct smu_cont
RE: [PATCH 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20 power functions.
I'm wondering whether we are able to handle all asic specific ppt function
gracefully. Add pm_enable check into ppt functions would be boring since we
have to do similar jobs for each ASIC. Instead, is it possible to return early
from amdgpu_smu interface level?
Take smu_handle_task for the example, all the amdgpu_smu level interface
invoked from this function could be stopped if we check smu pm_enable flag at
the beginning of smu_handle_task. We probably have better approach to eliminate
this patch.
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 2/2] drm/amd/powerplay: Enable "disable dpm" feature for vega20
power functions.
[CAUTION: External Email]
use pm_enabled to control all power related functions about vega20.
Signed-off-by: Chengming Gui
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 48 +++---
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 8fafcbd..7faa6a1 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -411,7 +411,8 @@ amd_pm_state_type vega20_get_current_power_state(struct
smu_context *smu)
enum amd_pm_state_type pm_type;
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
- if (!smu_dpm_ctx->dpm_context ||
+ if (!smu->pm_enabled ||
+ !smu_dpm_ctx->dpm_context ||
!smu_dpm_ctx->dpm_current_power_state)
return -EINVAL;
@@ -491,12 +492,14 @@ static void vega20_init_single_dpm_state(struct
vega20_dpm_state *dpm_state)
static int vega20_set_default_dpm_table(struct smu_context *smu) {
- int ret;
+ int ret = 0;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct vega20_dpm_table *dpm_table = NULL;
struct vega20_single_dpm_table *single_dpm_table;
+ if (smu->pm_enabled)
+ return ret;
dpm_table = smu_dpm->dpm_context;
/* socclk */
@@ -738,6 +741,8 @@ static int vega20_print_clk_levels(struct smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
switch (type) {
case PP_SCLK:
ret = smu_get_current_clk_freq(smu, PPCLK_GFXCLK, &now); @@
-969,6 +974,8 @@ static int vega20_upload_dpm_level(struct smu_context *smu,
bool max,
uint32_t freq;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
dpm_table = smu->smu_dpm.dpm_context;
if (smu_feature_is_enabled(smu, FEATURE_DPM_GFXCLK_BIT) && @@ -1058,6
+1065,8 @@ static int vega20_force_clk_levels(struct smu_context *smu,
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
int ret = 0;
+ if (!smu->pm_enabled)
+ return -EINVAL;
if (smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) {
pr_info("force clock level is for dpm manual mode only.\n");
return -EINVAL;
@@ -1232,8 +1241,9 @@ static int vega20_get_clock_by_type_with_latency(struct
smu_context *smu,
dpm_table = smu_dpm->dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
mutex_lock(&smu->mutex);
-
switch (type) {
case amd_pp_sys_clock:
single_dpm_table = &(dpm_table->gfx_table); @@ -1265,6 +1275,8
@@ static int vega20_overdrive_get_gfx_clk_base_voltage(struct smu_context
*smu, {
int ret;
+ if (!smu->pm_enabled)
+ return -EINVAL;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_GetAVFSVoltageByDpm,
((AVFS_CURVE << 24) | (OD8_HOTCURVE_TEMPERATURE << 16)
| freq)); @@ -1287,7 +1299,7 @@ static int
vega20_set_default_od8_setttings(struct smu_context *smu)
PPTable_t *smc_pptable = table_context->driver_pptable;
int i, ret;
- if (table_context->od8_settings)
+ if (!smu->pm_enabled || table_context->od8_settings)
return -EINVAL;
table_context->od8_settings = kzalloc(sizeof(struct
vega20_od8_settings), GFP_KERNEL); @@ -1474,6 +1486,8 @@ static int
vega20_get_od_percentage(struct smu_context *smu,
dpm_table = smu_dpm->dpm_context;
golden_table = smu_dpm->golden_dpm_context;
+ if (!smu->pm_enabled)
+ return -EINVAL;
switch (type) {
case OD_SCLK:
single_dpm_table = &(dpm_table->gfx_table); @@ -1509,7 +1523,7
@@ vega20_get_profiling_clk_mask(struct smu_context *smu,
struct vega20_single_dpm_table *mem_dpm_table;
struct vega20_single_dpm_table *soc_dpm_table;
- if (!smu->smu_dpm.dpm_context)
+ if (!smu->pm_enabled || !smu->smu_dpm.dpm_context)
return -EINVAL;
gfx_dpm_tabl
