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

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

[Jacopo Mondi]

Hello,
   I've addressed Hans comments on naming and control registration.

I've moved "dt-bindings: Add media properties" to the end so that it can be left
out if controversial.


v10->v11
- s/V4L2_ORIENTATION/V4L2_CAMERA_ORIENTATION
- Minor adjustments to the controls registration

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
  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
  dt-bindings: Add media 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          |  61 ++-
 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, 753 insertions(+), 15 deletions(-)
 create mode 100644 include/dt-bindings/media/video-interfaces.h

--
2.26.1

[PATCH v11 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.

Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Tomasz Figa <tfiga@chromium.org>
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

[PATCH v11 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 v11 03/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..5d7fca799f2a8 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 (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_CAMERA_ORIENTATION_FRONT``
+    orientation, while a camera installed on the opposite side of the front one
+    is said to be have ``V4L2_CAMERA_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_CAMERA_ORIENTATION_EXTERNAL`` orientation.
+
+
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - ``V4L2_CAMERA_ORIENTATION_FRONT``
+      - The camera is oriented towards the user facing side of the device.
+    * - ``V4L2_CAMERA_ORIENTATION_BACK``
+      - The camera is oriented towards the back facing side of the device.
+    * - ``V4L2_CAMERA_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

[PATCH v11 04/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 5d7fca799f2a8..d9a117f75c9c8 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 v11 05/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 | 13 +++++++++++++
 include/uapi/linux/v4l2-controls.h   |  7 +++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 1c617b42a944d..92c3e39efc28f 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",
+		"Back",
+		"External",
+		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! */
@@ -1293,6 +1303,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_HEVC_LOOP_FILTER_MODE:
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:
+	case V4L2_CID_CAMERA_ORIENTATION:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_LINK_FREQ:
@@ -1482,6 +1493,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RDS_RX_TRAFFIC_ANNOUNCEMENT:
 	case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
 	case V4L2_CID_RDS_RX_MUSIC_SPEECH:
+	case V4L2_CID_CAMERA_ORIENTATION:
+	case V4L2_CID_CAMERA_SENSOR_ROTATION:
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_RF_TUNER_PLL_LOCK:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 0ba1005c96519..62271418c1bea 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_CAMERA_ORIENTATION_FRONT		0
+#define V4L2_CAMERA_ORIENTATION_BACK		1
+#define V4L2_CAMERA_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

[PATCH v11 06/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 v11 07/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 v11 08/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 92c3e39efc28f..4273d56dac653 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 v11 09/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 4273d56dac653..b188577db40f0 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 {					\
@@ -4621,3 +4622,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_CAMERA_ORIENTATION_FRONT;
+			break;
+		case V4L2_FWNODE_ORIENTATION_BACK:
+			orientation_ctrl = V4L2_CAMERA_ORIENTATION_BACK;
+			break;
+		case V4L2_FWNODE_ORIENTATION_EXTERNAL:
+			orientation_ctrl = V4L2_CAMERA_ORIENTATION_EXTERNAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (!v4l2_ctrl_new_std_menu(hdl, ctrl_ops,
+					    V4L2_CID_CAMERA_ORIENTATION,
+					    V4L2_CAMERA_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 v11 10/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 v11 11/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 v11 12/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

[PATCH v11 13/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>
---

I currently don't have users in mainline for this, I understand this implies
this is probably not going to be accepted. At the same time we don't have a
common place for media-related definitions, which support properties defined in
video-interfaces.txt

I leave it here at the end of the series for discussions, but I'm fine dropping
it from the series.

Thanks
  j

---
 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 v11 13/13] dt-bindings: Add media properties

[Hans Verkuil]

On 09/05/2020 11:04, 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>
> ---
> 
> I currently don't have users in mainline for this, I understand this implies
> this is probably not going to be accepted. At the same time we don't have a
> common place for media-related definitions, which support properties defined in
> video-interfaces.txt
> 
> I leave it here at the end of the series for discussions, but I'm fine dropping
> it from the series.
> 
> Thanks
>   j
> 
> ---
>  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>

Uh, shouldn't that be 0, 1 and 2 instead of <0>, <1> and <2> ?

I'm skipping this patch for the PR.

Regards,

	Hans

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

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

[Rob Herring]

On Sat, May 09, 2020 at 11:04:56AM +0200, 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>
> ---
> 
> I currently don't have users in mainline for this, I understand this implies
> this is probably not going to be accepted. At the same time we don't have a
> common place for media-related definitions, which support properties defined in
> video-interfaces.txt

Fine for me with no users if values already defined in binding.

> 
> I leave it here at the end of the series for discussions, but I'm fine dropping
> it from the series.
> 
> Thanks
>   j
> 
> ---
>  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 */

Dual license please.

> +/*
> + * 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 v11 13/13] dt-bindings: Add media properties

[Jacopo Mondi]

Hi Hans,

On Mon, May 11, 2020 at 09:27:57AM +0200, Hans Verkuil wrote:
> On 09/05/2020 11:04, 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>
> > ---
> >
> > I currently don't have users in mainline for this, I understand this implies
> > this is probably not going to be accepted. At the same time we don't have a
> > common place for media-related definitions, which support properties defined in
> > video-interfaces.txt
> >
> > I leave it here at the end of the series for discussions, but I'm fine dropping
> > it from the series.
> >
> > Thanks
> >   j
> >
> > ---
> >  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>
>
> Uh, shouldn't that be 0, 1 and 2 instead of <0>, <1> and <2> ?

I used that notation to be able to write
        orientation = FRONT_CAMERA
in place of
        orientation = <FRONT_CAMERA>

Do you think it's wrong ?

Thanks
  j

>
> I'm skipping this patch for the PR.
>
> Regards,
>
> 	Hans
>
> > +
> > +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
> > --
> > 2.26.1
> >
>

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

Hi!

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

Well.. except we have devices where camera is both front and back
according to this definiton.

https://www.gsmarena.com/samsung_galaxy_fold-9523.php

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

On Sat 2020-05-09 11:04:45, Jacopo Mondi wrote:
> 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.

So.. how is this supposed to work. There's a phone, with its main
camera. It is designed to be used in both portrait and landscape
modes, internal accelerometr can tell between the two.

Plus you have phone with a keyboard. Common usage is portrait with
keyboard closed, and landscape with keyboard open...

And yes, there's linux with v4l2 working on Nokia N900.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

[Jacopo Mondi]

[-- Attachment #1: Type: text/plain, Size: 3193 bytes --]

Pavel,

On Mon, May 25, 2020 at 01:16:21PM +0200, Pavel Machek wrote:
> On Sat 2020-05-09 11:04:45, Jacopo Mondi wrote:
> > 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.
>
> So.. how is this supposed to work. There's a phone, with its main
> camera. It is designed to be used in both portrait and landscape
> modes, internal accelerometr can tell between the two.
>
> Plus you have phone with a keyboard. Common usage is portrait with
> keyboard closed, and landscape with keyboard open...
>

While I agree for 'orientation' we're far from perfection (and that's
why I've been asked s/location/orientation/ to reserve 'location' for
the future), 'rotation' is only defined by the camera sensor mounting
rotation and the external scene. The way you move your phone should
not impact the pixel read-out sequence.

All phones can be used in portarait or landscape mode, I don't see why
having a mechanical keyboard is different. You can type in landscape
mode on a touchscreen keyboard, if you like to.

The property gives you the rotation correction (actually, the control
gives the correction) for display in the 'intended usage orientation'.
If your concern is displaying the image,  as you sense the
accellerometers to rotate the UI, the application that display the
image will be rotated too, adding to the corrected image (capture image +
rotation here specified) the required UI rotation.

On the 'intended usage orientation', as there is one for the UI, there
is one for all other components in the phone, so if your system
establishes that 'portrait' is the (rotation = 0) point for the UI,
this applies to the rest of the system.

> And yes, there's linux with v4l2 working on Nokia N900.
>

How is this and orientation handled there ?


> Best regards,
> 								Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Hans Verkuil]

On 20/05/2020 09:52, Jacopo Mondi wrote:
> Hi Hans,
> 
> On Mon, May 11, 2020 at 09:27:57AM +0200, Hans Verkuil wrote:
>> On 09/05/2020 11:04, 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>
>>> ---
>>>
>>> I currently don't have users in mainline for this, I understand this implies
>>> this is probably not going to be accepted. At the same time we don't have a
>>> common place for media-related definitions, which support properties defined in
>>> video-interfaces.txt
>>>
>>> I leave it here at the end of the series for discussions, but I'm fine dropping
>>> it from the series.
>>>
>>> Thanks
>>>   j
>>>
>>> ---
>>>  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>
>>
>> Uh, shouldn't that be 0, 1 and 2 instead of <0>, <1> and <2> ?
> 
> I used that notation to be able to write
>         orientation = FRONT_CAMERA
> in place of
>         orientation = <FRONT_CAMERA>
> 
> Do you think it's wrong ?

Yes, I think that's wrong :-)

From what I can see in existing device tree files you do not hide the '<>'.

Regards,

	Hans

> 
> Thanks
>   j
> 
>>
>> I'm skipping this patch for the PR.
>>
>> Regards,
>>
>> 	Hans
>>
>>> +
>>> +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
>>> --
>>> 2.26.1
>>>
>>