search

NixOS / nixos-hardware

A collection of NixOS modules covering hardware quirks.
Creative Commons Zero v1.0 Universal
1.94k stars 596 forks source link

Surface book 2 camera not working #523

Open damianoognissanti opened 1 year ago

damianoognissanti commented 1 year ago

I believe this issue is NixOS-specific since I got it working on Arch with the same laptop. I am not sure if this is an issue with the kernel of libcamera though, so please tell me if this is the wrong place to ask.

I have trouble with getting any of the cameras working on my Surface book 2 using NixOS.

I have tried the kernel version 6.0.11 with libcamera 0.0.1, 0.0.2 and 0.0.3 without success. I have also tried kernel version 5.19.17 with libcamera 0.0.3, but I get nothing.

Running LIBCAMERA_LOG_LEVELS=*:0 cam --list I get

[0:37:24.968980164] [15709]  INFO IPAManager ipa_manager.cpp:143 libcamera is not installed. Adding '/nix/store/src/ipa' to the IPA search path
[0:37:24.969141964] [15709] DEBUG IPAModule ipa_module.cpp:329 ipa_ipu3.so: IPA module /nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_ipu3.so is signed
[0:37:24.969185396] [15709] DEBUG IPAManager ipa_manager.cpp:245 Loaded IPA module '/nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_ipu3.so'
[0:37:24.969250552] [15709] DEBUG IPAModule ipa_module.cpp:329 ipa_rkisp1.so: IPA module /nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_rkisp1.so is signed
[0:37:24.969282293] [15709] DEBUG IPAManager ipa_manager.cpp:245 Loaded IPA module '/nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_rkisp1.so'
[0:37:24.969378327] [15709] DEBUG IPAModule ipa_module.cpp:329 ipa_rpi.so: IPA module /nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_rpi.so is signed
[0:37:24.969416616] [15709] DEBUG IPAManager ipa_manager.cpp:245 Loaded IPA module '/nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_rpi.so'
[0:37:24.969467644] [15709] DEBUG IPAModule ipa_module.cpp:329 ipa_vimc.so: IPA module /nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_vimc.so is signed
[0:37:24.969501217] [15709] DEBUG IPAManager ipa_manager.cpp:245 Loaded IPA module '/nix/store/rrvl5bznjcaic4w9qf8y2vmaqyk658nb-libcamera-0.0.3/lib/libcamera/ipa_vimc.so'
[0:37:24.969534511] [15709]  INFO Camera camera_manager.cpp:299 libcamera v0.0.3
[0:37:24.969682895] [15712] DEBUG Camera camera_manager.cpp:108 Starting camera manager
[0:37:24.979284447] [15712] DEBUG DeviceEnumerator device_enumerator.cpp:224 New media device "ipu3-cio2" created from /dev/media0
[0:37:24.979431423] [15712] DEBUG DeviceEnumerator device_enumerator_udev.cpp:95 Defer media device /dev/media0 due to 8 missing dependencies
[0:37:24.988479799] [15712] DEBUG DeviceEnumerator device_enumerator_udev.cpp:320 All dependencies for media device /dev/media0 found
[0:37:24.988569712] [15712] DEBUG DeviceEnumerator device_enumerator.cpp:252 Added device /dev/media0: ipu3-cio2
[0:37:24.989124628] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerISI'
[0:37:24.989213146] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerIPU3'
[0:37:24.989343892] [15712] DEBUG DeviceEnumerator device_enumerator.cpp:312 Successful match for media device "ipu3-cio2"
[0:37:24.989492038] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerRPi'
[0:37:24.989582178] [15712] DEBUG RPI raspberrypi.cpp:1166 Unable to acquire a Unicam instance
[0:37:24.989619051] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerRkISP1'
[0:37:24.989673154] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'SimplePipelineHandler'
[0:37:24.989717495] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerUVC'
[0:37:24.989757843] [15712] DEBUG Camera camera_manager.cpp:151 Found registered pipeline handler 'PipelineHandlerVimc'
Available cameras:

If I run dmesg | grep ov8865 I get these lines repeated over and over and over:

[    3.878794] ov8865 i2c-INT347A:00: supply dvdd not found, using dummy regulator
[    3.878846] ov8865 i2c-INT347A:00: supply dovdd not found, using dummy regulator
[    3.878871] ov8865 i2c-INT347A:00: supply avdd not found, using dummy regulator

Running v4l2-ctl --list-devices I get

Intel IPU3 CIO2 (PCI:0000:00:14.3):
    /dev/video0
    /dev/video1
    /dev/video2
    /dev/video3
    /dev/media0

But ffplay /dev/video0 says:

[video4linux2,v4l2 @ 0x7f91fc000c80] Not a video capture device.
/dev/video0: No such device

And the output is the same for the others too.

Running guvcview it says no device found and You seem to have video devices installed. Do you want to try one? but in the drop down there are four entries which all read Intel IPU3 CIO2 and none of them work.

mexisme commented 1 year ago

Hi @damianoognissanti, I don't have access to a Surface Book, so might need some help. For me, on a Surface Go 1, better driver support took a while ;-) .

Some early thoughts:

damianoognissanti commented 1 year ago

Hi @damianoognissanti, I don't have access to a Surface Book, so might need some help. For me, on a Surface Go 1, better driver support took a while ;-) .

Some early thoughts:

  • Do you know how your Arch config differs from your NixOS one?

    • e.g. whether Arch has some extra config or scripts to make devices like this work?

  • Do you know if the kernel driver is loading at all?

I am not sure, but I have since also tried to install Void Linux too to see if it worked there. There I had to compile the kernel from source and it worked (I tested both for version 6.0.15 as well as 6.1.3). The instructions I followed are here: https://github.com/mournfully/linux-surface-for-void-linux The config was farily standard but had the lines from the linux-surface config appended at the bottom.

damianoognissanti commented 1 year ago

Looking at module-ids-and-sensor-mappings the modules seem to be different even though the cameras use the same sensors.

mexisme commented 1 year ago

Looking at module-ids-and-sensor-mappings the modules seem to be different even though the cameras use the same sensors.

537 just merged, which should make it a lot easier to create a specialisation for the Surface Book.

Since I don't have access to a Surface Book, perhaps you (@damianoognissanti) would be interested in trying to create something like microsoft/surface/surface-book-intel/default.nix under microsoft/surface/? This could be based on microsoft/surface/surface-pro-intel/.

And then we can work on trying to troubleshoot that config. for your needs?

damianoognissanti commented 1 year ago

I added microsoft/surface/surface-pro-intel/ to my config and it compiled the kernel (again), but for some reason it still doesn't work. I truly don't understand why applying the patches manually works (in void) but this doesn't. As far as I can the config uses this uses the config from here (right?): https://github.com/linux-surface/linux-surface/tree/master/configs

Is there something else in NixOS' structure that makes it not work?

damianoognissanti commented 1 year ago

I have tried to create a working config, but it just doesn't work.

Now to an even stranger thing: I tried to install Linux Mint on the laptop and chose to install Ubuntu's OEMKernel (version 6.1.0) and with that the camera works. In NixOS the camera neither works with the standard 6.1.2 kernel (installed with pkgs.linuxPackages_latest) nor with the patched one here.

Compiling also takes extremely long and I can't find how to replace the kernel config (so that I can use a localmodconfig when debugging). It's easy enough to add more kernel config options, but not to change config file or remove unneeded options (to decrease compile time).

I almost feel like giving up at this point...

Mic92 commented 1 year ago

Some more details on how to build kernels from source in nixos: https://blog.thalheim.io/2022/12/17/hacking-on-kernel-modules-in-nixos

damianoognissanti commented 1 year ago

Correction, the OEM-kernel detects all cameras on Linux Mint, but they don't work unless the linux-surface kernel is installed. In NixOS they aren't even detected at all. I will look at he link you sent @Mic92.

mexisme commented 1 year ago

Correction, the OEM-kernel detects all cameras on Linux Mint, but they don't work unless the linux-surface kernel is installed. In NixOS they aren't even detected at all. I will look at he link you sent @Mic92.

I've had a chance to look at the Camera Support page, and one thing that jumped for me out is that the IPU3 firmware may not be being correctly loaded:

This jumped out as I had to do something similar to get the Surface Go's camera working a few years ago. The code for that is here:

Is there anything in journalctl or dmesg mentioning this device or firmware?

