Skip to contentSkip to conversation

bttv: convert to vb2

PATCH Superseded Deborah Brouwer 2 participants 17 messages May 2, 2023 – May 26, 2023

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover bttv: convert to vb2

This series converts the bttv driver to vb2.

Compliance test results are the same as in v1:
https://lore.kernel.org/linux-media/[email protected]/

Changes since v1:
- revised commit messages in patches 04/13 and 05/13 to clearly explain treatment of format/width/height and certain vbi fields

- In patch "media: bttv: refactor bttv_set_dma()": drop superfluous parentheses and rename the argument 'override' as 'start_capture' to be more descriptive of its function.

- In patch "media: bttv: convert to vb2":
- use BT848_CAP_CTL_* when calling bttv_set_dma()
- fix compiler warning that vb2_queue may be uninitialized
- fix vbi sequence counter to avoid incrementing it twice
if both video and vbi are streaming

Deborah Brouwer (13):
media: bttv: use video_drvdata to get bttv
media: bttv: replace BUG with WARN_ON
media: bttv: radio use v4l2_fh instead of bttv_fh
media: bttv: copy vid fmt/width/height from fh
media: bttv: copy vbi_fmt from bttv_fh
media: bttv: move do_crop flag out of bttv_fh
media: bttv: remove format field from bttv_buffer
media: bttv: remove tvnorm field from bttv_buffer
media: bttv: remove crop info from bttv_buffer
media: bttv: move vbi_skip/vbi_count out of buffer
media: bttv: refactor bttv_set_dma()
media: bttv: use audio defaults for winfast2000
media: bttv: convert to vb2

 drivers/media/pci/bt8xx/Kconfig           |   2 +-
 drivers/media/pci/bt8xx/bttv-audio-hook.c |  10 +-
 drivers/media/pci/bt8xx/bttv-driver.c     | 994 +++++++---------------
 drivers/media/pci/bt8xx/bttv-risc.c       | 414 +++++----
 drivers/media/pci/bt8xx/bttv-vbi.c        | 267 +++---
 drivers/media/pci/bt8xx/bttvp.h           |  78 +-
 6 files changed, 683 insertions(+), 1082 deletions(-)
Signature · 1 line
2.39.2
#1 lore raw

PATCH v2 1/13 media: bttv: use video_drvdata to get bttv

Use video_drvdata(file) instead of fh->btv to get the bttv pointer wherever the bttv pointer will still be needed after the vb2 conversion.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 77 ++++++++++-----------------
 drivers/media/pci/bt8xx/bttv-vbi.c    |  8 +--
 2 files changed, 32 insertions(+), 53 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +28 -49
index 734f02b91aa3..0d5fdb8479d5 100644
@@ -1654,8 +1654,7 @@ static void radio_enable(struct bttv *btv)
 
 static int bttv_s_std(struct file *file, void *priv, v4l2_std_id id)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	unsigned int i;
 
 	for (i = 0; i < BTTV_TVNORMS; i++)
@@ -1670,8 +1669,7 @@ static int bttv_s_std(struct file *file, void *priv, v4l2_std_id id)
 
 static int bttv_g_std(struct file *file, void *priv, v4l2_std_id *id)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	*id = btv->std;
 	return 0;
@@ -1679,8 +1677,7 @@ static int bttv_g_std(struct file *file, void *priv, v4l2_std_id *id)
 
 static int bttv_querystd(struct file *file, void *f, v4l2_std_id *id)
 {
-	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (btread(BT848_DSTATUS) & BT848_DSTATUS_NUML)
 		*id &= V4L2_STD_625_50;
@@ -1692,8 +1689,7 @@ static int bttv_querystd(struct file *file, void *f, v4l2_std_id *id)
 static int bttv_enum_input(struct file *file, void *priv,
 					struct v4l2_input *i)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (i->index >= bttv_tvcards[btv->c.type].video_inputs)
 		return -EINVAL;
@@ -1725,8 +1721,7 @@ static int bttv_enum_input(struct file *file, void *priv,
 
 static int bttv_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	*i = btv->input;
 
@@ -1735,8 +1730,7 @@ static int bttv_g_input(struct file *file, void *priv, unsigned int *i)
 
 static int bttv_s_input(struct file *file, void *priv, unsigned int i)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (i >= bttv_tvcards[btv->c.type].video_inputs)
 		return -EINVAL;
@@ -1748,8 +1742,7 @@ static int bttv_s_input(struct file *file, void *priv, unsigned int i)
 static int bttv_s_tuner(struct file *file, void *priv,
 					const struct v4l2_tuner *t)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (t->index)
 		return -EINVAL;
@@ -1767,8 +1760,7 @@ static int bttv_s_tuner(struct file *file, void *priv,
 static int bttv_g_frequency(struct file *file, void *priv,
 					struct v4l2_frequency *f)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (f->tuner)
 		return -EINVAL;
@@ -1804,8 +1796,7 @@ static void bttv_set_frequency(struct bttv *btv, const struct v4l2_frequency *f)
 static int bttv_s_frequency(struct file *file, void *priv,
 					const struct v4l2_frequency *f)
 {
-	struct bttv_fh *fh  = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (f->tuner)
 		return -EINVAL;
@@ -1817,8 +1808,7 @@ static int bttv_s_frequency(struct file *file, void *priv,
 static int bttv_log_status(struct file *file, void *f)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct bttv_fh *fh  = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	v4l2_ctrl_handler_log_status(vdev->ctrl_handler, btv->c.v4l2_dev.name);
 	bttv_call_all(btv, core, log_status);
@@ -1829,8 +1819,7 @@ static int bttv_log_status(struct file *file, void *f)
 static int bttv_g_register(struct file *file, void *f,
 					struct v4l2_dbg_register *reg)
 {
-	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	/* bt848 has a 12-bit register space */
 	reg->reg &= 0xfff;
@@ -1843,8 +1832,7 @@ static int bttv_g_register(struct file *file, void *f,
 static int bttv_s_register(struct file *file, void *f,
 					const struct v4l2_dbg_register *reg)
 {
-	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	/* bt848 has a 12-bit register space */
 	btwrite(reg->val, reg->reg & 0xfff);
@@ -2106,7 +2094,7 @@ static int bttv_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	const struct bttv_format *fmt;
 	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	enum v4l2_field field;
 	__s32 width, height;
 	__s32 height2;
@@ -2165,7 +2153,7 @@ static int bttv_s_fmt_vid_cap(struct file *file, void *priv,
 	int retval;
 	const struct bttv_format *fmt;
 	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	__s32 width, height;
 	unsigned int width_mask, width_bias;
 	enum v4l2_field field;
@@ -2209,8 +2197,7 @@ static int bttv_s_fmt_vid_cap(struct file *file, void *priv,
 static int bttv_querycap(struct file *file, void  *priv,
 				struct v4l2_capability *cap)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (0 == v4l2)
 		return -EINVAL;
@@ -2274,7 +2261,7 @@ static int bttv_querybuf(struct file *file, void *priv,
 static int bttv_qbuf(struct file *file, void *priv, struct v4l2_buffer *b)
 {
 	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	int res = bttv_resource(fh);
 
 	if (!check_alloc_btres_lock(btv, fh, res))
@@ -2322,8 +2309,7 @@ static int bttv_streamoff(struct file *file, void *priv,
 static int bttv_g_parm(struct file *file, void *f,
 				struct v4l2_streamparm *parm)
 {
-	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (parm->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
@@ -2337,8 +2323,7 @@ static int bttv_g_parm(struct file *file, void *f,
 static int bttv_g_tuner(struct file *file, void *priv,
 				struct v4l2_tuner *t)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;
@@ -2360,8 +2345,7 @@ static int bttv_g_tuner(struct file *file, void *priv,
 static int bttv_g_pixelaspect(struct file *file, void *priv,
 			      int type, struct v4l2_fract *f)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
@@ -2374,7 +2358,7 @@ static int bttv_g_pixelaspect(struct file *file, void *priv,
 static int bttv_g_selection(struct file *file, void *f, struct v4l2_selection *sel)
 {
 	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
@@ -2404,7 +2388,7 @@ static int bttv_g_selection(struct file *file, void *f, struct v4l2_selection *s
 static int bttv_s_selection(struct file *file, void *f, struct v4l2_selection *sel)
 {
 	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	const struct v4l2_rect *b;
 	int retval;
 	struct bttv_crop c;
@@ -2781,7 +2765,7 @@ static int radio_open(struct file *file)
 static int radio_release(struct file *file)
 {
 	struct bttv_fh *fh = file->private_data;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	struct saa6588_command cmd;
 
 	file->private_data = NULL;
@@ -2800,8 +2784,7 @@ static int radio_release(struct file *file)
 
 static int radio_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;
@@ -2823,8 +2806,7 @@ static int radio_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 static int radio_s_tuner(struct file *file, void *priv,
 					const struct v4l2_tuner *t)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (0 != t->index)
 		return -EINVAL;
@@ -2837,8 +2819,7 @@ static int radio_s_tuner(struct file *file, void *priv,
 static int radio_s_hw_freq_seek(struct file *file, void *priv,
 					const struct v4l2_hw_freq_seek *a)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (btv->has_tea575x)
 		return snd_tea575x_s_hw_freq_seek(file, &btv->tea, a);
@@ -2849,8 +2830,7 @@ static int radio_s_hw_freq_seek(struct file *file, void *priv,
 static int radio_enum_freq_bands(struct file *file, void *priv,
 					 struct v4l2_frequency_band *band)
 {
-	struct bttv_fh *fh = priv;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 
 	if (btv->has_tea575x)
 		return snd_tea575x_enum_freq_bands(&btv->tea, band);
@@ -2861,8 +2841,7 @@ static int radio_enum_freq_bands(struct file *file, void *priv,
 static ssize_t radio_read(struct file *file, char __user *data,
 			 size_t count, loff_t *ppos)
 {
-	struct bttv_fh *fh = file->private_data;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	struct saa6588_command cmd;
 
 	cmd.block_count = count / 3;
@@ -2880,7 +2859,7 @@ static ssize_t radio_read(struct file *file, char __user *data,
 static __poll_t radio_poll(struct file *file, poll_table *wait)
 {
 	struct bttv_fh *fh = file->private_data;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	__poll_t req_events = poll_requested_events(wait);
 	struct saa6588_command cmd;
 	__poll_t res = 0;
drivers/media/pci/bt8xx/bttv-vbi.c +4 -4
index ce36a2c0f60b..24b28a05bfd9 100644
@@ -299,8 +299,7 @@ static int try_fmt(struct v4l2_vbi_format *f, const struct bttv_tvnorm *tvnorm,
 
 int bttv_try_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 {
-	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	const struct bttv_tvnorm *tvnorm;
 	__s32 crop_start;
 
@@ -318,7 +317,7 @@ int bttv_try_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 int bttv_s_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 {
 	struct bttv_fh *fh = f;
-	struct bttv *btv = fh->btv;
+	struct bttv *btv = video_drvdata(file);
 	const struct bttv_tvnorm *tvnorm;
 	__s32 start1, end;
 	int rc;
@@ -367,10 +366,11 @@ int bttv_g_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 {
 	struct bttv_fh *fh = f;
 	const struct bttv_tvnorm *tvnorm;
+	struct bttv *btv = video_drvdata(file);
 
 	frt->fmt.vbi = fh->vbi_fmt.fmt;
 
-	tvnorm = &bttv_tvnorms[fh->btv->tvnorm];
+	tvnorm = &bttv_tvnorms[btv->tvnorm];
 
 	if (tvnorm != fh->vbi_fmt.tvnorm) {
 		__s32 max_end;
Signature · 1 line
2.39.2
#2 lore raw

PATCH v2 2/13 media: bttv: replace BUG with WARN_ON

In reply to Deborah Brouwer

Both BUG and BUG_ON are replaced with WARN_ON wherever they would still be present after the vb2 conversion. WARN_ON is sufficient in these cases.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 10 +++++-----
 drivers/media/pci/bt8xx/bttv-risc.c   |  8 ++++----
 drivers/media/pci/bt8xx/bttv-vbi.c    |  6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +5 -5
index 0d5fdb8479d5..6e19d3d35ffb 100644
@@ -1111,8 +1111,8 @@ set_tvnorm(struct bttv *btv, unsigned int norm)
 	const struct bttv_tvnorm *tvnorm;
 	v4l2_std_id id;
 
-	BUG_ON(norm >= BTTV_TVNORMS);
-	BUG_ON(btv->tvnorm >= BTTV_TVNORMS);
+	WARN_ON(norm >= BTTV_TVNORMS);
+	WARN_ON(btv->tvnorm >= BTTV_TVNORMS);
 
 	tvnorm = &bttv_tvnorms[norm];
 
@@ -1910,8 +1910,8 @@ limit_scaled_size_lock       (struct bttv_fh *               fh,
 	__s32 max_height;
 	int rc;
 
-	BUG_ON((int) width_mask >= 0 ||
-	       width_bias >= (unsigned int) -width_mask);
+	WARN_ON((int)width_mask >= 0 ||
+		width_bias >= (unsigned int)(-width_mask));
 
 	/* Make sure tvnorm, vbi_end and the current cropping parameters
 	   remain consistent until we're done. */
@@ -2026,7 +2026,7 @@ static int bttv_resource(struct bttv_fh *fh)
 		res = RESOURCE_VBI;
 		break;
 	default:
-		BUG();
+		WARN_ON(1);
 	}
 	return res;
 }
drivers/media/pci/bt8xx/bttv-risc.c +4 -4
index 4fa4b9da9634..fae8b10de7a9 100644
@@ -106,7 +106,7 @@ bttv_risc_packed(struct bttv *btv, struct btcx_riscmem *risc,
 
 	/* save pointer to jmp instruction address */
 	risc->jmp = rp;
-	BUG_ON((risc->jmp - risc->cpu + 2) * sizeof(*risc->cpu) > risc->size);
+	WARN_ON((risc->jmp - risc->cpu + 2) * sizeof(*risc->cpu) > risc->size);
 	return 0;
 }
 
@@ -227,7 +227,7 @@ bttv_risc_planar(struct bttv *btv, struct btcx_riscmem *risc,
 
 	/* save pointer to jmp instruction address */
 	risc->jmp = rp;
-	BUG_ON((risc->jmp - risc->cpu + 2) * sizeof(*risc->cpu) > risc->size);
+	WARN_ON((risc->jmp - risc->cpu + 2) * sizeof(*risc->cpu) > risc->size);
 	return 0;
 }
 
@@ -646,7 +646,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 					 bpf,bpl,0,0,buf->vb.height >> 1);
 			break;
 		default:
-			BUG();
+			WARN_ON(1);
 		}
 	}
 
@@ -737,7 +737,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 					 0);
 			break;
 		default:
-			BUG();
+			WARN_ON(1);
 		}
 	}
 
drivers/media/pci/bt8xx/bttv-vbi.c +3 -3
index 24b28a05bfd9..0b05309d1fb7 100644
@@ -250,7 +250,7 @@ static int try_fmt(struct v4l2_vbi_format *f, const struct bttv_tvnorm *tvnorm,
 	if (min_start > max_start)
 		return -EBUSY;
 
-	BUG_ON(max_start >= max_end);
+	WARN_ON(max_start >= max_end);
 
 	f->sampling_rate    = tvnorm->Fsc;
 	f->samples_per_line = VBI_BPL;
@@ -430,8 +430,8 @@ void bttv_vbi_fmt_reset(struct bttv_vbi_fmt *f, unsigned int norm)
 	real_count              = ((tvnorm->cropcap.defrect.top >> 1)
 				   - tvnorm->vbistart[0]);
 
-	BUG_ON(real_samples_per_line > VBI_BPL);
-	BUG_ON(real_count > VBI_DEFLINES);
+	WARN_ON(real_samples_per_line > VBI_BPL);
+	WARN_ON(real_count > VBI_DEFLINES);
 
 	f->tvnorm               = tvnorm;
 
Signature · 1 line
2.39.2
#3 lore raw

PATCH v2 3/13 media: bttv: radio use v4l2_fh instead of bttv_fh

In reply to Deborah Brouwer

Use a v4l2_fh when opening a radio device instead of a bttv_fh and manage it with v4l2_fh_open() and v4l2_fh_release() and v4l2_ctrl_poll(). This eliminates bttv_fh from the radio in preparation for vb2 conversion which stops using separate bttv file handles altogether.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 37 ++++++++-------------------
 1 file changed, 10 insertions(+), 27 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +10 -27
index 6e19d3d35ffb..e59f40dfccc3 100644
@@ -2740,45 +2740,34 @@ static int radio_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct bttv *btv = video_drvdata(file);
-	struct bttv_fh *fh;
+	int ret = v4l2_fh_open(file);
 
-	dprintk("open dev=%s\n", video_device_node_name(vdev));
+	if (ret)
+		return ret;
 
+	dprintk("open dev=%s\n", video_device_node_name(vdev));
 	dprintk("%d: open called (radio)\n", btv->c.nr);
 
-	/* allocate per filehandle data */
-	fh = kmalloc(sizeof(*fh), GFP_KERNEL);
-	if (unlikely(!fh))
-		return -ENOMEM;
-	file->private_data = fh;
-	*fh = btv->init;
-	v4l2_fh_init(&fh->fh, vdev);
-
 	btv->radio_user++;
 	audio_mute(btv, btv->mute);
 
-	v4l2_fh_add(&fh->fh);
-
 	return 0;
 }
 
 static int radio_release(struct file *file)
 {
-	struct bttv_fh *fh = file->private_data;
 	struct bttv *btv = video_drvdata(file);
 	struct saa6588_command cmd;
 
-	file->private_data = NULL;
-	v4l2_fh_del(&fh->fh);
-	v4l2_fh_exit(&fh->fh);
-	kfree(fh);
-
 	btv->radio_user--;
 
 	bttv_call_all(btv, core, command, SAA6588_CMD_CLOSE, &cmd);
 
 	if (btv->radio_user == 0)
 		btv->has_radio_tuner = 0;
+
+	v4l2_fh_release(file);
+
 	return 0;
 }
 
@@ -2858,23 +2847,17 @@ static ssize_t radio_read(struct file *file, char __user *data,
 
 static __poll_t radio_poll(struct file *file, poll_table *wait)
 {
-	struct bttv_fh *fh = file->private_data;
 	struct bttv *btv = video_drvdata(file);
-	__poll_t req_events = poll_requested_events(wait);
 	struct saa6588_command cmd;
-	__poll_t res = 0;
+	__poll_t rc = v4l2_ctrl_poll(file, wait);
 
-	if (v4l2_event_pending(&fh->fh))
-		res = EPOLLPRI;
-	else if (req_events & EPOLLPRI)
-		poll_wait(file, &fh->fh.wait, wait);
 	radio_enable(btv);
 	cmd.instance = file;
 	cmd.event_list = wait;
-	cmd.poll_mask = res;
+	cmd.poll_mask = 0;
 	bttv_call_all(btv, core, command, SAA6588_CMD_POLL, &cmd);
 
-	return cmd.poll_mask;
+	return rc | cmd.poll_mask;
 }
 
 static const struct v4l2_file_operations radio_fops =
Signature · 1 line
2.39.2
#4 lore raw

PATCH v2 4/13 media: bttv: copy vid fmt/width/height from fh

In reply to Deborah Brouwer

In preparation for the vb2 conversion, copy the video format, width and height fields from struct bttv_fh and add them to the main struct bttv. Use these fields from struct bttv wherever they will be needed after the vb2 conversion which stops using separate bttv file handles altogether. To avoid changing more code than necessary, just leave the video format, width and height fields in separate file handles wherever the code will be subsequently removed by vb2.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 34 +++++++++++++--------------
 drivers/media/pci/bt8xx/bttvp.h       |  3 +++
 2 files changed, 20 insertions(+), 17 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +17 -17
index e59f40dfccc3..7e7658a7ed40 100644
@@ -2066,11 +2066,11 @@ static int bttv_g_fmt_vid_cap(struct file *file, void *priv,
 					struct v4l2_format *f)
 {
 	struct bttv_fh *fh  = priv;
+	struct bttv *btv = video_drvdata(file);
 
-	pix_format_set_size(&f->fmt.pix, fh->fmt,
-				fh->width, fh->height);
+	pix_format_set_size(&f->fmt.pix, btv->fmt, btv->width, btv->height);
 	f->fmt.pix.field        = fh->cap.field;
-	f->fmt.pix.pixelformat  = fh->fmt->fourcc;
+	f->fmt.pix.pixelformat  = btv->fmt->fourcc;
 	f->fmt.pix.colorspace   = V4L2_COLORSPACE_SMPTE170M;
 
 	return 0;
@@ -2190,6 +2190,9 @@ static int bttv_s_fmt_vid_cap(struct file *file, void *priv,
 	btv->init.fmt        = fmt;
 	btv->init.width      = f->fmt.pix.width;
 	btv->init.height     = f->fmt.pix.height;
+	btv->fmt = fmt;
+	btv->width = f->fmt.pix.width;
+	btv->height = f->fmt.pix.height;
 
 	return 0;
 }
@@ -2446,21 +2449,15 @@ static int bttv_s_selection(struct file *file, void *f, struct v4l2_selection *s
 
 	fh->do_crop = 1;
 
-	if (fh->width < c.min_scaled_width) {
-		fh->width = c.min_scaled_width;
-		btv->init.width = c.min_scaled_width;
-	} else if (fh->width > c.max_scaled_width) {
-		fh->width = c.max_scaled_width;
-		btv->init.width = c.max_scaled_width;
-	}
+	if (btv->width < c.min_scaled_width)
+		btv->width = c.min_scaled_width;
+	else if (btv->width > c.max_scaled_width)
+		btv->width = c.max_scaled_width;
 
-	if (fh->height < c.min_scaled_height) {
-		fh->height = c.min_scaled_height;
-		btv->init.height = c.min_scaled_height;
-	} else if (fh->height > c.max_scaled_height) {
-		fh->height = c.max_scaled_height;
-		btv->init.height = c.max_scaled_height;
-	}
+	if (btv->height < c.min_scaled_height)
+		btv->height = c.min_scaled_height;
+	else if (btv->height > c.max_scaled_height)
+		btv->height = c.max_scaled_height;
 
 	return 0;
 }
@@ -3636,6 +3633,9 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id)
 	btv->init.fmt         = format_by_fourcc(V4L2_PIX_FMT_BGR24);
 	btv->init.width       = 320;
 	btv->init.height      = 240;
+	btv->fmt = format_by_fourcc(V4L2_PIX_FMT_BGR24);
+	btv->width = 320;
+	btv->height = 240;
 	btv->input = 0;
 
 	v4l2_ctrl_new_std(hdl, &bttv_ctrl_ops,
drivers/media/pci/bt8xx/bttvp.h +3 -0
index 717f002a41df..7f02dd5866d7 100644
@@ -449,6 +449,9 @@ struct bttv {
 
 	unsigned int users;
 	struct bttv_fh init;
+	const struct bttv_format *fmt;
+	int width;
+	int height;
 
 	/* used to make dvb-bt8xx autoloadable */
 	struct work_struct request_module_wk;
Signature · 1 line
2.39.2
#5 lore raw

PATCH v2 5/13 media: bttv: copy vbi_fmt from bttv_fh

In reply to Deborah Brouwer

In preparation for the vb2 conversion, copy the vbi format from struct bttv_fh and add it to the main struct bttv. Use vbi format from struct bttv wherever it will be needed after the vb2 conversion which stops using separate bttv file handles altogether. To avoid changing more code than necessary, just leave the vbi format in separate file handles wherever it will be subsequently removed by vb2.

---
 drivers/media/pci/bt8xx/bttv-driver.c |  6 ++++--
 drivers/media/pci/bt8xx/bttv-vbi.c    | 13 +++++++------
 drivers/media/pci/bt8xx/bttvp.h       |  1 +
 3 files changed, 12 insertions(+), 8 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +4 -2
index 7e7658a7ed40..f9a6f671277f 100644
@@ -672,12 +672,12 @@ int check_alloc_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bit)
 		   Claim scan lines crop[].rect.top to bottom. */
 		btv->crop_start = top;
 	} else if (bit & VBI_RESOURCES) {
-		__s32 end = fh->vbi_fmt.end;
+		__s32 end = btv->vbi_fmt.end;
 
 		if (end > btv->crop_start)
 			goto fail;
 
-		/* Claim scan lines above fh->vbi_fmt.end. */
+		/* Claim scan lines above btv->vbi_fmt.end. */
 		btv->vbi_end = end;
 	}
 
@@ -3637,6 +3637,8 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id)
 	btv->width = 320;
 	btv->height = 240;
 	btv->input = 0;
+	btv->tvnorm = 0; /* Index into bttv_tvnorms[] i.e. PAL. */
+	bttv_vbi_fmt_reset(&btv->vbi_fmt, btv->tvnorm);
 
 	v4l2_ctrl_new_std(hdl, &bttv_ctrl_ops,
 			V4L2_CID_BRIGHTNESS, 0, 0xff00, 0x100, 32768);
drivers/media/pci/bt8xx/bttv-vbi.c +7 -6
index 0b05309d1fb7..4e7fd9a78ace 100644
@@ -350,6 +350,9 @@ int bttv_s_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 	fh->vbi_fmt.fmt    = frt->fmt.vbi;
 	fh->vbi_fmt.tvnorm = tvnorm;
 	fh->vbi_fmt.end    = end;
+	btv->vbi_fmt.fmt = frt->fmt.vbi;
+	btv->vbi_fmt.tvnorm = tvnorm;
+	btv->vbi_fmt.end = end;
 
 	mutex_unlock(&fh->vbi.vb_lock);
 
@@ -364,15 +367,14 @@ int bttv_s_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 
 int bttv_g_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 {
-	struct bttv_fh *fh = f;
 	const struct bttv_tvnorm *tvnorm;
 	struct bttv *btv = video_drvdata(file);
 
-	frt->fmt.vbi = fh->vbi_fmt.fmt;
+	frt->fmt.vbi = btv->vbi_fmt.fmt;
 
 	tvnorm = &bttv_tvnorms[btv->tvnorm];
 
-	if (tvnorm != fh->vbi_fmt.tvnorm) {
+	if (tvnorm != btv->vbi_fmt.tvnorm) {
 		__s32 max_end;
 		unsigned int i;
 
@@ -388,9 +390,8 @@ int bttv_g_fmt_vbi_cap(struct file *file, void *f, struct v4l2_format *frt)
 		for (i = 0; i < 2; ++i) {
 			__s32 new_start;
 
-			new_start = frt->fmt.vbi.start[i]
-				+ tvnorm->vbistart[i]
-				- fh->vbi_fmt.tvnorm->vbistart[i];
+			new_start = frt->fmt.vbi.start[i] + tvnorm->vbistart[i]
+				- btv->vbi_fmt.tvnorm->vbistart[i];
 
 			frt->fmt.vbi.start[i] = min(new_start, max_end - 1);
 			frt->fmt.vbi.count[i] =
drivers/media/pci/bt8xx/bttvp.h +1 -0
index 7f02dd5866d7..a36817dfaaec 100644
@@ -452,6 +452,7 @@ struct bttv {
 	const struct bttv_format *fmt;
 	int width;
 	int height;
+	struct bttv_vbi_fmt vbi_fmt;
 
 	/* used to make dvb-bt8xx autoloadable */
 	struct work_struct request_module_wk;
Signature · 1 line
2.39.2
#6 lore raw

PATCH v2 6/13 media: bttv: move do_crop flag out of bttv_fh

In reply to Deborah Brouwer

The do_crop flag indicates whether a cropping rectangle has been set. Instead of storing this flag separately in each file handle, move do_crop to struct bttv in preparation for vb2 conversion which stops using separate bttv file handles.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 24 +++++++++---------------
 drivers/media/pci/bt8xx/bttvp.h       |  5 ++---
 2 files changed, 11 insertions(+), 18 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +9 -15
index f9a6f671277f..af295ce3e9f0 100644
@@ -663,7 +663,7 @@ int check_alloc_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bit)
 	if ((bit & VIDEO_RESOURCES)
 	    && 0 == (btv->resources & VIDEO_RESOURCES)) {
 		/* Do crop - use current, don't - use default parameters. */
-		__s32 top = btv->crop[!!fh->do_crop].rect.top;
+		__s32 top = btv->crop[!!btv->do_crop].rect.top;
 
 		if (btv->vbi_end > top)
 			goto fail;
@@ -1493,7 +1493,6 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 			       unsigned int width, unsigned int height,
 			       enum v4l2_field field)
 {
-	struct bttv_fh *fh = q->priv_data;
 	int redo_dma_risc = 0;
 	struct bttv_crop c;
 	int norm;
@@ -1523,7 +1522,7 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 		c.rect = bttv_tvnorms[norm].cropcap.defrect;
 	} else {
 		norm = btv->tvnorm;
-		c = btv->crop[!!fh->do_crop];
+		c = btv->crop[!!btv->do_crop];
 
 		if (width < c.min_scaled_width ||
 		    width > c.max_scaled_width ||
@@ -1919,9 +1918,9 @@ limit_scaled_size_lock       (struct bttv_fh *               fh,
 	b = &bttv_tvnorms[btv->tvnorm].cropcap.bounds;
 
 	/* Do crop - use current, don't - use default parameters. */
-	c = &btv->crop[!!fh->do_crop];
+	c = &btv->crop[!!btv->do_crop];
 
-	if (fh->do_crop
+	if (btv->do_crop
 	    && adjust_size
 	    && adjust_crop
 	    && !locked_btres(btv, VIDEO_RESOURCES)) {
@@ -2121,7 +2120,7 @@ static int bttv_try_fmt_vid_cap(struct file *file, void *priv,
 		}
 		fallthrough;
 	default: /* FIELD_ANY case */
-		height2 = btv->crop[!!fh->do_crop].rect.height >> 1;
+		height2 = btv->crop[!!btv->do_crop].rect.height >> 1;
 		field = (f->fmt.pix.height > height2)
 			? V4L2_FIELD_INTERLACED
 			: V4L2_FIELD_BOTTOM;
@@ -2360,7 +2359,6 @@ static int bttv_g_pixelaspect(struct file *file, void *priv,
 
 static int bttv_g_selection(struct file *file, void *f, struct v4l2_selection *sel)
 {
-	struct bttv_fh *fh = f;
 	struct bttv *btv = video_drvdata(file);
 
 	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -2368,12 +2366,7 @@ static int bttv_g_selection(struct file *file, void *f, struct v4l2_selection *s
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP:
-		/*
-		 * No fh->do_crop = 1; because btv->crop[1] may be
-		 * inconsistent with fh->width or fh->height and apps
-		 * do not expect a change here.
-		 */
-		sel->r = btv->crop[!!fh->do_crop].rect;
+		sel->r = btv->crop[!!btv->do_crop].rect;
 		break;
 	case V4L2_SEL_TGT_CROP_DEFAULT:
 		sel->r = bttv_tvnorms[btv->tvnorm].cropcap.defrect;
@@ -2447,7 +2440,7 @@ static int bttv_s_selection(struct file *file, void *f, struct v4l2_selection *s
 
 	btv->crop[1] = c;
 
-	fh->do_crop = 1;
+	btv->do_crop = 1;
 
 	if (btv->width < c.min_scaled_width)
 		btv->width = c.min_scaled_width;
@@ -2610,7 +2603,7 @@ static int bttv_open(struct file *file)
 	   current video standard, and VIDIOC_S_FMT will not implicitly
 	   change the cropping parameters until VIDIOC_S_SELECTION has been
 	   called. */
-	fh->do_crop = !reset_crop; /* module parameter */
+	btv->do_crop = !reset_crop; /* module parameter */
 
 	/* Likewise there should be one global set of VBI capture
 	   parameters, but for compatibility with V4L apps and earlier
@@ -3639,6 +3632,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id)
 	btv->input = 0;
 	btv->tvnorm = 0; /* Index into bttv_tvnorms[] i.e. PAL. */
 	bttv_vbi_fmt_reset(&btv->vbi_fmt, btv->tvnorm);
+	btv->do_crop = 0;
 
 	v4l2_ctrl_new_std(hdl, &bttv_ctrl_ops,
 			V4L2_CID_BRIGHTNESS, 0, 0xff00, 0x100, 32768);
drivers/media/pci/bt8xx/bttvp.h +2 -3
index a36817dfaaec..48317c5935d1 100644
@@ -206,9 +206,6 @@ struct bttv_fh {
 	int                      width;
 	int                      height;
 
-	/* Application called VIDIOC_S_SELECTION. */
-	int                      do_crop;
-
 	/* vbi capture */
 	struct videobuf_queue    vbi;
 	/* Current VBI capture window as seen through this fh (cannot
@@ -453,6 +450,8 @@ struct bttv {
 	int width;
 	int height;
 	struct bttv_vbi_fmt vbi_fmt;
+	/* Application called VIDIOC_S_SELECTION. */
+	int do_crop;
 
 	/* used to make dvb-bt8xx autoloadable */
 	struct work_struct request_module_wk;
Signature · 1 line
2.39.2
#7 lore raw

PATCH v2 7/13 media: bttv: remove format field from bttv_buffer

In reply to Deborah Brouwer

Instead of storing the format (video or vbi) in each bttv buffer separately, just use the global bttv format because the format does not change per buffer.

---
 drivers/media/pci/bt8xx/bttv-driver.c |  3 +-
 drivers/media/pci/bt8xx/bttv-risc.c   | 52 +++++++++++++--------------
 drivers/media/pci/bt8xx/bttvp.h       |  1 -
 3 files changed, 27 insertions(+), 29 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +1 -2
index af295ce3e9f0..0032e1436111 100644
@@ -1553,7 +1553,7 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 	/* alloc + fill struct bttv_buffer (if changed) */
 	if (buf->vb.width != width || buf->vb.height != height ||
 	    buf->vb.field != field ||
-	    buf->tvnorm != norm || buf->fmt != fmt ||
+	    buf->tvnorm != norm || btv->fmt != fmt ||
 	    buf->crop.top != c.rect.top ||
 	    buf->crop.left != c.rect.left ||
 	    buf->crop.width != c.rect.width ||
@@ -1562,7 +1562,6 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 		buf->vb.height = height;
 		buf->vb.field  = field;
 		buf->tvnorm    = norm;
-		buf->fmt       = fmt;
 		buf->crop      = c.rect;
 		redo_dma_risc = 1;
 	}
drivers/media/pci/bt8xx/bttv-risc.c +26 -26

132 lines not on this page · raw message

drivers/media/pci/bt8xx/bttvp.h +0 -1
index 48317c5935d1..402fe1f1846e 100644
@@ -145,7 +145,6 @@ struct bttv_buffer {
 	struct videobuf_buffer     vb;
 
 	/* bttv specific */
-	const struct bttv_format   *fmt;
 	unsigned int               tvnorm;
 	int                        btformat;
 	int                        btswap;
Signature · 1 line
2.39.2
#8 lore raw

PATCH v2 8/13 media: bttv: remove tvnorm field from bttv_buffer

In reply to Deborah Brouwer

Instead of storing the tvnorm in each bttv buffer separately, just use the global bttv tvnorm because the tvnorm does not change per buffer.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 4 ++--
 drivers/media/pci/bt8xx/bttv-risc.c   | 2 +-
 drivers/media/pci/bt8xx/bttvp.h       | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +2 -2
index 0032e1436111..f32050849691 100644
@@ -1553,7 +1553,7 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 	/* alloc + fill struct bttv_buffer (if changed) */
 	if (buf->vb.width != width || buf->vb.height != height ||
 	    buf->vb.field != field ||
-	    buf->tvnorm != norm || btv->fmt != fmt ||
+	    btv->tvnorm != norm || btv->fmt != fmt ||
 	    buf->crop.top != c.rect.top ||
 	    buf->crop.left != c.rect.left ||
 	    buf->crop.width != c.rect.width ||
@@ -1561,7 +1561,7 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 		buf->vb.width  = width;
 		buf->vb.height = height;
 		buf->vb.field  = field;
-		buf->tvnorm    = norm;
+		btv->tvnorm    = norm;
 		buf->crop      = c.rect;
 		redo_dma_risc = 1;
 	}
drivers/media/pci/bt8xx/bttv-risc.c +1 -1
index 67ea7ed42623..0a296dc1a7a5 100644
@@ -606,7 +606,7 @@ bttv_buffer_activate_video(struct bttv *btv,
 int
 bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 {
-	const struct bttv_tvnorm *tvnorm = bttv_tvnorms + buf->tvnorm;
+	const struct bttv_tvnorm *tvnorm = bttv_tvnorms + btv->tvnorm;
 	struct videobuf_dmabuf *dma=videobuf_to_dma(&buf->vb);
 
 	dprintk("%d: buffer field: %s  format: 0x%08x  size: %dx%d\n",
drivers/media/pci/bt8xx/bttvp.h +0 -1
index 402fe1f1846e..bbb5fc060fad 100644
@@ -145,7 +145,6 @@ struct bttv_buffer {
 	struct videobuf_buffer     vb;
 
 	/* bttv specific */
-	unsigned int               tvnorm;
 	int                        btformat;
 	int                        btswap;
 	struct bttv_geometry       geo;
Signature · 1 line
2.39.2
#9 lore raw

PATCH v2 9/13 media: bttv: remove crop info from bttv_buffer

In reply to Deborah Brouwer

Instead of storing the cropping parameters in each bttv buffer separately, just use the global bttv crop because it won't change per buffer.

---
 drivers/media/pci/bt8xx/bttv-driver.c | 10 +++++-----
 drivers/media/pci/bt8xx/bttv-risc.c   | 12 ++++++------
 drivers/media/pci/bt8xx/bttvp.h       |  1 -
 3 files changed, 11 insertions(+), 12 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +5 -5
index f32050849691..15825fedb4e0 100644
@@ -1554,15 +1554,15 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
 	if (buf->vb.width != width || buf->vb.height != height ||
 	    buf->vb.field != field ||
 	    btv->tvnorm != norm || btv->fmt != fmt ||
-	    buf->crop.top != c.rect.top ||
-	    buf->crop.left != c.rect.left ||
-	    buf->crop.width != c.rect.width ||
-	    buf->crop.height != c.rect.height) {
+	    btv->crop[!!btv->do_crop].rect.top != c.rect.top ||
+	    btv->crop[!!btv->do_crop].rect.left != c.rect.left ||
+	    btv->crop[!!btv->do_crop].rect.width != c.rect.width ||
+	    btv->crop[!!btv->do_crop].rect.height != c.rect.height) {
 		buf->vb.width  = width;
 		buf->vb.height = height;
 		buf->vb.field  = field;
 		btv->tvnorm    = norm;
-		buf->crop      = c.rect;
+		btv->crop[!!btv->do_crop].rect = c.rect;
 		redo_dma_risc = 1;
 	}
 
drivers/media/pci/bt8xx/bttv-risc.c +6 -6
index 0a296dc1a7a5..e9bc6bcc7333 100644
@@ -620,7 +620,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 
 		bttv_calc_geo(btv,&buf->geo,buf->vb.width,buf->vb.height,
 			      V4L2_FIELD_HAS_BOTH(buf->vb.field),
-			      tvnorm,&buf->crop);
+			      tvnorm, &btv->crop[!!btv->do_crop].rect);
 
 		switch (buf->vb.field) {
 		case V4L2_FIELD_TOP:
@@ -674,7 +674,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 		case V4L2_FIELD_TOP:
 			bttv_calc_geo(btv,&buf->geo,buf->vb.width,
 				      buf->vb.height,/* both_fields */ 0,
-				      tvnorm,&buf->crop);
+				      tvnorm, &btv->crop[!!btv->do_crop].rect);
 			bttv_risc_planar(btv, &buf->top, dma->sglist,
 					 0,buf->vb.width,0,buf->vb.height,
 					 uoffset, voffset, btv->fmt->hshift,
@@ -683,7 +683,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 		case V4L2_FIELD_BOTTOM:
 			bttv_calc_geo(btv,&buf->geo,buf->vb.width,
 				      buf->vb.height,0,
-				      tvnorm,&buf->crop);
+				      tvnorm, &btv->crop[!!btv->do_crop].rect);
 			bttv_risc_planar(btv, &buf->bottom, dma->sglist,
 					 0,buf->vb.width,0,buf->vb.height,
 					 uoffset, voffset, btv->fmt->hshift,
@@ -692,7 +692,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 		case V4L2_FIELD_INTERLACED:
 			bttv_calc_geo(btv,&buf->geo,buf->vb.width,
 				      buf->vb.height,1,
-				      tvnorm,&buf->crop);
+				      tvnorm, &btv->crop[!!btv->do_crop].rect);
 			lines    = buf->vb.height >> 1;
 			ypadding = buf->vb.width;
 			cpadding = buf->vb.width >> btv->fmt->hshift;
@@ -715,7 +715,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 		case V4L2_FIELD_SEQ_TB:
 			bttv_calc_geo(btv,&buf->geo,buf->vb.width,
 				      buf->vb.height,1,
-				      tvnorm,&buf->crop);
+				      tvnorm, &btv->crop[!!btv->do_crop].rect);
 			lines    = buf->vb.height >> 1;
 			ypadding = buf->vb.width;
 			cpadding = buf->vb.width >> btv->fmt->hshift;
@@ -746,7 +746,7 @@ bttv_buffer_risc(struct bttv *btv, struct bttv_buffer *buf)
 		/* build risc code */
 		buf->vb.field = V4L2_FIELD_SEQ_TB;
 		bttv_calc_geo(btv,&buf->geo,tvnorm->swidth,tvnorm->sheight,
-			      1,tvnorm,&buf->crop);
+			      1, tvnorm, &btv->crop[!!btv->do_crop].rect);
 		bttv_risc_packed(btv, &buf->top,  dma->sglist,
 				 /* offset */ 0, RAW_BPL, /* padding */ 0,
 				 /* skip_lines */ 0, RAW_LINES);
drivers/media/pci/bt8xx/bttvp.h +0 -1
index bbb5fc060fad..b5bb69ab8ab0 100644
@@ -150,7 +150,6 @@ struct bttv_buffer {
 	struct bttv_geometry       geo;
 	struct btcx_riscmem        top;
 	struct btcx_riscmem        bottom;
-	struct v4l2_rect           crop;
 	unsigned int               vbi_skip[2];
 	unsigned int               vbi_count[2];
 };
Signature · 1 line
2.39.2
#10 lore raw

PATCH v2 10/13 media: bttv: move vbi_skip/vbi_count out of buffer

In reply to Deborah Brouwer

Instead of storing vbi_skip and vbi_count in each bttv buffer separately, move them to the main bttv struct as they won't change per buffer.

---
 drivers/media/pci/bt8xx/bttv-driver.c |  2 ++
 drivers/media/pci/bt8xx/bttv-risc.c   |  4 ++--
 drivers/media/pci/bt8xx/bttv-vbi.c    | 16 ++++++++--------
 drivers/media/pci/bt8xx/bttvp.h       |  7 +++++--
 4 files changed, 17 insertions(+), 12 deletions(-)
drivers/media/pci/bt8xx/bttv-driver.c +2 -0
index 15825fedb4e0..4d1e0743f193 100644
@@ -3631,6 +3631,8 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id)
 	btv->input = 0;
 	btv->tvnorm = 0; /* Index into bttv_tvnorms[] i.e. PAL. */
 	bttv_vbi_fmt_reset(&btv->vbi_fmt, btv->tvnorm);
+	btv->vbi_count[0] = VBI_DEFLINES;
+	btv->vbi_count[1] = VBI_DEFLINES;
 	btv->do_crop = 0;
 
 	v4l2_ctrl_new_std(hdl, &bttv_ctrl_ops,
drivers/media/pci/bt8xx/bttv-risc.c +2 -2
index e9bc6bcc7333..97248e340a28 100644
@@ -525,12 +525,12 @@ bttv_buffer_activate_vbi(struct bttv *btv,
 			btwrite(crop,	BT848_O_CROP);
 		}
 
-		if (vbi->vbi_count[0] > 0) {
+		if (btv->vbi_count[0] > 0) {
 			top = &vbi->top;
 			top_irq_flags = 4;
 		}
 
-		if (vbi->vbi_count[1] > 0) {
+		if (btv->vbi_count[1] > 0) {
 			top_irq_flags = 0;
 			bottom = &vbi->bottom;
 			bottom_irq_flags = 4;
drivers/media/pci/bt8xx/bttv-vbi.c +8 -8
index 4e7fd9a78ace..2fd990039adf 100644
@@ -125,14 +125,14 @@ static int vbi_buffer_prepare(struct videobuf_queue *q,
 
 	redo_dma_risc = 0;
 
-	if (buf->vbi_skip[0] != skip_lines0 ||
-	    buf->vbi_skip[1] != skip_lines1 ||
-	    buf->vbi_count[0] != fh->vbi_fmt.fmt.count[0] ||
-	    buf->vbi_count[1] != fh->vbi_fmt.fmt.count[1]) {
-		buf->vbi_skip[0] = skip_lines0;
-		buf->vbi_skip[1] = skip_lines1;
-		buf->vbi_count[0] = fh->vbi_fmt.fmt.count[0];
-		buf->vbi_count[1] = fh->vbi_fmt.fmt.count[1];
+	if (btv->vbi_skip[0] != skip_lines0 ||
+	    btv->vbi_skip[1] != skip_lines1 ||
+	    btv->vbi_count[0] != fh->vbi_fmt.fmt.count[0] ||
+	    btv->vbi_count[1] != fh->vbi_fmt.fmt.count[1]) {
+		btv->vbi_skip[0] = skip_lines0;
+		btv->vbi_skip[1] = skip_lines1;
+		btv->vbi_count[0] = fh->vbi_fmt.fmt.count[0];
+		btv->vbi_count[1] = fh->vbi_fmt.fmt.count[1];
 		redo_dma_risc = 1;
 	}
 
drivers/media/pci/bt8xx/bttvp.h +5 -2
index b5bb69ab8ab0..bce2401de9bd 100644
@@ -150,8 +150,6 @@ struct bttv_buffer {
 	struct bttv_geometry       geo;
 	struct btcx_riscmem        top;
 	struct btcx_riscmem        bottom;
-	unsigned int               vbi_skip[2];
-	unsigned int               vbi_count[2];
 };
 
 struct bttv_buffer_set {
@@ -239,6 +237,8 @@ void bttv_dma_free(struct videobuf_queue *q, struct bttv *btv,
 /* ---------------------------------------------------------- */
 /* bttv-vbi.c                                                 */
 
+#define VBI_DEFLINES 16
+
 int bttv_try_fmt_vbi_cap(struct file *file, void *fh, struct v4l2_format *f);
 int bttv_g_fmt_vbi_cap(struct file *file, void *fh, struct v4l2_format *f);
 int bttv_s_fmt_vbi_cap(struct file *file, void *fh, struct v4l2_format *f);
@@ -447,6 +447,9 @@ struct bttv {
 	int width;
 	int height;
 	struct bttv_vbi_fmt vbi_fmt;
+	unsigned int vbi_skip[2];
+	unsigned int vbi_count[2];
+
 	/* Application called VIDIOC_S_SELECTION. */
 	int do_crop;
 
Signature · 1 line
2.39.2
#11 lore raw

PATCH v2 11/13 media: bttv: refactor bttv_set_dma()

In reply to Deborah Brouwer

Break bttv_set_dma() into several smaller, separate functions so it is easier to read the risc and dma code. Replace numeric values with descriptive macros. Also remove the unused field btv->cap_ctl.

---
 drivers/media/pci/bt8xx/bttv-risc.c | 111 ++++++++++++++++++----------
 drivers/media/pci/bt8xx/bttvp.h     |   1 -
 2 files changed, 71 insertions(+), 41 deletions(-)
drivers/media/pci/bt8xx/bttv-risc.c +71 -40
index 97248e340a28..3e0dac56de54 100644
@@ -360,21 +360,80 @@ bttv_apply_geo(struct bttv *btv, struct bttv_geometry *geo, int odd)
 /* ---------------------------------------------------------- */
 /* risc group / risc main loop / dma management               */
 
-void
-bttv_set_dma(struct bttv *btv, int override)
+static void bttv_set_risc_status(struct bttv *btv)
 {
-	unsigned long cmd;
-	int capctl;
+	unsigned long cmd = BT848_RISC_JUMP;
+	/*
+	 * The value of btv->loop_irq sets or resets the RISC_STATUS for video
+	 * and/or vbi by setting the value of bits [23:16] in the first dword
+	 * of the JUMP instruction:
+	 * video risc: set (1) and reset (~1)
+	 * vbi risc: set(4) and reset (~4)
+	 */
+	if (btv->loop_irq) {
+		cmd |= BT848_RISC_IRQ;
+		cmd |= (btv->loop_irq  & 0x0f) << 16;
+		cmd |= (~btv->loop_irq & 0x0f) << 20;
+	}
+	btv->main.cpu[RISC_SLOT_LOOP] = cpu_to_le32(cmd);
+}
+
+static void bttv_set_irq_timer(struct bttv *btv)
+{
+	if (btv->curr.frame_irq || btv->loop_irq || btv->cvbi)
+		mod_timer(&btv->timeout, jiffies + BTTV_TIMEOUT);
+	else
+		del_timer(&btv->timeout);
+}
+
+static int bttv_set_capture_control(struct bttv *btv, int start_capture)
+{
+	int capctl = 0;
+
+	if (btv->curr.top || btv->curr.bottom)
+		capctl = BT848_CAP_CTL_CAPTURE_ODD |
+			 BT848_CAP_CTL_CAPTURE_EVEN;
+
+	if (btv->cvbi)
+		capctl |= BT848_CAP_CTL_CAPTURE_VBI_ODD |
+			  BT848_CAP_CTL_CAPTURE_VBI_EVEN;
+
+	capctl |= start_capture;
+
+	btaor(capctl, ~0x0f, BT848_CAP_CTL);
+
+	return capctl;
+}
+
+static void bttv_start_dma(struct bttv *btv)
+{
+	if (btv->dma_on)
+		return;
+	btwrite(btv->main.dma, BT848_RISC_STRT_ADD);
+	btor(0x3, BT848_GPIO_DMA_CTL);
+	btv->dma_on = 1;
+}
+
+static void bttv_stop_dma(struct bttv *btv)
+{
+	if (!btv->dma_on)
+		return;
+	btand(~0x3, BT848_GPIO_DMA_CTL);
+	btv->dma_on = 0;
+}
+
+void bttv_set_dma(struct bttv *btv, int start_capture)
+{
+	int capctl = 0;
 
-	btv->cap_ctl = 0;
-	if (NULL != btv->curr.top)      btv->cap_ctl |= 0x02;
-	if (NULL != btv->curr.bottom)   btv->cap_ctl |= 0x01;
-	if (NULL != btv->cvbi)          btv->cap_ctl |= 0x0c;
+	bttv_set_risc_status(btv);
+	bttv_set_irq_timer(btv);
+	capctl = bttv_set_capture_control(btv, start_capture);
 
-	capctl  = 0;
-	capctl |= (btv->cap_ctl & 0x03) ? 0x03 : 0x00;  /* capture  */
-	capctl |= (btv->cap_ctl & 0x0c) ? 0x0c : 0x00;  /* vbi data */
-	capctl |= override;
+	if (capctl)
+		bttv_start_dma(btv);
+	else
+		bttv_stop_dma(btv);
 
 	d2printk("%d: capctl=%x lirq=%d top=%08llx/%08llx even=%08llx/%08llx\n",
 		 btv->c.nr,capctl,btv->loop_irq,
@@ -382,34 +441,6 @@ bttv_set_dma(struct bttv *btv, int override)
 		 btv->curr.top     ? (unsigned long long)btv->curr.top->top.dma        : 0,
 		 btv->cvbi         ? (unsigned long long)btv->cvbi->bottom.dma         : 0,
 		 btv->curr.bottom  ? (unsigned long long)btv->curr.bottom->bottom.dma  : 0);
-
-	cmd = BT848_RISC_JUMP;
-	if (btv->loop_irq) {
-		cmd |= BT848_RISC_IRQ;
-		cmd |= (btv->loop_irq  & 0x0f) << 16;
-		cmd |= (~btv->loop_irq & 0x0f) << 20;
-	}
-	if (btv->curr.frame_irq || btv->loop_irq || btv->cvbi) {
-		mod_timer(&btv->timeout, jiffies+BTTV_TIMEOUT);
-	} else {
-		del_timer(&btv->timeout);
-	}
-	btv->main.cpu[RISC_SLOT_LOOP] = cpu_to_le32(cmd);
-
-	btaor(capctl, ~0x0f, BT848_CAP_CTL);
-	if (capctl) {
-		if (btv->dma_on)
-			return;
-		btwrite(btv->main.dma, BT848_RISC_STRT_ADD);
-		btor(3, BT848_GPIO_DMA_CTL);
-		btv->dma_on = 1;
-	} else {
-		if (!btv->dma_on)
-			return;
-		btand(~3, BT848_GPIO_DMA_CTL);
-		btv->dma_on = 0;
-	}
-	return;
 }
 
 int
drivers/media/pci/bt8xx/bttvp.h +0 -1
index bce2401de9bd..b750dfbc75cc 100644
@@ -430,7 +430,6 @@ struct bttv {
 	int                     loop_irq;
 	int                     new_input;
 
-	unsigned long cap_ctl;
 	unsigned long dma_on;
 	struct timer_list timeout;
 	struct bttv_suspend_state state;
Signature · 1 line
2.39.2
#12 lore raw

PATCH v2 12/13 media: bttv: use audio defaults for winfast2000

In reply to Deborah Brouwer

The winfast2000 card advertised rxsubchans that weren't compatible with its default audmode. Just use the default audmode (V4L2_TUNER_MODE_MONO) and default audio reception flag (V4L2_TUNER_SUB_MONO) for this card. Fixes compliance test failures.

---
 drivers/media/pci/bt8xx/bttv-audio-hook.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
drivers/media/pci/bt8xx/bttv-audio-hook.c +1 -9
index da1914a20b81..b5d071835354 100644
@@ -293,16 +293,8 @@ void winfast2000_audio(struct bttv *btv, struct v4l2_tuner *t, int set)
 {
 	unsigned long val;
 
-	if (!set) {
-		/* Not much to do here */
-		t->audmode = V4L2_TUNER_MODE_LANG1;
-		t->rxsubchans = V4L2_TUNER_SUB_MONO |
-				V4L2_TUNER_SUB_STEREO |
-				V4L2_TUNER_SUB_LANG1 |
-				V4L2_TUNER_SUB_LANG2;
-
+	if (!set)
 		return;
-	}
 
 	/*btor (0xc32000, BT848_GPIO_OUT_EN);*/
 	switch (t->audmode) {
Signature · 1 line
2.39.2
#13 lore raw

PATCH v2 13/13 media: bttv: convert to vb2

In reply to Deborah Brouwer

Convert this driver from the old videobuf framework to videobuf2.

---
 drivers/media/pci/bt8xx/Kconfig       |   2 +-
 drivers/media/pci/bt8xx/bttv-driver.c | 809 ++++++++------------------
 drivers/media/pci/bt8xx/bttv-risc.c   | 265 +++++----
 drivers/media/pci/bt8xx/bttv-vbi.c    | 240 +++-----
 drivers/media/pci/bt8xx/bttvp.h       |  60 +-
 5 files changed, 496 insertions(+), 880 deletions(-)
drivers/media/pci/bt8xx/Kconfig +1 -1
index 2d674dc28cec..2f77628246e9 100644
@@ -3,7 +3,7 @@ config VIDEO_BT848
 	tristate "BT848 Video For Linux"
 	depends on PCI && I2C && VIDEO_DEV
 	select I2C_ALGOBIT
-	select VIDEOBUF_DMA_SG
+	select VIDEOBUF2_DMA_SG
 	depends on RC_CORE
 	depends on MEDIA_RADIO_SUPPORT
 	select VIDEO_TUNER
drivers/media/pci/bt8xx/bttv-driver.c +240 -569

1,109 lines not on this page · raw message

drivers/media/pci/bt8xx/bttv-risc.c +141 -124

351 lines not on this page · raw message

drivers/media/pci/bt8xx/bttv-vbi.c +84 -156

296 lines not on this page · raw message

drivers/media/pci/bt8xx/bttvp.h +30 -30

124 lines not on this page · raw message

Signature · 1 line
2.39.2
#14 lore raw

Re: [PATCH v2 13/13] media: bttv: convert to vb2

Hi Deb,

When testing this I was wondering why the sequence counter didn't detect dropped frames (which happens when you start/stop a vbi stream while streaming video).

Quoted 6 lines · Deborah Brouwer
drivers/media/pci/bt8xx/bttv-driver.c +0 -6
-static void bttv_field_count(struct bttv *btv)
-{
-	int need_count = 0;
-
-	if (btv->users)
-		need_count++;

drivers/media/pci/bt8xx/bttv-driver.c void bttv_gpio_tracking(struct bttv *btv, char *comment) · line 1471

Quoted 10 lines
drivers/media/pci/bt8xx/bttv-driver.c +0 -10
-
-	if (need_count) {
-		/* start field counter */
-		btor(BT848_INT_VSYNC,BT848_INT_MASK);
-	} else {
-		/* stop field counter */
-		btand(~BT848_INT_VSYNC,BT848_INT_MASK);
-		btv->field_count = 0;
-	}
-}

This is the root cause: this function is used to turn on VSYNC interrupts and in the interrupt handler the field_count is incremented.

In the vb1 version of this driver this field_count is passed on to vb1, which uses it to set the sequence counter to field_count / 2.

By removing this function the VSYNC irq is never enabled and so field_count is always 0. So I think in bttv the seqnr field should be dropped and the field_count mechanism re-instated.

Comparing the number of dropped frames when starting/stopping vbi it looks like in both cases (vb1 and vb2) one frame is dropped when starting vbi. But when stopping vbi no frames are dropped in the vb1 case, but 3 in the vb2 case.

Another thing I discovered is that for PAL the vcr_hack control has to be enabled, otherwise the video is full of glitches. This was present before your series, and happens even with a video signal from a proper PAL video generator, so this is really strange. I can't remember that I needed this in the past, but it has been years since I last tested it.

PAL capture is fine for Top/Bottom/Alternate settings, it only fails for Interlaced and Sequential Top/Bottom capture modes.

When I have more time I will dig into this a bit more.

Regards,

Hans

#15 lore raw

Re: [PATCH v2 13/13] media: bttv: convert to vb2

Quoted 29 lines · Hans Verkuil

Hi Deb,

When testing this I was wondering why the sequence counter didn't detect dropped frames (which happens when you start/stop a vbi stream while streaming video).

Quoted 6 lines · Deborah Brouwer
drivers/media/pci/bt8xx/bttv-driver.c +0 -6
-static void bttv_field_count(struct bttv *btv)
-{
-	int need_count = 0;
-
-	if (btv->users)
-		need_count++;

drivers/media/pci/bt8xx/bttv-driver.c void bttv_gpio_tracking(struct bttv *btv, char *comment) · line 1471

Quoted 10 lines
drivers/media/pci/bt8xx/bttv-driver.c +0 -10
-
-	if (need_count) {
-		/* start field counter */
-		btor(BT848_INT_VSYNC,BT848_INT_MASK);
-	} else {
-		/* stop field counter */
-		btand(~BT848_INT_VSYNC,BT848_INT_MASK);
-		btv->field_count = 0;
-	}
-}

This is the root cause: this function is used to turn on VSYNC interrupts and in the interrupt handler the field_count is incremented.

In the vb1 version of this driver this field_count is passed on to vb1, which uses it to set the sequence counter to field_count / 2.

Quoted 7 lines

By removing this function the VSYNC irq is never enabled and so field_count is always 0. So I think in bttv the seqnr field should be dropped and the field_count mechanism re-instated.

Comparing the number of dropped frames when starting/stopping vbi it looks like in both cases (vb1 and vb2) one frame is dropped when starting vbi. But when stopping vbi no frames are dropped in the vb1 case, but 3 in the vb2 case.

Ok, I used btv->field_count >> 1 to set the sequence numbers and I added VSYNC to the interrupt mask when start_streaming() is called. I can see the dropped video frames in v4l2-ctl when vbi starts and stops (or vice versa), but I am not sure if this means that we are actually dropping any valid, captured pixels.

The internal FIELD change signal (even/odd) that is picked up by VSYNC seems to run independently from capturing. For example, out of curiosity, I added VSYNC to the interrupt mask at probe, and it starts to increment field_count (and so the sequence numbers) immediately and continuously even though capture isn't enabled.

So I am wondering if field_count is a totally reliable way to assign sequence numbers to the buffers. For example, using field_count, the buffer sequence numbers start with 3 instead of 0 in both vb1 and vb2 (sometimes it starts at 2 in vb2). Maybe the reason for this is that the field signal (even/odd) continues to be sent while the captured pixels are being input into the chip's FIFO buffer. The FIFO buffer is preparing the pixels for output by adding the timing/control
information but hasn't yet made them available to be picked up by the DMA controller for output to memory. So, there is a disconnect, when streaming starts, between field_count (which keeps incrementing) and the actual availability of pixels.

Maybe there is a similar disconnect happening when vbi starts or stops. Instead of actually dropping pixels what we really have is just a delay in the RISC program that has to cycle through its instructions. vb2 has made this a bit worse when streaming stops because we wait for all the buffers to finish. I will investigate more if there is an easy way to fix this in vb2 without changing the RISC program.

Quoted 9 lines

Another thing I discovered is that for PAL the vcr_hack control has to be enabled, otherwise the video is full of glitches. This was present before your series, and happens even with a video signal from a proper PAL video generator, so this is really strange. I can't remember that I needed this in the past, but it has been years since I last tested it.

PAL capture is fine for Top/Bottom/Alternate settings, it only fails for Interlaced and Sequential Top/Bottom capture modes.

Interesting to know that this happened in your tests also. Obviously the easy solution would be just to enable the vcr_hack by default. It removes the last four scan lines and so prevents the risc program counter from getting stuck at a buffer memory address (which definitely causes dropped frames) instead of returning to the main risc program address.

Quoted 6 lines

When I have more time I will dig into this a bit more.

Regards,

Hans

#16 lore raw

Re: [PATCH v2 13/13] media: bttv: convert to vb2

Hi Deb,

Quoted 43 lines · Deborah Brouwer
Quoted 29 lines · Hans Verkuil

Hi Deb,

When testing this I was wondering why the sequence counter didn't detect dropped frames (which happens when you start/stop a vbi stream while streaming video).

Quoted 6 lines · Deborah Brouwer
drivers/media/pci/bt8xx/bttv-driver.c +0 -6
-static void bttv_field_count(struct bttv *btv)
-{
-	int need_count = 0;
-
-	if (btv->users)
-		need_count++;

drivers/media/pci/bt8xx/bttv-driver.c void bttv_gpio_tracking(struct bttv *btv, char *comment) · line 1471

Quoted 10 lines
drivers/media/pci/bt8xx/bttv-driver.c +0 -10
-
-	if (need_count) {
-		/* start field counter */
-		btor(BT848_INT_VSYNC,BT848_INT_MASK);
-	} else {
-		/* stop field counter */
-		btand(~BT848_INT_VSYNC,BT848_INT_MASK);
-		btv->field_count = 0;
-	}
-}

This is the root cause: this function is used to turn on VSYNC interrupts and in the interrupt handler the field_count is incremented.

In the vb1 version of this driver this field_count is passed on to vb1, which uses it to set the sequence counter to field_count / 2.

Quoted 7 lines

By removing this function the VSYNC irq is never enabled and so field_count is always 0. So I think in bttv the seqnr field should be dropped and the field_count mechanism re-instated.

Comparing the number of dropped frames when starting/stopping vbi it looks like in both cases (vb1 and vb2) one frame is dropped when starting vbi. But when stopping vbi no frames are dropped in the vb1 case, but 3 in the vb2 case.

Ok, I used btv->field_count >> 1 to set the sequence numbers and I added VSYNC to the interrupt mask when start_streaming() is called. I can see the dropped video frames in v4l2-ctl when vbi starts and stops (or vice versa), but I am not sure if this means that we are actually dropping any valid, captured pixels.

Quoted 4 lines

The internal FIELD change signal (even/odd) that is picked up by VSYNC seems to run independently from capturing. For example, out of curiosity, I added VSYNC to the interrupt mask at probe, and it starts to increment field_count (and so the sequence numbers) immediately and continuously even though capture isn't enabled.

How exactly it is triggered, I'm not sure myself (I never dug deep enough into the technical details), but remember that this is analog video, and it behaves quite differently from modern video interfaces. Just think of the way old TVs worked: even though there is no valid signal, you still get a picture (snow).

Quoted 10 lines

So I am wondering if field_count is a totally reliable way to assign sequence numbers to the buffers. For example, using field_count, the buffer sequence numbers start with 3 instead of 0 in both vb1 and vb2 (sometimes it starts at 2 in vb2). Maybe the reason for this is that the field signal (even/odd) continues to be sent while the captured pixels are being input into the chip's FIFO buffer. The FIFO buffer is preparing the pixels for output by adding the timing/control
information but hasn't yet made them available to be picked up by the DMA controller for output to memory. So, there is a disconnect, when streaming starts, between field_count (which keeps incrementing) and the actual availability of pixels.

Quoted 5 lines

Maybe there is a similar disconnect happening when vbi starts or stops. Instead of actually dropping pixels what we really have is just a delay in the RISC program that has to cycle through its instructions. vb2 has made this a bit worse when streaming stops because we wait for all the buffers to finish. I will investigate more if there is an easy way to fix this in vb2 without changing the RISC program.

You can clearly see based on the timestamps that bttv really drops frames. That corresponds with the field count that's based on the VSYNC.

An alternative to using the VSYNC would be to just use the timestamps and detect if frames have been dropped.

Quoted 8 lines
Quoted 8 lines

Another thing I discovered is that for PAL the vcr_hack control has to be enabled, otherwise the video is full of glitches. This was present before your series, and happens even with a video signal from a proper PAL video generator, so this is really strange. I can't remember that I needed this in the past, but it has been years since I last tested it.

Quoted 8 lines
Quoted 2 lines

PAL capture is fine for Top/Bottom/Alternate settings, it only fails for Interlaced and Sequential Top/Bottom capture modes.

Interesting to know that this happened in your tests also. Obviously the easy solution would be just to enable the vcr_hack by default. It removes the last four scan lines and so prevents the risc program counter from getting stuck at a buffer memory address (which definitely causes dropped frames) instead of returning to the main risc program address.

Dropping 4 lines is really too much. A single line was sufficient to make it work again for PAL, but it's just weird behavior.

Unfortunately, I won't have time to dig deeper into this and since it is not something that this series introduced, I don't think it is important enough to work on this, at least for now.

Regards,

Hans

Quoted 7 lines
Quoted 7 lines

When I have more time I will dig into this a bit more.

Regards,

Hans