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

dell-precision-5560: init #1122

Merged
merged 4 commits into from
Sep 28, 2024
Merged

dell-precision-5560: init #1122

merged 4 commits into from
Sep 28, 2024

Conversation

matdibu
Copy link
Contributor

@matdibu matdibu commented Sep 12, 2024

Description of changes

Added config for Dell Precision 5560

Things done

@matdibu
Copy link
Contributor Author

matdibu commented Sep 23, 2024

I've also added a README

hardware.enableRedistributableFirmware = lib.mkDefault true;

boot = {
kernelParams = [ "i915.modeset=1" ];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Mic92 what about this? I think it should at least be wrapped in a
lib.mkIf (config.hardware.intelgpu.driver == "i915")
but at that point, maybe it should be in common ?

Copy link
Member

Choose a reason for hiding this comment

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

You are probably right.

Suggested change
kernelParams = [ "i915.modeset=1" ];
kernelParams = lib.mkIf (config.hardware.intelgpu.driver == "i915") [ "i915.modeset=1" ];

Also is this laptop not new enough to use xe anyway? Also isn't modset=1 the default since along time also?

lsmod | grep 'xe'
xe                   2875392  0
drm_gpuvm              45056  1 xe
drm_exec               12288  2 drm_gpuvm,xe
gpu_sched              65536  1 xe
drm_suballoc_helper    12288  1 xe
drm_ttm_helper         12288  1 xe
drm_buddy              20480  2 xe,i915
ttm                   102400  3 drm_ttm_helper,xe,i915
drm_display_helper    258048  2 xe,i915
cec                    73728  3 drm_display_helper,xe,i915
i2c_algo_bit           20480  2 xe,i915
video                  81920  2 xe,i915
backlight              28672  3 video,xe,i915

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is, I left a note in the readme about using xe instead of i915
I simply don't know if we should default to that or not, since it's pretty new and might be less stable than i915

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, I'm not sure about i915.modeset, I can see this locally:

$ modinfo i915 | grep modeset:
parm:           modeset:Use kernel modesetting [KMS] (0=disable, 1=on, -1=force vga console preference [default]) (int)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are you on a precision5560 right now? I had to use force_probe to swap to xe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair enough, and thanks for reviewing btw!
but now I'm not sure the conditional in common/gpu/intel/tiger-lake/default.nix makes sense
could there be other tigerlake igpu device IDs that are whitelisted for xe? if not, maybe we should remove xe from that config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked through the kernel sources for xe, found this struct:

static const struct xe_device_desc tgl_desc = {
        .graphics = &graphics_xelp,
        .media = &media_xem,
        PLATFORM(TIGERLAKE),
        .has_display = true,
        .has_llc = true,
        .require_force_probe = true,
};

so tigerlake has require_force_probe = true

Copy link
Member

Choose a reason for hiding this comment

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

Seems like it, only LUNARLAKE and BATTLEMAGE will actually use the driver without force probe:

static const struct xe_device_desc lnl_desc = {
	PLATFORM(LUNARLAKE),
	.has_display = true,
};

static const struct xe_device_desc bmg_desc = {
	DGFX_FEATURES,
	PLATFORM(BATTLEMAGE),
	.has_display = true,
	.has_heci_cscfi = 1,
};

So we should probably adapt it for nixos-hardware as well.

Copy link
Member

Choose a reason for hiding this comment

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

So this was introduced by @fidgetingbits in #944

Copy link
Member

Choose a reason for hiding this comment

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

@Mic92 Mic92 merged commit fb08bde into NixOS:master Sep 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants