Surface Pro 9: Set correct front camera rotation

[gabrielstedman]

media: i2c: ov5693: add rotation control and apply 180° quirk for OVTI5693

The OVTI5693 module used in the Surface Pro 9 front camera is mounted upside‑down. libcamera ignores HFLIP/VFLIP and relies on V4L2_CID_CAMERA_SENSOR_ROTATION to determine sensor orientation, but the OV5693 driver did not expose this control.

This patch adds a rotation control to the driver and applies a 180° default for the OVTI5693 ACPI‑matched variant.

[qzed]

If I understand correctly, this would rotate all OV5693 senors, regardless of the actual device / mounting. Is there some way to get the mounting orientation from ACPI?

@djrscally maybe you know more?

[gabrielstedman]

I could remove the auto-rotation and leave the rotation control implemented if that's better?
It would be nice if this could be done automatically.

[gabrielstedman]

Switched default rotation to 0:

	ctrls->rotation = v4l2_ctrl_new_std(&ctrls->handler, ops,
					V4L2_CID_CAMERA_SENSOR_ROTATION, 0, 270, 90, 0);

[qzed]

Okay, looks good to me now.

I'm still a bit concerned about the rotation being applied for all OVTI5693 matches. I have the feeling that that somehow should be specified in some ACPI method, but that might need some reverse engineering to figure out. At least from the ACPI dumps it looks like the older devices all use the INT33BE match, so for now this should not create problems changing the orientation on those. I still want to give it a test first before I merge though.

[gabrielstedman]

I've set this up to use a parameter instead to set the default rotation, as it's the only way I managed to get it to work.
Setting the value itself has no-effect.
With this I just needed to have /etc/modprobe.d/ov5693.conf with:

options ov5693 sensor_default_rotation=180

And I can see:

v4l2-ctl --device /dev/v4l-subdev4 --list-ctrls

User Controls

                       exposure 0x00980911 (int)    : min=1 max=2070 step=1 default=1030 value=1030 flags=has-min-max
                horizontal_flip 0x00980914 (bool)   : default=0 value=0 flags=has-min-max
                  vertical_flip 0x00980915 (bool)   : default=0 value=0 flags=has-min-max

Camera Controls

             camera_orientation 0x009a0922 (menu)   : min=0 max=2 default=0 value=0 (Front) flags=read-only, has-min-max
         camera_sensor_rotation 0x009a0923 (int)    : min=0 max=270 step=90 default=180 value=180 flags=read-only, has-min-max

Image Source Controls

              vertical_blanking 0x009e0901 (int)    : min=4 max=63591 step=1 default=134 value=134 flags=has-min-max
            horizontal_blanking 0x009e0902 (int)    : min=96 max=96 step=1 default=96 value=96 flags=read-only, has-min-max
                  analogue_gain 0x009e0903 (int)    : min=1 max=127 step=1 default=8 value=8 flags=has-min-max

Image Processing Controls

                 link_frequency 0x009f0901 (intmenu): min=0 max=0 default=0 value=0 (419200000 0x18fc7c00) flags=read-only, has-min-max
                     pixel_rate 0x009f0902 (int64)  : min=0 max=167680000 step=1 default=167680000 value=167680000 flags=read-only, has-min-max
                   test_pattern 0x009f0903 (menu)   : min=0 max=3 default=0 value=0 (Disabled) flags=has-min-max
                   digital_gain 0x009f0905 (int)    : min=1 max=4095 step=1 default=1024 value=1024 flags=has-min-max

This seems more of a workaround than a long-term solution but at least it's a way to get the correct rotation.
As reported here INT33BE would also need this for SP8.

Feel free to close this if it's not up to standard but I thought I'd share in case anyone else wants to look into this.

[qzed]

Okay, for some reason this flips the front camera on my SB2 (same sensor) by default. With sensor_default_rotation=180 it looks correct. I'm not sure how the rotation is handled on the SB2, but I guess we'd want something similar here as well instead.

[gabrielstedman]

Okay, for some reason this flips the front camera on my SB2 (same sensor) by default. With sensor_default_rotation=180 it looks correct. I'm not sure how the rotation is handled on the SB2, but I guess we'd want something similar here as well instead.

Good catch so my patch causes regression in your SB2 I'm guessing because the firmware does return the correct rotation but my patch is overriding that.
In the case of SP9 the firmware doesn't seem to return rotation (I may be wrong).
I can amend this to only run this block:

/* Rotation */
	ctrls->rotation = v4l2_ctrl_new_std(&ctrls->handler, ops,
					V4L2_CID_CAMERA_SENSOR_ROTATION, 0, 270, 90, sensor_default_rotation);

If the parameter is explicitly set - should we try this?

[qzed]

Yeah, sounds like that should work.