nixos-hardware icon indicating copy to clipboard operation
nixos-hardware copied to clipboard
verified publisher icon NixOS

Let's upstream all profiles in `common/` to nixos

Open Mic92 opened this issue 1 year ago • 11 comments

Having generic hardware modules profiles available allows us to include them automatically in tools like nixos-generate-config and co.

Hence I am proposing to upstream all our modules in common and remove them from this repository:

Open questions:

  • Do we upstream them as module options or as importable profiles?

Mic92 avatar Jun 20 '24 09:06 Mic92

I want to throw the following snippet from nixos-modules into the ring, too.

    hardware.opengl = {
      extraPackages = with pkgs; [
        intel-compute-runtime # OpenCL library for iGPU
        # video encoding/decoding hardware acceleration
        intel-media-driver # broadwell or newer
        intel-vaapi-driver # older hardware like haswell
      ];
      extraPackages32 = with pkgs.pkgsi686Linux; [
        # video encoding/decoding hardware acceleration
        intel-media-driver # broadwell or newer
        intel-vaapi-driver # older hardware like haswell
      ];
    };

I think we should not upstream them as is, otherwise we have 20+ new modules which only contain minimal things. We should probably combine them into a general intel module where we have some options for CPU/GPU/etc.

SuperSandro2000 avatar Jun 20 '24 09:06 SuperSandro2000

We should be soon able to drop the amdgpu module from nixos-hardware after https://nixpk.gs/pr-tracker.html?pr=319865 landed.

Mic92 avatar Jun 20 '24 14:06 Mic92

I like the idea of configurable modules. Then we can have generic cpu/gpu modules where we can specify say, "kaby lake" or "rdna2", etc.

Lyndeno avatar Jun 24 '24 23:06 Lyndeno

Something along the idea of:

hardware.graphics.family = "rdna2";
hardware.processor.family = "zen3";
hardware.graphics.opencl.enable = true;
hardware.processor.enableMicrocode = true;

Then have the modules enable the proper settings: opencl packages, vaapi, thermald, patches, microcode (if unsupported in tree, perhaps for apple m series?) all configured for the given hardware families.

Since it is possible to have multiple GPUs, perhaps the graphics.family setting could take a list of families, and ensure setup for each.

Lyndeno avatar Jun 25 '24 00:06 Lyndeno

We should have at least a vendor for both gpu and cpus, to avoid collision names between family names.

Mic92 avatar Jun 25 '24 07:06 Mic92

We want to provide the needed information with this project: https://github.com/numtide/nixos-facter

Mic92 avatar Jun 25 '24 07:06 Mic92

What's the migration path if I'm using some of the deprecated/removed modules?

Neither the warning or the issue description describe what I should do as an end-user :wink:

The PR talks about upstreaming stuff, but I can't see any obvious recent changes in nixpkgs/nixos/hardware's history :thinking:

For my case specifically:

I was using the common-cpu-intel-kaby-lake module, which was renamed to common-gpu-intel-kaby-lake without any deprecation warnings or aliases. I just got an "attr doesn't exist" error after updating my lockfile, so I had to go hunting for recent changes.

The new name has a deprecation warning, but I can't work out what the alternative is supposed to be?

MattSturgeon avatar Jul 18 '24 21:07 MattSturgeon

There is no alternative currently.

I plan to upstream some of this soon. Perhaps we should not have a deprecation warning until then.

Lyndeno avatar Jul 18 '24 22:07 Lyndeno

Before you upstream things, a large corrective+feature pass would be nice to do, as I assume afterwards it might be harder to get things in there.

https://github.com/NixOS/nixos-hardware/issues/1205 https://github.com/NixOS/nixos-hardware/issues/1243 https://github.com/NixOS/nixos-hardware/issues/1244 https://github.com/NixOS/nixos-hardware/issues/1245 https://github.com/NixOS/nixos-hardware/issues/1246

In general the Intel GPU side needs a lot of work. There is for example a comet-lake.nix, but no such Intel iGPU generation exists. 10th gen (Comet Lake) CPUs use the Ice Lake GPU generation. There's a lot of places where psr, fbc and guc are manually set to 1 or 2 when they already already set to -1 ( = turn on, unless problematic) by default by the kernel for those devices.

It'd also just be nice to add all the CPU and GPU gens, even if it'll be mostly copy-paste/boilerplate. Perhaps also nice to either add the generation name in the filename or file itself. For example for Intel CPUs, doing skylake-7.nix and coffeelake-8.nix. Most people don't know their CPU generation's name, but they know 11400 = 11th gen.

I'm fully willing to do the labor for that once I have the time, but I thought I should bring it up before an upstream happens.

ahydronous avatar Nov 22 '24 03:11 ahydronous

Why not bring it up, while you upstream a module? The modules that we bring upstream do not need to be literally the same but can be improved version. They just need to cover the same area so. But you can also do pull requests in nixos-hardware if you want. Sorry, I don't have time to go and address your issues, but I would merge your pull requests.

Mic92 avatar Nov 22 '24 16:11 Mic92

It would be really nice if the warnings were only added once there is actually a migration path.

And additionally, there should be a grace period after the warning is added where deprecated modules continue to work, e.g. as aliases.

The other issue is that currently warnings are only shown when using the flake outputs, not when importing modules directly.

I.e.

  1. module gets upstreamed
  2. a deprecation warning is added
  3. in-tree module remains, or is aliased to the nixos module if possible
  4. after a release cycle, in-tre module/alias is removed

An alias module could look like:

let
  loc =  "/path/to/upstream/module";
  warning = "This module is deprecated and will be removed after 25.11, use the upstream nixos module at `${loc}' instead.";
in
builtins.warn warning (
  { modulesPath, ... }:
  {
    imports = [ (modulesPath + loc) ];
  }
)

By including the warning withing the module itself, you will warn whenever the file is used, even via "${inputs.hardware}/common/cpu/intel/kaby-lake", for instance.

However I still feel strongly that warnings should not be added until there is actually a way for the user to resolve the warning.

MattSturgeon avatar May 14 '25 22:05 MattSturgeon