[PATCH] saa7164: convert to the control framework

[PATCH] saa7164: convert to the control framework

[Hans Verkuil]

Convert this driver to the control framework. Note that the VBI device
nodes have no controls as there is nothing to control.

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

Steve, can you test this patch? For some reason my saa7164 isn't seen anymore
on the PCIe bus, so I can't test it myself :-(

This patch greatly simplifies this driver w.r.t. control handling and it will
now be fully compliant to the spec as well.

Thanks,

	Hans

---
 drivers/media/pci/saa7164/saa7164-encoder.c | 464 +++++-----------------------
 drivers/media/pci/saa7164/saa7164-vbi.c     | 359 ---------------------
 drivers/media/pci/saa7164/saa7164.h         |   2 +
 3 files changed, 84 insertions(+), 741 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-encoder.c b/drivers/media/pci/saa7164/saa7164-encoder.c
index 4434e0f..f5e1236 100644
--- a/drivers/media/pci/saa7164/saa7164-encoder.c
+++ b/drivers/media/pci/saa7164/saa7164-encoder.c
@@ -35,24 +35,6 @@ static struct saa7164_tvnorm saa7164_tvnorms[] = {
 	}
 };
 
-static const u32 saa7164_v4l2_ctrls[] = {
-	V4L2_CID_BRIGHTNESS,
-	V4L2_CID_CONTRAST,
-	V4L2_CID_SATURATION,
-	V4L2_CID_HUE,
-	V4L2_CID_AUDIO_VOLUME,
-	V4L2_CID_SHARPNESS,
-	V4L2_CID_MPEG_STREAM_TYPE,
-	V4L2_CID_MPEG_VIDEO_ASPECT,
-	V4L2_CID_MPEG_VIDEO_B_FRAMES,
-	V4L2_CID_MPEG_VIDEO_GOP_SIZE,
-	V4L2_CID_MPEG_AUDIO_MUTE,
-	V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
-	V4L2_CID_MPEG_VIDEO_BITRATE,
-	V4L2_CID_MPEG_VIDEO_BITRATE_PEAK,
-	0
-};
-
 /* Take the encoder configuration form the port struct and
  * flush it to the hardware.
  */
@@ -396,253 +378,47 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_g_ctrl(struct file *file, void *priv,
-	struct v4l2_control *ctl)
+static int saa7164_s_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct saa7164_encoder_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	struct saa7164_dev *dev = port->dev;
-
-	dprintk(DBGLVL_ENC, "%s(id=%d, value=%d)\n", __func__,
-		ctl->id, ctl->value);
-
-	switch (ctl->id) {
-	case V4L2_CID_BRIGHTNESS:
-		ctl->value = port->ctl_brightness;
-		break;
-	case V4L2_CID_CONTRAST:
-		ctl->value = port->ctl_contrast;
-		break;
-	case V4L2_CID_SATURATION:
-		ctl->value = port->ctl_saturation;
-		break;
-	case V4L2_CID_HUE:
-		ctl->value = port->ctl_hue;
-		break;
-	case V4L2_CID_SHARPNESS:
-		ctl->value = port->ctl_sharpness;
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-		ctl->value = port->ctl_volume;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int vidioc_s_ctrl(struct file *file, void *priv,
-	struct v4l2_control *ctl)
-{
-	struct saa7164_encoder_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
+	struct saa7164_port *port =
+		container_of(ctrl->handler, struct saa7164_port, ctrl_handler);
+	struct saa7164_encoder_params *params = &port->encoder_params;
 	struct saa7164_dev *dev = port->dev;
 	int ret = 0;
 
-	dprintk(DBGLVL_ENC, "%s(id=%d, value=%d)\n", __func__,
-		ctl->id, ctl->value);
-
-	switch (ctl->id) {
+	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_brightness = ctl->value;
-			saa7164_api_set_usercontrol(port,
-				PU_BRIGHTNESS_CONTROL);
-		} else
-			ret = -EINVAL;
+		port->ctl_brightness = ctrl->val;
+		saa7164_api_set_usercontrol(port, PU_BRIGHTNESS_CONTROL);
 		break;
 	case V4L2_CID_CONTRAST:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_contrast = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_CONTRAST_CONTROL);
-		} else
-			ret = -EINVAL;
+		port->ctl_contrast = ctrl->val;
+		saa7164_api_set_usercontrol(port, PU_CONTRAST_CONTROL);
 		break;
 	case V4L2_CID_SATURATION:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_saturation = ctl->value;
-			saa7164_api_set_usercontrol(port,
-				PU_SATURATION_CONTROL);
-		} else
-			ret = -EINVAL;
+		port->ctl_saturation = ctrl->val;
+		saa7164_api_set_usercontrol(port, PU_SATURATION_CONTROL);
 		break;
 	case V4L2_CID_HUE:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_hue = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_HUE_CONTROL);
-		} else
-			ret = -EINVAL;
+		port->ctl_hue = ctrl->val;
+		saa7164_api_set_usercontrol(port, PU_HUE_CONTROL);
 		break;
 	case V4L2_CID_SHARPNESS:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_sharpness = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_SHARPNESS_CONTROL);
-		} else
-			ret = -EINVAL;
+		port->ctl_sharpness = ctrl->val;
+		saa7164_api_set_usercontrol(port, PU_SHARPNESS_CONTROL);
 		break;
 	case V4L2_CID_AUDIO_VOLUME:
