Re: [External] Re: [RFC] ACPI: platform-profile: support for AC vs DC modes

[Hans de Goede <hdegoede@redhat.com>]

Hi Mario,

On 3/14/22 14:39, Limonciello, Mario wrote:
> [Public]
> 
>>>
>>> I cycled through a few different implementations but came down on what I
>>> proposed. I considered 6 values - but I don't think that makes sense and
>>> makes it overall more complicated than it needs to be and less flexible.
>>
>> Ah, so to be clear, my 2 scenarios above were theoretical scenarios,
>> because I'm wondering how the firmware API here actually looks like,
>> something which so far is not really clear to me.
>>
>> When you say that you considered using 6 values, then I guess that
>> the firmware API actually offers 6 values which we can write to a single slot:
>> ac-low-power,dc-lowpower,ac-balanced,dc-balanced,ac-performance,dc-
>> performance
>>
>> ?
>>
>> But that is not what the RFC patch that started this thread shows at all,
>> the API to the driver is totally unchanged and does not get passed
>> any info on ac/dc selection ?  So it seems to me that the ACPI API Linux
>> uses for this writes only 1 of 3 values to a single slot and the EC automatically
>> switches between say ac-balanced and dc-balanced internally.
>>
>> IOW there really being 2 differently tuned balance-profiles is not visible to
>> the OS at all, this is handled internally inside the EC, correct ?
>>
> 
> No - on Lenovo's platform there are 6 different profiles that can be selected
> from the kernel driver.  3 are intended for use on battery, 3 are intended for
> use on AC.

Ah, I already got that feeling from the rest of the thread, so I reread
Mark's RFC again before posting my reply today and the RFC looked like
the same 3 profiles were being set and the only functionality added
was auto profile switching when changing between AC/battery.

Thank you for clarifying this. Having 6 different stories
indeed is a very different story.

>> Otherwise I would expect the kernel internal driver API to also change and
>> to also see a matching thinkpad_acpi patch in the RFC series?
> 
> The idea I see from Mark's thread was to send out RFC change for the platform profile
> and based on the direction try to implement the thinkpad-acpi change after that.
> 
> Because of the confusion @Mark I think you should send out an RFC v2 with thinkpad acpi
> modeled on top of this the way that you want.

I fully agree and since you introduce the concept of being on AC/battery to the
drivers/acpi/platform_profile.c cpde, please change the 
profile_set and profile_get function prototypes in struct platform_profile_handler
to also take a "bool on_battery" extra argument and use that in the thinkpad
driver to select either the ac or the battery tuned low/balanced/performance 
profile.

And please also include an update to Documentation/ABI/testing/sysfs-platform_profile
in the next RFC.

Also notice how I've tried to consistently use AC/battery in my last reply,
DC really is not a good term for "on battery". AC also is sort of dubious
for "connected to an external power-supply" but its use for that is sorta
common and it is nice and short.

Sorry if this seems like bikeshedding, but using DC for "on battery" just
feels wrong to me.


>>> The biggest use case I can think of is that a user wants performance
>>> when plugged in and power-save when unplugged; and they want that to
>>> happen automatically.
>>
>> Right, so this what I have understood all along and I'm not disagreeing
>> that this is a desirable feature, but it _does not belong in the kernel_!
>>
>> Also taking Mario's remark about the EC-firmware using differently
>> tuned balanced profiles based on ac vs dc, here is how I envision this
>> working:
>>
>> 1. Laptop is connected to charger
>> 2. EC notices this and:
>> 2.1 Internally switches from balanced-dc settings to balanced-ac settings
>> 2.2 Sends out an event about the laptop now being on AC, which the kernel
>>     picks up and then sends to userspace (this already happens)
>> 3. Userspace, e.g. power-profiles-daemon, gets the event that the laptop is
>>    now an AC and in its settings sees that the user wants to switch to
>>    performance mode on AC and uses the platform_api in its current form to
>>    ask for a switch to performance mode
>> 4. The EC gets a command telling it to switch to performance mode and
>>    switches to the ac-tuned version of performance mode since the laptop is
>>    on ac.
>>
> 
> None of this happens internally on the EC.

Ack, I understand now thank you for clarifying this.


> Also there is nothing in this design
> that guarantees it needs to be EC driven profile changes.  It could be other
> mailboxes, ASL code, SMM etc.
> 
> The key point here is that thinkpad acpi has 3 AC and 3 DC profiles to choose from,
> so some level from thinkpad acpi above needs to pick among them.

Ack.

Regards,

Hans