[PATCH v10 00/13] media: report camera properties

[PATCH v10 00/13] media: report camera properties

[Jacopo Mondi]

Hello,
   this v10 is not just a rename s/location/orientation as the documentation
around the property has changed slightly as well and should probably be re-read.

Anyway, most notable change is indeed the usa of 'orientation' in place of
location, so that we have an 'orientation' DT property, a
V4L2_CID_CAMERA_ORIENTATION control id and V4L2_ORIENTATION_* control values.

A new patch 'dt-bindings: Add media properties' add an header to help DT users
by providing macros for the currently supported locations.

I've added a patch for a new sensor driver I have used for testing (imx219) and
made the V4L2_CID_CAMERA_ORIENTATION a menu control as it was meant to be.

I know the additional DT header could slow the series inclusion, I'm fine
leaving it out if it proves controversial.

The result of the two new controls inspected with v4l2-ctl -L for a camera
with EXTERNAL orientation and 180 degrees rotation is the following:

------------------------------------------------------------------------------------------------------------------
Camera Controls

             camera_orientation 0x009a0922 (menu)   : min=0 max=2 default=2 value=2 flags=read-only
				0: Front Camera
				1: Back Camera
				2: External Camera
         camera_sensor_rotation 0x009a0923 (int)    : min=180 max=180 step=1 default=180 value=180 flags=read-only
------------------------------------------------------------------------------------------------------------------

Thanks
   j

v9->v10:
- s/location/orientation and documentation update
- Add DT bindings header for media properties
- Make V4L2_CID_CAMERA_ORIENTATION a TYPE_MENU control
- Add patch for imx219

v8->v9:
- Rebased on media master which has moved media documentation

v7->v8:
- Add Rob's ack to 03/11
- Address Hans typographical comments in 03/11

Jacopo Mondi (13):
  dt-bindings: video-interfaces: Document 'orientation' property
  dt-bindings: video-interface: Replace 'rotation' description
  dt-bindings: Add media properties
  media: v4l2-ctrl: Document V4L2_CID_CAMERA_ORIENTATION
  media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION
  media: v4l2-ctrls: Add camera orientation and rotation
  media: v4l2-fwnode: Add helper to parse device properties
  include: v4l2-ctrl: Sort forward declarations
  media: v4l2-ctrls: Sort includes alphabetically
  media: v4l2-ctrls: Add helper to register properties
  media: i2c: ov5670: Parse and register properties
  media: i2c: ov13858: Parse and register properties
  media: i2c: imx219: Parse and register properties

 .../bindings/media/video-interfaces.txt       | 372 +++++++++++++++++-
 .../media/v4l/ext-ctrls-camera.rst            | 151 +++++++
 drivers/media/i2c/imx219.c                    |  12 +-
 drivers/media/i2c/ov13858.c                   |  13 +-
 drivers/media/i2c/ov5670.c                    |  14 +-
 drivers/media/v4l2-core/v4l2-ctrls.c          |  63 ++-
 drivers/media/v4l2-core/v4l2-fwnode.c         |  42 ++
 include/dt-bindings/media/video-interfaces.h  |  15 +
 include/media/v4l2-ctrls.h                    |  34 +-
 include/media/v4l2-fwnode.h                   |  47 +++
 include/uapi/linux/v4l2-controls.h            |   7 +
 11 files changed, 755 insertions(+), 15 deletions(-)
 create mode 100644 include/dt-bindings/media/video-interfaces.h

--
2.26.1

[PATCH v10 01/13] dt-bindings: video-interfaces: Document 'orientation' property

[Jacopo Mondi]

Add the 'orientation' device property, used to specify the device mounting
position. The property is particularly meaningful for mobile devices
with a well defined usage orientation.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 .../devicetree/bindings/media/video-interfaces.txt    | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index f884ada0bffc8..b1ff492c7da7a 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -89,6 +89,17 @@ Optional properties
   but a number of degrees counter clockwise. Typical values are 0 and 180
   (upside down).

+- orientation: The orientation of a device (typically an image sensor or a flash
+  LED) describing its mounting position relative to the usage orientation of the
+  system where the device is installed on.
+  Possible values are:
+  0 - Front. The device is mounted on the front facing side of the system.
+  For mobile devices such as smartphones, tablets and laptops the front side is
+  the user facing side.
+  1 - Back. The device is mounted on the back side of the system, which is
+  defined as the opposite side of the front facing one.
+  2 - External. The device is not attached directly to the system but is
+  attached in a way that allows it to move freely.

 Optional endpoint properties
 ----------------------------
--
2.26.1

Re: [PATCH v10 01/13] dt-bindings: video-interfaces: Document 'orientation' property

[Rob Herring]

On Fri,  8 May 2020 12:01:46 +0200, Jacopo Mondi wrote:
> Add the 'orientation' device property, used to specify the device mounting
> position. The property is particularly meaningful for mobile devices
> with a well defined usage orientation.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  .../devicetree/bindings/media/video-interfaces.txt    | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

Re: [PATCH v10 01/13] dt-bindings: video-interfaces: Document 'orientation' property

[Jacopo Mondi]

Hi Rob,

On Mon, May 11, 2020 at 12:20:08PM -0500, Rob Herring wrote:
> On Fri,  8 May 2020 12:01:46 +0200, Jacopo Mondi wrote:
> > Add the 'orientation' device property, used to specify the device mounting
> > position. The property is particularly meaningful for mobile devices
> > with a well defined usage orientation.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  .../devicetree/bindings/media/video-interfaces.txt    | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>

Yours and Tomasz's tags are back in v11, for which Hans has sent a
pull request.

Sorry I've missed them in v10, not sure what happened :)

Thanks
   j

> If a tag was not added on purpose, please state why and what changed.
>

[PATCH v10 02/13] dt-bindings: video-interface: Replace 'rotation' description

[Jacopo Mondi]

Replace the 'rotation' property description by providing a definition
relative to the camera sensor pixel array coordinate system and the
captured scene.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 .../bindings/media/video-interfaces.txt       | 359 +++++++++++++++++-
 1 file changed, 356 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index b1ff492c7da7a..3920f25a91235 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -85,9 +85,362 @@ Optional properties

 - lens-focus: A phandle to the node of the focus lens controller.

