On Wed, 3 Apr 2024, Luke Jones wrote: > On Wednesday, 3 April 2024 12:01:15 AM NZDT Ilpo Järvinen wrote: > > On Tue, 2 Apr 2024, Luke D. Jones wrote: > > > Add support for an MCU powersave WMI call. This is intended to set the > > > MCU in to a low-power mode when sleeping. This mode can cut sleep power > > > use by around half. > > > > > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx> > > > > Hi, > > > > I fail to follow the logic of the patch here. This patch makes it > > configurable which is not bad in itself but what is the reason why a user > > would not always want to cut sleep power usage down? So this sounds like a > > feature that the user wants always enabled if available. > > > > So what are the downsides of enabling it if it's supported? > > Honestly, I'm not sure. Previously it was a source of issues but with recent > bios and more work in the various gaming-handheld distros it's much less of a > problem. The issue before was that the MCU would appear every second suspend > due to the suspend sequence being more parallel compared to windows being > sequential - the acpi calls related to this would "unplug" the USB devices > that are related to the n-key (keyboard, same internal dev as laptops) and not > complete it before suspend. And then on resume it was unreliable. > > I worked around this by calling the unplug very early, and trying to "plug" > super early also so that subsystems wouldn't notice the absence of the device > and create new devices + paths on add. Most of the requirement for that came > from some userspace programs unable to handle the unplug/plug part, but also > bad device state occurring. > > But now with the forced wait for the device to finish its task, and then the > forced wait before letting it add itself back everything is fine - although it > does mean everything sees a device properly unplugged and plugged. > > All of the above is to say that the "powersave" function was also part of the > issue as delayed things further and we couldn't add the device back before > subsystems noticed. > > Distros like bazzite and chimeraOS are now using this patch, and I'd like to > leave it to them to set a default for now. If it turns out everything is > indeed safe as houses then we can adjust the kernel default. > > A side-note I think is that because there is a forced wait time due to unable > to use the right acpi path, the old excuse of "but users might want the device > to wake faster by turning off powersave" doesn't really apply now. > > I will discuss with the main stakeholders, but for now can we accept as is? If > changes are required we'll get them done before the merge window. Yes, I think it is okay to make it configurable first and then look separately into making it default on. -- i.
