From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Mario Limonciello <superm1@kernel.org>
Cc: mario.limonciello@amd.com, Shyam-sundar.S-k@amd.com,
Hans de Goede <hdegoede@redhat.com>,
Yijun Shen <Yijun.Shen@dell.com>,
stable@vger.kernel.org, Yijun Shen <Yijun_Shen@Dell.com>,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v3] platform/x86: amd: pmf: Fix STT limits
Date: Fri, 11 Apr 2025 13:03:01 +0300 (EEST) [thread overview]
Message-ID: <8bab41c3-2c66-9dd4-fe9c-af7a250928bc@linux.intel.com> (raw)
In-Reply-To: <20250407181915.1482450-1-superm1@kernel.org>
On Mon, 7 Apr 2025, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> On some platforms it has been observed that STT limits are not being
> applied properly causing poor performance as power limits are set too low.
>
> STT limits that are sent to the platform are supposed to be in Q8.8
> format. Convert them before sending.
>
> Reported-by: Yijun Shen <Yijun.Shen@dell.com>
> Fixes: 7c45534afa443 ("platform/x86/amd/pmf: Add support for PMF Policy Binary")
> Cc: stable@vger.kernel.org
> Tested-by: Yijun Shen <Yijun_Shen@Dell.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v3:
> * Add a helper with a generic name (so it can be easily be moved to library
> code in the future)
I've applied this to review-ilpo-fixes branch. In the end, I decided to
rename from ..._from_integer() to _fromint() which matches some drm
usage in context of fixed point integers and is shorter.
--
i.
> ---
> drivers/platform/x86/amd/pmf/auto-mode.c | 4 ++--
> drivers/platform/x86/amd/pmf/cnqf.c | 8 ++++----
> drivers/platform/x86/amd/pmf/core.c | 14 ++++++++++++++
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> drivers/platform/x86/amd/pmf/sps.c | 12 ++++++++----
> drivers/platform/x86/amd/pmf/tee-if.c | 6 ++++--
> 6 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> index 02ff68be10d01..1400ac70c52d1 100644
> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> @@ -120,9 +120,9 @@ static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
> amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pwr_ctrl->sppt_apu_only, NULL);
> amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pwr_ctrl->stt_min, NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> - pwr_ctrl->stt_skin_temp[STT_TEMP_APU], NULL);
> + fixp_q88_from_integer(pwr_ctrl->stt_skin_temp[STT_TEMP_APU]), NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> - pwr_ctrl->stt_skin_temp[STT_TEMP_HS2], NULL);
> + fixp_q88_from_integer(pwr_ctrl->stt_skin_temp[STT_TEMP_HS2]), NULL);
>
> if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> apmf_update_fan_idx(dev, config_store.mode_set[idx].fan_control.manual,
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index bc8899e15c914..3cde8a5de64a9 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -81,10 +81,10 @@ static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
> amd_pmf_send_cmd(dev, SET_SPPT, false, pc->sppt, NULL);
> amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pc->sppt_apu_only, NULL);
> amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pc->stt_min, NULL);
> - amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, pc->stt_skin_temp[STT_TEMP_APU],
> - NULL);
> - amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, pc->stt_skin_temp[STT_TEMP_HS2],
> - NULL);
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> + fixp_q88_from_integer(pc->stt_skin_temp[STT_TEMP_APU]), NULL);
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> + fixp_q88_from_integer(pc->stt_skin_temp[STT_TEMP_HS2]), NULL);
>
> if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> apmf_update_fan_idx(dev,
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index a2cb2d5544f5b..5209996eba674 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -176,6 +176,20 @@ static void __maybe_unused amd_pmf_dump_registers(struct amd_pmf_dev *dev)
> dev_dbg(dev->dev, "AMD_PMF_REGISTER_MESSAGE:%x\n", value);
> }
>
> +/**
> + * fixp_q88_from_integer: Convert integer to Q8.8
> + * @val: input value
> + *
> + * Converts an integer into binary fixed point format where 8 bits
> + * are used for integer and 8 bits are used for the decimal.
> + *
> + * Return: unsigned integer converted to Q8.8 format
> + */
> +u32 fixp_q88_from_integer(u32 val)
> +{
> + return val << 8;
> +}
> +
> int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data)
> {
> int rc;
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index e6bdee68ccf34..2865e0a70b43d 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -777,6 +777,7 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
> int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
> int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer);
> int amd_pmf_notify_sbios_heartbeat_event_v2(struct amd_pmf_dev *dev, u8 flag);
> +u32 fixp_q88_from_integer(u32 val);
>
> /* SPS Layer */
> int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
> index d3083383f11fb..dfc5285b681f7 100644
> --- a/drivers/platform/x86/amd/pmf/sps.c
> +++ b/drivers/platform/x86/amd/pmf/sps.c
> @@ -198,9 +198,11 @@ static void amd_pmf_update_slider_v2(struct amd_pmf_dev *dev, int idx)
> amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
> apts_config_store.val[idx].stt_min_limit, NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> - apts_config_store.val[idx].stt_skin_temp_limit_apu, NULL);
> + fixp_q88_from_integer(apts_config_store.val[idx].stt_skin_temp_limit_apu),
> + NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> - apts_config_store.val[idx].stt_skin_temp_limit_hs2, NULL);
> + fixp_q88_from_integer(apts_config_store.val[idx].stt_skin_temp_limit_hs2),
> + NULL);
> }
>
> void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
> @@ -217,9 +219,11 @@ void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
> amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
> config_store.prop[src][idx].stt_min, NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> - config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU], NULL);
> + fixp_q88_from_integer(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU]),
> + NULL);
> amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> - config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2], NULL);
> + fixp_q88_from_integer(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2]),
> + NULL);
> } else if (op == SLIDER_OP_GET) {
> amd_pmf_send_cmd(dev, GET_SPL, true, ARG_NONE, &table->prop[src][idx].spl);
> amd_pmf_send_cmd(dev, GET_FPPT, true, ARG_NONE, &table->prop[src][idx].fppt);
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index a1e43873a07b0..22d48048f9d01 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -123,7 +123,8 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>
> case PMF_POLICY_STT_SKINTEMP_APU:
> if (dev->prev_data->stt_skintemp_apu != val) {
> - amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, val, NULL);
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> + fixp_q88_from_integer(val), NULL);
> dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
> dev->prev_data->stt_skintemp_apu = val;
> }
> @@ -131,7 +132,8 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>
> case PMF_POLICY_STT_SKINTEMP_HS2:
> if (dev->prev_data->stt_skintemp_hs2 != val) {
> - amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, val, NULL);
> + amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> + fixp_q88_from_integer(val), NULL);
> dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
> dev->prev_data->stt_skintemp_hs2 = val;
> }
>
prev parent reply other threads:[~2025-04-11 10:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-07 18:18 [PATCH v3] platform/x86: amd: pmf: Fix STT limits Mario Limonciello
2025-04-08 6:39 ` Shyam Sundar S K
2025-04-08 11:42 ` Ilpo Järvinen
2025-04-11 10:03 ` Ilpo Järvinen [this message]
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=8bab41c3-2c66-9dd4-fe9c-af7a250928bc@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=Yijun.Shen@dell.com \
--cc=Yijun_Shen@Dell.com \
--cc=hdegoede@redhat.com \
--cc=mario.limonciello@amd.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=superm1@kernel.org \
/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