V4L/DVB (6116): ivtv: VBI cleanups and fixes

Besides some VBI cleanups this patch also fixes a subtle problem with the
VBI re-insertion stream where the PIO work handler wasn't called quickly
enough, resulting in occasional corrupt data.

Furthermore the CC output didn't disable CC correctly and at the right time,
causing duplicates to be sent.

An saa7127 fix for VPS output was also added: the wrong data was sent.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>

drivers/media/video/ivtv/ivtv-driver.h

@@ -489,6 +489,15 @@ struct yuv_playback_info
 #define IVTV_VBI_FRAMES 32
 
 /* VBI data */
+struct vbi_cc {
+	u8 odd[2];	/* two-byte payload of odd field */
+	u8 even[2];	/* two-byte payload of even field */;
+};
+
+struct vbi_vps {
+	u8 data[5];	/* five-byte VPS payload */
+};
+
 struct vbi_info {
 	/* VBI general fixed card data */
 	u32 raw_decoder_line_size;              /* raw VBI line size from digitizer */
@@ -502,15 +511,14 @@ struct vbi_info {
 	u32 enc_start, enc_size;
 	int fpi;
 	u32 frame;
-	u8 cc_data_odd[256];
-	u8 cc_data_even[256];
-	int cc_pos;
-	u8 cc_no_update;
-	u8 vps[5];
-	u8 vps_found;
-	int wss;
-	u8 wss_found;
-	u8 wss_no_update;
+	struct vbi_cc cc_payload[256];		/* Sliced VBI CC payload array. It is an array to
+						   prevent dropping CC data if they couldn't be
+						   processed fast enough. */
+	int cc_payload_idx;			/* Index in cc_payload */
+	u8 cc_missing_cnt;			/* Counts number of frames without CC for passthrough mode */
+	int wss_payload;			/* Sliced VBI WSS payload */
+	u8 wss_missing_cnt;			/* Counts number of frames without WSS for passthrough mode */
+	struct vbi_vps vps_payload;		/* Sliced VBI VPS payload */
 	struct v4l2_format in;
 	/* convenience pointer to sliced struct in vbi_in union */
 	struct v4l2_sliced_vbi_format *sliced_in;

drivers/media/video/ivtv/ivtv-fileops.c

@@ -563,8 +563,11 @@ ssize_t ivtv_v4l2_write(struct file *filp, const char __user *user_buf, size_t c
 
 	/* This stream does not need to start any decoding */
 	if (s->type == IVTV_DEC_STREAM_TYPE_VOUT) {
+		int elems = count / sizeof(struct v4l2_sliced_vbi_data);
+
 		set_bit(IVTV_F_S_APPL_IO, &s->s_flags);
-		return ivtv_write_vbi(itv, user_buf, count);
+		ivtv_write_vbi(itv, (const struct v4l2_sliced_vbi_data *)user_buf, elems);
+		return elems * sizeof(struct v4l2_sliced_vbi_data);
 	}
 
 	mode = s->type == IVTV_DEC_STREAM_TYPE_MPG ? OUT_MPG : OUT_YUV;
@@ -828,10 +831,10 @@ int ivtv_v4l2_close(struct inode *inode, struct file *filp)
 		ivtv_stop_decoding(id, VIDEO_CMD_STOP_TO_BLACK | VIDEO_CMD_STOP_IMMEDIATELY, 0);
 
 		/* If all output streams are closed, and if the user doesn't have
-		   IVTV_DEC_STREAM_TYPE_VOUT open, then disable VBI on TV-out. */
+		   IVTV_DEC_STREAM_TYPE_VOUT open, then disable CC on TV-out. */
 		if (itv->output_mode == OUT_NONE && !test_bit(IVTV_F_S_APPL_IO, &s_vout->s_flags)) {
-			/* disable VBI on TV-out */
-			ivtv_disable_vbi(itv);
+			/* disable CC on TV-out */
+			ivtv_disable_cc(itv);
 		}
 	} else {
 		ivtv_stop_capture(id, 0);

drivers/media/video/ivtv/ivtv-irq.c

@@ -786,7 +786,10 @@ static void ivtv_irq_vsync(struct ivtv *itv)
 			wake_up(&s->waitq);
 
 		/* Send VBI to saa7127 */
-		if (frame) {
+		if (frame && (itv->output_mode == OUT_PASSTHROUGH ||
+			test_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags) ||
+			test_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags) ||
+			test_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags))) {
 			set_bit(IVTV_F_I_WORK_HANDLER_VBI, &itv->i_flags);
 			set_bit(IVTV_F_I_HAVE_WORK, &itv->i_flags);
 		}
@@ -809,7 +812,7 @@ static void ivtv_irq_vsync(struct ivtv *itv)
 	}
 }
 
