Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 16 Aug 2021 11:00:01 +1200
From:   Luke Jones <luke@ljones.dev>
Subject: Re: [PATCH v3 0/1] asus-wmi: add platform_profile support
To:     Hans de Goede <hdegoede@redhat.com>
Cc:     linux-kernel@vger.kernel.org, hadess@hadess.net,
        platform-driver-x86@vger.kernel.org
Message-Id: <18LWXQ.AIB2DGCZUVMW@ljones.dev>
In-Reply-To: <a918bfe0-03c4-e7d3-8a99-efb1d11333e4@redhat.com>
References: <20210814043103.2535842-1-luke@ljones.dev>
        <THKTXQ.ELSNF0TA7RAV1@ljones.dev>
        <a918bfe0-03c4-e7d3-8a99-efb1d11333e4@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Precedence: bulk


On Sun, Aug 15 2021 at 15:48:49 +0200, Hans de Goede 
<hdegoede@redhat.com> wrote:
> Hi,
> 
> On 8/14/21 9:51 AM, Luke Jones wrote:
>> 
>> 
>>  On Sat, Aug 14 2021 at 16:31:02 +1200, Luke D. Jones 
>> <luke@ljones.dev> wrote:
>>>  Changelog:
>>>  - V2
>>>    + Correctly unregister from platform_profile if
>>>      throttle_thermal_policy fails
>>>    + Do platform_profile_notify() in both 
>>> throttle_thermal_policy_store()
>>>      and in throttle_thermal_policy_switch_next()
>>>    + Remove unnecessary prep for possible fan-boost modes as this
>>>      doesn't match expected platform_profile behaviour
>>>  - V3
>>>    + Add missing declaration for err in
>>>      throttle_thermal_policy_switch_next
>>> 
>>>  Luke D. Jones (1):
>>>    asus-wmi: Add support for platform_profile
>>> 
>>>   drivers/platform/x86/asus-wmi.c | 139 
>>> +++++++++++++++++++++++++++++++-
>>>   1 file changed, 135 insertions(+), 4 deletions(-)
>>> 
>>>  --
>>>  2.31.1
>> 
>>  Hi,
>> 
>>  I teested the patch again and it appears that the 
>> platform_profile_notify() in both throttle_thermal_policy_store() 
>> and throttle_thermal_policy_switch_next() updates the 
>> /sys/firmware/acpi/platform_profile sysfs path fine, but userspace 
>> isn't updated?
>> 
>>  The way I'm checking is:
>>  1. echo 1 |sudo tee 
>> /sys/devices/platform/asus-nb-wmi/throttle_thermal_policy
>>  2. cat -p /sys/firmware/acpi/platform_profile
>>    - performance (updated correctly by platform_profile_notify)
>>  3. Check gnome-settings, not updated.
>> 
>>  Doing `echo "performance" |sudo tee 
>> /sys/firmware/acpi/platform_profile` updates both 
>> throttle_thermal_policy and userspace as expected. I'm wondering if 
>> I've missed something?
> 
> If you add a printk where you call platform_profile_notify() and you 
> see that
> happening, then you are likely seeing a userspace bug. Possibly your
> power-profile-daemon is simply a bit old and therefor does not support
> the combination of profiles which asus-wmi offers, IIRC it falls back 
> to
> using intel-pstate in that case.

It's possible that it's a userspace bug then. The power-profile-daemon 
I'm using is fresh from git (0.9+). To be clear updating via 
/sys/firmware/acpi/platform_profile works perfectly fine and 
power-profile-daemon updates etc. But if I do platform_profile_notify() 
then it doesn't seem to be updated. Nevertheless I will finalise the 
patch as it is and submit for merging and we can go from there.

> 
> You could try building the latest power-profile-daemon from git and 
> run
> it in verbose mode. If it sees the changes and the control-panel 
> applet is
> still not updating then I would not worry about that. The userspace 
> code
> is still somewhat new and I'm not sure which version your distro is
> running and how well it is keeping up with gnome-updates.
> 
> Regards,
> 
> Hans
>