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... 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. > >> > > >
Attachment:
signature.asc
Description: PGP signature