-		if ((ctl->value >= -83) && (ctl->value <= 24)) {
-			port->ctl_volume = ctl->value;
-			saa7164_api_set_audio_volume(port, port->ctl_volume);
-		} else
-			ret = -EINVAL;
-		break;
-	default:
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
-static int saa7164_get_ctrl(struct saa7164_port *port,
-	struct v4l2_ext_control *ctrl)
-{
-	struct saa7164_encoder_params *params = &port->encoder_params;
-
-	switch (ctrl->id) {
-	case V4L2_CID_MPEG_VIDEO_BITRATE:
-		ctrl->value = params->bitrate;
-		break;
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		ctrl->value = params->stream_type;
-		break;
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		ctrl->value = params->ctl_mute;
-		break;
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		ctrl->value = params->ctl_aspect;
-		break;
-	case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
-		ctrl->value = params->bitrate_mode;
-		break;
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		ctrl->value = params->refdist;
-		break;
-	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
-		ctrl->value = params->bitrate_peak;
-		break;
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		ctrl->value = params->gop_size;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int vidioc_g_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_encoder_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_get_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
-static int saa7164_try_ctrl(struct v4l2_ext_control *ctrl, int ac3)
-{
-	int ret = -EINVAL;
-
-	switch (ctrl->id) {
-	case V4L2_CID_MPEG_VIDEO_BITRATE:
-		if ((ctrl->value >= ENCODER_MIN_BITRATE) &&
-			(ctrl->value <= ENCODER_MAX_BITRATE))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		if ((ctrl->value == V4L2_MPEG_STREAM_TYPE_MPEG2_PS) ||
-			(ctrl->value == V4L2_MPEG_STREAM_TYPE_MPEG2_TS))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		if ((ctrl->value >= 0) &&
-			(ctrl->value <= 1))
-			ret = 0;
+		port->ctl_volume = ctrl->val;
+		saa7164_api_set_audio_volume(port, port->ctl_volume);
 		break;
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		if ((ctrl->value >= V4L2_MPEG_VIDEO_ASPECT_1x1) &&
-			(ctrl->value <= V4L2_MPEG_VIDEO_ASPECT_221x100))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		if ((ctrl->value >= 0) &&
-			(ctrl->value <= 255))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
-		if ((ctrl->value == V4L2_MPEG_VIDEO_BITRATE_MODE_VBR) ||
-			(ctrl->value == V4L2_MPEG_VIDEO_BITRATE_MODE_CBR))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		if ((ctrl->value >= 1) &&
-			(ctrl->value <= 3))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
-		if ((ctrl->value >= ENCODER_MIN_BITRATE) &&
-			(ctrl->value <= ENCODER_MAX_BITRATE))
-			ret = 0;
-		break;
-	default:
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
-static int vidioc_try_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_try_ctrl(ctrl, 0);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-	}
-
-	return -EINVAL;
-}
-
-static int saa7164_set_ctrl(struct saa7164_port *port,
-	struct v4l2_ext_control *ctrl)
-{
-	struct saa7164_encoder_params *params = &port->encoder_params;
-	int ret = 0;
-
-	switch (ctrl->id) {
 	case V4L2_CID_MPEG_VIDEO_BITRATE:
-		params->bitrate = ctrl->value;
+		params->bitrate = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_STREAM_TYPE:
-		params->stream_type = ctrl->value;
+		params->stream_type = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_AUDIO_MUTE:
-		params->ctl_mute = ctrl->value;
+		params->ctl_mute = ctrl->val;
 		ret = saa7164_api_audio_mute(port, params->ctl_mute);
 		if (ret != SAA_OK) {
 			printk(KERN_ERR "%s() error, ret = 0x%x\n", __func__,
@@ -651,7 +427,7 @@ static int saa7164_set_ctrl(struct saa7164_port *port,
 		}
 		break;
 	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		params->ctl_aspect = ctrl->value;
+		params->ctl_aspect = ctrl->val;
 		ret = saa7164_api_set_aspect_ratio(port);
 		if (ret != SAA_OK) {
 			printk(KERN_ERR "%s() error, ret = 0x%x\n", __func__,
@@ -660,55 +436,24 @@ static int saa7164_set_ctrl(struct saa7164_port *port,
 		}
 		break;
 	case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
-		params->bitrate_mode = ctrl->value;
+		params->bitrate_mode = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		params->refdist = ctrl->value;
+		params->refdist = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
-		params->bitrate_peak = ctrl->value;
+		params->bitrate_peak = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		params->gop_size = ctrl->value;
+		params->gop_size = ctrl->val;
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	/* TODO: Update the hardware */
-
 	return ret;
 }
 
-static int vidioc_s_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_encoder_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_try_ctrl(ctrl, 0);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-			err = saa7164_set_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
 static int vidioc_querycap(struct file *file, void  *priv,
 	struct v4l2_capability *cap)
 {
@@ -802,88 +547,6 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int fill_queryctrl(struct saa7164_encoder_params *params,
-	struct v4l2_queryctrl *c)
-{
-	switch (c->id) {
-	case V4L2_CID_BRIGHTNESS:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 127);
-	case V4L2_CID_CONTRAST:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 66);
-	case V4L2_CID_SATURATION:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 62);
-	case V4L2_CID_HUE:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 128);
-	case V4L2_CID_SHARPNESS:
-		return v4l2_ctrl_query_fill(c, 0x0, 0x0f, 1, 8);
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		return v4l2_ctrl_query_fill(c, 0x0, 0x01, 1, 0);
-	case V4L2_CID_AUDIO_VOLUME:
-		return v4l2_ctrl_query_fill(c, -83, 24, 1, 20);
-	case V4L2_CID_MPEG_VIDEO_BITRATE:
-		return v4l2_ctrl_query_fill(c,
-			ENCODER_MIN_BITRATE, ENCODER_MAX_BITRATE,
-			100000, ENCODER_DEF_BITRATE);
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		return v4l2_ctrl_query_fill(c,
-			V4L2_MPEG_STREAM_TYPE_MPEG2_PS,
-			V4L2_MPEG_STREAM_TYPE_MPEG2_TS,
-			1, V4L2_MPEG_STREAM_TYPE_MPEG2_PS);
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		return v4l2_ctrl_query_fill(c,
-			V4L2_MPEG_VIDEO_ASPECT_1x1,
-			V4L2_MPEG_VIDEO_ASPECT_221x100,
-			1, V4L2_MPEG_VIDEO_ASPECT_4x3);
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		return v4l2_ctrl_query_fill(c, 1, 255, 1, 15);
-	case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
-		return v4l2_ctrl_query_fill(c,
-			V4L2_MPEG_VIDEO_BITRATE_MODE_VBR,
-			V4L2_MPEG_VIDEO_BITRATE_MODE_CBR,
-			1, V4L2_MPEG_VIDEO_BITRATE_MODE_VBR);
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		return v4l2_ctrl_query_fill(c,
-			1, 3, 1, 1);
-	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
-		return v4l2_ctrl_query_fill(c,
-			ENCODER_MIN_BITRATE, ENCODER_MAX_BITRATE,
-			100000, ENCODER_DEF_BITRATE);
-	default:
-		return -EINVAL;
-	}
-}
-
-static int vidioc_queryctrl(struct file *file, void *priv,
-	struct v4l2_queryctrl *c)
-{
-	struct saa7164_encoder_fh *fh = priv;
-	struct saa7164_port *port = fh->port;
-	int i, next;
-	u32 id = c->id;
-
-	memset(c, 0, sizeof(*c));
-
-	next = !!(id & V4L2_CTRL_FLAG_NEXT_CTRL);
-	c->id = id & ~V4L2_CTRL_FLAG_NEXT_CTRL;
-
-	for (i = 0; i < ARRAY_SIZE(saa7164_v4l2_ctrls); i++) {
-		if (next) {
-			if (c->id < saa7164_v4l2_ctrls[i])
-				c->id = saa7164_v4l2_ctrls[i];
-			else
-				continue;
-		}
-
-		if (c->id == saa7164_v4l2_ctrls[i])
-			return fill_queryctrl(&port->encoder_params, c);
-
-		if (c->id < saa7164_v4l2_ctrls[i])
-			break;
-	}
-
-	return -EINVAL;
-}
-
 static int saa7164_encoder_stop_port(struct saa7164_port *port)
 {
 	struct saa7164_dev *dev = port->dev;
@@ -1290,6 +953,10 @@ static unsigned int fops_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
+static const struct v4l2_ctrl_ops saa7164_ctrl_ops = {
+	.s_ctrl = saa7164_s_ctrl,
+};
+
 static const struct v4l2_file_operations mpeg_fops = {
 	.owner		= THIS_MODULE,
 	.open		= fops_open,
@@ -1309,17 +976,11 @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = {
 	.vidioc_s_tuner		 = vidioc_s_tuner,
 	.vidioc_g_frequency	 = vidioc_g_frequency,
 	.vidioc_s_frequency	 = vidioc_s_frequency,
-	.vidioc_s_ctrl		 = vidioc_s_ctrl,
-	.vidioc_g_ctrl		 = vidioc_g_ctrl,
 	.vidioc_querycap	 = vidioc_querycap,
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap	 = vidioc_g_fmt_vid_cap,
 	.vidioc_try_fmt_vid_cap	 = vidioc_try_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap	 = vidioc_s_fmt_vid_cap,
-	.vidioc_g_ext_ctrls	 = vidioc_g_ext_ctrls,
-	.vidioc_s_ext_ctrls	 = vidioc_s_ext_ctrls,
-	.vidioc_try_ext_ctrls	 = vidioc_try_ext_ctrls,
-	.vidioc_queryctrl	 = vidioc_queryctrl,
 };
 
 static struct video_device saa7164_mpeg_template = {
@@ -1357,6 +1018,7 @@ static struct video_device *saa7164_encoder_alloc(
 int saa7164_encoder_register(struct saa7164_port *port)
 {
 	struct saa7164_dev *dev = port->dev;
+	struct v4l2_ctrl_handler *hdl = &port->ctrl_handler;
 	int result = -ENODEV;
 
 	dprintk(DBGLVL_ENC, "%s()\n", __func__);
@@ -1381,19 +1043,54 @@ int saa7164_encoder_register(struct saa7164_port *port)
 	port->video_format = EU_VIDEO_FORMAT_MPEG_2;
 	port->audio_format = 0;
 	port->video_resolution = 0;
-	port->ctl_brightness = 127;
-	port->ctl_contrast = 66;
-	port->ctl_hue = 128;
-	port->ctl_saturation = 62;
-	port->ctl_sharpness = 8;
-	port->encoder_params.bitrate = ENCODER_DEF_BITRATE;
-	port->encoder_params.bitrate_peak = ENCODER_DEF_BITRATE;
-	port->encoder_params.bitrate_mode = V4L2_MPEG_VIDEO_BITRATE_MODE_CBR;
-	port->encoder_params.stream_type = V4L2_MPEG_STREAM_TYPE_MPEG2_PS;
-	port->encoder_params.ctl_mute = 0;
-	port->encoder_params.ctl_aspect = V4L2_MPEG_VIDEO_ASPECT_4x3;
-	port->encoder_params.refdist = 1;
-	port->encoder_params.gop_size = SAA7164_ENCODER_DEFAULT_GOP_SIZE;
+
+	v4l2_ctrl_handler_init(hdl, 14);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_BRIGHTNESS, 0, 255, 1, 127);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_CONTRAST, 0, 255, 1, 66);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_SATURATION, 0, 255, 1, 62);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_HUE, 0, 255, 1, 128);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_SHARPNESS, 0x0, 0x0f, 1, 8);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_MPEG_AUDIO_MUTE, 0x0, 0x01, 1, 0);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_AUDIO_VOLUME, -83, 24, 1, 20);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_MPEG_VIDEO_BITRATE,
+			  ENCODER_MIN_BITRATE, ENCODER_MAX_BITRATE,
+			  100000, ENCODER_DEF_BITRATE);
+	v4l2_ctrl_new_std_menu(hdl, &saa7164_ctrl_ops,
+			       V4L2_CID_MPEG_STREAM_TYPE,
+			       V4L2_MPEG_STREAM_TYPE_MPEG2_PS,
+			       V4L2_MPEG_STREAM_TYPE_MPEG2_TS,
+			       V4L2_MPEG_STREAM_TYPE_MPEG2_PS);
+	v4l2_ctrl_new_std_menu(hdl, &saa7164_ctrl_ops,
+			       V4L2_CID_MPEG_VIDEO_ASPECT,
+			       V4L2_MPEG_VIDEO_ASPECT_1x1,
+			       V4L2_MPEG_VIDEO_ASPECT_221x100,
+			       V4L2_MPEG_VIDEO_ASPECT_4x3);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_MPEG_VIDEO_GOP_SIZE, 1, 255, 1, 15);
+	v4l2_ctrl_new_std_menu(hdl, &saa7164_ctrl_ops,
+			       V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
+			       V4L2_MPEG_VIDEO_BITRATE_MODE_VBR,
+			       V4L2_MPEG_VIDEO_BITRATE_MODE_CBR,
+			       V4L2_MPEG_VIDEO_BITRATE_MODE_VBR);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_MPEG_VIDEO_B_FRAMES, 1, 3, 1, 1);
+	v4l2_ctrl_new_std(hdl, &saa7164_ctrl_ops,
+			  V4L2_CID_MPEG_VIDEO_BITRATE_PEAK,
+			  ENCODER_MIN_BITRATE, ENCODER_MAX_BITRATE,
+			  100000, ENCODER_DEF_BITRATE);
+	if (hdl->error) {
+		result = hdl->error;
+		goto failed;
+	}
+
 	port->std = V4L2_STD_NTSC_M;
 
 	if (port->encodernorm.id & V4L2_STD_525_60)
@@ -1412,6 +1109,8 @@ int saa7164_encoder_register(struct saa7164_port *port)
 		goto failed;
 	}
 
+	port->v4l_device->ctrl_handler = hdl;
+	v4l2_ctrl_handler_setup(hdl);
 	video_set_drvdata(port->v4l_device, port);
 	result = video_register_device(port->v4l_device,
 		VFL_TYPE_GRABBER, -1);
@@ -1466,6 +1165,7 @@ void saa7164_encoder_unregister(struct saa7164_port *port)
 
 		port->v4l_device = NULL;
 	}
+	v4l2_ctrl_handler_free(&port->ctrl_handler);
 
 	dprintk(DBGLVL_ENC, "%s(port=%d) done\n", __func__, port->nr);
 }
diff --git a/drivers/media/pci/saa7164/saa7164-vbi.c b/drivers/media/pci/saa7164/saa7164-vbi.c
index 859fd03..4858f59 100644
--- a/drivers/media/pci/saa7164/saa7164-vbi.c
+++ b/drivers/media/pci/saa7164/saa7164-vbi.c
@@ -31,10 +31,6 @@ static struct saa7164_tvnorm saa7164_tvnorms[] = {
 	}
 };
 
-static const u32 saa7164_v4l2_ctrls[] = {
-	0
-};
-
 /* Take the encoder configuration from the port struct and
  * flush it to the hardware.
  */
