Received-SPF: pass (google.com: domain of linux-kernel+bounces-176363-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Feedback-ID: i1719461a:Fastmail
Date: Fri, 10 May 2024 19:49:09 -0600
From: Lyndon Sanche <lsanche@lyndeno.ca>
Subject: Re: [PATCH v5] platform/x86: dell-laptop: Implement platform_profile
To: "Shen, Yijun" <Yijun.Shen@dell.com>
Cc: Mario Limonciello <mario.limonciello@amd.com>,
	Pali =?iso-8859-1?b?Um9o4XI=?= <pali@kernel.org>, Armin Wolf
	<W_Armin@gmx.de>, srinivas.pandruvada@linux.intel.com,
	Ilpo =?iso-8859-1?q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	kernel test robot <lkp@intel.com>, Hans de Goede <hdegoede@redhat.com>,
	Matthew Garrett <mjg59@srcf.ucam.org>, Jonathan Corbet <corbet@lwn.net>,
	Heiner Kallweit <hkallweit1@gmail.com>, Vegard Nossum
	<vegard.nossum@oracle.com>, platform-driver-x86@vger.kernel.org, LKML
	<linux-kernel@vger.kernel.org>, Dell Client Kernel
	<Dell.Client.Kernel@dell.com>
Message-Id: <X1TADS.8PTII3LXSHHX1@lyndeno.ca>
In-Reply-To: <aaa1ebb5-3e0a-40ef-b53d-d8adc1589085@app.fastmail.com>
References: <20240425172758.67831-1-lsanche@lyndeno.ca>
	<20240501215829.4991-2-lsanche@lyndeno.ca>
	<BY5PR19MB392256C65661E76FC292C0889AE52@BY5PR19MB3922.namprd19.prod.outlook.com>
	<aaa1ebb5-3e0a-40ef-b53d-d8adc1589085@app.fastmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed


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