nixos/tlp: revamp

nixos/modules/services/hardware/tlp.nix

@@ -1,39 +1,26 @@
 { config, lib, pkgs, ... }:
-
 with lib;
-
 let
-
-cfg = config.services.tlp;
-
-enableRDW = config.networking.networkmanager.enable;
-
-tlp = pkgs.tlp.override {
-  inherit enableRDW;
-};
-
-# XXX: We can't use writeTextFile + readFile here because it triggers
-# TLP build to get the .drv (even on --dry-run).
-confFile = pkgs.runCommand "tlp"
-  { config = cfg.extraConfig;
-    passAsFile = [ "config" ];
-    preferLocalBuild = true;
-  }
-  ''
-    cat ${tlp}/etc/default/tlp > $out
-    cat $configPath >> $out
-  '';
-
+  cfg = config.services.tlp;
+  enableRDW = config.networking.networkmanager.enable;
+  tlp = pkgs.tlp.override { inherit enableRDW; };
+  # TODO: Use this for having proper parameters in the future
+  mkTlpConfig = tlpConfig: generators.toKeyValue {
+    mkKeyValue = generators.mkKeyValueDefault {
+      mkValueString = val:
+        if isInt val then toString val
+        else if isString val then val
+        else if true == val then "1"
+        else if false == val then "0"
+        else if isList val then "\"" + (concatStringsSep " " val) + "\""
+        else err "invalid value provided to mkTlpConfig:" (toString val);
+    } "=";
+  } tlpConfig;
 in
-
 {
-
   ###### interface
-
   options = {
-
     services.tlp = {
-
       enable = mkOption {
         type = types.bool;
         default = false;
@@ -45,77 +32,64 @@ in
         default = "";
         description = "Additional configuration variables for TLP";
       };
-
     };
-
   };
 
-
   ###### implementation
-
   config = mkIf cfg.enable {
+    boot.kernelModules = [ "msr" ];
 
-    powerManagement.scsiLinkPolicy = null;
-    powerManagement.cpuFreqGovernor = null;
-    powerManagement.cpufreq.max = null;
-    powerManagement.cpufreq.min = null;
+    environment.etc = {
+      "tlp.conf".text = cfg.extraConfig;
+    } // optionalAttrs enableRDW {
+      "NetworkManager/dispatcher.d/99tlp-rdw-nm".source =
+        "${tlp}/etc/NetworkManager/dispatcher.d/99tlp-rdw-nm";
+    };
 
-    systemd.sockets.systemd-rfkill.enable = false;
+    environment.systemPackages = [ tlp ];
 
-    systemd.services = {
-      "systemd-rfkill@".enable = false;
-      systemd-rfkill.enable = false;
-
-      tlp = {
-        description = "TLP system startup/shutdown";
-
-        after = [ "multi-user.target" ];
-        wantedBy = [ "multi-user.target" ];
-        before = [ "shutdown.target" ];
-        restartTriggers = [ confFile ];
-
-        serviceConfig = {
-          Type = "oneshot";
-          RemainAfterExit = true;
-          ExecStart = "${tlp}/bin/tlp init start";
-          ExecStop = "${tlp}/bin/tlp init stop";
-        };
-      };
-
-      tlp-sleep = {
-        description = "TLP suspend/resume";
-
-        wantedBy = [ "sleep.target" ];
-        before = [ "sleep.target" ];
-
-        unitConfig = {
-          StopWhenUnneeded = true;
-        };
-
-        serviceConfig = {
-          Type = "oneshot";
-          RemainAfterExit = true;
-          ExecStart = "${tlp}/bin/tlp suspend";
-          ExecStop = "${tlp}/bin/tlp resume";
-        };
-      };
+    # FIXME: When the config is parametrized we need to move these into a
+    # conditional on the relevant options being enabled.
+    powerManagement = {
+      scsiLinkPolicy = null;
+      cpuFreqGovernor = null;
+      cpufreq.max = null;
+      cpufreq.min = null;
     };
 
     services.udev.packages = [ tlp ];
 
-    environment.etc =
-      {
-        "default/tlp".source = confFile;
-      } // optionalAttrs enableRDW {
-        "NetworkManager/dispatcher.d/99tlp-rdw-nm" = {
-          source = "${tlp}/etc/NetworkManager/dispatcher.d/99tlp-rdw-nm";
-        };
+    systemd = {
+      packages = [ tlp ];
+      # XXX: These must always be disabled/masked according to [1].
+      #
+      # [1]: https://github.com/linrunner/TLP/blob/a9ada09e0821f275ce5f93dc80a4d81a7ff62ae4/tlp-stat.in#L319
+      sockets.systemd-rfkill.enable = false;
+      services.systemd-rfkill.enable = false;
+
+      services.tlp = {
+        # XXX: The service should reload whenever the configuration changes,
+        # otherwise newly set power options remain inactive until reboot (or
+        # manual unit restart.)
+        restartTriggers = [ config.environment.etc."tlp.conf".source ];
+        # XXX: When using systemd.packages (which we do above) the [Install]
+        # section of systemd units does not work (citation needed) so we manually
+        # enforce it here.
+        wantedBy = [ "multi-user.target" ];
       };
 
-    environment.systemPackages = [ tlp ];
-
-    boot.kernelModules = [ "msr" ];
-
+      services.tlp-sleep = {
+        # XXX: When using systemd.packages (which we do above) the [Install]
+        # section of systemd units does not work (citation needed) so we manually
+        # enforce it here.
+        before = [ "sleep.target" ];
+        wantedBy = [ "sleep.target" ];
+        # XXX: `tlp suspend` requires /var/lib/tlp to exist in order to save
+        # some stuff in there. There is no way, that I know of, to do this in
+        # the package itself, so we do it here instead making sure the unit
+        # won't fail due to the save dir not existing.
+        serviceConfig.StateDirectory = "tlp";
+      };
+    };
   };
-
 }