* [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control
@ 2019-08-20 9:40 Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-08-20 9:40 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus,
linux-media, linux-kernel
Cc: Ricardo Ribalda Delgado
This control returns the unit cell size in nanometres. The struct provides
the width and the height in separated fields to take into consideration
asymmetric pixels and/or hardware binning.
This control is required for automatic calibration of sensors/cameras.
Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
include/media/v4l2-ctrls.h | 2 ++
include/uapi/linux/v4l2-controls.h | 3 +++
include/uapi/linux/videodev2.h | 11 +++++++++++
4 files changed, 27 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index cd1ae016706f..ec949453a081 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range";
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";
/* FM Radio Modulator controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
break;
+ case V4L2_CID_UNIT_CELL_SIZE:
+ *type = V4L2_CTRL_TYPE_AREA;
+ break;
default:
*type = V4L2_CTRL_TYPE_INTEGER;
break;
@@ -1423,6 +1427,7 @@ 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_UNIT_CELL_SIZE:
*flags |= V4L2_CTRL_FLAG_READ_ONLY;
break;
case V4L2_CID_RF_TUNER_PLL_LOCK:
@@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
case V4L2_CTRL_TYPE_FWHT_PARAMS:
break;
+ case V4L2_CTRL_TYPE_AREA:
+ break;
+
case V4L2_CTRL_TYPE_H264_SPS:
case V4L2_CTRL_TYPE_H264_PPS:
case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
@@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
break;
+ case V4L2_CTRL_TYPE_AREA:
+ elem_size = sizeof(struct v4l2_area);
+ break;
default:
if (type < V4L2_CTRL_COMPOUND_TYPES)
elem_size = sizeof(s32);
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 570ff4b0205a..9a3d11350e67 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -50,6 +50,7 @@ struct poll_table_struct;
* @p_h264_slice_params: Pointer to a struct v4l2_ctrl_h264_slice_params.
* @p_h264_decode_params: Pointer to a struct v4l2_ctrl_h264_decode_params.
* @p_vp8_frame_header: Pointer to a VP8 frame header structure.
+ * @p_area: Pointer to an area.
* @p: Pointer to a compound value.
*/
union v4l2_ctrl_ptr {
@@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
+ struct v4l2_area *p_area;
void *p;
};
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index a2669b79b294..1332eabd27b1 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -912,6 +912,9 @@ 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_UNIT_CELL_SIZE (V4L2_CID_CAMERA_CLASS_BASE+34)
+
+
/* FM Modulator class control IDs */
#define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2427bc4d8eba..751e2065ac02 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -422,6 +422,11 @@ struct v4l2_fract {
__u32 denominator;
};
+struct v4l2_area {
+ __u32 width;
+ __u32 height;
+};
+
/**
* struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
*
@@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_U8 = 0x0100,
V4L2_CTRL_TYPE_U16 = 0x0101,
V4L2_CTRL_TYPE_U32 = 0x0102,
+ /*
+ * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
+ * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
+ * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
+ */
+ V4L2_CTRL_TYPE_AREA = 0x0106,
};
/* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE
2019-08-20 9:40 [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Ricardo Ribalda Delgado
@ 2019-08-20 9:40 ` Ricardo Ribalda Delgado
2019-08-20 15:42 ` Philipp Zabel
2019-08-20 9:40 ` [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
2019-08-22 7:56 ` [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Hans Verkuil
2 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-08-20 9:40 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus,
linux-media, linux-kernel
Cc: Ricardo Ribalda Delgado
New control to pass to userspace the width/height of a pixel. Which is
needed for calibration and lens selection.
Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
index 51c1d5c9eb00..b43047d4e7a1 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
@@ -510,6 +510,14 @@ 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_UNIT_CELL_SIZE (struct)``
+ This control returns the unit cell size in nanometres. The struct provides
+ the width and the height in separated fields to take into consideration
+ asymmetric pixels and/or hardware binning.
+ The unit cell consist on the whole area of the pixel, sensitive and
+ non-sensitive.
+ This control is required for automatic calibration sensors/cameras.
+
.. [#f1]
This control may be changed to a menu control in the future, if more
options are required.
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE
2019-08-20 9:40 ` [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
@ 2019-08-20 15:42 ` Philipp Zabel
2019-08-21 16:40 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Philipp Zabel @ 2019-08-20 15:42 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, linux-media, linux-kernel
On Tue, 2019-08-20 at 11:40 +0200, Ricardo Ribalda Delgado wrote:
> New control to pass to userspace the width/height of a pixel. Which is
> needed for calibration and lens selection.
>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
> Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> index 51c1d5c9eb00..b43047d4e7a1 100644
> --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> @@ -510,6 +510,14 @@ 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_UNIT_CELL_SIZE (struct)``
> + This control returns the unit cell size in nanometres. The struct provides
> + the width and the height in separated fields to take into consideration
> + asymmetric pixels and/or hardware binning.
> + The unit cell consist on the whole area of the pixel, sensitive and
> + non-sensitive.
"consists of", otherwise this looks unambiguous to me.
I'm not sure if it is required to add a table for struct v4l2_area,
similarly to the other compound controls.
> + This control is required for automatic calibration sensors/cameras.
> +
> .. [#f1]
> This control may be changed to a menu control in the future, if more
> options are required.
regards
Philipp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE
2019-08-20 15:42 ` Philipp Zabel
@ 2019-08-21 16:40 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-08-21 16:40 UTC (permalink / raw)
To: Philipp Zabel
Cc: Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, linux-media, LKML
Hi Philipp
On Tue, Aug 20, 2019 at 5:42 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Tue, 2019-08-20 at 11:40 +0200, Ricardo Ribalda Delgado wrote:
> > New control to pass to userspace the width/height of a pixel. Which is
> > needed for calibration and lens selection.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> > ---
> > Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > index 51c1d5c9eb00..b43047d4e7a1 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-camera.rst
> > @@ -510,6 +510,14 @@ 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_UNIT_CELL_SIZE (struct)``
> > + This control returns the unit cell size in nanometres. The struct provides
> > + the width and the height in separated fields to take into consideration
> > + asymmetric pixels and/or hardware binning.
> > + The unit cell consist on the whole area of the pixel, sensitive and
> > + non-sensitive.
>
> "consists of", otherwise this looks unambiguous to me.
>
Thanks, fixing on v3. Will send after I get more feedback
> I'm not sure if it is required to add a table for struct v4l2_area,
> similarly to the other compound controls.
>
> > + This control is required for automatic calibration sensors/cameras.
> > +
> > .. [#f1]
> > This control may be changed to a menu control in the future, if more
> > options are required.
>
> regards
> Philipp
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
2019-08-20 9:40 [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
@ 2019-08-20 9:40 ` Ricardo Ribalda Delgado
2019-08-22 9:31 ` Philipp Zabel
2019-08-22 7:56 ` [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Hans Verkuil
2 siblings, 1 reply; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-08-20 9:40 UTC (permalink / raw)
To: Philipp Zabel, Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus,
linux-media, linux-kernel
Cc: Ricardo Ribalda Delgado
According to the product brief, the unit cell size is 1120 nanometers^2.
https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf
Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 159a3a604f0e..cc0a013ba7da 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -47,6 +47,7 @@ struct imx214 {
struct v4l2_ctrl *pixel_rate;
struct v4l2_ctrl *link_freq;
struct v4l2_ctrl *exposure;
+ struct v4l2_ctrl *unit_size;
struct regulator_bulk_data supplies[IMX214_NUM_SUPPLIES];
@@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
return ret;
}
+static void unit_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ ptr.p_area->width = 1120;
+ ptr.p_area->height = 1120;
+}
+
+static const struct v4l2_ctrl_type_ops unit_size_ops = {
+ .init = unit_size_init,
+};
+
+static struct v4l2_ctrl *new_unit_size_ctrl(struct v4l2_ctrl_handler *handler)
+{
+ static struct v4l2_ctrl_config ctrl = {
+ .id = V4L2_CID_UNIT_CELL_SIZE,
+ .type_ops = &unit_size_ops,
+ };
+
+ return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
+}
static int imx214_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
V4L2_CID_EXPOSURE,
0, 3184, 1, 0x0c70);
+ imx214->unit_size = new_unit_size_ctrl(&imx214->ctrls);
+
ret = imx214->ctrls.error;
if (ret) {
dev_err(&client->dev, "%s control init failed (%d)\n",
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
2019-08-20 9:40 ` [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
@ 2019-08-22 9:31 ` Philipp Zabel
2019-08-22 9:35 ` Ricardo Ribalda Delgado
0 siblings, 1 reply; 8+ messages in thread
From: Philipp Zabel @ 2019-08-22 9:31 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, linux-media, linux-kernel
On Tue, 2019-08-20 at 11:40 +0200, Ricardo Ribalda Delgado wrote:
> According to the product brief, the unit cell size is 1120 nanometers^2.
>
> https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf
>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
> drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 159a3a604f0e..cc0a013ba7da 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -47,6 +47,7 @@ struct imx214 {
> struct v4l2_ctrl *pixel_rate;
> struct v4l2_ctrl *link_freq;
> struct v4l2_ctrl *exposure;
> + struct v4l2_ctrl *unit_size;
This is never used.
Neither are pixel_rate and exposure, it appears. And link_freq is only
used locally in imx214_probe to set the read-only flag.
>
> struct regulator_bulk_data supplies[IMX214_NUM_SUPPLIES];
>
> @@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
> return ret;
> }
>
> +static void unit_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
> + union v4l2_ctrl_ptr ptr)
> +{
> + ptr.p_area->width = 1120;
> + ptr.p_area->height = 1120;
> +}
> +
> +static const struct v4l2_ctrl_type_ops unit_size_ops = {
> + .init = unit_size_init,
> +};
> +
> +static struct v4l2_ctrl *new_unit_size_ctrl(struct v4l2_ctrl_handler *handler)
> +{
> + static struct v4l2_ctrl_config ctrl = {
> + .id = V4L2_CID_UNIT_CELL_SIZE,
> + .type_ops = &unit_size_ops,
> + };
> +
> + return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
> +}
> static int imx214_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> @@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
> V4L2_CID_EXPOSURE,
> 0, 3184, 1, 0x0c70);
>
> + imx214->unit_size = new_unit_size_ctrl(&imx214->ctrls);
> +
> ret = imx214->ctrls.error;
> if (ret) {
> dev_err(&client->dev, "%s control init failed (%d)\n",
This seems like a lot of parts to assemble in every sensor driver just
to provide a constant area control. Should this be turned into a
v4l2_ctrl_new_area helper that takes a const struct v4l2_area as an
argument?
static const struct v4l2_area unit_cell_size = {
.width = 1120,
.height = 1120
};
v4l2_ctrl_new_area(&imx214->ctrls, V4L2_CID_UNIT_CELL_SIZE,
&unit_cell_size);
regards
Philipp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
2019-08-22 9:31 ` Philipp Zabel
@ 2019-08-22 9:35 ` Ricardo Ribalda Delgado
0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Ribalda Delgado @ 2019-08-22 9:35 UTC (permalink / raw)
To: Philipp Zabel
Cc: Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus, linux-media, LKML
Hi Philipp
On Thu, Aug 22, 2019 at 11:31 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Tue, 2019-08-20 at 11:40 +0200, Ricardo Ribalda Delgado wrote:
> > According to the product brief, the unit cell size is 1120 nanometers^2.
> >
> > https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> > ---
> > drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> > index 159a3a604f0e..cc0a013ba7da 100644
> > --- a/drivers/media/i2c/imx214.c
> > +++ b/drivers/media/i2c/imx214.c
> > @@ -47,6 +47,7 @@ struct imx214 {
> > struct v4l2_ctrl *pixel_rate;
> > struct v4l2_ctrl *link_freq;
> > struct v4l2_ctrl *exposure;
> > + struct v4l2_ctrl *unit_size;
>
> This is never used.
>
> Neither are pixel_rate and exposure, it appears. And link_freq is only
> used locally in imx214_probe to set the read-only flag.
>
> >
> > struct regulator_bulk_data supplies[IMX214_NUM_SUPPLIES];
> >
> > @@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
> > return ret;
> > }
> >
> > +static void unit_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
> > + union v4l2_ctrl_ptr ptr)
> > +{
> > + ptr.p_area->width = 1120;
> > + ptr.p_area->height = 1120;
> > +}
> > +
> > +static const struct v4l2_ctrl_type_ops unit_size_ops = {
> > + .init = unit_size_init,
> > +};
> > +
> > +static struct v4l2_ctrl *new_unit_size_ctrl(struct v4l2_ctrl_handler *handler)
> > +{
> > + static struct v4l2_ctrl_config ctrl = {
> > + .id = V4L2_CID_UNIT_CELL_SIZE,
> > + .type_ops = &unit_size_ops,
> > + };
> > +
> > + return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
> > +}
> > static int imx214_probe(struct i2c_client *client)
> > {
> > struct device *dev = &client->dev;
> > @@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
> > V4L2_CID_EXPOSURE,
> > 0, 3184, 1, 0x0c70);
> >
> > + imx214->unit_size = new_unit_size_ctrl(&imx214->ctrls);
> > +
> > ret = imx214->ctrls.error;
> > if (ret) {
> > dev_err(&client->dev, "%s control init failed (%d)\n",
>
> This seems like a lot of parts to assemble in every sensor driver just
> to provide a constant area control. Should this be turned into a
> v4l2_ctrl_new_area helper that takes a const struct v4l2_area as an
> argument?
I agree, I was planning on making a helper afterwards ;). I do not
mind adding it to this series.
>
> static const struct v4l2_area unit_cell_size = {
> .width = 1120,
> .height = 1120
> };
>
> v4l2_ctrl_new_area(&imx214->ctrls, V4L2_CID_UNIT_CELL_SIZE,
> &unit_cell_size);
>
> regards
> Philipp
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control
2019-08-20 9:40 [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
@ 2019-08-22 7:56 ` Hans Verkuil
2 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2019-08-22 7:56 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Philipp Zabel, Mauro Carvalho Chehab,
Sakari Ailus, linux-media, linux-kernel
On 8/20/19 11:40 AM, Ricardo Ribalda Delgado wrote:
> This control returns the unit cell size in nanometres. The struct provides
> the width and the height in separated fields to take into consideration
> asymmetric pixels and/or hardware binning.
> This control is required for automatic calibration of sensors/cameras.
>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
> include/media/v4l2-ctrls.h | 2 ++
> include/uapi/linux/v4l2-controls.h | 3 +++
> include/uapi/linux/videodev2.h | 11 +++++++++++
> 4 files changed, 27 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index cd1ae016706f..ec949453a081 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range";
> 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";
>
> /* FM Radio Modulator controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
> *type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
> break;
> + case V4L2_CID_UNIT_CELL_SIZE:
> + *type = V4L2_CTRL_TYPE_AREA;
> + break;
> default:
> *type = V4L2_CTRL_TYPE_INTEGER;
> break;
> @@ -1423,6 +1427,7 @@ 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_UNIT_CELL_SIZE:
> *flags |= V4L2_CTRL_FLAG_READ_ONLY;
> break;
> case V4L2_CID_RF_TUNER_PLL_LOCK:
> @@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> case V4L2_CTRL_TYPE_FWHT_PARAMS:
> break;
>
> + case V4L2_CTRL_TYPE_AREA:
> + break;
> +
> case V4L2_CTRL_TYPE_H264_SPS:
> case V4L2_CTRL_TYPE_H264_PPS:
> case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
> @@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
> elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
> break;
> + case V4L2_CTRL_TYPE_AREA:
> + elem_size = sizeof(struct v4l2_area);
> + break;
> default:
> if (type < V4L2_CTRL_COMPOUND_TYPES)
> elem_size = sizeof(s32);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 570ff4b0205a..9a3d11350e67 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -50,6 +50,7 @@ struct poll_table_struct;
> * @p_h264_slice_params: Pointer to a struct v4l2_ctrl_h264_slice_params.
> * @p_h264_decode_params: Pointer to a struct v4l2_ctrl_h264_decode_params.
> * @p_vp8_frame_header: Pointer to a VP8 frame header structure.
> + * @p_area: Pointer to an area.
> * @p: Pointer to a compound value.
> */
> union v4l2_ctrl_ptr {
> @@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
> struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
> struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
> struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
> + struct v4l2_area *p_area;
> void *p;
> };
>
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index a2669b79b294..1332eabd27b1 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -912,6 +912,9 @@ 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_UNIT_CELL_SIZE (V4L2_CID_CAMERA_CLASS_BASE+34)
> +
> +
> /* FM Modulator class control IDs */
>
> #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 2427bc4d8eba..751e2065ac02 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -422,6 +422,11 @@ struct v4l2_fract {
> __u32 denominator;
> };
>
> +struct v4l2_area {
> + __u32 width;
> + __u32 height;
> +};
> +
> /**
> * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
> *
> @@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
> V4L2_CTRL_TYPE_U8 = 0x0100,
> V4L2_CTRL_TYPE_U16 = 0x0101,
> V4L2_CTRL_TYPE_U32 = 0x0102,
> + /*
> + * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
> + * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
> + * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
> + */
> + V4L2_CTRL_TYPE_AREA = 0x0106,
This new type should be documented.
Regards,
Hans
> };
>
> /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-22 9:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 9:40 [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 2/3] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
2019-08-20 15:42 ` Philipp Zabel
2019-08-21 16:40 ` Ricardo Ribalda Delgado
2019-08-20 9:40 ` [PATCH v2 3/3] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE Ricardo Ribalda Delgado
2019-08-22 9:31 ` Philipp Zabel
2019-08-22 9:35 ` Ricardo Ribalda Delgado
2019-08-22 7:56 ` [PATCH v2 1/3] media: add V4L2_CID_UNIT_CELL_SIZE control Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).