* [PATCH v3] platform/x86: amd: pmf: Fix STT limits
@ 2025-04-07 18:18 Mario Limonciello
2025-04-08 6:39 ` Shyam Sundar S K
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Mario Limonciello @ 2025-04-07 18:18 UTC (permalink / raw)
To: mario.limonciello, Shyam-sundar.S-k, hdegoede, ilpo.jarvinen
Cc: Yijun Shen, stable, Yijun Shen, platform-driver-x86
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)
---
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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] platform/x86: amd: pmf: Fix STT limits
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
2 siblings, 0 replies; 4+ messages in thread
From: Shyam Sundar S K @ 2025-04-08 6:39 UTC (permalink / raw)
To: Mario Limonciello, mario.limonciello, hdegoede, ilpo.jarvinen
Cc: Yijun Shen, stable, Yijun Shen, platform-driver-x86
On 4/7/2025 23:48, 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>
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Thanks,
Shyam
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] platform/x86: amd: pmf: Fix STT limits
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
2 siblings, 0 replies; 4+ messages in thread
From: Ilpo Järvinen @ 2025-04-08 11:42 UTC (permalink / raw)
To: Mario Limonciello
Cc: mario.limonciello, Shyam-sundar.S-k, Hans de Goede, Yijun Shen,
stable, Yijun Shen, platform-driver-x86
[-- Attachment #1: Type: text/plain, Size: 7936 bytes --]
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>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> ---
> v3:
> * Add a helper with a generic name (so it can be easily be moved to library
> code in the future)
> ---
> 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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] platform/x86: amd: pmf: Fix STT limits
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
2 siblings, 0 replies; 4+ messages in thread
From: Ilpo Järvinen @ 2025-04-11 10:03 UTC (permalink / raw)
To: Mario Limonciello
Cc: mario.limonciello, Shyam-sundar.S-k, Hans de Goede, Yijun Shen,
stable, Yijun Shen, platform-driver-x86
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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-11 10:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox