[AMD Official Use Only]

Yes, I missed such conversions in powerplay. Thanks!

Reviewed-by: Lang Yu <lang...@amd.com> 

>-----Original Message-----
>From: Deucher, Alexander <alexander.deuc...@amd.com>
>Sent: Thursday, November 4, 2021 8:59 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Yu, Lang
><lang...@amd.com>; Powell, Darren <darren.pow...@amd.com>
>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 <lang...@amd.com>
>Cc: Darren Powell <darren.pow...@amd.com>
>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 <alexander.deuc...@amd.com>
>---
> .../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(struct pp_hwmgr
>*hwmgr,
>               break;
>       case OD_RANGE:
>               if (hwmgr->od_enabled) {
>-                      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",
>                               data-
>>golden_dpm_table.sclk_table.dpm_levels[0].value/100,
>                               hwmgr-
>>platform_descriptor.overdriveLimit.engineClock/100);
>@@ -5518,6 +5520,8 @@ static int smu7_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 %16s %16s %16s %16s %16s %16s\n",
>                       title[0], title[1], title[2], title[3],
>                       title[4], title[5], title[6], title[7]); diff --git
>a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>index b94a77e4e714..8e28a8eecefc 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c
>@@ -1550,6 +1550,8 @@ static int smu8_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>       uint32_t i, now;
>       int size = 0;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       switch (type) {
>       case PP_SCLK:
>               now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>index ad33983a8064..2a75da1e9f03 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu_helper.h
>@@ -109,6 +109,19 @@ int phm_irq_process(struct amdgpu_device *adev,
>                          struct amdgpu_irq_src *source,
>                          struct amdgpu_iv_entry *entry);
>
>+/*
>+ * Helper function to make sysfs_emit_at() happy. Align buf to
>+ * the current page boundary and record the offset.
>+ */
>+static inline void phm_get_sysfs_buf(char **buf, int *offset) {
>+      if (!*buf || !offset)
>+              return;
>+
>+      *offset = offset_in_page(*buf);
>+      *buf -= *offset;
>+}
>+
> int smu9_register_irq_handlers(struct pp_hwmgr *hwmgr);
>
> void *smu_atom_get_data_table(void *dev, uint32_t table, uint16_t *size, diff 
> --
>git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>index c152a61ddd2c..c981fc2882f0 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
>@@ -4548,6 +4548,8 @@ static int vega10_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
>       int ret = 0;
>       int size = 0;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       ret = vega10_get_enabled_smc_features(hwmgr, &features_enabled);
>       PP_ASSERT_WITH_CODE(!ret,
>                       "[EnableAllSmuFeatures] Failed to get enabled smc
>features!", @@ -4637,6 +4639,8 @@ static int vega10_print_clock_levels(struct
>pp_hwmgr *hwmgr,
>
>       int i, now, size = 0, count = 0;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       switch (type) {
>       case PP_SCLK:
>               if (data->registry_data.sclk_dpm_key_disabled)
>@@ -4717,7 +4721,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>
>       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");
>                       podn_vdd_dep = &data-
>>odn_dpm_table.vdd_dep_on_sclk;
>                       for (i = 0; i < podn_vdd_dep->count; i++)
>                               size += sysfs_emit_at(buf, size,
>"%d: %10uMhz %10umV\n", @@ -4727,7 +4731,7 @@ static int
>vega10_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");
>                       podn_vdd_dep = &data-
>>odn_dpm_table.vdd_dep_on_mclk;
>                       for (i = 0; i < podn_vdd_dep->count; i++)
>                               size += sysfs_emit_at(buf, size,
>"%d: %10uMhz %10umV\n", @@ -4737,7 +4741,7 @@ static int
>vega10_print_clock_levels(struct pp_hwmgr *hwmgr,
>               break;
>       case OD_RANGE:
>               if (hwmgr->od_enabled) {
>-                      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",
>                               data-
>>golden_dpm_table.gfx_table.dpm_levels[0].value/100,
>                               hwmgr-
>>platform_descriptor.overdriveLimit.engineClock/100);
>@@ -5112,6 +5116,8 @@ static int vega10_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/vega12_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>index 8558718e15a8..f7e783e1c888 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega12_hwmgr.c
>@@ -2141,6 +2141,8 @@ static int vega12_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
>       int ret = 0;
>       int size = 0;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       ret = vega12_get_enabled_smc_features(hwmgr, &features_enabled);
>       PP_ASSERT_WITH_CODE(!ret,
>               "[EnableAllSmuFeatures] Failed to get enabled smc features!",
>@@ -2244,6 +2246,8 @@ static int vega12_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>       int i, now, size = 0;
>       struct pp_clock_levels_with_latency clocks;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       switch (type) {
>       case PP_SCLK:
>               PP_ASSERT_WITH_CODE(
>diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>index 0cf39c1244b1..03e63be4ee27 100644
>--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
>@@ -3238,6 +3238,8 @@ static int vega20_get_ppfeature_status(struct
>pp_hwmgr *hwmgr, char *buf)
>       int ret = 0;
>       int size = 0;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       ret = vega20_get_enabled_smc_features(hwmgr, &features_enabled);
>       PP_ASSERT_WITH_CODE(!ret,
>                       "[EnableAllSmuFeatures] Failed to get enabled smc
>features!", @@ -3364,6 +3366,8 @@ static int vega20_print_clock_levels(struct
>pp_hwmgr *hwmgr,
>       int ret = 0;
>       uint32_t gen_speed, lane_width, current_gen_speed, current_lane_width;
>
>+      phm_get_sysfs_buf(&buf, &size);
>+
>       switch (type) {
>       case PP_SCLK:
>               ret = vega20_get_current_clk_freq(hwmgr, PPCLK_GFXCLK,
>&now); @@ -3479,7 +3483,7 @@ static int vega20_print_clock_levels(struct
>pp_hwmgr *hwmgr,
>       case OD_SCLK:
>               if (od8_settings[OD8_SETTING_GFXCLK_FMIN].feature_id &&
>                   od8_settings[OD8_SETTING_GFXCLK_FMAX].feature_id) {
>-                      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",
>                               od_table->GfxclkFmin);
>                       size += sysfs_emit_at(buf, size, "1: %10uMhz\n", @@ -
>3489,7 +3493,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>
>       case OD_MCLK:
>               if (od8_settings[OD8_SETTING_UCLK_FMAX].feature_id) {
>-                      size = sysfs_emit(buf, "%s:\n", "OD_MCLK");
>+                      size += sysfs_emit_at(buf, size, "%s:\n", "OD_MCLK");
>                       size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>                               od_table->UclkFmax);
>               }
>@@ -3503,7 +3507,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>                   od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id
>&&
>                   od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id
>&&
>                   od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id) {
>-                      size = sysfs_emit(buf, "%s:\n", "OD_VDDC_CURVE");
>+                      size += sysfs_emit_at(buf, size, "%s:\n",
>"OD_VDDC_CURVE");
>                       size += sysfs_emit_at(buf, size, "0: %10uMhz %10dmV\n",
>                               od_table->GfxclkFreq1,
>                               od_table->GfxclkVolt1 / VOLTAGE_SCALE); @@
>-3518,7 +3522,7 @@ static int vega20_print_clock_levels(struct pp_hwmgr
>*hwmgr,
>               break;
>
>       case OD_RANGE:
>-              size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>+              size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
>
>               if (od8_settings[OD8_SETTING_GFXCLK_FMIN].feature_id &&
>                   od8_settings[OD8_SETTING_GFXCLK_FMAX].feature_id) { @@
>-4003,6 +4007,8 @@ static int vega20_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, "%16s %s %s %s %s %s %s %s %s %s %s\n",
>                       title[0], title[1], title[2], title[3], title[4], 
> title[5],
>                       title[6], title[7], title[8], title[9], title[10]);
>--
>2.31.1

Reply via email to