[RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[Sakari Ailus]

Hi all,

The OMAP 3 ISP driver implements an HS_VS event which is triggered when
the reception of a frame begins. This functionality is very, very likely
not specific to OMAP 3 ISP so it should be standardised.

I have a few patches to do that. Additionally the next expected buffer
sequence number is provided with the event, unlike earlier.

There are a few open questions, however, and this is why I'm sending the
set as RFC.


1) Other frame synchronisation events. The CCDC block in the OMAP 3 ISP
is able to trigger interrupts at two chosen lines of the image. These
naturally can be translated to events. The driver uses both of them
internally at specific points of the frame. Nevertheless, there might be
some use for these in user space. Other hardware might implement a
number of these which wouldn't be used by the driver itself, but I don't
know of that at the moment. On the other hand high resolution timers are
also available in user space, so doing timing based on ISP provided
events is not quite as important as before --- as long as there's one
frame based event produced at a known time, such as V4L2_EVENT_FRAME_START.

Frame end events may be produced as well. This is not exactly the same
as just dequeueing the buffer at video node since the hardware may be
able to produce events even in cases there are no buffers and if the
very hardware block that processes the frame is not outputting it to
memory, handling by further blocks takes more time, and thus delays the
finishing of the buffer from the driver's queue. This is the reason why
the name of the struct related to the event is v4l2_event_frame_sync
rather than v4l2_event_frame_start.

2) Buffer sequence number location in the struct v4l2_event. the patches
create a new structure called v4l2_event_frame_sync which contains just
one field, buffer_sequence. Should buffer_sequence be part of this
struct, or should it be part of v4l2_event directly, as the id field?
Both buffer_sequence and id refer to another rather widely used concept
in V4L2.


Besides this, the first patch in the series moves the documentation of
structs inside v4l2_event to VIDIOC_DQEVENT documentation. I think it
belongs there rather than to VIDIOC_SUBSCRIBE_EVENT, since that's not
where they are being used.

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

[RFC 1/3] v4l: Move event documentation from SUBSCRIBE_EVENT to DQEVENT

[Sakari Ailus]

Move documentation of structures used in DQEVENT from SUBSCRIBE_EVENT to
DQEVENT.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/vidioc-dqevent.xml |  107 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  107 --------------------
 2 files changed, 107 insertions(+), 107 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 7769642..5200b68 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -135,6 +135,113 @@
       </tgroup>
     </table>
 
+    <table frame="none" pgwide="1" id="v4l2-event-vsync">
+      <title>struct <structname>v4l2_event_vsync</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u8</entry>
+	    <entry><structfield>field</structfield></entry>
+	    <entry>The upcoming field. See &v4l2-field;.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table frame="none" pgwide="1" id="v4l2-event-ctrl">
+      <title>struct <structname>v4l2_event_ctrl</structname></title>
+      <tgroup cols="4">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>changes</structfield></entry>
+	    <entry></entry>
+	    <entry>A bitmask that tells what has changed. See <xref linkend="changes-flags" />.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+	    <entry></entry>
+	    <entry>The type of the control. See &v4l2-ctrl-type;.</entry>
+	  </row>
+	  <row>
+	    <entry>union (anonymous)</entry>
+	    <entry></entry>
+	    <entry></entry>
+	    <entry></entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>__s32</entry>
+	    <entry><structfield>value</structfield></entry>
+	    <entry>The 32-bit value of the control for 32-bit control types.
+		This is 0 for string controls since the value of a string
+		cannot be passed using &VIDIOC-DQEVENT;.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>__s64</entry>
+	    <entry><structfield>value64</structfield></entry>
+	    <entry>The 64-bit value of the control for 64-bit control types.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>flags</structfield></entry>
+	    <entry></entry>
+	    <entry>The control flags. See <xref linkend="control-flags" />.</entry>
+	  </row>
+	  <row>
+	    <entry>__s32</entry>
+	    <entry><structfield>minimum</structfield></entry>
+	    <entry></entry>
+	    <entry>The minimum value of the control. See &v4l2-queryctrl;.</entry>
+	  </row>
+	  <row>
+	    <entry>__s32</entry>
+	    <entry><structfield>maximum</structfield></entry>
+	    <entry></entry>
+	    <entry>The maximum value of the control. See &v4l2-queryctrl;.</entry>
+	  </row>
+	  <row>
+	    <entry>__s32</entry>
+	    <entry><structfield>step</structfield></entry>
+	    <entry></entry>
+	    <entry>The step value of the control. See &v4l2-queryctrl;.</entry>
+	  </row>
+	  <row>
+	    <entry>__s32</entry>
+	    <entry><structfield>default_value</structfield></entry>
+	    <entry></entry>
+	    <entry>The default value value of the control. See &v4l2-queryctrl;.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table pgwide="1" frame="none" id="changes-flags">
+      <title>Changes</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <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>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
+	    <entry>0x0002</entry>
+	    <entry>This control event was triggered because the control flags
+		changed.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
   </refsect1>
   <refsect1>
     &return-value;
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 69c0d8a..275be96 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -183,113 +183,6 @@
       </tgroup>
     </table>
 
