From: "Lazar, Lijo" <lijo.lazar@amd.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
"Lang Yu" <lang.yu@amd.com>,
amd-gfx@lists.freedesktop.org
Cc: Alex Deucher <alexander.deucher@amd.com>,
Huang Rui <ray.huang@amd.com>, Tian Tao <tiantao6@hisilicon.com>
Subject: Re: [PATCH] drm/amdgpu: fix sysfs_emit/sysfs_emit_at warnings
Date: Wed, 8 Sep 2021 13:06:25 +0530 [thread overview]
Message-ID: <47ed143e-b9ab-a80e-dac0-cfa1ec39d033@amd.com> (raw)
In-Reply-To: <e8b39f62-ca0c-d4e0-92a9-52487fa0da81@gmail.com>
On 9/8/2021 12:07 PM, Christian König wrote:
> Am 08.09.21 um 07:56 schrieb Lang Yu:
>> sysfs_emit and sysfs_emit_at requrie a page boundary
>> aligned buf address. Make them happy!
>>
>> Warning Log:
>> [ 492.545174] invalid sysfs_emit_at: buf:00000000f19bdfde at:0
>> [ 492.546416] WARNING: CPU: 7 PID: 1304 at fs/sysfs/file.c:765
>> sysfs_emit_at+0x4a/0xa0
>> [ 492.654805] Call Trace:
>> [ 492.655353] ? smu_cmn_get_metrics_table+0x40/0x50 [amdgpu]
>> [ 492.656780] vangogh_print_clk_levels+0x369/0x410 [amdgpu]
>> [ 492.658245] vangogh_common_print_clk_levels+0x77/0x80 [amdgpu]
>> [ 492.659733] ? preempt_schedule_common+0x18/0x30
>> [ 492.660713] smu_print_ppclk_levels+0x65/0x90 [amdgpu]
>> [ 492.662107] amdgpu_get_pp_od_clk_voltage+0x13d/0x190 [amdgpu]
>> [ 492.663620] dev_attr_show+0x1d/0x40
>
> Mhm, that at least partially doesn't looks like the right approach to me.
>
> Why do we have string printing and sysfs code in the hardware version
> specific backend in the first place?
>
This is a callback meant for printing ASIC specific information to sysfs
node. The buffer passed in sysfs read is passed as it is to the callback
API.
> That stuff needs to be implemented for each hardware generation and is
> now cluttered with sysfs buffer offset calculations.
>
Looks like the warning happened because of this usage.
size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK,
buf+size);
size += amdgpu_dpm_print_clock_levels(adev,
OD_VDDC_CURVE, buf+size);
size += amdgpu_dpm_print_clock_levels(adev,
OD_VDDGFX_OFFSET, buf+size);
size += amdgpu_dpm_print_clock_levels(adev, OD_RANGE,
buf+size);
size += amdgpu_dpm_print_clock_levels(adev, OD_CCLK,
buf+size);
> Regards,
> Christian.
>
>>
>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>> ---
>> drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 9 +++++++--
>> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 5 ++++-
>> .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 5 ++++-
>> drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 15 +++++++++------
>> drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 3 +++
>> .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 13 +++++++++----
>> .../gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 7 +++++--
>> 7 files changed, 41 insertions(+), 16 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 e343cc218990..53185fe96d83 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>> @@ -771,8 +771,13 @@ static int arcturus_print_clk_levels(struct
>> smu_context *smu,
>> struct smu_11_0_dpm_context *dpm_context = NULL;
>> uint32_t gen_speed, lane_width;
>> - if (amdgpu_ras_intr_triggered())
>> - return sysfs_emit(buf, "unavailable\n");
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> + if (amdgpu_ras_intr_triggered()) {
>> + size += sysfs_emit_at(buf, size, "unavailable\n");
>> + return size;
>> + }
>> dpm_context = smu_dpm->dpm_context;
>> 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 4c81989b8162..5490e8e66e14 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> @@ -1279,6 +1279,9 @@ static int navi10_print_clk_levels(struct
>> smu_context *smu,
>> struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
>> uint32_t min_value, max_value;
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> switch (clk_type) {
>> case SMU_GFXCLK:
>> case SMU_SCLK:
>> @@ -1392,7 +1395,7 @@ static int navi10_print_clk_levels(struct
>> smu_context *smu,
>> case SMU_OD_RANGE:
>> if (!smu->od_enabled || !od_table || !od_settings)
>> break;
>> - size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>> + size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
>> if (navi10_od_feature_is_supported(od_settings,
>> SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
>> navi10_od_setting_get_range(od_settings,
>> SMU_11_0_ODSETTING_GFXCLKFMIN,
>> 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 5e292c3f5050..817ad6de3854 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
>> @@ -1058,6 +1058,9 @@ static int
>> sienna_cichlid_print_clk_levels(struct smu_context *smu,
>> uint32_t min_value, max_value;
>> uint32_t smu_version;
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> switch (clk_type) {
>> case SMU_GFXCLK:
>> case SMU_SCLK:
>> @@ -1180,7 +1183,7 @@ static int
>> sienna_cichlid_print_clk_levels(struct smu_context *smu,
>> if (!smu->od_enabled || !od_table || !od_settings)
>> break;
>> - size = sysfs_emit(buf, "%s:\n", "OD_RANGE");
>> + size += sysfs_emit_at(buf, size, "%s:\n", "OD_RANGE");
>> if (sienna_cichlid_is_od_feature_supported(od_settings,
>> SMU_11_0_7_ODCAP_GFXCLK_LIMITS)) {
>> sienna_cichlid_get_od_setting_range(od_settings,
>> SMU_11_0_7_ODSETTING_GFXCLKFMIN,
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>> index 3a3421452e57..c7842c69b570 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>> @@ -592,7 +592,7 @@ static int vangogh_print_legacy_clk_levels(struct
>> smu_context *smu,
>> switch (clk_type) {
>> case SMU_OD_SCLK:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - 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",
>> (smu->gfx_actual_hard_min_freq > 0) ?
>> smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
>> size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>> @@ -601,7 +601,7 @@ static int vangogh_print_legacy_clk_levels(struct
>> smu_context *smu,
>> break;
>> case SMU_OD_CCLK:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - size = sysfs_emit(buf, "CCLK_RANGE in Core%d:\n",
>> smu->cpu_core_id_select);
>> + size += sysfs_emit_at(buf, size, "CCLK_RANGE in
>> Core%d:\n", smu->cpu_core_id_select);
>> size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
>> (smu->cpu_actual_soft_min_freq > 0) ?
>> smu->cpu_actual_soft_min_freq : smu->cpu_default_soft_min_freq);
>> size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>> @@ -610,7 +610,7 @@ static int vangogh_print_legacy_clk_levels(struct
>> smu_context *smu,
>> break;
>> case SMU_OD_RANGE:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - 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",
>> smu->gfx_default_hard_min_freq,
>> smu->gfx_default_soft_max_freq);
>> size += sysfs_emit_at(buf, size, "CCLK: %7uMhz %10uMhz\n",
>> @@ -682,6 +682,9 @@ static int vangogh_print_clk_levels(struct
>> smu_context *smu,
>> uint32_t cur_value = 0, value = 0, count = 0;
>> bool cur_value_match_level = false;
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> memset(&metrics, 0, sizeof(metrics));
>> ret = smu_cmn_get_metrics_table(smu, &metrics, false);
>> @@ -691,7 +694,7 @@ static int vangogh_print_clk_levels(struct
>> smu_context *smu,
>> switch (clk_type) {
>> case SMU_OD_SCLK:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - 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",
>> (smu->gfx_actual_hard_min_freq > 0) ?
>> smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
>> size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>> @@ -700,7 +703,7 @@ static int vangogh_print_clk_levels(struct
>> smu_context *smu,
>> break;
>> case SMU_OD_CCLK:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - size = sysfs_emit(buf, "CCLK_RANGE in Core%d:\n",
>> smu->cpu_core_id_select);
>> + size += sysfs_emit_at(buf, size, "CCLK_RANGE in
>> Core%d:\n", smu->cpu_core_id_select);
>> size += sysfs_emit_at(buf, size, "0: %10uMhz\n",
>> (smu->cpu_actual_soft_min_freq > 0) ?
>> smu->cpu_actual_soft_min_freq : smu->cpu_default_soft_min_freq);
>> size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>> @@ -709,7 +712,7 @@ static int vangogh_print_clk_levels(struct
>> smu_context *smu,
>> break;
>> case SMU_OD_RANGE:
>> if (smu_dpm_ctx->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {
>> - 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",
>> smu->gfx_default_hard_min_freq,
>> smu->gfx_default_soft_max_freq);
>> size += sysfs_emit_at(buf, size, "CCLK: %7uMhz %10uMhz\n",
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>> b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>> index 5aa175e12a78..86e7978b6d63 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>> @@ -491,6 +491,9 @@ static int renoir_print_clk_levels(struct
>> smu_context *smu,
>> struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
>> bool cur_value_match_level = false;
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> memset(&metrics, 0, sizeof(metrics));
>> ret = smu_cmn_get_metrics_table(smu, &metrics, false);
>> 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 ab652028e003..6349f27e9efc 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>> @@ -733,15 +733,20 @@ static int aldebaran_print_clk_levels(struct
>> smu_context *smu,
>> uint32_t freq_values[3] = {0};
>> uint32_t min_clk, max_clk;
>> - if (amdgpu_ras_intr_triggered())
>> - return sysfs_emit(buf, "unavailable\n");
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> + if (amdgpu_ras_intr_triggered()) {
>> + size += sysfs_emit_at(buf, size, "unavailable\n");
>> + return size;
>> + }
>> dpm_context = smu_dpm->dpm_context;
>> switch (type) {
>> case SMU_OD_SCLK:
>> - size = sysfs_emit(buf, "%s:\n", "GFXCLK");
>> + size += sysfs_emit_at(buf, size, "%s:\n", "GFXCLK");
>> fallthrough;
>> case SMU_SCLK:
>> ret = aldebaran_get_current_clk_freq_by_table(smu,
>> SMU_GFXCLK, &now);
>> @@ -795,7 +800,7 @@ static int aldebaran_print_clk_levels(struct
>> smu_context *smu,
>> break;
>> case SMU_OD_MCLK:
>> - size = sysfs_emit(buf, "%s:\n", "MCLK");
>> + size += sysfs_emit_at(buf, size, "%s:\n", "MCLK");
>> fallthrough;
>> case SMU_MCLK:
>> ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_UCLK,
>> &now);
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>> b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>> index 627ba2eec7fd..3b21d9143b96 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>> @@ -1052,16 +1052,19 @@ static int yellow_carp_print_clk_levels(struct
>> smu_context *smu,
>> int i, size = 0, ret = 0;
>> uint32_t cur_value = 0, value = 0, count = 0;
>> + size = offset_in_page(buf);
>> + buf = buf - size;
>> +
>> switch (clk_type) {
>> case SMU_OD_SCLK:
>> - 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",
>> (smu->gfx_actual_hard_min_freq > 0) ?
>> smu->gfx_actual_hard_min_freq : smu->gfx_default_hard_min_freq);
>> size += sysfs_emit_at(buf, size, "1: %10uMhz\n",
>> (smu->gfx_actual_soft_max_freq > 0) ?
>> smu->gfx_actual_soft_max_freq : smu->gfx_default_soft_max_freq);
>> break;
>> case SMU_OD_RANGE:
>> - 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",
>> smu->gfx_default_hard_min_freq,
>> smu->gfx_default_soft_max_freq);
>> break;
>
next prev parent reply other threads:[~2021-09-08 7:36 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-08 5:56 [PATCH] drm/amdgpu: fix sysfs_emit/sysfs_emit_at warnings Lang Yu
2021-09-08 6:37 ` Christian König
2021-09-08 7:36 ` Lazar, Lijo [this message]
2021-09-08 7:44 ` Yu, Lang
2021-09-08 8:55 ` Lazar, Lijo
2021-09-08 9:02 ` Yu, Lang
2021-09-08 9:29 ` Lazar, Lijo
2021-09-08 9:38 ` Christian König
2021-09-08 10:22 ` Lazar, Lijo
2021-09-08 10:55 ` Yu, Lang
2021-09-08 12:40 ` Christian König
2021-09-08 12:43 ` Christian König
2021-09-08 22:17 ` Powell, Darren
2021-09-09 2:43 ` Yu, Lang
2021-09-09 3:28 ` Lazar, Lijo
2021-09-09 8:01 ` Christian König
2021-09-09 8:36 ` Lazar, Lijo
2021-09-09 8:50 ` Christian König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47ed143e-b9ab-a80e-dac0-cfa1ec39d033@amd.com \
--to=lijo.lazar@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=lang.yu@amd.com \
--cc=ray.huang@amd.com \
--cc=tiantao6@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox