-
-
Notifications
You must be signed in to change notification settings - Fork 16.6k
intel-npu-driver: init at 1.22.0 #382756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
intel-npu-driver: init at 1.22.0 #382756
Conversation
I created a flake for it, and verified the firmware worked well. |
($device eq "0x7d1d" || $device eq "0xad1d" || | ||
$device eq "0x643e" || $device eq "0xb03e"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PCI ID are copied from the intel_vpu linux driver module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have trivial change requests, package seems sufficiently up to standard otherwise.
The NPU firmware is strictly connected with NPU compiler and driver. This is a problem, because whenever you want to use newer driver, ex. upcoming v1.14.0 release, then you should use the same firmware from the release as well. If the firmware is not matched with compiler, then we do not guarantee that all works correctly. Best would be to have a common package - firmware, driver and compiler. That is why the NPU firmware never reached the linux-firmware repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I question this entire module. It just does to little to be added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just trying to get rid of the firmware loading error at the first place, while according to @jwludzik, other packages are also required to get NPU really work, I think it's good to keep the module to install all related packages together.
@@ -196,6 +196,11 @@ sub pciCheck { | |||
($device eq "0x4229" || $device eq "0x4230" || | |||
$device eq "0x4222" || $device eq "0x4227"); | |||
|
|||
push @attrs, "hardware.cpu.intel.enableNpuFirmware = true;" if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend
push @attrs, "hardware.cpu.intel.enableNpuFirmware = true;" if | |
push @attrs, "hardware.firmware = [ pkgs.intel-npu-firmware ];" if |
Thank you guys for reviewing this, I am changing this to WIP for packaging related driver and compiler as well. |
4d7bb9c
to
2d2b4c4
Compare
74fcd25
to
c232a62
Compare
c232a62
to
0ba746a
Compare
Hi @pseudocc, I wanted to share an experiment I conducted to streamline the package compilation process as suggested by @SuperSandro2000. Here's what I did:
I hope this approach might be helpful as we work towards refining the packaging process. |
Hi @nilp0inter , I think that there might be a use case that the user just want the firmware only. If so, my implementation will not take any time preparing cmake env and building the artifacts since the firmware binary is in the source tree. |
Thanks for the clarification @pseudocc . I was missing the point completely. Cheers! |
Latest version of |
inherit (intel-npu-driver) version src; | ||
pname = "intel-npu-firmware"; | ||
|
||
installPhase = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we then set dontBuild to make that obvious?
I am not sure if that optimisation is worth it. Having two packages to update is some overhead and r-ryantm cannot deal with that properly, also when doing inherits.
Why not comine them again and move the firmware to the respecting output?
owner = "intel"; | ||
repo = "linux-npu-driver"; | ||
rev = "v${version}"; | ||
fetchSubmodules = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vendoring libraries we already have in nixpkgs is not nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However it is still necessary, please see: intel/level-zero-npu-extensions#36 intel-npu-driver
has level-zero-npu-extensions
vendored as git submodule from github directly. Please approve this PR @SuperSandro2000
udev | ||
openssl | ||
boost | ||
level-zero |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make sure we are using our level-zero, we should delete the vendored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it in the postPatch section
rm third_party/cmake/level-zero.cmake
0ba746a
to
2893356
Compare
Ran
|
@SuperSandro2000 Nudge, any suggested changes? |
Only support the standalone build. Closes: NixOS#348739
2893356
to
1943dbf
Compare
For the firmware part looks like it's now upstreamed to linux-firmware https://web.git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/?id=9b870dde196d030a6c8872e8965dd28a0146c990 |
@SuperSandro2000 @osbm @nmouha seems like this PR is more than 6 months old already and requested changes are made. Could you approve your reviews and lets merge this? I've been custom configuring this to my config to utilize my NPU for far too long. Thanks! @pseudocc |
Could you tell me how you did the custom config? I've been waiting for this PR for about six months now and would really like to use the NPU. Thanks 🙏 |
@NeuerUser You may refer to mine (if you are also using flake): pseudocc/nix-config@6434c09 |
Thanks a lot. I do, however, get an SHA error: `
` |
Just as a datapoint, on 25.05 I managed to run validation without applying the module $ LD_LIBRARY_PATH="$(nix-build -A intel-npu-driver.out)/lib" "$(nix-build -A intel-npu-driver.validation)/bin/npu-umd-test"
...
[ RUN ] Umd.ConfigurationCheck
/build/source/validation/umd-test/umd_test.cpp:201: Failure
Expected: (config.size()) > (0), actual: 0 vs 0
Missed configuration file.
Test scope is reduced.
Provide valid configurtion file, use:
npu-umd-test -c/--config [CONFIGURATION_PATH]
[ FAILED ] Umd.ConfigurationCheck (0 ms)
...
[ DISABLED ] MemoryExecution.DISABLED_CopyingFromUnpinnedHostMemoryShouldBeAllowed
... all other tests are either passing or skipped And for kmd $ LD_LIBRARY_PATH="$(nix-build -A intel-npu-driver.out)/lib" sudo "$(nix-build -A intel-npu-driver.validation)/bin/npu-kmd-test"
...
[ RUN ] Device.ResetComputeEngine
/build/source/validation/kmd-test/test_device.cpp:118: Failure
Expected equality of these values:
write_debugfs_file("reset_engine", 0)
Which is: 19
0
[ FAILED ] Device.ResetComputeEngine (3 ms)
[ DISABLED ] Device.DISABLED_ResetInvalidEngine
...
[ DISABLED ] UapiMem.DISABLED_BoClosedDuringCreate
... and all other passing |
Packaging the Intel NPU firmware (for Linux kernel module: intel_vpu).
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.