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.

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

intel-lpmd: init at 0.0.8 #349198

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

Frontear
Copy link
Member

@Frontear Frontear commented Oct 17, 2024

Introduces https://github.com/intel/intel-lpmd as a package with an accompanying NixOS module.

Closes #349187.
cc @Anifyuli for testing.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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.

Add a 👍 reaction to pull requests you find important.

@Frontear
Copy link
Member Author

Written a rudimentary NixOS module that, for the most part, covers everything that I could see documented in the man pages and on the GitHub repo itself.

There are a lot of things that aren't explicitly documented, and a lot of technical jargon that I presume pertains uniquely to an Intel CPU. Documenting this was outside of my scope and arguably I do not believe it should be documented here anyways, but rather something that the user must figure out themselves.

I will write a NixOS test for this module as well. That's to come later.

cc @Anifyuli for testing of the module, including some of the more elusive options. An exhaustive test of everything would be the most preferrable, but a minimal test is still fine.

@Anifyuli
Copy link

For testing purposes, I just installed it on my machine? Or anything else? I haven't experienced testing modules or packages in NixOS previously

@Frontear
Copy link
Member Author

For testing purposes, I just installed it on my machine? Or anything else? I haven't experienced testing modules or packages in NixOS previously

Main thing to look for is trying to get the systemd service running and configuring the daemon using the module settings in services.intel-lpmd.settings. They currently are 1-to-1 with the xml attributes on the github repo, so you can easily configure them from there.

Comment on lines +69 to +73

platforms = platforms.linux;
license = licenses.gpl2Only;
maintainers = with maintainers; [ frontear ];

Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be an empty lines here.


meta = with lib; {
homepage = "https://github.com/intel/intel-lpmd";
description = "Linux daemon used to optimize active idle power.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "Linux daemon used to optimize active idle power.";
description = "Linux daemon used to optimize active idle power";

nativeBuildInputs = [
autoreconfHook
pkg-config

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
];
buildInputs = [

Comment on lines +5 to +15

autoreconfHook,
pkg-config,

glib,
gtk-doc,
libnl,
libxml2,
systemd,
upower,

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
autoreconfHook,
pkg-config,
glib,
gtk-doc,
libnl,
libxml2,
systemd,
upower,
autoreconfHook,
pkg-config,
glib,
gtk-doc,
libnl,
libxml2,
systemd,
upower,

};
in
{
###### interface
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
###### interface

those comments are legacy copy&paste and don't serve any purpose

};

IgnoreITMT = lib.mkOption {
default = false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
default = false;
default = false;
type = lib.types.bool;

Comment on lines +481 to +483
If enabled, ITMT is disabled upon entering LPM,
and re-enabled upon exiting LPM. Otherwise, the
ITMT setting is ignored entirely.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If enabled, ITMT is disabled upon entering LPM,
and re-enabled upon exiting LPM. Otherwise, the
ITMT setting is ignored entirely.
If enabled, ITMT is disabled upon entering LPM and re-enabled upon exiting LPM.
Otherwise the ITMT setting is ignored entirely.

Comment on lines +463 to +473
lp_mode_epp = lib.mkOption {
default = null;
description = ''
EPP (energy performance preferences) to use in LPM.
Leave as `null` to ignore this setting.

Accepts a value from **0** (favor performance)
to **255** (favor power).
'';

type = with lib.types; nullOr numbers.between 0 255;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lp_mode_epp = lib.mkOption {
default = null;
description = ''
EPP (energy performance preferences) to use in LPM.
Leave as `null` to ignore this setting.
Accepts a value from **0** (favor performance)
to **255** (favor power).
'';
type = with lib.types; nullOr numbers.between 0 255;
lp_mode_epp = lib.mkOption {
type = with lib.types; nullOr numbers.between 0 255;
default = null;
description = ''
EPP (energy performance preferences) to use in LPM.
Leave as `null` to ignore this setting.
Accepts a value from **0** (favor performance)
to **255** (favor power).
'';

We should put the 0 to 255 range into the type.

Comment on lines +216 to +253
statesSubmodule = {
options = {
CPUFamily = lib.mkOption {
description = ''
The CPU generation to match.
'';

type = with lib.types; str;
};

CPUModel = lib.mkOption {
description = ''
The CPU model to match.
'';

type = with lib.types; str;
};

CPUConfig = lib.mkOption {
description = ''
Define a configuration of CPUs and TDP to match different skews
for the same CPU model and family.

See `man 5 intel_lpmd_config.xml` for more details.
'';

type = with lib.types; str;
};

State = lib.mkOption {
default = [ ];
description = ''
List of "state" definitions.
'';

type = with lib.types; listOf (submodule perStateSubmodule);
};
};
Copy link
Member

Choose a reason for hiding this comment

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

We should add an example to those.

Comment on lines +353 to +360
HfiLpmEnable = {
default = false;
description = ''
Specifies if the HFI monitor can capture HFI hints for LPM.
'';

type = with lib.types; bool;
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
HfiLpmEnable = {
default = false;
description = ''
Specifies if the HFI monitor can capture HFI hints for LPM.
'';
type = with lib.types; bool;
};
HfiLpmEnable = {
type = lib.types.bool;
default = false;
description = ''
Specifies if the HFI monitor can capture HFI hints for LPM.
'';
};

we shouldn't use with lib here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: intel-lpmd
3 participants