-#define IVTV_IRQ_DMA (IVTV_IRQ_DMA_READ | IVTV_IRQ_ENC_DMA_COMPLETE | IVTV_IRQ_DMA_ERR | IVTV_IRQ_ENC_START_CAP | IVTV_IRQ_ENC_VBI_CAP | IVTV_IRQ_DEC_DATA_REQ)
+#define IVTV_IRQ_DMA (IVTV_IRQ_DMA_READ | IVTV_IRQ_ENC_DMA_COMPLETE | IVTV_IRQ_DMA_ERR | IVTV_IRQ_ENC_START_CAP | IVTV_IRQ_ENC_VBI_CAP | IVTV_IRQ_DEC_DATA_REQ | IVTV_IRQ_DEC_VBI_RE_INSERT)
 
 irqreturn_t ivtv_irq_handler(int irq, void *dev_id)
 {
@@ -942,8 +945,9 @@ irqreturn_t ivtv_irq_handler(int irq, void *dev_id)
 		}
 	}
 
-	if (test_and_clear_bit(IVTV_F_I_HAVE_WORK, &itv->i_flags))
+	if (test_and_clear_bit(IVTV_F_I_HAVE_WORK, &itv->i_flags)) {
 		queue_work(itv->irq_work_queues, &itv->irq_work_queue);
+	}
 
 	spin_unlock(&itv->dma_reg_lock);
 

drivers/media/video/ivtv/ivtv-vbi.c

@@ -23,8 +23,7 @@
 #include "ivtv-queue.h"
 #include "ivtv-vbi.h"
 