damianoognissanti commented 1 year ago

I tried to use <nixos-hardware/microsoft/surface/surface-go> in my configuration (to see if it changed anything), but still nothing.

dmesg indeed mentions ipu3, but it seems to get enabled:

[    3.645009] ipu3-cio2 0000:00:14.3: enabling device (0000 -> 0002)
[    3.645170] ipu3-cio2 0000:00:14.3: device 0x9d32 (rev: 0x1)

dmesg also mentions ov8865 (the back camera), but not ov5693 (front). These lines are repeated 29 times:

ov8865 i2c-INT347A:00: supply avdd not found, using dummy regulator
ov8865 i2c-INT347A:00: supply dvdd not found, using dummy regulator
ov8865 i2c-INT347A:00: supply dovdd not found, using dummy regulator
leonm1 commented 1 year ago

I am also running into this issue and can confirm @damianoognissanti's experience that it Just Works™ on Arch Linux with the patched linux-surface kernel, while it does not seem to work on NixOS with the same exact set of kernel patches.

I noticed the same ov8865 i2c-INT347A:00: supply avdd not found, using dummy regulator messages logged on Arch (where the camera works), so I believe that is likely a red herring.

Looking at the difference between Arch's and Nix's dmesg | grep ipu3, I am leaning towards the issue being related to ipu3 and loading its firmware.

Arch `dmesg | grep ipu3` ```dmesg [ 3.732771] ipu3_imgu: module is from the staging directory, the quality is unknown, you have been warned. [ 3.765969] ipu3-cio2 0000:00:14.3: Found supported sensor INT33BE:00 [ 3.766158] ipu3-cio2 0000:00:14.3: Found supported sensor INT347A:00 [ 3.766276] ipu3-cio2 0000:00:14.3: Found supported sensor INT347E:00 [ 3.766297] ipu3-cio2 0000:00:14.3: Connected 3 cameras [ 3.766314] ipu3-cio2 0000:00:14.3: enabling device (0000 -> 0002) [ 3.766520] ipu3-cio2 0000:00:14.3: device 0x9d32 (rev: 0x1) [ 3.771942] ipu3-imgu 0000:00:05.0: enabling device (0000 -> 0002) [ 3.772105] ipu3-imgu 0000:00:05.0: device 0x1919 (rev: 0x1) [ 3.772122] ipu3-imgu 0000:00:05.0: physical base address 0x00000000a1000000, 4194304 bytes [ 3.855583] ipu3-imgu 0000:00:05.0: loaded firmware version irci_irci_ecr-master_20161208_0213_20170112_1500, 17 binaries, 1212984 bytes ```
NixOS `dmesg | grep ipu3` ```dmesg [ 3.813630] ipu3-cio2 0000:00:14.3: enabling device (0000 -> 0002) [ 3.813829] ipu3-cio2 0000:00:14.3: device 0x9d32 (rev: 0x1) ```
NixOS Surface-related config ```nix { config, lib, pkgs, ... }: let unstable = import { config = { allowUnfree = true; }; }; libcamera_0_0_3 = unstable.libcamera.overrideAttrs (oldAttrs: rec { version = "0.0.3"; src = fetchGit { url = "https://git.libcamera.org/libcamera/libcamera.git"; rev = "f66a5c447b65bce774a1bc2d01034f437bf764b5"; }; }); linuxFirmwareNonfree = fetchGit { url = "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git"; rev = "2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f"; }; in { imports = [ ]; microsoft-surface.surface-control.enable = true; microsoft-surface.ipts.enable = true; services.udev.packages = [ pkgs.iptsd ]; ### Camera-related stuffs # Install the IPU3 firmware from linux-firmware to the path mentioned in the linux-surface wiki hardware.enableAllFirmware = true; hardware.firmware = [ (pkgs.runCommandNoCC "firmware-ipu3" { } '' mkdir -p $out/lib/firmware/intel/ cp ${linuxFirmwareNonfree}/intel/irci_irci_ecr-master_20161208_0213_20170112_1500.bin $out/lib/firmware/intel/ipu3-fw.bin '') ]; boot.kernelParams = [ "intel_pstate=no_hwp" "mem_sleep_default=deep" "acpi_enforce_resources=lax" ]; environment.systemPackages = with pkgs; [ libcamera_0_0_3 ]; } ```
leonm1 commented 1 year ago

Another interesting finding: despite CONFIG_VIDEO_IPU3_IMGU seemingly being enabled in the nixos-hardware surface kernel config, modprobe ipu3_imgu fails on NixOS, while on Arch it succeeds.

On Arch, the kernel module is available at /lib/modules/6.1.6-arch1-1-surface/kernel/drivers/staging/media/ipu3/ipu3-imgu.ko.zst and is loaded by the kernel (per lsmod:)

Arch `lsmod | grep ipu3` ``` ipu3_imgu 245760 0 ipu3_cio2 57344 0 videobuf2_dma_sg 20480 2 ipu3_cio2,ipu3_imgu videobuf2_v4l2 40960 2 ipu3_cio2,ipu3_imgu v4l2_fwnode 32768 4 ipu3_cio2,ov7251,ov5693,ov8865 videobuf2_common 86016 5 ipu3_cio2,videobuf2_v4l2,videobuf2_dma_sg,ipu3_imgu,videobuf2_memops v4l2_async 32768 5 v4l2_fwnode,ipu3_cio2,ov7251,ov5693,ov8865 videodev 319488 8 v4l2_async,ipu3_cio2,ov7251,videobuf2_v4l2,ov5693,videobuf2_common,ov8865,ipu3_imgu mc 77824 9 v4l2_async,videodev,ipu3_cio2,ov7251,videobuf2_v4l2,ov5693,videobuf2_common,ov8865,ipu3_imgu ```
NixOS `lsmod | grep ipu3` ``` ipu3_cio2 45056 0 videobuf2_dma_sg 16384 1 ipu3_cio2 videobuf2_v4l2 36864 1 ipu3_cio2 videobuf2_common 69632 4 ipu3_cio2,videobuf2_v4l2,videobuf2_dma_sg,videobuf2_memops v4l2_fwnode 32768 4 ipu3_cio2,ov7251,ov5693,ov8865 v4l2_async 28672 5 v4l2_fwnode,ipu3_cio2,ov7251,ov5693,ov8865 videodev 278528 7 v4l2_async,ipu3_cio2,ov7251,videobuf2_v4l2,ov5693,videobuf2_common,ov8865 mc 69632 8 v4l2_async,videodev,ipu3_cio2,ov7251,videobuf2_v4l2,ov5693,videobuf2_common,ov8865 ```

On Nix, the ipu3_cio2 kernel module exists and is loaded, but the ipu3_imgu module is not (and does not exist: modprobe ipu3_imgu leads to modprobe: FATAL: Module ipu3_imgu not found in directory /run/booted-system/kernel-modules/lib/modules/6.0.17.

On Arch, the ipu3_imgu module is at /lib/modules/6.1.6-arch1-1-surface/kernel/drivers/ staging/media/ipu3/ipu3-imgu.ko.zst. In Nix, the /run/booted-system/kernel-modules/lib/modules/6.0.17/kernel/drivers/staging/ directory exists, but has no subdirectory media.

I'm guessing it's related to the kernel config option CONFIG_STAGING_MEDIA. On the Arch config used to build the linux-surface kernel for Arch, CONFIG_STAGING_MEDIA=y. On my running NixOS 6.0.17 patched kernel, it is not set: cat /proc/config.gz | gunzip | grep STAGING: 

CONFIG_STAGING=y
# CONFIG_STAGING_MEDIA is not set

I will fork the nixos-hardware repo, add CONFIG_STAGING_MEDIA = yes and re-compile, then update tonight if that changes anything.

leonm1 commented 1 year ago

I have forked the nixos-hardware repo, added CONFIG_STAGING_MEDIA = yes, added it to my nix-channel with sudo nix-channel --remove nixos-hardware; sudo nix-channel --add https://github.com/leonm1/nixos-hardware/archive/master.tar.gz, ran sudo nix-channel --update, and somehow it does not seem to trigger a rebuild on sudo nixos-rebuild switch. Does anyone have any tips for how to incorporate this kernel config change?

damianoognissanti commented 1 year ago

I just realized all kernel configs in patches.nix have the CONFIG_ prefix, which they shouldn't .

I have made a local copy of the microsoft folder now and removed all prefixes from structuredExtraConfig and am compiling (I added STAGING_MEDIA too). Will report back when it's done.

damianoognissanti commented 1 year ago

Still doesn't work with the fixed prefix.nix. I also see that 5.19.17 had the correct config format (and that didn't work when I tried it in December).

