search

joshuagrisham / samsung-galaxybook-extras

Samsung Galaxybook Linux platform driver and accompanying utilities.
116 stars 15 forks source link

Fan speed does not work for models lacking FANS and FANT #34

Closed joshuagrisham closed 3 weeks ago

joshuagrisham commented 3 weeks ago

Per information here: https://github.com/joshuagrisham/samsung-galaxybook-extras/issues/32#issuecomment-2418268856

Some models have multiple fans and do not have FANT and FANS present in the ACPI to support the current logic in the driver.

The fan_speed logic needs to be reworked a bit to account for these different cases, as per comment in https://github.com/joshuagrisham/samsung-galaxybook-extras/issues/32#issuecomment-2421527712

I am not sure how it is "typically" done in the kernel when the _FST method is working correctly but not supported out-of-the-box by ACPI Fan support... my assumption is that this driver can just add them to its own hwmon support but I will need to redo the logic a bit to maybe do something like this:

  1. get all fans for the device
  2. for each one, see if they are somehow supported out-of-the-box already (e.g. all 4 methods are present or similar)
  3. if not, test their _FST method
  4. if _FST method is good, then add fan_speed_rpm attribute and add hwmon fan channel for it, plus ensure that the "get_speed" method will execute _FST and fetch the speed from the method
  5. if _FST method is not good, then check for existence of FANS and FANT and then set those up instead (similar to point 4 above)

FYI @seaasses

joshuagrisham commented 3 weeks ago

@seaasses would you please mind to try the latest version at this branch https://github.com/joshuagrisham/samsung-galaxybook-extras/tree/34-fan-speed-does-not-work-for-models-lacking-fans-and-fant and see if this works better with the fans for your model?

The intention is that you should now have 2 fans under the samsung_galaxybook device in sensors; one called "CPU Fan" and one called "GPU Fan" (since that is what your device's ACPI seems to report their description as), with the correct speed readings on them.

seaasses commented 3 weeks ago

Hey @joshuagrisham, sorry, I didn't reply because my laptop has been with Samsung for a few days (that usb c problem was really a problem), I will get it back tomorrow and will test all the things you tag me