-    <table frame="none" pgwide="1" id="v4l2-event-vsync">
-      <title>struct <structname>v4l2_event_vsync</structname></title>
-      <tgroup cols="3">
-	&cs-str;
-	<tbody valign="top">
-	  <row>
-	    <entry>__u8</entry>
-	    <entry><structfield>field</structfield></entry>
-	    <entry>The upcoming field. See &v4l2-field;.</entry>
-	  </row>
-	</tbody>
-      </tgroup>
-    </table>
-
-    <table frame="none" pgwide="1" id="v4l2-event-ctrl">
-      <title>struct <structname>v4l2_event_ctrl</structname></title>
-      <tgroup cols="4">
-	&cs-str;
-	<tbody valign="top">
-	  <row>
-	    <entry>__u32</entry>
-	    <entry><structfield>changes</structfield></entry>
-	    <entry></entry>
-	    <entry>A bitmask that tells what has changed. See <xref linkend="changes-flags" />.</entry>
-	  </row>
-	  <row>
-	    <entry>__u32</entry>
-	    <entry><structfield>type</structfield></entry>
-	    <entry></entry>
-	    <entry>The type of the control. See &v4l2-ctrl-type;.</entry>
-	  </row>
-	  <row>
-	    <entry>union (anonymous)</entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry>__s32</entry>
-	    <entry><structfield>value</structfield></entry>
-	    <entry>The 32-bit value of the control for 32-bit control types.
-		This is 0 for string controls since the value of a string
-		cannot be passed using &VIDIOC-DQEVENT;.</entry>
-	  </row>
-	  <row>
-	    <entry></entry>
-	    <entry>__s64</entry>
-	    <entry><structfield>value64</structfield></entry>
-	    <entry>The 64-bit value of the control for 64-bit control types.</entry>
-	  </row>
-	  <row>
-	    <entry>__u32</entry>
-	    <entry><structfield>flags</structfield></entry>
-	    <entry></entry>
-	    <entry>The control flags. See <xref linkend="control-flags" />.</entry>
-	  </row>
-	  <row>
-	    <entry>__s32</entry>
-	    <entry><structfield>minimum</structfield></entry>
-	    <entry></entry>
-	    <entry>The minimum value of the control. See &v4l2-queryctrl;.</entry>
-	  </row>
-	  <row>
-	    <entry>__s32</entry>
-	    <entry><structfield>maximum</structfield></entry>
-	    <entry></entry>
-	    <entry>The maximum value of the control. See &v4l2-queryctrl;.</entry>
-	  </row>
-	  <row>
-	    <entry>__s32</entry>
-	    <entry><structfield>step</structfield></entry>
-	    <entry></entry>
-	    <entry>The step value of the control. See &v4l2-queryctrl;.</entry>
-	  </row>
-	  <row>
-	    <entry>__s32</entry>
-	    <entry><structfield>default_value</structfield></entry>
-	    <entry></entry>
-	    <entry>The default value value of the control. See &v4l2-queryctrl;.</entry>
-	  </row>
-	</tbody>
-      </tgroup>
-    </table>
-
-    <table pgwide="1" frame="none" id="changes-flags">
-      <title>Changes</title>
-      <tgroup cols="3">
-	&cs-def;
-	<tbody valign="top">
-	  <row>
-	    <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>
-	  </row>
-	  <row>
-	    <entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
-	    <entry>0x0002</entry>
-	    <entry>This control event was triggered because the control flags
-		changed.</entry>
-	  </row>
-	</tbody>
-      </tgroup>
-    </table>
   </refsect1>
   <refsect1>
     &return-value;
-- 
1.7.2.5

[RFC 2/3] v4l: events: Define frame start event

[Sakari Ailus]

Define a frame start event to tell user space when the reception of a frame
starts.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   26 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   18 +++++++++++++
 include/linux/videodev2.h                          |   12 +++++++--
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 5200b68..d2cb8db 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -88,6 +88,12 @@
 	  </row>
 	  <row>
 	    <entry></entry>
+	    <entry>&v4l2-event-frame-sync;</entry>
+            <entry><structfield>frame</structfield></entry>
+	    <entry>Event data for event V4L2_EVENT_FRAME_START.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
 	    <entry>__u8</entry>
             <entry><structfield>data</structfield>[64]</entry>
 	    <entry>Event data. Defined by the event type. The union
@@ -220,6 +226,26 @@
       </tgroup>
     </table>
 
+    <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
+      <title>struct <structname>v4l2_event_frame_sync</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>buffer_sequence</structfield></entry>
+	    <entry>
+	      The sequence number of the buffer to be handled next or
+	      currently handled by the driver.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <para>&v4l2-event-frame-sync; is associated with
+    <constant>V4L2_EVENT_FRAME_START</constant> event.</para>
+
     <table pgwide="1" frame="none" id="changes-flags">
       <title>Changes</title>
       <tgroup cols="3">
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 275be96..7ec42bb 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -139,6 +139,24 @@
 	    </entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_EVENT_FRAME_START</constant></entry>
+	    <entry>4</entry>
+	    <entry>
+	      <para>Triggered immediately when the reception of a
+	      frame has begun. This event has a
+	      &v4l2-event-frame-sync; associated with it.</para>
+
+	      <para>A driver will only generate this event when the
+	      hardware can generate it. This might not be the case
+	      e.g. when the hardware has no DMA buffer to write the
+	      image data to. In such cases the
+	      <structfield>buffer_sequence</structfield> field in
+	      &v4l2-event-frame-sync; will not be incremented either.
+	      This causes two consecutive buffer sequence numbers to
+	      have n times frame interval in between them.</para>
+	    </entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
 	    <entry>0x08000000</entry>
 	    <entry>Base event number for driver-private events.</entry>
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..4265102 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2006,6 +2006,7 @@ struct v4l2_streamparm {
 #define V4L2_EVENT_VSYNC			1
 #define V4L2_EVENT_EOS				2
 #define V4L2_EVENT_CTRL				3
+#define V4L2_EVENT_FRAME_START			4
 #define V4L2_EVENT_PRIVATE_START		0x08000000
 
 /* Payload for V4L2_EVENT_VSYNC */
@@ -2032,12 +2033,17 @@ struct v4l2_event_ctrl {
 	__s32 default_value;
 };
 
+struct v4l2_event_frame_sync {
+	__u32 buffer_sequence;
+};
+
 struct v4l2_event {
 	__u32				type;
 	union {
-		struct v4l2_event_vsync vsync;
-		struct v4l2_event_ctrl	ctrl;
-		__u8			data[64];
+		struct v4l2_event_vsync		vsync;
+		struct v4l2_event_ctrl		ctrl;
+		struct v4l2_event_frame_sync	frame_sync;
+		__u8				data[64];
 	} u;
 	__u32				pending;
 	__u32				sequence;
-- 
1.7.2.5

[RFC 3/3] omap3isp: ccdc: Make frame start event generic

[Sakari Ailus]

The ccdc block in the omap3isp produces frame start events. These events
were previously specific to the omap3isp. Make them generic.

Also add sequence number to the frame. This is stored to the id field.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/video4linux/omap3isp.txt |    9 +++++----
 drivers/media/video/omap3isp/ispccdc.c |    7 +++++--
 include/linux/omap3isp.h               |    2 --
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/video4linux/omap3isp.txt b/Documentation/video4linux/omap3isp.txt
index 69be2c7..84f24cf 100644
--- a/Documentation/video4linux/omap3isp.txt
+++ b/Documentation/video4linux/omap3isp.txt
@@ -70,10 +70,11 @@ Events
 The OMAP 3 ISP driver does support the V4L2 event interface on CCDC and
 statistics (AEWB, AF and histogram) subdevs.
 
-The CCDC subdev produces V4L2_EVENT_OMAP3ISP_HS_VS type event on HS_VS
-interrupt which is used to signal frame start. The event is triggered exactly
-when the reception of the first line of the frame starts in the CCDC module.
-The event can be subscribed on the CCDC subdev.
+The CCDC subdev produces V4L2_EVENT_FRAME_START type event on HS_VS
+interrupt which is used to signal frame start. Earlier version of this
+driver used V4L2_EVENT_OMAP3ISP_HS_VS for this purpose. The event is
+triggered exactly when the reception of the first line of the frame starts
+in the CCDC module. The event can be subscribed on the CCDC subdev.
 
 (When using parallel interface one must pay account to correct configuration
 of the VS signal polarity. This is automatically correct when using the serial
diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index 6766247..61c9e3d 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1402,11 +1402,14 @@ static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event)
 
 static void ccdc_hs_vs_isr(struct isp_ccdc_device *ccdc)
 {
+	struct isp_pipeline *pipe =
+		to_isp_pipeline(&ccdc->video_out.video.entity);
 	struct video_device *vdev = &ccdc->subdev.devnode;
 	struct v4l2_event event;
 
 	memset(&event, 0, sizeof(event));
-	event.type = V4L2_EVENT_OMAP3ISP_HS_VS;
+	event.type = V4L2_EVENT_FRAME_START;
+	event.u.frame_sync.buffer_sequence = atomic_read(&pipe->frame_number);
 
 	v4l2_event_queue(vdev, &event);
 }
@@ -1688,7 +1691,7 @@ static long ccdc_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 static int ccdc_subscribe_event(struct v4l2_subdev *sd, struct v4l2_fh *fh,
 				struct v4l2_event_subscription *sub)
 {
-	if (sub->type != V4L2_EVENT_OMAP3ISP_HS_VS)
+	if (sub->type != V4L2_EVENT_FRAME_START)
 		return -EINVAL;
 
 	return v4l2_event_subscribe(fh, sub, OMAP3ISP_CCDC_NEVENTS);
diff --git a/include/linux/omap3isp.h b/include/linux/omap3isp.h
index b6111f8..c73a34c 100644
--- a/include/linux/omap3isp.h
+++ b/include/linux/omap3isp.h
@@ -62,14 +62,12 @@
  * V4L2_EVENT_OMAP3ISP_AEWB: AEWB statistics data ready
  * V4L2_EVENT_OMAP3ISP_AF: AF statistics data ready
  * V4L2_EVENT_OMAP3ISP_HIST: Histogram statistics data ready
- * V4L2_EVENT_OMAP3ISP_HS_VS: Horizontal/vertical synchronization detected
  */
 
 #define V4L2_EVENT_OMAP3ISP_CLASS	(V4L2_EVENT_PRIVATE_START | 0x100)
 #define V4L2_EVENT_OMAP3ISP_AEWB	(V4L2_EVENT_OMAP3ISP_CLASS | 0x1)
 #define V4L2_EVENT_OMAP3ISP_AF		(V4L2_EVENT_OMAP3ISP_CLASS | 0x2)
 #define V4L2_EVENT_OMAP3ISP_HIST	(V4L2_EVENT_OMAP3ISP_CLASS | 0x3)
-#define V4L2_EVENT_OMAP3ISP_HS_VS	(V4L2_EVENT_OMAP3ISP_CLASS | 0x4)
 
 struct omap3isp_stat_event_status {
 	__u32 frame_number;
-- 
1.7.2.5

Re: [RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[Sylwester Nawrocki]

Hi Sakari,

On 07/19/2011 03:37 PM, Sakari Ailus wrote:
> Hi all,
> 
> The OMAP 3 ISP driver implements an HS_VS event which is triggered when
> the reception of a frame begins. This functionality is very, very likely
> not specific to OMAP 3 ISP so it should be standardised.
> 
> I have a few patches to do that. Additionally the next expected buffer
> sequence number is provided with the event, unlike earlier.
> 
> There are a few open questions, however, and this is why I'm sending the
> set as RFC.
> 
> 
> 1) Other frame synchronisation events. The CCDC block in the OMAP 3 ISP
> is able to trigger interrupts at two chosen lines of the image. These
> naturally can be translated to events. The driver uses both of them
> internally at specific points of the frame. Nevertheless, there might be
> some use for these in user space. Other hardware might implement a
> number of these which wouldn't be used by the driver itself, but I don't
> know of that at the moment. On the other hand high resolution timers are
> also available in user space, so doing timing based on ISP provided
> events is not quite as important as before --- as long as there's one
> frame based event produced at a known time, such as V4L2_EVENT_FRAME_START.

I'm curious, have you perhaps tried to measure latency of such up calls
to a user space process? I mean this is going to be a real time stuff,
with HSYNC periods of 50 us order. Could a user space thread be receiving
such periodic events reliably ? From my experience I doubt this can work
reliably outside of an interrupt handler even with high priority real time
threads.

V4L2_EVENT_FRAME_START event seems OK, but HSYNC events in user space
sound rather tricky to me :-)

Also HS_VS looks a bit more descriptive than FRAME_START for me.
But unfortunately I can't come up with a better name, e.g. something like
V4L2_EVENT_FRAME_AV_START - frame active video start. Just in case in
future there are more specific events added.

> 
> Frame end events may be produced as well. This is not exactly the same
> as just dequeueing the buffer at video node since the hardware may be
> able to produce events even in cases there are no buffers and if the
> very hardware block that processes the frame is not outputting it to
> memory, handling by further blocks takes more time, and thus delays the
> finishing of the buffer from the driver's queue. This is the reason why
> the name of the struct related to the event is v4l2_event_frame_sync
> rather than v4l2_event_frame_start.
> 
> 2) Buffer sequence number location in the struct v4l2_event. the patches
> create a new structure called v4l2_event_frame_sync which contains just
> one field, buffer_sequence. Should buffer_sequence be part of this
> struct, or should it be part of v4l2_event directly, as the id field?
> Both buffer_sequence and id refer to another rather widely used concept
> in V4L2.
> 
> 
> Besides this, the first patch in the series moves the documentation of
> structs inside v4l2_event to VIDIOC_DQEVENT documentation. I think it
> belongs there rather than to VIDIOC_SUBSCRIBE_EVENT, since that's not
> where they are being used.
> 

--
Regards,
Sylwester

Re: [RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[Sakari Ailus]

Sylwester Nawrocki wrote:
> Hi Sakari,

Hi Sylwester,

Thanks for the comments.

> On 07/19/2011 03:37 PM, Sakari Ailus wrote:
>> Hi all,
>>
>> The OMAP 3 ISP driver implements an HS_VS event which is triggered when
>> the reception of a frame begins. This functionality is very, very likely
>> not specific to OMAP 3 ISP so it should be standardised.
>>
>> I have a few patches to do that. Additionally the next expected buffer
>> sequence number is provided with the event, unlike earlier.
>>
>> There are a few open questions, however, and this is why I'm sending the
>> set as RFC.
>>
>>
>> 1) Other frame synchronisation events. The CCDC block in the OMAP 3 ISP
>> is able to trigger interrupts at two chosen lines of the image. These
>> naturally can be translated to events. The driver uses both of them
>> internally at specific points of the frame. Nevertheless, there might be
>> some use for these in user space. Other hardware might implement a
>> number of these which wouldn't be used by the driver itself, but I don't
>> know of that at the moment. On the other hand high resolution timers are
>> also available in user space, so doing timing based on ISP provided
>> events is not quite as important as before --- as long as there's one
>> frame based event produced at a known time, such as V4L2_EVENT_FRAME_START.
> 
> I'm curious, have you perhaps tried to measure latency of such up calls
> to a user space process? I mean this is going to be a real time stuff,
> with HSYNC periods of 50 us order. Could a user space thread be receiving
> such periodic events reliably ? From my experience I doubt this can work
> reliably outside of an interrupt handler even with high priority real time
> threads.
> 
> V4L2_EVENT_FRAME_START event seems OK, but HSYNC events in user space
> sound rather tricky to me :-)

I think the user space could be interested in just one or two of these
per frame, not for every line. But how to subscribe them --- if they are
needed?

Perhaps it'd be better to start with just one and add more once necessary?

> Also HS_VS looks a bit more descriptive than FRAME_START for me.

HS_VS doesn't really tell which one it is (horizontal or vertical), and
we already have a VSYNC event but it's used for a different purpose.
HS_VS is specific to the CCDC block and doesn't have that meaning in
context of serial interfaces.

This is why I proposed FRAME_START.

> But unfortunately I can't come up with a better name, e.g. something like
> V4L2_EVENT_FRAME_AV_START - frame active video start. Just in case in
> future there are more specific events added.

What additional information would AV add which isn't evident from
FRAME_START?

I admit that there could be differencies in terminology used in this
area; terms that are meaningful to some might not be to others, or they
could mean different things to them.

Regards,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[Sylwester Nawrocki]

Hi,

On 07/22/2011 12:39 PM, Sakari Ailus wrote:
...
>> On 07/19/2011 03:37 PM, Sakari Ailus wrote:
>>> Hi all,
>>>
>>> The OMAP 3 ISP driver implements an HS_VS event which is triggered when
>>> the reception of a frame begins. This functionality is very, very likely
>>> not specific to OMAP 3 ISP so it should be standardised.
>>>
>>> I have a few patches to do that. Additionally the next expected buffer
>>> sequence number is provided with the event, unlike earlier.
>>>
>>> There are a few open questions, however, and this is why I'm sending the
>>> set as RFC.
>>>
>>>
>>> 1) Other frame synchronisation events. The CCDC block in the OMAP 3 ISP
>>> is able to trigger interrupts at two chosen lines of the image. These
>>> naturally can be translated to events. The driver uses both of them
>>> internally at specific points of the frame. Nevertheless, there might be
>>> some use for these in user space. Other hardware might implement a
>>> number of these which wouldn't be used by the driver itself, but I don't
>>> know of that at the moment. On the other hand high resolution timers are
>>> also available in user space, so doing timing based on ISP provided
>>> events is not quite as important as before --- as long as there's one
>>> frame based event produced at a known time, such as V4L2_EVENT_FRAME_START.
>>
>> I'm curious, have you perhaps tried to measure latency of such up calls
>> to a user space process? I mean this is going to be a real time stuff,
>> with HSYNC periods of 50 us order. Could a user space thread be receiving
>> such periodic events reliably ? From my experience I doubt this can work
>> reliably outside of an interrupt handler even with high priority real time
>> threads.
>>
>> V4L2_EVENT_FRAME_START event seems OK, but HSYNC events in user space
>> sound rather tricky to me :-)
> 
> I think the user space could be interested in just one or two of these
> per frame, not for every line. But how to subscribe them --- if they are
> needed?

Yes, that was my understanding. But still we need much better accuracy than
in case of VSYNC/FRAME_START. It seems really hard to guarantee in Linux
that a specific line event will be received in user space when that actual
line is transmitted. There is much better chance that a FRAME_START event
is received during the time when a frame that triggered it is being processed.
And VSYNC signals last over several horizontal lines (if not tens or thousands)
which makes it easier to receive an event when a VSYNC pulse/period hasn't yet
expired.

> 
> Perhaps it'd be better to start with just one and add more once necessary?

Maybe we could accommodate the struct v4l2_event_subscription::id field for 
a horizontal line number, with a special bit indicating any one ?
But again I'm not convinced to horizontal line events :) There might be
situations when even interrupt handlers are to slow for this.

> 
>> Also HS_VS looks a bit more descriptive than FRAME_START for me.
> 
> HS_VS doesn't really tell which one it is (horizontal or vertical), and
> we already have a VSYNC event but it's used for a different purpose.
> HS_VS is specific to the CCDC block and doesn't have that meaning in
> context of serial interfaces.
> 
> This is why I proposed FRAME_START.

OK, initially I thought it was that HS_VS means a moment when vertical
and horizontal sync (blanking?) signals go off and thus video data 
of the first line in a frame is started to be transmitted.
I agree that HS_VS isn't that relevant in the CSI terminology.

> 
>> But unfortunately I can't come up with a better name, e.g. something like
>> V4L2_EVENT_FRAME_AV_START - frame active video start. Just in case in
>> future there are more specific events added.
> 
> What additional information would AV add which isn't evident from
> FRAME_START?

Given different formats of frame headers across the standards (ITU-R BT.656,
MIPI-CSI2 and the like) it might be not so obvious at which moment the
FRAME_START event is to be triggered. But not being too paranoid I guess
we're perfectly fine with VSYNC and FRAME_START events.

> 
> I admit that there could be differencies in terminology used in this
> area; terms that are meaningful to some might not be to others, or they
> could mean different things to them.

Yes, I entirely agree. So perhaps we're better off with V4L2_EVENT_FRAME_START
name and having the exact meaning described in details in the documentation.

--
Regards,
Sylwester

Re: [RFC 0/3] Frame synchronisation events and support for them in the OMAP 3 ISP driver

[Sakari Ailus]

Sylwester Nawrocki wrote:
> Hi,

Hi Sylwester,

Thanks for your comments.

