Re: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile

[Lyndon Sanche <lsanche@lyndeno.ca>]

On Thu, May 9 2024 at 09:10:51 AM -06:00:00, Lyndon Sanche 
<lsanche@lyndeno.ca> wrote:
> On Wed, May 8, 2024, at 8:24 AM, Shen, Yijun wrote:
>>  Hi Lyndon,
>> 
>>   Thanks for working on this patch.
>> 
>> 
>>   Dell side has an initial testing with this patch on some laptops, 
>> it
>>  looks good. While changing the platform profile:
>>  1. The corresponding USTT option in BIOS will be changed.
>>  2. thermald will not be impacted. The related PSVT and ITMT will be
>>  loaded.
>>   Some Dell DTs does not have the USTT, Dell'll have a check if 
>> nothing
>>  is broken.
>> 
>>    Additional, with this patch, follow behavior is found:
>>   1. For example, the platform profile is quiet.
>>   2. Reboot the system and change the USTT to performance.
>>   3. Boot to desktop, the platform profile is "quiet", the USTT will 
>> be
>>  changed back to "quiet".
>>   This looks like not a proper user experience. The platform profile
>>  should honor the BIOS setting, aka, the platform profile should be
>>  switched to "performance".
> 
> Hello:
> 
> Thank you for your email. This is definitely undesirable behaviour, I 
> will have a look at the code to see why this is happening. Does it 
> always revert to quiet on boot, or always the mode that you had 
> switched to prior to reboot?
> 
> Do you happen to have power-profiles-daemon or something similar 
> running? My understanding is it remembers profiles across reboots, 
> this could potentially also revert the profile back to what it was. 
> See this release for details:
> https://gitlab.freedesktop.org/upower/power-profiles-daemon/-/releases/0.9.0
> 
> I will assume there is a bug in my code at this point. I will test 
> with and without ppd running on my system to see if it changes across 
> reboots.
> 
> Are USTT settings exposed in your BIOS configuration menu? On my 
> laptop they are not and I have to use smbios-thermal-ctl.
> 
> Thank you,
> 
> Lyndon

Hi Yijun:

I tested this on my computer (XPS 9560). I do not have access to the 
USTT settings in the BIOS screen so to substitute that, I booted 
without the patch and set the USTT manually using smbios-thermal-ctl. 
Here are my findings:

Scenario #1: Without power-profiles-daemon (ppd) running

1. Boot with patch, set platform_profile to quiet
2. Boot without patch applied (no platform_profile)
 - smbios-thermal-ctl confirms USTT is set to quiet
 - use smbios-thermal-ctl to set USTT to performance
 - confirm set to performance
3. Boot with patch again
 - platform_profile is set to performance

Scenario #2: With ppd running
1. Boot with patch, set platform_profile to performance with ppd
 - Confirm platform_profile is performance
2. Boot without patch applied (no platform_profile)
 - smbios-thermal-ctl confirms USTT is set to performance
 - ppd reverts to balanced (only controlling intel_pstate in this case)
 - use smbios-thermal-ctl to set USTT to quiet
 - confirm set to quiet
3. Boot with patch again
 - platform_profile and ppd is set to performance

In my case, the setting in the smbios is honored if it was switched 
with another method. When using a userspace program that manipulates 
the platform_profile, the program seems to remember the previous state 
and switch to that.

So I do not think there is a bug in this patch related to this issue, 
at least in my case. Please let me know if you have any questions.

Thanks,

Lyndon