diff mbox series

[v2,02/10] media: v4l2-ctrl: Document V4L2_CID_CAMERA_SENSOR_LOCATION

Message ID 20190827092339.8858-3-jacopo@jmondi.org (mailing list archive)
State New, archived
Headers show
Series media: Report camera sensor properties | expand

Commit Message

Jacopo Mondi Aug. 27, 2019, 9:23 a.m. UTC
Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera
control. The newly added read-only control reports the camera device
mounting position.

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

Comments

Laurent Pinchart Aug. 27, 2019, 12:50 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Aug 27, 2019 at 11:23:28AM +0200, Jacopo Mondi wrote:
> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera
> control. The newly added read-only control reports the camera device
> mounting position.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  .../media/uapi/v4l/ext-ctrls-camera.rst       | 34 +++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> index 51c1d5c9eb00..ecf151f3f0f4 100644
> --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> @@ -510,6 +510,40 @@ 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_SENSOR_LOCATION (integer)``
> +    This read-only control describes the camera sensor location by
> +    reporting its mounting position on the device where the camera is
> +    installed. The control value is constant and not modifiable by software
> +    and its value is retrieved from the firmware interface by parsing the
> +    'location' property.

How the value is known to the driver is irrelevant in this context, I
would drop "and its value ...".

> This control is particularly meaningful for
> +    devices which have a well defined orientation, such as phones, laptops
> +    and portable devices as the camera location is expressed as a position
> +    relative to the device intended usage orientation. In example, a camera

s/In example/For example/

> +    sensor installed on the user-facing side of a phone, a tablet or a
> +    laptop device is said to be installed in the ``V4L2_LOCATION_FRONT``
> +    location while camera sensors installed on the side opposed to the

s/opposed to the front one/opposite the front/

> +    front one are said to be installed in the ``V4L2_LOCATION_BACK``
> +    location. Camera sensors connected to the device by extension cables
> +    which are freely movable regardless of the device orientation, such as
> +    webcams and digital cameras, are said to be have
> +    ``V4L2_LOCATION_EXTERNAL`` location.

Same comment as for the DT bindings regarding cables.

> +
> +
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_LOCATION_FRONT``
> +      - The camera sensor is located on the front side of the device.
> +    * - ``V4L2_LOCATION_BACK``
> +      - The camera sensor is located on the back side of the device.
> +    * - ``V4L2_LOCATION_EXTERNAL``
> +      - The camera sensor is connected by extension cables to the device and
> +        it's freely movable.

s/it's/is/

> +
> +
> +
>  .. [#f1]
>     This control may be changed to a menu control in the future, if more
>     options are required.
Jacopo Mondi Aug. 29, 2019, 12:47 p.m. UTC | #2
Hi Laurent,

On Tue, Aug 27, 2019 at 03:50:08PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Aug 27, 2019 at 11:23:28AM +0200, Jacopo Mondi wrote:
> > Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera
> > control. The newly added read-only control reports the camera device
> > mounting position.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  .../media/uapi/v4l/ext-ctrls-camera.rst       | 34 +++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > index 51c1d5c9eb00..ecf151f3f0f4 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > @@ -510,6 +510,40 @@ 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_SENSOR_LOCATION (integer)``
> > +    This read-only control describes the camera sensor location by
> > +    reporting its mounting position on the device where the camera is
> > +    installed. The control value is constant and not modifiable by software
> > +    and its value is retrieved from the firmware interface by parsing the
> > +    'location' property.
>
> How the value is known to the driver is irrelevant in this context, I
> would drop "and its value ...".
>

I recall Sakari pointed out it was not mentioned the control value
comes from firmware in review of v1. Are you both ok in removing this?

> > This control is particularly meaningful for
> > +    devices which have a well defined orientation, such as phones, laptops
> > +    and portable devices as the camera location is expressed as a position
> > +    relative to the device intended usage orientation. In example, a camera
>
> s/In example/For example/
>
> > +    sensor installed on the user-facing side of a phone, a tablet or a
> > +    laptop device is said to be installed in the ``V4L2_LOCATION_FRONT``
> > +    location while camera sensors installed on the side opposed to the
>
> s/opposed to the front one/opposite the front/
>
> > +    front one are said to be installed in the ``V4L2_LOCATION_BACK``
> > +    location. Camera sensors connected to the device by extension cables
> > +    which are freely movable regardless of the device orientation, such as
> > +    webcams and digital cameras, are said to be have
> > +    ``V4L2_LOCATION_EXTERNAL`` location.
>
> Same comment as for the DT bindings regarding cables.
>
> > +
> > +
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_LOCATION_FRONT``
> > +      - The camera sensor is located on the front side of the device.
> > +    * - ``V4L2_LOCATION_BACK``
> > +      - The camera sensor is located on the back side of the device.
> > +    * - ``V4L2_LOCATION_EXTERNAL``
> > +      - The camera sensor is connected by extension cables to the device and
> > +        it's freely movable.
>
> s/it's/is/
>
> > +
> > +
> > +
> >  .. [#f1]
> >     This control may be changed to a menu control in the future, if more
> >     options are required.
>
> --
> Regards,
>
> Laurent Pinchart
diff mbox series

Patch

diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
index 51c1d5c9eb00..ecf151f3f0f4 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
@@ -510,6 +510,40 @@  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_SENSOR_LOCATION (integer)``
+    This read-only control describes the camera sensor location by
+    reporting its mounting position on the device where the camera is
+    installed. The control value is constant and not modifiable by software
+    and its value is retrieved from the firmware interface by parsing the
+    'location' property. This control is particularly meaningful for
+    devices which have a well defined orientation, such as phones, laptops
+    and portable devices as the camera location is expressed as a position
+    relative to the device intended usage orientation. In example, a camera
+    sensor installed on the user-facing side of a phone, a tablet or a
+    laptop device is said to be installed in the ``V4L2_LOCATION_FRONT``
+    location while camera sensors installed on the side opposed to the
+    front one are said to be installed in the ``V4L2_LOCATION_BACK``
+    location. Camera sensors connected to the device by extension cables
+    which are freely movable regardless of the device orientation, such as
+    webcams and digital cameras, are said to be have
+    ``V4L2_LOCATION_EXTERNAL`` location.
+
+
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - ``V4L2_LOCATION_FRONT``
+      - The camera sensor is located on the front side of the device.
+    * - ``V4L2_LOCATION_BACK``
+      - The camera sensor is located on the back side of the device.
+    * - ``V4L2_LOCATION_EXTERNAL``
+      - The camera sensor is connected by extension cables to the device and
+        it's freely movable.
+
+
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.