* [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE
@ 2015-03-19 15:21 Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE Ricardo Ribalda Delgado
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
This new set of patches overrides the old patchset
media/v4l2-ctrls: Always run s_ctrl on volatile ctrls
with the comments from Hans and Laurent.
Ricardo Ribalda Delgado (5):
media/v4l2-ctrls: volatiles should not generate CH_VALUE
media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE
media/v4l2-ctrls: Add execute flags to write_only controls
media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls
media/Documentation: New flag EXECUTE_ON_WRITE
Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 7 ++++---
.../DocBook/media/v4l/vidioc-queryctrl.xml | 15 +++++++++++++--
Documentation/video4linux/v4l2-controls.txt | 4 +++-
drivers/media/v4l2-core/v4l2-ctrls.c | 22 +++++++++++++++++++---
include/uapi/linux/videodev2.h | 1 +
5 files changed, 40 insertions(+), 9 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
@ 2015-03-19 15:21 ` Ricardo Ribalda Delgado
2015-03-20 12:38 ` Sakari Ailus
2015-03-19 15:21 ` [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
Volatile controls should not generate CH_VALUE events.
Set has_changed to false to prevent this happening.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 45c5b47..627d4c7 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1609,6 +1609,15 @@ static int cluster_changed(struct v4l2_ctrl *master)
if (ctrl == NULL)
continue;
+ /*
+ * Set has_changed to false to avoid generating
+ * the event V4L2_EVENT_CTRL_CH_VALUE
+ */
+ if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE){
+ ctrl->has_changed = false;
+ continue;
+ }
+
for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++)
ctrl_changed = !ctrl->type_ops->equal(ctrl, idx,
ctrl->p_cur, ctrl->p_new);
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE Ricardo Ribalda Delgado
@ 2015-03-19 15:21 ` Ricardo Ribalda Delgado
2015-03-20 13:34 ` Hans Verkuil
2015-03-19 15:21 ` [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls Ricardo Ribalda Delgado
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
Create a new flag that represent controls that represent controls that
its value needs to be passed to the driver even if it has not changed.
They typically represent actions, like triggering a flash or clearing an
error flag.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
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 fbdc360..1e33e10 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1456,6 +1456,7 @@ struct v4l2_querymenu {
#define V4L2_CTRL_FLAG_WRITE_ONLY 0x0040
#define V4L2_CTRL_FLAG_VOLATILE 0x0080
#define V4L2_CTRL_FLAG_HAS_PAYLOAD 0x0100
+#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE 0x0200
/* Query flags, to be ORed with the control ID */
#define V4L2_CTRL_FLAG_NEXT_CTRL 0x80000000
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
@ 2015-03-19 15:21 ` Ricardo Ribalda Delgado
2015-03-20 14:16 ` Laurent Pinchart
2015-03-19 15:21 ` [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE Ricardo Ribalda Delgado
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
Any control that sets FLAG_WRITE_ONLY should OR it with FLAG_ACTION.
So we can keep the current meaning of WRITE_ONLY.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 627d4c7..da0ffd3 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -991,7 +991,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_AUTO_FOCUS_START:
case V4L2_CID_AUTO_FOCUS_STOP:
*type = V4L2_CTRL_TYPE_BUTTON;
- *flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
+ *flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
+ V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
*min = *max = *step = *def = 0;
break;
case V4L2_CID_POWER_LINE_FREQUENCY:
@@ -1172,7 +1173,8 @@ 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:
- *flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
+ *flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
+ V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
break;
case V4L2_CID_FLASH_STROBE_STATUS:
case V4L2_CID_AUTO_FOCUS_STATUS:
@@ -1983,7 +1985,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
sz_extra = 0;
if (type == V4L2_CTRL_TYPE_BUTTON)
- flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
+ flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
+ V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
else if (type == V4L2_CTRL_TYPE_CTRL_CLASS)
flags |= V4L2_CTRL_FLAG_READ_ONLY;
else if (type == V4L2_CTRL_TYPE_INTEGER64 ||
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
` (2 preceding siblings ...)
2015-03-19 15:21 ` [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls Ricardo Ribalda Delgado
@ 2015-03-19 15:21 ` Ricardo Ribalda Delgado
2015-03-20 13:46 ` Hans Verkuil
2015-03-19 15:21 ` [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE Ricardo Ribalda Delgado
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
Any control with V4L2_CTRL_FLAG_EXECUTE_ON_WRITE set should return
changed == true in cluster_changed.
This forces the value to be passed to the driver even if it has not
changed.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index da0ffd3..8f96478 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1611,6 +1611,10 @@ static int cluster_changed(struct v4l2_ctrl *master)
if (ctrl == NULL)
continue;
+
+ if (ctrl->flags & V4L2_CTRL_FLAG_EXECUTE_ON_WRITE)
+ changed = true;
+
/*
* Set has_changed to false to avoid generating
* the event V4L2_EVENT_CTRL_CH_VALUE
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
` (3 preceding siblings ...)
2015-03-19 15:21 ` [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls Ricardo Ribalda Delgado
@ 2015-03-19 15:21 ` Ricardo Ribalda Delgado
2015-03-20 13:56 ` Hans Verkuil
4 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda Delgado @ 2015-03-19 15:21 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Cc: Ricardo Ribalda Delgado
Document new flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE, and the new behavior
of CH_VALUE event on VOLATILE controls.
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 7 ++++---
Documentation/DocBook/media/v4l/vidioc-queryctrl.xml | 15 +++++++++++++--
Documentation/video4linux/v4l2-controls.txt | 4 +++-
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index b036f89..38d907e 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -318,9 +318,10 @@
<entry><constant>V4L2_EVENT_CTRL_CH_VALUE</constant></entry>
<entry>0x0001</entry>
<entry>This control event was triggered because the value of the control
- changed. Special case: if a button control is pressed, then this
- event is sent as well, even though there is not explicit value
- associated with a button control.</entry>
+ changed. Special cases: Volatile controls do no generate this event;
+ If a button control is pressed, then this event is sent as well,
+ even though there is not explicit value associated with a button
+ control.</entry>
</row>
<row>
<entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
index 2bd98fd..d389292 100644
--- a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
@@ -599,8 +599,11 @@ writing a value will cause the device to carry out a given action
<entry>This control is volatile, which means that the value of the control
changes continuously. A typical example would be the current gain value if the device
is in auto-gain mode. In such a case the hardware calculates the gain value based on
-the lighting conditions which can change over time. Note that setting a new value for
-a volatile control will have no effect. The new value will just be ignored.</entry>
+the lighting conditions which can change over time. Another example would be an error
+flag (missed trigger, invalid voltage on the sensor). In those situations the user
+could write to the control to acknowledge the error, but that write will never
+generate a <constant>V4L2_EVENT_CTRL_CH_VALUE</constant> event and the flag
+<constant>V4L2_CTRL_FLAG_EXECUTE_ON_WRITE</constant> must be set.<entry>
</row>
<row>
<entry><constant>V4L2_CTRL_FLAG_HAS_PAYLOAD</constant></entry>
@@ -610,6 +613,14 @@ using one of the pointer fields of &v4l2-ext-control;. This flag is set for cont
that are an array, string, or have a compound type. In all cases you have to set a
pointer to memory containing the payload of the control.</entry>
</row>
+ <row>
+ <entry><constant>V4L2_CTRL_FLAG_EXECUTE_ON_WRITE</constant></entry>
+ <entry>0x0200</entry>
+ <entry>The value provided to the control will be propagated to the driver
+even if remains constant. This is required when the controls represents an action
+on the hardware. For example: clearing an error flag or triggering the flash. All the
+controls of the type <constant>V4L2_CTRL_TYPE_BUTTON</constant> have this flag set.</entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
index 0f84ce8..5517db6 100644
--- a/Documentation/video4linux/v4l2-controls.txt
+++ b/Documentation/video4linux/v4l2-controls.txt
@@ -344,7 +344,9 @@ implement g_volatile_ctrl like this:
}
Note that you use the 'new value' union as well in g_volatile_ctrl. In general
-controls that need to implement g_volatile_ctrl are read-only controls.
+controls that need to implement g_volatile_ctrl are read-only controls. If they
+are not, a V4L2_EVENT_CTRL_CH_VALUE will not be generated when the control
+changes.
To mark a control as volatile you have to set V4L2_CTRL_FLAG_VOLATILE:
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE
2015-03-19 15:21 ` [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE Ricardo Ribalda Delgado
@ 2015-03-20 12:38 ` Sakari Ailus
0 siblings, 0 replies; 11+ messages in thread
From: Sakari Ailus @ 2015-03-20 12:38 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Hans Verkuil, Mauro Carvalho Chehab,
Arun Kumar K, Sylwester Nawrocki, Antti Palosaari, linux-media,
linux-kernel, Laurent Pinchart
Ricardo Ribalda Delgado wrote:
> Volatile controls should not generate CH_VALUE events.
>
> Set has_changed to false to prevent this happening.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 45c5b47..627d4c7 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1609,6 +1609,15 @@ static int cluster_changed(struct v4l2_ctrl *master)
>
> if (ctrl == NULL)
> continue;
> + /*
> + * Set has_changed to false to avoid generating
> + * the event V4L2_EVENT_CTRL_CH_VALUE
> + */
Tabs for indentation, please.
> + if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE){
s/){/) {/
> + ctrl->has_changed = false;
> + continue;
> + }
> +
> for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++)
> ctrl_changed = !ctrl->type_ops->equal(ctrl, idx,
> ctrl->p_cur, ctrl->p_new);
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE
2015-03-19 15:21 ` [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
@ 2015-03-20 13:34 ` Hans Verkuil
0 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2015-03-20 13:34 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Hans Verkuil, Mauro Carvalho Chehab,
Arun Kumar K, Sylwester Nawrocki, Sakari Ailus, Antti Palosaari,
linux-media, linux-kernel, Laurent Pinchart
On 03/19/2015 04:21 PM, Ricardo Ribalda Delgado wrote:
> Create a new flag that represent controls that represent controls that
Double 'that represent controls' :-)
> its value needs to be passed to the driver even if it has not changed.
>
> They typically represent actions, like triggering a flash or clearing an
> error flag.
I would add something like:
"So writing to such a control means some action is executed."
This ties in a bit better with the name of the flag.
Hans
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> 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 fbdc360..1e33e10 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1456,6 +1456,7 @@ struct v4l2_querymenu {
> #define V4L2_CTRL_FLAG_WRITE_ONLY 0x0040
> #define V4L2_CTRL_FLAG_VOLATILE 0x0080
> #define V4L2_CTRL_FLAG_HAS_PAYLOAD 0x0100
> +#define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE 0x0200
>
> /* Query flags, to be ORed with the control ID */
> #define V4L2_CTRL_FLAG_NEXT_CTRL 0x80000000
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls
2015-03-19 15:21 ` [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls Ricardo Ribalda Delgado
@ 2015-03-20 13:46 ` Hans Verkuil
0 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2015-03-20 13:46 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Hans Verkuil, Mauro Carvalho Chehab,
Arun Kumar K, Sylwester Nawrocki, Sakari Ailus, Antti Palosaari,
linux-media, linux-kernel, Laurent Pinchart
One comment:
On 03/19/2015 04:21 PM, Ricardo Ribalda Delgado wrote:
> Any control with V4L2_CTRL_FLAG_EXECUTE_ON_WRITE set should return
> changed == true in cluster_changed.
>
> This forces the value to be passed to the driver even if it has not
> changed.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index da0ffd3..8f96478 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1611,6 +1611,10 @@ static int cluster_changed(struct v4l2_ctrl *master)
>
> if (ctrl == NULL)
> continue;
> +
> + if (ctrl->flags & V4L2_CTRL_FLAG_EXECUTE_ON_WRITE)
> + changed = true;
I think this should be:
changed = ctrl_changed = true;
This marks the cluster as changed (correct), and the control as changed
as well (that means a CH_VALUE event will be sent). The next volatile
check will put ctrl_changed back to false if it was volatile, otherwise
the for loop will just be skipped.
Right now writing to a non-button non-volatile control with EXECUTE_ON_WRITE
set and if you write the same value to it (so ctrl->type_ops->equal() returns
0) won't send the CH_VALUE event.
Very much a corner case, but still, it's easily solved.
Hans
> +
> /*
> * Set has_changed to false to avoid generating
> * the event V4L2_EVENT_CTRL_CH_VALUE
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE
2015-03-19 15:21 ` [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE Ricardo Ribalda Delgado
@ 2015-03-20 13:56 ` Hans Verkuil
0 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2015-03-20 13:56 UTC (permalink / raw)
To: Ricardo Ribalda Delgado, Hans Verkuil, Mauro Carvalho Chehab,
Arun Kumar K, Sylwester Nawrocki, Sakari Ailus, Antti Palosaari,
linux-media, linux-kernel, Laurent Pinchart
On 03/19/2015 04:21 PM, Ricardo Ribalda Delgado wrote:
> Document new flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE, and the new behavior
> of CH_VALUE event on VOLATILE controls.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 7 ++++---
> Documentation/DocBook/media/v4l/vidioc-queryctrl.xml | 15 +++++++++++++--
> Documentation/video4linux/v4l2-controls.txt | 4 +++-
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> index b036f89..38d907e 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -318,9 +318,10 @@
> <entry><constant>V4L2_EVENT_CTRL_CH_VALUE</constant></entry>
> <entry>0x0001</entry>
> <entry>This control event was triggered because the value of the control
> - changed. Special case: if a button control is pressed, then this
> - event is sent as well, even though there is not explicit value
> - associated with a button control.</entry>
> + changed. Special cases: Volatile controls do no generate this event;
> + If a button control is pressed, then this event is sent as well,
Not just a button control, but any non-volatile EXECUTE_ON_WRITE control.
> + even though there is not explicit value associated with a button
> + control.</entry>
> </row>
> <row>
> <entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> index 2bd98fd..d389292 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> @@ -599,8 +599,11 @@ writing a value will cause the device to carry out a given action
> <entry>This control is volatile, which means that the value of the control
> changes continuously. A typical example would be the current gain value if the device
> is in auto-gain mode. In such a case the hardware calculates the gain value based on
> -the lighting conditions which can change over time. Note that setting a new value for
> -a volatile control will have no effect. The new value will just be ignored.</entry>
> +the lighting conditions which can change over time. Another example would be an error
> +flag (missed trigger, invalid voltage on the sensor). In those situations the user
> +could write to the control to acknowledge the error, but that write will never
> +generate a <constant>V4L2_EVENT_CTRL_CH_VALUE</constant> event and the flag
> +<constant>V4L2_CTRL_FLAG_EXECUTE_ON_WRITE</constant> must be set.<entry>
I am not really happy with this change. In particular this dropped the "Note that setting
a new value for a volatile control will have no effect. The new value will just be ignored.".
This is still true as long as EXECUTE_ON_WRITE isn't set.
I would keep the original sentence, but add something like: "will have no effect and
no <constant>V4L2_EVENT_CTRL_CH_VALUE</constant> will be sent, unless
the <constant>V4L2_CTRL_FLAG_EXECUTE_ON_WRITE</constant> flag (see below) is also set.
Otherwise the new value will just be ignored."
And just skip the trigger example since that's already described in the EXECUTE_ON_WRITE
documentation.
> </row>
> <row>
> <entry><constant>V4L2_CTRL_FLAG_HAS_PAYLOAD</constant></entry>
> @@ -610,6 +613,14 @@ using one of the pointer fields of &v4l2-ext-control;. This flag is set for cont
> that are an array, string, or have a compound type. In all cases you have to set a
> pointer to memory containing the payload of the control.</entry>
> </row>
> + <row>
> + <entry><constant>V4L2_CTRL_FLAG_EXECUTE_ON_WRITE</constant></entry>
> + <entry>0x0200</entry>
> + <entry>The value provided to the control will be propagated to the driver
> +even if remains constant. This is required when the controls represents an action
s/controls/control/
> +on the hardware. For example: clearing an error flag or triggering the flash. All the
> +controls of the type <constant>V4L2_CTRL_TYPE_BUTTON</constant> have this flag set.</entry>
> + </row>
> </tbody>
> </tgroup>
> </table>
> diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
> index 0f84ce8..5517db6 100644
> --- a/Documentation/video4linux/v4l2-controls.txt
> +++ b/Documentation/video4linux/v4l2-controls.txt
> @@ -344,7 +344,9 @@ implement g_volatile_ctrl like this:
> }
>
> Note that you use the 'new value' union as well in g_volatile_ctrl. In general
> -controls that need to implement g_volatile_ctrl are read-only controls.
> +controls that need to implement g_volatile_ctrl are read-only controls. If they
> +are not, a V4L2_EVENT_CTRL_CH_VALUE will not be generated when the control
> +changes.
>
> To mark a control as volatile you have to set V4L2_CTRL_FLAG_VOLATILE:
>
>
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls
2015-03-19 15:21 ` [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls Ricardo Ribalda Delgado
@ 2015-03-20 14:16 ` Laurent Pinchart
0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2015-03-20 14:16 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Hans Verkuil, Mauro Carvalho Chehab, Arun Kumar K,
Sylwester Nawrocki, Sakari Ailus, Antti Palosaari, linux-media,
linux-kernel
Hi Ricardo,
Thank you for the patch.
On Thursday 19 March 2015 16:21:24 Ricardo Ribalda Delgado wrote:
> Any control that sets FLAG_WRITE_ONLY should OR it with FLAG_ACTION.
Do you mean FLAG_EXECUTE_ON_WRITE ?
> So we can keep the current meaning of WRITE_ONLY.
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c index 627d4c7..da0ffd3 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -991,7 +991,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type, case V4L2_CID_AUTO_FOCUS_START:
> case V4L2_CID_AUTO_FOCUS_STOP:
> *type = V4L2_CTRL_TYPE_BUTTON;
> - *flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> + *flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> + V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> *min = *max = *step = *def = 0;
> break;
> case V4L2_CID_POWER_LINE_FREQUENCY:
> @@ -1172,7 +1173,8 @@ 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:
> - *flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> + *flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> + V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> break;
> case V4L2_CID_FLASH_STROBE_STATUS:
> case V4L2_CID_AUTO_FOCUS_STATUS:
> @@ -1983,7 +1985,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl,
>
> sz_extra = 0;
> if (type == V4L2_CTRL_TYPE_BUTTON)
> - flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> + flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
> + V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
> else if (type == V4L2_CTRL_TYPE_CTRL_CLASS)
> flags |= V4L2_CTRL_FLAG_READ_ONLY;
> else if (type == V4L2_CTRL_TYPE_INTEGER64 ||
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-03-20 14:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-19 15:21 [PATCH 0/5] V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
2015-03-19 15:21 ` [PATCH 1/5] media/v4l2-ctrls: volatiles should not generate CH_VALUE Ricardo Ribalda Delgado
2015-03-20 12:38 ` Sakari Ailus
2015-03-19 15:21 ` [PATCH 2/5] media: New flag V4L2_CTRL_FLAG_EXECUTE_ON_WRITE Ricardo Ribalda Delgado
2015-03-20 13:34 ` Hans Verkuil
2015-03-19 15:21 ` [PATCH 3/5] media/v4l2-ctrls: Add execute flags to write_only controls Ricardo Ribalda Delgado
2015-03-20 14:16 ` Laurent Pinchart
2015-03-19 15:21 ` [PATCH 4/5] media/v4l2-ctrls: Always execute EXECUTE_ON_WRITE ctrls Ricardo Ribalda Delgado
2015-03-20 13:46 ` Hans Verkuil
2015-03-19 15:21 ` [PATCH 5/5] media/Documentation: New flag EXECUTE_ON_WRITE Ricardo Ribalda Delgado
2015-03-20 13:56 ` 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).