Message-ID: <zelin5tbkup26skhs3dwacwxl33h4ryzgrn3nefay7fxotb5v7@aumb6v7hexpc>
Date: Tue, 7 Jan 2025 12:25:40 -0500
From: Kurt Borja <kuurtb@...il.com>
To: "Limonciello, Mario" <mario.limonciello@....com>
Cc: Hridesh MG <hridesh699@...il.com>, 
	Mark Pearson <mpearson-lenovo@...ebb.ca>, 
	"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>, josh@...huagrisham.com, 
	"Derek J . Clark" <derekjohn.clark@...il.com>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Len Brown <lenb@...nel.org>, Maximilian Luz <luzmaximilian@...il.com>, 
	Hans de Goede <hdegoede@...hat.com>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Lee Chun-Yi <jlee@...e.com>, Shyam Sundar S K <Shyam-sundar.S-k@....com>, 
	Corentin Chary <corentin.chary@...il.com>, "Luke D . Jones" <luke@...nes.dev>, 
	Lyndon Sanche <lsanche@...deno.ca>, Ike Panhc <ike.pan@...onical.com>, 
	Henrique de Moraes Holschuh <hmh@....eng.br>, Armin Wolf <W_Armin@....de>, 
	Colin Ian King <colin.i.king@...il.com>, Alexis Belmonte <alexbelm48@...il.com>, 
	Uwe Kleine-König <u.kleine-koenig@...libre.com>, Ai Chao <aichao@...inos.cn>, Gergo Koteles <soyer@....hu>, 
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>, linux-kernel@...r.kernel.org, Dell.Client.Kernel@...l.com, 
	ibm-acpi-devel@...ts.sourceforge.net
Subject: Re: [RFC PATCH 0/3] ACPI: platform_profile: Let drivers dynamically
 refresh choices

On Tue, Jan 07, 2025 at 10:47:39AM -0600, Limonciello, Mario wrote:
> 
> 
> On 1/7/2025 10:33 AM, Hridesh MG wrote:
> > On Tue, Jan 7, 2025 at 9:21 PM Mario Limonciello
> > <mario.limonciello@....com> wrote:
> > > 
> > > On 1/7/2025 07:14, Hridesh MG wrote:
> > > > On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@...ebb.ca> wrote:
> > > > > 
> > > > > Hi Kurt,
> > > > > 
> > > > > On Sun, Jan 5, 2025, at 11:45 PM, Kurt Borja wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > Some drivers may need to dynamically modify their selected `choices`.
> > > > > > Such is the case of the acer-wmi driver, which implemented their own
> > > > > > profile cycling method, because users expect different profiles to be
> > > > > > available whether the laptop is on AC or not [1].
> > > > > > 
> > > > > > These series would allow acer-wmi to simplify this custom cycling method
> > > > > > to use platform_profile_cycle(), as it's already being proposed in these
> > > > > > series [2]; without changing expected behaviors, by refreshing their
> > > > > > selected choices on AC connect/disconnect events, which would also solve
> > > > > > this discussion [3].
> > > > > > 
> > > > > > Additionally, I think the platform_profile_ops approach would enable us
> > > > > > to hide the platform_profile_handler in the future, and instead just pass
> > > > > > the class device to get/set methods like the HWMON subsystem does.
> > > > > > 
> > > > > > I think having this kind of flexibility is valuable. Let me know what you
> > > > > > think!
> > > > > > 
> > > > > 
> > > > > I personally would love to see how this would be used for the acer issue highlighted to see how it would work out. It feels like the series is short a patch :)
> > > > 
> > > > I do think that having this flexibility would be good, as i was
> > > > considering manually clearing the set bits and calling platform_notify
> > > > for the acer series, but in my specific case (for devices using the
> > > > predator v4 interface) it was found that the hardware was responsive
> > > > to all profiles regardless of AC/battery mode so it made sense to
> > > > leave this kind of artificial limiting of profiles to the
> > > > userspace--similar to how the Windows driver handles it through the
> > > > Nitro Sense app.
> > > > 
> > > > I feel like users should have the power to utilize their hardware in
> > > > the way they want it to, though if there is a compelling reason to
> > > > limit the profiles then i'd be more than happy to add this to the
> > > > series :)
> > > > 
> > > > 
> > > > --
> > > > Hridesh MG
> > > 
> > > I agree with Mark, this series is missing the patch that shows exactly
> > > how this would be used.  Furthermore; what exactly are the differences
> > > in choices between AC or DC?
> > On the predator series, the Windows OEM application only allows you to
> > select the low-power and balanced platform profiles on DC. These
> > profiles can however still be activated through WMI methods and the
> > hardware will apply them.
> 
> So on Windows if you are on DC and pick "performance" then go to AC it will
> automatically switch you back to "balanced" and you can't pick "performance"
> again until you go to "DC"?
> 
> This sounds totally counterintuitive to me at least.  If you're going to
> gimp people on anything, gimp them on DC.
> 
> > 
> > > To "userspace" I fail to understand how "balanced" is different from AC
> > > to DC for example.
> > It is not, the profiles or states themselves are not modified between
> > AC and DC, just the switching between them is affected.
> > 
> 
> IMO - just because Windows does this doesn't mean we need to in Linux. If
> there are only 3 sets of profiles, I'm of the opinion we should let users
> pick any of the 3 profiles in "AC" or "DC" state.

Hi!

After giving it some thought, I agree with you and Hridesh. Kernel
should not limit profile choices if they *are* selectable.

If a "proof of concept" patch is still interesting I'll be glad to send
it, otherwise I think my original idea has too many problems. User-space
should be able to handle these special cases.

I think an attribute allowing/disallowing power sensitive values is
interesting. Maybe allow users too attach/detach individual profiles
from being selected/cycled? On that note, it would also be interesting to
be able to detach invidivual "profile handlers" from the legacy
`acpi_kobj`. But I'm not sure if this added complexity would be worth it.

Anyway.. Mario, do you think hiding platform_profile_handler from
drivers is something worth pursuing? Similar to what the hwmon class
does. I feel having some struct members like `minor` and `choices`
exposed, or having the profile_get/profile_set callbacks not being
const, while it's not the end of the world, could be problematic.

~ Kurt

> 

Powered by blists - more mailing lists