Move the NVIDIA support into its own module

Previously all card-specific stuff was scattered across xserver.nix
and opengl.nix, which is ugly. Now it can be kept together in a single
card-specific module. This required the addition of a few internal
options:

- services.xserver.drivers: A list of { name, driverName, modules,
  libPath } sets.

- hardware.opengl.package: The OpenGL implementation. Note that there
  can be only one OpenGL implementation at a time in a system
  configuration (i.e. no dynamic detection).

- hardware.opengl.package32: The 32-bit OpenGL implementation.

nixos/modules/hardware/opengl.nix

@@ -10,6 +10,15 @@ let
 
   videoDrivers = config.services.xserver.videoDrivers;
 
+  makePackage = p: p.buildEnv {
+    name = "mesa-drivers+txc-${p.mesa_drivers.version}";
+    paths =
+      [ p.mesa_drivers
+        p.mesa_noglu # mainly for libGL
+        (if cfg.s3tcSupport then p.libtxc_dxtn else p.libtxc_dxtn_s2tc)
+      ];
+  };
+
 in
 
 {
@@ -52,73 +61,64 @@ in
       '';
     };
 
+    hardware.opengl.package = mkOption {
+      type = types.package;
+      internal = true;
+      description = ''
+        The package that provides the OpenGL implementation.
+      '';
+    };
+
+    hardware.opengl.package32 = mkOption {
+      type = types.package;
+      internal = true;
+      description = ''
+        The package that provides the 32-bit OpenGL implementation on
+        64-bit systems.  Used when <option>driSupport32Bit</option> is
+        set.
+      '';
+    };
+
   };
 
   config = mkIf cfg.enable {
+
     assertions = pkgs.lib.singleton {
       assertion = cfg.driSupport32Bit -> pkgs.stdenv.isx86_64;
-      message = "Option driSupport32Bit only makes sens on a 64-bit system.";
+      message = "Option driSupport32Bit only makes sense on a 64-bit system.";
     };
 
-    system.activationScripts.setup-opengl.deps = [];
+    system.activationScripts.setup-opengl =
-    system.activationScripts.setup-opengl.text = ''
+      ''
-      rm -f /run/opengl-driver{,-32}
+        ln -sfn ${cfg.package} /run/opengl-driver
-      ${optionalString (pkgs.stdenv.isi686) "ln -sf opengl-driver /run/opengl-driver-32"}
+        ${if pkgs.stdenv.isi686 then ''
-    ''
+          ln -sfn opengl-driver /run/opengl-driver-32
-      #TODO:  The OpenGL driver should depend on what's detected at runtime.
+        '' else if cfg.driSupport32Bit then ''
-     +( if elem "nvidia" videoDrivers then
+          ln -sfn ${cfg.package32} /run/opengl-driver-32
-          ''
+        '' else ''
-            ln -sf ${kernelPackages.nvidia_x11} /run/opengl-driver
+          rm -f /run/opengl-driver-32
-            ${optionalString cfg.driSupport32Bit
+        ''}
-              "ln -sf ${pkgs_i686.linuxPackages.nvidia_x11.override { libsOnly = true; kernel = null; } } /run/opengl-driver-32"}
+      '';
-          ''
-        else if elem "nvidiaLegacy173" videoDrivers then
-          "ln -sf ${kernelPackages.nvidia_x11_legacy173} /run/opengl-driver"
-        else if elem "nvidiaLegacy304" videoDrivers then
-          ''
-            ln -sf ${kernelPackages.nvidia_x11_legacy304} /run/opengl-driver
-            ${optionalString cfg.driSupport32Bit
-              "ln -sf ${pkgs_i686.linuxPackages.nvidia_x11_legacy304.override { libsOnly = true; kernel = null; } } /run/opengl-driver-32"}
-          ''
-        else if elem "ati_unfree" videoDrivers then
-          "ln -sf ${kernelPackages.ati_drivers_x11} /run/opengl-driver"
-        else
-          let
-            lib_fun = p: p.buildEnv {
-              name = "mesa-drivers+txc-${p.mesa_drivers.version}";
-              paths = [
-                p.mesa_drivers
-                p.mesa_noglu # mainly for libGL
-                (if cfg.s3tcSupport then p.libtxc_dxtn else p.libtxc_dxtn_s2tc)
-              ];
-            };
-          in
-          ''
-            ${optionalString cfg.driSupport "ln -sf ${lib_fun pkgs} /run/opengl-driver"}
-            ${optionalString cfg.driSupport32Bit
-              "ln -sf ${lib_fun pkgs_i686} /run/opengl-driver-32"}
-          ''
-      );
 
     environment.variables.LD_LIBRARY_PATH =
       [ "/run/opengl-driver/lib" "/run/opengl-driver-32/lib" ];
 
+    # FIXME: move this into card-specific modules.
+    hardware.opengl.package = mkDefault
+      (if elem "ati_unfree" videoDrivers then
+        kernelPackages.ati_drivers_x11
+      else
+        makePackage pkgs);
+
+    hardware.opengl.package32 = mkDefault (makePackage pkgs_i686);
+
     boot.extraModulePackages =
-      optional (elem "nvidia" videoDrivers) kernelPackages.nvidia_x11 ++
-      optional (elem "nvidiaLegacy173" videoDrivers) kernelPackages.nvidia_x11_legacy173 ++
-      optional (elem "nvidiaLegacy304" videoDrivers) kernelPackages.nvidia_x11_legacy304 ++
       optional (elem "virtualbox" videoDrivers) kernelPackages.virtualboxGuestAdditions ++
       optional (elem "ati_unfree" videoDrivers) kernelPackages.ati_drivers_x11;
 
-    boot.blacklistedKernelModules =
-      optionals (elem "nvidia" videoDrivers) [ "nouveau" "nvidiafb" ];
-
     environment.etc =
-      (optionalAttrs (elem "ati_unfree" videoDrivers) {
+      optionalAttrs (elem "ati_unfree" videoDrivers) {
         "ati".source = "${kernelPackages.ati_drivers_x11}/etc/ati";
-      })
+      };
-      // (optionalAttrs (elem "nvidia" videoDrivers) {
-        "OpenCL/vendors/nvidia.icd".source = "${kernelPackages.nvidia_x11}/lib/vendors/nvidia.icd";
-      });
   };
 }

