linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "Limonciello, Mario" <Mario.Limonciello@amd.com>,
	Mark Pearson <markpearson@lenovo.com>
Cc: "rafael@kernel.org" <rafael@kernel.org>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>
Subject: Re: [RFC] ACPI: platform-profile: support for AC vs DC modes
Date: Tue, 8 Mar 2022 17:10:47 +0100	[thread overview]
Message-ID: <1cd4ae24-ae92-302e-ac87-76ef15472a9f@redhat.com> (raw)
In-Reply-To: <BL1PR12MB5157125246817A70649C12F7E2099@BL1PR12MB5157.namprd12.prod.outlook.com>

Hi,

On 3/8/22 16:55, Limonciello, Mario wrote:
> [AMD Official Use Only]
> 
>>> I don't think that's right for the PSC Thinkpads.  They have dedicated
>>> different tunings for each of the slider positions on AC vs DC.
>>>
>>> So "balanced" on AC will not be the same as "balanced" on DC.
>>
>> I see, but it is not like balanced on AC is closer to performance
>> on DC then it is to balanced on DC, right? IOW in the UI we should
>> still call them both balanced ?
> 
> I feel that's a gross oversimplification to say balanced on AC is close
> to performance on DC.  There are *so many* other (otherwise invisible)
> tuning knobs behind what PSC does that Lenovo has weighed out the benefits
> of using for different circumstances.  
> 
> You nerf all this by just having one user space facing knob and let userspace
> change to performance mode when you on charger.

The way I see this there are 2 ways this can work on the kernel to fw/ec
boundary:

1. There are actually 6 values we can write to a single slot:
   ac-low-power,dc-lowpower,ac-balanced,dc-balanced,ac-performance,dc-performance

2. There are separate ac-setting + dc-setting slots to which we can
   write one of 3 values: low-power, balanced, performance; and the fw/ec
   automatically picks which slot to used based on ac vs battery status

If 1 is the case for PSC then I agree that the kernel should indeed get involved
and it should automatically write either the ac or dc variant of the last
userspace requested value so that things behave as expected.

If 2 however is the case then I think all that is necessary is for the
driver to just write the last userspace selected value to both slots.

Note that neither case requires a userspace API change when solved
as I suggest.

> At least the way Windows does this is that it offers "one" UI slider but you
> have last selected values based on if you're plugged in or on battery.
> 
> 1) So on battery I might have balanced selected to start out.
> 2) Then I plug in a charger, and balanced is still selected but this has
> different characteristics from balanced on battery.
> 3) Now I change to performance while on charger.
> 4) Then I unplug charger and it goes back to my selection for battery: "balanced".

The above is more about policy then it is about mechanism, userspace
can easily remember 2 separate settings for ac vs battery and restore
the last set value for ac or battery when changing between the 2.

Since this mostly about the policy which profile to set when this
really belongs in userspace IMHO and solving this in userspace means that
we will have a single universal solution for all the different
platform_profile implementations, and we seem to have quite a lot of
those (at least one per laptop vendor, Lenovo currently has 2)

>> If that is right then I think my point still stands, if PSC
>> has 2 separate slots (one AC one DC) for the performance
>> setting, then we can just set both when userspace selects a
>> performance level and have the actual e.g. balanced -> performance
>> change be done by userspace when userspace select the machine
>> has been connected to a charger.
> 
> But you *don't want to* actually select performance when you switch to a
> charger.  If you have 3 value slots for AC and 3 value slots for DC you
> should only be swapping between what is in those 3 values slots.

That only works if all implementation have separate AC and DC profile
slots, which most won't have. If we just sync the 2 slots for implementations
which do have 2 slots and then always "fake" 2 slots in userspace we
have a universal implementation which will work well everywhere, without
any significant downside to the implementations which do have 2 slots.

Regards,

Hans


  reply	other threads:[~2022-03-08 16:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01 20:15 [RFC] ACPI: platform-profile: support for AC vs DC modes Mark Pearson
2022-03-03  2:53 ` Mario Limonciello
2022-03-03 17:08   ` [External] " Mark Pearson
2022-03-03 17:40     ` Limonciello, Mario
2022-03-08 14:39 ` Hans de Goede
2022-03-08 14:50   ` Limonciello, Mario
2022-03-08 15:16     ` Hans de Goede
2022-03-08 15:55       ` Limonciello, Mario
2022-03-08 16:10         ` Hans de Goede [this message]
2022-03-08 17:44           ` [External] " Mark Pearson
2022-03-14 12:45             ` Hans de Goede
2022-03-14 13:39               ` Limonciello, Mario
2022-03-14 14:43                 ` Hans de Goede
2022-03-14 14:59                   ` Mark Pearson
2022-03-14 15:05                     ` Hans de Goede
2022-03-14 15:31                   ` Hans de Goede
2022-03-14 15:32                     ` Mark Pearson
2022-03-14 16:56                       ` Hans de Goede
2022-03-14 17:10                         ` Limonciello, Mario
2022-03-14 17:13                         ` Mark Pearson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1cd4ae24-ae92-302e-ac87-76ef15472a9f@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=Mario.Limonciello@amd.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=markpearson@lenovo.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).