Skip to content

Add Lenovo ThinkPad P14s Intel Gen 2 #1509

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ See code for all available configurations.
| [Lenovo ThinkPad P14s AMD Gen 3](lenovo/thinkpad/p14s/amd/gen3) | `<nixos-hardware/lenovo/thinkpad/p14s/amd/gen3>` | `lenovo-thinkpad-p14s-amd-gen3` |
| [Lenovo ThinkPad P14s AMD Gen 4](lenovo/thinkpad/p14s/amd/gen4) | `<nixos-hardware/lenovo/thinkpad/p14s/amd/gen4>` | `lenovo-thinkpad-p14s-amd-gen4` |
| [Lenovo ThinkPad P14s AMD Gen 5](lenovo/thinkpad/p14s/amd/gen5) | `<nixos-hardware/lenovo/thinkpad/p14s/amd/gen5>` | `lenovo-thinkpad-p14s-amd-gen5` |
| [Lenovo ThinkPad P14s Intel Gen 2](lenovo/thinkpad/p14s/intel/gen2) | `<nixos-hardware/lenovo/thinkpad/p14s/intel/gen2>` | `lenovo-thinkpad-p14s-intel-gen2`|
| [Lenovo ThinkPad P14s Intel Gen 3](lenovo/thinkpad/p14s/intel/gen3) | `<nixos-hardware/lenovo/thinkpad/p14s/intel/gen3>` | `lenovo-thinkpad-p14s-intel-gen3`|
| [Lenovo ThinkPad P14s Intel Gen 5](lenovo/thinkpad/p14s/intel/gen5) | `<nixos-hardware/lenovo/thinkpad/p14s/intel/gen5>` | `lenovo-thinkpad-p14s-intel-gen5`|
| [Lenovo ThinkPad P16s AMD Gen 1](lenovo/thinkpad/p16s/amd/gen1) | `<nixos-hardware/lenovo/thinkpad/p16s/amd/gen1>` | `lenovo-thinkpad-p16s-amd-gen1` |
Expand Down
22 changes: 1 addition & 21 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@
lenovo-thinkpad-p14s-amd-gen3 = import ./lenovo/thinkpad/p14s/amd/gen3;
lenovo-thinkpad-p14s-amd-gen4 = import ./lenovo/thinkpad/p14s/amd/gen4;
lenovo-thinkpad-p14s-amd-gen5 = import ./lenovo/thinkpad/p14s/amd/gen5;
lenovo-thinkpad-p14s-intel-gen2 = import ./lenovo/thinkpad/p14s/intel/gen2;
lenovo-thinkpad-p14s-intel-gen3 = import ./lenovo/thinkpad/p14s/intel/gen3;
lenovo-thinkpad-p14s-intel-gen5 = import ./lenovo/thinkpad/p14s/intel/gen5;
lenovo-thinkpad-p16s-amd-gen1 = import ./lenovo/thinkpad/p16s/amd/gen1;
Expand Down
7 changes: 7 additions & 0 deletions lenovo/thinkpad/p14s/intel/gen2/README
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Lenovo ThinkPad P14s Intel Gen 2

Much of the extraneous settings are filled out using the [wiki](https://wiki.nixos.org/wiki/NVIDIA#Hybrid_graphics_with_PRIME), which gives guidance on how to determine the correct PCI bus IDs to assign, aswell as the note to include the `modesetting` driver in `services.xserver.videoDrivers`.

## BIOS Settings to adjust

For suspending to RAM to work, set Config -> Power -> Sleep State to "Linux S3" in EFI.
18 changes: 18 additions & 0 deletions lenovo/thinkpad/p14s/intel/gen2/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
imports = [
../../../../../common/gpu/nvidia/prime.nix
../../../../../common/gpu/nvidia/turing
../../../../../common/cpu/intel/tiger-lake
Copy link
Member

Choose a reason for hiding this comment

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

the cpu profile also includes gpu modules. I had it in my memories that the "modesettings" driver was included by default in xserver? That's why I dropped it for now. However please correct me on that in which case I would add this rather to the intel gpu profile than here.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, so I have done testing on this previous, and just double checked the effect of including or excluding services.xserver.videoDrivers = [ "modesetting" ]; with your other changes. We can indeed exclude the redundant tiger-lake gpu module.

Originally, I found that excluding it caused the computer to hang when changing into suspend mode. After lots of research, I understand that this has to do with the hand-off failing on systems with a discrete GPU, such as thing one. This is also noted in the wiki, here.

A quick rg \"modesetting\' call in the repo yeilds:

raspberry-pi/4/modesetting.nix
109:      "modesetting" # Prefer the modesetting driver in X11

raspberry-pi/5/default.nix
24:    Driver "modesetting"

common/gpu/amd/default.nix
6:    services.xserver.videoDrivers = lib.mkDefault [ "modesetting" ];

lenovo/thinkpad/p14s/intel/gen2/default.nix
19:  services.xserver.videoDrivers = [ "modesetting" ];

So, it turns out that this isn't actually set. Ostensibly, this 'should be set at needed, implicitly', but reading the wiki, it seems that the 'modesetting' functionality requires different drivers depending on what CPU you have, either AMD or Intel. This means that the common/gpu/nvidia/prime.nix module which defines PRIME Offload needs to detect which CPU type you have, and include the correct driver accordingly. Or, a different solution that is CPU aware which I am not well-versed enough to design and implement...

I believe, that although services.xserver.videoDrivers does include "modesetting", as per this options search, the common/gpu/nvidia/default.nix module overrides this with services.xserver.videoDrivers = lib.mkDefault [ "nvidia" ];, so that the default now excludes "modesetting" and "fbdev".

Copy link
Author

Choose a reason for hiding this comment

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

As a follow-up, I'm going to see what happens if I change services.xserver.videoDriver = mkDefault [ "nvidia" ]; to services.xserver.videoDriver = [ "nvidia" ]; and see if Nix will just add the "nvidia" drivers, so I don't need to 'add back' the "modesetting" drivers.

Copy link
Author

Choose a reason for hiding this comment

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

That didn't seem to work. Suspend still breaks.

I am adding back in the "modesetting" driver. I'll add the link to the Wiki in the upcoming README for clarification.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Probably a lot of our hybrid configurations are wrong than. I never had a device dual intel/nvidia so far.

../../../../../common/pc/laptop
../../../../../common/pc/ssd
../../../../../common/pc
../.
];

hardware.nvidia.prime = {
intelBusId = "PCI:0:2:0";
nvidiaBusId = "PCI:1:0:0";
};

services.xserver.videoDrivers = [ "modesetting" ];
}