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

[Lyndon Sanche]

On Thu, May 9 2024 at 09:10:51 AM -06:00:00, Lyndon Sanche 
<lsanche@xxxxxxxxxx> 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