* [PATCH 1/5] drm/amd/pm: Rename a couple of functions (v3)
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
@ 2021-10-18 23:49 ` Luben Tuikov
2021-10-18 23:49 ` [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value Luben Tuikov
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-18 23:49 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher, Paul Menzel
Rename
sienna_cichlid_is_support_fine_grained_dpm() to
sienna_cichlid_supports_fine_grained_dpm().
Rename
navi10_is_support_fine_grained_dpm() to
navi10_supports_fine_grained_dpm().
v2: Fix function name in commit message to reflect
the change being done: add a missing 's'.
v3: Start the subject with a verb. (Suggested by
Paul M.)
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 7 ++++---
drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 7 ++++---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 71161f6b78fea9..0fe9790f67f5af 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1231,7 +1231,8 @@ static int navi10_get_current_clk_freq_by_table(struct smu_context *smu,
value);
}
-static bool navi10_is_support_fine_grained_dpm(struct smu_context *smu, enum smu_clk_type clk_type)
+static bool navi10_supports_fine_grained_dpm(struct smu_context *smu,
+ enum smu_clk_type clk_type)
{
PPTable_t *pptable = smu->smu_table.driver_pptable;
DpmDescriptor_t *dpm_desc = NULL;
@@ -1299,7 +1300,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
- if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
if (ret)
@@ -1465,7 +1466,7 @@ static int navi10_force_clk_levels(struct smu_context *smu,
case SMU_UCLK:
case SMU_FCLK:
/* There is only 2 levels for fine grained DPM */
- if (navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (navi10_supports_fine_grained_dpm(smu, clk_type)) {
soft_max_level = (soft_max_level >= 1 ? 1 : 0);
soft_min_level = (soft_min_level >= 1 ? 1 : 0);
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 15e66e1912de33..3f5721baa5ff50 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1006,7 +1006,8 @@ static int sienna_cichlid_get_current_clk_freq_by_table(struct smu_context *smu,
}
-static bool sienna_cichlid_is_support_fine_grained_dpm(struct smu_context *smu, enum smu_clk_type clk_type)
+static bool sienna_cichlid_supports_fine_grained_dpm(struct smu_context *smu,
+ enum smu_clk_type clk_type)
{
DpmDescriptor_t *dpm_desc = NULL;
DpmDescriptor_t *table_member;
@@ -1084,7 +1085,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- if (!sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
if (ret)
@@ -1235,7 +1236,7 @@ static int sienna_cichlid_force_clk_levels(struct smu_context *smu,
case SMU_UCLK:
case SMU_FCLK:
/* There is only 2 levels for fine grained DPM */
- if (sienna_cichlid_is_support_fine_grained_dpm(smu, clk_type)) {
+ if (sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
soft_max_level = (soft_max_level >= 1 ? 1 : 0);
soft_min_level = (soft_min_level >= 1 ? 1 : 0);
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
2021-10-18 23:49 ` [PATCH 1/5] drm/amd/pm: Rename a couple of functions (v3) Luben Tuikov
@ 2021-10-18 23:49 ` Luben Tuikov
2021-10-18 23:49 ` [PATCH 3/5] drm/amd/pm: Rename freq_values --> freq_value Luben Tuikov
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-18 23:49 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher
Rename "cur_value", which stands for "cursor
value" to "curr_value", which stands for "current
value".
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 ++++++------
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++-------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 0fe9790f67f5af..f810549df493d5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,7 +1267,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
@@ -1292,7 +1292,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
case SMU_VCLK:
case SMU_DCLK:
case SMU_DCEFCLK:
- ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
return size;
@@ -1307,7 +1307,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
return size;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1317,9 +1317,9 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
if (mark_index != 1)
freq_values[1] = (freq_values[0] + freq_values[2]) / 2;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3f5721baa5ff50..3ebded3a99b5f2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1052,7 +1052,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
@@ -1073,10 +1073,11 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
case SMU_DCLK:
case SMU_DCLK1:
case SMU_DCEFCLK:
- ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
goto print_clk_out;
+
/* no need to disable gfxoff when retrieving the current gfxclk */
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
amdgpu_gfx_off_ctrl(adev, false);
@@ -1092,7 +1093,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
goto print_clk_out;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1102,9 +1103,9 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
@@ -1114,7 +1115,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
for (i = 0; i < count; i++) {
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
- cur_value == freq_values[i] ? "*" : "");
+ curr_value == freq_values[i] ? "*" : "");
}
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/5] drm/amd/pm: Rename freq_values --> freq_value
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
2021-10-18 23:49 ` [PATCH 1/5] drm/amd/pm: Rename a couple of functions (v3) Luben Tuikov
2021-10-18 23:49 ` [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value Luben Tuikov
@ 2021-10-18 23:49 ` Luben Tuikov
2021-10-18 23:49 ` [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-18 23:49 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher
By usage: read freq_values[x] to freq_value[x].
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++--------
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 18 +++++++++---------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index f810549df493d5..646e9bbf8af42a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1268,7 +1268,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
uint16_t *curve_settings;
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
- uint32_t freq_values[3] = {0};
+ uint32_t freq_value[3] = {0, 0, 0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
uint32_t gen_speed, lane_width;
@@ -1310,21 +1310,21 @@ static int navi10_print_clk_levels(struct smu_context *smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
if (ret)
return size;
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_values[2]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
if (ret)
return size;
- freq_values[1] = curr_value;
- mark_index = curr_value == freq_values[0] ? 0 :
- curr_value == freq_values[2] ? 2 : 1;
+ freq_value[1] = curr_value;
+ mark_index = curr_value == freq_value[0] ? 0 :
+ curr_value == freq_value[2] ? 2 : 1;
if (mark_index != 1)
- freq_values[1] = (freq_values[0] + freq_values[2]) / 2;
+ freq_value[1] = (freq_value[0] + freq_value[2]) / 2;
for (i = 0; i < 3; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
i == mark_index ? "*" : "");
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3ebded3a99b5f2..f630d5e928ccfe 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1053,7 +1053,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
uint32_t curr_value = 0, value = 0, count = 0;
- uint32_t freq_values[3] = {0};
+ uint32_t freq_value[3] = {0, 0, 0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
uint32_t min_value, max_value;
@@ -1096,26 +1096,26 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
if (ret)
goto print_clk_out;
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_values[2]);
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
if (ret)
goto print_clk_out;
- freq_values[1] = curr_value;
- mark_index = curr_value == freq_values[0] ? 0 :
- curr_value == freq_values[2] ? 2 : 1;
+ freq_value[1] = curr_value;
+ mark_index = curr_value == freq_value[0] ? 0 :
+ curr_value == freq_value[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
count = 2;
- freq_values[1] = freq_values[2];
+ freq_value[1] = freq_value[2];
}
for (i = 0; i < count; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
- curr_value == freq_values[i] ? "*" : "");
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
+ curr_value == freq_value[i] ? "*" : "");
}
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
` (2 preceding siblings ...)
2021-10-18 23:49 ` [PATCH 3/5] drm/amd/pm: Rename freq_values --> freq_value Luben Tuikov
@ 2021-10-18 23:49 ` Luben Tuikov
2021-10-19 3:38 ` Lazar, Lijo
2021-10-18 23:49 ` [PATCH 5/5] dpm/amd/pm: Navi10: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
2021-10-19 7:23 ` [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Paul Menzel
5 siblings, 1 reply; 18+ messages in thread
From: Luben Tuikov @ 2021-10-18 23:49 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher, Lijo Lazar, Paul Menzel
A current value of a clock frequency of 0, means
that the IP block is in some kind of low power
state. Ignore it and don't report it here. Here we
only report the possible operating (non-zero)
frequencies of the block requested. So, if the
current clock value is 0, then print the DPM
frequencies, but don't report a current value.
v2: Don't report the minimum one as the current
one when reported one is 0, i.e. don't add an
asterisk (Lijo). LT: It is conceivable that this
may confuse user-mode tools if they scan and look
for a current one, i.e. look for an asterisk, but
they'll have to adapt and use other methods for
finding power states of the chip--we can't report
0 as current.
v3: Start the subject title with a verb. (PaulM)
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Lijo Lazar <Lijo.Lazar@amd.com>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 60 ++++++++++++-------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index f630d5e928ccfe..6fe792be77dbbb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1040,7 +1040,8 @@ static void sienna_cichlid_get_od_setting_range(struct smu_11_0_7_overdrive_tabl
}
static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
- enum smu_clk_type clk_type, char *buf)
+ enum smu_clk_type clk_type,
+ char *buf)
{
struct amdgpu_device *adev = smu->adev;
struct smu_table_context *table_context = &smu->smu_table;
@@ -1052,12 +1053,12 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t curr_value = 0, value = 0, count = 0;
+ uint32_t curr_value, value, count;
uint32_t freq_value[3] = {0, 0, 0};
- uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
uint32_t min_value, max_value;
uint32_t smu_version;
+ bool fine_grained;
smu_cmn_get_sysfs_buf(&buf, &size);
@@ -1077,6 +1078,20 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
+ &freq_value[0]);
+ if (ret)
+ goto print_clk_out;
+
+ /* A current value of a clock frequency of 0, means
+ * that the IP block is in some kind of low power
+ * state. Ignore it and don't report it here. Here we
+ * only report the possible operating (non-zero)
+ * frequencies of the block requested. So, if the
+ * current clock value is 0, then we don't report a
+ * "current" value from the DPM states, i.e. we don't
+ * add an asterisk.
+ */
/* no need to disable gfxoff when retrieving the current gfxclk */
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
@@ -1086,38 +1101,43 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
- for (i = 0; i < count; i++) {
+ fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu, clk_type);
+ if (!fine_grained) {
+ /* We already got the 0-th index--print it
+ * here and continue thereafter.
+ */
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
+ curr_value == freq_value[0] ? "*" : "");
+ for (i = 1; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
if (ret)
goto print_clk_out;
-
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
- if (ret)
- goto print_clk_out;
+ freq_value[1] = curr_value ?: freq_value[0];
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
if (ret)
goto print_clk_out;
- freq_value[1] = curr_value;
- mark_index = curr_value == freq_value[0] ? 0 :
- curr_value == freq_value[2] ? 2 : 1;
-
- count = 3;
- if (mark_index != 1) {
+ if (freq_value[1] == freq_value[0]) {
+ i = 1;
+ count = 3;
+ } else if (freq_value[1] == freq_value[2]) {
+ i = 0;
count = 2;
- freq_value[1] = freq_value[2];
+ } else {
+ i = 0;
+ count = 3;
}
- for (i = 0; i < count; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
- curr_value == freq_value[i] ? "*" : "");
+ for ( ; i < count; i++) {
+ size += sysfs_emit_at(buf, size,
+ "%d: %uMhz %s\n",
+ i, freq_value[i],
+ curr_value == freq_value[i] ? "*" : "");
}
-
}
break;
case SMU_PCIE:
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
2021-10-18 23:49 ` [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
@ 2021-10-19 3:38 ` Lazar, Lijo
2021-10-19 4:15 ` Luben Tuikov
0 siblings, 1 reply; 18+ messages in thread
From: Lazar, Lijo @ 2021-10-19 3:38 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher, Paul Menzel
On 10/19/2021 5:19 AM, Luben Tuikov wrote:
> A current value of a clock frequency of 0, means
> that the IP block is in some kind of low power
> state. Ignore it and don't report it here. Here we
> only report the possible operating (non-zero)
> frequencies of the block requested. So, if the
> current clock value is 0, then print the DPM
> frequencies, but don't report a current value.
>
> v2: Don't report the minimum one as the current
> one when reported one is 0, i.e. don't add an
> asterisk (Lijo). LT: It is conceivable that this
> may confuse user-mode tools if they scan and look
> for a current one, i.e. look for an asterisk, but
> they'll have to adapt and use other methods for
> finding power states of the chip--we can't report
> 0 as current.
> v3: Start the subject title with a verb. (PaulM)
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Cc: Lijo Lazar <Lijo.Lazar@amd.com>
> Cc: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 60 ++++++++++++-------
> 1 file changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index f630d5e928ccfe..6fe792be77dbbb 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -1040,7 +1040,8 @@ static void sienna_cichlid_get_od_setting_range(struct smu_11_0_7_overdrive_tabl
> }
>
> static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
> - enum smu_clk_type clk_type, char *buf)
> + enum smu_clk_type clk_type,
> + char *buf)
> {
> struct amdgpu_device *adev = smu->adev;
> struct smu_table_context *table_context = &smu->smu_table;
> @@ -1052,12 +1053,12 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
> OverDriveTable_t *od_table =
> (OverDriveTable_t *)table_context->overdrive_table;
> int i, size = 0, ret = 0;
> - uint32_t curr_value = 0, value = 0, count = 0;
> + uint32_t curr_value, value, count;
> uint32_t freq_value[3] = {0, 0, 0};
> - uint32_t mark_index = 0;
> uint32_t gen_speed, lane_width;
> uint32_t min_value, max_value;
> uint32_t smu_version;
> + bool fine_grained;
>
> smu_cmn_get_sysfs_buf(&buf, &size);
>
> @@ -1077,6 +1078,20 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
> if (ret)
> goto print_clk_out;
>
> + ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
> + &freq_value[0]);
> + if (ret)
> + goto print_clk_out;
> +
> + /* A current value of a clock frequency of 0, means
> + * that the IP block is in some kind of low power
> + * state. Ignore it and don't report it here. Here we
> + * only report the possible operating (non-zero)
> + * frequencies of the block requested. So, if the
> + * current clock value is 0, then we don't report a
> + * "current" value from the DPM states, i.e. we don't
> + * add an asterisk.
> + */
>
> /* no need to disable gfxoff when retrieving the current gfxclk */
> if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
> @@ -1086,38 +1101,43 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
> if (ret)
> goto print_clk_out;
>
> - if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
> - for (i = 0; i < count; i++) {
> + fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu, clk_type);
> + if (!fine_grained) {
> + /* We already got the 0-th index--print it
> + * here and continue thereafter.
> + */
> + size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
> + curr_value == freq_value[0] ? "*" : "");
> + for (i = 1; i < count; i++) {
> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
> if (ret)
> goto print_clk_out;
> -
> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
> curr_value == value ? "*" : "");
> }
> } else {
> - ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
> - if (ret)
> - goto print_clk_out;
> + freq_value[1] = curr_value ?: freq_value[0];
Omitting second expression is not standard C -
https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
Thanks,
Lijo
> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
> if (ret)
> goto print_clk_out;
>
> - freq_value[1] = curr_value;
> - mark_index = curr_value == freq_value[0] ? 0 :
> - curr_value == freq_value[2] ? 2 : 1;
> -
> - count = 3;
> - if (mark_index != 1) {
> + if (freq_value[1] == freq_value[0]) {
> + i = 1;
> + count = 3;
> + } else if (freq_value[1] == freq_value[2]) {
> + i = 0;
> count = 2;
> - freq_value[1] = freq_value[2];
> + } else {
> + i = 0;
> + count = 3;
> }
>
> - for (i = 0; i < count; i++) {
> - size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
> - curr_value == freq_value[i] ? "*" : "");
> + for ( ; i < count; i++) {
> + size += sysfs_emit_at(buf, size,
> + "%d: %uMhz %s\n",
> + i, freq_value[i],
> + curr_value == freq_value[i] ? "*" : "");
> }
> -
> }
> break;
> case SMU_PCIE:
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
2021-10-19 3:38 ` Lazar, Lijo
@ 2021-10-19 4:15 ` Luben Tuikov
2021-10-19 4:38 ` Lazar, Lijo
0 siblings, 1 reply; 18+ messages in thread
From: Luben Tuikov @ 2021-10-19 4:15 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx; +Cc: Alex Deucher, Paul Menzel
[-- Attachment #1: Type: text/html, Size: 7625 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
2021-10-19 4:15 ` Luben Tuikov
@ 2021-10-19 4:38 ` Lazar, Lijo
2021-10-19 4:50 ` Luben Tuikov
0 siblings, 1 reply; 18+ messages in thread
From: Lazar, Lijo @ 2021-10-19 4:38 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher, Paul Menzel
On 10/19/2021 9:45 AM, Luben Tuikov wrote:
> On 2021-10-18 23:38, Lazar, Lijo wrote:
>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:
>>> A current value of a clock frequency of 0, means
>>> that the IP block is in some kind of low power
>>> state. Ignore it and don't report it here. Here we
>>> only report the possible operating (non-zero)
>>> frequencies of the block requested. So, if the
>>> current clock value is 0, then print the DPM
>>> frequencies, but don't report a current value.
>>>
>>> v2: Don't report the minimum one as the current
>>> one when reported one is 0, i.e. don't add an
>>> asterisk (Lijo). LT: It is conceivable that this
>>> may confuse user-mode tools if they scan and look
>>> for a current one, i.e. look for an asterisk, but
>>> they'll have to adapt and use other methods for
>>> finding power states of the chip--we can't report
>>> 0 as current.
>>> v3: Start the subject title with a verb. (PaulM)
>>>
>>> Cc: Alex Deucher<Alexander.Deucher@amd.com>
>>> Cc: Lijo Lazar<Lijo.Lazar@amd.com>
>>> Cc: Paul Menzel<pmenzel@molgen.mpg.de>
>>> Signed-off-by: Luben Tuikov<luben.tuikov@amd.com>
>>> ---
>>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 60 ++++++++++++-------
>>> 1 file changed, 40 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> index f630d5e928ccfe..6fe792be77dbbb 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> @@ -1040,7 +1040,8 @@ static void sienna_cichlid_get_od_setting_range(struct smu_11_0_7_overdrive_tabl
>>> }
>>>
>>> static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> - enum smu_clk_type clk_type, char *buf)
>>> + enum smu_clk_type clk_type,
>>> + char *buf)
>>> {
>>> struct amdgpu_device *adev = smu->adev;
>>> struct smu_table_context *table_context = &smu->smu_table;
>>> @@ -1052,12 +1053,12 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> OverDriveTable_t *od_table =
>>> (OverDriveTable_t *)table_context->overdrive_table;
>>> int i, size = 0, ret = 0;
>>> - uint32_t curr_value = 0, value = 0, count = 0;
>>> + uint32_t curr_value, value, count;
>>> uint32_t freq_value[3] = {0, 0, 0};
>>> - uint32_t mark_index = 0;
>>> uint32_t gen_speed, lane_width;
>>> uint32_t min_value, max_value;
>>> uint32_t smu_version;
>>> + bool fine_grained;
>>>
>>> smu_cmn_get_sysfs_buf(&buf, &size);
>>>
>>> @@ -1077,6 +1078,20 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> + ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
>>> + &freq_value[0]);
>>> + if (ret)
>>> + goto print_clk_out;
>>> +
>>> + /* A current value of a clock frequency of 0, means
>>> + * that the IP block is in some kind of low power
>>> + * state. Ignore it and don't report it here. Here we
>>> + * only report the possible operating (non-zero)
>>> + * frequencies of the block requested. So, if the
>>> + * current clock value is 0, then we don't report a
>>> + * "current" value from the DPM states, i.e. we don't
>>> + * add an asterisk.
>>> + */
>>>
>>> /* no need to disable gfxoff when retrieving the current gfxclk */
>>> if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
>>> @@ -1086,38 +1101,43 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> - if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
>>> - for (i = 0; i < count; i++) {
>>> + fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu, clk_type);
>>> + if (!fine_grained) {
>>> + /* We already got the 0-th index--print it
>>> + * here and continue thereafter.
>>> + */
>>> + size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
>>> + curr_value == freq_value[0] ? "*" : "");
>>> + for (i = 1; i < count; i++) {
>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
>>> if (ret)
>>> goto print_clk_out;
>>> -
>>> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
>>> curr_value == value ? "*" : "");
>>> }
>>> } else {
>>> - ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
>>> - if (ret)
>>> - goto print_clk_out;
>>> + freq_value[1] = curr_value ?: freq_value[0];
>> Omitting second expression is not standard C -
>> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
>
> Lijo just clarified to me that:
>
>> well, i had to look up as I haven't seen it before
>
> I hope the following should make it clear about its usage:
>
> $cd linux/
> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
> 1042
> $_
>
Thanks Luben!
Thanks,
Lijo
> Regards,
> Luben
>
>> Thanks,
>> Lijo
>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
>>> if (ret)
>>> goto print_clk_out;
>>>
>>> - freq_value[1] = curr_value;
>>> - mark_index = curr_value == freq_value[0] ? 0 :
>>> - curr_value == freq_value[2] ? 2 : 1;
>>> -
>>> - count = 3;
>>> - if (mark_index != 1) {
>>> + if (freq_value[1] == freq_value[0]) {
>>> + i = 1;
>>> + count = 3;
>>> + } else if (freq_value[1] == freq_value[2]) {
>>> + i = 0;
>>> count = 2;
>>> - freq_value[1] = freq_value[2];
>>> + } else {
>>> + i = 0;
>>> + count = 3;
>>> }
>>>
>>> - for (i = 0; i < count; i++) {
>>> - size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
>>> - curr_value == freq_value[i] ? "*" : "");
>>> + for ( ; i < count; i++) {
>>> + size += sysfs_emit_at(buf, size,
>>> + "%d: %uMhz %s\n",
>>> + i, freq_value[i],
>>> + curr_value == freq_value[i] ? "*" : "");
>>> }
>>> -
>>> }
>>> break;
>>> case SMU_PCIE:
>>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)
2021-10-19 4:38 ` Lazar, Lijo
@ 2021-10-19 4:50 ` Luben Tuikov
2021-10-19 7:44 ` Use of conditionals with omitted operands in amdgpu (x? : y) (was: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)) Paul Menzel
0 siblings, 1 reply; 18+ messages in thread
From: Luben Tuikov @ 2021-10-19 4:50 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx; +Cc: Alex Deucher, Paul Menzel, Tuikov, Luben
On 2021-10-19 00:38, Lazar, Lijo wrote:
>
> On 10/19/2021 9:45 AM, Luben Tuikov wrote:
>> On 2021-10-18 23:38, Lazar, Lijo wrote:
>>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:
>>>> A current value of a clock frequency of 0, means
>>>> that the IP block is in some kind of low power
>>>> state. Ignore it and don't report it here. Here we
>>>> only report the possible operating (non-zero)
>>>> frequencies of the block requested. So, if the
>>>> current clock value is 0, then print the DPM
>>>> frequencies, but don't report a current value.
>>>>
>>>> v2: Don't report the minimum one as the current
>>>> one when reported one is 0, i.e. don't add an
>>>> asterisk (Lijo). LT: It is conceivable that this
>>>> may confuse user-mode tools if they scan and look
>>>> for a current one, i.e. look for an asterisk, but
>>>> they'll have to adapt and use other methods for
>>>> finding power states of the chip--we can't report
>>>> 0 as current.
>>>> v3: Start the subject title with a verb. (PaulM)
>>>>
>>>> Cc: Alex Deucher<Alexander.Deucher@amd.com>
>>>> Cc: Lijo Lazar<Lijo.Lazar@amd.com>
>>>> Cc: Paul Menzel<pmenzel@molgen.mpg.de>
>>>> Signed-off-by: Luben Tuikov<luben.tuikov@amd.com>
>>>> ---
>>>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 60 ++++++++++++-------
>>>> 1 file changed, 40 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>> index f630d5e928ccfe..6fe792be77dbbb 100644
>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>> @@ -1040,7 +1040,8 @@ static void sienna_cichlid_get_od_setting_range(struct smu_11_0_7_overdrive_tabl
>>>> }
>>>>
>>>> static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>>> - enum smu_clk_type clk_type, char *buf)
>>>> + enum smu_clk_type clk_type,
>>>> + char *buf)
>>>> {
>>>> struct amdgpu_device *adev = smu->adev;
>>>> struct smu_table_context *table_context = &smu->smu_table;
>>>> @@ -1052,12 +1053,12 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>>> OverDriveTable_t *od_table =
>>>> (OverDriveTable_t *)table_context->overdrive_table;
>>>> int i, size = 0, ret = 0;
>>>> - uint32_t curr_value = 0, value = 0, count = 0;
>>>> + uint32_t curr_value, value, count;
>>>> uint32_t freq_value[3] = {0, 0, 0};
>>>> - uint32_t mark_index = 0;
>>>> uint32_t gen_speed, lane_width;
>>>> uint32_t min_value, max_value;
>>>> uint32_t smu_version;
>>>> + bool fine_grained;
>>>>
>>>> smu_cmn_get_sysfs_buf(&buf, &size);
>>>>
>>>> @@ -1077,6 +1078,20 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>>> if (ret)
>>>> goto print_clk_out;
>>>>
>>>> + ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
>>>> + &freq_value[0]);
>>>> + if (ret)
>>>> + goto print_clk_out;
>>>> +
>>>> + /* A current value of a clock frequency of 0, means
>>>> + * that the IP block is in some kind of low power
>>>> + * state. Ignore it and don't report it here. Here we
>>>> + * only report the possible operating (non-zero)
>>>> + * frequencies of the block requested. So, if the
>>>> + * current clock value is 0, then we don't report a
>>>> + * "current" value from the DPM states, i.e. we don't
>>>> + * add an asterisk.
>>>> + */
>>>>
>>>> /* no need to disable gfxoff when retrieving the current gfxclk */
>>>> if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
>>>> @@ -1086,38 +1101,43 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
>>>> if (ret)
>>>> goto print_clk_out;
>>>>
>>>> - if (!sienna_cichlid_supports_fine_grained_dpm(smu, clk_type)) {
>>>> - for (i = 0; i < count; i++) {
>>>> + fine_grained = sienna_cichlid_supports_fine_grained_dpm(smu, clk_type);
>>>> + if (!fine_grained) {
>>>> + /* We already got the 0-th index--print it
>>>> + * here and continue thereafter.
>>>> + */
>>>> + size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
>>>> + curr_value == freq_value[0] ? "*" : "");
>>>> + for (i = 1; i < count; i++) {
>>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
>>>> if (ret)
>>>> goto print_clk_out;
>>>> -
>>>> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
>>>> curr_value == value ? "*" : "");
>>>> }
>>>> } else {
>>>> - ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
>>>> - if (ret)
>>>> - goto print_clk_out;
>>>> + freq_value[1] = curr_value ?: freq_value[0];
>>> Omitting second expression is not standard C -
>>> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
>> Lijo just clarified to me that:
>>
>>> well, i had to look up as I haven't seen it before
>> I hope the following should make it clear about its usage:
>>
>> $cd linux/
>> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
>> 1042
>> $_
>>
> Thanks Luben!
You're welcome. I'm glad you're learning new things from my patches. Would've been easier if you'd just said in your email that you've never seen this ternary conditional shortcut before and that you've just learned of it from my patch. (Or not post anything at all in this very case and get in touch with me privately via email or Teams--I would've gladly clarified it there.)
I hope the find+egrep above is also edifying, so you can use it in the future in your learning process.
Regards,
Luben
>
> Thanks,
> Lijo
>
>> Regards,
>> Luben
>>
>>> Thanks,
>>> Lijo
>>>> ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
>>>> if (ret)
>>>> goto print_clk_out;
>>>>
>>>> - freq_value[1] = curr_value;
>>>> - mark_index = curr_value == freq_value[0] ? 0 :
>>>> - curr_value == freq_value[2] ? 2 : 1;
>>>> -
>>>> - count = 3;
>>>> - if (mark_index != 1) {
>>>> + if (freq_value[1] == freq_value[0]) {
>>>> + i = 1;
>>>> + count = 3;
>>>> + } else if (freq_value[1] == freq_value[2]) {
>>>> + i = 0;
>>>> count = 2;
>>>> - freq_value[1] = freq_value[2];
>>>> + } else {
>>>> + i = 0;
>>>> + count = 3;
>>>> }
>>>>
>>>> - for (i = 0; i < count; i++) {
>>>> - size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
>>>> - curr_value == freq_value[i] ? "*" : "");
>>>> + for ( ; i < count; i++) {
>>>> + size += sysfs_emit_at(buf, size,
>>>> + "%d: %uMhz %s\n",
>>>> + i, freq_value[i],
>>>> + curr_value == freq_value[i] ? "*" : "");
>>>> }
>>>> -
>>>> }
>>>> break;
>>>> case SMU_PCIE:
>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread* Use of conditionals with omitted operands in amdgpu (x? : y) (was: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3))
2021-10-19 4:50 ` Luben Tuikov
@ 2021-10-19 7:44 ` Paul Menzel
2021-10-19 8:07 ` Luben Tuikov
0 siblings, 1 reply; 18+ messages in thread
From: Paul Menzel @ 2021-10-19 7:44 UTC (permalink / raw)
To: Luben Tuikov; +Cc: Alex Deucher, Lijo Lazar, amd-gfx
Dear Luben,
Am 19.10.21 um 06:50 schrieb Luben Tuikov:
> On 2021-10-19 00:38, Lazar, Lijo wrote:
>>
>> On 10/19/2021 9:45 AM, Luben Tuikov wrote:
>>> On 2021-10-18 23:38, Lazar, Lijo wrote:
>>>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:
[…]
>>>>> - if (ret)
>>>>> - goto print_clk_out;
>>>>> + freq_value[1] = curr_value ?: freq_value[0];
>>>> Omitting second expression is not standard C -
>>>> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
>>> Lijo just clarified to me that:
>>>
>>>> well, i had to look up as I haven't seen it before
>>> I hope the following should make it clear about its usage:
>>>
>>> $cd linux/
>>> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
>>> 1042
>>> $_
$ git grep -E "\?:" -- '*amdgpu*.[ch]'
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c: * Solution?:
So for the AMDGPU subsystem, as the only result is a comment, currently,
conditionals with omitted operands are not used. So, it’s a valid
question, if the use should be introduced into the subsystem.
The GCC documentation also states:
> In this simple case, the ability to omit the middle operand is not
> especially useful. When it becomes useful is when the first operand
> does, or may (if it is a macro argument), contain a side effect. Then
> repeating the operand in the middle would perform the side effect
> twice. Omitting the middle operand uses the value already computed
> without the undesirable effects of recomputing it.
So, in your case, there are no side effect, if I am not mistaken.
I do not care, if the extension is going to be used or not. The
maintainers might want to officially confirm the use in the subsystem,
as using these extensions is surprising for some C developers not
knowing the GNU extensions.
>> Thanks Luben!
>
> You're welcome. I'm glad you're learning new things from my patches.
> Would've been easier if you'd just said in your email that you've
> never seen this ternary conditional shortcut before and that you've
> just learned of it from my patch. (Or not post anything at all in
> this very case and get in touch with me privately via email or
> Teams--I would've gladly clarified it there.)
In my opinion, asking this on the list is perfectly valid, as other
readers, might have the same question. But being more elaborate to avoid
misunderstandings is always a good thing.
> I hope the find+egrep above is also edifying, so you can use it in
> the future in your learning process.
I hope, you like my solution without using find. ;-)
Kind regards,
Paul
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: Use of conditionals with omitted operands in amdgpu (x? : y) (was: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3))
2021-10-19 7:44 ` Use of conditionals with omitted operands in amdgpu (x? : y) (was: [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3)) Paul Menzel
@ 2021-10-19 8:07 ` Luben Tuikov
0 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-19 8:07 UTC (permalink / raw)
To: Paul Menzel, Deucher, Alexander, Koenig, Christian
Cc: Alex Deucher, Lijo Lazar, amd-gfx, LKML
+AlexD
+ChrisianK
+LKML
On 2021-10-19 03:44, Paul Menzel wrote:
> Dear Luben,
>
>
> Am 19.10.21 um 06:50 schrieb Luben Tuikov:
>> On 2021-10-19 00:38, Lazar, Lijo wrote:
>>> On 10/19/2021 9:45 AM, Luben Tuikov wrote:
>>>> On 2021-10-18 23:38, Lazar, Lijo wrote:
>>>>> On 10/19/2021 5:19 AM, Luben Tuikov wrote:
> […]
>
>>>>>> - if (ret)
>>>>>> - goto print_clk_out;
>>>>>> + freq_value[1] = curr_value ?: freq_value[0];
>>>>> Omitting second expression is not standard C -
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fonlinedocs%2Fgcc%2FConditionals.html&data=04%7C01%7Cluben.tuikov%40amd.com%7Ca0515ae37fc64695640408d992d44452%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637702263078635669%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=cPRhR4Fcns4Kx%2BjkAjvN16xDk0jDEbkCO0EooJzEUlA%3D&reserved=0
>>>> Lijo just clarified to me that:
>>>>
>>>>> well, i had to look up as I haven't seen it before
>>>> I hope the following should make it clear about its usage:
>>>>
>>>> $cd linux/
>>>> $find . -name "*.[ch]" -exec grep -E "\?:" \{\} \+ | wc -l
>>>> 1042
>>>> $_
> $ git grep -E "\?:" -- '*amdgpu*.[ch]'
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c: * Solution?:
>
> So for the AMDGPU subsystem, as the only result is a comment, currently,
> conditionals with omitted operands are not used. So, it’s a valid
> question, if the use should be introduced into the subsystem.
>
> The GCC documentation also states:
>
>> In this simple case, the ability to omit the middle operand is not
>> especially useful. When it becomes useful is when the first operand
>> does, or may (if it is a macro argument), contain a side effect. Then
>> repeating the operand in the middle would perform the side effect
>> twice. Omitting the middle operand uses the value already computed
>> without the undesirable effects of recomputing it.
> So, in your case, there are no side effect, if I am not mistaken.
The explanation you quoted above makes a case *for* using the extension. It's telling you that it is a good thing to use the extension so should the first operand be a macro with an argument, it'll avoid being evaluated twice.
>
> I do not care, if the extension is going to be used or not.
So then why post this message, if you don't care? Since your email continues after this, like this:
> The
> maintainers might want to officially confirm the use in the subsystem,
I've added the maintainers, Alex and Christian, as well as LKML to the To: list of this email.
I believe that it is perfectly fine to use the ternary conditional abbreviation "c = a ?: b;", as the kernel uses it extensively, over 1000 occurrences in the kernel. It also eliminates side effects should 'a' be a macro with an argument which evaluates.
> as using these extensions is surprising for some C developers not
> knowing the GNU extensions.
>
>>> Thanks Luben!
>> You're welcome. I'm glad you're learning new things from my patches.
>> Would've been easier if you'd just said in your email that you've
>> never seen this ternary conditional shortcut before and that you've
>> just learned of it from my patch. (Or not post anything at all in
>> this very case and get in touch with me privately via email or
>> Teams--I would've gladly clarified it there.)
> In my opinion, asking this on the list is perfectly valid, as other
> readers, might have the same question. But being more elaborate to avoid
> misunderstandings is always a good thing.
Lijo wasn't asking anything. There was no question in any of his emails on this thread which is all about the use of "?:", which is a well-established practice.
Why are we having a thread about the use of "?:" ?
Regards,
Luben
>
>> I hope the find+egrep above is also edifying, so you can use it in
>> the future in your learning process.
> I hope, you like my solution without using find. ;-)
>
>
> Kind regards,
>
> Paul
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/5] dpm/amd/pm: Navi10: Remove 0 MHz as a current clock frequency (v3)
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
` (3 preceding siblings ...)
2021-10-18 23:49 ` [PATCH 4/5] dpm/amd/pm: Sienna: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
@ 2021-10-18 23:49 ` Luben Tuikov
2021-10-19 7:23 ` [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Paul Menzel
5 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-18 23:49 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher, Lijo Lazar, Paul Menzel
A current value of a clock frequency of 0, means
that the IP block is in some kind of low power
state. Ignore it and don't report it here. Here we
only report the possible operating (non-zero)
frequencies of the block requested. So, if the
current clock value is 0, then print the DPM
frequencies, but don't report a current value.
v2: Don't report the minimum one as the current
one when reported one is 0, i.e. don't add an
asterisk (Lijo). LT: It is conceivable that this
may confuse user-mode tools if they scan and look
for a current one, i.e. look for an asterisk, but
they'll have to adapt and use other methods for
finding power states of the chip--we can't report
0 as current.
v3: Start the subject title with a verb. (PaulM)
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Cc: Lijo Lazar <Lijo.Lazar@amd.com>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 44 ++++++++++++-------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 646e9bbf8af42a..2af6fd336352aa 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,9 +1267,8 @@ static int navi10_print_clk_levels(struct smu_context *smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t curr_value = 0, value = 0, count = 0;
+ uint32_t curr_value, value, count;
uint32_t freq_value[3] = {0, 0, 0};
- uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
uint32_t gen_speed, lane_width;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
@@ -1279,6 +1278,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
(OverDriveTable_t *)table_context->overdrive_table;
struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
uint32_t min_value, max_value;
+ bool fine_grained;
smu_cmn_get_sysfs_buf(&buf, &size);
@@ -1296,12 +1296,20 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0,
+ &freq_value[0]);
+ if (ret)
+ return size;
+
ret = smu_v11_0_get_dpm_level_count(smu, clk_type, &count);
if (ret)
return size;
- if (!navi10_supports_fine_grained_dpm(smu, clk_type)) {
- for (i = 0; i < count; i++) {
+ fine_grained = navi10_supports_fine_grained_dpm(smu, clk_type);
+ if (!fine_grained) {
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", 0, freq_value[0],
+ curr_value == freq_value[0] ? "*" : "");
+ for (i = 1; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
if (ret)
return size;
@@ -1310,24 +1318,28 @@ static int navi10_print_clk_levels(struct smu_context *smu,
curr_value == value ? "*" : "");
}
} else {
- ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_value[0]);
- if (ret)
- return size;
+ freq_value[1] = curr_value ?: freq_value[0];
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_value[2]);
if (ret)
return size;
- freq_value[1] = curr_value;
- mark_index = curr_value == freq_value[0] ? 0 :
- curr_value == freq_value[2] ? 2 : 1;
- if (mark_index != 1)
- freq_value[1] = (freq_value[0] + freq_value[2]) / 2;
-
- for (i = 0; i < 3; i++) {
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_value[i],
- i == mark_index ? "*" : "");
+ if (freq_value[1] == freq_value[0]) {
+ i = 1;
+ count = 3;
+ } else if (freq_value[1] == freq_value[2]) {
+ i = 0;
+ count = 2;
+ } else {
+ i = 0;
+ count = 3;
}
+ for ( ; i < count; i++) {
+ size += sysfs_emit_at(buf, size,
+ "%d: %uMhz %s\n",
+ i, freq_value[i],
+ curr_value == freq_value[i] ? "*" : "");
+ }
}
break;
case SMU_PCIE:
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4)
2021-10-18 23:49 [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Luben Tuikov
` (4 preceding siblings ...)
2021-10-18 23:49 ` [PATCH 5/5] dpm/amd/pm: Navi10: Remove 0 MHz as a current clock frequency (v3) Luben Tuikov
@ 2021-10-19 7:23 ` Paul Menzel
2021-10-19 7:43 ` Luben Tuikov
5 siblings, 1 reply; 18+ messages in thread
From: Paul Menzel @ 2021-10-19 7:23 UTC (permalink / raw)
To: Luben Tuikov; +Cc: amd-gfx
Dear Luben,
Sorry, two more style nits.
1. Could you please use 75 characters per line for the text width of
the commit messages. Currently, especially 4/5, are hard to read being
so short.
2. No idea, what is done in amd-gfx, but for me it is more common to
put the iteration number (reroll count) in the PATCH tag in the
beginning. No idea, how Patchwork deals with it.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4)
2021-10-19 7:23 ` [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4) Paul Menzel
@ 2021-10-19 7:43 ` Luben Tuikov
2021-10-19 7:54 ` Paul Menzel
0 siblings, 1 reply; 18+ messages in thread
From: Luben Tuikov @ 2021-10-19 7:43 UTC (permalink / raw)
To: Paul Menzel; +Cc: amd-gfx, Deucher, Alexander, Tuikov, Luben
On 2021-10-19 03:23, Paul Menzel wrote:
> Dear Luben,
>
>
> Sorry, two more style nits.
>
> 1. Could you please use 75 characters per line for the text width of
> the commit messages. Currently, especially 4/5, are hard to read being
> so short.
This is the default we use--I've not made any changes to the wrap. git-log(1) indents the text by 4/8 chars and it looks better if the text doesn't roll past 75 chars per line in git-log.
>
> 2. No idea, what is done in amd-gfx, but for me it is more common to
> put the iteration number (reroll count) in the PATCH tag in the
> beginning. No idea, how Patchwork deals with it.
This is what we do in amd-gfx and particularly in amdgpu, so that the version of the patch is recorded in the title of the patch and in history.
Regards,
Luben
>
>
> Kind regards,
>
> Paul
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/5] Remove 0 MHz as a valid current frequency (v4)
2021-10-19 7:43 ` Luben Tuikov
@ 2021-10-19 7:54 ` Paul Menzel
0 siblings, 0 replies; 18+ messages in thread
From: Paul Menzel @ 2021-10-19 7:54 UTC (permalink / raw)
To: Luben Tuikov; +Cc: amd-gfx, Alexander Deucher
Dear Luben,
Thank you for your quick reply.
Am 19.10.21 um 09:43 schrieb Luben Tuikov:
> On 2021-10-19 03:23, Paul Menzel wrote:
>> Sorry, two more style nits.
>>
>> 1. Could you please use 75 characters per line for the text width of
>> the commit messages. Currently, especially 4/5, are hard to read being
>> so short.
>
> This is the default we use--I've not made any changes to the wrap.
What do you mean? Your editor wraps the lines at the point, where you
configured it, doesn’t it?
> git-log(1) indents the text by 4/8 chars and it looks better if the
> text doesn't roll past 75 chars per line in git-log.
Patch 4/5 uses a text width of 50 characters, which is too short. From
commit 2a076f40d8c9 (checkpatch, SubmittingPatches: suggest line
wrapping commit messages at 75 columns) [1], which added a check for too
long lines:
> Suggest line wrapping at 75 columns so the default git commit log
> indentation of 4 plus the commit message text still fits on an 80 column
> screen.
>> 2. No idea, what is done in amd-gfx, but for me it is more common to
>> put the iteration number (reroll count) in the PATCH tag in the
>> beginning. No idea, how Patchwork deals with it.
>
> This is what we do in amd-gfx and particularly in amdgpu, so that the
> version of the patch is recorded in the title of the patch and in history.
I forgot. Thank you.
Kind regards,
Paul
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a076f40d8c9be95bee7bcf18436655e1140447f
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value
2021-10-15 0:50 [PATCH 0/5] 0 MHz is not a valid current frequency (v3) Luben Tuikov
@ 2021-10-15 0:50 ` Luben Tuikov
0 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-15 0:50 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher
Rename "cur_value", which stands for "cursor
value" to "curr_value", which stands for "current
value".
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 ++++++------
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++-------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 0fe9790f67f5af..f810549df493d5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,7 +1267,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
@@ -1292,7 +1292,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
case SMU_VCLK:
case SMU_DCLK:
case SMU_DCEFCLK:
- ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
return size;
@@ -1307,7 +1307,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
return size;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1317,9 +1317,9 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
if (mark_index != 1)
freq_values[1] = (freq_values[0] + freq_values[2]) / 2;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3f5721baa5ff50..3ebded3a99b5f2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1052,7 +1052,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
@@ -1073,10 +1073,11 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
case SMU_DCLK:
case SMU_DCLK1:
case SMU_DCEFCLK:
- ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
goto print_clk_out;
+
/* no need to disable gfxoff when retrieving the current gfxclk */
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
amdgpu_gfx_off_ctrl(adev, false);
@@ -1092,7 +1093,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
goto print_clk_out;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1102,9 +1103,9 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
@@ -1114,7 +1115,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
for (i = 0; i < count; i++) {
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
- cur_value == freq_values[i] ? "*" : "");
+ curr_value == freq_values[i] ? "*" : "");
}
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value
2021-10-13 16:11 [PATCH 0/5] 0 MHz is not a valid current frequency (v2) Luben Tuikov
@ 2021-10-13 16:11 ` Luben Tuikov
0 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-13 16:11 UTC (permalink / raw)
To: amd-gfx; +Cc: Luben Tuikov, Alex Deucher
Rename "cur_value", which stands for "cursor
value" to "curr_value", which stands for "current
value".
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 ++++++------
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++-------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 0fe9790f67f5af..f810549df493d5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,7 +1267,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
@@ -1292,7 +1292,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
case SMU_VCLK:
case SMU_DCLK:
case SMU_DCEFCLK:
- ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
return size;
@@ -1307,7 +1307,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
return size;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1317,9 +1317,9 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
if (mark_index != 1)
freq_values[1] = (freq_values[0] + freq_values[2]) / 2;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3f5721baa5ff50..3ebded3a99b5f2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1052,7 +1052,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
@@ -1073,10 +1073,11 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
case SMU_DCLK:
case SMU_DCLK1:
case SMU_DCEFCLK:
- ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
goto print_clk_out;
+
/* no need to disable gfxoff when retrieving the current gfxclk */
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
amdgpu_gfx_off_ctrl(adev, false);
@@ -1092,7 +1093,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
goto print_clk_out;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1102,9 +1103,9 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
@@ -1114,7 +1115,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
for (i = 0; i < count; i++) {
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
- cur_value == freq_values[i] ? "*" : "");
+ curr_value == freq_values[i] ? "*" : "");
}
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] drm/amd/pm: Rename cur_value to curr_value
2021-10-13 3:10 [PATCH 0/5] 0 MHz is not a valid current frequency Luben Tuikov
@ 2021-10-13 3:10 ` Luben Tuikov
0 siblings, 0 replies; 18+ messages in thread
From: Luben Tuikov @ 2021-10-13 3:10 UTC (permalink / raw)
To: amd-gfx; +Cc: Alex Deucher, Luben Tuikov
Rename "cur_value", which stands for "cursor
value" to "curr_value", which stands for "current
value".
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 ++++++------
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++-------
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 0fe9790f67f5af..f810549df493d5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1267,7 +1267,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
{
uint16_t *curve_settings;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
struct smu_table_context *table_context = &smu->smu_table;
@@ -1292,7 +1292,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
case SMU_VCLK:
case SMU_DCLK:
case SMU_DCEFCLK:
- ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
return size;
@@ -1307,7 +1307,7 @@ static int navi10_print_clk_levels(struct smu_context *smu,
return size;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1317,9 +1317,9 @@ static int navi10_print_clk_levels(struct smu_context *smu,
if (ret)
return size;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
if (mark_index != 1)
freq_values[1] = (freq_values[0] + freq_values[2]) / 2;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 3f5721baa5ff50..3ebded3a99b5f2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1052,7 +1052,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
OverDriveTable_t *od_table =
(OverDriveTable_t *)table_context->overdrive_table;
int i, size = 0, ret = 0;
- uint32_t cur_value = 0, value = 0, count = 0;
+ uint32_t curr_value = 0, value = 0, count = 0;
uint32_t freq_values[3] = {0};
uint32_t mark_index = 0;
uint32_t gen_speed, lane_width;
@@ -1073,10 +1073,11 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
case SMU_DCLK:
case SMU_DCLK1:
case SMU_DCEFCLK:
- ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ ret = sienna_cichlid_get_current_clk_freq_by_table(smu, clk_type, &curr_value);
if (ret)
goto print_clk_out;
+
/* no need to disable gfxoff when retrieving the current gfxclk */
if ((clk_type == SMU_GFXCLK) || (clk_type == SMU_SCLK))
amdgpu_gfx_off_ctrl(adev, false);
@@ -1092,7 +1093,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
goto print_clk_out;
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, value,
- cur_value == value ? "*" : "");
+ curr_value == value ? "*" : "");
}
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
@@ -1102,9 +1103,9 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
if (ret)
goto print_clk_out;
- freq_values[1] = cur_value;
- mark_index = cur_value == freq_values[0] ? 0 :
- cur_value == freq_values[2] ? 2 : 1;
+ freq_values[1] = curr_value;
+ mark_index = curr_value == freq_values[0] ? 0 :
+ curr_value == freq_values[2] ? 2 : 1;
count = 3;
if (mark_index != 1) {
@@ -1114,7 +1115,7 @@ static int sienna_cichlid_print_clk_levels(struct smu_context *smu,
for (i = 0; i < count; i++) {
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, freq_values[i],
- cur_value == freq_values[i] ? "*" : "");
+ curr_value == freq_values[i] ? "*" : "");
}
}
--
2.33.1.558.g2bd2f258f4
^ permalink raw reply related [flat|nested] 18+ messages in thread