Merge #537

537: microsoft/surface profile -- change dir structure to support easier specialisation by model r=Mic92 a=mexisme



Co-authored-by: mexisme <wildjim+dev@kiwinet.org>
Co-authored-by: mexisme <mexisme@users.noreply.github.com>

README.md

@@ -193,8 +193,11 @@ See code for all available configurations.
 | [LENOVO Yoga 6 13ALC6 82ND](lenovo/yoga/6/13ALC6)                   | `<nixos-hardware/lenovo/yoga/6/13ALC6>`            |
 | [MSI GS60 2QE](msi/gs60)                                            | `<nixos-hardware/msi/gs60>`                        |
 | [MSI GL62/CX62](msi/gl62)                                           | `<nixos-hardware/msi/gl62>`                        |
+| [Microsoft Surface Go](microsoft/surface/surface-go)                | `<nixos-hardware/microsoft/surface/surface-go>`    |
+| [Microsoft Surface Pro (Intel)](microsoft/surface/surface-pro-intel)| `<nixos-hardware/microsoft/surface/surface-pro-intel>`|
+| [Microsoft Surface Laptop (AMD)](microsoft/surface/surface-laptop-amd)| `<nixos-hardware/microsoft/surface/surface-laptop-amd>`|
+| [Microsoft Surface Range (Common Modules)](microsoft/surface/common)| `<nixos-hardware/microsoft/surface/common>`        |
 | [Microsoft Surface Pro 3](microsoft/surface-pro/3)                  | `<nixos-hardware/microsoft/surface-pro/3>`         |