-static void ivtv_set_vps(struct ivtv *itv, int enabled, u8 vps1, u8 vps2, u8 vps3,
-		  u8 vps4, u8 vps5)
+static void ivtv_set_vps(struct ivtv *itv, int enabled)
 {
 	struct v4l2_sliced_vbi_data data;
 
@@ -33,15 +32,15 @@ static void ivtv_set_vps(struct ivtv *itv, int enabled, u8 vps1, u8 vps2, u8 vps
 	data.id = V4L2_SLICED_VPS;
 	data.field = 0;
 	data.line = enabled ? 16 : 0;
-	data.data[4] = vps1;
-	data.data[10] = vps2;
-	data.data[11] = vps3;
-	data.data[12] = vps4;
-	data.data[13] = vps5;
+	data.data[2] = itv->vbi.vps_payload.data[0];
+	data.data[8] = itv->vbi.vps_payload.data[1];
+	data.data[9] = itv->vbi.vps_payload.data[2];
+	data.data[10] = itv->vbi.vps_payload.data[3];
+	data.data[11] = itv->vbi.vps_payload.data[4];
 	ivtv_saa7127(itv, VIDIOC_INT_S_VBI_DATA, &data);
 }
 
-static void ivtv_set_cc(struct ivtv *itv, int mode, u8 cc1, u8 cc2, u8 cc3, u8 cc4)
+static void ivtv_set_cc(struct ivtv *itv, int mode, const struct vbi_cc *cc)
 {
 	struct v4l2_sliced_vbi_data data;
 
@@ -50,13 +49,13 @@ static void ivtv_set_cc(struct ivtv *itv, int mode, u8 cc1, u8 cc2, u8 cc3, u8 c
 	data.id = V4L2_SLICED_CAPTION_525;
 	data.field = 0;
 	data.line = (mode & 1) ? 21 : 0;
-	data.data[0] = cc1;
-	data.data[1] = cc2;
+	data.data[0] = cc->odd[0];
+	data.data[1] = cc->odd[1];
 	ivtv_saa7127(itv, VIDIOC_INT_S_VBI_DATA, &data);
 	data.field = 1;
 	data.line = (mode & 2) ? 21 : 0;
-	data.data[0] = cc3;
-	data.data[1] = cc4;
+	data.data[0] = cc->even[0];
+	data.data[1] = cc->even[1];
 	ivtv_saa7127(itv, VIDIOC_INT_S_VBI_DATA, &data);
 }
 
@@ -92,62 +91,50 @@ static int odd_parity(u8 c)
 	return c & 1;
 }
 
-static void passthrough_vbi_data(struct ivtv *itv, int cnt)
+void ivtv_write_vbi(struct ivtv *itv, const struct v4l2_sliced_vbi_data *sliced, size_t cnt)
 {
-	int wss = 0;
-	u8 cc[4] = { 0x80, 0x80, 0x80, 0x80 };
-	u8 vps[13];
+	struct vbi_info *vi = &itv->vbi;
+	struct vbi_cc cc = { .odd = { 0x80, 0x80 }, .even = { 0x80, 0x80 } };
 	int found_cc = 0;
-	int found_wss = 0;
-	int found_vps = 0;
-	int cc_pos = itv->vbi.cc_pos;
-	int i;
+	size_t i;
 
 	for (i = 0; i < cnt; i++) {
-		struct v4l2_sliced_vbi_data *d = itv->vbi.sliced_dec_data + i;
+		const struct v4l2_sliced_vbi_data *d = sliced + i;
 
 		if (d->id == V4L2_SLICED_CAPTION_525 && d->line == 21) {
-			found_cc = 1;
 			if (d->field) {
-				cc[2] = d->data[0];
-				cc[3] = d->data[1];
+				cc.even[0] = d->data[0];
+				cc.even[1] = d->data[1];
 			} else {
-				cc[0] = d->data[0];
-				cc[1] = d->data[1];
+				cc.odd[0] = d->data[0];
+				cc.odd[1] = d->data[1];
 			}
+			found_cc = 1;
 		}
 		else if (d->id == V4L2_SLICED_VPS && d->line == 16 && d->field == 0) {
-			memcpy(vps, d->data, sizeof(vps));
-			found_vps = 1;
+			struct vbi_vps vps;
+
+			vps.data[0] = d->data[2];
+			vps.data[1] = d->data[8];
+			vps.data[2] = d->data[9];
+			vps.data[3] = d->data[10];
+			vps.data[4] = d->data[11];
+			if (memcmp(&vps, &vi->vps_payload, sizeof(vps))) {
+				vi->vps_payload = vps;
+				set_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags);
+			}
 		}
 		else if (d->id == V4L2_SLICED_WSS_625 && d->line == 23 && d->field == 0) {
-			wss = d->data[0] | d->data[1] << 8;
-			found_wss = 1;
-		}
-	}
+			int wss = d->data[0] | d->data[1] << 8;
 
-	if (itv->vbi.wss_found != found_wss || itv->vbi.wss != wss) {
-		itv->vbi.wss = wss;
-		itv->vbi.wss_found = found_wss;
-		set_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags);
-	}
-
-	if (found_vps || itv->vbi.vps_found) {
-		itv->vbi.vps[0] = vps[2];
-		itv->vbi.vps[1] = vps[8];
-		itv->vbi.vps[2] = vps[9];
-		itv->vbi.vps[3] = vps[10];
-		itv->vbi.vps[4] = vps[11];
-		itv->vbi.vps_found = found_vps;
-		set_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags);
+			if (vi->wss_payload != wss) {
+				vi->wss_payload = wss;
+				set_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags);
+			}
+		}
 	}
