On 8/15/19 4:34 PM, Jacopo Mondi wrote: > Hi Hans, > > On Thu, Aug 15, 2019 at 04:14:38PM +0200, Hans Verkuil wrote: >> On 8/15/19 4:10 PM, Hans Verkuil wrote: >>> On 8/15/19 12:43 AM, Laurent Pinchart wrote: >>>> Hi Jacopo, >>>> >>>> Thank you for the patch. >>>> >>>> On Wed, Aug 14, 2019 at 10:28:12PM +0200, Jacopo Mondi wrote: >>>>> Add documentation for the V4L2_CID_LOCATION camera control. The newly >>>>> added read-only control reports the camera device mounting position. >>>>> >>>>> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx> >>>>> --- >>>>> .../media/uapi/v4l/ext-ctrls-camera.rst | 23 +++++++++++++++++++ >>>>> 1 file changed, 23 insertions(+) >>>>> >>>>> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst >>>>> index 51c1d5c9eb00..fc0a02eee6d4 100644 >>>>> --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst >>>>> +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst >>>>> @@ -510,6 +510,29 @@ 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_LOCATION (integer)`` >>>> >>>> Maybe V4L2_CID_CAMERA_SENSOR_LOCATION ? Same for the values below. >>> >>> Probably a better name, if a bit long. But we might need other location >>> controls in the future (e.g. flash location), so CID_LOCATION is just too >>> generic. >> > > Thanks for the feedback. > >> Note that the location defines themselves can most likely be used with any >> LOCATION control, so V4L2_LOCATION_FRONT would be fine with any control. >> > > What do you think instead of the control type? Would a single integer > control do or an integer menu one would be better? I see merit in both > proposals actually... Single integer. It's read-only, so it just reports the location. It would be different if this was a writable control: then you need to know which locations are possible to set, and that requires a menu type. But it doesn't make sense to set the location from software. However, the location might change as a result of other changes: e.g. if the camera has motor control of the tilt and the tilt changes from forward facing to downward facing, then the driver might change the location from FRONT to DOWN. A convoluted example perhaps, but this is just brainstorming. Regards, Hans > > Once this is clarified, I can send a proper v1. > > Thanks > j > >> Regards, >> >> Hans >> >>> >>> Regards, >>> >>> Hans >>> >>>> >>>>> + This read-only control describes the camera location by reporting its >>>> >>>> Here too I would mention camera sensor instead of just camera (or >>>> possibly imaging sensor). >>>> >>>>> + mounting position on the device where the camera is installed. 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 position. In example, a camera installed on the user-facing side >>>>> + of a phone device is said to be installed in the ``V4L2_LOCATION_FRONT`` >>>>> + position. >>>> >>>> The DT bindings could use such an example :-) I would extend this to >>>> tablets and laptops. >>>> >>>>> + >>>>> + >>>>> + >>>> >>>> Do we need three blank lines ? >>>> >>>>> +.. flat-table:: >>>>> + :header-rows: 0 >>>>> + :stub-columns: 0 >>>>> + >>>>> + * - ``V4L2_LOCATION_FRONT`` >>>>> + - The camera device is located on the front side of the device. >>>>> + * - ``V4L2_LOCATION_BACK`` >>>>> + - The camera device is located on the back side of the device. >>>>> + >>>>> + >>>>> + >>>>> .. [#f1] >>>>> This control may be changed to a menu control in the future, if more >>>>> options are required. >>>> >>> >>