nixos/modules/hardware/video/nvidia.nix

@@ -0,0 +1,54 @@
+# This module provides the proprietary NVIDIA X11 / OpenGL drivers.
+
+{ config, lib, pkgs, pkgs_i686, ... }:
+
+with lib;
+
+let
+
+  drivers = config.services.xserver.videoDrivers;
+
+  # FIXME: should introduce an option like
+  # ‘hardware.video.nvidia.package’ for overriding the default NVIDIA
+  # driver.
+  enabled = elem "nvidia" drivers || elem "nvidiaLegacy173" drivers || elem "nvidiaLegacy304" drivers;
+
+  nvidia_x11 =
+    if elem "nvidia" drivers then
+      config.boot.kernelPackages.nvidia_x11
+    else if elem "nvidiaLegacy173" drivers then
+      config.boot.kernelPackages.nvidia_x11_legacy173
+    else if elem "nvidiaLegacy304" videoDrivers then
+      config.boot.kernelPackages.nvidia_x11_legacy304
+    else throw "impossible";
+
+in
+
+{
+
+  config = mkIf enabled {
+
+    services.xserver.drivers = singleton
+      { name = "nvidia"; modules = [ nvidia_x11 ]; libPath = [ nvidia_x11 ]; };
+
+    services.xserver.screenSection =
+      ''
+        Option "RandRRotation" "on"
+      '';
+
+    hardware.opengl.package = nvidia_x11;
+    hardware.opengl.package32 = pkgs_i686.linuxPackages.nvidia_x11.override { libsOnly = true; kernel = null; };
+
+    environment.systemPackages = [ nvidia_x11 ];
+
+    boot.extraModulePackages = [ nvidia_x11 ];
+
+    boot.blacklistedKernelModules = [ "nouveau" "nvidiafb" ];
+
+    services.acpid.enable = true;
+
+    environment.etc."OpenCL/vendors/nvidia.icd".source = "${nvidia_x11}/lib/vendors/nvidia.icd";
+
+  };
+
+}

nixos/modules/module-list.nix

@@ -32,6 +32,7 @@
   ./hardware/opengl.nix
   ./hardware/pcmcia.nix
   ./hardware/video/bumblebee.nix
+  ./hardware/video/nvidia.nix
   ./installer/tools/nixos-checkout.nix
   ./installer/tools/tools.nix
   ./misc/assertions.nix

nixos/modules/services/x11/xserver.nix

@@ -11,30 +11,16 @@ let
   xorg = pkgs.xorg;
 
 