> On 07/22/2011 12:39 PM, Sakari Ailus wrote:
> ...
>>> On 07/19/2011 03:37 PM, Sakari Ailus wrote:
>>>> Hi all,
>>>>
>>>> The OMAP 3 ISP driver implements an HS_VS event which is triggered when
>>>> the reception of a frame begins. This functionality is very, very likely
>>>> not specific to OMAP 3 ISP so it should be standardised.
>>>>
>>>> I have a few patches to do that. Additionally the next expected buffer
>>>> sequence number is provided with the event, unlike earlier.
>>>>
>>>> There are a few open questions, however, and this is why I'm sending the
>>>> set as RFC.
>>>>
>>>>
>>>> 1) Other frame synchronisation events. The CCDC block in the OMAP 3 ISP
>>>> is able to trigger interrupts at two chosen lines of the image. These
>>>> naturally can be translated to events. The driver uses both of them
>>>> internally at specific points of the frame. Nevertheless, there might be
>>>> some use for these in user space. Other hardware might implement a
>>>> number of these which wouldn't be used by the driver itself, but I don't
>>>> know of that at the moment. On the other hand high resolution timers are
>>>> also available in user space, so doing timing based on ISP provided
>>>> events is not quite as important as before --- as long as there's one
>>>> frame based event produced at a known time, such as V4L2_EVENT_FRAME_START.
>>>
>>> I'm curious, have you perhaps tried to measure latency of such up calls
>>> to a user space process? I mean this is going to be a real time stuff,
>>> with HSYNC periods of 50 us order. Could a user space thread be receiving
>>> such periodic events reliably ? From my experience I doubt this can work
>>> reliably outside of an interrupt handler even with high priority real time
>>> threads.
>>>
>>> V4L2_EVENT_FRAME_START event seems OK, but HSYNC events in user space
>>> sound rather tricky to me :-)
>>
>> I think the user space could be interested in just one or two of these
>> per frame, not for every line. But how to subscribe them --- if they are
>> needed?
> 
> Yes, that was my understanding. But still we need much better accuracy than
> in case of VSYNC/FRAME_START. It seems really hard to guarantee in Linux
> that a specific line event will be received in user space when that actual
> line is transmitted. There is much better chance that a FRAME_START event

There's nothing that a driver may do itself to make the user space
receive the event in time, especially if the user space process is not
running on real-time priority. This is a completely separate issue.

The timestamp is very important here, more important than guaranteed
timely event delivery.

> is received during the time when a frame that triggered it is being processed.

This is what happens in practice and I do not consider it as an issue.
Any configuration which is related to the processing of that frame must
have been given to the driver well beforehand.

> And VSYNC signals last over several horizontal lines (if not tens or thousands)
> which makes it easier to receive an event when a VSYNC pulse/period hasn't yet
> expired.

I'm not sure I can follow you here. It's the responsibility of the
driver / hardware to provide the event. If it can't, then it shouldn't
allow subscribing it.

>>
>> Perhaps it'd be better to start with just one and add more once necessary?
> 
> Maybe we could accommodate the struct v4l2_event_subscription::id field for 
> a horizontal line number, with a special bit indicating any one ?

That's an interesting idea. The subscription problem for line events
still remains.

> But again I'm not convinced to horizontal line events :) There might be
> situations when even interrupt handlers are to slow for this.

Events may be delivered when there's an interrupt source which can
generate an interrupt per event. This is no more complex than generating
one in the beginning of the frame.

Nevertheless, I don't either think these would be very useful, so I
think they could be ignored, at least for the time being.

>>
>>> Also HS_VS looks a bit more descriptive than FRAME_START for me.
>>
>> HS_VS doesn't really tell which one it is (horizontal or vertical), and
>> we already have a VSYNC event but it's used for a different purpose.
>> HS_VS is specific to the CCDC block and doesn't have that meaning in
>> context of serial interfaces.
>>
>> This is why I proposed FRAME_START.
> 
> OK, initially I thought it was that HS_VS means a moment when vertical
> and horizontal sync (blanking?) signals go off and thus video data 
> of the first line in a frame is started to be transmitted.
> I agree that HS_VS isn't that relevant in the CSI terminology.

The OMAP 3 ISP may produce interrupts for either vertical or horizontal
sync events but the driver only uses it for vertical sync. I guess
there's little or no imaginable use for HS_VS to generate an interrupt
per every line. VD_INT interrupts Laurent mentioned are used to generate
an interrupt on a specific line of the frame.

>>> But unfortunately I can't come up with a better name, e.g. something like
>>> V4L2_EVENT_FRAME_AV_START - frame active video start. Just in case in
>>> future there are more specific events added.
>>
>> What additional information would AV add which isn't evident from
>> FRAME_START?
> 
> Given different formats of frame headers across the standards (ITU-R BT.656,
> MIPI-CSI2 and the like) it might be not so obvious at which moment the
> FRAME_START event is to be triggered. But not being too paranoid I guess
> we're perfectly fine with VSYNC and FRAME_START events.

It should be trigered when the first bit of data is being received. What
about FRAME_DATA_START?

Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [RFC 1/3] v4l: Move event documentation from SUBSCRIBE_EVENT to DQEVENT

[Hans Verkuil]

On Tuesday, July 19, 2011 15:38:06 Sakari Ailus wrote:
> Move documentation of structures used in DQEVENT from SUBSCRIBE_EVENT to
> DQEVENT.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

This is a much better place!

Regards,

	Hans

