dmitry-s93 / MControlCenter

An application that allows you to change the settings of MSI laptops running Linux
GNU General Public License v3.0
250 stars 64 forks source link

RFE: Moving some of the functionality to a kernel driver #78

Open jwrdegoede opened 1 year ago

jwrdegoede commented 1 year ago

Short self introduction I'm a Linux developer mostly working on hw-enablement for Linux laptops. I'm also the subsystem maintainer for kernel drivers under: drivers/platform/x86

While helping a MSI laptop user with some other issue I got pointed to MControlCenter.

First of all thank you for helping MSI laptop users with your app.

Some of the EC functionality which MControlCenter exposes has standardized kernel APIs. The most interesting feature probably being setting battery charge thresholds. On ThinkPad notebooks (and also on some other type laptops). These thresholds are exposed through the standardized kernel power-supply API: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing/sysfs-class-power

For example on my ThinkPad:

[hans@x1 ~]$ ls /sys/class/power_supply/BAT0/
alarm                           charge_stop_threshold  manufacturer   subsystem
capacity                        cycle_count            model_name     technology
capacity_level                  device                 power          type
charge_behaviour                energy_full            power_now      uevent
charge_control_end_threshold    energy_full_design     present        voltage_min_design
charge_control_start_threshold  energy_now             serial_number  voltage_now
charge_start_threshold          hwmon2                 status
[hans@x1 ~]$ cat /sys/class/power_supply/BAT0/charge_control_start_threshold 
50
[hans@x1 ~]$ cat /sys/class/power_supply/BAT0/charge_control_end_threshold 
85

There are plans (but no code yet) to add support to upower and to GNOME's UI to allow users to set battery thresholds through these standardized sysfs attributes. And AFAIK in KDE powerdevil + the UI already support this.

I know that the kernel has a reputation of being hard to contribute to / an unfriendly place. But I ensure you that that is generally not the case and certainly not the case for the drivers/platform/x86 code which I maintain (under which code for MSI laptop's EC would fall).

It would be nice if these same kernel APIs would also work on MSI laptops, so I'm wondering if you would be willing to contribute support to the kernel for this? If you are worried about the review process you can send initial version of patches for this to me directly and I can do an initial review before posting the patches on the public mailinglist.

rottenpants466 commented 1 year ago

The user was me Hans +1 (You got the hotkeys working btw! this will surely help other people too!!)

If i may, a while ago i also tried getting some help with other 2 people in order to solve this issue and others:

@ThePBone https://github.com/ThePBone/msi-ec-modern @BeardOverflow https://github.com/BeardOverflow/msi-ec (Original MSI ec author, not as active)

This may be useful.

In the meantime i have been using MControlCenter, all the features work in my laptop! Kudos to dmitry!

Edit: this may be useful too: @musikid https://github.com/musikid/acpi_ec

jwrdegoede commented 1 year ago

The user was me Hans +1 (You got the hotkeys working btw! this will surely help other people too!!)

For anyone curious about the hotkey fixes for the MSI Summit E16 Flip, see: https://github.com/systemd/systemd/pull/25824

If i may, a while ago i also tried getting some help with other 2 people in order to solve this issue and others:

@ThePBone https://github.com/ThePBone/msi-ec-modern @BeardOverflow https://github.com/BeardOverflow/msi-ec (Original MSI ec author, not as active)

So lots of activity from the community to try and get MSI laptops better supported under Linux, which is great to see.

As the kernel subsystem maintainers for all the various laptop specific drivers including the original https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/msi-laptop.c it would be great if one or more people from the community could help with getting better support for MSI laptops into the upstream Linux kernel.

As already mentioned above I know that the kernel has a reputation of being hard to contribute to / an unfriendly place. But I ensure you that that is generally not the case and certainly not the case for the drivers/platform/x86 code which I maintain.

Also let me repeat that e.g. setting battery charging limits would be a good place to start with extending the mainline kernel support.

Starting with some other feature would be fine too if that is preferred, but I do encourage people to start with introducing just 1 feature and then work towards getting more stuff supported from there.

Also I wonder if anyone has ever looked at how MSI's own Windows software does things, does that talk directly to the EC, or are there maybe higher level firmware interfaces like ACPI functions or WMI interfaces which can be used?

Those would possibly be a better abstraction level to talk to since they might hide away differences between different hw versions / models.

jwrdegoede commented 1 year ago

Note starting tomorrow I'm taking time of from work for 2 weeks and I won't be reading email (or responding to github issues) during that time.

So if you are wondering why I'm quiet / not responding that is why :)

I'll "see" you all in 2 weeks.

Merry Christmas and a happy new year!

rottenpants466 commented 1 year ago

Merry Christmas and a happy new year šŸŽ„šŸŽ‰

dmitry-s93 commented 1 year ago

It would be nice if these same kernel APIs would also work on MSI laptops, so I'm wondering if you would be willing to contribute support to the kernel for this?

Hello. It would be nice to add the ability to set the battery charge threshold. Maybe I'll try later.

teackot commented 1 year ago

Hi! msi-ec is having some progress in this direction. It has standardized charge threshold implemented. I just added battery capacity and status in my fork. I also started adding support for different laptops (values are hardcoded for now, should probably be in config files)

rottenpants466 commented 1 year ago

Hi! msi-ec is having some progress in this direction. It has standardized charge threshold implemented. ~I just added battery capacity and status in my fork~. I also started adding support for different laptops (values are hardcoded for now, should probably be in config files)

Hi, awesome work!

I just asked @BeardOverflow if he saw this post in order to work something out. It seems that @ThePBone also has expertise in this matter and maybe he has time to help out :)

jwrdegoede commented 1 year ago

Hi! msi-ec is having some progress in this direction. It has standardized charge threshold implemented. ~I just added battery capacity and status in my fork~. I also started adding support for different laptops (values are hardcoded for now, should probably be in config files)

Thank you for your work on this, I just commented (with some high level remarks) on your pull-request here: https://github.com/BeardOverflow/msi-ec/pull/13#issuecomment-1413805438