-  # Map video driver names to driver packages.
+  # Map video driver names to driver packages. FIXME: move into card-specific modules.
   knownVideoDrivers = {
     ati_unfree   = { modules = [ kernelPackages.ati_drivers_x11 ]; driverName = "fglrx"; };
     nouveau       = { modules = [ pkgs.xf86_video_nouveau ]; };
-    nvidia       = { modules = [ kernelPackages.nvidia_x11 ]; };
-    nvidiaLegacy173 = { modules = [ kernelPackages.nvidia_x11_legacy173 ]; driverName = "nvidia"; };
-    nvidiaLegacy304 = { modules = [ kernelPackages.nvidia_x11_legacy304 ]; driverName = "nvidia"; };
     unichrome    = { modules = [ pkgs.xorgVideoUnichrome ]; };
     virtualbox   = { modules = [ kernelPackages.virtualboxGuestAdditions ]; driverName = "vboxvideo"; };
     ati = { modules = [ pkgs.xorg.xf86videoati pkgs.xorg.glamoregl ]; };
     intel-testing = { modules = with pkgs.xorg; [ xf86videointel-testing glamoregl ]; driverName = "intel"; };
   };
 
-  driverNames = cfg.videoDrivers;
-
-  needsAcpid =
-     (elem "nvidia" driverNames) ||
-     (elem "nvidiaLegacy173" driverNames) ||
-     (elem "nvidiaLegacy304" driverNames);
-
-  drivers = flip map driverNames
-    (name: { inherit name; driverName = name; } //
-      attrByPath [name] (if (hasAttr ("xf86video" + name) xorg) then { modules = [(getAttr ("xf86video" + name) xorg) ]; } else throw "unknown video driver `${name}'") knownVideoDrivers);
-
   fontsForXServer =
     config.fonts.fonts ++
     # We don't want these fonts in fonts.conf, because then modern,
@@ -79,7 +65,6 @@ let
     monitors = foldl mkMonitor [] xrandrHeads;
   in concatMapStrings (getAttr "value") monitors;
 
-
   configFile = pkgs.stdenv.mkDerivation {
     name = "xserver.conf";
 
@@ -204,6 +189,15 @@ in
         '';
       };
 
+      drivers = mkOption {
+        type = types.listOf types.attrs;
+        internal = true;
+        description = ''
+          A list of attribute sets specifying drivers to be loaded by
+          the X11 server.
+        '';
+      };
+
       vaapiDrivers = mkOption {
         type = types.listOf types.path;
         default = [ ];
@@ -397,9 +391,21 @@ in
   ###### implementation
 
   config = mkIf cfg.enable {
-    hardware.opengl.enable = true;
+
+    hardware.opengl.enable = mkDefault true;
+
     services.xserver.videoDrivers = mkIf (cfg.videoDriver != null) [ cfg.videoDriver ];
 
+    # FIXME: somehow check for unknown driver names.
+    services.xserver.drivers = flip concatMap cfg.videoDrivers (name:
+      let driver =
+        attrByPath [name]
+          (if (hasAttr ("xf86video" + name) xorg)
+           then { modules = [(getAttr ("xf86video" + name) xorg) ]; }
+           else null)
+          knownVideoDrivers;
+      in optional (driver != null) ({ inherit name; driverName = name; } // driver));
+
     assertions =
       [ { assertion = !(config.programs.ssh.startAgent && cfg.startGnuPGAgent);
           message =
@@ -438,24 +444,18 @@ in
         pkgs.xterm
         pkgs.xdg_utils
       ]
-      ++ optional (elem "nvidia" driverNames) kernelPackages.nvidia_x11
+      ++ optional (elem "virtualbox" cfg.videoDrivers) xorg.xrefresh
-      ++ optional (elem "nvidiaLegacy173" driverNames) kernelPackages.nvidia_x11_legacy173
+      ++ optional (elem "ati_unfree" cfg.videoDrivers) kernelPackages.ati_drivers_x11;
-      ++ optional (elem "nvidiaLegacy304" driverNames) kernelPackages.nvidia_x11_legacy304
-      ++ optional (elem "virtualbox" driverNames) xorg.xrefresh
-      ++ optional (elem "ati_unfree" driverNames) kernelPackages.ati_drivers_x11;
-
-    services.acpid.enable = mkIf needsAcpid true;
 
     environment.pathsToLink =
       [ "/etc/xdg" "/share/xdg" "/share/applications" "/share/icons" "/share/pixmaps" ];
 
     systemd.defaultUnit = mkIf cfg.autorun "graphical.target";
 
-    systemd.services."display-manager" =
+    systemd.services.display-manager =
       { description = "X11 Server";
 
-        after = [ "systemd-udev-settle.service" "local-fs.target" ]
+        after = [ "systemd-udev-settle.service" "local-fs.target" "acpid.service" ];
-                ++ optional needsAcpid "acpid.service";
 
         restartIfChanged = false;
 
@@ -463,15 +463,11 @@ in
           { FONTCONFIG_FILE = "/etc/fonts/fonts.conf"; # !!! cleanup
             XKB_BINDIR = "${xorg.xkbcomp}/bin"; # Needed for the Xkb extension.
             XORG_DRI_DRIVER_PATH = "/run/opengl-driver/lib/dri"; # !!! Depends on the driver selected at runtime.
-          } // optionalAttrs (elem "nvidia" driverNames) {
+            LD_LIBRARY_PATH = concatStringsSep ":" (
-            LD_LIBRARY_PATH = "${xorg.libX11}/lib:${xorg.libXext}/lib:${kernelPackages.nvidia_x11}/lib";
+              [ "${xorg.libX11}/lib" "${xorg.libXext}/lib" ]
-          } // optionalAttrs (elem "nvidiaLegacy173" driverNames) {
+              ++ optionals (elem "ati_unfree" cfg.videoDrivers)
-            LD_LIBRARY_PATH = "${xorg.libX11}/lib:${xorg.libXext}/lib:${kernelPackages.nvidia_x11_legacy173}/lib";
+                [ "${kernelPackages.ati_drivers_x11}/lib" "${kernelPackages.ati_drivers_x11}/X11R6/lib64/modules/linux" ]
-          } // optionalAttrs (elem "nvidiaLegacy304" driverNames) {
+              ++ concatLists (catAttrs "libPath" cfg.drivers));
-            LD_LIBRARY_PATH = "${xorg.libX11}/lib:${xorg.libXext}/lib:${kernelPackages.nvidia_x11_legacy304}/lib";
-          } // optionalAttrs (elem "ati_unfree" driverNames) {
-            LD_LIBRARY_PATH = "${xorg.libX11}/lib:${xorg.libXext}/lib:${kernelPackages.ati_drivers_x11}/lib:${kernelPackages.ati_drivers_x11}/X11R6/lib64/modules/linux";
-            #XORG_DRI_DRIVER_PATH = "${kernelPackages.ati_drivers_x11}/lib/dri"; # is ignored because ati drivers ship their own unpatched libglx.so !
           } // cfg.displayManager.job.environment;
 
         preStart =
@@ -501,7 +497,7 @@ in
       ] ++ optional (!cfg.enableTCP) "-nolisten tcp";
 
     services.xserver.modules =
-      concatLists (catAttrs "modules" drivers) ++
+      concatLists (catAttrs "modules" cfg.drivers) ++
       [ xorg.xorgserver
         xorg.xf86inputevdev
       ];
@@ -537,7 +533,7 @@ in
           ${cfg.serverLayoutSection}
           # Reference the Screen sections for each driver.  This will
           # cause the X server to try each in turn.
-          ${flip concatMapStrings drivers (d: ''
+          ${flip concatMapStrings cfg.drivers (d: ''
             Screen "Screen-${d.name}[0]"
           '')}
         EndSection
@@ -551,11 +547,11 @@ in
 
         # For each supported driver, add a "Device" and "Screen"
         # section.
-        ${flip concatMapStrings drivers (driver: ''
+        ${flip concatMapStrings cfg.drivers (driver: ''
 
           Section "Device"
             Identifier "Device-${driver.name}[0]"
-            Driver "${driver.driverName}"
+            Driver "${driver.driverName or driver.name}"
             ${if cfg.useGlamor then ''Option "AccelMethod" "glamor"'' else ""}
             ${cfg.deviceSection}
             ${xrandrDeviceSection}
@@ -574,10 +570,6 @@ in
               DefaultDepth ${toString cfg.defaultDepth}
             ''}
 
-            ${optionalString (driver.name == "nvidia") ''
-              Option "RandRRotation" "on"
-            ''}
-
             ${optionalString
                 (driver.name != "virtualbox" &&
                  (cfg.resolutions != [] ||