Re: [PATCH v3 0/1] asus-wmi: add platform_profile support

[Luke Jones]

On Sun, Aug 15 2021 at 15:48:49 +0200, Hans de Goede 
<hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx> 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