Re: [PATCH RFC 1/2] media: i2c: ov8865: fix horizontal flip control polarity

[Sakari Ailus]

Hi Jacob,

Thanks for the set.

On Mon, Jul 20, 2026 at 04:28:40PM +0200, Jakob Berg Jespersen wrote:
> The ov8865's native readout is horizontally mirrored and the
> FORMAT2 FLIP_HORZ bits (reg 0x3821) un-mirror it: with the bits
> cleared the image is mirrored, with them set it is not. The driver
> maps V4L2_CID_HFLIP=1 to setting the bits, so requesting a flip
> produces an unflipped image and vice versa.
> 
> This is user-visible on the Surface Pro 7+ rear camera (mounted with a
> 180 degree rotation, SSDB degree=180): libcamera requests HFLIP=1+VFLIP=1
> to undo the mount rotation and gets a horizontally flipped image.
> 
> Verified by a live 4-state flip/image matrix on the streaming sensor:
> 
>   hflip=0 vflip=0 -> 180 degree rotation (both flips)
>   hflip=1 vflip=0 -> vertical flip only
>   hflip=0 vflip=1 -> correct image
>   hflip=1 vflip=1 -> horizontal flip only
> 
> (vertical flip = reflection over a horizontal mirror line, as on water;
> horizontal flip = reflection over a vertical mirror line, as in a mirror)
> 
> which is only consistent with an inverted HFLIP and a correct VFLIP.
> 
> Invert the polarity so HFLIP=0 yields the unflipped image.

Is this just about, looking at the patch, that the driver before this patch
effectively implements an inverted HFLIP control? I think the description
could be simplified.

> 
> Signed-off-by: Jakob Berg Jespersen <dev@xxxxxxx>
> ---
>  drivers/media/i2c/ov8865.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index c6d53c3d55ca..901ea7c16395 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -2204,8 +2204,14 @@ static int ov8865_flip_horz_configure(struct ov8865_sensor *sensor, bool enable)
>  	u8 bits = OV8865_FORMAT2_FLIP_HORZ_ISP_EN |
>  		  OV8865_FORMAT2_FLIP_HORZ_SENSOR_EN;
>  
> +	/*
> +	 * The sensor's native readout is horizontally mirrored; the
> +	 * FLIP_HORZ bits un-mirror it. Map the control so that HFLIP=0
> +	 * yields an unmirrored image (verified on Surface Pro 7+ rear
> +	 * camera by a live flip-control/image matrix test).
> +	 */

Just a comment saying this is inverted in the sensor considering HFLIP
should be enough here.

>  	return ov8865_update_bits(sensor, OV8865_FORMAT2_REG, bits,
> -				  enable ? bits : 0);
> +				  enable ? 0 : bits);
>  }
>  
>  /* Test Pattern */
> 

-- 
Regards,

Sakari Ailus