Re: [PATCH 0/2] parse horizontal/vertical flip properties

From: Fabian Pfitzner
Date: Wed Jul 23 2025 - 09:48:46 EST

On 7/23/25 15:00, Dave Stevenson wrote:
Hi Jacopo and Fabian

On Wed, 23 Jul 2025 at 13:21, Jacopo Mondi
<jacopo.mondi@xxxxxxxxxxxxxxxx> wrote:
Hi Fabian

On Wed, Jul 23, 2025 at 12:09:58PM +0200, Fabian Pfitzner wrote:
On 7/23/25 11:44, Jacopo Mondi wrote:
On Wed, Jul 23, 2025 at 11:29:27AM +0200, Fabian Pfitzner wrote:
On 7/23/25 11:17, Jacopo Mondi wrote:
Hi Fabian

On Wed, Jul 23, 2025 at 10:58:28AM +0200, Fabian Pfitzner wrote:
There are cameras containing a mirror on their optical path e. g. when
mounted upside down.
How is this different from 'rotation = 180' ?
If you simply want to flip the output (e. g. horizontally), you cannot do
this with a rotation.
The camera I'm referring to is not only upside down, but also flipped
horizontally.
180 degress rotation = HFLIP + VFLIP
I do not want to do both. Only one of them.
Yes, you can't express 'mirror' in DTS, because DTS are about the
physical mounting rotation of the camera. Sensor drivers shall not
apply any flip control automatically, it's userspace that by parsing
the rotation property through the associated v4l2 controls should decide
if it has to apply flips or not to correct the images.

What is the use case you had in mind ? Tell the driver through a DTS
property it has to apply flips to auto-compensate ? Because I think we
shouldn't and if I'm not mistaken we also document it:
https://www.kernel.org/doc/html/latest/userspace-api/media/drivers/camera-sensor.html#rotation-orientation-and-flipping
I have a camera that does a horizontal flip in its hardware, so the output
Sorry, I don't want to be annoying, but what does it mean "does a
horizontal flip in the hardware" ?

In my understanding either "in hardware" means you can't control it
from software (and so there's no point in telling drivers what to do)
or you can control it from software and it's a regular HFLIP.
Can you say what this sensor/module is?
ClairPixel 8320

To change flips due to physical sensor orientation is a very unusual
one. That would imply some weird mechanics in the sensor to add the
mirror and some form of orientation sensor being built in.
Really? Imagine a door bell where an arbitrary camera is mounted such that it faces upwards (e. g. due to space limitations).
Then you need a mirror in order to point into the "correct" direction. Fixing the driver for an arbitrary camera driver does not seem to be a good solution.

The closest instance I can think of would be ov5647 where the sense of
the H & V flip register bits are in opposition, but that doesn't
change based on how the sensor is mounted.
In that case the driver just needs to account for it when programming
those registers [1]. And I now note that I haven't upstreamed the
patch adding flip controls - another one for the to-do list. The
hardcoded register set in the mainline driver sets HFLIP (0x3821 bit
2) but not VFLIP (0x3820 bit 2) [2].

Dave

[1] https://github.com/raspberrypi/linux/commit/9e5d3fd3f47e91806a5c26f96732284f39098a58
[2] https://elixir.bootlin.com/linux/v6.15.7/source/drivers/media/i2c/ov5647.c#L153

is not what I want. My example above was misleading. The rotation fixes the
"upside down" problem, but does not fix the flip.

Doing that in userspace might be a solution, but in my opinion it is a bit
ugly to write a script that always sets the flip property from userspace
when the device was started.
A much cleaner way would be to simply set this property in the device tree
such that the driver can be initially configured with the proper values.
Sorry, don't agree here. What if a sensor is mounted 90/270 degrees
rotated (typical for mobile devices in example) ? You can't compensate
it completely with flips, would you 270+HFLIP=90 ? would you leave it
unmodified ? Userspace has to know and act accordingly, doing things
in driver (will all drivers behave the same ? Will some compensate or
other won't ?) is a recipe for more complex behaviours to handle.

PS: I have to send this email twice. The first one contained HTML parts that
were rejected by some receivers...

TL;DR drivers shall not flip, userspace should. Mirroring is an effect
of drivers applying an HFLIP, because unless I'm missing something
obvious, 'mirror' is not a physical mounting configuration of the camera
sensor.

FIY we're talking about something similar in libcamera
https://lists.libcamera.org/pipermail/libcamera-devel/2025-July/051533.html

Introduce two options to change the device's flip property via device tree.

As there is already support for the panel-common driver [1], add it for cameras in the same way.

[1] commit 3c0ecd83eee9 ("dt-bindings: display: panel: Move flip properties to panel-common")

Signed-off-by: Fabian Pfitzner <f.pfitzner@xxxxxxxxxxxxxx>
---
Fabian Pfitzner (2):
media: dt-bindings: add flip properties
media: v4l: fwnode: parse horizontal/vertical flip properties

.../devicetree/bindings/media/video-interface-devices.yaml | 8 ++++++++
drivers/media/v4l2-core/v4l2-fwnode.c | 3 +++
include/media/v4l2-fwnode.h | 4 ++++
3 files changed, 15 insertions(+)
---
base-commit: 6832a9317eee280117cd695fa885b2b7a7a38daf
change-id: 20250718-fpf-media-dt-flip-7fcad30bcfb7

Best regards,
--
Fabian Pfitzner <f.pfitzner@xxxxxxxxxxxxxx>

--
Pengutronix e.K. | Fabian Pfitzner |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |

--
Pengutronix e.K. | Fabian Pfitzner |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |


--
Pengutronix e.K. | Fabian Pfitzner |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |