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

["Limonciello, Mario" <Mario.Limonciello@amd.com>]

[Public]

+Shyam

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, March 14, 2022 11:56
> To: Mark Pearson <markpearson@lenovo.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Cc: rafael@kernel.org; linux-acpi@vger.kernel.org; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [External] Re: [RFC] ACPI: platform-profile: support for AC vs DC
> modes
> 
> Hi,
> 
> On 3/14/22 16:32, Mark Pearson wrote:
> >
> >
> > On 2022-03-14 11:31, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 3/14/22 15:43, Hans de Goede wrote:
> >>> 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.
> >>
> >> One last request for the v2 RFC, please also Cc Bastien Nocera, so that
> >> he can take a look at the proposed uapi changes from the userspace side
> >> of things.
> >>
> > Ack - will do.
> 
> So I've been thinking a bit more about this while I was outside for some
> fresh air.
> 
> First of all let me say that I do agree that the having in essence 6
> different profiles thing needs a kernel solution.
> 
> What I'm not entirely sure about is if this needs to be something
> generic, with a new userspace-API as you proposed in the v1 RFC,
> or if it would be better to just solve this in thinkpad_acpi.c .
> 
> Now that I've a better grasp of the problem, I'll start a new email
> thread on this tomorrow with all the various take-holders in the Cc
> to try and answer that question.

I've been viewing Lenovo's implementation as "the first one to do it this way".
In the future AMD plans to submit a driver that provides platform profiles
for some SOCs.  It will provide similar capabilities to what thinkpad_acpi does.

I haven't mentioned it thus far because it's still under development and of
course this could change or be cancelled.  I expect that the decision made
around what to do with thinpad_acpi will also have ramifications for
what that driver will do too when it's finished and submitted.

Please include Shyam (now on CC) in the new discussion thread.  His team
owns that new solution I mention and this can affect their development 
direction too.

I say this all to mean "thinkpad_acpi might not be the outlier".

> 
> It probably is a good idea to wait with doing a v2 of the RFC until
> we've had that discussion...
> 
> Regards,
> 
> Hans