-
-	if (found_cc && cc_pos < sizeof(itv->vbi.cc_data_even)) {
-		itv->vbi.cc_data_odd[cc_pos] = cc[0];
-		itv->vbi.cc_data_odd[cc_pos + 1] = cc[1];
-		itv->vbi.cc_data_even[cc_pos] = cc[2];
-		itv->vbi.cc_data_even[cc_pos + 1] = cc[3];
-		itv->vbi.cc_pos = cc_pos + 2;
+	if (found_cc && vi->cc_payload_idx < sizeof(vi->cc_payload)) {
+		vi->cc_payload[vi->cc_payload_idx++] = cc;
 		set_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags);
 	}
 }
@@ -271,69 +258,6 @@ static int ivtv_convert_ivtv_vbi(struct ivtv *itv, u8 *p)
 	return line * sizeof(itv->vbi.sliced_dec_data[0]);
 }
 
-ssize_t ivtv_write_vbi(struct ivtv *itv, const char __user *ubuf, size_t count)
-{
-	/* Should be a __user pointer, but sparse doesn't parse this bit correctly. */
-	const struct v4l2_sliced_vbi_data *p = (const struct v4l2_sliced_vbi_data *)ubuf;
-	u8 cc[4] = { 0x80, 0x80, 0x80, 0x80 };
-	int found_cc = 0;
-	int cc_pos = itv->vbi.cc_pos;
-
-	while (count >= sizeof(struct v4l2_sliced_vbi_data)) {
-		switch (p->id) {
-		case V4L2_SLICED_CAPTION_525:
-			if (p->line == 21) {
-				found_cc = 1;
-				if (p->field) {
-					cc[2] = p->data[0];
-					cc[3] = p->data[1];
-				} else {
-					cc[0] = p->data[0];
-					cc[1] = p->data[1];
-				}
-			}
-			break;
-
-		case V4L2_SLICED_VPS:
-			if (p->line == 16 && p->field == 0) {
-				itv->vbi.vps[0] = p->data[2];
-				itv->vbi.vps[1] = p->data[8];
-				itv->vbi.vps[2] = p->data[9];
-				itv->vbi.vps[3] = p->data[10];
-				itv->vbi.vps[4] = p->data[11];
-				itv->vbi.vps_found = 1;
-				set_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags);
-			}
-			break;
-
-		case V4L2_SLICED_WSS_625:
-			if (p->line == 23 && p->field == 0) {
-				/* No lock needed for WSS */
-				itv->vbi.wss = p->data[0] | (p->data[1] << 8);
-				itv->vbi.wss_found = 1;
-				set_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags);
-			}
-			break;
-
-		default:
-			break;
-		}
-		count -= sizeof(*p);
-		p++;
-	}
-
-	if (found_cc && cc_pos < sizeof(itv->vbi.cc_data_even)) {
-		itv->vbi.cc_data_odd[cc_pos] = cc[0];
-		itv->vbi.cc_data_odd[cc_pos + 1] = cc[1];
-		itv->vbi.cc_data_even[cc_pos] = cc[2];
-		itv->vbi.cc_data_even[cc_pos + 1] = cc[3];
-		itv->vbi.cc_pos = cc_pos + 2;
-		set_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags);
-	}
-
-	return (const char __user *)p - ubuf;
-}
-
 /* Compress raw VBI format, removes leading SAV codes and surplus space after the
    field.
    Returns new compressed size. */
@@ -482,108 +406,95 @@ void ivtv_process_vbi_data(struct ivtv *itv, struct ivtv_buffer *buf,
 		memcpy(buf->buf, itv->vbi.sliced_dec_data, cnt);
 		buf->bytesused = cnt;
 
-		passthrough_vbi_data(itv, cnt / sizeof(itv->vbi.sliced_dec_data[0]));
+		ivtv_write_vbi(itv, itv->vbi.sliced_dec_data,
+			       cnt / sizeof(itv->vbi.sliced_dec_data[0]));
 		return;
 	}
 }
 
-void ivtv_disable_vbi(struct ivtv *itv)
+void ivtv_disable_cc(struct ivtv *itv)
 {
-	clear_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags);
-	clear_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags);
+	struct vbi_cc cc = { .odd = { 0x80, 0x80 }, .even = { 0x80, 0x80 } };
+
 	clear_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags);
-	ivtv_set_wss(itv, 0, 0);
-	ivtv_set_cc(itv, 0, 0, 0, 0, 0);
-	ivtv_set_vps(itv, 0, 0, 0, 0, 0, 0);
-	itv->vbi.vps_found = itv->vbi.wss_found = 0;
-	itv->vbi.wss = 0;
-	itv->vbi.cc_pos = 0;
+	ivtv_set_cc(itv, 0, &cc);
+	itv->vbi.cc_payload_idx = 0;
 }
 
 
 void ivtv_vbi_work_handler(struct ivtv *itv)
 {
+	struct vbi_info *vi = &itv->vbi;
 	struct v4l2_sliced_vbi_data data;
+	struct vbi_cc cc = { .odd = { 0x80, 0x80 }, .even = { 0x80, 0x80 } };
 
 	/* Lock */
 	if (itv->output_mode == OUT_PASSTHROUGH) {
-		/* Note: currently only the saa7115 is used in a PVR350,
-		   so these commands are for now saa7115 specific. */
 		if (itv->is_50hz) {
 			data.id = V4L2_SLICED_WSS_625;
 			data.field = 0;
 
 			if (itv->video_dec_func(itv, VIDIOC_INT_G_VBI_DATA, &data) == 0) {
 				ivtv_set_wss(itv, 1, data.data[0] & 0xf);
-				itv->vbi.wss_no_update = 0;
-			} else if (itv->vbi.wss_no_update == 4) {
+				vi->wss_missing_cnt = 0;
+			} else if (vi->wss_missing_cnt == 4) {
 				ivtv_set_wss(itv, 1, 0x8);  /* 4x3 full format */
 			} else {
-				itv->vbi.wss_no_update++;
+				vi->wss_missing_cnt++;
 			}
 		}
 		else {
-			u8 c1 = 0, c2 = 0, c3 = 0, c4 = 0;
 			int mode = 0;
 
 			data.id = V4L2_SLICED_CAPTION_525;
 			data.field = 0;
 			if (itv->video_dec_func(itv, VIDIOC_INT_G_VBI_DATA, &data) == 0) {
 				mode |= 1;
-				c1 = data.data[0];
-				c2 = data.data[1];
+				cc.odd[0] = data.data[0];
+				cc.odd[1] = data.data[1];
 			}
 			data.field = 1;
 			if (itv->video_dec_func(itv, VIDIOC_INT_G_VBI_DATA, &data) == 0) {
 				mode |= 2;
-				c3 = data.data[0];
-				c4 = data.data[1];
+				cc.even[0] = data.data[0];
+				cc.even[1] = data.data[1];
 			}
 			if (mode) {
-				itv->vbi.cc_no_update = 0;
-				ivtv_set_cc(itv, mode, c1, c2, c3, c4);
-			} else if (itv->vbi.cc_no_update == 4) {
-				ivtv_set_cc(itv, 0, 0, 0, 0, 0);
+				vi->cc_missing_cnt = 0;
+				ivtv_set_cc(itv, mode, &cc);
+			} else if (vi->cc_missing_cnt == 4) {
+				ivtv_set_cc(itv, 0, &cc);
 			} else {
-				itv->vbi.cc_no_update++;
+				vi->cc_missing_cnt++;
 			}
 		}
 		return;
 	}
 
 	if (test_and_clear_bit(IVTV_F_I_UPDATE_WSS, &itv->i_flags)) {
-		/* Lock */
-		ivtv_set_wss(itv, itv->vbi.wss_found, itv->vbi.wss & 0xf);
+		ivtv_set_wss(itv, 1, vi->wss_payload & 0xf);
 	}
 
-	if (test_and_clear_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags)) {
-		if (itv->vbi.cc_pos == 0) {
-			ivtv_set_cc(itv, 3, 0x80, 0x80, 0x80, 0x80);
+	if (test_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags)) {
+		if (vi->cc_payload_idx == 0) {
+			clear_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags);
+			ivtv_set_cc(itv, 3, &cc);
 		}
-		while (itv->vbi.cc_pos) {
-			u8 cc_odd0 = itv->vbi.cc_data_odd[0];
-			u8 cc_odd1 = itv->vbi.cc_data_odd[1];
-			u8 cc_even0 = itv->vbi.cc_data_even[0];
-			u8 cc_even1 = itv->vbi.cc_data_even[1];
-
-			memcpy(itv->vbi.cc_data_odd, itv->vbi.cc_data_odd + 2, sizeof(itv->vbi.cc_data_odd) - 2);
-			memcpy(itv->vbi.cc_data_even, itv->vbi.cc_data_even + 2, sizeof(itv->vbi.cc_data_even) - 2);
-			itv->vbi.cc_pos -= 2;
-			if (itv->vbi.cc_pos && cc_odd0 == 0x80 && cc_odd1 == 0x80)
+		while (vi->cc_payload_idx) {
+			cc = vi->cc_payload[0];
+
+			memcpy(vi->cc_payload, vi->cc_payload + 1,
+					sizeof(vi->cc_payload) - sizeof(vi->cc_payload[0]));
+			vi->cc_payload_idx--;
+			if (vi->cc_payload_idx && cc.odd[0] == 0x80 && cc.odd[1] == 0x80)
 				continue;
 
-			/* Send to Saa7127 */
-			ivtv_set_cc(itv, 3, cc_odd0, cc_odd1, cc_even0, cc_even1);
-			if (itv->vbi.cc_pos == 0)
-				set_bit(IVTV_F_I_UPDATE_CC, &itv->i_flags);
+			ivtv_set_cc(itv, 3, &cc);
 			break;
 		}
 	}
 
 	if (test_and_clear_bit(IVTV_F_I_UPDATE_VPS, &itv->i_flags)) {
-		/* Lock */
-		ivtv_set_vps(itv, itv->vbi.vps_found,
-			itv->vbi.vps[0], itv->vbi.vps[1],
-			itv->vbi.vps[2], itv->vbi.vps[3], itv->vbi.vps[4]);
+		ivtv_set_vps(itv, 1);
 	}
 }

drivers/media/video/ivtv/ivtv-vbi.h

@@ -20,11 +20,11 @@
 #ifndef IVTV_VBI_H
 #define IVTV_VBI_H
 
-ssize_t ivtv_write_vbi(struct ivtv *itv, const char __user *ubuf, size_t count);
+void ivtv_write_vbi(struct ivtv *itv, const struct v4l2_sliced_vbi_data *sliced, size_t count);
 void ivtv_process_vbi_data(struct ivtv *itv, struct ivtv_buffer *buf,
 			   u64 pts_stamp, int streamtype);
 int ivtv_used_line(struct ivtv *itv, int line, int field);
-void ivtv_disable_vbi(struct ivtv *itv);
+void ivtv_disable_cc(struct ivtv *itv);
 void ivtv_set_vbi(unsigned long arg);
 void ivtv_vbi_work_handler(struct ivtv *itv);
 

drivers/media/video/saa7127.c

@@ -332,11 +332,11 @@ static int saa7127_set_vps(struct i2c_client *client, struct v4l2_sliced_vbi_dat
 	if (!enable)
 		return 0;
 
-	state->vps_data[0] = data->data[4];
-	state->vps_data[1] = data->data[10];
-	state->vps_data[2] = data->data[11];
-	state->vps_data[3] = data->data[12];
-	state->vps_data[4] = data->data[13];
+	state->vps_data[0] = data->data[2];
+	state->vps_data[1] = data->data[8];
+	state->vps_data[2] = data->data[9];
+	state->vps_data[3] = data->data[10];
+	state->vps_data[4] = data->data[11];
 	v4l_dbg(1, debug, client, "Set VPS data %02x %02x %02x %02x %02x\n",
 		state->vps_data[0], state->vps_data[1],
 		state->vps_data[2], state->vps_data[3],