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

Surface book 2 camera not working #523

Open
damianoognissanti opened this issue Dec 22, 2022 · 34 comments
Open

Surface book 2 camera not working #523

damianoognissanti opened this issue Dec 22, 2022 · 34 comments

Comments

@damianoognissanti
Copy link
Contributor

damianoognissanti commented Dec 22, 2022

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
Copy link
Contributor

mexisme commented Jan 9, 2023

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?

@damianoognissanti
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@mexisme
Copy link
Contributor

mexisme commented Jan 10, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

Mic92 commented Jan 13, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

mexisme commented Jan 13, 2023

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
Copy link
Contributor Author

damianoognissanti commented Jan 14, 2023

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
Copy link

leonm1 commented Jan 17, 2023

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`
[    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`
[    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
{ config, lib, pkgs, ... }:

let
  unstable = import <nixos-unstable> { 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 = [
    <nixos-hardware/microsoft/surface/surface-pro-intel>
  ];

  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
Copy link

leonm1 commented Jan 17, 2023

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
Copy link

leonm1 commented Jan 17, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

mexisme commented Jan 18, 2023

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
Copy link
Contributor

mexisme commented Jan 18, 2023

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

@leonm1
Copy link

leonm1 commented Jan 18, 2023

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
Copy link
Contributor

mexisme commented Jan 18, 2023

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
Copy link
Contributor Author

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
Copy link

leonm1 commented Jan 18, 2023

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
Copy link
Contributor

mexisme commented Jan 19, 2023

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
Copy link
Contributor

mexisme commented Jan 19, 2023

 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

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

@damianoognissanti
Copy link
Contributor Author

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

  • Install libcamera

  • Add the following to your nix config:

  # Make camera work in Skype, Zoom, etc
  boot.extraModulePackages = with config.boot.kernelPackages;
    [ v4l2loopback.out ];
  boot.kernelModules = [ "v4l2loopback" ];
  boot.extraModprobeConfig = ''
    # exclusive_caps: Skype, Zoom, Teams etc. will only show device when actually streaming
    # card_label: Name of virtual camera, how it'll show up in Skype, Zoom, Teams
    # https://github.com/umlaeute/v4l2loopback
    options v4l2loopback exclusive_caps=1 card_label="Virtual Camera"
  '';
  • Run this in a terminal to activate the camera:
    gst-launch-1.0 libcamerasrc camera-name='\\\_SB_.PCI0.I2C2.CAMF' ! videoconvert ! v4l2sink device=/dev/video0

  • Start Zoom

@damianoognissanti
Copy link
Contributor Author

damianoognissanti commented Jan 19, 2023

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
Copy link
Contributor

mexisme commented Jan 26, 2023

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
Copy link
Member

Mic92 commented Feb 18, 2023

is this solved now?

@leonm1
Copy link

leonm1 commented Feb 18, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

damianoognissanti commented Oct 24, 2023

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.

damianoognissanti added a commit to damianoognissanti/nixos-hardware that referenced this issue Jan 16, 2024
As discussed here: NixOS#523

1) `structuredExtraConfig` is called `extraStructuredConfig` when using `kernelPatches`

2) STREAMING_MEDIA should be STAGING_MEDIA
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

No branches or pull requests

4 participants