| Message ID | 20250106044605.12494-1-kuurtb@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ACPI: platform_profile: Let drivers dynamically refresh choices | expand |
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 :) As a side note, I did (many moons ago) propose a change to alter profiles used depending on AC/battery mode (in the thinkpad driver), and it was rejected as something that should be done in user space. Your use case does seem somewhat different, but it's similar enough that if you get it working I'd be interested to see if I can take advantage of the approach too. Mark
On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@squebb.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
On 1/7/2025 07:14, Hridesh MG wrote: > On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@squebb.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? To "userspace" I fail to understand how "balanced" is different from AC to DC for example. If an implementation has differences for AC vs DC would it make more sense to have that abstraction in the "profile handlers" instead of the ops? IE always have "low-power, balanced, performance" for choices. If the system is on AC the profile handler might do something different than on DC for a given state. Another idea I had while thinking about this is that the platform profile core can have a sysfs file to indicate whether or not to "allow" power state sensitive values. It could default to "1" and then it can send AC/DC events to the platform profile handlers when enabled. If userspace prefers not to use it, then they can set it to 0 and then the profile handler would stop reacting.
On Tue, Jan 7, 2025 at 9:21 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 1/7/2025 07:14, Hridesh MG wrote: > > On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@squebb.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. > 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.
On 1/7/2025 10:33 AM, Hridesh MG wrote: > On Tue, Jan 7, 2025 at 9:21 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> On 1/7/2025 07:14, Hridesh MG wrote: >>> On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@squebb.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.
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@amd.com> wrote: > > > > > > On 1/7/2025 07:14, Hridesh MG wrote: > > > > On Tue, Jan 7, 2025 at 7:49 AM Mark Pearson <mpearson-lenovo@squebb.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 >
> > 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. Yeah, I think this is still an interesting idea that's still worth pursuing. Making the API simpler for drivers is a net benefit and reduction in tech debt.
On Tue, Jan 07, 2025 at 11:28:06AM -0600, Limonciello, Mario wrote: > > > > 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. > > Yeah, I think this is still an interesting idea that's still worth pursuing. > > Making the API simpler for drivers is a net benefit and reduction in tech > debt. That's good to hear. I'm working on it! Hopefully I'll be able to submit it in a couple of days. ~ Kurt >