From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Lang Yu <lang.yu@amd.com>, amd-gfx@lists.freedesktop.org
Cc: Alex Deucher <alexander.deucher@amd.com>,
Huang Rui <ray.huang@amd.com>, Lijo Lazar <lijo.lazar@amd.com>,
Tian Tao <tiantao6@hisilicon.com>
Subject: Re: [PATCH] drm/amdgpu: fix sysfs_emit/sysfs_emit_at warnings
Date: Wed, 8 Sep 2021 08:37:33 +0200 [thread overview]
Message-ID: <e8b39f62-ca0c-d4e0-92a9-52487fa0da81@gmail.com> (raw)
In-Reply-To: <20210908055615.3781901-1-lang.yu@amd.com>
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?
That stuff needs to be implemented for each hardware generation and is
now cluttered with sysfs buffer offset calculations.
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 6:37 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 [this message]
2021-09-08 7:36 ` Lazar, Lijo
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=e8b39f62-ca0c-d4e0-92a9-52487fa0da81@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=lang.yu@amd.com \
--cc=lijo.lazar@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