@@ -368,286 +364,6 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_g_ctrl(struct file *file, void *priv,
-	struct v4l2_control *ctl)
-{
-	struct saa7164_vbi_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	struct saa7164_dev *dev = port->dev;
-
-	dprintk(DBGLVL_VBI, "%s(id=%d, value=%d)\n", __func__,
-		ctl->id, ctl->value);
-
-	switch (ctl->id) {
-	case V4L2_CID_BRIGHTNESS:
-		ctl->value = port->ctl_brightness;
-		break;
-	case V4L2_CID_CONTRAST:
-		ctl->value = port->ctl_contrast;
-		break;
-	case V4L2_CID_SATURATION:
-		ctl->value = port->ctl_saturation;
-		break;
-	case V4L2_CID_HUE:
-		ctl->value = port->ctl_hue;
-		break;
-	case V4L2_CID_SHARPNESS:
-		ctl->value = port->ctl_sharpness;
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-		ctl->value = port->ctl_volume;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int vidioc_s_ctrl(struct file *file, void *priv,
-	struct v4l2_control *ctl)
-{
-	struct saa7164_vbi_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	struct saa7164_dev *dev = port->dev;
-	int ret = 0;
-
-	dprintk(DBGLVL_VBI, "%s(id=%d, value=%d)\n", __func__,
-		ctl->id, ctl->value);
-
-	switch (ctl->id) {
-	case V4L2_CID_BRIGHTNESS:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_brightness = ctl->value;
-			saa7164_api_set_usercontrol(port,
-				PU_BRIGHTNESS_CONTROL);
-		} else
-			ret = -EINVAL;
-		break;
-	case V4L2_CID_CONTRAST:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_contrast = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_CONTRAST_CONTROL);
-		} else
-			ret = -EINVAL;
-		break;
-	case V4L2_CID_SATURATION:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_saturation = ctl->value;
-			saa7164_api_set_usercontrol(port,
-				PU_SATURATION_CONTROL);
-		} else
-			ret = -EINVAL;
-		break;
-	case V4L2_CID_HUE:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_hue = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_HUE_CONTROL);
-		} else
-			ret = -EINVAL;
-		break;
-	case V4L2_CID_SHARPNESS:
-		if ((ctl->value >= 0) && (ctl->value <= 255)) {
-			port->ctl_sharpness = ctl->value;
-			saa7164_api_set_usercontrol(port, PU_SHARPNESS_CONTROL);
-		} else
-			ret = -EINVAL;
-		break;
-	case V4L2_CID_AUDIO_VOLUME:
-		if ((ctl->value >= -83) && (ctl->value <= 24)) {
-			port->ctl_volume = ctl->value;
-			saa7164_api_set_audio_volume(port, port->ctl_volume);
-		} else
-			ret = -EINVAL;
-		break;
-	default:
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
-static int saa7164_get_ctrl(struct saa7164_port *port,
-	struct v4l2_ext_control *ctrl)
-{
-	struct saa7164_vbi_params *params = &port->vbi_params;
-
-	switch (ctrl->id) {
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		ctrl->value = params->stream_type;
-		break;
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		ctrl->value = params->ctl_mute;
-		break;
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		ctrl->value = params->ctl_aspect;
-		break;
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		ctrl->value = params->refdist;
-		break;
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		ctrl->value = params->gop_size;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int vidioc_g_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_vbi_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_get_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
-static int saa7164_try_ctrl(struct v4l2_ext_control *ctrl, int ac3)
-{
-	int ret = -EINVAL;
-
-	switch (ctrl->id) {
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		if ((ctrl->value == V4L2_MPEG_STREAM_TYPE_MPEG2_PS) ||
-			(ctrl->value == V4L2_MPEG_STREAM_TYPE_MPEG2_TS))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		if ((ctrl->value >= 0) &&
-			(ctrl->value <= 1))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		if ((ctrl->value >= V4L2_MPEG_VIDEO_ASPECT_1x1) &&
-			(ctrl->value <= V4L2_MPEG_VIDEO_ASPECT_221x100))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		if ((ctrl->value >= 0) &&
-			(ctrl->value <= 255))
-			ret = 0;
-		break;
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		if ((ctrl->value >= 1) &&
-			(ctrl->value <= 3))
-			ret = 0;
-		break;
-	default:
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
-static int vidioc_try_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_try_ctrl(ctrl, 0);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-	}
-
-	return -EINVAL;
-}
-
-static int saa7164_set_ctrl(struct saa7164_port *port,
-	struct v4l2_ext_control *ctrl)
-{
-	struct saa7164_vbi_params *params = &port->vbi_params;
-	int ret = 0;
-
-	switch (ctrl->id) {
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		params->stream_type = ctrl->value;
-		break;
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		params->ctl_mute = ctrl->value;
-		ret = saa7164_api_audio_mute(port, params->ctl_mute);
-		if (ret != SAA_OK) {
-			printk(KERN_ERR "%s() error, ret = 0x%x\n", __func__,
-				ret);
-			ret = -EIO;
-		}
-		break;
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		params->ctl_aspect = ctrl->value;
-		ret = saa7164_api_set_aspect_ratio(port);
-		if (ret != SAA_OK) {
-			printk(KERN_ERR "%s() error, ret = 0x%x\n", __func__,
-				ret);
-			ret = -EIO;
-		}
-		break;
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		params->refdist = ctrl->value;
-		break;
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		params->gop_size = ctrl->value;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	/* TODO: Update the hardware */
-
-	return ret;
-}
-
-static int vidioc_s_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_vbi_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_try_ctrl(ctrl, 0);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-			err = saa7164_set_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
 static int vidioc_querycap(struct file *file, void  *priv,
 	struct v4l2_capability *cap)
 {
@@ -741,75 +457,6 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int fill_queryctrl(struct saa7164_vbi_params *params,
-	struct v4l2_queryctrl *c)
-{
-	switch (c->id) {
-	case V4L2_CID_BRIGHTNESS:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 127);
-	case V4L2_CID_CONTRAST:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 66);
-	case V4L2_CID_SATURATION:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 62);
-	case V4L2_CID_HUE:
-		return v4l2_ctrl_query_fill(c, 0x0, 0xff, 1, 128);
-	case V4L2_CID_SHARPNESS:
-		return v4l2_ctrl_query_fill(c, 0x0, 0x0f, 1, 8);
-	case V4L2_CID_MPEG_AUDIO_MUTE:
-		return v4l2_ctrl_query_fill(c, 0x0, 0x01, 1, 0);
-	case V4L2_CID_AUDIO_VOLUME:
-		return v4l2_ctrl_query_fill(c, -83, 24, 1, 20);
-	case V4L2_CID_MPEG_STREAM_TYPE:
-		return v4l2_ctrl_query_fill(c,
-			V4L2_MPEG_STREAM_TYPE_MPEG2_PS,
-			V4L2_MPEG_STREAM_TYPE_MPEG2_TS,
-			1, V4L2_MPEG_STREAM_TYPE_MPEG2_PS);
-	case V4L2_CID_MPEG_VIDEO_ASPECT:
-		return v4l2_ctrl_query_fill(c,
-			V4L2_MPEG_VIDEO_ASPECT_1x1,
-			V4L2_MPEG_VIDEO_ASPECT_221x100,
-			1, V4L2_MPEG_VIDEO_ASPECT_4x3);
-	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		return v4l2_ctrl_query_fill(c, 1, 255, 1, 15);
-	case V4L2_CID_MPEG_VIDEO_B_FRAMES:
-		return v4l2_ctrl_query_fill(c,
-			1, 3, 1, 1);
-	default:
-		return -EINVAL;
-	}
-}
-
-static int vidioc_queryctrl(struct file *file, void *priv,
-	struct v4l2_queryctrl *c)
-{
-	struct saa7164_vbi_fh *fh = priv;
-	struct saa7164_port *port = fh->port;
-	int i, next;
-	u32 id = c->id;
-
-	memset(c, 0, sizeof(*c));
-
-	next = !!(id & V4L2_CTRL_FLAG_NEXT_CTRL);
-	c->id = id & ~V4L2_CTRL_FLAG_NEXT_CTRL;
-
-	for (i = 0; i < ARRAY_SIZE(saa7164_v4l2_ctrls); i++) {
-		if (next) {
-			if (c->id < saa7164_v4l2_ctrls[i])
-				c->id = saa7164_v4l2_ctrls[i];
-			else
-				continue;
-		}
-
-		if (c->id == saa7164_v4l2_ctrls[i])
-			return fill_queryctrl(&port->vbi_params, c);
-
-		if (c->id < saa7164_v4l2_ctrls[i])
-			break;
-	}
-
-	return -EINVAL;
-}
-
 static int saa7164_vbi_stop_port(struct saa7164_port *port)
 {
 	struct saa7164_dev *dev = port->dev;
@@ -1255,17 +902,11 @@ static const struct v4l2_ioctl_ops vbi_ioctl_ops = {
 	.vidioc_s_tuner		 = vidioc_s_tuner,
 	.vidioc_g_frequency	 = vidioc_g_frequency,
 	.vidioc_s_frequency	 = vidioc_s_frequency,
-	.vidioc_s_ctrl		 = vidioc_s_ctrl,
-	.vidioc_g_ctrl		 = vidioc_g_ctrl,
 	.vidioc_querycap	 = vidioc_querycap,
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap	 = vidioc_g_fmt_vid_cap,
 	.vidioc_try_fmt_vid_cap	 = vidioc_try_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap	 = vidioc_s_fmt_vid_cap,
-	.vidioc_g_ext_ctrls	 = vidioc_g_ext_ctrls,
-	.vidioc_s_ext_ctrls	 = vidioc_s_ext_ctrls,
-	.vidioc_try_ext_ctrls	 = vidioc_try_ext_ctrls,
-	.vidioc_queryctrl	 = vidioc_queryctrl,
 	.vidioc_g_fmt_vbi_cap	 = saa7164_vbi_fmt,
 	.vidioc_try_fmt_vbi_cap	 = saa7164_vbi_fmt,
 	.vidioc_s_fmt_vbi_cap	 = saa7164_vbi_fmt,
diff --git a/drivers/media/pci/saa7164/saa7164.h b/drivers/media/pci/saa7164/saa7164.h
index 18906e0..b3828c6 100644
--- a/drivers/media/pci/saa7164/saa7164.h
+++ b/drivers/media/pci/saa7164/saa7164.h
@@ -64,6 +64,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
 
 #include "saa7164-reg.h"
 #include "saa7164-types.h"
@@ -381,6 +382,7 @@ struct saa7164_port {
 	/* Encoder */
 	/* Defaults established in saa7164-encoder.c */
 	struct saa7164_tvnorm encodernorm;
+	struct v4l2_ctrl_handler ctrl_handler;
 	v4l2_std_id std;
 	u32 height;
 	u32 width;
-- 
2.1.4

Re: [PATCH] saa7164: convert to the control framework

[Ricardo Ribalda Delgado]

Hello Hans

With this patch I guess two of my previous patches are not needed.
Shall i resend the patchset or you just cherry pick the appropriate
ones?


Thanks!

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/22/2015 09:24 AM, Ricardo Ribalda Delgado wrote:
> Hello Hans
> 
> With this patch I guess two of my previous patches are not needed.
> Shall i resend the patchset or you just cherry pick the appropriate
> ones?

Let's see how long it takes before I get an Ack (or not) from Steve. If that's
quick, then you can incorporate my patch in your patch series, if it takes
longer (I know he's busy), then we can proceed with your patch series and I'll
rebase on top of that later.

Regards,

	Hans

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

On Sat, Aug 22, 2015 at 6:47 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 08/22/2015 09:24 AM, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> With this patch I guess two of my previous patches are not needed.
>> Shall i resend the patchset or you just cherry pick the appropriate
>> ones?
>
> Let's see how long it takes before I get an Ack (or not) from Steve. If that's
> quick, then you can incorporate my patch in your patch series, if it takes
> longer (I know he's busy), then we can proceed with your patch series and I'll
> rebase on top of that later.

Hans, thanks for the work here.

I've skimmed the patch buts its too much to eyeball to give a direct ack.

Has anyone tested the patch and validated each of the controls continue to work?

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/22/2015 02:06 PM, Steven Toth wrote:
> On Sat, Aug 22, 2015 at 6:47 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 08/22/2015 09:24 AM, Ricardo Ribalda Delgado wrote:
>>> Hello Hans
>>>
>>> With this patch I guess two of my previous patches are not needed.
>>> Shall i resend the patchset or you just cherry pick the appropriate
>>> ones?
>>
>> Let's see how long it takes before I get an Ack (or not) from Steve. If that's
>> quick, then you can incorporate my patch in your patch series, if it takes
>> longer (I know he's busy), then we can proceed with your patch series and I'll
>> rebase on top of that later.
> 
> Hans, thanks for the work here.
> 
> I've skimmed the patch buts its too much to eyeball to give a direct ack.
> 
> Has anyone tested the patch and validated each of the controls continue to work?

As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
you could test it.

Regards,

	Hans

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

On Sat, Aug 22, 2015 at 8:46 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 08/22/2015 02:06 PM, Steven Toth wrote:
>> On Sat, Aug 22, 2015 at 6:47 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> On 08/22/2015 09:24 AM, Ricardo Ribalda Delgado wrote:
>>>> Hello Hans
>>>>
>>>> With this patch I guess two of my previous patches are not needed.
>>>> Shall i resend the patchset or you just cherry pick the appropriate
>>>> ones?
>>>
>>> Let's see how long it takes before I get an Ack (or not) from Steve. If that's
>>> quick, then you can incorporate my patch in your patch series, if it takes
>>> longer (I know he's busy), then we can proceed with your patch series and I'll
>>> rebase on top of that later.
>>
>> Hans, thanks for the work here.
>>
>> I've skimmed the patch buts its too much to eyeball to give a direct ack.
>>
>> Has anyone tested the patch and validated each of the controls continue to work?
>
> As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
> you could test it.

OK, will do. I probably won't get to this until the weekend, but I'll
put this on my todo list.

Thx.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/26/15 15:08, Steven Toth wrote:
> On Sat, Aug 22, 2015 at 8:46 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 08/22/2015 02:06 PM, Steven Toth wrote:
>>> On Sat, Aug 22, 2015 at 6:47 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>>> On 08/22/2015 09:24 AM, Ricardo Ribalda Delgado wrote:
>>>>> Hello Hans
>>>>>
>>>>> With this patch I guess two of my previous patches are not needed.
>>>>> Shall i resend the patchset or you just cherry pick the appropriate
>>>>> ones?
>>>>
>>>> Let's see how long it takes before I get an Ack (or not) from Steve. If that's
>>>> quick, then you can incorporate my patch in your patch series, if it takes
>>>> longer (I know he's busy), then we can proceed with your patch series and I'll
>>>> rebase on top of that later.
>>>
>>> Hans, thanks for the work here.
>>>
>>> I've skimmed the patch buts its too much to eyeball to give a direct ack.
>>>
>>> Has anyone tested the patch and validated each of the controls continue to work?
>>
>> As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
>> you could test it.
> 
> OK, will do. I probably won't get to this until the weekend, but I'll
> put this on my todo list.

That's OK, there is no hurry. I tried to put my saa7164 in a different PC as well,
but it seems to be really broken as nothing appears in lspci :-(

Regards,

	Hans

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

>>>> Has anyone tested the patch and validated each of the controls continue to work?
>>>
>>> As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
>>> you could test it.
>>
>> OK, will do. I probably won't get to this until the weekend, but I'll
>> put this on my todo list.
>
> That's OK, there is no hurry. I tried to put my saa7164 in a different PC as well,
> but it seems to be really broken as nothing appears in lspci :-(

Send me your shipping address _privately_, I talk to Hauppauge about a
replacement.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/26/2015 03:23 PM, Steven Toth wrote:
>>>>> Has anyone tested the patch and validated each of the controls continue to work?
>>>>
>>>> As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
>>>> you could test it.
>>>
>>> OK, will do. I probably won't get to this until the weekend, but I'll
>>> put this on my todo list.
>>
>> That's OK, there is no hurry. I tried to put my saa7164 in a different PC as well,
>> but it seems to be really broken as nothing appears in lspci :-(
> 
> Send me your shipping address _privately_, I talk to Hauppauge about a
> replacement.
> 

No need, I managed to get it working if I use a PCI-to-PCIe adapter card. Very
strange, it won't work in the PCIe slot of my motherboard, but using the PCI slot
and that adapter it works fine.

It's good that it was tested since the menu control creation code was wrong.

One thing that is very confusing to me: I have this board:

[ 1878.280918] CORE saa7164[0]: subsystem: 0070:8900, board: Hauppauge WinTV-HVR2200 [card=5,autodetected]
[ 1878.280928] saa7164[0]/0: found at 0000:09:00.0, rev: 129, irq: 18, latency: 0, mmio: 0xfb800000
[ 1878.327399] tveeprom 14-0000: Hauppauge model 89519, rev B2F2, serial# 4029789519
[ 1878.327405] tveeprom 14-0000: MAC address is 00:0d:fe:31:b5:4f
[ 1878.327409] tveeprom 14-0000: tuner model is NXP 18271C2_716x (idx 152, type 4)
[ 1878.327413] tveeprom 14-0000: TV standards PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xfc)
[ 1878.327416] tveeprom 14-0000: audio processor is SAA7164 (idx 43)
[ 1878.327418] tveeprom 14-0000: decoder processor is CX23887A (idx 39)
[ 1878.327420] tveeprom 14-0000: has radio
[ 1878.327423] saa7164[0]: Hauppauge eeprom: model=89519

but the default firmware with size 4919072 fails to work (image corrupt), instead
I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).

For that I have to patch the driver.

Do you have an overview of which firmware is for which board?

There are a bunch of firmwares here:

http://www.steventoth.net/linux/hvr22xx/firmwares

but there are no instructions or an overview of which should be used.

I faintly remember asking you this before, but that's been a long time ago
and I can't find it in my mail archive.

I'm willing to do some driver cleanup and fix v4l2-compliance issues, but
I'd really like to fix this firmware issue first.

Regards,

	Hans

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/28/2015 09:59 AM, Hans Verkuil wrote:
> On 08/26/2015 03:23 PM, Steven Toth wrote:
>>>>>> Has anyone tested the patch and validated each of the controls continue to work?
>>>>>
>>>>> As I said: my saa7146 card is no longer recognized (not sure why), so I was hoping
>>>>> you could test it.
>>>>
>>>> OK, will do. I probably won't get to this until the weekend, but I'll
>>>> put this on my todo list.
>>>
>>> That's OK, there is no hurry. I tried to put my saa7164 in a different PC as well,
>>> but it seems to be really broken as nothing appears in lspci :-(
>>
>> Send me your shipping address _privately_, I talk to Hauppauge about a
>> replacement.
>>
> 
> No need, I managed to get it working if I use a PCI-to-PCIe adapter card. Very
> strange, it won't work in the PCIe slot of my motherboard, but using the PCI slot
> and that adapter it works fine.
> 
> It's good that it was tested since the menu control creation code was wrong.
> 
> One thing that is very confusing to me: I have this board:
> 
> [ 1878.280918] CORE saa7164[0]: subsystem: 0070:8900, board: Hauppauge WinTV-HVR2200 [card=5,autodetected]
> [ 1878.280928] saa7164[0]/0: found at 0000:09:00.0, rev: 129, irq: 18, latency: 0, mmio: 0xfb800000
> [ 1878.327399] tveeprom 14-0000: Hauppauge model 89519, rev B2F2, serial# 4029789519
> [ 1878.327405] tveeprom 14-0000: MAC address is 00:0d:fe:31:b5:4f
> [ 1878.327409] tveeprom 14-0000: tuner model is NXP 18271C2_716x (idx 152, type 4)
> [ 1878.327413] tveeprom 14-0000: TV standards PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xfc)
> [ 1878.327416] tveeprom 14-0000: audio processor is SAA7164 (idx 43)
> [ 1878.327418] tveeprom 14-0000: decoder processor is CX23887A (idx 39)
> [ 1878.327420] tveeprom 14-0000: has radio
> [ 1878.327423] saa7164[0]: Hauppauge eeprom: model=89519
> 
> but the default firmware with size 4919072 fails to work (image corrupt), instead
> I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).

That's v4l-saa7164-1.0.2-3.fw, sorry for the confusion.

Googling suggests that you have patches for this that never made it upstream.
Can you post it?

Regards,

	Hans

> 
> For that I have to patch the driver.
> 
> Do you have an overview of which firmware is for which board?
> 
> There are a bunch of firmwares here:
> 
> http://www.steventoth.net/linux/hvr22xx/firmwares
> 
> but there are no instructions or an overview of which should be used.
> 
> I faintly remember asking you this before, but that's been a long time ago
> and I can't find it in my mail archive.
> 
> I'm willing to do some driver cleanup and fix v4l2-compliance issues, but
> I'd really like to fix this firmware issue first.
> 
> Regards,
> 
> 	Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

>> Send me your shipping address _privately_, I talk to Hauppauge about a
>> replacement.
>>
>
> No need, I managed to get it working if I use a PCI-to-PCIe adapter card. Very
> strange, it won't work in the PCIe slot of my motherboard, but using the PCI slot
> and that adapter it works fine.

Excellent.

>
> It's good that it was tested since the menu control creation code was wrong.

Ahh.

>
> One thing that is very confusing to me: I have this board:
>
> [ 1878.280918] CORE saa7164[0]: subsystem: 0070:8900, board: Hauppauge WinTV-HVR2200 [card=5,autodetected]
> [ 1878.280928] saa7164[0]/0: found at 0000:09:00.0, rev: 129, irq: 18, latency: 0, mmio: 0xfb800000
> [ 1878.327399] tveeprom 14-0000: Hauppauge model 89519, rev B2F2, serial# 4029789519
> [ 1878.327405] tveeprom 14-0000: MAC address is 00:0d:fe:31:b5:4f
> [ 1878.327409] tveeprom 14-0000: tuner model is NXP 18271C2_716x (idx 152, type 4)
> [ 1878.327413] tveeprom 14-0000: TV standards PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xfc)
> [ 1878.327416] tveeprom 14-0000: audio processor is SAA7164 (idx 43)
> [ 1878.327418] tveeprom 14-0000: decoder processor is CX23887A (idx 39)
> [ 1878.327420] tveeprom 14-0000: has radio
> [ 1878.327423] saa7164[0]: Hauppauge eeprom: model=89519
>
> but the default firmware with size 4919072 fails to work (image corrupt), instead
> I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).
>
> For that I have to patch the driver.

Take a look at your board, on the main large PCIe IC, its probably
marked as either a REV2 or a REV3, or a -02 or -03, what do you have?

I suspect you have a rev-02 chip. Not many of them go out into
production. (A few thousand, compared to significantly more -03
chips).

>
> Do you have an overview of which firmware is for which board?

Generally, for a long time, I was recommending that everyone run
NXP7164-2010-03-10.1.fw, this is actually the latest firmware. I've
been told it isn't reliable on the REV2 hardware though.

I'll go back to the windows driver and check how they're making the
firmware load decision. I can bring this logic into the Linux driver
and we can load the most appropriate f/w.

>
> There are a bunch of firmwares here:
>
> http://www.steventoth.net/linux/hvr22xx/firmwares
>
> but there are no instructions or an overview of which should be used.

If the stock -inkernel driver is wrong for the -02 then we should fix
that. It should be fine for the -03 though.

>
> I faintly remember asking you this before, but that's been a long time ago
> and I can't find it in my mail archive.
>
> I'm willing to do some driver cleanup and fix v4l2-compliance issues, but
> I'd really like to fix this firmware issue first.

Noted.

>
> Regards,
>
>         Hans

Best,

- Steve

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com
+1.646.355.8490

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

>> but the default firmware with size 4919072 fails to work (image corrupt), instead
>> I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).
>
> That's v4l-saa7164-1.0.2-3.fw, sorry for the confusion.

Right, you need to load the -02 firmware on a -02 board.

>
> Googling suggests that you have patches for this that never made it upstream.
> Can you post it?

I will. If you can confirm you have a -02 PCIe IC then I'll prep some
firmware patches for testing. To be clear, I think the current in
kernel tree is perfectly good for all -03 boards. For -02 boards
(fewer of these in the field) it may be a problem, but I'll fix.

I also plan to test the control framework changes, and the compliance
patches (thanks btw).

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] saa7164: convert to the control framework

[Hans Verkuil]

On 08/28/2015 02:44 PM, Steven Toth wrote:
>>> Send me your shipping address _privately_, I talk to Hauppauge about a
>>> replacement.
>>>
>>
>> No need, I managed to get it working if I use a PCI-to-PCIe adapter card. Very
>> strange, it won't work in the PCIe slot of my motherboard, but using the PCI slot
>> and that adapter it works fine.
> 
> Excellent.
> 
>>
>> It's good that it was tested since the menu control creation code was wrong.
> 
> Ahh.
> 
>>
>> One thing that is very confusing to me: I have this board:
>>
>> [ 1878.280918] CORE saa7164[0]: subsystem: 0070:8900, board: Hauppauge WinTV-HVR2200 [card=5,autodetected]
>> [ 1878.280928] saa7164[0]/0: found at 0000:09:00.0, rev: 129, irq: 18, latency: 0, mmio: 0xfb800000
>> [ 1878.327399] tveeprom 14-0000: Hauppauge model 89519, rev B2F2, serial# 4029789519
>> [ 1878.327405] tveeprom 14-0000: MAC address is 00:0d:fe:31:b5:4f
>> [ 1878.327409] tveeprom 14-0000: tuner model is NXP 18271C2_716x (idx 152, type 4)
>> [ 1878.327413] tveeprom 14-0000: TV standards PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xfc)
>> [ 1878.327416] tveeprom 14-0000: audio processor is SAA7164 (idx 43)
>> [ 1878.327418] tveeprom 14-0000: decoder processor is CX23887A (idx 39)
>> [ 1878.327420] tveeprom 14-0000: has radio
>> [ 1878.327423] saa7164[0]: Hauppauge eeprom: model=89519
>>
>> but the default firmware with size 4919072 fails to work (image corrupt), instead
>> I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).
>>
>> For that I have to patch the driver.
> 
> Take a look at your board, on the main large PCIe IC, its probably
> marked as either a REV2 or a REV3, or a -02 or -03, what do you have?
> 
> I suspect you have a rev-02 chip. Not many of them go out into
> production. (A few thousand, compared to significantly more -03
> chips).

The text on the chip is:

SAA7164E/2
P60962.00	10
ESG07271Y

I suspect the /2 means REV2.

Regards,

	Hans

Re: [PATCH] saa7164: convert to the control framework

[Steven Toth]

>>> but the default firmware with size 4919072 fails to work (image corrupt), instead
>>> I need to use the firmware with size 4038864 (v4l-saa7164-1.0.3-3.fw).
>>>
>>> For that I have to patch the driver.
>>
>> Take a look at your board, on the main large PCIe IC, its probably
>> marked as either a REV2 or a REV3, or a -02 or -03, what do you have?
>>
>> I suspect you have a rev-02 chip. Not many of them go out into
>> production. (A few thousand, compared to significantly more -03
>> chips).
>
> The text on the chip is:
>
> SAA7164E/2
> P60962.00       10
> ESG07271Y
>
> I suspect the /2 means REV2.

Correct, thanks for confirming. I'll look into this.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com