damianoognissanti commented 1 year ago

I have forked the nixos-hardware repo, added CONFIG_STAGING_MEDIA = yes, added it to my nix-channel with sudo nix-channel --remove nixos-hardware; sudo nix-channel --add https://github.com/leonm1/nixos-hardware/archive/master.tar.gz, ran sudo nix-channel --update, and somehow it does not seem to trigger a rebuild on sudo nixos-rebuild switch. Does anyone have any tips for how to incorporate this kernel config change?

What I did for debugging was to git clone the repo to /etc/nixos and modified the files in /nixos-hardware/microsoft/surface/common/kernel/linux-6.0.17. In configuration.nix I then changed:

imports =
    [ 
        ./hardware-configuration.nix
        <nixos-hardware/microsoft/surface/surface-go>
    ];

to

imports =
    [ 
        ./hardware-configuration.nix
        ./nixos-hardware/microsoft/surface/surface-go
    ];

Note that it's best if you have the entire nixos-hardware structure there since e.g., the surface-go config loads files from nixos-hardware/common (so if you just move the microsoft folder to /etc/nixos, it won't work). You can of course store the nixos-hardware folder somewhere else and change the path in imports accordingly.

mexisme commented 1 year ago

I just realized all kernel configs in patches.nix have the CONFIG_ prefix, which they shouldn't .

I have made a local copy of the microsoft folder now and removed all prefixes from structuredExtraConfig and am compiling (I added STAGING_MEDIA too). Will report back when it's done.

Ugh, crap, that's likely a copy-paste typo on my part, from the linux-surface config file. I have a fix for 6.0.17 here: #544

And I updated the #534 PR.

mexisme commented 1 year ago

BTW: With the fixed patches file (i.e. with the CONFIG_ prefix removed) can it load the ipu3_imgu module correctly?

leonm1 commented 1 year ago

Thanks for pointing out I can clone the repo locally and modify it! That's significantly simpler then commit + push + update revision in my nixos config.

I used the updated version without the CONFIG_ prefix, and the kernel still didn't rebuild, so I figured there must be some issue with how we configure it.

Based on the way the hardened kernel adds structuredExtraConfig, I believe we need to move the extra kernel config from a patch directly to a structuredExtraConfig attribute passed to the linuxPackage/buildLinux call here.

After 1. stripping the CONFIG_ prefix, 2. moving the config options here and 3. nixos-rebuild, I received a promising error message: along the lines of "CONFIG_SURFACE_AGGREGATOR_ERROR_INJECTION is not a configuration option", which I saw as confirmation that the config was actually being loaded this time.

I removed that line, and now nixos-rebuild is rebuilding both the kernel config and the kernel itself. I will report back later tonight whether this fixes the issue.

mexisme commented 1 year ago

I have forked the nixos-hardware repo, added CONFIG_STAGING_MEDIA = yes, added it to my nix-channel with sudo nix-channel --remove nixos-hardware; sudo nix-channel --add https://github.com/leonm1/nixos-hardware/archive/master.tar.gz, ran sudo nix-channel --update, and somehow it does not seem to trigger a rebuild on sudo nixos-rebuild switch. Does anyone have any tips for how to incorporate this kernel config change?

What I did for debugging was to git clone the repo to /etc/nixos and modified the files in /nixos-hardware/microsoft/surface/common/kernel/linux-6.0.17. In configuration.nix I then changed:

imports =
    [ 
        ./hardware-configuration.nix
        <nixos-hardware/microsoft/surface/surface-go>
    ];

to

imports =
    [ 
        ./hardware-configuration.nix
        ./nixos-hardware/microsoft/surface/surface-go
    ];

Note that it's best if you have the entire nixos-hardware structure there since e.g., the surface-go config loads files from nixos-hardware/common (so if you just move the microsoft folder to /etc/nixos, it won't work). You can of course store the nixos-hardware folder somewhere else and change the path in imports accordingly.

FYI: This is a sound process, IMHO! I also do something similar when trying to test and diagnose my nixos-hardware changes, as it really is too slow to push frequently to Github.

My main difference is perhaps that I moved to Nix flakes about 1½ years ago, and recently started leveraging temporary flake-registry changes when testing nixos-hardware (among others).

damianoognissanti commented 1 year ago

BTW: With the fixed patches file (i.e. with the CONFIG_ prefix removed) can it load the ipu3_imgu module correctly?

Nope, modprobe ipu3_imgu says it's missing. One thing I noticed though. When compiling the kernel with the CONFIG_ prefix present (so with an invalid config) the touchpad stopped working, so the config seems to do something, which is good ^^.

leonm1 commented 1 year ago

Huzzah! I'm pleased to report that the cameras are working for me now with the changes here. I am prohibited by my employment contract from sending a PR to a project with CC0 licencing (hint, hint)

All of the following are required:

  1. Remove the CONFIG_ prefix from the config options
  2. Move the structuredExtraConfig from a patch to the linuxPackage call in linux-6.0.17/default.nix
  3. Add STAGING_MEDIA = yes;
mexisme commented 1 year ago

BTW: With the fixed patches file (i.e. with the CONFIG_ prefix removed) can it load the ipu3_imgu module correctly?

Nope, modprobe ipu3_imgu says it's missing. One thing I noticed though. When compiling the kernel with the CONFIG_ prefix present (so with an invalid config) the touchpad stopped working, so the config seems to do something, which is good ^^.

Thanks for checking, @damianoognissanti !

Huzzah! I'm pleased to report that the cameras are working for me now with the changes here. I am prohibited by my employment contract from sending a PR to a project with CC0 licencing (hint, hint)

All of the following are required:

1. Remove the `CONFIG_` prefix from the config options

2. Move the `structuredExtraConfig` from a patch to the [linuxPackage call in `linux-6.0.17/default.nix`](https://github.com/leonm1/nixos-hardware/blob/fe7e33603e5480c58e847fe70a4976264c0f2a77/microsoft/surface/common/kernel/linux-6.0.17/default.nix#L22)

3. Add `STAGING_MEDIA = yes;`

Brilliant news, @leonm1 !

I have PRs for the 6.1.x series, and just created one for the 6.1.6 patches: #545 I'll take a look when I get a chance, and would like to also incorporate your fixes — or something based on them? — into the 6.1.6 PR.

If these fixes also work on that, I'm hoping to drop the 6.0.17 kernel for 6.1.x as 6.0.x was EOL'd with 6.0.19.

mexisme commented 1 year ago 
 yes;`

Brilliant news, @leonm1 !

I have PRs for the 6.1.x series, and just created one for the 6.1.6 patches: #545 I'll take a look when I get a chance, and would like to also incorporate your fixes — or something based on them? — into the 6.1.6 PR.

If these fixes also work on that, I'm hoping to drop the 6.0.17 kernel for 6.1.x as 6.0.x was EOL'd with 6.0.19.

Hmmm, there might be something fundamentally different about my set-up, asI simply couldn't get your new code structure to build, @leonm1.

e.g. My NixOS set-up is based on release 22.11, and I build my system with Flakes, rather than Nix paths.

I was getting about override.__functionArgs not being available, and that structuredExtraConfig contained { lib = false; } when trying to use the structuredExtraConfig attrset?

damianoognissanti commented 1 year ago

So, it seems that structuredExtraConfig is called extraStructuredConfig when using kernelPatches (intuitive, I know...) (source)

I changed that line in patches.nix and it started rebuilding the kernel with the new config (I checked it by setting adding a fake option, something like asd = yes; to see if it would crash, inspired by what leonm1 wrote). It did crash, but it also warned about more unused options (such as ANDROID and others), so after removing the fake option, I added ignoreConfigErrors=true; to default.nix and now it builds.

Will report back if it works in an hour.

damianoognissanti commented 1 year ago

Huzzah! I'm pleased to report that the cameras are working for me now with the changes here. I am prohibited by my employment contract from sending a PR to a project with CC0 licencing (hint, hint)

All of the following are required:

1. Remove the `CONFIG_` prefix from the config options

2. Move the `structuredExtraConfig` from a patch to the [linuxPackage call in `linux-6.0.17/default.nix`](https://github.com/leonm1/nixos-hardware/blob/fe7e33603e5480c58e847fe70a4976264c0f2a77/microsoft/surface/common/kernel/linux-6.0.17/default.nix#L22)

3. Add `STAGING_MEDIA = yes;`

Wow, this is incredible! Good job!!!

damianoognissanti commented 1 year ago

Wow! It's working!! I have pushed the changes to my fork. :smile: The change I did was:

damianoognissanti commented 1 year ago

Additional info: To make it work with Zoom, Skype, etc. you must

damianoognissanti commented 1 year ago

Oh, I have a custom libcamera based on the build instructions from the linux-surface page. The standard package doesn't seem to work.

This might be off topic, but I have uploaded it here.

mexisme commented 1 year ago

So, it seems that structuredExtraConfig is called extraStructuredConfig when using kernelPatches (intuitive, I know...) (source)

Oh sheesh! I would never have noticed this, skimming the Nix manuals or code!!

[...]

Wow! It's working!! I have pushed the changes to my fork. smile The change I did was:

Changed structuredExtraConfig to extraStructuredConfig
Removed CONFIG_ prefixes
Added STAGING_MEDIA = yes;
Added ignoreConfigErrors=true;

Nice! I'll have to take a look!

FYI: The 6.1.6 kernel was merged a few days ago. I'll re-try to update + test the code with your changes, as I agree they're more readable.

If you'd like to mess with it, you'll have to select it with microsoft-surface.kernelVersion = "6.1.6"; in your config.

Mic92 commented 1 year ago

is this solved now?

leonm1 commented 1 year ago

No; a few additional changes are required:

  1. Changing the structuredExtraConfig in microsoft/surface/common/kernel/*/patches.nix to extraStructuredConfig
  2. Changing STREAMING_MEDIA to STAGING_MEDIA
  3. Removing SURFACE_AGGREGATOR_ERROR_INJECTION, which causes the build to fail

For the 6.1.6 kernel, that looks like this:

diff --git a/microsoft/surface/common/kernel/linux-6.1.6/patches.nix b/microsoft/surface/common/kernel/linux-6.1.6/patches.nix
index 7b3cfdca5610..dadc45d34bf1 100644
--- a/microsoft/surface/common/kernel/linux-6.1.6/patches.nix
+++ b/microsoft/surface/common/kernel/linux-6.1.6/patches.nix
@@ -7,14 +7,13 @@
   {
     name = "microsoft-surface-patches-linux-${version}";
     patch = null;
-    structuredExtraConfig = with kernel; {
-      STREAMING_MEDIA = yes;
+    extraStructuredConfig = with kernel; {
+      STAGING_MEDIA = yes;

       #
       # Surface Aggregator Module
       #
       SURFACE_AGGREGATOR = module;
-      SURFACE_AGGREGATOR_ERROR_INJECTION = no;
       SURFACE_AGGREGATOR_BUS = yes;
       SURFACE_AGGREGATOR_CDEV = module;
       SURFACE_AGGREGATOR_HUB = module;
damianoognissanti commented 1 year ago

The kernels here still use structuredExtraConfig instead of extraStructuredConfig which means the patches are never applied: https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/common/kernel/linux-5.19.17/patches.nix https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/common/kernel/linux-6.0.17/patches.nix https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/common/kernel/linux-6.1.18/patches.nix

Can this be fixed, please? And is there a possibility that a binary kernel could be cached in some way (like other software in the nix store) so that recompilation is not necessary when updating?

damianoognissanti commented 11 months ago

Sorry for spamming, but the kernels get updated, but still use structuredExtraConfig instead of extraStructuredConfig, which means if I update the kernel the camera breaks if I don't manually fix it every time.

https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/common/kernel/linux-6.1.55/patches.nix https://github.com/NixOS/nixos-hardware/blob/master/microsoft/surface/common/kernel/linux-6.5.5/patches.nix

And the option STREAMING_MEDIA = yes; should be STAGING_MEDIA = yes;.

EDIT: It looked like it was fixed when I compiled the kernel (it looked like the patches were applied), but the camera won't work with the new kernel, so the patches aren't applied.