-- rotation: The device, typically an image sensor, is not mounted upright,
-  but a number of degrees counter clockwise. Typical values are 0 and 180
-  (upside down).
+- rotation: The camera rotation is expressed as the angular difference in
+  degrees between two reference systems, one relative to the camera module, and
+  one defined on the external world scene to be captured when projected on the
+  image sensor pixel array.
+
+  A camera sensor has a 2-dimensional reference system 'Rc' defined by
+  its pixel array read-out order. The origin is set to the first pixel
+  being read out, the X-axis points along the column read-out direction
+  towards the last columns, and the Y-axis along the row read-out
+  direction towards the last row.
+
+  A typical example for a sensor with a 2592x1944 pixel array matrix
+  observed from the front is:
+
+              2591       X-axis          0
+                <------------------------+ 0
+                .......... ... ..........!
+                .......... ... ..........! Y-axis
+                           ...           !
+                .......... ... ..........!
+                .......... ... ..........! 1943
+                                         V
+
+  The external world scene reference system 'Rs' is a 2-dimensional
+  reference system on the focal plane of the camera module. The origin is
+  placed on the top-left corner of the visible scene, the X-axis points
+  towards the right, and the Y-axis points towards the bottom of the
+  scene. The top, bottom, left and right directions are intentionally not
+  defined and depend on the environment in which the camera is used.
+
+  A typical example of a (very common) picture of a shark swimming from
+  left to right, as seen from the camera, is:
+
+               0               X-axis
+             0 +------------------------------------->
+               !
+               !
+               !
+               !           |\____)\___
+               !           ) _____  __`<
+               !           |/     )/
+               !
+               !
+               !
+               V
+             Y-axis
+
+  with the reference system 'Rs' placed on the camera focal plane:
+
+                                  ¸.·˙!
+                              ¸.·˙    !
+                  _       ¸.·˙        !
+               +-/ \-+¸.·˙            !
+               | (o) |                ! Camera focal plane
+               +-----+˙·.¸            !
+                          ˙·.¸        !
+                              ˙·.¸    !
+                                  ˙·.¸!
+
+  When projected on the sensor's pixel array, the image and the associated
+  reference system 'Rs' are typically (but not always) inverted, due to
+  the camera module's lens optical inversion effect.
+
+  Assuming the above represented scene of the swimming shark, the lens
+  inversion projects the scene and its reference system onto the sensor
+  pixel array, seen from the front of the camera sensor, as follows:
+
+            Y-axis
+               ^
+               !
+               !
+               !
+               !            |\_____)\__
+               !            ) ____  ___.<
+               !            |/    )/
+               !
+               !
+               !
+             0 +------------------------------------->
+               0               X-axis
+
+  Note the shark being upside-down.
+
+  The resulting projected reference system is named 'Rp'.
+
+  The camera rotation property is then defined as the angular difference
+  in the counter-clockwise direction between the camera reference system
+  'Rc' and the projected scene reference system 'Rp'. It is expressed in
+  degrees as a number in the range [0, 360[.
+
+  Examples
+
+  0 degrees camera rotation:
+
+
+                    Y-Rp
+                     ^
+              Y-Rc   !
+               ^     !
+               !     !
+               !     !
+               !     !
+               !     !
+               !     !
+               !     !
+               !     !
+               !   0 +------------------------------------->
+               !     0               X-Rp
+             0 +------------------------------------->
+               0               X-Rc
+
+
+                                X-Rc                0
+               <------------------------------------+ 0
+                           X-Rp                 0   !
+           <------------------------------------+ 0 !
+                                                !   !
+                                                !   !
+                                                !   !
+                                                !   !
+                                                !   !
+                                                !   !
+                                                !   !
+                                                !   V
+                                                !  Y-Rc
+                                                V
+                                               Y-Rp
+
+  90 degrees camera rotation:
+
+               0        Y-Rc
+             0 +-------------------->
+               !   Y-Rp
+               !    ^
+               !    !
+               !    !
+               !    !
+               !    !
+               !    !
+               !    !
+               !    !
+               !    !
+               !    !
+               !  0 +------------------------------------->
+               !    0              X-Rp
+               !
+               !
+               !
+               !
+               V
+              X-Rc
+
+  180 degrees camera rotation:
+
+                                            0
+       <------------------------------------+ 0
+                        X-Rc                !
+              Y-Rp                          !
+               ^                            !
+               !                            !
+               !                            !
+               !                            !
+               !                            !
+               !                            !
+               !                            !
+               !                            V
+               !                           Y-Rc
+             0 +------------------------------------->
+               0              X-Rp
+
+  270 degrees camera rotation:
+
+               0        Y-Rc
+             0 +-------------------->
+               !                                        0
+               !    <-----------------------------------+ 0
+               !                    X-Rp                !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        !
+               !                                        V
+               !                                       Y-Rp
+               !
+               !
+               !
+               !
+               V
+              X-Rc
+
+
+  Example one - Webcam
+
+  A camera module installed on the user facing part of a laptop screen
+  casing used for video calls. The captured images are meant to be
+  displayed in landscape mode (width > height) on the laptop screen.
+
+  The camera is typically mounted upside-down to compensate the lens
+  optical inversion effect:
+
+                    Y-Rp
+              Y-Rc   ^
+               ^     !
+               !     !
+               !     !       |\_____)\__
+               !     !       ) ____  ___.<
+               !     !       |/    )/
+               !     !
+               !     !
+               !     !
+               !   0 +------------------------------------->
+               !     0           X-Rp
+             0 +------------------------------------->
+               0            X-Rc
+
+  The two reference systems are aligned, the resulting camera rotation is
+  0 degrees, no rotation correction needs to be applied to the resulting
+  image once captured to memory buffers to correctly display it to users:
+
+               +--------------------------------------+
+               !                                      !
+               !                                      !
+               !                                      !
+               !             |\____)\___              !
+               !             ) _____  __`<            !
+               !             |/     )/                !
+               !                                      !
+               !                                      !
+               !                                      !
+               +--------------------------------------+
+
+  If the camera sensor is not mounted upside-down to compensate for the
+  lens optical inversion, the two reference systems will not be aligned,
+  with 'Rp' being rotated 180 degrees relatively to 'Rc':
+
+
+                        X-Rc                0
+       <------------------------------------+ 0
+                                            !
+              Y-Rp                          !
+               ^                            !
+               !                            !
+               !       |\_____)\__          !
+               !       ) ____  ___.<        !
+               !       |/    )/             !
+               !                            !
+               !                            !
+               !                            V
+               !                           Y-Rc
+             0 +------------------------------------->
+               0            X-Rp
+
+  The image once captured to memory will then be rotated by 180 degrees:
+
+               +--------------------------------------+
+               !                                      !
+               !                                      !
+               !                                      !
+               !              __/(_____/|             !
+               !            >.___  ____ (             !
+               !                 \(    \|             !
+               !                                      !
+               !                                      !
+               !                                      !
+               +--------------------------------------+
+
+  A software rotation correction of 180 degrees should be applied to
+  correctly display the image:
+
+               +--------------------------------------+
+               !                                      !
+               !                                      !
+               !                                      !
+               !             |\____)\___              !
+               !             ) _____  __`<            !
+               !             |/     )/                !
+               !                                      !
+               !                                      !
+               !                                      !
+               +--------------------------------------+
+
+  Example two - Phone camera
+
+  A camera installed on the back side of a mobile device facing away from
+  the user. The captured images are meant to be displayed in portrait mode
+  (height > width) to match the device screen orientation and the device
+  usage orientation used when taking the picture.
+
+  The camera sensor is typically mounted with its pixel array longer side
+  aligned to the device longer side, upside-down mounted to compensate for
+  the lens optical inversion effect:
+
+               0        Y-Rc
+             0 +-------------------->
+               !   Y-Rp
+               !    ^
+               !    !
+               !    !
+               !    !
+               !    !            |\_____)\__
+               !    !            ) ____  ___.<
+               !    !            |/    )/
+               !    !
+               !    !
+               !    !
+               !  0 +------------------------------------->
+               !    0                X-Rp
+               !
+               !
+               !
+               !
+               V
+              X-Rc
+
+  The two reference systems are not aligned and the 'Rp' reference
+  system is rotated by 90 degrees in the counter-clockwise direction
+  relatively to the 'Rc' reference system.
+
+  The image once captured to memory will be rotated:
+
+               +-------------------------------------+
+               |                 _ _                 |
+               |                \   /                |
+               |                 | |                 |
+               |                 | |                 |
+               |                 |  >                |
+               |                <  |                 |
+               |                 | |                 |
+               |                   .                 |
+               |                  V                  |
+               +-------------------------------------+
+
+  A correction of 90 degrees in counter-clockwise direction has to be
+  applied to correctly display the image in portrait mode on the device
+  screen:
+
+                        +--------------------+
+                        |                    |
+                        |                    |
+                        |                    |
+                        |                    |
+                        |                    |
+                        |                    |
+                        |   |\____)\___      |
+                        |   ) _____  __`<    |
+                        |   |/     )/        |
+                        |                    |
+                        |                    |
+                        |                    |
+                        |                    |
+                        |                    |
+                        +--------------------+

 - orientation: The orientation of a device (typically an image sensor or a flash
   LED) describing its mounting position relative to the usage orientation of the
