Go 3: Camera probe fails

[PhilDevProg]

https://github.com/linux-surface/linux-surface/issues/91#issuecomment-1422439737

[djrscally]

OK so, I did a write up on reverse engineering steps which details what you need to do. Can you give that a go when you get the time? When you get to the point that the I2cTestTool is talking to the PMIC properly I'll run through next steps. If you have compilation problems with the Microsoft ASL tool try Intel's compiler instead, though you'll still need to use asl.exe to instal the new file.

I'm expecting that the outcome will be "it's all the same as the Go1/Go2", but better to be safe than sorry...

[PhilDevProg]

@djrscally I installed ASL and I'm in the directory of asl.exe and I did runasl.exe /tab=DSDTbut no file was created... And also, should I just add the example in the Wiki or do I need to add something else too?

Edit: I copied asl.exe to my Downloads folder and there it created the file. I copied the content from the Wiki into it but I couldn't find the version number in DefinitionBlock() and when I try to compile it via asl.exe DSDT.ASL, I get these errors:

Microsoft ACPI Source Language Assembler Version 5.0.0NT Copyright (c) 1996,2014 Microsoft Corporation Compliant with the ACPI 5.0 Specification DSDT.ASL: asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0GFX0 asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0ISP0 asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0ALSI asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0CDCK asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0CBLV asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0GSSE asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0PEG0 asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0PEG0PEGP asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0PEG1 asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0PEG2 asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0DD1F asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0GSCI asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0CLID asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0HDAS asl_ERR: GetNameSpaceObj: invalid name - \/♥_PR_PR00LPSS asl_ERR: GetNameSpaceObj: invalid name - \/♥_PR_PR00TPSS asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP01PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP02PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP03PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP04PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP05PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP06PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP07PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP08PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP09PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP10PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP11PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP12PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP13PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP14PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP15PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP16PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP17PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP18PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP19PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP20PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP21PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP22PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP23PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0RP24PPRW asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0XHC_PS0X asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0XHC_PS3X asl_ERR: GetNameSpaceObj: invalid name - \/♣_SB_PCI0XHC_RHUBPS0X asl_ERR: GetNameSpaceObj: invalid name - \/♣_SB_PCI0XHC_RHUBPS2X asl_ERR: GetNameSpaceObj: invalid name - \/♣_SB_PCI0XHC_RHUBPS3X asl_ERR: GetNameSpaceObj: invalid name - \/♣_SB_PCI0XHC_RHUBINIR asl_ERR: GetNameSpaceObj: invalid name - \/♣_SB_PCI0LPCBH_ECXDAT asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0HDASPS0X asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0HDASPS3X asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0HDASPPMS asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0HIDW asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_PCI0HIWC asl_ERR: GetNameSpaceObj: invalid name - /♥SAT0NVM1VLPM asl_ERR: GetNameSpaceObj: invalid name - /♥SAT0NVM2VLPM asl_ERR: GetNameSpaceObj: invalid name - /♥SAT0NVM3VLPM asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0SAT0SDSM asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_WSIDSHPS asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_LID0_LID asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0TCHE asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0GFX0STAT asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_TPMPTS asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0PAUDPUAM asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0PEG0HPME asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0PEG1HPME asl_ERR: GetNameSpaceObj: invalid name - \/♦_SB_PCI0PEG2HPME asl_ERR: GetNameSpaceObj: invalid name - \/♥_SB_AWACWAST 214: BNUM, 8, ^*** DSDT.ASL(214): error: BNUM already exist

and when I try to build it via Intel's compiler I get these errors:

Intel ACPI Component Architecture ASL+ Optimizing Compiler/Disassembler version 20221020 Copyright (c) 2000 - 2022 Intel Corporation Error 6126 - Compiler aborting due to parser-detected syntax error(s) DSDT.ASL 4868: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 4942: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5071: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5105: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5116: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5127: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5594: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5605: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5696: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 5697: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 5700: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 12182: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 14446: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 14447: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 14450: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 14479: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 14480: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 14483: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 18766: Arg0 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG0 DSDT.ASL 19237: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 19976: Notify(\_SB_.PCI0.PEG0, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 19982: Notify(\_SB_.PCI0.PEG1, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 19987: Notify(\_SB_.PCI0.PEG2, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 20445: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 20454: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 20468: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 23914: Error 6126 - syntax error, unexpected $end and premature End-Of-File Error 6126 - Compiler aborting due to parser-detected syntax error(s) DSDT.ASL 4868: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 4942: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5071: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5105: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5116: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5127: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5594: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5605: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 5696: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 5697: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 5700: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 12182: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 14446: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 14447: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 14450: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 14479: If(CondRefOf(HIDW, )) Error 6126 - ^ syntax error, unexpected PARSEOP_IF DSDT.ASL 14480: { Error 6126 - ^ syntax error, unexpected '{' DSDT.ASL 14483: Arg1 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG1 DSDT.ASL 18766: Arg0 Error 6126 - ^ syntax error, unexpected PARSEOP_ARG0 DSDT.ASL 19237: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 19976: Notify(\_SB_.PCI0.PEG0, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 19982: Notify(\_SB_.PCI0.PEG1, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 19987: Notify(\_SB_.PCI0.PEG2, 0x2) Error 6126 - ^ syntax error, unexpected PARSEOP_NOTIFY DSDT.ASL 20445: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 20454: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 20468: } Error 6126 - ^ syntax error, unexpected '}' DSDT.ASL 23914: Error 6126 - syntax error, unexpected $end and premature End-Of-File Input file: DSDT.ASL - Compilation aborted due to parser-detected syntax error(s) Compilation failed. 28 Errors, 0 Warnings, 0 Remarks No AML files were generated due to syntax error(s)

[qzed]

I believe you might have better luck trying to compile that under Linux. At least when I tried it on Windows I had similar issues and wasn't able to fix those.

[djrscally]

As qzed says, you might try compiling it when booted into linux. On my Go2 I have an sd card that both operating systems can see so it's fairly easy to share files between them - the linux compiler seems much better and will likely reduce that error list considerably, so it's worth trying. The linux compiler is also called iasl and can be installed with sudo apt install acpica-tools. Note there'll almost certainly still be some errors (how they manage to install a broken DSDT I have no idea) but they shouldn't be too bad.

And also, should I just add the example in the Wiki or do I need to add something else too?

The wiki example should be right in your case, you can just try that.

[PhilDevProg]

@djrscally I get these errors too and it can't compile:

Intel ACPI Component Architecture ASL+ Optimizing Compiler/Disassembler version 20220331 Copyright (c) 2000 - 2022 Intel Corporation Compiler aborting due to parser-detected syntax error(s) DSDT.ASL 4868: } Error 6126 - syntax error ^ DSDT.ASL 4942: } Error 6126 - syntax error ^ DSDT.ASL 5071: } Error 6126 - syntax error ^ DSDT.ASL 5105: } Error 6126 - syntax error ^ DSDT.ASL 5116: } Error 6126 - syntax error ^ DSDT.ASL 5127: } Error 6126 - syntax error ^ DSDT.ASL 5594: } Error 6126 - syntax error ^ DSDT.ASL 5605: } Error 6126 - syntax error ^ DSDT.ASL 5696: If(CondRefOf(HIDW, )) Error 6126 - syntax error ^ DSDT.ASL 5697: { Error 6126 - syntax error ^ DSDT.ASL 5700: Arg1 Error 6126 - syntax error ^ DSDT.ASL 12182: Arg1 Error 6126 - syntax error ^ DSDT.ASL 14446: If(CondRefOf(HIDW, )) Error 6126 - syntax error ^ DSDT.ASL 14447: { Error 6126 - syntax error ^ DSDT.ASL 14450: Arg1 Error 6126 - syntax error ^ DSDT.ASL 14479: If(CondRefOf(HIDW, )) Error 6126 - syntax error ^ DSDT.ASL 14480: { Error 6126 - syntax error ^ DSDT.ASL 14483: Arg1 Error 6126 - syntax error ^ DSDT.ASL 18766: Arg0 Error 6126 - syntax error ^ DSDT.ASL 19237: } Error 6126 - ^ syntax error DSDT.ASL 19976: Notify(\_SB_.PCI0.PEG0, 0x2) Error 6126 - syntax error ^ DSDT.ASL 19982: Notify(\_SB_.PCI0.PEG1, 0x2) Error 6126 - syntax error ^ DSDT.ASL 19987: Notify(\_SB_.PCI0.PEG2, 0x2) Error 6126 - syntax error ^ DSDT.ASL 20445: } Error 6126 - syntax error ^ DSDT.ASL 20454: } Error 6126 - ^ syntax error DSDT.ASL 20468: } Error 6126 - syntax error ^ DSDT.ASL 23914: Error 6126 - syntax error and premature End-Of-File Input file: DSDT.ASL - Compilation aborted due to parser-detected syntax error(s) Compilation failed. 0 Errors, 0 Warnings, 0 Remarks No AML files were generated due to syntax error(s)

[djrscally]

@PhilDevProg can you upload the DSDT.ASL file for me to take a look at? Annoyingly it just means working through these errors until it compiles properly as the first step.

[PhilDevProg]

@djrscally DSDT.zip

[WolfWings]

Fellow SG3 owner here but I don't have Windows installed currently.

Just was looking over their DSDT.asl output, and it looks like the MS ASL tool may be generating different disassembly than the Intel tool accepts now?

Would it be possible to extract the DSDT from the Linux side and use the Intel tool for the whole modification process instead @djrscally? From that side it seems to re-compile successfully with various warnings.

Looking at the decompile of that from the Intel tool it appears the DSDT binary itself isn't OS-specific (I see lots of _OSI calls for the OS name being compared against things that don't seem to change with different acpi_os_name settings on Linux) but I wasn't sure if there's some manipulation being done by Windows for the version that tool can access.

[djrscally]

Would it be possible to extract the DSDT from the Linux side and use the Intel tool for the whole modification process instead @djrscally? From that side it seems to re-compile successfully with various warnings.

Yes it should work fine - that's what I did for my Go2; I've been trying to fix all the warnings from the Windows-extracted one, but if the Linux extracted one will compile fine as is then we should use that. Can you share it? Or @PhilDevProg can you try to grab it from your Linux boot and see if that compiles cleanly?

[WolfWings]

DSDT.dsl.gz

Here's the unmodified DSDT decompile from my Go3, as I said it has several warnings but no outright syntax errors like the Windows-extracted one does.

I decompiled it using the https://archlinux.org/packages/community/x86_64/acpica/ packaged version which has the command as iasl to specify the Intel tool.

Just for reference the steps taken to generate this were:

1. sudo cp /sys/firmware/acpi/tables/DSDT DSDT.bin
2. iasl -d DSDT.bin (the tool defaults to a .dsl extension now for decompiles?)

Recompiling it with iasl DSDT.dsl generates 60-ish warnings, 250-ish remarks, but compiles successfully.

That said, I don't have Windows installed on my Go3 ATM so I can't test it until this weekend at the earliest.

[PhilDevProg]

@djrscally @WolfWings That worked! Until I copy and paste the content from the Wiki into it... Then I get this error:

DSDT.dsl 25529: Error 6126 - syntax error and premature End-Of-File Input file: DSDT.dsl - Compilation aborted due to parser-detected syntax error(s) Compilation failed. 0 Errors, 64 Warnings, 248 Remarks No AML files were generated due to syntax error(s)

My DSDT.dsl file: DSDT.zip

[WolfWings]

@PhilDevProg Figured it out, had a few spare minutes. Move the } above the Device(RHPX) line to AFTER the final } in the original disassembly.

If you want to be pedantic then indent the whole Device(RHPX){} block by four spaces as well to match the new brace structure.

[PhilDevProg]

@djrscally I could compile it now but when I try to insert it into Windows via asl.exe /loadtable -v DSDT.aml I get this:

Microsoft ACPI Source Language Assembler Version 5.0.0NT Copyright (c) 1996,2014 Microsoft Corporation Compliant with the ACPI 5.0 Specification DSDT info for update image (DSDT.aml) Size of image: 110626 OEM id.......: MSFT OEM Table id.: MSFT OEM revision.: 00000000 Error: Could not access the registry path: System\CurrentControlSet\Services\ACPI\Parameters\DSDT\MSFT__\MSFT____\00000000

[djrscally]

@PhilDevProg are you running asl.exe as Administrator? And also, did you remember to increment the version number in the DSDT before compiling it? The zip you uploaded doesn't seem to work so I can't check, but it's the last parameter in the first DefinitionBlock:

DefinitionBlock("DSDT.AML", "DSDT", 0x02, "MSFT  ", "MSFT    ", 0x00000000)

You need to increment that number by one each time you try to insert a new DSDT

[PhilDevProg]

@djrscally It worked now. Is there somewhere an exe file for the I2CTestTool or can you build one? From what I read I understood that you need Visual Studio which requires 8GB of storage which I don't have because I have also installed Fedora which is encrypted. Although the partition is resizable the Windows partition can't be extended (because the unused space isn't on the right of it).

[djrscally]

I2cTestTool.zip

@PhilDevProg does this work?

[PhilDevProg]

@djrscally It says that MSVCP140D.dll couldn't be found...

[djrscally]

Bleh; I'll see if I can figure out how to compile it without relying on shared objects. Won't be till the evening though.

[PhilDevProg]

No problem, vccorlib104d.DLL and VCRUNTIME140D.dll and VCRUNTIME140_1D.dll are missing too.

[WolfWings]

Alternative @PhilDevProg might be to install (quite literally) each of the VC Redistributable packages and see if it starts working? https://learn.microsoft.com/en-US/cpp/windows/latest-supported-vc-redist is where to start, the 2015/2017/2019/2022 combined redist is very likely the only one you need, so just https://aka.ms/vs/17/release/vc_redist.x64.exe (which yes, MS documents as a permalink) may be enough.

[PhilDevProg]

@WolfWings That sadly didn't work. The same DLLs are still missing

[qzed]

I think the D/d suffixes here stand for debug versions. @djrscally can you try uploading a release build? Then the standard VC Redistributable installer(s) should work.

[djrscally]

@qzed Thanks, good thought. @PhilDevProg try this one?

I2cTestTool.zip

[PhilDevProg]

@djrscally Yes! That works perfect! What should I do now?

[djrscally]

@PhilDevProg Nice! OK, run ./I2cTestTool 0x4d I2C2, you should get a prompt. If you run writeread { ff } 1 it should return "21" which means you're talking to the chip properly.

Assuming that works, search for "Set up face sign in" in Start, go to "Windows hello face" and then whatever the options to progress through setup are. When you get to the stage where it shows your face with both the white and red leds on the front of the surface lit, you need to run writeread { 00 } 256 in the I2CTestTool prompt. It's important this happens whilst the red LED is lit, as that's when the ov7251 is powered on (and we're trying to see the settings that power that on correctly). Annoyingly the Hello setup doesn't take long if your face is central and face on...so you might need to weave your head around a lot to stop it completing whilst you run the read :D

The command should dump out a list of 256 numbers; paste them here please. If you feel like it you could also run the same writeread command whilst the world facing camera is running (no head weaving needed here) - I already have those settings but I didn't save them for some reason so it'd be nice to get another readout.

[PhilDevProg]

@djrscally writeread { ff } 1 returns 12 and writeread { 00 } 256 returns this:

0 20 f2 0 0 0 1 3 2 11 aa 20 1 d7 0 a 5 0 0 0 1 8 1 8 1 8 8a 8 8a 8 c2 8 c2 8 0 0 0 0 0 60 f0 1 a 0 0 0 0 1f 7 38 0 0 1f 1f c 0 0 0 0 0 6d 6d 34 34 34 6d c 3 0 1 1 0 4 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 21

For the rear camera, would it be the same commands while using the rear camera in the Camera app?

[djrscally]

For the rear camera, would it be the same commands while using the rear camera in the Camera app?

Correct

writeread { 00 } 256 returns this:

And formatted less annoying that is:

    0   1   2   3   4   5   6   7   8   9   a   b   c   d   e   f
00  0   20  f2  0   0   0   1   3   2   11  aa  20  1   d7  0   a
10  5   0   0   0   1   8   1   8   1   8   8a  8   8a  8   c2  8
20  c2  8   0   0   0   0   0   60  f0  1   a   0   0   0   0   1f
30  7   38  0   0   1f  1f  c   0   0   0   0   0   6d  6d  34  34
40  34  6d  c   3   0   1   1   0   4   0   0   0   0   0   0   0
50  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
60  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
70  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
80  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
90  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
a0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
b0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
c0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
d0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
e0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   0
f0  0   0   0   0   0   0   0   0   0   0   0   0   0   0   0   21

GPIOs 5&6 are on. 5 is the enable one on the Go2, so should be the same here. I think 6 is something to do with the IR LED which I'm trying to figure out now. Give me a few minutes and I'll have a patch for you; do you know how to build your own kernel?

[PhilDevProg]

@djrscally

Correct

Ok, here's the output for the rear camera:

0 20 1f 0 0 0 1 3 2 11 aa 20 1 d7 0 a 5 0 0 0 1 8 1 8 1 8 8a 8 8a 8 c2 8 c2 8 5 0 0 0 0 0 34 1 a 0 0 0 0 1f 7 38 0 0 1f 1f 0 0 0 0 0 0 6d 6d 34 34 34 6d c 3 1 0 0 1 5 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 21

do you know how to build your own kernel?

I'm using Fedora and last time I tried it (that was when you suggested to disable the sensor) I couldn't install it. But I can try it tomorrow if you send me the patch and tell me, how to install it.

[PhilDevProg]

@djrscally Will you still send me the patch or have you decided to put it directly into the surface-kernel?

[djrscally]

@PhilDevProg sorry; I got very badly distracted and forgot all about this. The rear camera's output reformatted is:

        0       1       2       3       4       5       6       7       8       9       a       b       c       d       e       f
00      0       20      1f      0       0       0       1       3       2       11      aa      20      1       d7      0       a
10      5       0       0       0       1       8       1       8       1       8       8a      8       8a      8       c2      8
20      c2      8       5       0       0       0       0       0       34      1       a       0       0       0       0       1f
30      7       38      0       0       1f      1f      0       0       0       0       0       0       6d      6d      34      34
40      34      6d      c       3       1       0       0       1       5       0       0       0       0       0       0       0
50      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
60      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
70      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
80      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
90      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
a0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
b0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
c0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
d0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
e0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       0
f0      0       0       0       0       0       0       0       0       0       0       0       0       0       0       0       21

Which conveniently matches what I already had so that's nice. All you should need then is to tell the camera to use the same GPIO lines as the SGo1/2, which should just be this (and it's in the attached patch too)

diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
index 309eab9c0558..76bb006874a2 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -161,7 +161,8 @@ static const struct int3472_tps68470_board_data surface_go3_tps68470_board_data
        .tps68470_regulator_pdata = &surface_go_tps68470_pdata,
        .n_gpiod_lookups = 1,
        .tps68470_gpio_lookup_tables = {
-               &surface_go_int347a_gpios
+               &surface_go_int347a_gpios,
+               &surface_go_int347e_gpios,
        },
 };

I'm using Fedora and last time I tried it (that was when you suggested to disable the sensor) I couldn't install it. But I can try it tomorrow if you send me the patch and tell me, how to install it.

So the simplest way in my view is to install the dependencies:

sudo apt-get install git fakeroot build-essential ncurses-dev xz-utils libssl-dev bc flex libelf-dev bison

Then dowload the attached files (which I will assume land in ~/Downloads). Next clone and patch the kernel:

git clone -b v6.1-surface-devel https://github.com/linux-surface/kernel
cd kernel
cp ~/Downloads/config.txt .config  << note the name change here, it's important
git am ~/Downloads/0001-platform-x86-int3472-Add-GPIOs-to-Surface-Go-3-Board.txt

And then at that point you can build packages to install on the Go2. I run ubuntu so I run make bindeb-pkg and then copy the output .debs to the Surface (I build on my PC) and install them with sudo dpkg -i linux*.deb. I have never used Fedora...there's also a make binrpm-pkg option which makes RPM packages which presumably would work similarly. Failing that there's make modules_install and make install which build and install the modules and kernel respectively (which would mean the compilation would be on the Surface). If you can't use an RPM or a DEB and don't want to compile on the Surface, then I think you're left with building on another machine and using rsync -rcv to transfer the modules to lib/modules/ and the initramfs, System.map and vmlinuz file to /boot on the Go and then updating Grub to notice them

config.txt 0001-platform-x86-int3472-Add-GPIOs-to-Surface-Go-3-Board.txt

[qzed]

Feel free to ping me once this is verified/tested and I'll add it to our patches.

[PhilDevProg]

@djrscally @qzed I installed it but I always get this error when I try to boot it:

error: ../../grub-core/loader/efi/linux.c:52:kernel DOS magic is invalid. error: ../../grub-core/loader/i386/efi/linux.c:258:you need to load the kernel first.

Secure Boot is disabled.

[qzed]

I'm not sure what exactly goes wrong, but that i386 and DOS magic seems a bit odd. Looks like 32 bit code and not 64 bit?

[PhilDevProg]

@djrscally @qzed When I don't use the config @djrscally send me but instead the one that's shipped with the surface kernel it doesn't have this error but it gets it stops booting when trying to start systemd-cryptsetup and just stays at that point...

[qzed]

Did you set up anything special for encryption? For example using the TPM?

[djrscally]

Rely on the surface config rather than mine then. Can you detail exactly how you're booting and share a picture of the boot log that shows the error?

On Sun, 26 Feb 2023, 10:07 PhilProg, @.***> wrote:

@djrscally https://github.com/djrscally @qzed https://github.com/qzed When I don't use the config @djrscally https://github.com/djrscally send me but the one that's shipped with the surface kernel it doesn't have this error but it gets into a bootloop while trying to start systemd-cryptsetup...

— Reply to this email directly, view it on GitHub https://github.com/linux-surface/linux-surface/issues/1055#issuecomment-1445315502, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBE257SBW4S5RCDWZYFLDWZMTN7ANCNFSM6AAAAAAUVF77TI . You are receiving this because you were mentioned.Message ID: @.***>

[PhilDevProg]

Did you set up anything special for encryption? For example using the TPM?

Yes, I use Clevis but it doesn't work without Secure Boot so I disabled it by clearing the slots...

To boot, I turn on the Surface and navigate in Grub to the right boot entry.

[qzed]

Does the normal kernel boot with Secure Boot disabled? I would have assumed that this is an issue with your boot chain being changed due to disabling Secure Boot, which will prevent the TPM from providing you with the key, but it looks like you're already aware of that. Unfortunately, I know next to nothing about clevis, so I'm not sure why it doesn't come up with the "standard" key entry prompt.

[PhilDevProg]

Does the normal kernel boot with Secure Boot disabled? I would have assumed that this is an issue with your boot chain being changed due to disabling Secure Boot, which will prevent the TPM from providing you with the key, but it looks like you're already aware of that. Unfortunately, I know next to nothing about clevis, so I'm not sure why it doesn't come up with the "standard" key entry prompt.

Yes, the normal surface kernel boots with Secure Boot disabled. It just shows the normal LUKS entry... It's also weird that it showed the LUKS entry some days ago with another custom kernel. I also tried to completely remove Clevis but that doesn't help.

[PhilDevProg]

I'll create an image of my SSD, clear the SSD and reinstall Fedora in the next days to try it with a clean installation.

[WolfWings]

Okay, so I'm back from a random work trip that extended my previous trip.

On Arch this looks to just be a simple matter of adding the patch as a new one to the PKGBUILD file and building a kernel update package one-off as the easiest way to test, yes? Since that's what my SGo3 is running, with shim-signed into systemd-boot, I admit it's not fedora or the usual grub-based install but I'm happy to test if it'll be useful?

I can compile it up and test tonight in a few hours if so.

[qzed]

On Arch this looks to just be a simple matter of adding the patch as a new one to the PKGBUILD file and building a kernel update package one-off as the easiest way to test, yes?

Yes, pretty much (add to PKGBUILD, run updpkgsums, build it). You'll probably want to change the pkgbase to linux-surface-dev or something similar (should start with linux-) so that it doesn't collide with our kernel, if you have that installed.

If you want to build directly from a source tree, I have a python script at https://github.com/qzed/linux-surface-kernel-pkgscripts, but that's not really well-documented.

[WolfWings]

Hrm... no change with the patch as posted. Looking over the patch and comparing it to the OG file, do we need to update this line as well?

.n_gpiod_lookups = 1,

I see on the surface_go_tps68470_board_data just above the separate go3 it has that set to 2 to match the number of pointers in the lookup tables array?

I code a lot, but I don't poke at kernel stuff so I admit I'm wary of just barging in and changing things since I don't want to break anything. :)

[djrscally]

Derp; yes - that needs setting to 2 as well. Good spot!

[WolfWings]

Just as a quick dmesg | grep -i ov7251 to minimize my comment here, full dmesg on request if needed?

Existing 6.1.12 Arch linux-surface kernel:

[    5.729870] ov7251 i2c-INT347E:00: cannot get enable gpio
[    6.720482] ov7251: probe of i2c-INT347E:00 failed with error -2

With the patch and the 1➡️2 edit:

[    6.664782] ov7251 i2c-INT347E:00: OV7251 revision 7 (1F) detected at address 0x60
[    6.665430] ov7251 i2c-INT347E:00: Consider updating driver ov7251 to match on endpoints

[djrscally]

@WolfWings That looks like "All is well" to me; the world and user facing cameras should work now...do they? The IR camera doesn't work through libcamera currently but I could give you a script to capture from it (though the IR LED doesn't work yet so it'd be dim - that's what I'm currently working on)

[WolfWings]

Might as well test all components, if you have a script to test the IR camera sure, I'll give that a spin too just to test everything all at once! I also admit I've been curious about poking at the IR camera for some projects. :)

[djrscally]

You will need two binaries installed in $PATH, first ipu3-unpack which is part of libcamera (in utils/ipu3). This converts the CIO2 packed data into normal Y10 form. Next, raw2rgbpnm which you can clone from git://salottisipuli.retiisi.org.uk/~sailus/raw2rgbpnm.git - this converts the Y10 data into a .ppm file that you can view.

With those two available you can run something like this to capture:

#!/bin/bash

# Get the devnode
VDEV=$(media-ctl -d1 -e "ipu3-cio2 2");

# Set links and format
media-ctl -d1 -l "'ov7251 3-0060':0 -> 'ipu3-csi2 2':0[1]";
media-ctl -d1 -V "'ipu3-csi2 2':0 [fmt:Y10_1X10/640x480]";
v4l2-ctl -d "$VDEV" --set-fmt-video="width=640,height=480,pixelformat=4";

# Capture
v4l2-ctl -d "$VDEV" --stream-mmap --stream-to=/home/djrscally/ov7251.bin --stream-count=1;

# Convert
ipu3-unpack ~/ov7251.bin ~/ov7251.raw
raw2rgbpnm -s 640x480 -f Y10 ~/ov7251.raw ~/ov7251.ppm

And you should be able to open ~/ov7251.ppm and get something like this:

The plan once I can drive the IR LED reliably is to make libcamera support the camera better so that this camera will just work as the other two do, but for now this is the only means of capture. It's different to the other two, as it doesn't need to pass data through the IPU3 which is what libcamera expects to happen, so it's not supported by the library for now.

[WolfWings]

Confirmed, all three camera sensors work, first two with QCam, last from the CLI method all photographing a lamp I had handy. I used rawpixels.net to view the output from ipu3-unpack directly (650x480 at 16-bit greyscale to account for the stride) as it was faster than building another tool.

[djrscally]

@WolfWings ok great - thanks very much for testing. In that case I'll open a PR with the change here and upstream it.