> ---
>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |  107 ++++++++++++++++++++
>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |  107 --------------------
>  2 files changed, 107 insertions(+), 107 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> index 7769642..5200b68 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -135,6 +135,113 @@
>        </tgroup>
>      </table>
>  
> +    <table frame="none" pgwide="1" id="v4l2-event-vsync">
> +      <title>struct <structname>v4l2_event_vsync</structname></title>
> +      <tgroup cols="3">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u8</entry>
> +	    <entry><structfield>field</structfield></entry>
> +	    <entry>The upcoming field. See &v4l2-field;.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +    <table frame="none" pgwide="1" id="v4l2-event-ctrl">
> +      <title>struct <structname>v4l2_event_ctrl</structname></title>
> +      <tgroup cols="4">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>changes</structfield></entry>
> +	    <entry></entry>
> +	    <entry>A bitmask that tells what has changed. See <xref linkend="changes-flags" />.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>type</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The type of the control. See &v4l2-ctrl-type;.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>union (anonymous)</entry>
> +	    <entry></entry>
> +	    <entry></entry>
> +	    <entry></entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +	    <entry>__s32</entry>
> +	    <entry><structfield>value</structfield></entry>
> +	    <entry>The 32-bit value of the control for 32-bit control types.
> +		This is 0 for string controls since the value of a string
> +		cannot be passed using &VIDIOC-DQEVENT;.</entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +	    <entry>__s64</entry>
> +	    <entry><structfield>value64</structfield></entry>
> +	    <entry>The 64-bit value of the control for 64-bit control types.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>flags</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The control flags. See <xref linkend="control-flags" />.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__s32</entry>
> +	    <entry><structfield>minimum</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The minimum value of the control. See &v4l2-queryctrl;.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__s32</entry>
> +	    <entry><structfield>maximum</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The maximum value of the control. See &v4l2-queryctrl;.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__s32</entry>
> +	    <entry><structfield>step</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The step value of the control. See &v4l2-queryctrl;.</entry>
> +	  </row>
> +	  <row>
> +	    <entry>__s32</entry>
> +	    <entry><structfield>default_value</structfield></entry>
> +	    <entry></entry>
> +	    <entry>The default value value of the control. See &v4l2-queryctrl;.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +    <table pgwide="1" frame="none" id="changes-flags">
> +      <title>Changes</title>
> +      <tgroup cols="3">
> +	&cs-def;
> +	<tbody valign="top">
> +	  <row>
> +	    <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>
> +	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
> +	    <entry>0x0002</entry>
> +	    <entry>This control event was triggered because the control flags
> +		changed.</entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
>    </refsect1>
>    <refsect1>
>      &return-value;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> index 69c0d8a..275be96 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -183,113 +183,6 @@
>        </tgroup>
>      </table>
>  
> -    <table frame="none" pgwide="1" id="v4l2-event-vsync">
> -      <title>struct <structname>v4l2_event_vsync</structname></title>
> -      <tgroup cols="3">
> -	&cs-str;
> -	<tbody valign="top">
> -	  <row>
> -	    <entry>__u8</entry>
> -	    <entry><structfield>field</structfield></entry>
> -	    <entry>The upcoming field. See &v4l2-field;.</entry>
> -	  </row>
> -	</tbody>
> -      </tgroup>
> -    </table>
> -
> -    <table frame="none" pgwide="1" id="v4l2-event-ctrl">
> -      <title>struct <structname>v4l2_event_ctrl</structname></title>
> -      <tgroup cols="4">
> -	&cs-str;
> -	<tbody valign="top">
> -	  <row>
> -	    <entry>__u32</entry>
> -	    <entry><structfield>changes</structfield></entry>
> -	    <entry></entry>
> -	    <entry>A bitmask that tells what has changed. See <xref linkend="changes-flags" />.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__u32</entry>
> -	    <entry><structfield>type</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The type of the control. See &v4l2-ctrl-type;.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>union (anonymous)</entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	  </row>
> -	  <row>
> -	    <entry></entry>
> -	    <entry>__s32</entry>
> -	    <entry><structfield>value</structfield></entry>
> -	    <entry>The 32-bit value of the control for 32-bit control types.
> -		This is 0 for string controls since the value of a string
> -		cannot be passed using &VIDIOC-DQEVENT;.</entry>
> -	  </row>
> -	  <row>
> -	    <entry></entry>
> -	    <entry>__s64</entry>
> -	    <entry><structfield>value64</structfield></entry>
> -	    <entry>The 64-bit value of the control for 64-bit control types.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__u32</entry>
> -	    <entry><structfield>flags</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The control flags. See <xref linkend="control-flags" />.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__s32</entry>
> -	    <entry><structfield>minimum</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The minimum value of the control. See &v4l2-queryctrl;.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__s32</entry>
> -	    <entry><structfield>maximum</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The maximum value of the control. See &v4l2-queryctrl;.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__s32</entry>
> -	    <entry><structfield>step</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The step value of the control. See &v4l2-queryctrl;.</entry>
> -	  </row>
> -	  <row>
> -	    <entry>__s32</entry>
> -	    <entry><structfield>default_value</structfield></entry>
> -	    <entry></entry>
> -	    <entry>The default value value of the control. See &v4l2-queryctrl;.</entry>
> -	  </row>
> -	</tbody>
> -      </tgroup>
> -    </table>
> -
> -    <table pgwide="1" frame="none" id="changes-flags">
> -      <title>Changes</title>
> -      <tgroup cols="3">
> -	&cs-def;
> -	<tbody valign="top">
> -	  <row>
> -	    <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>
> -	  </row>
> -	  <row>
> -	    <entry><constant>V4L2_EVENT_CTRL_CH_FLAGS</constant></entry>
> -	    <entry>0x0002</entry>
> -	    <entry>This control event was triggered because the control flags
> -		changed.</entry>
> -	  </row>
> -	</tbody>
> -      </tgroup>
> -    </table>
>    </refsect1>
>    <refsect1>
>      &return-value;
> 

Re: [RFC 2/3] v4l: events: Define frame start event

[Hans Verkuil]

On Tuesday, July 19, 2011 15:38:07 Sakari Ailus wrote:
> Define a frame start event to tell user space when the reception of a frame
> starts.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   26 ++++++++++++++++++++
>  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   18 +++++++++++++
>  include/linux/videodev2.h                          |   12 +++++++--
>  3 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> index 5200b68..d2cb8db 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -88,6 +88,12 @@
>  	  </row>
>  	  <row>
>  	    <entry></entry>
> +	    <entry>&v4l2-event-frame-sync;</entry>
> +            <entry><structfield>frame</structfield></entry>
> +	    <entry>Event data for event V4L2_EVENT_FRAME_START.</entry>

The name of the struct and the event are not in sync (pardon the expression :-) ).

Both should either be named FRAME_SYNC or FRAME_START.

> +	  </row>
> +	  <row>
> +	    <entry></entry>
>  	    <entry>__u8</entry>
>              <entry><structfield>data</structfield>[64]</entry>
>  	    <entry>Event data. Defined by the event type. The union
> @@ -220,6 +226,26 @@
>        </tgroup>
>      </table>
>  
> +    <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
> +      <title>struct <structname>v4l2_event_frame_sync</structname></title>
> +      <tgroup cols="3">
> +	&cs-str;
> +	<tbody valign="top">
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>buffer_sequence</structfield></entry>
> +	    <entry>
> +	      The sequence number of the buffer to be handled next or
> +	      currently handled by the driver.

So this refers to the sequence field in struct v4l2_buffer? Assuming it is,
then you definitely need to refer to that struct.

And to answer question 2 in the RFC: buffer_sequence is specific to this
event, and so belongs to a v4l2_event_frame_sync struct.

