[PATCH v3 1/5] media: v4l2_ctrl: Add p_def to v4l2_ctrl_config

[PATCH v3 1/5] media: v4l2_ctrl: Add p_def to v4l2_ctrl_config

[Ricardo Ribalda Delgado]

This allows setting the default value on compound controls created via
v4l2_ctrl_new_custom.

Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 2 +-
 include/media/v4l2-ctrls.h           | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index b4caf2d4d076..73d99c3561ce 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2690,7 +2690,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
 			type, min, max,
 			is_menu ? cfg->menu_skip_mask : step, def,
 			cfg->dims, cfg->elem_size,
-			flags, qmenu, qmenu_int, ptr_null, priv);
+			flags, qmenu, qmenu_int, cfg->p_def, priv);
 	if (ctrl)
 		ctrl->is_private = cfg->is_private;
 	return ctrl;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index e719d56fc024..78a97b10c89e 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -382,6 +382,7 @@ struct v4l2_ctrl_handler {
  * @max:	The control's maximum value.
  * @step:	The control's step value for non-menu controls.
  * @def:	The control's default value.
+ * @p_def:	The control's default value for compound controls.
  * @dims:	The size of each dimension.
  * @elem_size:	The size in bytes of the control.
  * @flags:	The control's flags.
@@ -410,6 +411,7 @@ struct v4l2_ctrl_config {
 	s64 max;
 	u64 step;
 	s64 def;
+	union v4l2_ctrl_ptr p_def;
 	u32 dims[V4L2_CTRL_MAX_DIMS];
 	u32 elem_size;
 	u32 flags;
-- 
2.24.0.rc1

[PATCH v3 2/5] media: v4l2_ctrl: Add const pointer to ctrl_ptr

[Ricardo Ribalda Delgado]

This pointer is used to point to data that is constant. Thanks to this
we can avoid a lot of casting and we make more clear when the data is
constant or variable.

Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
 include/media/v4l2-ctrls.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 78a97b10c89e..7db9e719a583 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -56,6 +56,7 @@ struct poll_table_struct;
  * @p_hevc_slice_params:	Pointer to an HEVC slice parameters structure.
  * @p_area:			Pointer to an area.
  * @p:				Pointer to a compound value.
+ * @p_const:			Pointer to a constant compound value.
  */
 union v4l2_ctrl_ptr {
 	s32 *p_s32;
@@ -78,6 +79,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
 	struct v4l2_area *p_area;
 	void *p;
+	const void *p_const;
 };
 
 /**
-- 
2.24.0.rc1

Re: [PATCH v3 2/5] media: v4l2_ctrl: Add const pointer to ctrl_ptr

[Hans Verkuil]

Hi Ricardo,

On 11/1/19 12:23 PM, Ricardo Ribalda Delgado wrote:
> This pointer is used to point to data that is constant. Thanks to this
> we can avoid a lot of casting and we make more clear when the data is
> constant or variable.
> 
> Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
>  include/media/v4l2-ctrls.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 78a97b10c89e..7db9e719a583 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -56,6 +56,7 @@ struct poll_table_struct;
>   * @p_hevc_slice_params:	Pointer to an HEVC slice parameters structure.
>   * @p_area:			Pointer to an area.
>   * @p:				Pointer to a compound value.
> + * @p_const:			Pointer to a constant compound value.
>   */
>  union v4l2_ctrl_ptr {
>  	s32 *p_s32;
> @@ -78,6 +79,7 @@ union v4l2_ctrl_ptr {
>  	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
>  	struct v4l2_area *p_area;
>  	void *p;
> +	const void *p_const;
>  };
>  
>  /**
> 

This addition makes it possible to use const void pointers elsewhere in
the control framework.

E.g. in std_equal you can use p_const in the memcmp at the end.

Can you go through the v4l2-ctrls.c source and replace .p by .p_const
where it makes sense?

Obviously this would be a separate patch.

Regards,

	Hans

[PATCH v3 3/5] media: vivid: Add an area control

[Ricardo Ribalda Delgado]

This control represents a generic read/write area.

Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
 drivers/media/platform/vivid/vivid-ctrls.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index b250fc3764e2..fd35863798a7 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -32,6 +32,7 @@
 #define VIVID_CID_U32_ARRAY		(VIVID_CID_CUSTOM_BASE + 8)
 #define VIVID_CID_U16_MATRIX		(VIVID_CID_CUSTOM_BASE + 9)
 #define VIVID_CID_U8_4D_ARRAY		(VIVID_CID_CUSTOM_BASE + 10)
+#define VIVID_CID_AREA			(VIVID_CID_CUSTOM_BASE + 11)
 
 #define VIVID_CID_VIVID_BASE		(0x00f00000 | 0xf000)
 #define VIVID_CID_VIVID_CLASS		(0x00f00000 | 1)
@@ -266,6 +267,18 @@ static const struct v4l2_ctrl_config vivid_ctrl_disconnect = {
 	.type = V4L2_CTRL_TYPE_BUTTON,
 };
 
+static const struct v4l2_area area = {
+	.width = 0xcafe,
+	.height = 0xb1b1d,
+};
+
+static const struct v4l2_ctrl_config vivid_ctrl_area = {
+	.ops = &vivid_user_gen_ctrl_ops,
+	.id = VIVID_CID_AREA,
+	.name = "Area",
+	.type = V4L2_CTRL_TYPE_AREA,
+	.p_def.p_const = &area,
+};
 
 /* Framebuffer Controls */
 
@@ -1574,6 +1587,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
 	dev->string = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_string, NULL);
 	dev->bitmask = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_bitmask, NULL);
 	dev->int_menu = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_int_menu, NULL);
+	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u8_4d_array, NULL);
-- 
2.24.0.rc1

Re: [PATCH v3 3/5] media: vivid: Add an area control

[Hans Verkuil]

On 11/1/19 12:23 PM, Ricardo Ribalda Delgado wrote:
> This control represents a generic read/write area.
> 
> Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
>  drivers/media/platform/vivid/vivid-ctrls.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> index b250fc3764e2..fd35863798a7 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -32,6 +32,7 @@
>  #define VIVID_CID_U32_ARRAY		(VIVID_CID_CUSTOM_BASE + 8)
>  #define VIVID_CID_U16_MATRIX		(VIVID_CID_CUSTOM_BASE + 9)
>  #define VIVID_CID_U8_4D_ARRAY		(VIVID_CID_CUSTOM_BASE + 10)
> +#define VIVID_CID_AREA			(VIVID_CID_CUSTOM_BASE + 11)
>  
>  #define VIVID_CID_VIVID_BASE		(0x00f00000 | 0xf000)
>  #define VIVID_CID_VIVID_CLASS		(0x00f00000 | 1)
> @@ -266,6 +267,18 @@ static const struct v4l2_ctrl_config vivid_ctrl_disconnect = {
>  	.type = V4L2_CTRL_TYPE_BUTTON,
>  };
>  
> +static const struct v4l2_area area = {
> +	.width = 0xcafe,
> +	.height = 0xb1b1d,

I don't think there is any need for these weird values. Just set this to
e.g. 1000x2000. Just as long as width and height are different.

Regards,

	Hans

> +};
> +
> +static const struct v4l2_ctrl_config vivid_ctrl_area = {
> +	.ops = &vivid_user_gen_ctrl_ops,
> +	.id = VIVID_CID_AREA,
> +	.name = "Area",
> +	.type = V4L2_CTRL_TYPE_AREA,
> +	.p_def.p_const = &area,
> +};
>  
>  /* Framebuffer Controls */
>  
> @@ -1574,6 +1587,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
>  	dev->string = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_string, NULL);
>  	dev->bitmask = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_bitmask, NULL);
>  	dev->int_menu = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_int_menu, NULL);
> +	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL);
>  	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL);
>  	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL);
>  	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u8_4d_array, NULL);
> 

