Re: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling
As discussed previous I don't think this is the right approach.
The distinction between sysfs_emit() and sysfs_emit_at() is exactly to
avoid that kind of stuff.
Instead we should probably add the size parameter to the functions in
question and so fix the calling convention.
Or even better move the sysfs print out of the powerplay backend.
Regards,
Christian.
Am 04.11.21 um 01:58 schrieb Alex Deucher:
sysfs_emit and sysfs_emit_at requrie a page boundary
aligned buf address. Make them happy!
v2: fix sysfs_emit -> sysfs_emit_at missed conversions
Cc: Lang Yu
Cc: Darren Powell
Fixes: 6db0c87a0a8e ("amdgpu/pm: Replace hwmgr smu usage of sprintf with
sysfs_emit")
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1774
Signed-off-by: Alex Deucher
---
.../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 8 ++--
.../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c| 10 +++---
.../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c| 2 ++
.../gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h| 13 +
.../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 12 +---
.../gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c | 4
.../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 14 ++
7 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
index 1de3ae77e03e..258c573acc97 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
@@ -1024,6 +1024,8 @@ static int smu10_print_clock_levels(struct pp_hwmgr
*hwmgr,
uint32_t min_freq, max_freq = 0;
uint32_t ret = 0;
+ phm_get_sysfs_buf(&buf, &size);
+
switch (type) {
case PP_SCLK:
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetGfxclkFrequency, &now);
@@ -1065,7 +1067,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr
*hwmgr,
if (ret)
return ret;
- size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
(data->gfx_actual_soft_min_freq > 0) ?
data->gfx_actual_soft_min_freq : min_freq);
size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
@@ -1081,7 +1083,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr
*hwmgr,
if (ret)
return ret;
- size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
size += sysfs_emit_at(buf, size, "SCLK: %7uMHz
%10uMHz\n",
min_freq, max_freq);
}
@@ -1456,6 +1458,8 @@ static int smu10_get_power_profile_mode(struct pp_hwmgr
*hwmgr, char *buf)
if (!buf)
return -EINVAL;
+ phm_get_sysfs_buf(&buf, &size);
+
size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
title[1], title[2], title[3], title[4], title[5]);
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index e7803ce8f67a..aceebf584225 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -4914,6 +4914,8 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
int size = 0;
uint32_t i, now, clock, pcie_speed;
+ phm_get_sysfs_buf(&buf, &size);
+
switch (type) {
case PP_SCLK:
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_API_GetSclkFrequency,
&clock);
@@ -4963,7 +4965,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
break;
case OD_SCLK:
if (hwmgr->od_enabled) {
- size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
for (i = 0; i < odn_sclk_table->num_of_pl; i++)
size += sysfs_emit_at(buf, size, "%d: %10uMHz
%10umV\n",
i, odn_sclk_table->entries[i].clock/100,
@@ -4972,7 +4974,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
break;
case OD_MCLK:
if (hwmgr->od_enabled) {
- size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
for (i = 0; i < odn_mclk_table->num_of_pl; i++)
size += sysfs_emit_at(buf, size, "%d: %10uMHz
%10umV\n",
i, odn_mclk_table->entries[i].clock/100,
@@ -4981,7 +4983,7 @@ static int smu7_print_clock_levels(str
RE: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling
[AMD Official Use Only]
Yes, I missed such conversions in powerplay. Thanks!
Reviewed-by: Lang Yu
>-Original Message-
>From: Deucher, Alexander
>Sent: Thursday, November 4, 2021 8:59 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Yu, Lang
>; Powell, Darren
>Subject: [PATCH] drm/amdgpu/powerplay: fix sysfs_emit/sysfs_emit_at handling
>
>sysfs_emit and sysfs_emit_at requrie a page boundary aligned buf address. Make
>them happy!
>
>v2: fix sysfs_emit -> sysfs_emit_at missed conversions
>
>Cc: Lang Yu
>Cc: Darren Powell
>Fixes: 6db0c87a0a8e ("amdgpu/pm: Replace hwmgr smu usage of sprintf with
>sysfs_emit")
>Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1774
>Signed-off-by: Alex Deucher
>---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c | 8 ++--
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c| 10 +++---
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c| 2 ++
> .../gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h| 13 +
> .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 12 +---
> .../gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c | 4
> .../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 14
>++
> 7 files changed, 51 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>index 1de3ae77e03e..258c573acc97 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
>@@ -1024,6 +1024,8 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> uint32_t min_freq, max_freq = 0;
> uint32_t ret = 0;
>
>+ phm_get_sysfs_buf(&buf, &size);
>+
> switch (type) {
> case PP_SCLK:
> smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_GetGfxclkFrequency, &now); @@ -1065,7 +1067,7 @@ static int
>smu10_print_clock_levels(struct pp_hwmgr *hwmgr,
> if (ret)
> return ret;
>
>- size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
> (data->gfx_actual_soft_min_freq > 0) ? data-
>>gfx_actual_soft_min_freq : min_freq);
> size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@ -
>1081,7 +1083,7 @@ static int smu10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> if (ret)
> return ret;
>
>- size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
> size += sysfs_emit_at(buf, size,
>"SCLK: %7uMHz %10uMHz\n",
> min_freq, max_freq);
> }
>@@ -1456,6 +1458,8 @@ static int smu10_get_power_profile_mode(struct
>pp_hwmgr *hwmgr, char *buf)
> if (!buf)
> return -EINVAL;
>
>+ phm_get_sysfs_buf(&buf, &size);
>+
> size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
> title[1], title[2], title[3], title[4], title[5]);
>
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>index e7803ce8f67a..aceebf584225 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>@@ -4914,6 +4914,8 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> int size = 0;
> uint32_t i, now, clock, pcie_speed;
>
>+ phm_get_sysfs_buf(&buf, &size);
>+
> switch (type) {
> case PP_SCLK:
> smum_send_msg_to_smc(hwmgr,
>PPSMC_MSG_API_GetSclkFrequency, &clock); @@ -4963,7 +4965,7 @@ static int
>smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
> break;
> case OD_SCLK:
> if (hwmgr->od_enabled) {
>- size = sysfs_emit(buf, "%s:\n", "OD_SCLK");
>+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_SCLK");
> for (i = 0; i < odn_sclk_table->num_of_pl; i++)
> size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
> i, odn_sclk_table->entries[i].clock/100,
>@@ -4972,7 +4974,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr
>*hwmgr,
> break;
> case OD_MCLK:
> if (hwmgr->od_enabled) {
>- size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+ size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
> for (i = 0; i < odn_mclk_table->num_of_pl; i++)
> size += sysfs_emit_at(buf, size,
>"%d: %10uMHz %10umV\n",
> i, odn_mclk_table->entries[i].clock/100,
>@@ -4981,7 +4983,7 @@