BTW, using 'id' to specify a specific line in the future seems reasonable to me.
Initially id is just set to 0, meaning the start of the frame.

Regards,

	Hans

> +	    </entry>
> +	  </row>
> +	</tbody>
> +      </tgroup>
> +    </table>
> +
> +    <para>&v4l2-event-frame-sync; is associated with
> +    <constant>V4L2_EVENT_FRAME_START</constant> event.</para>
> +
>      <table pgwide="1" frame="none" id="changes-flags">
>        <title>Changes</title>
>        <tgroup cols="3">
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> index 275be96..7ec42bb 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -139,6 +139,24 @@
>  	    </entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_EVENT_FRAME_START</constant></entry>
> +	    <entry>4</entry>
> +	    <entry>
> +	      <para>Triggered immediately when the reception of a
> +	      frame has begun. This event has a
> +	      &v4l2-event-frame-sync; associated with it.</para>
> +
> +	      <para>A driver will only generate this event when the
> +	      hardware can generate it. This might not be the case
> +	      e.g. when the hardware has no DMA buffer to write the
> +	      image data to. In such cases the
> +	      <structfield>buffer_sequence</structfield> field in
> +	      &v4l2-event-frame-sync; will not be incremented either.
> +	      This causes two consecutive buffer sequence numbers to
> +	      have n times frame interval in between them.</para>
> +	    </entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
>  	    <entry>0x08000000</entry>
>  	    <entry>Base event number for driver-private events.</entry>
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index fca24cc..4265102 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -2006,6 +2006,7 @@ struct v4l2_streamparm {
>  #define V4L2_EVENT_VSYNC			1
>  #define V4L2_EVENT_EOS				2
>  #define V4L2_EVENT_CTRL				3
> +#define V4L2_EVENT_FRAME_START			4
>  #define V4L2_EVENT_PRIVATE_START		0x08000000
>  
>  /* Payload for V4L2_EVENT_VSYNC */
> @@ -2032,12 +2033,17 @@ struct v4l2_event_ctrl {
>  	__s32 default_value;
>  };
>  
> +struct v4l2_event_frame_sync {
> +	__u32 buffer_sequence;
> +};
> +
>  struct v4l2_event {
>  	__u32				type;
>  	union {
> -		struct v4l2_event_vsync vsync;
> -		struct v4l2_event_ctrl	ctrl;
> -		__u8			data[64];
> +		struct v4l2_event_vsync		vsync;
> +		struct v4l2_event_ctrl		ctrl;
> +		struct v4l2_event_frame_sync	frame_sync;
> +		__u8				data[64];
>  	} u;
>  	__u32				pending;
>  	__u32				sequence;
> 

Re: [RFC 2/3] v4l: events: Define frame start event

[Sakari Ailus]

On Tue, Jul 26, 2011 at 01:52:44PM +0200, Hans Verkuil wrote:
> On Tuesday, July 19, 2011 15:38:07 Sakari Ailus wrote:
> > Define a frame start event to tell user space when the reception of a frame
> > starts.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   26 ++++++++++++++++++++
> >  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   18 +++++++++++++
> >  include/linux/videodev2.h                          |   12 +++++++--
> >  3 files changed, 53 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > index 5200b68..d2cb8db 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > @@ -88,6 +88,12 @@
> >  	  </row>
> >  	  <row>
> >  	    <entry></entry>
> > +	    <entry>&v4l2-event-frame-sync;</entry>
> > +            <entry><structfield>frame</structfield></entry>
> > +	    <entry>Event data for event V4L2_EVENT_FRAME_START.</entry>
> 
> The name of the struct and the event are not in sync (pardon the expression :-) ).
> 
> Both should either be named FRAME_SYNC or FRAME_START.

Should they be in sync? FRAME_START event is for frame start, not for other
purposes.

The buffer sequence number, however, could be used by other events, too.
This is directly related to the question of how to subscribe line-based
events. Albeit whether they are really ever needed is another question.

Getting _one_ event giving frame synchronisation timestamps is important,
however; that's also why I sent the RFC.

What I might do now is that we define a FRAME_SYNC (or FRAME_START) event
and specify the id == 0 always, and worry about the rest later on. It is
quite possible that line based events will never be needed.

If they are, then we must also specify how to subscribe them.

> > +	  </row>
> > +	  <row>
> > +	    <entry></entry>
> >  	    <entry>__u8</entry>
> >              <entry><structfield>data</structfield>[64]</entry>
> >  	    <entry>Event data. Defined by the event type. The union
> > @@ -220,6 +226,26 @@
> >        </tgroup>
> >      </table>
> >  
> > +    <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
> > +      <title>struct <structname>v4l2_event_frame_sync</structname></title>
> > +      <tgroup cols="3">
> > +	&cs-str;
> > +	<tbody valign="top">
> > +	  <row>
> > +	    <entry>__u32</entry>
> > +	    <entry><structfield>buffer_sequence</structfield></entry>
> > +	    <entry>
> > +	      The sequence number of the buffer to be handled next or
> > +	      currently handled by the driver.
> 
> So this refers to the sequence field in struct v4l2_buffer? Assuming it is,
> then you definitely need to refer to that struct.

Yes, it does.

> And to answer question 2 in the RFC: buffer_sequence is specific to this
> event, and so belongs to a v4l2_event_frame_sync struct.

Agreed.

> BTW, using 'id' to specify a specific line in the future seems reasonable to me.
> Initially id is just set to 0, meaning the start of the frame.
> 
> Regards,
> 
> 	Hans
> 
> > +	    </entry>
> > +	  </row>
> > +	</tbody>
> > +      </tgroup>
> > +    </table>
> > +
> > +    <para>&v4l2-event-frame-sync; is associated with
> > +    <constant>V4L2_EVENT_FRAME_START</constant> event.</para>
> > +
> >      <table pgwide="1" frame="none" id="changes-flags">
> >        <title>Changes</title>
> >        <tgroup cols="3">
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> > index 275be96..7ec42bb 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> > @@ -139,6 +139,24 @@
> >  	    </entry>
> >  	  </row>
> >  	  <row>
> > +	    <entry><constant>V4L2_EVENT_FRAME_START</constant></entry>
> > +	    <entry>4</entry>
> > +	    <entry>
> > +	      <para>Triggered immediately when the reception of a
> > +	      frame has begun. This event has a
> > +	      &v4l2-event-frame-sync; associated with it.</para>
> > +
> > +	      <para>A driver will only generate this event when the
> > +	      hardware can generate it. This might not be the case
> > +	      e.g. when the hardware has no DMA buffer to write the
> > +	      image data to. In such cases the
> > +	      <structfield>buffer_sequence</structfield> field in
> > +	      &v4l2-event-frame-sync; will not be incremented either.
> > +	      This causes two consecutive buffer sequence numbers to
> > +	      have n times frame interval in between them.</para>
> > +	    </entry>
> > +	  </row>
> > +	  <row>
> >  	    <entry><constant>V4L2_EVENT_PRIVATE_START</constant></entry>
> >  	    <entry>0x08000000</entry>
> >  	    <entry>Base event number for driver-private events.</entry>
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index fca24cc..4265102 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -2006,6 +2006,7 @@ struct v4l2_streamparm {
> >  #define V4L2_EVENT_VSYNC			1
> >  #define V4L2_EVENT_EOS				2
> >  #define V4L2_EVENT_CTRL				3
> > +#define V4L2_EVENT_FRAME_START			4
> >  #define V4L2_EVENT_PRIVATE_START		0x08000000
> >  
> >  /* Payload for V4L2_EVENT_VSYNC */
> > @@ -2032,12 +2033,17 @@ struct v4l2_event_ctrl {
> >  	__s32 default_value;
> >  };
> >  
> > +struct v4l2_event_frame_sync {
> > +	__u32 buffer_sequence;
> > +};
> > +
> >  struct v4l2_event {
> >  	__u32				type;
> >  	union {
> > -		struct v4l2_event_vsync vsync;
> > -		struct v4l2_event_ctrl	ctrl;
> > -		__u8			data[64];
> > +		struct v4l2_event_vsync		vsync;
> > +		struct v4l2_event_ctrl		ctrl;
> > +		struct v4l2_event_frame_sync	frame_sync;
> > +		__u8				data[64];
> >  	} u;
> >  	__u32				pending;
> >  	__u32				sequence;
> > 

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [RFC 2/3] v4l: events: Define frame start event

[Hans Verkuil]

On Tuesday, July 26, 2011 15:51:26 Sakari Ailus wrote:
> On Tue, Jul 26, 2011 at 01:52:44PM +0200, Hans Verkuil wrote:
> > On Tuesday, July 19, 2011 15:38:07 Sakari Ailus wrote:
> > > Define a frame start event to tell user space when the reception of a frame
> > > starts.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > ---
> > >  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   26 ++++++++++++++++++++
> > >  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   18 +++++++++++++
> > >  include/linux/videodev2.h                          |   12 +++++++--
> > >  3 files changed, 53 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > index 5200b68..d2cb8db 100644
> > > --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > @@ -88,6 +88,12 @@
> > >  	  </row>
> > >  	  <row>
> > >  	    <entry></entry>
> > > +	    <entry>&v4l2-event-frame-sync;</entry>
> > > +            <entry><structfield>frame</structfield></entry>
> > > +	    <entry>Event data for event V4L2_EVENT_FRAME_START.</entry>
> > 
> > The name of the struct and the event are not in sync (pardon the expression :-) ).
> > 
> > Both should either be named FRAME_SYNC or FRAME_START.
> 
> Should they be in sync? FRAME_START event is for frame start, not for other
> purposes.

Ah, you expect other events to reuse the same payload struct. I missed
that part.
 
> The buffer sequence number, however, could be used by other events, too.
> This is directly related to the question of how to subscribe line-based
> events. Albeit whether they are really ever needed is another question.
> 
> Getting _one_ event giving frame synchronisation timestamps is important,
> however; that's also why I sent the RFC.
> 
> What I might do now is that we define a FRAME_SYNC (or FRAME_START) event
> and specify the id == 0 always, and worry about the rest later on. It is
> quite possible that line based events will never be needed.

I would go for that.

> If they are, then we must also specify how to subscribe them.

Using 'id' as the line number seems sensible to me, but I would definitely
leave that part out for now. I am not convinced it is possible to use that
reliably in any case due to the difficult timing requirements.

Regards,

	Hans

Re: [RFC 2/3] v4l: events: Define frame start event

[Sakari Ailus]

Thanks for your comments, Hans!

On Tue, Jul 26, 2011 at 03:59:38PM +0200, Hans Verkuil wrote:
> On Tuesday, July 26, 2011 15:51:26 Sakari Ailus wrote:
> > On Tue, Jul 26, 2011 at 01:52:44PM +0200, Hans Verkuil wrote:
> > > On Tuesday, July 19, 2011 15:38:07 Sakari Ailus wrote:
> > > > Define a frame start event to tell user space when the reception of a frame
> > > > starts.
> > > > 
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > > ---
> > > >  Documentation/DocBook/media/v4l/vidioc-dqevent.xml |   26 ++++++++++++++++++++
> > > >  .../DocBook/media/v4l/vidioc-subscribe-event.xml   |   18 +++++++++++++
> > > >  include/linux/videodev2.h                          |   12 +++++++--
> > > >  3 files changed, 53 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > > index 5200b68..d2cb8db 100644
> > > > --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > > +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> > > > @@ -88,6 +88,12 @@
> > > >  	  </row>
> > > >  	  <row>
> > > >  	    <entry></entry>
> > > > +	    <entry>&v4l2-event-frame-sync;</entry>
> > > > +            <entry><structfield>frame</structfield></entry>
> > > > +	    <entry>Event data for event V4L2_EVENT_FRAME_START.</entry>
> > > 
> > > The name of the struct and the event are not in sync (pardon the expression :-) ).
> > > 
> > > Both should either be named FRAME_SYNC or FRAME_START.
> > 
> > Should they be in sync? FRAME_START event is for frame start, not for other
> > purposes.
> 
> Ah, you expect other events to reuse the same payload struct. I missed
> that part.
>  
> > The buffer sequence number, however, could be used by other events, too.
> > This is directly related to the question of how to subscribe line-based
> > events. Albeit whether they are really ever needed is another question.
> > 
> > Getting _one_ event giving frame synchronisation timestamps is important,
> > however; that's also why I sent the RFC.
> > 
> > What I might do now is that we define a FRAME_SYNC (or FRAME_START) event
> > and specify the id == 0 always, and worry about the rest later on. It is
> > quite possible that line based events will never be needed.
> 
> I would go for that.

Ack.

> > If they are, then we must also specify how to subscribe them.
> 
> Using 'id' as the line number seems sensible to me, but I would definitely
> leave that part out for now. I am not convinced it is possible to use that
> reliably in any case due to the difficult timing requirements.

Ah, I momentarily forgot how this works on controls. :-)

I think FRAME_SYNC is better name since it's reusable in the future as well
so I choose that one and resend the set.

Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi