linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).