sony_imx335: mirror and flip in the sensor, every mode

[openipc-ai]

.pfnMirrorFlip was NULL in the IMX335 driver, so applications flipped in VPSS instead. That costs low delay with flip and YUV compression with mirror. Majestic hands orientation to the sensor whenever pfnMirrorFlip is non-NULL, so with this change both features stay available.

The IMX335 can invert its own readout: HREVERSE (0x304E) mirrors and VREVERSE (0x304F) flips.

Unlike SP2305 (#244), the Bayer order is unchanged

Measured by comparing raw frames against the normal raw turned round. In every orientation of every mode, the best match is at an offset that keeps the colour order, and the per-CFA means don't change. So the ISP keeps RGGB, and nothing here touches SetPubAttr.

What does move is the field of view. The readout is larger than the picture, and the capture keeps the part that arrives first:

Mode	Shift per inverted axis
most modes	25 px or lines
2/2 binning	12
4M WDR windows	up to 33
2592x1520 crop, mirror	none

Registers

	normal	inverted
mirror 0x304E	0	1
flip 0x304F	0	1
AREA3_ST_ADR_1 (0x3074/75), all-pixel / crop / WDR	start	start + width + 8 (0x00B0→0x1010 full height, 0x0410→0x0CB0 1080p crop)
0x3081, 0x3083	0x02	0xFE (left alone, the raw saturates)
2/2 binning	start 0x00A8, its own 0x3080–0x3087, 0x30AD	start 0x1018, the datasheet's inverted table

• Each mode init notes its window.
• imx335_orientation_init() writes the group between REGHOLD (0x3001) = 1 and 0, so the switch lands on one frame. It retries a failed group once and always releases the hold.
• It runs at the end of IMX335_init, because every init writes 0x304E/0x304F back to normal. It also runs from the callback once the sensor is up.
• Unregister resets the orientation, so a new session starts unflipped.

Verified

A gk7205v300 + IMX335 and an hi3516ev300 + IMX335 were tested side by side, in the same room.

Modes: 5M 30 fps, 5M 40 fps, the 2592x1520 crop, the 1080p crop, the 1280x720 flexible crop, 2/2 binning, and line WDR at 5M, 4M and 2560x1440.

Coverage: every mode in all four orientations, set at start-up and toggled live, under three lights: a 6500 K LED panel, 3000 K bulbs, and the bulbs dimmed to 10 %.

Results:

• Registers read back as intended, and the hold is released.
• Against the same orientation done in VPSS, the picture matches apart from the shift above.
• Raw per-CFA means are within about 1 % of normal. The exception is 4 % in the 1280x720 crop, where the shift is a larger share of the window.
• No new encoder timeouts.
• Low delay stays on with flip.
• Right after an orientation change, AWB can land a few hundred kelvin away; it settles within seconds, and the raw colour doesn't move.

Both SDK codes (0x3516E200, 0x7205200) build.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Enable sensor-side mirror and flip for all IMX335 modes

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Route IMX335 orientation requests to the sensor, preserving low delay and YUV compression.

Diagram

graph TD
  A["Orientation request"] --> B{"Sensor initialized?"} -->|no| C["Saved orientation"] --> F["Mode initialization"] --> G["Window metadata"] --> D["Held register group"] --> E["IMX335 readout"]
  B -->|yes| G

Loading

High-Level Assessment

Sensor-side orientation is the appropriate approach: it retains low delay and YUV compression that the existing VPSS fallback can lose. Programming registers only in each mode initializer would not support live changes; the shared, register-held path handles both startup and runtime requests.

Files changed (2) +159 / -1

Enhancement (2) +159 / -1

imx335_cmos.cExpose and manage the IMX335 mirror/flip callback +24/-1

Expose and manage the IMX335 mirror/flip callback

• Registers a sensor mirror/flip callback that saves valid requests and applies them immediately when the sensor is initialized. Resets the saved orientation on successful unregister so the next session starts normally.

libraries/sensor/hi3516ev200/sony_imx335/imx335_cmos.c

imx335_sensor_ctl.cProgram orientation registers for every IMX335 mode +135/-0

Program orientation registers for every IMX335 mode

• Records each mode's readout window and applies mirror/flip settings after initialization or during live changes, including the distinct binning register table. Writes the register group under REGHOLD, retries a failed group, and attempts to release the hold even on failure.

libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A failed release can freeze sensor updates 🐞 Bug ☼ Reliability

Description

IMX335_init() discards a failed imx335_orientation_init() result and marks the sensor
initialized even if releasing the register hold failed. When both release attempts fail, exposure,
gain, and frame-length writes remain held while initialization proceeds.

Code

libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[380]

+	imx335_orientation_init(ViPipe); /* the init wrote 0x304E/0x304F back to normal */

Evidence

The new helper returns failure after two unsuccessful hold-release writes, and its own comment
states that a held sensor takes no exposure, gain, or VMAX updates. The new init call ignores that
result before setting bInit to true.

libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[262-283]
libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[379-381]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Initialization marks the sensor ready even when the new orientation group cannot release REGHOLD.
## Fix Focus Areas
- libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[271-283]
- libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[379-381]
## Recommended Fix
Handle the orientation result before setting `bInit`. If release fails, keep the sensor uninitialized and provide a recovery path that retries releasing the hold before normal operation resumes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Short writes can leave frames misoriented ✓ Resolved

Description

imx335_orientation_init() relies on IMX335_write_register() to detect failed group writes, but
that function reports success for any nonnegative result from a three-byte write. If a transaction
writes fewer than three bytes, the retry loop continues and can report success with only part of the
requested orientation programmed.

Code

libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[R267-269]

+		for (i = 0; i < u32Num && s32Ret == GK_SUCCESS; i++) {
+			s32Ret = IMX335_write_register(ViPipe, astGroup[i].u32Addr, astGroup[i].u32Data);
+		}

Evidence

The register writer sends a two-byte address and one-byte value but checks only whether write()
returned a negative number. The new group advances on GK_SUCCESS, so a short transfer bypasses its
retry path.

libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[127-148]
libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[265-275]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new orientation retry loop cannot detect an incomplete register write because the shared writer treats short transfers as successful.
## Fix Focus Areas
- libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[127-148]
- libraries/sensor/hi3516ev200/sony_imx335/imx335_sensor_ctl.c[265-269]
## Recommended Fix
Require `write()` to return the full address-plus-data length. Return failure for a shorter transfer so the orientation group retries rather than accepting a partially programmed state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