-| [Microsoft Surface Range](microsoft/surface)                        | `<nixos-hardware/microsoft/surface>`               |
 | [Omen en00015p](omen/en00015p)                                      | `<nixos-hardware/omen/en00015p>`                   |
 | [One-Netbook OneNetbook 4](onenetbook/4)                            | `<nixos-hardware/onenetbook/4>`                    |
 | [Panasonic Let's Note CF-LX4 ](panasonic/letsnote/cf-lx4)           | `<nixos-hardware/panasonic/letsnote/cf-lx4>`       |

flake.nix

@@ -130,7 +130,10 @@
       lenovo-thinkpad-z13 = import ./lenovo/thinkpad/z/z13;
       lenovo-yoga-6-13ALC6 = import ./lenovo/yoga/6/13ALC6;
       letsnote-cf-lx4 = import ./panasonic/letsnote/cf-lx4;
-      microsoft-surface = import ./microsoft/surface;
+      microsoft-surface-go = import ./microsoft/surface/surface-go;
+      microsoft-surface-pro-intel = import ./microsoft/surface/surface-pro-intel;
+      microsoft-surface-laptop-amd = import ./microsoft/surface/surface-laptop-amd;
+      microsoft-surface-common = import ./microsoft/surface/common;
       microsoft-surface-pro-3 = import ./microsoft/surface-pro/3;
       msi-gs60 = import ./msi/gs60;
       msi-gl62 = import ./msi/gl62;

microsoft/surface-pro/3/default.nix

@@ -1,3 +1,3 @@
 {
-  imports = [ ../../surface ];
+  imports = [ ../../surface/surface-pro-intel ];
 }

microsoft/surface/OLD-BEHAVIOUR-DEPRECATION.md

@@ -0,0 +1,49 @@
+# Changes to the microsoft/surface top-level
+
+## Overview
+
+When the microsoft/surface profile was created, there weren't that many differences between
+the various models of Surface.
+
+e.g. I had just acquired a Surface Go 1, and it was mostly safe to enable all the options for all the
+models, and they would fail gracefully enough that you could mostly ignore warnings or errors.
+
+Now, however --- as-of 2023-01-10 --- we have a much wider variety of chipsets, incl. models with
+some of the newer AMD CPUs, and this is breaking small things in annoying ways for more people.
+
+## How to update
+
+By preference, there will already be a specialised module for your model's configuration.
+
+If not, the `microsoft/surface/common/` module can also be imported directly, and the options
+provided can be used in your own system's configuration.
+
+Alternatively, you can create a new specialisation for your model under `microsoft/surface`
+configured for that model.
+
+## Changes
+
+### Model Specialisations
+
+In keeping with the broader structure of "nixos-hardware", I've also changed the structure of the
+microsoft/surface profile to make it easier for people to specialise for their hardware.
+
+Any code or modules that are specialised for a Surface model now have their own directory under this
+top-level.
+
+### "Common" modules
+
+All the "common" modules that were once in the top-level of the microsoft/surface profile have moved
+under the `common/` directory.
+
+Tools / services that are shared among several models are now extracted to their own module under
+`common/` and imported by `common/default.nix`.
+Most "common" modules now have an `enable` option, which is `false` by default.
+
+## Adding a new Model Specialisation
+
+This hasn't been finalised, partly as I now only have access to a Surface Go 1, these days, so I'm
+maybe not the best custodian of this code any longer.
+
+However, hopefully the (imminent) `surface-go/` module is a reasonable exmample, and we should be
+able to gather more examples for more model specialisations over time.

microsoft/surface/README.md

@@ -1,8 +1,29 @@
-# Derivatives for Microsoft Surface notebooks
+# NOTE: Structure changes from 2023-01-10
+
+Please read the [Deprecated Behaviour README](./OLD-BEHAVIOUR-DEPRECATED.md) to understand how some structural changes to
+the code might affect you!
+
+# Derivations for Microsoft Surface notebooks
 
 These derivatives use the patches from the [linux-surface repo](https://github.com/linux-surface/linux-surface/tree/master/patches).
 
-## Kernel
+## Importing
+
+By preference, there will already be a specialised module for your model's configuration.
+e.g. The `microsoft/surface/surface-go` module configures for the Surface Go.
+
+If not, the `microsoft/surface/common/` module can also be imported directly, and the options
+provided can be used in your own system's configuration.
+
+Alternatively, you can create a new specialisation for your model under `microsoft/surface`
+configured for that model.
+
+## Common Modules
+
+Most shared / common modules are under the [`common/`](./common/) directory.
+This includes the patched kernel build modules, as well as tools and service like `IPTSd` and `surface-control`.
+
+### Kernel
 
 The kernel needs several patches to make it work correctly with some of the hardware on various
 Surface models, e.g. keyboard/trackpad, camera, wifi.
@@ -11,21 +32,19 @@ Not all hardware is fully supported, but the
 [linux-surface feature matrix](https://github.com/linux-surface/linux-surface/wiki/Supported-Devices-and-Features#feature-matrix)
 provides details on which devices are supported on which types of machine.
 
-The kernel-specific derivations are under the `kernel/` sub-directory.
+The kernel-specific derivations are under the [`common/kernel/`](./common/kernel/) sub-directory.
 In order to simplify maintenance of the Nix code, only the most-recent kernel patch-set is expected
 to be maintained in this repo.
 
 _*NOTE:*_ Some built-in Kernel config items need to be set, that aren't set by default:
 - https://github.com/linux-surface/surface-aggregator-module/wiki/Testing-and-Installing
 
-## Firmware, Drivers and Support Tools
-
-### WiFi
-
-For the Surface Go, please see the "Issues" sections below.
+### Support Tools
 
 ### IPTS
 
+Enable this with the `microsoft-surface.ipts.enable = true;` config option.
+
 IPTS is used on most of the Surface range, except for Surface Go and Surface Laptop 3 (AMD version).
 
 Older kernels used specialised firmware which used a method that's no longer supported by the
@@ -44,10 +63,21 @@ kernel-space driver into events for the HID / input sub-system.
 
 #### surface-control
 
+Enable this with the `config.microsoft-surface.surface-control.enable = true;` config option.
+
 For controlling the performance modes and other aspects of the device, the [`surface-control`](https://github.com/linux-surface/surface-control) tool is included.
 
 To be able to control the performance mode without using `sudo`, add your user to the `surface-control` group.
 
+## Firmware and Drivers
+
+### WiFi on Surface Go
+
+For the Surface Go, please see the "Issues" sections below.
+
+Including the [`microsoft/surface/surface-go/firmware/ath-10k/`](./surface-go/firmware/ath-10k/)
+module will replace the default firmware with the updated firmware.
+
 # ToDo's Not Done
 
 See: [TODO.org](./TODO.org)
@@ -68,8 +98,8 @@ You will see messages like "Can't ping firmware".
 The most effective fix to-date is to remove the `board-2.bin` file or replace it with a copy of the
 `board.bin` file.
 
-The derivative in `firmware/surface-go/ath10k/` can configure this, if you set the option
-`config.hardware.microsoft-surface.firmware.surface-go-ath10k.replace` to `true`.
+The derivative in `surface-go/firmware/ath10k/` can configure this, with the
+`config.hardware.microsoft-surface.firmware.surface-go-ath10k.replace = true` config option.
 
 _*NOTE:*_ This is destructive, as it deletes all the `board.bin` and `board-2.bin` files for the
 `ath10k` QCA6174 device, and replaces them with KillerNetworking's version.

microsoft/surface/common/default.nix

@@ -0,0 +1,24 @@
+{ lib, ... }:
+
+let
+  inherit (lib) mkDefault;
+
+in {
+  imports = [
+    ./kernel
+    ./ipts
+    ./surface-control
+  ];
+
+  microsoft-surface.kernelVersion = mkDefault "6.0.17";
+
+  # Seems to be required to properly enable S0ix "Modern Standby":
+  boot.kernelParams = mkDefault [ "mem_sleep_default=deep" ];
+
+  # NOTE: Check the README before enabling TLP:
+  services.tlp.enable = mkDefault false;
+
+  # i.e. needed for wifi firmware, see https://github.com/NixOS/nixos-hardware/issues/364
+  hardware.enableRedistributableFirmware = mkDefault true;
+  hardware.sensor.iio.enable = mkDefault true;
+}

microsoft/surface/common/ipts/default.nix

@@ -0,0 +1,27 @@
+{ config, lib, pkgs, ... }:
+
+let
+  inherit (lib) mkDefault mkEnableOption mkIf mkMerge;
+
+  cfg = config.microsoft-surface.ipts;
+
+in {
+  options.microsoft-surface.ipts = {
+    enable = mkEnableOption "Enable IPTSd for Microsoft Surface";
+  };
+
+  config = mkMerge [
+    {
+      microsoft-surface.ipts.enable = mkDefault false;
+    }
+
+    (mkIf cfg.enable {
+      systemd.services.iptsd = {
+        description = "IPTSD";
+        path = with pkgs; [ iptsd ];
+        script = "iptsd";
+        wantedBy = [ "multi-user.target" ];
+      };
+    })
+  ];
+}

microsoft/surface/common/surface-control/default.nix

@@ -0,0 +1,25 @@
+{ config, lib, pkgs, ... }:
+
+
+let
+  inherit (lib) mkDefault mkEnableOption mkIf mkMerge;
+
+  cfg = config.microsoft-surface.surface-control;
+
+in {
+  options.microsoft-surface.surface-control = {
+    enable = mkEnableOption "Enable 'surface-control' for Microsoft Surface";
+  };
+
+  config = mkMerge [
+    {
+      microsoft-surface.surface-control.enable = mkDefault false;
+    }
+
+    (mkIf cfg.enable {
+      environment.systemPackages = with pkgs; [ surface-control ];
+      services.udev.packages = with pkgs; [ surface-control];
+      users.groups.surface-control = { };
+    })
+  ];
+}

microsoft/surface/default.nix

@@ -1,46 +1,10 @@
-{ lib, pkgs, ... }:
+{ ... }:
 
-let
-  inherit (lib) mkDefault;
-
-in {
-  imports = [
-    ./kernel
-    ./firmware/surface-go/ath10k
+{
+  assertions = [
+    {
+      assertion = false;
+      message = "Importing microsoft/surface/ (default.nix) directly is deprecated! See https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/OLD-BEHAVIOUR-DEPRECATED.md for more details.";
+    }
   ];
-
-  microsoft-surface.kernelVersion = mkDefault "6.0.17";
-
-  boot.extraModprobeConfig = mkDefault ''
-    options i915 enable_fbc=1 enable_rc6=1 modeset=1
-    options snd_hda_intel power_save=1
-    options snd_ac97_codec power_save=1
-    options iwlwifi power_save=Y
-    options iwldvm force_cam=N
-  '';
-
-  boot.kernelParams = [ "mem_sleep_default=deep" ];
-
-  # NOTE: Check the README before enabling TLP:
-  services.tlp.enable = mkDefault false;
-
-  # i.e. needed for wifi firmware, see https://github.com/NixOS/nixos-hardware/issues/364
-  hardware.enableRedistributableFirmware = mkDefault true;
-  hardware.sensor.iio.enable = mkDefault true;
-
-  environment.systemPackages = [
-    pkgs.surface-control
-  ];
-  users.groups.surface-control = { };
-  services.udev.packages = [
-    pkgs.surface-control
-  ];
-
-  systemd.services.iptsd = {
-    description = "IPTSD";
-    script = "${pkgs.iptsd}/bin/iptsd";
-    wantedBy = [
-      "multi-user.target"
-    ];
-  };
 }

microsoft/surface/surface-go/default.nix

@@ -0,0 +1,31 @@
+{ lib, ... }:
+
+# This module is intended to support the Surface Go range of devices.
+# The current version of this targets the Go 1, and other versions of the device may need further
+# config changes to work well.
+
+let
+  inherit (lib) mkDefault;
+in {
+  imports = [
+    ../common
+    ./firmware/ath10k
+    ../../../common/pc
+    ../../../common/pc/ssd
+    # The Intel CPU module auto-includes Intel's GPU:
+    ../../../common/cpu/intel
+    ../../../common/cpu/intel/kaby-lake
+  ];
+
+  boot.kernelParams = [
+    "i915.enable_rc6=1"
+    "i915.modeset=1"
+  ];
+
+  boot.extraModprobeConfig = mkDefault ''
+    options snd_hda_intel power_save=1
+    options snd_ac97_codec power_save=1
+    options iwlwifi power_save=Y
+    options iwldvm force_cam=N
+  '';
+}

microsoft/surface/surface-go/firmware/ath10k/ath10k-replace.nix

@@ -1,7 +1,7 @@
 { stdenv, lib, pkgs, firmwareLinuxNonfree, ... }:
 
 let
-  repos = pkgs.callPackage ../../../repos.nix {};
+  repos = pkgs.callPackage ../../../common/repos.nix {};
   killernetworking_firmware = repos.surface-go-ath10k-firmware_backup + "/K1535_Debian";
 
 in stdenv.mkDerivation {

microsoft/surface/surface-laptop-amd/default.nix

@@ -0,0 +1,22 @@
+{ lib, ... }:
+
+# This module is intended to support the Surface Laptop range, specifically those with AMD CPUs.
+# It's expected it will work equally well on many other Surface models, but they may need further
+# config changes to work well.
+
+let
+  inherit (lib) mkDefault;
+in {
+  imports = [
+    ../common
+    ../../../common/pc
+    ../../../common/pc/ssd
+    ../../../common/cpu/amd
+    ../../../common/cpu/amd/pstate.nix
+    ../../../common/gpu/amd
+  ];
+
+  # Note: The IPTS module is not often required on devices with Surface Laptop 3 (AMD).
+  microsoft-surface.ipts.enable = true;
+  microsoft-surface.surface-control.enable = true;
+}

microsoft/surface/surface-pro-intel/default.nix

@@ -0,0 +1,20 @@
+{ lib, ... }:
+
+# This module is intended to support the Surface Pro range, specifically those with Intel CPUs.
+# It's expected it will work equally well on many other Surface models, but they may need further
+# config changes to work well.
+
+let
+  inherit (lib) mkDefault;
+in {
+  imports = [
+    ../common
+    ../../../common/pc
+    ../../../common/pc/ssd
+    # The Intel CPU module auto-includes Intel's GPU:
+    ../../../common/cpu/intel
+  ];
+
+  microsoft-surface.ipts.enable = true;
+  microsoft-surface.surface-control.enable = true;
+}