--
2.26.1

[PATCH v10 03/13] dt-bindings: Add media properties

[Jacopo Mondi]

Add a DT header file to contain definitions for standard media properties.

The file is named after:
Documentation/devicetree/bindings/media/video-interfaces.txt
which contains the standard media properties definitions.

Initially add three macros to define the supported 'orientation'
property values.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/dt-bindings/media/video-interfaces.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 include/dt-bindings/media/video-interfaces.h

diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
new file mode 100644
index 0000000000000..404c697d6bd6e
--- /dev/null
+++ b/include/dt-bindings/media/video-interfaces.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * include/dt-bindings/media/video-interfaces.h
+ *
+ * Copyright (C) 2020 Jacopo Mondi <jacopo@jmondi.org>
+ */
+
+#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
+#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
+
+#define FRONT_CAMERA		<0>
+#define BACK_CAMERA		<1>
+#define EXTERNAL_CAMERA		<2>
+
+#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
--
2.26.1

Re: [PATCH v10 03/13] dt-bindings: Add media properties

[Hans Verkuil]

On 08/05/2020 12:01, Jacopo Mondi wrote:
> Add a DT header file to contain definitions for standard media properties.
> 
> The file is named after:
> Documentation/devicetree/bindings/media/video-interfaces.txt
> which contains the standard media properties definitions.
> 
> Initially add three macros to define the supported 'orientation'
> property values.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

For v11 just move this to the end of the series since it is an independent
patch.

> ---
>  include/dt-bindings/media/video-interfaces.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 include/dt-bindings/media/video-interfaces.h
> 
> diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
> new file mode 100644
> index 0000000000000..404c697d6bd6e
> --- /dev/null
> +++ b/include/dt-bindings/media/video-interfaces.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * include/dt-bindings/media/video-interfaces.h
> + *
> + * Copyright (C) 2020 Jacopo Mondi <jacopo@jmondi.org>
> + */
> +
> +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> +
> +#define FRONT_CAMERA		<0>
> +#define BACK_CAMERA		<1>
> +#define EXTERNAL_CAMERA		<2>

Wouldn't it be better to say CAMERA_FRONT (i.e. swap the words) or
even CAMERA_ORIENTATION_FRONT?

Regards,

	Hans

> +
> +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
> --
> 2.26.1
> 

Re: [PATCH v10 03/13] dt-bindings: Add media properties

[Jacopo Mondi]

Hi Hans,

On Fri, May 08, 2020 at 01:04:24PM +0200, Hans Verkuil wrote:
> On 08/05/2020 12:01, Jacopo Mondi wrote:
> > Add a DT header file to contain definitions for standard media properties.
> >
> > The file is named after:
> > Documentation/devicetree/bindings/media/video-interfaces.txt
> > which contains the standard media properties definitions.
> >
> > Initially add three macros to define the supported 'orientation'
> > property values.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> For v11 just move this to the end of the series since it is an independent
> patch.

Ack, I can leave it out as well, since I have no users if not a local
one for testing.

>
> > ---
> >  include/dt-bindings/media/video-interfaces.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >  create mode 100644 include/dt-bindings/media/video-interfaces.h
> >
> > diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
> > new file mode 100644
> > index 0000000000000..404c697d6bd6e
> > --- /dev/null
> > +++ b/include/dt-bindings/media/video-interfaces.h
> > @@ -0,0 +1,15 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * include/dt-bindings/media/video-interfaces.h
> > + *
> > + * Copyright (C) 2020 Jacopo Mondi <jacopo@jmondi.org>
> > + */
> > +
> > +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> > +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> > +
> > +#define FRONT_CAMERA		<0>
> > +#define BACK_CAMERA		<1>
> > +#define EXTERNAL_CAMERA		<2>
>
> Wouldn't it be better to say CAMERA_FRONT (i.e. swap the words) or
> even CAMERA_ORIENTATION_FRONT?

Once I wrote
                orientation = CAMERA_FRONT;

I realized that it was nicer to have

                orientation = FRONT_CAMERA:

I'll stop bikeshedding though and wait for other comments to see if
this can be included or not.

>
> Regards,
>
> 	Hans
>
> > +
> > +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
> > --
> > 2.26.1
> >
>

Re: [PATCH v10 03/13] dt-bindings: Add media properties

[Sakari Ailus]

Hi Jacopo, Hans,

On Sat, May 09, 2020 at 10:21:32AM +0200, Jacopo Mondi wrote:
> Hi Hans,
> 
> On Fri, May 08, 2020 at 01:04:24PM +0200, Hans Verkuil wrote:
> > On 08/05/2020 12:01, Jacopo Mondi wrote:
> > > Add a DT header file to contain definitions for standard media properties.
> > >
> > > The file is named after:
> > > Documentation/devicetree/bindings/media/video-interfaces.txt
> > > which contains the standard media properties definitions.
> > >
> > > Initially add three macros to define the supported 'orientation'
> > > property values.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > For v11 just move this to the end of the series since it is an independent
> > patch.
> 
> Ack, I can leave it out as well, since I have no users if not a local
> one for testing.
> 
> >
> > > ---
> > >  include/dt-bindings/media/video-interfaces.h | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >  create mode 100644 include/dt-bindings/media/video-interfaces.h
> > >
> > > diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
> > > new file mode 100644
> > > index 0000000000000..404c697d6bd6e
> > > --- /dev/null
> > > +++ b/include/dt-bindings/media/video-interfaces.h
> > > @@ -0,0 +1,15 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * include/dt-bindings/media/video-interfaces.h
> > > + *
> > > + * Copyright (C) 2020 Jacopo Mondi <jacopo@jmondi.org>
> > > + */
> > > +
> > > +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> > > +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> > > +
> > > +#define FRONT_CAMERA		<0>
> > > +#define BACK_CAMERA		<1>
> > > +#define EXTERNAL_CAMERA		<2>
> >
> > Wouldn't it be better to say CAMERA_FRONT (i.e. swap the words) or
> > even CAMERA_ORIENTATION_FRONT?
> 
> Once I wrote
>                 orientation = CAMERA_FRONT;
> 
> I realized that it was nicer to have
> 
>                 orientation = FRONT_CAMERA:
> 
> I'll stop bikeshedding though and wait for other comments to see if
> this can be included or not.

I'd be in favour of having "CAMERA" first. That gives it a nice prefix.

-- 
Kind regards,

Sakari Ailus

[PATCH v10 04/13] media: v4l2-ctrl: Document V4L2_CID_CAMERA_ORIENTATION

[Jacopo Mondi]

Add documentation for the V4L2_CID_CAMERA_ORIENTATION camera
control. The newly added read-only control reports the camera device
orientation relative to the usage orientation of the system the camera
is installed on.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 .../media/v4l/ext-ctrls-camera.rst            | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index e39f84d2447f8..01e104bab6b3d 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -510,6 +510,36 @@ enum v4l2_scene_mode -
     value down. A value of zero stops the motion if one is in progress
     and has no effect otherwise.
 
+``V4L2_CID_CAMERA_ORIENTATION (integer)``
+    This read-only control describes the camera orientation by reporting
+    its mounting position on the device where the camera is installed. The
+    control value is constant and not modifiable by software. This control is
+    particularly meaningful for devices which have a well defined orientation,
+    such as phones, laptops and portable devices since the control is expressed
+    as a position relative to the device's intended usage orientation.
+    For example, a camera installed on the user-facing side of a phone,
+    a tablet or a laptop device is said to be have ``V4L2_ORIENTATION_FRONT``
+    orientation, while a camera installed on the opposite side of the front one
+    is said to be have ``V4L2_ORIENTATION_BACK`` orientation. Camera sensors not
+    directly attached to the device, or attached in a way that allows them to
+    move freely, such as webcams and digital cameras, are said to have the
+    ``V4L2_ORIENTATION_EXTERNAL`` orientation.
+
+
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - ``V4L2_ORIENTATION_FRONT``
+      - The camera is oriented towards the user facing side of the device.
+    * - ``V4L2_ORIENTATION_BACK``
+      - The camera is oriented towards the back facing side of the device.
+    * - ``V4L2_ORIENTATION_EXTERNAL``
+      - The camera is not directly attached to the device and is freely movable.
+
+
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.
-- 
2.26.1

Re: [PATCH v10 04/13] media: v4l2-ctrl: Document V4L2_CID_CAMERA_ORIENTATION

[Hans Verkuil]

On 08/05/2020 12:01, Jacopo Mondi wrote:
> Add documentation for the V4L2_CID_CAMERA_ORIENTATION camera
> control. The newly added read-only control reports the camera device
> orientation relative to the usage orientation of the system the camera
> is installed on.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  .../media/v4l/ext-ctrls-camera.rst            | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index e39f84d2447f8..01e104bab6b3d 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -510,6 +510,36 @@ enum v4l2_scene_mode -
>      value down. A value of zero stops the motion if one is in progress
>      and has no effect otherwise.
>  
> +``V4L2_CID_CAMERA_ORIENTATION (integer)``

integer -> menu

> +    This read-only control describes the camera orientation by reporting
> +    its mounting position on the device where the camera is installed. The
> +    control value is constant and not modifiable by software. This control is
> +    particularly meaningful for devices which have a well defined orientation,
> +    such as phones, laptops and portable devices since the control is expressed
> +    as a position relative to the device's intended usage orientation.
> +    For example, a camera installed on the user-facing side of a phone,
> +    a tablet or a laptop device is said to be have ``V4L2_ORIENTATION_FRONT``
> +    orientation, while a camera installed on the opposite side of the front one
> +    is said to be have ``V4L2_ORIENTATION_BACK`` orientation. Camera sensors not
> +    directly attached to the device, or attached in a way that allows them to
> +    move freely, such as webcams and digital cameras, are said to have the
> +    ``V4L2_ORIENTATION_EXTERNAL`` orientation.
> +
> +
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_ORIENTATION_FRONT``

This really should be V4L2_CAMERA_ORIENTATION_FRONT. Yes, it is long, but just
'ORIENTATION' doesn't tell you which orientation is meant, that's too generic.

Regards,

	Hans

> +      - The camera is oriented towards the user facing side of the device.
> +    * - ``V4L2_ORIENTATION_BACK``
> +      - The camera is oriented towards the back facing side of the device.
> +    * - ``V4L2_ORIENTATION_EXTERNAL``
> +      - The camera is not directly attached to the device and is freely movable.
> +
> +
> +
>  .. [#f1]
>     This control may be changed to a menu control in the future, if more
>     options are required.
> 

[PATCH v10 05/13] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_ROTATION

[Jacopo Mondi]

Add documentation for the V4L2_CID_CAMERA_SENSOR_ROTATION camera
control. The newly added read-only control reports the rotation
correction to be applied to images before displaying them to the user.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 .../media/v4l/ext-ctrls-camera.rst            | 121 ++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index 01e104bab6b3d..6b618f3b51bf1 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -540,6 +540,127 @@ enum v4l2_scene_mode -
 
 
 
+``V4L2_CID_CAMERA_SENSOR_ROTATION (integer)``
+    This read-only control describes the rotation correction in degrees in the
+    counter-clockwise direction to be applied to the captured images once
+    captured to memory to compensate for the camera sensor mounting rotation.
+
+    For a precise definition of the sensor mounting rotation refer to the
+    extensive description of the 'rotation' properties in the device tree
+    bindings file 'video-interfaces.txt'.
+
+    A few examples are below reported, using a shark swimming from left to
+    right in front of the user as the example scene to capture. ::
+
+                 0               X-axis
+               0 +------------------------------------->
+                 !
+                 !
+                 !
+                 !           |\____)\___
+                 !           ) _____  __`<
+                 !           |/     )/
+                 !
+                 !
+                 !
+                 V
+               Y-axis
+
+    Example one - Webcam
+
+    Assuming you can bring your laptop with you while swimming with sharks,
+    the camera module of the laptop is installed on the user facing part of a
+    laptop screen casing, and is typically used for video calls. The captured
+    images are meant to be displayed in landscape mode (width > height) on the
+    laptop screen.
+
+    The camera is typically mounted upside-down to compensate the lens optical
+    inversion effect. In this case the value of the
+    V4L2_CID_CAMERA_SENSOR_ROTATION control is 0, no rotation is required to
+    display images correctly to the user.
+
+    If the camera sensor is not mounted upside-down it is required to compensate
+    the lens optical inversion effect and the value of the
+    V4L2_CID_CAMERA_SENSOR_ROTATION control is 180 degrees, as images will
+    result rotated when captured to memory. ::
+
+                 +--------------------------------------+
+                 !                                      !
+                 !                                      !
+                 !                                      !
+                 !              __/(_____/|             !
+                 !            >.___  ____ (             !
+                 !                 \(    \|             !
+                 !                                      !
+                 !                                      !
+                 !                                      !
+                 +--------------------------------------+
+
+    A software rotation correction of 180 degrees has to be applied to correctly
+    display the image on the user screen. ::
+
+                 +--------------------------------------+
+                 !                                      !
+                 !                                      !
+                 !                                      !
+                 !             |\____)\___              !
+                 !             ) _____  __`<            !
+                 !             |/     )/                !
+                 !                                      !
+                 !                                      !
+                 !                                      !
+                 +--------------------------------------+
+
+    Example two - Phone camera
+
+    It is more handy to go and swim with sharks with only your mobile phone
+    with you and take pictures with the camera that is installed on the back
+    side of the device, facing away from the user. The captured images are meant
+    to be displayed in portrait mode (height > width) to match the device screen
+    orientation and the device usage orientation used when taking the picture.
+
+    The camera sensor is typically mounted with its pixel array longer side
+    aligned to the device longer side, upside-down mounted to compensate for
+    the lens optical inversion effect.
+
+    The images once captured to memory will be rotated and the value of the
+    V4L2_CID_CAMERA_SENSOR_ROTATION will report a 90 degree rotation. ::
+
+
+                 +-------------------------------------+
+                 |                 _ _                 |
+                 |                \   /                |
+                 |                 | |                 |
+                 |                 | |                 |
+                 |                 |  >                |
+                 |                <  |                 |
+                 |                 | |                 |
+                 |                   .                 |
+                 |                  V                  |
+                 +-------------------------------------+
+
+    A correction of 90 degrees in counter-clockwise direction has to be
+    applied to correctly display the image in portrait mode on the device
+    screen. ::
+
+                          +--------------------+
+                          |                    |
+                          |                    |
+                          |                    |
+                          |                    |
+                          |                    |
+                          |                    |
+                          |   |\____)\___      |
+                          |   ) _____  __`<    |
+                          |   |/     )/        |
+                          |                    |
+                          |                    |
+                          |                    |
+                          |                    |
+                          |                    |
+                          +--------------------+
+
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.
-- 
2.26.1

[PATCH v10 06/13] media: v4l2-ctrls: Add camera orientation and rotation

[Jacopo Mondi]

Add support for the newly defined V4L2_CID_CAMERA_ORIENTATION
and V4L2_CID_CAMERA_SENSOR_ROTATION read-only controls used to report
the camera device mounting position and orientation respectively.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
 include/uapi/linux/v4l2-controls.h   |  7 +++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 1c617b42a944d..97765a57654d2 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -583,6 +583,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		"Annex B Start Code",
 		NULL,
 	};
+	static const char * const camera_orientation[] = {
+		"Front Camera",
+		"Back Camera",
+		"External Camera",
+		NULL,
+	};
 
 	switch (id) {
 	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -708,6 +714,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return hevc_decode_mode;
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
 		return hevc_start_code;
+	case V4L2_CID_CAMERA_ORIENTATION:
+		return camera_orientation;
 	default:
 		return NULL;
 	}
@@ -1020,6 +1028,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
 	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
 	case V4L2_CID_UNIT_CELL_SIZE:		return "Unit Cell Size";
+	case V4L2_CID_CAMERA_ORIENTATION:	return "Camera Orientation";
+	case V4L2_CID_CAMERA_SENSOR_ROTATION:	return "Camera Sensor Rotation";
 
 	/* FM Radio Modulator controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1295,6 +1305,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
+	case V4L2_CID_CAMERA_ORIENTATION:
+		*type = V4L2_CTRL_TYPE_MENU;
+		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
+		break;
 	case V4L2_CID_LINK_FREQ:
 		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
 		break;
@@ -1346,6 +1360,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
+	case V4L2_CID_CAMERA_SENSOR_ROTATION:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 0ba1005c96519..3da37c9cf5046 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -923,6 +923,13 @@ enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
 
+#define V4L2_CID_CAMERA_ORIENTATION		(V4L2_CID_CAMERA_CLASS_BASE+34)
+#define V4L2_ORIENTATION_FRONT			0
+#define V4L2_ORIENTATION_BACK			1
+#define V4L2_ORIENTATION_EXTERNAL		2
+
+#define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
+
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
-- 
2.26.1

Re: [PATCH v10 06/13] media: v4l2-ctrls: Add camera orientation and rotation

[Hans Verkuil]

On 08/05/2020 12:01, Jacopo Mondi wrote:
> Add support for the newly defined V4L2_CID_CAMERA_ORIENTATION
> and V4L2_CID_CAMERA_SENSOR_ROTATION read-only controls used to report
> the camera device mounting position and orientation respectively.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
>  include/uapi/linux/v4l2-controls.h   |  7 +++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 1c617b42a944d..97765a57654d2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -583,6 +583,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"Annex B Start Code",
>  		NULL,
>  	};
> +	static const char * const camera_orientation[] = {
> +		"Front Camera",
> +		"Back Camera",
> +		"External Camera",

You can drop 'Camera' here. The name of the control itself already specifies that
it is about the camera orientation, so that does (and should) not be repeated here.

> +		NULL,
> +	};
>  
>  	switch (id) {
>  	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -708,6 +714,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		return hevc_decode_mode;
>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
>  		return hevc_start_code;
> +	case V4L2_CID_CAMERA_ORIENTATION:
> +		return camera_orientation;
>  	default:
>  		return NULL;
>  	}
> @@ -1020,6 +1028,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
>  	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
>  	case V4L2_CID_UNIT_CELL_SIZE:		return "Unit Cell Size";
> +	case V4L2_CID_CAMERA_ORIENTATION:	return "Camera Orientation";
> +	case V4L2_CID_CAMERA_SENSOR_ROTATION:	return "Camera Sensor Rotation";
>  
>  	/* FM Radio Modulator controls */
>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1295,6 +1305,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
> +	case V4L2_CID_CAMERA_ORIENTATION:

Just move this up to just below 'case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE'
and add 'case V4L2_CID_CAMERA_ORIENTATION:' to the switch at the end of
this function that sets the READ_ONLY flag.

I.e. this function has two switches: one that sets the type and one at the
end that sets the flags. I see that the code is not entirely consistent anymore,
but we should try to keep to how it is supposed to be used.

> +		*type = V4L2_CTRL_TYPE_MENU;
> +		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +		break;
>  	case V4L2_CID_LINK_FREQ:
>  		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
>  		break;
> @@ -1346,6 +1360,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		break;
>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
>  	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:
> +	case V4L2_CID_CAMERA_SENSOR_ROTATION:
>  		*type = V4L2_CTRL_TYPE_INTEGER;
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;

Same here.

>  		break;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 0ba1005c96519..3da37c9cf5046 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -923,6 +923,13 @@ enum v4l2_auto_focus_range {
>  #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
>  #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
>  
> +#define V4L2_CID_CAMERA_ORIENTATION		(V4L2_CID_CAMERA_CLASS_BASE+34)
> +#define V4L2_ORIENTATION_FRONT			0
> +#define V4L2_ORIENTATION_BACK			1
> +#define V4L2_ORIENTATION_EXTERNAL		2
> +
> +#define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
> +
>  /* FM Modulator class control IDs */
>  
>  #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
> 

Regards,

	Hans

[PATCH v10 07/13] media: v4l2-fwnode: Add helper to parse device properties

[Jacopo Mondi]

Add an helper function to parse common device properties in the same
way as v4l2_fwnode_endpoint_parse() parses common endpoint properties.

Parse the 'rotation' and 'orientation' properties from the firmware
interface.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 42 ++++++++++++++++++++++++
 include/media/v4l2-fwnode.h           | 47 +++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 97f0f8b23b5dd..17b68cec8b083 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -756,6 +756,48 @@ int v4l2_fwnode_connector_add_link(struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_add_link);
 
+int v4l2_fwnode_device_parse(struct device *dev,
+			     struct v4l2_fwnode_device_properties *props)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	u32 val;
+	int ret;
+
+	memset(props, 0, sizeof(*props));
+
+	props->orientation = V4L2_FWNODE_PROPERTY_UNSET;
+	ret = fwnode_property_read_u32(fwnode, "orientation", &val);
+	if (!ret) {
+		switch (val) {
+		case V4L2_FWNODE_ORIENTATION_FRONT:
+		case V4L2_FWNODE_ORIENTATION_BACK:
+		case V4L2_FWNODE_ORIENTATION_EXTERNAL:
+			break;
+		default:
+			dev_warn(dev, "Unsupported device orientation: %u\n", val);
+			return -EINVAL;
+		}
+
+		props->orientation = val;
+		dev_dbg(dev, "device orientation: %u\n", val);
+	}
+
+	props->rotation = V4L2_FWNODE_PROPERTY_UNSET;
+	ret = fwnode_property_read_u32(fwnode, "rotation", &val);
+	if (!ret) {
+		if (val >= 360) {
+			dev_warn(dev, "Unsupported device rotation: %u\n", val);
+			return -EINVAL;
+		}
+
+		props->rotation = val;
+		dev_dbg(dev, "device rotation: %u\n", val);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_fwnode_device_parse);
+
 static int
 v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 					  struct v4l2_async_notifier *notifier,
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index dd82d6d9764ef..68c415fdaac53 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -109,6 +109,36 @@ struct v4l2_fwnode_endpoint {
 	unsigned int nr_of_link_frequencies;
 };
 
+/**
+ * V4L2_FWNODE_PROPERTY_UNSET - identify a non initialized property
+ *
+ * All properties in &struct v4l2_fwnode_device_properties are initialized
+ * to this value.
+ */
+#define V4L2_FWNODE_PROPERTY_UNSET   (-1U)
+
+/**
+ * enum v4l2_fwnode_orientation - possible device orientation
+ * @V4L2_FWNODE_ORIENTATION_FRONT: device installed on the front side
+ * @V4L2_FWNODE_ORIENTATION_BACK: device installed on the back side
+ * @V4L2_FWNODE_ORIENTATION_EXTERNAL: device externally located
+ */
+enum v4l2_fwnode_orientation {
+	V4L2_FWNODE_ORIENTATION_FRONT,
+	V4L2_FWNODE_ORIENTATION_BACK,
+	V4L2_FWNODE_ORIENTATION_EXTERNAL
+};
+
+/**
+ * struct v4l2_fwnode_device_properties - fwnode device properties
+ * @orientation: device orientation. See &enum v4l2_fwnode_orientation
+ * @rotation: device rotation
+ */
+struct v4l2_fwnode_device_properties {
+	enum v4l2_fwnode_orientation orientation;
+	unsigned int rotation;
+};
+
 /**
  * struct v4l2_fwnode_link - a link between two endpoints
  * @local_node: pointer to device_node of this endpoint
@@ -354,6 +384,23 @@ int v4l2_fwnode_connector_parse(struct fwnode_handle *fwnode,
 int v4l2_fwnode_connector_add_link(struct fwnode_handle *fwnode,
 				   struct v4l2_fwnode_connector *connector);
 
+/**
+ * v4l2_fwnode_device_parse() - parse fwnode device properties
+ * @dev: pointer to &struct device
+ * @props: pointer to &struct v4l2_fwnode_device_properties where to store the
+ *	   parsed properties values
+ *
+ * This function parses and validates the V4L2 fwnode device properties from the
+ * firmware interface, and fills the @struct v4l2_fwnode_device_properties
+ * provided by the caller.
+ *
+ * Return:
+ *	% 0 on success
+ *	%-EINVAL if a parsed property value is not valid
+ */
+int v4l2_fwnode_device_parse(struct device *dev,
+			     struct v4l2_fwnode_device_properties *props);
+
 /**
  * typedef parse_endpoint_func - Driver's callback function to be called on
  *	each V4L2 fwnode endpoint.
-- 
2.26.1

[PATCH v10 08/13] include: v4l2-ctrl: Sort forward declarations

[Jacopo Mondi]

Before adding a new forward declaration to the v4l2-ctrls.h header file,
sort the existing ones alphabetically.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/media/v4l2-ctrls.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 757a713bad412..224c10823bbff 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -25,14 +25,14 @@
 
 /* forward references */
 struct file;
+struct poll_table_struct;
+struct v4l2_ctrl;
 struct v4l2_ctrl_handler;
 struct v4l2_ctrl_helper;
-struct v4l2_ctrl;
-struct video_device;
+struct v4l2_fh;
 struct v4l2_subdev;
 struct v4l2_subscribed_event;
-struct v4l2_fh;
-struct poll_table_struct;
+struct video_device;
 
 /**
  * union v4l2_ctrl_ptr - A pointer to a control value.
-- 
2.26.1

[PATCH v10 09/13] media: v4l2-ctrls: Sort includes alphabetically

[Jacopo Mondi]

Before adding a new include directive, sort the existing ones in
alphabetical order.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 97765a57654d2..265452dae43bb 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -9,14 +9,14 @@
 #define pr_fmt(fmt) "v4l2-ctrls: " fmt
 
 #include <linux/ctype.h>
+#include <linux/export.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
-#include <linux/export.h>
-#include <media/v4l2-ioctl.h>
-#include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
-#include <media/v4l2-event.h>
 #include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
 
 #define dprintk(vdev, fmt, arg...) do {					\
 	if (!WARN_ON(!(vdev)) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
-- 
2.26.1

[PATCH v10 10/13] media: v4l2-ctrls: Add helper to register properties

[Jacopo Mondi]

Add an helper function to v4l2-ctrls to register controls associated
with a device property.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 40 ++++++++++++++++++++++++++++
 include/media/v4l2-ctrls.h           | 26 ++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 265452dae43bb..e4a679db55f09 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -16,6 +16,7 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-ioctl.h>
 
 #define dprintk(vdev, fmt, arg...) do {					\
@@ -4623,3 +4624,42 @@ __poll_t v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait)
 	return 0;
 }
 EXPORT_SYMBOL(v4l2_ctrl_poll);
+
+int v4l2_ctrl_new_fwnode_properties(struct v4l2_ctrl_handler *hdl,
+				    const struct v4l2_ctrl_ops *ctrl_ops,
+				    const struct v4l2_fwnode_device_properties *p)
+{
+	if (p->orientation != V4L2_FWNODE_PROPERTY_UNSET) {
+		u32 orientation_ctrl;
+
+		switch (p->orientation) {
+		case V4L2_FWNODE_ORIENTATION_FRONT:
+			orientation_ctrl = V4L2_ORIENTATION_FRONT;
+			break;
+		case V4L2_FWNODE_ORIENTATION_BACK:
+			orientation_ctrl = V4L2_ORIENTATION_BACK;
+			break;
+		case V4L2_FWNODE_ORIENTATION_EXTERNAL:
+			orientation_ctrl = V4L2_ORIENTATION_EXTERNAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (!v4l2_ctrl_new_std_menu(hdl, ctrl_ops,
+					    V4L2_CID_CAMERA_ORIENTATION,
+					    V4L2_ORIENTATION_EXTERNAL, 0,
+					    orientation_ctrl))
+			return hdl->error;
+	}
+
+	if (p->rotation != V4L2_FWNODE_PROPERTY_UNSET) {
+		if (!v4l2_ctrl_new_std(hdl, ctrl_ops,
+				       V4L2_CID_CAMERA_SENSOR_ROTATION,
+				       p->rotation, p->rotation, 1,
+				       p->rotation))
+			return hdl->error;
+	}
+
+	return hdl->error;
+}
+EXPORT_SYMBOL(v4l2_ctrl_new_fwnode_properties);
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 224c10823bbff..f40e2cbb21d34 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -30,6 +30,7 @@ struct v4l2_ctrl;
 struct v4l2_ctrl_handler;
 struct v4l2_ctrl_helper;
 struct v4l2_fh;
+struct v4l2_fwnode_device_properties;
 struct v4l2_subdev;
 struct v4l2_subscribed_event;
 struct video_device;
@@ -1428,4 +1429,29 @@ int v4l2_ctrl_subdev_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
  */
 int v4l2_ctrl_subdev_log_status(struct v4l2_subdev *sd);
 
+/**
+ * v4l2_ctrl_new_fwnode_properties() - Register controls for the device
+ *				       properties
+ *
+ * @hdl: pointer to &struct v4l2_ctrl_handler to register controls on
+ * @ctrl_ops: pointer to &struct v4l2_ctrl_ops to register controls with
+ * @p: pointer to &struct v4l2_fwnode_device_properties
+ *
+ * This function registers controls associated to device properties, using the
+ * property values contained in @p parameter, if the property has been set to
+ * a value.
+ *
+ * Currently the following v4l2 controls are parsed and registered:
+ * - V4L2_CID_CAMERA_ORIENTATION
+ * - V4L2_CID_CAMERA_SENSOR_ROTATION;
+ *
+ * Controls already registered by the caller with the @hdl control handler are
+ * not overwritten. Callers should register the controls they want to handle
+ * themselves before calling this function.
+ *
+ * Return: 0 on success, a negative error code on failure.
+ */
+int v4l2_ctrl_new_fwnode_properties(struct v4l2_ctrl_handler *hdl,
+				    const struct v4l2_ctrl_ops *ctrl_ops,
+				    const struct v4l2_fwnode_device_properties *p);
 #endif
-- 
2.26.1

[PATCH v10 11/13] media: i2c: ov5670: Parse and register properties

[Jacopo Mondi]

Parse device properties and register controls for them using the newly
introduced helpers.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5670.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 041fcbb4eebdf..f26252e35e08d 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -7,6 +7,7 @@
 #include <linux/pm_runtime.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 
 #define OV5670_REG_CHIP_ID		0x300a
 #define OV5670_CHIP_ID			0x005670
@@ -2059,6 +2060,8 @@ static const struct v4l2_ctrl_ops ov5670_ctrl_ops = {
 /* Initialize control handlers */
 static int ov5670_init_controls(struct ov5670 *ov5670)
 {
+	struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd);
+	struct v4l2_fwnode_device_properties props;
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	s64 vblank_max;
 	s64 vblank_def;
@@ -2067,7 +2070,7 @@ static int ov5670_init_controls(struct ov5670 *ov5670)
 	int ret;
 
 	ctrl_hdlr = &ov5670->ctrl_handler;
-	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
 	if (ret)
 		return ret;
 
@@ -2129,6 +2132,15 @@ static int ov5670_init_controls(struct ov5670 *ov5670)
 		goto error;
 	}
 
+	ret = v4l2_fwnode_device_parse(&client->dev, &props);
+	if (ret)
+		goto error;
+
+	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov5670_ctrl_ops,
+					      &props);
+	if (ret)
+		goto error;
+
 	ov5670->sd.ctrl_handler = ctrl_hdlr;
 
 	return 0;
-- 
2.26.1

[PATCH v10 12/13] media: i2c: ov13858: Parse and register properties

[Jacopo Mondi]

Parse device properties and register controls for them using the newly
introduced helpers.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov13858.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index aac6f77afa0f4..236ad2c816b70 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -7,6 +7,7 @@
 #include <linux/pm_runtime.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 
 #define OV13858_REG_VALUE_08BIT		1
 #define OV13858_REG_VALUE_16BIT		2
@@ -1589,6 +1590,7 @@ static const struct v4l2_subdev_internal_ops ov13858_internal_ops = {
 static int ov13858_init_controls(struct ov13858 *ov13858)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&ov13858->sd);
+	struct v4l2_fwnode_device_properties props;
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	s64 exposure_max;
 	s64 vblank_def;
@@ -1600,7 +1602,7 @@ static int ov13858_init_controls(struct ov13858 *ov13858)
 	int ret;
 
 	ctrl_hdlr = &ov13858->ctrl_handler;
-	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10);
 	if (ret)
 		return ret;
 
@@ -1666,6 +1668,15 @@ static int ov13858_init_controls(struct ov13858 *ov13858)
 		goto error;
 	}
 
+	ret = v4l2_fwnode_device_parse(&client->dev, &props);
+	if (ret)
+		goto error;
+
+	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov13858_ctrl_ops,
+					      &props);
+	if (ret)
+		goto error;
+
 	ov13858->sd.ctrl_handler = ctrl_hdlr;
 
 	return 0;
-- 
2.26.1

[PATCH v10 13/13] media: i2c: imx219: Parse and register properties

[Jacopo Mondi]

Parse device properties and register controls for them using the newly
introduced helpers.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/imx219.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index cb03bdec1f9c8..8f72ff5c1585e 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -1171,11 +1171,12 @@ static int imx219_init_controls(struct imx219 *imx219)
 	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	unsigned int height = imx219->mode->height;
+	struct v4l2_fwnode_device_properties props;
 	int exposure_max, exposure_def, hblank;
 	int i, ret;
 
 	ctrl_hdlr = &imx219->ctrl_handler;
-	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 11);
 	if (ret)
 		return ret;
 
@@ -1254,6 +1255,15 @@ static int imx219_init_controls(struct imx219 *imx219)
 		goto error;
 	}
 
+	ret = v4l2_fwnode_device_parse(&client->dev, &props);
+	if (ret)
+		goto error;
+
+	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &imx219_ctrl_ops,
+					      &props);
+	if (ret)
+		goto error;
+
 	imx219->sd.ctrl_handler = ctrl_hdlr;
 
 	return 0;
-- 
2.26.1