Re: [PATCH v2 2/2] media: i2c: ov5693: fix horizontal flip polarity and Bayer phase

["D. Manresa" <dmanresa@gmail.com>]

[Resent to the list at Jakob's request: the original, sent 29 Aug, reached
him alone - a mail tooling error on my side that dropped the Cc headers on
several replies that week, since fixed. Content unchanged. Jakob: yes, it
was always meant for the record - thanks for checking before quoting it.]

On Sat, 29 Aug 2026, Jakob Berg Jespersen wrote:
> One oddity I cannot explain: the vertical flip moves the phase by a
> column, not a row. [...] I report it as measured; I have no mechanism to
> offer for it.

I think I have the mechanism. All eight of your full-resolution states
reproduce here exactly - same phases, same two states that will not stream -
so this is a second unit agreeing with you, and then some extra measurements
that localise the column shift to one register bit.

Same setup as before: OV5693 at 2-0036 behind an IPU6, raw SBGGR10 off the
CSI-2 receiver, no ISP, no test pattern, registers written over i2c while
streaming, 40-frame captures.

1) The VFLIP output is a *pure* vertical mirror. Correlating each state
   against the reference frame and against its h-flipped, v-flipped and
   180-rotated copies (on 2x2-block means, so the metric is phase-blind)
   gives, for 0x3820=0x42 with the FLIP_HORZ bits set: vflip 0.996 against
   hflip 0.182. Nothing horizontal is being mirrored, so the column shift is
   not a hidden mirror.

2) The row phase does not move, and that part is correct behaviour. At
   VFLIP=0 the phase is GBRG, so row 0 is G B G B; at VFLIP=1 it is BGGR, so
   row 0 is B G B G. Both are the blue row: the sensor's one-row
   Bayer-preserving compensation is present and works. Only the column phase
   moves.

3) The sensor does not expose the shift. Reading 0x3800..0x3821 back in every
   state: CROP_START_X = 0x0010, CROP_START_Y = 0x0006, CROP_END_X = 0x0a30,
   CROP_END_Y = 0x079e, OFFSET_Y = 0, unchanged throughout.

4) Splitting 0x3820 into its two bits is what localises it. The driver writes
   FLIP_VERT_ISP_EN (bit 6) and FLIP_VERT_SENSOR_EN (bit 1) together; taken
   apart, at 0x3821=0x1e, offset 0:

     0x3820 = 0x40 (ISP bit alone):    no flip at all, phase unchanged (GBRG),
                                       image registers at (0,0) against the
                                       reference. A complete no-op on its own.
     0x3820 = 0x02 (sensor bit alone): flips the picture (vflip corr 0.997),
                                       and does NOT move the column phase -
                                       but the colour path then delivers R and
                                       B at the noise floor: plane means
                                       18.6 / 18.4, std 8.5, against 175 / 173
                                       and std 251 for the two green planes.
                                       Unusable alone.
     0x3820 = 0x42 (both):             flips, R and B are restored, and the
                                       column phase has moved (BGGR).

   So the ISP-side flip bit is the one that both rescues R/B and carries the
   spurious column shift. It behaves like the internal pipeline being told the
   mosaic moved, and moving it one column too far.

5) It really is a one-column window move, and it composes with one as an XOR.
   Calibrated against a known one-column change, at 2592x1944:

     FLIP_HORZ set (0x3821=0x1e), OFFSET_START_X=0:
       CROP_START_X 16:  VFLIP=0 GBRG    VFLIP=1 BGGR
       CROP_START_X 15:  VFLIP=0 BGGR    VFLIP=1 GBRG

     FLIP_HORZ cleared (0x3821=0x18), CROP_START_X=16:
       OFFSET_START_X 0: VFLIP=0 BGGR    VFLIP=1 GBRG
       OFFSET_START_X 1: VFLIP=0 GBRG    VFLIP=1 BGGR

   Pixel registration agrees: a per-colour-class-normalised, high-passed,
   15-frame-averaged correlation gives an envelope centre of -1.15 px for a
   deliberate CROP_START_X 16->15 move, -0.90 px for VFLIP, and 0.00 px for
   the self-comparison. Same signature, same magnitude, same sign.

Two by-products that bear on how a v3 could be written:

- Your two non-streaming states are a plain window overrun, not a flip
  interaction: CROP_END_X - CROP_START_X = 2608 - 16 = 2592, exactly the
  output width, so OFFSET_START_X=1 runs off the end. With CROP_START_X=15,
  the state 0x3820=0x42 / 0x3821=0x1e / OFFSET_START_X=1 streams cleanly here
  (40 frames) and decodes BGGR. Symmetrically, CROP_START_X=17 with
  OFFSET_START_X=0 stalls. So the state you needed is reachable after all -
  through the crop window rather than the ISP offset.

- With the FLIP_HORZ bits cleared, CROP_START_X has no effect on the phase at
  all (the window appears to be anchored at CROP_END_X in the mirrored
  readout); there, OFFSET_START_X is the knob that works. With the bits set it
  is the other way round. Any compensation therefore has to pick its register
  according to the horizontal state.

Measured combinations that give a clean BGGR at 2592x1944 on this unit:

    VFLIP=0, 0x3821=0x1e, CROP_START_X=15, OFFSET_START_X=0
    VFLIP=0, 0x3821=0x18, CROP_START_X=16, OFFSET_START_X=0
    VFLIP=1, 0x3821=0x1e, CROP_START_X=16, OFFSET_START_X=0
    VFLIP=1, 0x3821=0x18, CROP_START_X=16, OFFSET_START_X=1

On your plan: reporting the media bus code as a function of both flip
controls, with V4L2_CTRL_FLAG_MODIFY_LAYOUT, and inverting the polarity only
after that, sounds right to me, and matches what the numbers above say - the
phase is a function of both flips, so no HFLIP-keyed compensation can be
correct in both vertical states. It also has the advantage of not needing the
sensor to do anything it does not want to do. If you would rather compensate
than report, the tables above say it is possible, but it needs both flips as
inputs and a different register in each horizontal state, which is a lot of
machinery next to just telling userspace the truth.

For whatever it is worth downstream: my out-of-tree binned mode is unaffected
either way, since it keys its window offset on the mode rather than on the
flip controls.

Happy to test a v3, and to re-run any of the above on request - the harness is
a loop of i2c writes plus raw captures, so extra states are cheap.

D. Manresa <dmanresa@gmail.com>