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

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

[Fabian Pfitzner]

There are cameras containing a mirror on their optical path e. g. when
mounted upside down.

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@pengutronix.de>
---
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@pengutronix.de>

[PATCH 1/2] media: dt-bindings: add flip properties

[Fabian Pfitzner]

There are cameras containing a mirror on their optical path e. g. when
mounted upside down.

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 as well.

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

Signed-off-by: Fabian Pfitzner <f.pfitzner@pengutronix.de>
---
 .../devicetree/bindings/media/video-interface-devices.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interface-devices.yaml b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
index cf7712ad297c0..36d766992b271 100644
--- a/Documentation/devicetree/bindings/media/video-interface-devices.yaml
+++ b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
@@ -383,6 +383,14 @@ properties:
                         |                    |
                         +--------------------+
 
+  flip-horizontal:
+    description: boolean to flip image horizontally
+    type: boolean
+
+  flip-vertical:
+    description: boolean to flip image vertically
+    type: boolean
+
   orientation:
     description:
       The orientation of a device (typically an image sensor or a flash LED)

-- 
2.39.5

[PATCH 2/2] media: v4l: fwnode: parse horizontal/vertical flip properties

[Fabian Pfitzner]

There are cameras containing a mirror on their optical path e. g. when
mounted upside down.

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 as well.

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

Signed-off-by: Fabian Pfitzner <f.pfitzner@pengutronix.de>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 3 +++
 include/media/v4l2-fwnode.h           | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index cb153ce42c45d..7fd0936fbda80 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -845,6 +845,9 @@ int v4l2_fwnode_device_parse(struct device *dev,
 		dev_dbg(dev, "device rotation: %u\n", val);
 	}
 
+	props->hflip = fwnode_property_read_bool(fwnode, "flip-horizontal");
+	props->vflip = fwnode_property_read_bool(fwnode, "flip-vertical");
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_device_parse);
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index f7c57c7765898..f8a51ab8317ae 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -75,10 +75,14 @@ enum v4l2_fwnode_orientation {
  * struct v4l2_fwnode_device_properties - fwnode device properties
  * @orientation: device orientation. See &enum v4l2_fwnode_orientation
  * @rotation: device rotation
+ * @hflip: device horizontal flip
+ * @vflip: device vertical flip
  */
 struct v4l2_fwnode_device_properties {
 	enum v4l2_fwnode_orientation orientation;
 	unsigned int rotation;
+	bool hflip;
+	bool vflip;
 };
 
 /**

-- 
2.39.5

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

[Jacopo Mondi]

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' ?

>
> 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@pengutronix.de>
> ---
> 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@pengutronix.de>
>

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

[Fabian Pfitzner]

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.
>
>> 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@pengutronix.de>
>> ---
>> 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@pengutronix.de>
>>
-- 
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    |

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

[Jacopo Mondi]

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

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

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@pengutronix.de>
> > > ---
> > > 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@pengutronix.de>
> > >
> --
> 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    |
>

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

[Fabian Pfitzner]

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 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.

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@pengutronix.de>
>>>> ---
>>>> 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@pengutronix.de>
>>>>
>> --
>> 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    |

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

[Jacopo Mondi]

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.

> 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@pengutronix.de>
> > > > > ---
> > > > > 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@pengutronix.de>
> > > > >
> > > --
> > > 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    |
>

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

[Fabian Pfitzner]

On 7/23/25 14:21, Jacopo Mondi 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" ?

Between the sensor and lenses (optical path) there might be a mirror 
that causes a flip of the image.
"Might be" because I do not know if its really something that comes from 
the hardware itself or whether its inside the firmware (for the camera 
I'm using right now).
Either way, I cannot change it. So my idea was to set it via a device 
tree property, such that the driver can "revert" the flip that was made.
I thought, that it might be useful for other drivers as well that face 
similar issues.

>
> 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.
>
>> 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.
Hmm, sure there might be more complex scenarios.
The device tree property is for developer who know that there will not 
be any changes in user space anymore (e. g. cameras that are fixed in place)
And even if, it is still possible to make modifications from user space 
(like for the rotation property). My changes are not enforce any 
restrictions regarding that it is just an optional attribute.
>
>> 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@pengutronix.de>
>>>>>> ---
>>>>>> 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@pengutronix.de>
>>>>>>
>>>> --
>>>> 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    |

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

[Dave Stevenson]

Hi Jacopo and Fabian

On Wed, 23 Jul 2025 at 13:21, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> 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?

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.

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@pengutronix.de>
> > > > > > ---
> > > > > > 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@pengutronix.de>
> > > > > >
> > > > --
> > > > 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    |
> >
>

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

[Fabian Pfitzner]

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@ideasonboard.com> 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@pengutronix.de>
>>>>>>> ---
>>>>>>> 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@pengutronix.de>
>>>>>>>
>>>>> --
>>>>> 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    |

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

[Dave Stevenson]

On Wed, 23 Jul 2025 at 14:48, Fabian Pfitzner <f.pfitzner@pengutronix.de> wrote:
>
> 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@ideasonboard.com> 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.

That's not a function of the sensor then. I'd interpreted what you'd
written as the sensor itself magically changed the readout order to
add flips based on how it was mounted.

I'll agree with Jacopo that it is up to userspace to set the required
flips based on information provided by the driver. Userspace could
choose to flip the displayed image when rendering instead, which may
be necessary if the sensor driver doesn't support flip controls.

Your second patch parses these new properties into struct
v4l2_fwnode_device_properties, but then does nothing further with
them. I would have expected similar handling to V4L2_CID_ORIENTATION
and V4L2_CID_ROTATION in v4l2_ctrl_new_fwnode_properties to convert
them into V4L2 controls. Trying to change the behaviour in the driver
would again require changes for each and every sensor driver.
It does run the risk of conflicting with rotation though, so needs
some careful thought and specification with regard operation order
(rot 90 + HFLIP != HFLIP + rot 90).

  Dave

> 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@pengutronix.de>
> >>>>>>> ---
> >>>>>>> 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@pengutronix.de>
> >>>>>>>
> >>>>> --
> >>>>> 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    |
>

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

[Laurent Pinchart]

On Wed, Jul 23, 2025 at 03:33:48PM +0100, Dave Stevenson wrote:
> On Wed, 23 Jul 2025 at 14:48, Fabian Pfitzner wrote:
> > On 7/23/25 15:00, Dave Stevenson wrote:
> > > On Wed, 23 Jul 2025 at 13:21, Jacopo Mondi wrote:
> > >> 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.
> 
> That's not a function of the sensor then. I'd interpreted what you'd
> written as the sensor itself magically changed the readout order to
> add flips based on how it was mounted.
> 
> I'll agree with Jacopo that it is up to userspace to set the required
> flips based on information provided by the driver. Userspace could
> choose to flip the displayed image when rendering instead, which may
> be necessary if the sensor driver doesn't support flip controls.
> 
> Your second patch parses these new properties into struct
> v4l2_fwnode_device_properties, but then does nothing further with
> them. I would have expected similar handling to V4L2_CID_ORIENTATION
> and V4L2_CID_ROTATION in v4l2_ctrl_new_fwnode_properties to convert
> them into V4L2 controls. Trying to change the behaviour in the driver
> would again require changes for each and every sensor driver.
> It does run the risk of conflicting with rotation though, so needs
> some careful thought and specification with regard operation order
> (rot 90 + HFLIP != HFLIP + rot 90).

I agree. If the optical path contains a mirror, that can be indicated in
DT, and should be reported to userspace through a new (and carefully
documented) control. Drivers must not flip automatically, the same way
they must not automatically rotate by 180° when the sensor is mounted
upside down.

> > 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@pengutronix.de>
> > >>>>>>> ---
> > >>>>>>> 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

-- 
Regards,

Laurent Pinchart

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

[Hans de Goede]

Hi,

On 23-Jul-25 12:09 PM, 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 is not what I want. My example above was misleading. The rotation fixes the "upside down" problem, but does not fix the flip.

Hmm, so is there anything in the optical path causing just
a horizontal flip ? 

Because if not it sounds like the driver is simply buggy and
is applying hflip by default / applying hflip while the hflip
control is set to 0.

We've had this case a couple of times before. So it might just
be that we need to fix the driver here ...

Regards,

Hans

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

[Hans de Goede]

Hi,

On 23-Jul-25 12:09 PM, 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 is not what I want. My example above was misleading. The rotation fixes the "upside down" problem, but does not fix the flip.

Hmm, so is there anything in the optical path causing just
a horizontal flip ? 

Because if not it sounds like the driver is simply buggy and
is applying hflip by default / applying hflip while the hflip
control is set to 0.

We've had this case a couple of times before. So it might just
be that we need to fix the driver here ...

Regards,

Hans