Skip to content
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.

Sign up for GitHub

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

Jump to bottom

nixos/hardware/intel: add opencl option #219657

Closed
wants to merge 1 commit into from
Closed

nixos/hardware/intel: add opencl option #219657

wants to merge 1 commit into from

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Mar 5, 2023

Description of changes

Part of NixOS/nixos-hardware#573

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Mar 5, 2023
@Mic92 Mic92 mentioned this pull request Mar 5, 2023
Closed
2 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Mar 5, 2023
@Kiskae
Copy link
Contributor

Kiskae commented Mar 5, 2023 •
edited
Loading

OpenCL on intel is messy, I believe this is the support table as it currently exists:

pkg CPU GPU Notes
intel-compute-runtime no yes since Broadwell 5th gen, open source
intel-ocl yes no1 since Ivy Bridge 3rd gen, closed source, deprecated
beignet no yes since Ivy Bridge 3rd gen, open source, deprecated
intel-opencl-runtime yes2 no newer releases are only available as an SDK, not available in nixpkgs

Footnotes

  1. the source includes an icd for the GPU driver, but it does not implement the opencl-loader spec correctly. ↩

  2. according to the arch wiki, it supports non-intel CPUs as well. ↩

@Mic92
Copy link
Member Author

Mic92 commented Mar 6, 2023

Thanks for the table! Should we make intel-ocl opt-in if unfree packages are allowed and have an option that we can turn off in future?

@Kiskae
Copy link
Contributor

Kiskae commented Mar 6, 2023

That is probably the best approach, perhaps another option to use beignet on older GPUs?

@SuperSandro2000
Copy link
Member

That is probably the best approach, perhaps another option to use beignet on older GPUs?

That then belongs into nixos-hardware propably.

SuperSandro2000
SuperSandro2000 reviewed Mar 26, 2023
View reviewed changes
nixos/modules/hardware/video/intel.nix Outdated
config = lib.mkIf config.hardware.intel.opencl.enable {
hardware.opengl.extraPackages = with pkgs; [
intel-compute-runtime
intel-ocl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having that under video is not fully correct, as it also has parts that only use the CPU, it is more general intel hardware.

SuperSandro2000
SuperSandro2000 reviewed Mar 26, 2023
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also update https://github.com/NixOS/nixpkgs/blob/6f558d3009c20cd2decc5b36057f78a033a6ff11/nixos/doc/manual/configuration/gpu-accel.chapter.md#intel-sec-gpu-accel-opencl-intel and possible

nixpkgs/nixos/modules/hardware/opengl.nix

Line 119 in 6f558d3

example = literalExpression "with pkgs; [ intel-media-driver intel-ocl vaapiIntel ]";

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
JohnRTitor
JohnRTitor reviewed Jun 15, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mic92 could you rebase this and address the issues raised?

JohnRTitor
JohnRTitor previously requested changes Jun 15, 2024
View reviewed changes
nixos/modules/hardware/cpu/intel-opencl.nix Outdated Show resolved Hide resolved
Mic92
Mic92 commented Jun 15, 2024
View reviewed changes
nixos/modules/hardware/cpu/intel-opencl.nix
);
default = null;
description = ''
Select the Intel OpenCL runtime to use. Choose your runtime based on your Intel CPU/GPU generation.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider this option documentation, also it's the same amount of code to add stuff to extraPackages, this one makes it easier to discover what needs to be done to get opencl running on a given Intel CPU.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 15, 2024
@Mic92
Copy link
Member Author

Mic92 commented Jun 19, 2024

we should also update https://github.com/NixOS/nixpkgs/blob/6f558d3009c20cd2decc5b36057f78a033a6ff11/nixos/doc/manual/configuration/gpu-accel.chapter.md#intel-sec-gpu-accel-opencl-intel and possible

nixpkgs/nixos/modules/hardware/opengl.nix

Line 119 in 6f558d3

example = literalExpression "with pkgs; [ intel-media-driver intel-ocl vaapiIntel ]";

Done. Anything else?

@JohnRTitor JohnRTitor dismissed their stale review June 20, 2024 15:18

changes addressed

JohnRTitor
JohnRTitor approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but this probably is too minimal to have its own module

I agree with NixOS/nixos-hardware#992 (comment) and I feel like small modules like this can be merged into a big and general intel module.

@Mic92
Copy link
Member Author

Mic92 commented Jun 27, 2024 •
edited
Loading

LGTM, but this probably is too minimal to have its own module

I agree with NixOS/nixos-hardware#992 (comment) and I feel like small modules like this can be merged into a big and general intel module.

If we would make a cut and ignore anything older than broadwell, this could probably just be part of an igpu module instead? I am not sure how relevant opencl was in previous generations...
Because ideally we could separate somehow CPU/iGPU because there are enough Intel server cpus out there that do not have a integrated GPU.

@Lyndeno
Copy link
Contributor

Lyndeno commented Jul 3, 2024 •
edited
Loading

Perhaps this module could be moved as-is to gpu? Like nixos/modules/services/hardware/amdgpu.nix

Edit: Oops, amdgpu is not under gpu directory, but maybe it should be?

@Mic92
Copy link
Member Author

Mic92 commented Jul 3, 2024

Perhaps this module could be moved as-is to gpu? Like nixos/modules/services/hardware/amdgpu.nix

Edit: Oops, amdgpu is not under gpu directory, but maybe it should be?

Sounds good. If someone wants to take over, feel free. I might get back to this later otherwise.

@Mic92 Mic92 closed this Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@JohnRTitor JohnRTitor JohnRTitor approved these changes

@Lyndeno Lyndeno Lyndeno approved these changes

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Mic92 @Kiskae @SuperSandro2000 @Lyndeno @JohnRTitor @wegank