joshuagrisham/samsung-galaxybook-extras

Is it possible to install just some driver features?

mhagnumdw opened this issue · 10 comments

For example, just the Keyboard backlight controller.

You mean as different modules or one module with features that can be disabled?

You mean as different modules or one module with features that can be disabled?

Whatever is best to implement for the project maintainer.

The easiest to implement is module parameters to disable some functionality, but I don't understand why you would want that, considering unused features do not bother. E.g. if you never change the performance mode then it's the same if the driver is loaded or not.

The only thing that would make sense to disable would be the handling of the brightness key, in case you have userspace software that can do that in a better way.

As the driver is not a finished work and there is the warn use at your own risk! and my note even works well with Fedora 38 (although without camera, without fingerprint and keyboard brightness always on), I would like to be the least intrusive as possible to try and avoid damaging the hardware in any way.

I'd say it's unlikely anything bad should happen, but you could fork the driver and trim away everything except the keyboard code.

Hi @mhagnumdw one thing I can think is to maybe add some config parameters which could be set either at compile or boot time (like exists in some other drivers) if this is really super necessary? But unless I am not thinking of something, from what I have seen this seems to usually only be done for things like "debug" flags in Platform drivers? (for example like this: https://github.com/torvalds/linux/blob/13a2e429f644691fca70049ea1c75f135957c788/drivers/platform/x86/amd/pmf/auto-mode.c#L18)

As @jcarrano said I would guess this driver should be relatively safe because it is using ACPI for everything right now. As of right now, the only things that will be "touched" without you doing anything (e.g. the driver is loaded but you have not done anything to read or set any of the flags etc) are:

  1. A specific Samsung ACPI device method will be executed with a series of "init" payloads that seems to enable being able to control some of the settings like for example the keyboard backlight controller, the performance mode setting, etc
  2. The performance mode will be set to "Balanced" by default when the module is loaded which will call a specific Samsung ACPI device method to enable that performance mode

Otherwise all other features you would need to actually interact with in some way before anything would actually happen (e.g. try to read or set the value for "Battery Saver" for example), and even there it would be using the ACPI methods to do these things.

Even having said all of that, in general with the Linux ACPI subsystem there are a lot of safe-guards in place:

  1. if the requested ACPI device does not exist (SAM0429) then the driver would not even load at all of course, so nothing to worry there ;)
  2. If the various ACPI methods that are called do not exist, require different request payloads or have different response payloads than what is written into this driver, then execution of those methods or reading of the various fields will just "fail" (so an error will be printed in the kernel log, for example), but nothing else would actually "happen" with the device

The only case I can think that might get into "gray area" is if Samsung uses the exactly same ACPI device and method, with exactly the same signature (has exactly same number, type, and length of parameters both in and out), but that behave very differently with the exact payloads that are working in the current devices -- a bit of a long-shot, and the hope is that Samsung would have definitely thought about this even for their own ease of maintenance and to minimize risk of causing problems... so I would be REALLY shocked if this ever revealed to be the case!

That is the long answer, but I guess the TL;DR is basically what @jcarrano said -- yeah, some parameters or etc could be created to flag certain features to be enabled or disabled, but that is a bit of an anti-pattern for platform drivers to create their own new flags (as opposed to reacting to other standard parameters that already exist, which is quite common e.g. CONFIG_PM_SLEEP etc) and anyway I see quite low risk that anything in this driver would cause any problem (the "feature" would just not work basically), and otherwise anyone could always fork the repo and rip out whatever they don't like 😁

Hi @joshuagrisham ! Firstly, thank you for your work.

if this is really super necessary?

Given everything you've said, I don't think so.

I saw in other threads that your goal is:

my goal would be to get some version of this driver mainlined into the kernel so it just comes as part of the shipped kernel anyway so I have not focused so much on things like that

I believe this is more relevant.

Again, thanks for this driver! I will still install it. I use a Samsung Galaxy Book3 Ultra.

I installed the driver. For now everything is ok! If I can help by giving you any feedback, please let me know.

It would be so good if there was already a solution for the camera and fingerprint reader.

For the speaker to work I run: curl -s -L https://raw.githubusercontent.com/joshuagrisham/galaxy-book2-pro-linux/main/sound/necessary-verbs.sh | sudo bash

  • Fedora 38
  • Samsung Galaxy Book3 Ultra

Cool @mhagnumdw great to hear!

Regarding the camera I have not looked into this on the GB3 (it "just works" on the GB2) but I think there were some Intel and/or other things in the works there?

Regarding the fingerprint reader, I made a driver for this too that is now part of libfprint. If you can manage to install libfprint >= v1.94.7 in your distro then it should hopefully "just work", otherwise you can get and build latest libfprint from source: https://gitlab.freedesktop.org/libfprint/libfprint

If you can manage to install libfprint >= v1.94.7 in your distro then it should hopefully "just work", otherwise you can get and build latest libfprint from source: https://gitlab.freedesktop.org/libfprint/libfprint

  • Fedora 38 with libfprint.x86_64 1.94.6-1.fc38
  • Gnome 44.10
  • Samsung Galaxy Book3 Ultra

So I started manually installing libfprint version 1.94.7:

sudo dnf groupinstall "Development Tools" "Development Libraries"
sudo dnf install meson python3 python3-pip python3-setuptools python3-wheel ninja-build
sudo dnf install gcc-c++
sudo dnf install cmake
sudo dnf install libgusb-devel
sudo dnf install cairo-devel gobject-introspection-devel
sudo dnf install nss-devel
sudo dnf install libgudev-devel
sudo dnf install gtk-doc
sudo dnf install valgrind

git clone https://gitlab.freedesktop.org/libfprint/libfprint.git
cd libfprint
git checkout v1.94.7
meson setup builddir && cd builddir
meson test --print-errorlogs
sudo meson install

reboot

sudo systemctl start fprintd
sudo systemctl status fprintd # ok - running

Still, the fingerprint reader option does not appear in Gnome. I'm also still not sure if the device is recognized.

Then I will try more.

I'm going to close this thread, as it is already deviating from the original topic.

Thank you very much!