X86 platform drivers
 help / color / mirror / Atom feed
* [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot
@ 2025-09-15  9:41 Shyam Sundar S K
  2025-09-15 18:54 ` Lyndon Sanche
  2025-09-15 19:00 ` Mario Limonciello
  0 siblings, 2 replies; 5+ messages in thread
From: Shyam Sundar S K @ 2025-09-15  9:41 UTC (permalink / raw)
  To: lsanche, hdegoede, ilpo.jarvinen
  Cc: platform-driver-x86, Patil.Reddy, mario.limonciello,
	Shyam Sundar S K, Yijun Shen

After a reboot, if the user changes the thermal setting in the BIOS, the
BIOS applies this change. However, the current `dell-pc` driver does not
recognize the updated USTT value, resulting in inconsistent thermal
profiles between Windows and Linux.

To ensure alignment with Windows behavior, the proposed change involves
reading the current USTT setting during driver initialization and updating
the dell-pc USTT profile accordingly whenever a change is detected.

Cc: Yijun Shen <Yijun.Shen@Dell.com>
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/dell/dell-pc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
index 48cc7511905a..22248cfe21c5 100644
--- a/drivers/platform/x86/dell/dell-pc.c
+++ b/drivers/platform/x86/dell/dell-pc.c
@@ -228,6 +228,8 @@ static int thermal_platform_profile_get(struct device *dev,
 
 static int thermal_platform_profile_probe(void *drvdata, unsigned long *choices)
 {
+	int current_mode;
+
 	if (supported_modes & DELL_QUIET)
 		__set_bit(PLATFORM_PROFILE_QUIET, choices);
 	if (supported_modes & DELL_COOL_BOTTOM)
@@ -237,6 +239,28 @@ static int thermal_platform_profile_probe(void *drvdata, unsigned long *choices)
 	if (supported_modes & DELL_PERFORMANCE)
 		__set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
 
+	/* Read current thermal mode from the BIOS and set the mode explicitly */
+	current_mode = thermal_get_mode();
+	if (current_mode < 0)
+		return current_mode;
+
+	switch (current_mode) {
+	case DELL_BALANCED:
+		thermal_set_mode(DELL_BALANCED);
+		break;
+	case DELL_PERFORMANCE:
+		thermal_set_mode(DELL_PERFORMANCE);
+		break;
+	case DELL_COOL_BOTTOM:
+		thermal_set_mode(DELL_COOL_BOTTOM);
+		break;
+	case DELL_QUIET:
+		thermal_set_mode(DELL_QUIET);
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot
  2025-09-15  9:41 [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot Shyam Sundar S K
@ 2025-09-15 18:54 ` Lyndon Sanche
  2025-09-16  9:58   ` Shyam Sundar S K
  2025-09-15 19:00 ` Mario Limonciello
  1 sibling, 1 reply; 5+ messages in thread
From: Lyndon Sanche @ 2025-09-15 18:54 UTC (permalink / raw)
  To: Shyam Sundar S K, Hans de Goede, Ilpo Järvinen
  Cc: platform-driver-x86, Patil.Reddy, Mario Limonciello, Yijun Shen

On Mon, Sep 15, 2025, at 3:41 AM, Shyam Sundar S K wrote:
> After a reboot, if the user changes the thermal setting in the BIOS, the
> BIOS applies this change. However, the current `dell-pc` driver does not
> recognize the updated USTT value, resulting in inconsistent thermal
> profiles between Windows and Linux.
>
> To ensure alignment with Windows behavior, the proposed change involves
> reading the current USTT setting during driver initialization and updating
> the dell-pc USTT profile accordingly whenever a change is detected.

Hello:

Thank you for the patch. I think I need help understanding why this is not working currently, as this is implemented in the thermal_get_mode functionality.

This change, upon my first read, seems to read the mode from the BIOS and then write what it read back to the BIOS. Is my understanding correct? I do not have a computer where I can change the mode from the BIOS, so I cannot test that specific case. What does the platform_profile get set to on boot without this patch?

Thanks,

Lyndon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot
  2025-09-15  9:41 [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot Shyam Sundar S K
  2025-09-15 18:54 ` Lyndon Sanche
@ 2025-09-15 19:00 ` Mario Limonciello
  2025-09-16  9:59   ` Shyam Sundar S K
  1 sibling, 1 reply; 5+ messages in thread
From: Mario Limonciello @ 2025-09-15 19:00 UTC (permalink / raw)
  To: Shyam Sundar S K, lsanche, hdegoede, ilpo.jarvinen
  Cc: platform-driver-x86, Patil.Reddy, Yijun Shen

On 9/15/25 4:41 AM, Shyam Sundar S K wrote:
> After a reboot, if the user changes the thermal setting in the BIOS, the
> BIOS applies this change. However, the current `dell-pc` driver does not
> recognize the updated USTT value, resulting in inconsistent thermal
> profiles between Windows and Linux.
> 
> To ensure alignment with Windows behavior, the proposed change involves
> reading the current USTT setting during driver initialization and updating
> the dell-pc USTT profile accordingly whenever a change is detected.

No need to refer to "this change" or "proposed change" in a commit message.

> 
> Cc: Yijun Shen <Yijun.Shen@Dell.com>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>   drivers/platform/x86/dell/dell-pc.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/dell-pc.c
> index 48cc7511905a..22248cfe21c5 100644
> --- a/drivers/platform/x86/dell/dell-pc.c
> +++ b/drivers/platform/x86/dell/dell-pc.c
> @@ -228,6 +228,8 @@ static int thermal_platform_profile_get(struct device *dev,
>   
>   static int thermal_platform_profile_probe(void *drvdata, unsigned long *choices)
>   {
> +	int current_mode;
> +
>   	if (supported_modes & DELL_QUIET)
>   		__set_bit(PLATFORM_PROFILE_QUIET, choices);
>   	if (supported_modes & DELL_COOL_BOTTOM)
> @@ -237,6 +239,28 @@ static int thermal_platform_profile_probe(void *drvdata, unsigned long *choices)
>   	if (supported_modes & DELL_PERFORMANCE)
>   		__set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>   
> +	/* Read current thermal mode from the BIOS and set the mode explicitly */

It's got to do with extra ACPI actions that happen "by the act" of 
setting a profile, right?

I think it's worth explaining in more detail in the comment /why/ this 
helps so it doesn't get removed some day in the future as part of an 
optimization.

> +	current_mode = thermal_get_mode();
> +	if (current_mode < 0)
> +		return current_mode;
> +
> +	switch (current_mode) {
> +	case DELL_BALANCED:
> +		thermal_set_mode(DELL_BALANCED);
> +		break;
> +	case DELL_PERFORMANCE:
> +		thermal_set_mode(DELL_PERFORMANCE);
> +		break;
> +	case DELL_COOL_BOTTOM:
> +		thermal_set_mode(DELL_COOL_BOTTOM);
> +		break;
> +	case DELL_QUIET:
> +		thermal_set_mode(DELL_QUIET);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +

Why even have the switch/case?  If thermal_get_mode() returns an invalid 
value you already will return current_mode.

IE I'd think it's as simple as:

/*
  * Make sure that ACPI is in sync with the profile set by USTT.
  */
current_mode = thermal_get_mode();
if (current_mode < 0)
	return current_mode;
thermal_set_mode(current_mode);

>   	return 0;
>   }
>   


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot
  2025-09-15 18:54 ` Lyndon Sanche
@ 2025-09-16  9:58   ` Shyam Sundar S K
  0 siblings, 0 replies; 5+ messages in thread
From: Shyam Sundar S K @ 2025-09-16  9:58 UTC (permalink / raw)
  To: Lyndon Sanche, Hans de Goede, Ilpo Järvinen
  Cc: platform-driver-x86, Patil.Reddy, Mario Limonciello, Yijun Shen

Hi Lyndon,

On 9/16/2025 00:24, Lyndon Sanche wrote:
> On Mon, Sep 15, 2025, at 3:41 AM, Shyam Sundar S K wrote:
>> After a reboot, if the user changes the thermal setting in the BIOS, the
>> BIOS applies this change. However, the current `dell-pc` driver does not
>> recognize the updated USTT value, resulting in inconsistent thermal
>> profiles between Windows and Linux.
>>
>> To ensure alignment with Windows behavior, the proposed change involves
>> reading the current USTT setting during driver initialization and updating
>> the dell-pc USTT profile accordingly whenever a change is detected.
> 
> Hello:
> 
> Thank you for the patch. I think I need help understanding why this is not working currently, as this is implemented in the thermal_get_mode functionality.

Okay. Let me elaborate:

There are scenarios where the Dell BIOS needs to send specific events
to the AMD PMF driver. The `amd_pmf` driver registers a handler with
the ACPI subsystem to listen for these events, which should originate
from the `dell_pc` driver. However, during probe time, the `dell_pc`
driver lacks awareness of any changes to the USTT value. As a result,
it may report outdated USTT data, leading to incorrect thermal
configurations.

Further debug revealed that Windows adjust to changes in USTT and
behave accordingly.

To align with Windows behavior, we now read the current USTT value
during the `dell_pc` driver's probe phase. This ensures that any
updates are captured, enabling the `amd_pmf` driver to receive
accurate information through the ACPI events it listens to.

> 
> This change, upon my first read, seems to read the mode from the BIOS and then write what it read back to the BIOS. Is my understanding correct?
Yes, via the dell_smbios_*() calls.

> I do not have a computer where I can change the mode from the BIOS, so I cannot test that specific case.
Okay. Alex (Cc'ed) from Dell has tested this on Multiple platforms.

> What does the platform_profile get set to on boot without this patch?

"custom"

Thanks,
Shyam

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot
  2025-09-15 19:00 ` Mario Limonciello
@ 2025-09-16  9:59   ` Shyam Sundar S K
  0 siblings, 0 replies; 5+ messages in thread
From: Shyam Sundar S K @ 2025-09-16  9:59 UTC (permalink / raw)
  To: Mario Limonciello, lsanche, hdegoede, ilpo.jarvinen
  Cc: platform-driver-x86, Patil.Reddy, Yijun Shen

Hi Mario,

On 9/16/2025 00:30, Mario Limonciello wrote:
> On 9/15/25 4:41 AM, Shyam Sundar S K wrote:
>> After a reboot, if the user changes the thermal setting in the BIOS,
>> the
>> BIOS applies this change. However, the current `dell-pc` driver does
>> not
>> recognize the updated USTT value, resulting in inconsistent thermal
>> profiles between Windows and Linux.
>>
>> To ensure alignment with Windows behavior, the proposed change involves
>> reading the current USTT setting during driver initialization and
>> updating
>> the dell-pc USTT profile accordingly whenever a change is detected.
> 
> No need to refer to "this change" or "proposed change" in a commit
> message.

Ack.

> 
>>
>> Cc: Yijun Shen <Yijun.Shen@Dell.com>
>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>   drivers/platform/x86/dell/dell-pc.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/
>> x86/dell/dell-pc.c
>> index 48cc7511905a..22248cfe21c5 100644
>> --- a/drivers/platform/x86/dell/dell-pc.c
>> +++ b/drivers/platform/x86/dell/dell-pc.c
>> @@ -228,6 +228,8 @@ static int thermal_platform_profile_get(struct
>> device *dev,
>>     static int thermal_platform_profile_probe(void *drvdata,
>> unsigned long *choices)
>>   {
>> +    int current_mode;
>> +
>>       if (supported_modes & DELL_QUIET)
>>           __set_bit(PLATFORM_PROFILE_QUIET, choices);
>>       if (supported_modes & DELL_COOL_BOTTOM)
>> @@ -237,6 +239,28 @@ static int thermal_platform_profile_probe(void
>> *drvdata, unsigned long *choices)
>>       if (supported_modes & DELL_PERFORMANCE)
>>           __set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>>   +    /* Read current thermal mode from the BIOS and set the mode
>> explicitly */
> 
> It's got to do with extra ACPI actions that happen "by the act" of
> setting a profile, right?
> 
> I think it's worth explaining in more detail in the comment /why/ this
> helps so it doesn't get removed some day in the future as part of an
> optimization.
> 
>> +    current_mode = thermal_get_mode();
>> +    if (current_mode < 0)
>> +        return current_mode;
>> +
>> +    switch (current_mode) {
>> +    case DELL_BALANCED:
>> +        thermal_set_mode(DELL_BALANCED);
>> +        break;
>> +    case DELL_PERFORMANCE:
>> +        thermal_set_mode(DELL_PERFORMANCE);
>> +        break;
>> +    case DELL_COOL_BOTTOM:
>> +        thermal_set_mode(DELL_COOL_BOTTOM);
>> +        break;
>> +    case DELL_QUIET:
>> +        thermal_set_mode(DELL_QUIET);
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
> 
> Why even have the switch/case?  If thermal_get_mode() returns an
> invalid value you already will return current_mode.
> 
> IE I'd think it's as simple as:
> 
> /*
>  * Make sure that ACPI is in sync with the profile set by USTT.
>  */
> current_mode = thermal_get_mode();
> if (current_mode < 0)
>     return current_mode;
> thermal_set_mode(current_mode);
> 
>>       return 0;
>>   }

Okay, will make this in v2.

Thanks,
Shyam

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-16  9:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15  9:41 [PATCH] platform/x86/dell: Set USTT mode according to BIOS after reboot Shyam Sundar S K
2025-09-15 18:54 ` Lyndon Sanche
2025-09-16  9:58   ` Shyam Sundar S K
2025-09-15 19:00 ` Mario Limonciello
2025-09-16  9:59   ` Shyam Sundar S K

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox