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

[Hans de Goede]

Hi,

On 20-Jul-26 16:28, 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.
> 
> Signed-off-by: Jakob Berg Jespersen <dev@xxxxxxx>

Thank you for your patch. At first I was a bit worried about this
patch because sometimes people get confused that a camera is not
a mirror and they enabling mirroring because of this.

But testing on a Microsoft Surface Go with an older kernel shows
that indeed the image currently is mirrored, so this is another
OV sensor where the mirror bit in the format register is inverted:

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>

Regards,

Hans



> ---
>  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).
> +	 */
>  	return ov8865_update_bits(sensor, OV8865_FORMAT2_REG, bits,
> -				  enable ? bits : 0);
> +				  enable ? 0 : bits);
>  }
>  
>  /* Test Pattern */
>