Re: [PATCH v3 3/5] media: vivid: Add an area control

[Ricardo Ribalda Delgado]

Hi hans

On Mon, Nov 4, 2019 at 10:44 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 11/1/19 12:23 PM, Ricardo Ribalda Delgado wrote:
> > This control represents a generic read/write area.
> >
> > Suggested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> > ---
> >  drivers/media/platform/vivid/vivid-ctrls.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
> > index b250fc3764e2..fd35863798a7 100644
> > --- a/drivers/media/platform/vivid/vivid-ctrls.c
> > +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> > @@ -32,6 +32,7 @@
> >  #define VIVID_CID_U32_ARRAY          (VIVID_CID_CUSTOM_BASE + 8)
> >  #define VIVID_CID_U16_MATRIX         (VIVID_CID_CUSTOM_BASE + 9)
> >  #define VIVID_CID_U8_4D_ARRAY                (VIVID_CID_CUSTOM_BASE + 10)
> > +#define VIVID_CID_AREA                       (VIVID_CID_CUSTOM_BASE + 11)
> >
> >  #define VIVID_CID_VIVID_BASE         (0x00f00000 | 0xf000)
> >  #define VIVID_CID_VIVID_CLASS                (0x00f00000 | 1)
> > @@ -266,6 +267,18 @@ static const struct v4l2_ctrl_config vivid_ctrl_disconnect = {
> >       .type = V4L2_CTRL_TYPE_BUTTON,
> >  };
> >
> > +static const struct v4l2_area area = {
> > +     .width = 0xcafe,
> > +     .height = 0xb1b1d,
>
> I don't think there is any need for these weird values. Just set this to
> e.g. 1000x2000. Just as long as width and height are different.
>
> Regards,

Sure. I will change that.

I though anyone liked vivid cafe ;P


>
>         Hans
>
> > +};
> > +
> > +static const struct v4l2_ctrl_config vivid_ctrl_area = {
> > +     .ops = &vivid_user_gen_ctrl_ops,
> > +     .id = VIVID_CID_AREA,
> > +     .name = "Area",
> > +     .type = V4L2_CTRL_TYPE_AREA,
> > +     .p_def.p_const = &area,
> > +};
> >
> >  /* Framebuffer Controls */
> >
> > @@ -1574,6 +1587,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
> >       dev->string = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_string, NULL);
> >       dev->bitmask = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_bitmask, NULL);
> >       dev->int_menu = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_int_menu, NULL);
> > +     v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL);
> >       v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL);
> >       v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL);
> >       v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u8_4d_array, NULL);
> >
>

[PATCH v3 4/5] media: v4l2_core: Add p_area to struct v4l2_ext_control

[Ricardo Ribalda Delgado]

Allow accessing V4L2_CTRL_TYPE_AREA controls without any casting.

Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
 include/uapi/linux/videodev2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index f98bbcced8ff..04481c717fee 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1684,6 +1684,7 @@ struct v4l2_ext_control {
 		__u8 __user *p_u8;
 		__u16 __user *p_u16;
 		__u32 __user *p_u32;
+		struct v4l2_area __user *p_area;
 		void __user *ptr;
 	};
 } __attribute__ ((packed));
-- 
2.24.0.rc1

[PATCH v3 5/5] Documentation: v42l_core: v4l2_ext_control

[Ricardo Ribalda Delgado]

Describe p_area field from v4l2_ext_ctrl

Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
---
 Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 13dc1a986249..271cac18afbb 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -198,6 +198,11 @@ still cause this situation.
       - ``p_u32``
       - A pointer to a matrix control of unsigned 32-bit values. Valid if
 	this control is of type ``V4L2_CTRL_TYPE_U32``.
+    * -
+      - :c:type:`v4l2_area` *
+      - ``p_area``
+      - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
+        of type ``V4L2_CTRL_TYPE_AREA``.
     * -
       - void *
       - ``ptr``
-- 
2.24.0.rc1