* [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array
@ 2022-05-09 3:58 Darren Powell
2022-05-09 3:58 ` [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2 Darren Powell
2022-05-09 4:47 ` [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Lazar, Lijo
0 siblings, 2 replies; 5+ messages in thread
From: Darren Powell @ 2022-05-09 3:58 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, lijo.lazar, Le.Ma, Darren Powell, evan.quan,
kenneth.feng
Size of pp_clock_levels_with_latency is PP_MAX_CLOCK_LEVELS, not MAX_NUM_CLOCKS.
Both are currently defined as 16, modifying in case one value is modified in future
Changed code in both arcturus and aldabaran.
Also removed unneeded var count, and used min_t function
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 9 +++++----
drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 9 +++++----
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 201563072189..bdd1e1a35a12 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -573,12 +573,13 @@ static int arcturus_get_clk_table(struct smu_context *smu,
struct pp_clock_levels_with_latency *clocks,
struct smu_11_0_dpm_table *dpm_table)
{
- int i, count;
+ uint32_t i;
- count = (dpm_table->count > MAX_NUM_CLOCKS) ? MAX_NUM_CLOCKS : dpm_table->count;
- clocks->num_levels = count;
+ clocks->num_levels = min_t(uint32_t,
+ dpm_table->count,
+ (uint32_t)PP_MAX_CLOCK_LEVELS);
- for (i = 0; i < count; i++) {
+ for (i = 0; i < clocks->num_levels; i++) {
clocks->data[i].clocks_in_khz =
dpm_table->dpm_levels[i].value * 1000;
clocks->data[i].latency_in_us = 0;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 38af648cb857..6a4fca47ae53 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -549,12 +549,13 @@ static int aldebaran_get_clk_table(struct smu_context *smu,
struct pp_clock_levels_with_latency *clocks,
struct smu_13_0_dpm_table *dpm_table)
{
- int i, count;
+ uint32_t i;
- count = (dpm_table->count > MAX_NUM_CLOCKS) ? MAX_NUM_CLOCKS : dpm_table->count;
- clocks->num_levels = count;
+ clocks->num_levels = min_t(uint32_t,
+ dpm_table->count,
+ (uint32_t)PP_MAX_CLOCK_LEVELS);
- for (i = 0; i < count; i++) {
+ for (i = 0; i < clocks->num_levels; i++) {
clocks->data[i].clocks_in_khz =
dpm_table->dpm_levels[i].value * 1000;
clocks->data[i].latency_in_us = 0;
base-commit: 8bb14fbec5ae45c31cbefe217db2418cc683612f
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2
2022-05-09 3:58 [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Darren Powell
@ 2022-05-09 3:58 ` Darren Powell
2022-05-09 4:50 ` Lazar, Lijo
2022-05-09 4:47 ` [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Lazar, Lijo
1 sibling, 1 reply; 5+ messages in thread
From: Darren Powell @ 2022-05-09 3:58 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, lijo.lazar, Le.Ma, Darren Powell, evan.quan,
kenneth.feng
added a check to populate and use SCLK shim table freq_values only
if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or
AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM
removed clocks.num_levels from calculation of shim table size
removed unsafe accesses to shim table freq_values
output gfx_table values if using other dpm levels
added check for freq_match when using freq_values for when now == min_clk
== Test ==
LOGFILE=aldebaran-sclk.test.log
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" > $LOGFILE
FILES="pp_od_clk_voltage
pp_dpm_sclk"
for f in $FILES
do
echo === $f === >> $LOGFILE
cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 60 +++++++++----------
1 file changed, 29 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 6a4fca47ae53..a653668e8402 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -740,9 +740,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
struct smu_13_0_dpm_table *single_dpm_table;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct smu_13_0_dpm_context *dpm_context = NULL;
- uint32_t display_levels;
uint32_t freq_values[3] = {0};
- uint32_t min_clk, max_clk;
+ uint32_t min_clk, max_clk, display_levels = 0;
smu_cmn_get_sysfs_buf(&buf, &size);
@@ -765,46 +764,45 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
return ret;
}
- single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
- return ret;
- }
-
- display_levels = clocks.num_levels;
-
- min_clk = pstate_table->gfxclk_pstate.curr.min;
- max_clk = pstate_table->gfxclk_pstate.curr.max;
-
- freq_values[0] = min_clk;
- freq_values[1] = max_clk;
-
- /* fine-grained dpm has only 2 levels */
- if (now > min_clk && now < max_clk) {
- display_levels = clocks.num_levels + 1;
- freq_values[2] = max_clk;
- freq_values[1] = now;
- }
+ if ((smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
+ smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) {
+ single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
+ ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
+ if (ret) {
+ dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
+ return ret;
+ }
- /*
- * For DPM disabled case, there will be only one clock level.
- * And it's safe to assume that is always the current clock.
- */
- if (display_levels == clocks.num_levels) {
for (i = 0; i < clocks.num_levels; i++)
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
- freq_values[i],
+ clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ?
"*" :
(aldebaran_freqs_in_same_level(
- freq_values[i], now) ?
+ clocks.data[i].clocks_in_khz / 1000, now) ?
"*" :
""));
} else {
+ /* fine-grained dpm has only 2 levels */
+ display_levels = 2;
+
+ min_clk = pstate_table->gfxclk_pstate.curr.min;
+ max_clk = pstate_table->gfxclk_pstate.curr.max;
+
+ freq_values[0] = min_clk;
+ freq_values[1] = max_clk;
+
+ if (now > min_clk && now < max_clk) {
+ display_levels++;
+ freq_values[2] = max_clk;
+ freq_values[1] = now;
+ }
+
for (i = 0; i < display_levels; i++)
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
- freq_values[i], i == 1 ? "*" : "");
+ freq_values[i],
+ aldebaran_freqs_in_same_level(freq_values[i], now) ?
+ "*" : "");
}
break;
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array
2022-05-09 3:58 [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Darren Powell
2022-05-09 3:58 ` [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2 Darren Powell
@ 2022-05-09 4:47 ` Lazar, Lijo
1 sibling, 0 replies; 5+ messages in thread
From: Lazar, Lijo @ 2022-05-09 4:47 UTC (permalink / raw)
To: Darren Powell, amd-gfx; +Cc: Le.Ma, kevin1.wang, kenneth.feng, evan.quan
On 5/9/2022 9:28 AM, Darren Powell wrote:
> Size of pp_clock_levels_with_latency is PP_MAX_CLOCK_LEVELS, not MAX_NUM_CLOCKS.
> Both are currently defined as 16, modifying in case one value is modified in future
> Changed code in both arcturus and aldabaran.
>
> Also removed unneeded var count, and used min_t function
>
> Signed-off-by: Darren Powell <darren.powell@amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
> ---
> drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 9 +++++----
> drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 9 +++++----
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> index 201563072189..bdd1e1a35a12 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> @@ -573,12 +573,13 @@ static int arcturus_get_clk_table(struct smu_context *smu,
> struct pp_clock_levels_with_latency *clocks,
> struct smu_11_0_dpm_table *dpm_table)
> {
> - int i, count;
> + uint32_t i;
>
> - count = (dpm_table->count > MAX_NUM_CLOCKS) ? MAX_NUM_CLOCKS : dpm_table->count;
> - clocks->num_levels = count;
> + clocks->num_levels = min_t(uint32_t,
> + dpm_table->count,
> + (uint32_t)PP_MAX_CLOCK_LEVELS);
>
> - for (i = 0; i < count; i++) {
> + for (i = 0; i < clocks->num_levels; i++) {
> clocks->data[i].clocks_in_khz =
> dpm_table->dpm_levels[i].value * 1000;
> clocks->data[i].latency_in_us = 0;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 38af648cb857..6a4fca47ae53 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -549,12 +549,13 @@ static int aldebaran_get_clk_table(struct smu_context *smu,
> struct pp_clock_levels_with_latency *clocks,
> struct smu_13_0_dpm_table *dpm_table)
> {
> - int i, count;
> + uint32_t i;
>
> - count = (dpm_table->count > MAX_NUM_CLOCKS) ? MAX_NUM_CLOCKS : dpm_table->count;
> - clocks->num_levels = count;
> + clocks->num_levels = min_t(uint32_t,
> + dpm_table->count,
> + (uint32_t)PP_MAX_CLOCK_LEVELS);
>
> - for (i = 0; i < count; i++) {
> + for (i = 0; i < clocks->num_levels; i++) {
> clocks->data[i].clocks_in_khz =
> dpm_table->dpm_levels[i].value * 1000;
> clocks->data[i].latency_in_us = 0;
>
> base-commit: 8bb14fbec5ae45c31cbefe217db2418cc683612f
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2
2022-05-09 3:58 ` [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2 Darren Powell
@ 2022-05-09 4:50 ` Lazar, Lijo
2022-05-10 22:07 ` Powell, Darren
0 siblings, 1 reply; 5+ messages in thread
From: Lazar, Lijo @ 2022-05-09 4:50 UTC (permalink / raw)
To: Darren Powell, amd-gfx; +Cc: Le.Ma, kevin1.wang, kenneth.feng, evan.quan
On 5/9/2022 9:28 AM, Darren Powell wrote:
> added a check to populate and use SCLK shim table freq_values only
> if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or
> AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM
> removed clocks.num_levels from calculation of shim table size
> removed unsafe accesses to shim table freq_values
> output gfx_table values if using other dpm levels
> added check for freq_match when using freq_values for when now == min_clk
>
> == Test ==
> LOGFILE=aldebaran-sclk.test.log
> AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
> AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
> HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
>
> lspci -nn | grep "VGA\|Display" > $LOGFILE
> FILES="pp_od_clk_voltage
> pp_dpm_sclk"
>
> for f in $FILES
> do
> echo === $f === >> $LOGFILE
> cat $HWMON_DIR/device/$f >> $LOGFILE
> done
> cat $LOGFILE
>
> Signed-off-by: Darren Powell <darren.powell@amd.com>
> ---
> .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 60 +++++++++----------
> 1 file changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 6a4fca47ae53..a653668e8402 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -740,9 +740,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
> struct smu_13_0_dpm_table *single_dpm_table;
> struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> struct smu_13_0_dpm_context *dpm_context = NULL;
> - uint32_t display_levels;
> uint32_t freq_values[3] = {0};
> - uint32_t min_clk, max_clk;
> + uint32_t min_clk, max_clk, display_levels = 0;
>
> smu_cmn_get_sysfs_buf(&buf, &size);
>
> @@ -765,46 +764,45 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
> return ret;
> }
>
> - single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
> - ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
> - return ret;
> - }
> -
> - display_levels = clocks.num_levels;
> -
> - min_clk = pstate_table->gfxclk_pstate.curr.min;
> - max_clk = pstate_table->gfxclk_pstate.curr.max;
> -
> - freq_values[0] = min_clk;
> - freq_values[1] = max_clk;
> -
> - /* fine-grained dpm has only 2 levels */
> - if (now > min_clk && now < max_clk) {
> - display_levels = clocks.num_levels + 1;
> - freq_values[2] = max_clk;
> - freq_values[1] = now;
> - }
> + if ((smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
> + smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) {
> + single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
> + ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> + if (ret) {
> + dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
> + return ret;
> + }
There are only two levels for GFX clock in aldebaran - min and max.
Regardless of the mode, gfxclk_pstate.curr.min/max should reflect the
current min/max level.
Could you explain the issue you are seeing? It's not so clear from the
commit message.
Thanks,
Lijo
>
> - /*
> - * For DPM disabled case, there will be only one clock level.
> - * And it's safe to assume that is always the current clock.
> - */
> - if (display_levels == clocks.num_levels) {
> for (i = 0; i < clocks.num_levels; i++)
> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
> - freq_values[i],
> + clocks.data[i].clocks_in_khz / 1000,
> (clocks.num_levels == 1) ?
> "*" :
> (aldebaran_freqs_in_same_level(
> - freq_values[i], now) ?
> + clocks.data[i].clocks_in_khz / 1000, now) ?
> "*" :
> ""));
> } else {
> + /* fine-grained dpm has only 2 levels */
> + display_levels = 2;
> +
> + min_clk = pstate_table->gfxclk_pstate.curr.min;
> + max_clk = pstate_table->gfxclk_pstate.curr.max;
> +
> + freq_values[0] = min_clk;
> + freq_values[1] = max_clk;
> +
> + if (now > min_clk && now < max_clk) {
> + display_levels++;
> + freq_values[2] = max_clk;
> + freq_values[1] = now;
> + }
> +
> for (i = 0; i < display_levels; i++)
> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
> - freq_values[i], i == 1 ? "*" : "");
> + freq_values[i],
> + aldebaran_freqs_in_same_level(freq_values[i], now) ?
> + "*" : "");
> }
>
> break;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2
2022-05-09 4:50 ` Lazar, Lijo
@ 2022-05-10 22:07 ` Powell, Darren
0 siblings, 0 replies; 5+ messages in thread
From: Powell, Darren @ 2022-05-10 22:07 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx@lists.freedesktop.org
Cc: Ma, Le, Quan, Evan, Feng, Kenneth, Wang, Yang(Kevin)
[-- Attachment #1: Type: text/plain, Size: 6995 bytes --]
[AMD Official Use Only - General]
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Monday, May 9, 2022 12:50 AM
To: Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Quan, Evan <Evan.Quan@amd.com>; Ma, Le <Le.Ma@amd.com>
Subject: Re: [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2
On 5/9/2022 9:28 AM, Darren Powell wrote:
> added a check to populate and use SCLK shim table freq_values only
> if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or
> AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM
> removed clocks.num_levels from calculation of shim table size
> removed unsafe accesses to shim table freq_values
> output gfx_table values if using other dpm levels
> added check for freq_match when using freq_values for when now == min_clk
>
> == Test ==
> LOGFILE=aldebaran-sclk.test.log
> AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
> AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
> HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
>
> lspci -nn | grep "VGA\|Display" > $LOGFILE
> FILES="pp_od_clk_voltage
> pp_dpm_sclk"
>
> for f in $FILES
> do
> echo === $f === >> $LOGFILE
> cat $HWMON_DIR/device/$f >> $LOGFILE
> done
> cat $LOGFILE
>
> Signed-off-by: Darren Powell <darren.powell@amd.com>
> ---
> .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 60 +++++++++----------
> 1 file changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 6a4fca47ae53..a653668e8402 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -740,9 +740,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
> struct smu_13_0_dpm_table *single_dpm_table;
> struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> struct smu_13_0_dpm_context *dpm_context = NULL;
> - uint32_t display_levels;
> uint32_t freq_values[3] = {0};
> - uint32_t min_clk, max_clk;
> + uint32_t min_clk, max_clk, display_levels = 0;
>
> smu_cmn_get_sysfs_buf(&buf, &size);
>
> @@ -765,46 +764,45 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
> return ret;
> }
>
> - single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
> - ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
> - return ret;
> - }
> -
> - display_levels = clocks.num_levels;
> -
> - min_clk = pstate_table->gfxclk_pstate.curr.min;
> - max_clk = pstate_table->gfxclk_pstate.curr.max;
> -
> - freq_values[0] = min_clk;
> - freq_values[1] = max_clk;
> -
> - /* fine-grained dpm has only 2 levels */
> - if (now > min_clk && now < max_clk) {
> - display_levels = clocks.num_levels + 1;
> - freq_values[2] = max_clk;
> - freq_values[1] = now;
> - }
> + if ((smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
> + smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) {
> + single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
> + ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> + if (ret) {
> + dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
> + return ret;
> + }
There are only two levels for GFX clock in aldebaran - min and max.
Regardless of the mode, gfxclk_pstate.curr.min/max should reflect the
current min/max level.
Could you explain the issue you are seeing? It's not so clear from the
commit message.
Thanks,
Lijo
[DP] I was concerned by the initialization of display_levels from clocks.num_levels and how it is used as the loop iterator
while accessing the freq_values array. My mistake was that I assumed that meant you intended to access clocks.data array.
If aldebaran only uses the curr.min and curr.max values that simplifies this greatly,
I'll respin this to initialize display_levels to 2 , and then output from freq_values array.
>
> - /*
> - * For DPM disabled case, there will be only one clock level.
> - * And it's safe to assume that is always the current clock.
> - */
> - if (display_levels == clocks.num_levels) {
> for (i = 0; i < clocks.num_levels; i++)
> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
> - freq_values[i],
> + clocks.data[i].clocks_in_khz / 1000,
> (clocks.num_levels == 1) ?
> "*" :
> (aldebaran_freqs_in_same_level(
> - freq_values[i], now) ?
> + clocks.data[i].clocks_in_khz / 1000, now) ?
> "*" :
> ""));
> } else {
> + /* fine-grained dpm has only 2 levels */
> + display_levels = 2;
> +
> + min_clk = pstate_table->gfxclk_pstate.curr.min;
> + max_clk = pstate_table->gfxclk_pstate.curr.max;
> +
> + freq_values[0] = min_clk;
> + freq_values[1] = max_clk;
> +
> + if (now > min_clk && now < max_clk) {
> + display_levels++;
> + freq_values[2] = max_clk;
> + freq_values[1] = now;
> + }
> +
> for (i = 0; i < display_levels; i++)
> size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
> - freq_values[i], i == 1 ? "*" : "");
> + freq_values[i],
> + aldebaran_freqs_in_same_level(freq_values[i], now) ?
> + "*" : "");
> }
>
> break;
>
[-- Attachment #2: Type: text/html, Size: 17373 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-10 22:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-09 3:58 [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Darren Powell
2022-05-09 3:58 ` [PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2 Darren Powell
2022-05-09 4:50 ` Lazar, Lijo
2022-05-10 22:07 ` Powell, Darren
2022-05-09 4:47 ` [PATCH v1 1/2] amdgpu/pm: Fix incorrect variable for size of clocks array Lazar, Lijo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox