Skip to content

Surface Pro 9: fix rear camera rotation - #170

Open
jmmartinf wants to merge 2 commits into
linux-surface:v6.19-surface-develfrom
jmmartinf:sp9-rear-camera-rotation
Open

Surface Pro 9: fix rear camera rotation#170
jmmartinf wants to merge 2 commits into
linux-surface:v6.19-surface-develfrom
jmmartinf:sp9-rear-camera-rotation

Conversation

@jmmartinf

Copy link
Copy Markdown

The rear OV13858 camera (OVTID858) of the Surface Pro 9 comes out rotated 180°: the firmware SSDB reports degree=0, and the existing DMI quirk in ipu-bridge only covers the front OVTI5693 camera (added in #160).

Fixing it takes two commits:

  1. media: ipu-bridge: check all DMI entries when overriding sensor rotationipu_bridge_parse_rotation() uses dmi_first_match(), which always stops at the first entry matching the running machine, so a second entry for the same machine (needed for a second sensor) is unreachable. Walk the whole table and match each entry for the running machine against the sensor's ACPI HID instead. This is a general fix, not SP9-specific; I intend to send it to linux-media mainline as well.
  2. media: ipu-bridge: fix rear camera rotation on Surface Pro 9 — add the Surface Pro 9OVTID858 quirk entry, next to the existing front-camera one.

Tested on a Surface Pro 9 (kernel 6.19.8-3.surface.fc43, module rebuilt from v6.19-surface):

  • libcamera now reports Rotation = 180 for the rear camera (was 0) and the image renders upright in GNOME Snapshot.
  • No regression on the front camera: still Rotation = 180, upright image, cold first-open works.
  • Both cameras keep delivering frames (cam --capture), no new ipu-bridge/ov5693/ov13858 errors in dmesg.

@zR-JB

zR-JB commented Aug 12, 2026

Copy link
Copy Markdown

Maybe we could combine all the surface devices with this rear sensor? Surface Pro 12 for Business Intel needs the same fix:

linux-surface/linux-surface#2144 (comment)

SP12 uses the same mechanism for OVTID858:

see #175

A machine can have more than one sensor whose rotation needs to be
overridden, which takes one upside_down_sensor_dmi_ids[] entry per
sensor, all sharing the same DMI match but with different ACPI HIDs in
driver_data.

ipu_bridge_parse_rotation() uses dmi_first_match(), which always stops
at the first entry matching the running machine, so any further entry
for the same machine is unreachable and only one sensor per machine can
ever be corrected.

Walk the whole table and match every entry for the running machine
against the sensor's ACPI HID instead.

Fixes: b757101 ("media: ipu-bridge: Add DMI quirk for Dell XPS laptops with upside down sensors")
Cc: stable@vger.kernel.org
Signed-off-by: José María Martín <jmmartinf@hotmail.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
(cherry picked from commit 4900cad020c0580dfb1be27776ff10a4ef110cfa)
The SSDB provided by the firmware for the rear OV13858 camera
(OVTID858) of the Microsoft Surface Pro 9 reports degree=0, but the
module is mounted upside down, so the image comes out rotated 180
degrees. Add a DMI quirk entry for it, next to the existing one for
the front OVTI5693 camera of the same machine.

Tested on a Surface Pro 9: both cameras now report Rotation = 180 and
render upright in libcamera clients, with no regression on the front
camera.

Signed-off-by: José María Martín <jmmartinf@hotmail.com>
@jmmartinf
jmmartinf force-pushed the sp9-rear-camera-rotation branch from ec19192 to e357c42 Compare August 14, 2026 00:21
@jmmartinf

jmmartinf commented Aug 14, 2026

Copy link
Copy Markdown
Author

Hi @zR-JB, thanks for the pointer. Good to hear the SP12 has its rear sensor mounted upside down just like the SP9 does — same mounting, same fix, which makes this a lot less lonely a corner than it looked.

Small update on part 1 of this PR (the dmi_first_match() fix in ipu_bridge_parse_rotation()): it was picked up by linux-media on 2026-08-11. Sakari added a Cc: stable when applying it, and it carries a Fixes: tag for the original XPS quirk commit, so it should land in mainline in the next merge window and be backported to the stable kernels afterwards — nothing that needs to be carried downstream long-term.

On combining the devices into one entry: the entries themselves can't be folded together, unfortunately. Inside a single dmi_system_id the DMI_MATCH() slots are combined with AND — dmi_matches() walks the four slots and bails out on the first mismatch — and there is no OR combinator within an entry, so "product X or Y" isn't expressible. The OR lives between entries. That's why the Dell XPS machines are one entry per model — four of them in mainline — all sharing .driver_data = "OVTI02C1". One entry per model, all pointing at OVTID858, is already the most compact form the table allows.

Which leaves the question of SP10 and SP11. I'd rather not add entries for machines nobody has run the code on: the table is matched against the live machine, so a wrong entry doesn't fail loudly — it silently flips a camera that was fine. You tested the SP12 and I tested the SP9, and I think those two are what we can honestly claim today.

That said, the barrier is lower than it looks, so this is an open invitation to anyone on linux-surface/linux-surface#2144 with an SP10 or SP11. Finding out whether your machine needs it takes no building at allcam -l lists the cameras and their indices, and cam -c<N> -p prints the properties of one of them (redirect stderr, the useful output is on stdout). If the rear camera reports Rotation = 0 while its image comes out upside down, it needs the quirk. cat /sys/class/dmi/id/sys_vendor /sys/class/dmi/id/product_name then gives the exact strings the entry needs. Building only comes in for the "tested on" part: ipu-bridge compiles as a standalone module against kernel-surface-devel in a matter of seconds, so validating your own quirk is an afternoon at most, not a kernel rebuild. Happy to share the scripts I used and to review the patch before you send it — with the DMI strings and a "tested on" line, a one-entry addition like this is about as approachable as kernel contributions get.

Worth noting for #175: it works today without part 1 because the SP12 currently has a single entry in the table — the same situation as the Pro 10. As soon as a front-camera entry is added for the SP12, it would hit exactly the dmi_first_match() limitation this PR fixes, since the second entry for the same machine is unreachable. With the fix upstream, that case is already covered.

I've just refreshed this PR so that its first commit is byte-identical to the one that went into media.git — same code, same message, plus a (cherry picked from commit 4900cad020c0) line so its origin is on the record. That matters for whoever maintains this branch: when it is eventually rebased onto a kernel that already carries the upstream fix, git will recognise the commit by its patch-id and drop it silently, instead of producing a conflict. Before the refresh it carried the pre-review version of the patch and would have conflicted. Nothing else changed, and the resulting code is the same one I validated on the machine.

Both PRs insert just before the terminating entry, so whichever lands second needs a trivial rebase — happy to do that on my side. And once the upstream fix reaches this branch on its own, the first commit here can simply be dropped, leaving only the SP9 quirk; for now both are needed for the rear camera to come out upright.

For anyone who doesn't want to wait for the kernel to come around, ipu-bridge builds as a standalone module, so there's no need to rebuild a kernel:

  1. Install the matching kernel-surface-devel for the running kernel (this also repairs /lib/modules/$(uname -r)/build).
  2. Build just that one file against it — make -C /lib/modules/$(uname -r)/build M=<dir> modules — which takes seconds, not hours.
  3. If Secure Boot is on, sign the resulting ipu-bridge.ko with your enrolled MOK.
  4. Drop it into /lib/modules/$(uname -r)/updates/ and run depmod -a. updates/ takes precedence over kernel/, so the distro's own module is never touched; reverting is deleting one file.
  5. Reboot and check with cam -c2 -p that the rear sensor reports Rotation = 180.

It has to be redone after every kernel update, which is the whole reason for getting the quirk merged. ipu-bridge isn't in the initramfs and isn't needed to boot, so the worst case is a camera that doesn't initialise, not a machine that doesn't start. I have the four scripts I used for this (build+sign, install, test, revert) if they'd be useful to anyone.

@zR-JB

zR-JB commented Aug 14, 2026

Copy link
Copy Markdown

Sp12 and sp11 do not have the omnivision front sensor but a Sony one and I think this one ist not mounted upside down! So I think the two sensors in list issue does not matter here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants