saa7134: improve v4l2-compliance

Make saa7134 driver more V4L2 compliant: implement V4L2_CAP_DEVICE_CAPS support and fix all capabilities problems reported by v4l2-compliance.

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |   55 ++++++++++++++++------------
 1 files changed, 31 insertions(+), 24 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +20 -8

index 3abf527..ce15f1f 100644
@@ -1805,6 +1805,8 @@ static int saa7134_querycap(struct file *file, void  *priv,
 {
 	struct saa7134_fh *fh = priv;
 	struct saa7134_dev *dev = fh->dev;
+	struct video_device *vdev = video_devdata(file);
+	u32 radio_caps, video_caps, vbi_caps;
 
 	unsigned int tuner_type = dev->tuner_type;
 

@@ -1812,19 +1814,37 @@ static int saa7134_querycap(struct file *file, void  *priv,
 	strlcpy(cap->card, saa7134_boards[dev->board].name,
 		sizeof(cap->card));
 	sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
-	cap->capabilities =
-		V4L2_CAP_VIDEO_CAPTURE |
-		V4L2_CAP_VBI_CAPTURE |
-		V4L2_CAP_READWRITE |
-		V4L2_CAP_STREAMING |
-		V4L2_CAP_TUNER;
+
+	cap->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
+	if ((tuner_type != TUNER_ABSENT) && (tuner_type != UNSET))
+		cap->device_caps |= V4L2_CAP_TUNER;
+
+	radio_caps = V4L2_CAP_RADIO;
 	if (dev->has_rds)
-		cap->capabilities |= V4L2_CAP_RDS_CAPTURE;
+		radio_caps |= V4L2_CAP_RDS_CAPTURE;
+
+	video_caps = V4L2_CAP_VIDEO_CAPTURE;
 	if (saa7134_no_overlay <= 0)
-		cap->capabilities |= V4L2_CAP_VIDEO_OVERLAY;
+		video_caps |= V4L2_CAP_VIDEO_OVERLAY;
+
+	vbi_caps = V4L2_CAP_VBI_CAPTURE;
+
+	switch (vdev->vfl_type) {
+	case VFL_TYPE_RADIO:
+		cap->device_caps |= radio_caps;
+		break;
+	case VFL_TYPE_GRABBER:

drivers/media/pci/saa7134/saa7134-video.c +0 -15

-	if ((tuner_type == TUNER_ABSENT) || (tuner_type == UNSET))
-		cap->capabilities &= ~V4L2_CAP_TUNER;
 	return 0;
 }
 

@@ -2299,19 +2319,6 @@ static int vidioc_s_register (struct file *file, void *priv,
 }
 #endif
 
-static int radio_querycap(struct file *file, void *priv,
-					struct v4l2_capability *cap)
-{
-	struct saa7134_fh *fh = file->private_data;
-	struct saa7134_dev *dev = fh->dev;
-
-	strcpy(cap->driver, "saa7134");
-	strlcpy(cap->card, saa7134_boards[dev->board].name, sizeof(cap->card));
-	sprintf(cap->bus_info, "PCI:%s", pci_name(dev->pci));
-	cap->capabilities = V4L2_CAP_TUNER;
-	return 0;
-}
-
 static int radio_g_tuner(struct file *file, void *priv,
 					struct v4l2_tuner *t)
 {

Make saa7134 driver more V4L2 compliant: filter audio modes that came from tuner - keep only MONO/STEREO in radio mode

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +1 -0

index ce15f1f..db8da32 100644
@@ -2333,6 +2333,7 @@ static int radio_g_tuner(struct file *file, void *priv,
 	t->type = V4L2_TUNER_RADIO;
 
 	saa_call_all(dev, tuner, g_tuner, t);
+	t->audmode &= V4L2_TUNER_MODE_MONO | V4L2_TUNER_MODE_STEREO;
 	if (dev->input->amux == TV) {
 		t->signal = 0xf800 - ((saa_readb(0x581) & 0x1f) << 11);
 		t->rxsubchans = (saa_readb(0x529) & 0x08) ?

Make saa7134 driver more V4L2 compliant: remove broken priority handling and use v4l2_fh instead

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-core.c  |    3 +-
 drivers/media/pci/saa7134/saa7134-video.c |   61 +++-------------------------
 drivers/media/pci/saa7134/saa7134.h       |    4 +-
 3 files changed, 10 insertions(+), 58 deletions(-)

drivers/media/pci/saa7134/saa7134-core.c +1 -2

index 8976d0e..ba08bd6 100644
@@ -805,6 +805,7 @@ static struct video_device *vdev_init(struct saa7134_dev *dev,
 	vfd->debug   = video_debug;
 	snprintf(vfd->name, sizeof(vfd->name), "%s %s (%s)",
 		 dev->name, type, saa7134_boards[dev->board].name);
+	set_bit(V4L2_FL_USE_FH_PRIO, &vfd->flags);
 	video_set_drvdata(vfd, dev);
 	return vfd;
 }

@@ -1028,8 +1029,6 @@ static int __devinit saa7134_initdev(struct pci_dev *pci_dev,
 		}
 	}
 
-	v4l2_prio_init(&dev->prio);
-
 	mutex_lock(&saa7134_devlist_lock);
 	list_for_each_entry(mops, &mops_list, next)
 		mpeg_ops_attach(mops, dev);

drivers/media/pci/saa7134/saa7134-video.c +7 -54

index db8da32..be745c0 100644
@@ -1176,14 +1176,6 @@ int saa7134_s_ctrl_internal(struct saa7134_dev *dev,  struct saa7134_fh *fh, str
 	int restart_overlay = 0;
 	int err;
 
-	/* When called from the empress code fh == NULL.
-	   That needs to be fixed somehow, but for now this is
-	   good enough. */
-	if (fh) {
-		err = v4l2_prio_check(&dev->prio, fh->prio);
-		if (0 != err)
-			return err;
-	}
 	err = -EINVAL;
 
 	mutex_lock(&dev->lock);

@@ -1352,6 +1344,7 @@ static int video_open(struct file *file)
 	if (NULL == fh)
 		return -ENOMEM;
 
+	v4l2_fh_init(&fh->fh, vdev);
 	file->private_data = fh;
 	fh->dev      = dev;
 	fh->radio    = radio;

@@ -1359,7 +1352,6 @@ static int video_open(struct file *file)
 	fh->fmt      = format_by_fourcc(V4L2_PIX_FMT_BGR24);
 	fh->width    = 720;
 	fh->height   = 576;
-	v4l2_prio_open(&dev->prio, &fh->prio);
 
 	videobuf_queue_sg_init(&fh->cap, &video_qops,
 			    &dev->pci->dev, &dev->slock,

@@ -1384,6 +1376,8 @@ static int video_open(struct file *file)
 		/* switch to video/vbi mode */
 		video_mux(dev,dev->ctl_input);
 	}
+	v4l2_fh_add(&fh->fh);
+
 	return 0;
 }
 

@@ -1504,7 +1498,8 @@ static int video_release(struct file *file)
 	saa7134_pgtable_free(dev->pci,&fh->pt_cap);
 	saa7134_pgtable_free(dev->pci,&fh->pt_vbi);
 
-	v4l2_prio_close(&dev->prio, fh->prio);
+	v4l2_fh_del(&fh->fh);
+	v4l2_fh_exit(&fh->fh);
 	file->private_data = NULL;
 	kfree(fh);
 	return 0;

@@ -1784,11 +1779,6 @@ static int saa7134_s_input(struct file *file, void *priv, unsigned int i)
 {
 	struct saa7134_fh *fh = priv;
 	struct saa7134_dev *dev = fh->dev;
-	int err;
-
-	err = v4l2_prio_check(&dev->prio, fh->prio);
-	if (0 != err)
-		return err;
 
 	if (i >= SAA7134_INPUT_MAX)
 		return -EINVAL;

@@ -1853,16 +1843,8 @@ int saa7134_s_std_internal(struct saa7134_dev *dev, struct saa7134_fh *fh, v4l2_
 	unsigned long flags;
 	unsigned int i;
 	v4l2_std_id fixup;
-	int err;
 
-	/* When called from the empress code fh == NULL.
-	   That needs to be fixed somehow, but for now this is
-	   good enough. */
-	if (fh) {
-		err = v4l2_prio_check(&dev->prio, fh->prio);
-		if (0 != err)
-			return err;
-	} else if (res_locked(dev, RESOURCE_OVERLAY)) {
+	if (!fh && res_locked(dev, RESOURCE_OVERLAY)) {
 		/* Don't change the std from the mpeg device
 		   if overlay is active. */
 		return -EBUSY;

@@ -2047,11 +2029,7 @@ static int saa7134_s_tuner(struct file *file, void *priv,
 {
 	struct saa7134_fh *fh = priv;
 	struct saa7134_dev *dev = fh->dev;
-	int rx, mode, err;
-
-	err = v4l2_prio_check(&dev->prio, fh->prio);
-	if (0 != err)
-		return err;
+	int rx, mode;
 
 	mode = dev->thread.mode;
 	if (UNSET == mode) {

@@ -2081,11 +2059,6 @@ static int saa7134_s_frequency(struct file *file, void *priv,
 {
 	struct saa7134_fh *fh = priv;
 	struct saa7134_dev *dev = fh->dev;
-	int err;
-
-	err = v4l2_prio_check(&dev->prio, fh->prio);
-	if (0 != err)
-		return err;
 
 	if (0 != f->tuner)
 		return -EINVAL;

@@ -2114,24 +2087,6 @@ static int saa7134_s_audio(struct file *file, void *priv, const struct v4l2_audi
 	return 0;
 }
 
-static int saa7134_g_priority(struct file *file, void *f, enum v4l2_priority *p)
-{
-	struct saa7134_fh *fh = f;
-	struct saa7134_dev *dev = fh->dev;
-
-	*p = v4l2_prio_max(&dev->prio);
-	return 0;
-}
-
-static int saa7134_s_priority(struct file *file, void *f,
-					enum v4l2_priority prio)
-{
-	struct saa7134_fh *fh = f;
-	struct saa7134_dev *dev = fh->dev;
-
-	return v4l2_prio_change(&dev->prio, &fh->prio, prio);
-}
-
 static int saa7134_enum_fmt_vid_cap(struct file *file, void  *priv,
 					struct v4l2_fmtdesc *f)
 {

@@ -2460,8 +2415,6 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_fbuf			= saa7134_g_fbuf,
 	.vidioc_s_fbuf			= saa7134_s_fbuf,
 	.vidioc_overlay			= saa7134_overlay,
-	.vidioc_g_priority		= saa7134_g_priority,
-	.vidioc_s_priority		= saa7134_s_priority,
 	.vidioc_g_parm			= saa7134_g_parm,
 	.vidioc_g_frequency		= saa7134_g_frequency,
 	.vidioc_s_frequency		= saa7134_s_frequency,

drivers/media/pci/saa7134/saa7134.h +2 -2

index 9059d08..2ffe069 100644
@@ -35,6 +35,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fh.h>
 #include <media/tuner.h>
 #include <media/rc-core.h>
 #include <media/ir-kbd-i2c.h>

@@ -466,11 +467,11 @@ struct saa7134_dmaqueue {
 
 /* video filehandle status */
 struct saa7134_fh {
+	struct v4l2_fh             fh;
 	struct saa7134_dev         *dev;
 	unsigned int               radio;
 	enum v4l2_buf_type         type;
 	unsigned int               resources;
-	enum v4l2_priority	   prio;
 
 	/* video overlay */
 	struct v4l2_window         win;

@@ -541,7 +542,6 @@ struct saa7134_dev {
 	struct list_head           devlist;
 	struct mutex               lock;
 	spinlock_t                 slock;
-	struct v4l2_prio_state     prio;
 	struct v4l2_device         v4l2_dev;
 	/* workstruct for loading modules */
 	struct work_struct request_module_wk;

Make saa7134 driver more V4L2 compliant: don't cache frequency in s_frequency/g_frequency but return real one instead

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    6 ++++--
 drivers/media/pci/saa7134/saa7134.h       |    1 -
 2 files changed, 4 insertions(+), 3 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +4 -2

index be745c0..87b2b9e 100644
@@ -2048,8 +2048,11 @@ static int saa7134_g_frequency(struct file *file, void *priv,
 	struct saa7134_fh *fh = priv;
 	struct saa7134_dev *dev = fh->dev;
 
+	if (0 != f->tuner)
+		return -EINVAL;
+
 	f->type = fh->radio ? V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
-	f->frequency = dev->ctl_freq;
+	saa_call_all(dev, tuner, g_frequency, f);
 
 	return 0;
 }

@@ -2067,7 +2070,6 @@ static int saa7134_s_frequency(struct file *file, void *priv,
 	if (1 == fh->radio && V4L2_TUNER_RADIO != f->type)
 		return -EINVAL;
 	mutex_lock(&dev->lock);
-	dev->ctl_freq = f->frequency;
 
 	saa_call_all(dev, tuner, s_frequency, f);
 

drivers/media/pci/saa7134/saa7134.h +0 -1

index 2ffe069..d0ee05e 100644
@@ -604,7 +604,6 @@ struct saa7134_dev {
 	int                        ctl_contrast;
 	int                        ctl_hue;
 	int                        ctl_saturation;
-	int                        ctl_freq;
 	int                        ctl_mute;             /* audio */
 	int                        ctl_volume;
 	int                        ctl_invert;           /* private */

Make saa7134 driver more V4L2 compliant: return real frequency range in g_tuner and fail in s_tuner for non-zero tuner

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +4 -1

index 87b2b9e..0b42f0c 100644
@@ -2011,11 +2011,11 @@ static int saa7134_g_tuner(struct file *file, void *priv,
 	if (NULL != card_in(dev, n).name) {
 		strcpy(t->name, "Television");
 		t->type = V4L2_TUNER_ANALOG_TV;
+		saa_call_all(dev, tuner, g_tuner, t);
 		t->capability = V4L2_TUNER_CAP_NORM |
 			V4L2_TUNER_CAP_STEREO |
 			V4L2_TUNER_CAP_LANG1 |
 			V4L2_TUNER_CAP_LANG2;
-		t->rangehigh = 0xffffffffUL;
 		t->rxsubchans = saa7134_tvaudio_getstereo(dev);
 		t->audmode = saa7134_tvaudio_rx2mode(t->rxsubchans);
 	}

@@ -2031,6 +2031,9 @@ static int saa7134_s_tuner(struct file *file, void *priv,
 	struct saa7134_dev *dev = fh->dev;
 	int rx, mode;
 
+	if (0 != t->index)
+		return -EINVAL;
+
 	mode = dev->thread.mode;
 	if (UNSET == mode) {
 		rx   = saa7134_tvaudio_getstereo(dev);

Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC flag in enum_input as it's for digital video only

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -0

index 0b42f0c..fff6735 100644

Em Sun, 27 Jan 2013 20:45:11 +0100
Ondrej Zary <[email protected]> escreveu:

Quoted 13 lines

Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC flag in enum_input as it's for digital video only

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -0

index 0b42f0c..fff6735 100644

drivers/media/pci/saa7134/saa7134-video.c static int saa7134_enum_input(struct file *file, void *priv, · line 1762

Quoted 8 lines

drivers/media/pci/saa7134/saa7134-video.c +0 -2

@@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv,
 
 		if (0 != (v1 & 0x40))
 			i->status |= V4L2_IN_ST_NO_H_LOCK;
-		if (0 != (v2 & 0x40))
-			i->status |= V4L2_IN_ST_NO_SYNC;
 		if (0 != (v2 & 0x0e))
 			i->status |= V4L2_IN_ST_MACROVISION;
 	}

Hmm... I'm not sure about this one. Very few drivers use those definitions, but I suspect that this might potentially break some userspace applications.

It sounds more likely that v4l-compliance is doing the wrong thing here, if it is complaining about that.

Make saa7134 driver more V4L2 compliant: don't set bogus V4L2_IN_ST_NO_SYNC flag in enum_input as it's for digital video only

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -2

index 0b42f0c..fff6735 100644
@@ -1757,8 +1757,6 @@ static int saa7134_enum_input(struct file *file, void *priv,
 
 		if (0 != (v1 & 0x40))
 			i->status |= V4L2_IN_ST_NO_H_LOCK;
-		if (0 != (v2 & 0x40))
-			i->status |= V4L2_IN_ST_NO_SYNC;
 		if (0 != (v2 & 0x0e))
 			i->status |= V4L2_IN_ST_MACROVISION;
 	}

Make saa7134 driver more V4L2 compliant: remove empty g_audio and s_audio functions and don't set audioset in enum_input

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |   30 -----------------------------
 1 files changed, 0 insertions(+), 30 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -30

index fff6735..b63cdad 100644
@@ -1750,7 +1750,6 @@ static int saa7134_enum_input(struct file *file, void *priv,
 	strcpy(i->name, card_in(dev, n).name);
 	if (card_in(dev, n).tv)
 		i->type = V4L2_INPUT_TYPE_TUNER;
-	i->audioset = 1;
 	if (n == dev->ctl_input) {
 		int v1 = saa_readb(SAA7134_STATUS_VIDEO1);
 		int v2 = saa_readb(SAA7134_STATUS_VIDEO2);

@@ -2079,17 +2078,6 @@ static int saa7134_s_frequency(struct file *file, void *priv,
 	return 0;
 }
 
-static int saa7134_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
-{
-	strcpy(a->name, "audio");
-	return 0;
-}
-
-static int saa7134_s_audio(struct file *file, void *priv, const struct v4l2_audio *a)
-{
-	return 0;
-}
-
 static int saa7134_enum_fmt_vid_cap(struct file *file, void  *priv,
 					struct v4l2_fmtdesc *f)
 {

@@ -2330,20 +2318,6 @@ static int radio_g_input(struct file *filp, void *priv, unsigned int *i)
 	return 0;
 }
 
-static int radio_g_audio(struct file *file, void *priv,
-					struct v4l2_audio *a)
-{
-	memset(a, 0, sizeof(*a));
-	strcpy(a->name, "Radio");
-	return 0;
-}
-
-static int radio_s_audio(struct file *file, void *priv,
-					const struct v4l2_audio *a)
-{
-	return 0;
-}
-
 static int radio_s_input(struct file *filp, void *priv, unsigned int i)
 {
 	return 0;

@@ -2394,8 +2368,6 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
-	.vidioc_g_audio			= saa7134_g_audio,
-	.vidioc_s_audio			= saa7134_s_audio,
 	.vidioc_cropcap			= saa7134_cropcap,
 	.vidioc_reqbufs			= saa7134_reqbufs,
 	.vidioc_querybuf		= saa7134_querybuf,

@@ -2440,9 +2412,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 	.vidioc_querycap	= saa7134_querycap,
 	.vidioc_g_tuner		= radio_g_tuner,
 	.vidioc_enum_input	= radio_enum_input,
-	.vidioc_g_audio		= radio_g_audio,
 	.vidioc_s_tuner		= radio_s_tuner,
-	.vidioc_s_audio		= radio_s_audio,
 	.vidioc_s_input		= radio_s_input,
 	.vidioc_s_std		= radio_s_std,
 	.vidioc_queryctrl	= radio_queryctrl,

Hello,
this patch series improves v4l2-compliance of saa7134 driver. There are still some problems. Controls require conversion to control framework which I was unable to finish (because the driver accesses other controls and also the file handle from within s_ctrl).

Radio is now OK except for controls.
Video has problems with controls, debugging, formats and buffers:
Debug ioctls:
test VIDIOC_DBG_G_CHIP_IDENT: OK (Not Supported)
fail: v4l2-test-debug.cpp(84): doioctl(node, VIDIOC_DBG_G_CHIP_IDENT, &chip)
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
fail: v4l2-test-formats.cpp(836): !cap->readbuffers

test VIDIOC_G/S_PARM: FAIL
fail: v4l2-test-formats.cpp(335): !fmt.width || !fmt.height
test VIDIOC_G_FBUF: FAIL
fail: v4l2-test-formats.cpp(382): !pix.colorspace

Buffer ioctls:

                fail: v4l2-test-buffers.cpp(109): can_stream && !mmap_valid && !userptr_valid && !dmabuf_valid

test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL

Haven't looked into VBI yet:
Format ioctls:
fail: v4l2-test-formats.cpp(914): G/S_PARM is only allowed for video capture/output
test VIDIOC_G/S_PARM: FAIL
fail: v4l2-test-formats.cpp(432): vbi.reserved not zeroed
fail: v4l2-test-formats.cpp(570): VBI Capture is valid, but TRY_FMT failed to return a format
test VIDIOC_TRY_FMT: FAIL
fail: v4l2-test-formats.cpp(432): vbi.reserved not zeroed
fail: v4l2-test-formats.cpp(728): VBI Capture is valid, but no S_FMT was implemented
test VIDIOC_S_FMT: FAIL

Quoted 5 lines · On Sun January 27 2013 20:45:05 Ondrej Zary wrote:

Hello,
this patch series improves v4l2-compliance of saa7134 driver. There are still some problems. Controls require conversion to control framework which I was unable to finish (because the driver accesses other controls and also the file handle from within s_ctrl).

To convert to the control framework this driver needs quite a bit of work:
the saa6752hs driver should be done first (and moved to media/i2c as well as it really doesn't belong here).

The filehandle shouldn't be a problem, I think after the prio conversion that's no longer needed at all.

Quoted 8 lines

Radio is now OK except for controls.
Video has problems with controls, debugging, formats and buffers:
Debug ioctls:
test VIDIOC_DBG_G_CHIP_IDENT: OK (Not Supported)
fail: v4l2-test-debug.cpp(84): doioctl(node,
VIDIOC_DBG_G_CHIP_IDENT, &chip) Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
fail: v4l2-test-formats.cpp(836): !cap->readbuffers

That should be easy to fix. It's a pretty bogus field and I usually set it to the minimum number of buffers (which is 2 for this driver).

Quoted 4 lines

test VIDIOC_G/S_PARM: FAIL
fail: v4l2-test-formats.cpp(335): !fmt.width ||
!fmt.height test VIDIOC_G_FBUF: FAIL

                fail: v4l2-test-formats.cpp(382): !pix.colorspace

That's easy enough to solve. Typically this should be set to V4L2_COLORSPACE_SMPTE170M.

But after solving this you'll probably get a bunch of other issues due to a problem this driver shared with quite a few other related drivers: the format state is stored in struct saa7134_fh instead of in the top-level struct. These format states are all global and should never have been placed in this struct.

In fact if I look at the fields in saa7134_fh then:

- radio and type can be removed (this info can be obtained from existing fields elsewhere)
- the fields win until pt_vbi should all be global fields
- I suspect resources and qos_request should also be global, but you would have to analyze that.

In fact, it is likely that the whole structure can be removed and only v4l2_fh be used instead.

Quoted 31 lines · On Monday 28 January 2013 11:56:59 Hans Verkuil wrote:

Quoted 5 lines · On Sun January 27 2013 20:45:05 Ondrej Zary wrote:

Hello,
this patch series improves v4l2-compliance of saa7134 driver. There are still some problems. Controls require conversion to control framework which I was unable to finish (because the driver accesses other controls and also the file handle from within s_ctrl).

To convert to the control framework this driver needs quite a bit of work:
the saa6752hs driver should be done first (and moved to media/i2c as well as it really doesn't belong here).

The filehandle shouldn't be a problem, I think after the prio conversion that's no longer needed at all.

Quoted 8 lines

Radio is now OK except for controls.
Video has problems with controls, debugging, formats and buffers:
Debug ioctls:
test VIDIOC_DBG_G_CHIP_IDENT: OK (Not Supported)
fail: v4l2-test-debug.cpp(84): doioctl(node,
VIDIOC_DBG_G_CHIP_IDENT, &chip) Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
fail: v4l2-test-formats.cpp(836): !cap->readbuffers

That should be easy to fix. It's a pretty bogus field and I usually set it to the minimum number of buffers (which is 2 for this driver).

Quoted 4 lines

test VIDIOC_G/S_PARM: FAIL
fail: v4l2-test-formats.cpp(335): !fmt.width ||
!fmt.height test VIDIOC_G_FBUF: FAIL

                fail: v4l2-test-formats.cpp(382): !pix.colorspace

Quoted 8 lines

That's easy enough to solve. Typically this should be set to V4L2_COLORSPACE_SMPTE170M.

But after solving this you'll probably get a bunch of other issues due to a problem this driver shared with quite a few other related drivers: the format state is stored in struct saa7134_fh instead of in the top-level struct. These format states are all global and should never have been placed in this struct.

Got this after setting colorspace:

               fail: v4l2-test-formats.cpp(460): win.field == V4L2_FIELD_ANY

I don't know what win.field is supposed to be and where the value should came from.

From the spec:

It's now taken from fh->win which is probably all zeros because no overlay is running?
The same problem is with g_fbuf - what should fmt.width and fmt.height be?

Make saa7134 driver more V4L2 compliant: remove empty g_parm function

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -6

index b63cdad..3e88041 100644
@@ -2232,12 +2232,6 @@ static int saa7134_streamoff(struct file *file, void 
 *priv,
 	return 0;
 }
 
-static int saa7134_g_parm(struct file *file, void *fh,
-				struct v4l2_streamparm *parm)
-{
-	return 0;
-}
-
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static int vidioc_g_register (struct file *file, void *priv,

drivers/media/pci/saa7134/saa7134-video.c +0 -1

 			      struct v4l2_dbg_register *reg)

@@ -2390,7 +2384,6 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_fbuf			= saa7134_g_fbuf,
 	.vidioc_s_fbuf			= saa7134_s_fbuf,
 	.vidioc_overlay			= saa7134_overlay,
-	.vidioc_g_parm			= saa7134_g_parm,
 	.vidioc_g_frequency		= saa7134_g_frequency,
 	.vidioc_s_frequency		= saa7134_s_frequency,
 #ifdef CONFIG_VIDEO_ADV_DEBUG

Make saa7134 driver more V4L2 compliant: clear VBI structure completely before assigning values to make sure any reserved space is cleared

Signed-off-by Ondrej Zary <[email protected]>

---
 drivers/media/pci/saa7134/saa7134-video.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/media/pci/saa7134/saa7134-video.c +0 -0

index 3e88041..adb83b5 100644

drivers/media/pci/saa7134/saa7134-video.c +0 -0

 	f->fmt.vbi.sampling_rate = 6750000 * 4;
 	f->fmt.vbi.samples_per_line = 2048 /* VBI_LINE_LENGTH */;

f->fmt.vbi.sample_format = V4L2_PIX_FMT_GREY;