| Message ID | 20230406-feature-controls-lens-v2-5-faa8ad2bc404@wolfvision.net |
|---|---|
| State | New |
| Headers | show |
| Series | media: v4l2-ctrls: add controls for complex lens controller devices | expand |
On 25/04/2023 11:45, Michael Riesch wrote: > Add the controls V4L2_CID_LENS_CALIB_CONTROL and V4L2_CID_LENS_CALIB_STATUS > that facilitate the control of the lens group calibration procedure. > > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> > --- > .../userspace-api/media/v4l/ext-ctrls-camera.rst | 35 ++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 +++ > include/uapi/linux/v4l2-controls.h | 12 ++++++++ > 3 files changed, 51 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > index a17620ab03b9..8b54a0f3a617 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > @@ -297,6 +297,41 @@ enum v4l2_auto_focus_range - > (V4L2_CID_ZOOM_ABSOLUTE and V4L2_CID_ZOOM_RELATIVE). The unit is > driver-specific. The value should be a positive integer. > > +``V4L2_CID_LENS_CALIB_CONTROL (bitmask)`` > + Control the calibration procedure (or individual parts thereof) of the lens > + groups. For example, this could include the mechanical range detection > + of zoom lens motors. This is a write-only control. > + > +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_LENS_CALIB_STOP`` > + - Stop the lens calibration procedure. > + * - ``V4L2_LENS_CALIB_START`` > + - Start the complete lens calibration procedure. I don't like this as a bitmask control. Wouldn't it be better to have two 'button' controls? One to start the calibration, one to stop? Are more calibration control actions expected? > + > +``V4L2_CID_LENS_CALIB_STATUS (bitmask)`` > + The status of the calibration procedure (or individual parts thereof) of > + the lens groups. This is a read-only control. > + > +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_LENS_CALIB_IDLE`` > + - Lens calibration procedure has not yet been started. > + * - ``V4L2_LENS_CALIB_BUSY`` > + - Lens calibration procedure is in progress. > + * - ``V4L2_LENS_CALIB_COMPLETE`` > + - Lens calibration procedure is complete. > + * - ``V4L2_LENS_CALIB_FAILED`` > + - Lens calibration procedure has failed. > + > ``V4L2_CID_IRIS_ABSOLUTE (integer)`` > This control sets the camera's aperture to the specified value. The > unit is undefined. Larger values open the iris wider, smaller values > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > index 3ef465ba73bd..faddfecba6d9 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > @@ -1050,6 +1050,8 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_ZOOM_STATUS: return "Zoom, Status"; > case V4L2_CID_FOCUS_SPEED: return "Focus, Speed"; > case V4L2_CID_ZOOM_SPEED: return "Zoom, Speed"; > + case V4L2_CID_LENS_CALIB_CONTROL: return "Lens Calibration, Control"; > + case V4L2_CID_LENS_CALIB_STATUS: return "Lens Calibration, Status"; > > /* FM Radio Modulator controls */ > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > @@ -1596,6 +1598,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_FOCUS_RELATIVE: > case V4L2_CID_IRIS_RELATIVE: > case V4L2_CID_ZOOM_RELATIVE: > + case V4L2_CID_LENS_CALIB_CONTROL: > *flags |= V4L2_CTRL_FLAG_WRITE_ONLY | > V4L2_CTRL_FLAG_EXECUTE_ON_WRITE; > break; > @@ -1603,6 +1606,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_FOCUS_STATUS: > case V4L2_CID_ZOOM_CURRENT: > case V4L2_CID_ZOOM_STATUS: > + case V4L2_CID_LENS_CALIB_STATUS: > *flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE; It makes no sense that this is volatile. Besides the comments I made in patch 3/6, there is also the fact that I would expect the device to provide some sort of interrupt to indicate calibration status changes. Otherwise userspace would have to keep polling to know when the calibration finishes, which would be a really poor hardware design. Is this really the case for the hardware you are considering? > break; > case V4L2_CID_FLASH_STROBE_STATUS: > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index 8d84508d4db8..24c0eb5f4d29 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -1004,6 +1004,18 @@ enum v4l2_auto_focus_range { > #define V4L2_CID_FOCUS_SPEED (V4L2_CID_CAMERA_CLASS_BASE+41) > #define V4L2_CID_ZOOM_SPEED (V4L2_CID_CAMERA_CLASS_BASE+42) > > +#define V4L2_LENS_CALIB_STOP (0 << 0) > +#define V4L2_LENS_CALIB_START (1 << 0) > + > +#define V4L2_CID_LENS_CALIB_CONTROL (V4L2_CID_CAMERA_CLASS_BASE+43) > + > +#define V4L2_LENS_CALIB_IDLE (0 << 0) > +#define V4L2_LENS_CALIB_BUSY (1 << 0) > +#define V4L2_LENS_CALIB_COMPLETE (1 << 1) > +#define V4L2_LENS_CALIB_FAILED (1 << 2) > + > +#define V4L2_CID_LENS_CALIB_STATUS (V4L2_CID_CAMERA_CLASS_BASE+44) > + > /* FM Modulator class control IDs */ > > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900) > Regards, Hans
Hi Michael On Tue, 25 Apr 2023 at 10:45, Michael Riesch <michael.riesch@wolfvision.net> wrote: > > Add the controls V4L2_CID_LENS_CALIB_CONTROL and V4L2_CID_LENS_CALIB_STATUS > that facilitate the control of the lens group calibration procedure. > > Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> > --- > .../userspace-api/media/v4l/ext-ctrls-camera.rst | 35 ++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 +++ > include/uapi/linux/v4l2-controls.h | 12 ++++++++ > 3 files changed, 51 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > index a17620ab03b9..8b54a0f3a617 100644 > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst > @@ -297,6 +297,41 @@ enum v4l2_auto_focus_range - > (V4L2_CID_ZOOM_ABSOLUTE and V4L2_CID_ZOOM_RELATIVE). The unit is > driver-specific. The value should be a positive integer. > > +``V4L2_CID_LENS_CALIB_CONTROL (bitmask)`` > + Control the calibration procedure (or individual parts thereof) of the lens > + groups. For example, this could include the mechanical range detection > + of zoom lens motors. This is a write-only control. > + > +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_LENS_CALIB_STOP`` > + - Stop the lens calibration procedure. > + * - ``V4L2_LENS_CALIB_START`` > + - Start the complete lens calibration procedure. > + > +``V4L2_CID_LENS_CALIB_STATUS (bitmask)`` > + The status of the calibration procedure (or individual parts thereof) of > + the lens groups. This is a read-only control. > + > +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| > + > +.. flat-table:: > + :header-rows: 0 > + :stub-columns: 0 > + > + * - ``V4L2_LENS_CALIB_IDLE`` > + - Lens calibration procedure has not yet been started. Idle as a term would generally encompass both COMPLETE and FAILED as well as this case. Use V4L2_LENS_CALIB_UNCABLIBRATED or similar instead? > + * - ``V4L2_LENS_CALIB_BUSY`` > + - Lens calibration procedure is in progress. > + * - ``V4L2_LENS_CALIB_COMPLETE`` > + - Lens calibration procedure is complete. > + * - ``V4L2_LENS_CALIB_FAILED`` > + - Lens calibration procedure has failed. > + > ``V4L2_CID_IRIS_ABSOLUTE (integer)`` > This control sets the camera's aperture to the specified value. The > unit is undefined. Larger values open the iris wider, smaller values > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > index 3ef465ba73bd..faddfecba6d9 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c > @@ -1050,6 +1050,8 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_ZOOM_STATUS: return "Zoom, Status"; > case V4L2_CID_FOCUS_SPEED: return "Focus, Speed"; > case V4L2_CID_ZOOM_SPEED: return "Zoom, Speed"; > + case V4L2_CID_LENS_CALIB_CONTROL: return "Lens Calibration, Control"; > + case V4L2_CID_LENS_CALIB_STATUS: return "Lens Calibration, Status"; > > /* FM Radio Modulator controls */ > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > @@ -1596,6 +1598,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_FOCUS_RELATIVE: > case V4L2_CID_IRIS_RELATIVE: > case V4L2_CID_ZOOM_RELATIVE: > + case V4L2_CID_LENS_CALIB_CONTROL: > *flags |= V4L2_CTRL_FLAG_WRITE_ONLY | > V4L2_CTRL_FLAG_EXECUTE_ON_WRITE; > break; > @@ -1603,6 +1606,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, > case V4L2_CID_FOCUS_STATUS: > case V4L2_CID_ZOOM_CURRENT: > case V4L2_CID_ZOOM_STATUS: > + case V4L2_CID_LENS_CALIB_STATUS: > *flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE; > break; > case V4L2_CID_FLASH_STROBE_STATUS: > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index 8d84508d4db8..24c0eb5f4d29 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -1004,6 +1004,18 @@ enum v4l2_auto_focus_range { > #define V4L2_CID_FOCUS_SPEED (V4L2_CID_CAMERA_CLASS_BASE+41) > #define V4L2_CID_ZOOM_SPEED (V4L2_CID_CAMERA_CLASS_BASE+42) > > +#define V4L2_LENS_CALIB_STOP (0 << 0) > +#define V4L2_LENS_CALIB_START (1 << 0) Why a bitmask? What would setting both bits at the same time mean? > + > +#define V4L2_CID_LENS_CALIB_CONTROL (V4L2_CID_CAMERA_CLASS_BASE+43) > + > +#define V4L2_LENS_CALIB_IDLE (0 << 0) > +#define V4L2_LENS_CALIB_BUSY (1 << 0) > +#define V4L2_LENS_CALIB_COMPLETE (1 << 1) > +#define V4L2_LENS_CALIB_FAILED (1 << 2) Ditto. The status will only ever be one of these values, other than possibly COMPLETE | FAILED. Renaming COMPLETE to SUCCESS would remove that ambiguity and definitely make it one bit at a time. Dave > + > +#define V4L2_CID_LENS_CALIB_STATUS (V4L2_CID_CAMERA_CLASS_BASE+44) > + > /* FM Modulator class control IDs */ > > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900) > > -- > 2.37.2 >
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst index a17620ab03b9..8b54a0f3a617 100644 --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst @@ -297,6 +297,41 @@ enum v4l2_auto_focus_range - (V4L2_CID_ZOOM_ABSOLUTE and V4L2_CID_ZOOM_RELATIVE). The unit is driver-specific. The value should be a positive integer. +``V4L2_CID_LENS_CALIB_CONTROL (bitmask)`` + Control the calibration procedure (or individual parts thereof) of the lens + groups. For example, this could include the mechanical range detection + of zoom lens motors. This is a write-only control. + +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + + * - ``V4L2_LENS_CALIB_STOP`` + - Stop the lens calibration procedure. + * - ``V4L2_LENS_CALIB_START`` + - Start the complete lens calibration procedure. + +``V4L2_CID_LENS_CALIB_STATUS (bitmask)`` + The status of the calibration procedure (or individual parts thereof) of + the lens groups. This is a read-only control. + +.. tabularcolumns:: |p{6.8cm}|p{10.7cm}| + +.. flat-table:: + :header-rows: 0 + :stub-columns: 0 + + * - ``V4L2_LENS_CALIB_IDLE`` + - Lens calibration procedure has not yet been started. + * - ``V4L2_LENS_CALIB_BUSY`` + - Lens calibration procedure is in progress. + * - ``V4L2_LENS_CALIB_COMPLETE`` + - Lens calibration procedure is complete. + * - ``V4L2_LENS_CALIB_FAILED`` + - Lens calibration procedure has failed. + ``V4L2_CID_IRIS_ABSOLUTE (integer)`` This control sets the camera's aperture to the specified value. The unit is undefined. Larger values open the iris wider, smaller values diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c index 3ef465ba73bd..faddfecba6d9 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c @@ -1050,6 +1050,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_ZOOM_STATUS: return "Zoom, Status"; case V4L2_CID_FOCUS_SPEED: return "Focus, Speed"; case V4L2_CID_ZOOM_SPEED: return "Zoom, Speed"; + case V4L2_CID_LENS_CALIB_CONTROL: return "Lens Calibration, Control"; + case V4L2_CID_LENS_CALIB_STATUS: return "Lens Calibration, Status"; /* FM Radio Modulator controls */ /* Keep the order of the 'case's the same as in v4l2-controls.h! */ @@ -1596,6 +1598,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_FOCUS_RELATIVE: case V4L2_CID_IRIS_RELATIVE: case V4L2_CID_ZOOM_RELATIVE: + case V4L2_CID_LENS_CALIB_CONTROL: *flags |= V4L2_CTRL_FLAG_WRITE_ONLY | V4L2_CTRL_FLAG_EXECUTE_ON_WRITE; break; @@ -1603,6 +1606,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_FOCUS_STATUS: case V4L2_CID_ZOOM_CURRENT: case V4L2_CID_ZOOM_STATUS: + case V4L2_CID_LENS_CALIB_STATUS: *flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_VOLATILE; break; case V4L2_CID_FLASH_STROBE_STATUS: diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 8d84508d4db8..24c0eb5f4d29 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -1004,6 +1004,18 @@ enum v4l2_auto_focus_range { #define V4L2_CID_FOCUS_SPEED (V4L2_CID_CAMERA_CLASS_BASE+41) #define V4L2_CID_ZOOM_SPEED (V4L2_CID_CAMERA_CLASS_BASE+42) +#define V4L2_LENS_CALIB_STOP (0 << 0) +#define V4L2_LENS_CALIB_START (1 << 0) + +#define V4L2_CID_LENS_CALIB_CONTROL (V4L2_CID_CAMERA_CLASS_BASE+43) + +#define V4L2_LENS_CALIB_IDLE (0 << 0) +#define V4L2_LENS_CALIB_BUSY (1 << 0) +#define V4L2_LENS_CALIB_COMPLETE (1 << 1) +#define V4L2_LENS_CALIB_FAILED (1 << 2) + +#define V4L2_CID_LENS_CALIB_STATUS (V4L2_CID_CAMERA_CLASS_BASE+44) + /* FM Modulator class control IDs */ #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
Add the controls V4L2_CID_LENS_CALIB_CONTROL and V4L2_CID_LENS_CALIB_STATUS that facilitate the control of the lens group calibration procedure. Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> --- .../userspace-api/media/v4l/ext-ctrls-camera.rst | 35 ++++++++++++++++++++++ drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 +++ include/uapi/linux/v4l2-controls.h | 12 ++++++++ 3 files changed, 51 insertions(+)