[PATCH 0/6] Various rcar-vin and adv* fixes

[PATCH 0/6] Various rcar-vin and adv* fixes

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

While testing Niklas' new rcar-vin driver I found various problems
that is fixed in this patch series.

The main problem is the adv7180 driver which was simply broken when
it comes to detecting standards. This should now be fixed (at least
it works for me now).

As a result the sta2x11_vip driver needed to be changed as well.

The new rcar-vin driver driver didn't support the source change event.
While not relevant for the adv7180 driver (at least on the Koelsch
board the irq from that chip isn't hooked up), it certainly is needed
for the adv7612 driver. It doesn't hurt to add it here.

Niklas, just fold it in your patch.

The v4l2-compliance test complained about a missing V4L2_CID_DV_RX_POWER_PRESENT
control. This was because that control was marked private. The reality
is that that makes no sense and the control should just be inherited by
the rcar-vin driver. In practice making this private is just annoying.

The last patch is an rcar-vin bug fix. Niklas, just fold it in your
patch for the v8.

Niklas, Federico, can you both check the adv7180 changes?

Regards,

	Hans

Hans Verkuil (6):
  adv7180: fix broken standards handling
  sta2x11_vip: fix s_std
  rcar-vin: support the source change event and fix s_std
  tc358743: drop bogus comment
  media/i2c/adv*: make controls inheritable instead of private
  rcar-vin: failed start_streaming didn't call s_stream(0)

 drivers/media/i2c/ad9389b.c                 |   8 --
 drivers/media/i2c/adv7180.c                 | 118 +++++++++++++++++++---------
 drivers/media/i2c/adv7511.c                 |   6 --
 drivers/media/i2c/adv7604.c                 |   8 --
 drivers/media/i2c/adv7842.c                 |   6 --
 drivers/media/i2c/tc358743.c                |   1 -
 drivers/media/pci/sta2x11/sta2x11_vip.c     |  26 +++---
 drivers/media/platform/rcar-vin/rcar-dma.c  |   4 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  48 ++++++++++-
 9 files changed, 139 insertions(+), 86 deletions(-)

-- 
2.8.0.rc3

[PATCH 1/6] adv7180: fix broken standards handling

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The adv7180 attempts to autodetect the standard. Unfortunately this
is seriously broken.

This patch removes the autodetect completely. Only the querystd op
will detect the standard. Since the design of the adv7180 requires
that you switch to a special autodetect mode you cannot call querystd
when you are streaming.

So the s_stream op has been added so we know whether we are streaming
or not, and if we are, then querystd returns EBUSY.

After testing this with a signal generator is became obvious that
a sleep is needed between changing the standard to autodetect and
reading the status. So the autodetect can never have worked well.

The s_std call now just sets the new standard without any querying.

If the driver supports the interrupt, then when it detects a standard
change it will signal that by sending the V4L2_EVENT_SOURCE_CHANGE
event.

With these changes this driver now behaves like all other video
receivers.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/i2c/adv7180.c | 118 ++++++++++++++++++++++++++++++--------------
 1 file changed, 80 insertions(+), 38 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 51a92b3..5a75a91 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -26,8 +26,9 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/of.h>
-#include <media/v4l2-ioctl.h>
 #include <linux/videodev2.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 #include <linux/mutex.h>
@@ -192,8 +193,8 @@ struct adv7180_state {
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
 	v4l2_std_id		curr_norm;
-	bool			autodetect;
 	bool			powered;
+	bool			streaming;
 	u8			input;
 
 	struct i2c_client	*client;
@@ -338,12 +339,26 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
 	if (err)
 		return err;
 
-	/* when we are interrupt driven we know the state */
-	if (!state->autodetect || state->irq > 0)
-		*std = state->curr_norm;
-	else
-		err = __adv7180_status(state, NULL, std);
+	if (state->streaming) {
+		err = -EBUSY;
+		goto unlock;
+	}
+
+	err = adv7180_set_video_standard(state,
+			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
+	if (err)
+		goto unlock;
 
+	msleep(100);
+	__adv7180_status(state, NULL, std);
+
+	err = v4l2_std_to_adv7180(state->curr_norm);
+	if (err < 0)
+		goto unlock;
+
+	err = adv7180_set_video_standard(state, err);
+
+unlock:
 	mutex_unlock(&state->mutex);
 	return err;
 }
@@ -387,23 +402,13 @@ static int adv7180_program_std(struct adv7180_state *state)
 {
 	int ret;
 
-	if (state->autodetect) {
-		ret = adv7180_set_video_standard(state,
-			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
-		if (ret < 0)
-			return ret;
-
-		__adv7180_status(state, NULL, &state->curr_norm);
-	} else {
-		ret = v4l2_std_to_adv7180(state->curr_norm);
-		if (ret < 0)
-			return ret;
-
-		ret = adv7180_set_video_standard(state, ret);
-		if (ret < 0)
-			return ret;
-	}
+	ret = v4l2_std_to_adv7180(state->curr_norm);
+	if (ret < 0)
+		return ret;
 
+	ret = adv7180_set_video_standard(state, ret);
+	if (ret < 0)
+		return ret;
 	return 0;
 }
 
@@ -415,18 +420,12 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 	if (ret)
 		return ret;
 
-	/* all standards -> autodetect */
-	if (std == V4L2_STD_ALL) {
-		state->autodetect = true;
-	} else {
-		/* Make sure we can support this std */
-		ret = v4l2_std_to_adv7180(std);
-		if (ret < 0)
-			goto out;
+	/* Make sure we can support this std */
+	ret = v4l2_std_to_adv7180(std);
+	if (ret < 0)
+		goto out;
 
-		state->curr_norm = std;
-		state->autodetect = false;
-	}
+	state->curr_norm = std;
 
 	ret = adv7180_program_std(state);
 out:
@@ -747,6 +746,40 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
 	return 0;
 }
 
+static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct adv7180_state *state = to_state(sd);
+	int ret;
+
+	/* It's always safe to stop streaming, no need to take the lock */
+	if (!enable) {
+		state->streaming = enable;
+		return 0;
+	}
+
+	/* Must wait until querystd released the lock */
+	ret = mutex_lock_interruptible(&state->mutex);
+	if (ret)
+		return ret;
+	state->streaming = enable;
+	mutex_unlock(&state->mutex);
+	return 0;
+}
+
+static int adv7180_subscribe_event(struct v4l2_subdev *sd,
+				   struct v4l2_fh *fh,
+				   struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
+	case V4L2_EVENT_CTRL:
+		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 	.s_std = adv7180_s_std,
 	.g_std = adv7180_g_std,
@@ -756,10 +789,13 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 	.g_mbus_config = adv7180_g_mbus_config,
 	.cropcap = adv7180_cropcap,
 	.g_tvnorms = adv7180_g_tvnorms,
+	.s_stream = adv7180_s_stream,
 };
 
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
 	.s_power = adv7180_s_power,
+	.subscribe_event = adv7180_subscribe_event,
+	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
 };
 
 static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
@@ -784,8 +820,14 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
 	/* clear */
 	adv7180_write(state, ADV7180_REG_ICR3, isr3);
 
-	if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
-		__adv7180_status(state, NULL, &state->curr_norm);
+	if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
+		static const struct v4l2_event src_ch = {
+			.type = V4L2_EVENT_SOURCE_CHANGE,
+			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
+		};
+
+		v4l2_subdev_notify_event(&state->sd, &src_ch);
+	}
 	mutex_unlock(&state->mutex);
 
 	return IRQ_HANDLED;
@@ -1230,7 +1272,7 @@ static int adv7180_probe(struct i2c_client *client,
 
 	state->irq = client->irq;
 	mutex_init(&state->mutex);
-	state->autodetect = true;
+	state->curr_norm = V4L2_STD_NTSC;
 	if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
 		state->powered = true;
 	else
@@ -1238,7 +1280,7 @@ static int adv7180_probe(struct i2c_client *client,
 	state->input = 0;
 	sd = &state->sd;
 	v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
-	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
 
 	ret = adv7180_init_controls(state);
 	if (ret)
-- 
2.8.0.rc3

[PATCH 2/6] sta2x11_vip: fix s_std

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The s_std ioctl was broken in this driver, partially due to the
changes to the adv7180 driver (this affected the handling of
V4L2_STD_ALL) and partially because the new standard was never
stored in vip->std.

The handling of V4L2_STD_ALL has been rewritten to just call querystd
and the new standard is now stored correctly.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/pci/sta2x11/sta2x11_vip.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 753411c..c79623c 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -444,27 +444,21 @@ static int vidioc_querycap(struct file *file, void *priv,
 static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id std)
 {
 	struct sta2x11_vip *vip = video_drvdata(file);
-	v4l2_std_id oldstd = vip->std, newstd;
+	v4l2_std_id oldstd = vip->std;
 	int status;
 
-	if (V4L2_STD_ALL == std) {
-		v4l2_subdev_call(vip->decoder, video, s_std, std);
-		ssleep(2);
-		v4l2_subdev_call(vip->decoder, video, querystd, &newstd);
-		v4l2_subdev_call(vip->decoder, video, g_input_status, &status);
-		if (status & V4L2_IN_ST_NO_SIGNAL)
+	/*
+	 * This is here for backwards compatibility only.
+	 * The use of V4L2_STD_ALL to trigger a querystd is non-standard.
+	 */
+	if (std == V4L2_STD_ALL) {
+		v4l2_subdev_call(vip->decoder, video, querystd, &std);
+		if (std == V4L2_STD_UNKNOWN)
 			return -EIO;
-		std = vip->std = newstd;
-		if (oldstd != std) {
-			if (V4L2_STD_525_60 & std)
-				vip->format = formats_60[0];
-			else
-				vip->format = formats_50[0];
-		}
-		return 0;
 	}
 
-	if (oldstd != std) {
+	if (vip->std != std) {
+		vip->std = std;
 		if (V4L2_STD_525_60 & std)
 			vip->format = formats_60[0];
 		else
-- 
2.8.0.rc3

[PATCH 3/6] rcar-vin: support the source change event and fix s_std

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The patch adds support for the source change event and it fixes
the s_std support: changing the standard will also change the
resolution, and that was never updated.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Cc: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 48 +++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 8ac6149..49058ea 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -421,8 +421,25 @@ static int rvin_s_std(struct file *file, void *priv, v4l2_std_id a)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_sd(vin);
+	struct v4l2_subdev_format fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	struct v4l2_mbus_framefmt *mf = &fmt.format;
+	int ret = v4l2_subdev_call(sd, video, s_std, a);
+
+	if (ret < 0)
+		return ret;
 
-	return v4l2_subdev_call(sd, video, s_std, a);
+	/* Changing the standard will change the width/height */
+	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
+	if (ret) {
+		vin_err(vin, "Failed to get initial format\n");
+		return ret;
+	}
+
+	vin->format.width = mf->width;
+	vin->format.height = mf->height;
+	return 0;
 }
 
 static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
@@ -433,6 +450,16 @@ static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
 	return v4l2_subdev_call(sd, video, g_std, a);
 }
 
+static int rvin_subscribe_event(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_event_subscribe(fh, sub, 4, NULL);
+	}
+	return v4l2_ctrl_subscribe_event(fh, sub);
+}
+
 static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_querycap		= rvin_querycap,
 	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
@@ -464,7 +491,7 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_streamoff		= vb2_ioctl_streamoff,
 
 	.vidioc_log_status		= v4l2_ctrl_log_status,
-	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
+	.vidioc_subscribe_event		= rvin_subscribe_event,
 	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
 };
 
@@ -623,6 +650,21 @@ void rvin_v4l2_remove(struct rvin_dev *vin)
 	video_unregister_device(&vin->vdev);
 }
 
+static void rvin_notify(struct v4l2_subdev *sd,
+			unsigned int notification, void *arg)
+{
+	struct rvin_dev *vin =
+		container_of(sd->v4l2_dev, struct rvin_dev, v4l2_dev);
+
+	switch (notification) {
+	case V4L2_DEVICE_NOTIFY_EVENT:
+		v4l2_event_queue(&vin->vdev, arg);
+		break;
+	default:
+		break;
+	}
+}
+
 int rvin_v4l2_probe(struct rvin_dev *vin)
 {
 	struct v4l2_subdev_format fmt = {
@@ -635,6 +677,8 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
 
 	v4l2_set_subdev_hostdata(sd, vin);
 
+	vin->v4l2_dev.notify = rvin_notify;
+
 	ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms);
 	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
 		return ret;
-- 
2.8.0.rc3

[PATCH 4/6] tc358743: drop bogus comment

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The control in question is not a private control, so drop that
comment. Copy-and-paste left-over.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/tc358743.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 73e0cef..6cf6d06 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1863,7 +1863,6 @@ static int tc358743_probe(struct i2c_client *client,
 	/* control handlers */
 	v4l2_ctrl_handler_init(&state->hdl, 3);
 
-	/* private controls */
 	state->detect_tx_5v_ctrl = v4l2_ctrl_new_std(&state->hdl, NULL,
 			V4L2_CID_DV_RX_POWER_PRESENT, 0, 1, 0, 0);
 
-- 
2.8.0.rc3

[PATCH 5/6] media/i2c/adv*: make controls inheritable instead of private

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Marking these controls as private seemed a good idea at one time,
but in practice it makes no sense. So drop this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/i2c/ad9389b.c | 8 --------
 drivers/media/i2c/adv7511.c | 6 ------
 drivers/media/i2c/adv7604.c | 8 --------
 drivers/media/i2c/adv7842.c | 6 ------
 4 files changed, 28 deletions(-)

diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 788967d..0462f46 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1130,8 +1130,6 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id *
 	hdl = &state->hdl;
 	v4l2_ctrl_handler_init(hdl, 5);
 
-	/* private controls */
-
 	state->hdmi_mode_ctrl = v4l2_ctrl_new_std_menu(hdl, &ad9389b_ctrl_ops,
 			V4L2_CID_DV_TX_MODE, V4L2_DV_TX_MODE_HDMI,
 			0, V4L2_DV_TX_MODE_DVI_D);
@@ -1151,12 +1149,6 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id *
 
 		goto err_hdl;
 	}
-	state->hdmi_mode_ctrl->is_private = true;
-	state->hotplug_ctrl->is_private = true;
-	state->rx_sense_ctrl->is_private = true;
-	state->have_edid0_ctrl->is_private = true;
-	state->rgb_quantization_range_ctrl->is_private = true;
-
 	state->pad.flags = MEDIA_PAD_FL_SINK;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err)
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index bd822f0..39271c3 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -1502,12 +1502,6 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id *
 		err = hdl->error;
 		goto err_hdl;
 	}
-	state->hdmi_mode_ctrl->is_private = true;
-	state->hotplug_ctrl->is_private = true;
-	state->rx_sense_ctrl->is_private = true;
-	state->have_edid0_ctrl->is_private = true;
-	state->rgb_quantization_range_ctrl->is_private = true;
-
 	state->pad.flags = MEDIA_PAD_FL_SINK;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 41a1bfc..beb2841 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3141,7 +3141,6 @@ static int adv76xx_probe(struct i2c_client *client,
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
 
-	/* private controls */
 	state->detect_tx_5v_ctrl = v4l2_ctrl_new_std(hdl, NULL,
 			V4L2_CID_DV_RX_POWER_PRESENT, 0,
 			(1 << state->info->num_dv_ports) - 1, 0, 0);
@@ -3164,13 +3163,6 @@ static int adv76xx_probe(struct i2c_client *client,
 		err = hdl->error;
 		goto err_hdl;
 	}
-	state->detect_tx_5v_ctrl->is_private = true;
-	state->rgb_quantization_range_ctrl->is_private = true;
-	if (adv76xx_has_afe(state))
-		state->analog_sampling_phase_ctrl->is_private = true;
-	state->free_run_color_manual_ctrl->is_private = true;
-	state->free_run_color_ctrl->is_private = true;
-
 	if (adv76xx_s_detect_tx_5v_ctrl(sd)) {
 		err = -ENODEV;
 		goto err_hdl;
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 7ccb85d..ecaacb0 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3300,12 +3300,6 @@ static int adv7842_probe(struct i2c_client *client,
 		err = hdl->error;
 		goto err_hdl;
 	}
-	state->detect_tx_5v_ctrl->is_private = true;
-	state->rgb_quantization_range_ctrl->is_private = true;
-	state->analog_sampling_phase_ctrl->is_private = true;
-	state->free_run_color_ctrl_manual->is_private = true;
-	state->free_run_color_ctrl->is_private = true;
-
 	if (adv7842_s_detect_tx_5v_ctrl(sd)) {
 		err = -ENODEV;
 		goto err_hdl;
-- 
2.8.0.rc3

[PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0)

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This can leave adv7180 in an inconsistent state

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 644ec9b..087c30c 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1058,8 +1058,10 @@ static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
 	ret = rvin_capture_start(vin);
 out:
 	/* Return all buffers if something went wrong */
-	if (ret)
+	if (ret) {
 		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
+		v4l2_subdev_call(sd, video, s_stream, 0);
+	}
 
 	spin_unlock_irqrestore(&vin->qlock, flags);
 
-- 
2.8.0.rc3

Re: [PATCH 2/6] sta2x11_vip: fix s_std

[Federico Vaga]

Acked-by: Federico Vaga <federico.vaga@gmail.com>

It sounds fine to me (even the ADV7180 patch). Unfortunately I do not have the 
hardware to test it.

On Friday, April 22, 2016 03:03:38 PM Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The s_std ioctl was broken in this driver, partially due to the
> changes to the adv7180 driver (this affected the handling of
> V4L2_STD_ALL) and partially because the new standard was never
> stored in vip->std.
> 
> The handling of V4L2_STD_ALL has been rewritten to just call querystd
> and the new standard is now stored correctly.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Federico Vaga <federico.vaga@gmail.com>
> ---
>  drivers/media/pci/sta2x11/sta2x11_vip.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
> b/drivers/media/pci/sta2x11/sta2x11_vip.c index 753411c..c79623c 100644
> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
> @@ -444,27 +444,21 @@ static int vidioc_querycap(struct file *file, void
> *priv, static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id
> std) {
>  	struct sta2x11_vip *vip = video_drvdata(file);
> -	v4l2_std_id oldstd = vip->std, newstd;
> +	v4l2_std_id oldstd = vip->std;
>  	int status;
> 
> -	if (V4L2_STD_ALL == std) {
> -		v4l2_subdev_call(vip->decoder, video, s_std, std);
> -		ssleep(2);
> -		v4l2_subdev_call(vip->decoder, video, querystd, &newstd);
> -		v4l2_subdev_call(vip->decoder, video, g_input_status, &status);
> -		if (status & V4L2_IN_ST_NO_SIGNAL)
> +	/*
> +	 * This is here for backwards compatibility only.
> +	 * The use of V4L2_STD_ALL to trigger a querystd is non-standard.
> +	 */
> +	if (std == V4L2_STD_ALL) {
> +		v4l2_subdev_call(vip->decoder, video, querystd, &std);
> +		if (std == V4L2_STD_UNKNOWN)
>  			return -EIO;
> -		std = vip->std = newstd;
> -		if (oldstd != std) {
> -			if (V4L2_STD_525_60 & std)
> -				vip->format = formats_60[0];
> -			else
> -				vip->format = formats_50[0];
> -		}
> -		return 0;
>  	}
> 
> -	if (oldstd != std) {
> +	if (vip->std != std) {
> +		vip->std = std;
>  		if (V4L2_STD_525_60 & std)
>  			vip->format = formats_60[0];
>  		else

-- 
Federico Vaga

Re: [PATCH 2/6] sta2x11_vip: fix s_std

[Hans Verkuil]

On 04/22/2016 03:15 PM, Federico Vaga wrote:
> Acked-by: Federico Vaga <federico.vaga@gmail.com>
> 
> It sounds fine to me (even the ADV7180 patch). Unfortunately I do not have the 
> hardware to test it.

Your Ack will have to suffice :-)

Can you Ack the adv7180 patch as well? Would be nice.

Regards,

	Hans

> 
> On Friday, April 22, 2016 03:03:38 PM Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The s_std ioctl was broken in this driver, partially due to the
>> changes to the adv7180 driver (this affected the handling of
>> V4L2_STD_ALL) and partially because the new standard was never
>> stored in vip->std.
>>
>> The handling of V4L2_STD_ALL has been rewritten to just call querystd
>> and the new standard is now stored correctly.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: Federico Vaga <federico.vaga@gmail.com>
>> ---
>>  drivers/media/pci/sta2x11/sta2x11_vip.c | 26 ++++++++++----------------
>>  1 file changed, 10 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
>> b/drivers/media/pci/sta2x11/sta2x11_vip.c index 753411c..c79623c 100644
>> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
>> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
>> @@ -444,27 +444,21 @@ static int vidioc_querycap(struct file *file, void
>> *priv, static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id
>> std) {
>>  	struct sta2x11_vip *vip = video_drvdata(file);
>> -	v4l2_std_id oldstd = vip->std, newstd;
>> +	v4l2_std_id oldstd = vip->std;
>>  	int status;
>>
>> -	if (V4L2_STD_ALL == std) {
>> -		v4l2_subdev_call(vip->decoder, video, s_std, std);
>> -		ssleep(2);
>> -		v4l2_subdev_call(vip->decoder, video, querystd, &newstd);
>> -		v4l2_subdev_call(vip->decoder, video, g_input_status, &status);
>> -		if (status & V4L2_IN_ST_NO_SIGNAL)
>> +	/*
>> +	 * This is here for backwards compatibility only.
>> +	 * The use of V4L2_STD_ALL to trigger a querystd is non-standard.
>> +	 */
>> +	if (std == V4L2_STD_ALL) {
>> +		v4l2_subdev_call(vip->decoder, video, querystd, &std);
>> +		if (std == V4L2_STD_UNKNOWN)
>>  			return -EIO;
>> -		std = vip->std = newstd;
>> -		if (oldstd != std) {
>> -			if (V4L2_STD_525_60 & std)
>> -				vip->format = formats_60[0];
>> -			else
>> -				vip->format = formats_50[0];
>> -		}
>> -		return 0;
>>  	}
>>
>> -	if (oldstd != std) {
>> +	if (vip->std != std) {
>> +		vip->std = std;
>>  		if (V4L2_STD_525_60 & std)
>>  			vip->format = formats_60[0];
>>  		else
> 

Re: [PATCH 1/6] adv7180: fix broken standards handling

[Federico Vaga]

Acked-by: Federico Vaga <federico.vaga@gmail.com>

On Friday, April 22, 2016 03:03:37 PM Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The adv7180 attempts to autodetect the standard. Unfortunately this
> is seriously broken.
> 
> This patch removes the autodetect completely. Only the querystd op
> will detect the standard. Since the design of the adv7180 requires
> that you switch to a special autodetect mode you cannot call querystd
> when you are streaming.
> 
> So the s_stream op has been added so we know whether we are streaming
> or not, and if we are, then querystd returns EBUSY.
> 
> After testing this with a signal generator is became obvious that
> a sleep is needed between changing the standard to autodetect and
> reading the status. So the autodetect can never have worked well.
> 
> The s_std call now just sets the new standard without any querying.
> 
> If the driver supports the interrupt, then when it detects a standard
> change it will signal that by sending the V4L2_EVENT_SOURCE_CHANGE
> event.
> 
> With these changes this driver now behaves like all other video
> receivers.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Federico Vaga <federico.vaga@gmail.com>
> ---
>  drivers/media/i2c/adv7180.c | 118
> ++++++++++++++++++++++++++++++-------------- 1 file changed, 80
> insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 51a92b3..5a75a91 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -26,8 +26,9 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> -#include <media/v4l2-ioctl.h>
>  #include <linux/videodev2.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
>  #include <linux/mutex.h>
> @@ -192,8 +193,8 @@ struct adv7180_state {
>  	struct mutex		mutex; /* mutual excl. when accessing chip */
>  	int			irq;
>  	v4l2_std_id		curr_norm;
> -	bool			autodetect;
>  	bool			powered;
> +	bool			streaming;
>  	u8			input;
> 
>  	struct i2c_client	*client;
> @@ -338,12 +339,26 @@ static int adv7180_querystd(struct v4l2_subdev *sd,
> v4l2_std_id *std) if (err)
>  		return err;
> 
> -	/* when we are interrupt driven we know the state */
> -	if (!state->autodetect || state->irq > 0)
> -		*std = state->curr_norm;
> -	else
> -		err = __adv7180_status(state, NULL, std);
> +	if (state->streaming) {
> +		err = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	err = adv7180_set_video_standard(state,
> +			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> +	if (err)
> +		goto unlock;
> 
> +	msleep(100);
> +	__adv7180_status(state, NULL, std);
> +
> +	err = v4l2_std_to_adv7180(state->curr_norm);
> +	if (err < 0)
> +		goto unlock;
> +
> +	err = adv7180_set_video_standard(state, err);
> +
> +unlock:
>  	mutex_unlock(&state->mutex);
>  	return err;
>  }
> @@ -387,23 +402,13 @@ static int adv7180_program_std(struct adv7180_state
> *state) {
>  	int ret;
> 
> -	if (state->autodetect) {
> -		ret = adv7180_set_video_standard(state,
> -			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> -		if (ret < 0)
> -			return ret;
> -
> -		__adv7180_status(state, NULL, &state->curr_norm);
> -	} else {
> -		ret = v4l2_std_to_adv7180(state->curr_norm);
> -		if (ret < 0)
> -			return ret;
> -
> -		ret = adv7180_set_video_standard(state, ret);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = v4l2_std_to_adv7180(state->curr_norm);
> +	if (ret < 0)
> +		return ret;
> 
> +	ret = adv7180_set_video_standard(state, ret);
> +	if (ret < 0)
> +		return ret;
>  	return 0;
>  }
> 
> @@ -415,18 +420,12 @@ static int adv7180_s_std(struct v4l2_subdev *sd,
> v4l2_std_id std) if (ret)
>  		return ret;
> 
> -	/* all standards -> autodetect */
> -	if (std == V4L2_STD_ALL) {
> -		state->autodetect = true;
> -	} else {
> -		/* Make sure we can support this std */
> -		ret = v4l2_std_to_adv7180(std);
> -		if (ret < 0)
> -			goto out;
> +	/* Make sure we can support this std */
> +	ret = v4l2_std_to_adv7180(std);
> +	if (ret < 0)
> +		goto out;
> 
> -		state->curr_norm = std;
> -		state->autodetect = false;
> -	}
> +	state->curr_norm = std;
> 
>  	ret = adv7180_program_std(state);
>  out:
> @@ -747,6 +746,40 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd,
> v4l2_std_id *norm) return 0;
>  }
> 
> +static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct adv7180_state *state = to_state(sd);
> +	int ret;
> +
> +	/* It's always safe to stop streaming, no need to take the lock */
> +	if (!enable) {
> +		state->streaming = enable;
> +		return 0;
> +	}
> +
> +	/* Must wait until querystd released the lock */
> +	ret = mutex_lock_interruptible(&state->mutex);
> +	if (ret)
> +		return ret;
> +	state->streaming = enable;
> +	mutex_unlock(&state->mutex);
> +	return 0;
> +}
> +
> +static int adv7180_subscribe_event(struct v4l2_subdev *sd,
> +				   struct v4l2_fh *fh,
> +				   struct v4l2_event_subscription *sub)
> +{
> +	switch (sub->type) {
> +	case V4L2_EVENT_SOURCE_CHANGE:
> +		return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> +	case V4L2_EVENT_CTRL:
> +		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>  	.s_std = adv7180_s_std,
>  	.g_std = adv7180_g_std,
> @@ -756,10 +789,13 @@ static const struct v4l2_subdev_video_ops
> adv7180_video_ops = { .g_mbus_config = adv7180_g_mbus_config,
>  	.cropcap = adv7180_cropcap,
>  	.g_tvnorms = adv7180_g_tvnorms,
> +	.s_stream = adv7180_s_stream,
>  };
> 
>  static const struct v4l2_subdev_core_ops adv7180_core_ops = {
>  	.s_power = adv7180_s_power,
> +	.subscribe_event = adv7180_subscribe_event,
> +	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>  };
> 
>  static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
> @@ -784,8 +820,14 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
>  	/* clear */
>  	adv7180_write(state, ADV7180_REG_ICR3, isr3);
> 
> -	if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
> -		__adv7180_status(state, NULL, &state->curr_norm);
> +	if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
> +		static const struct v4l2_event src_ch = {
> +			.type = V4L2_EVENT_SOURCE_CHANGE,
> +			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> +		};
> +
> +		v4l2_subdev_notify_event(&state->sd, &src_ch);
> +	}
>  	mutex_unlock(&state->mutex);
> 
>  	return IRQ_HANDLED;
> @@ -1230,7 +1272,7 @@ static int adv7180_probe(struct i2c_client *client,
> 
>  	state->irq = client->irq;
>  	mutex_init(&state->mutex);
> -	state->autodetect = true;
> +	state->curr_norm = V4L2_STD_NTSC;
>  	if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
>  		state->powered = true;
>  	else
> @@ -1238,7 +1280,7 @@ static int adv7180_probe(struct i2c_client *client,
>  	state->input = 0;
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> -	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> 
>  	ret = adv7180_init_controls(state);
>  	if (ret)

-- 
Federico Vaga

Re: [PATCH 1/6] adv7180: fix broken standards handling

[Niklas Söderlund]

Hi Hans,

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

On 2016-04-22 15:03:37 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The adv7180 attempts to autodetect the standard. Unfortunately this
> is seriously broken.
> 
> This patch removes the autodetect completely. Only the querystd op
> will detect the standard. Since the design of the adv7180 requires
> that you switch to a special autodetect mode you cannot call querystd
> when you are streaming.
> 
> So the s_stream op has been added so we know whether we are streaming
> or not, and if we are, then querystd returns EBUSY.
> 
> After testing this with a signal generator is became obvious that
> a sleep is needed between changing the standard to autodetect and
> reading the status. So the autodetect can never have worked well.
> 
> The s_std call now just sets the new standard without any querying.
> 
> If the driver supports the interrupt, then when it detects a standard
> change it will signal that by sending the V4L2_EVENT_SOURCE_CHANGE
> event.
> 
> With these changes this driver now behaves like all other video
> receivers.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Federico Vaga <federico.vaga@gmail.com>
> ---
>  drivers/media/i2c/adv7180.c | 118 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 80 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index 51a92b3..5a75a91 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -26,8 +26,9 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> -#include <media/v4l2-ioctl.h>
>  #include <linux/videodev2.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
>  #include <linux/mutex.h>
> @@ -192,8 +193,8 @@ struct adv7180_state {
>  	struct mutex		mutex; /* mutual excl. when accessing chip */
>  	int			irq;
>  	v4l2_std_id		curr_norm;
> -	bool			autodetect;
>  	bool			powered;
> +	bool			streaming;
>  	u8			input;
>  
>  	struct i2c_client	*client;
> @@ -338,12 +339,26 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
>  	if (err)
>  		return err;
>  
> -	/* when we are interrupt driven we know the state */
> -	if (!state->autodetect || state->irq > 0)
> -		*std = state->curr_norm;
> -	else
> -		err = __adv7180_status(state, NULL, std);
> +	if (state->streaming) {
> +		err = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	err = adv7180_set_video_standard(state,
> +			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> +	if (err)
> +		goto unlock;
>  
> +	msleep(100);
> +	__adv7180_status(state, NULL, std);
> +
> +	err = v4l2_std_to_adv7180(state->curr_norm);
> +	if (err < 0)
> +		goto unlock;
> +
> +	err = adv7180_set_video_standard(state, err);
> +
> +unlock:
>  	mutex_unlock(&state->mutex);
>  	return err;
>  }
> @@ -387,23 +402,13 @@ static int adv7180_program_std(struct adv7180_state *state)
>  {
>  	int ret;
>  
> -	if (state->autodetect) {
> -		ret = adv7180_set_video_standard(state,
> -			ADV7180_STD_AD_PAL_BG_NTSC_J_SECAM);
> -		if (ret < 0)
> -			return ret;
> -
> -		__adv7180_status(state, NULL, &state->curr_norm);
> -	} else {
> -		ret = v4l2_std_to_adv7180(state->curr_norm);
> -		if (ret < 0)
> -			return ret;
> -
> -		ret = adv7180_set_video_standard(state, ret);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = v4l2_std_to_adv7180(state->curr_norm);
> +	if (ret < 0)
> +		return ret;
>  
> +	ret = adv7180_set_video_standard(state, ret);
> +	if (ret < 0)
> +		return ret;
>  	return 0;
>  }
>  
> @@ -415,18 +420,12 @@ static int adv7180_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
>  	if (ret)
>  		return ret;
>  
> -	/* all standards -> autodetect */
> -	if (std == V4L2_STD_ALL) {
> -		state->autodetect = true;
> -	} else {
> -		/* Make sure we can support this std */
> -		ret = v4l2_std_to_adv7180(std);
> -		if (ret < 0)
> -			goto out;
> +	/* Make sure we can support this std */
> +	ret = v4l2_std_to_adv7180(std);
> +	if (ret < 0)
> +		goto out;
>  
> -		state->curr_norm = std;
> -		state->autodetect = false;
> -	}
> +	state->curr_norm = std;
>  
>  	ret = adv7180_program_std(state);
>  out:
> @@ -747,6 +746,40 @@ static int adv7180_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
>  	return 0;
>  }
>  
> +static int adv7180_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> +	struct adv7180_state *state = to_state(sd);
> +	int ret;
> +
> +	/* It's always safe to stop streaming, no need to take the lock */
> +	if (!enable) {
> +		state->streaming = enable;
> +		return 0;
> +	}
> +
> +	/* Must wait until querystd released the lock */
> +	ret = mutex_lock_interruptible(&state->mutex);
> +	if (ret)
> +		return ret;
> +	state->streaming = enable;
> +	mutex_unlock(&state->mutex);
> +	return 0;
> +}
> +
> +static int adv7180_subscribe_event(struct v4l2_subdev *sd,
> +				   struct v4l2_fh *fh,
> +				   struct v4l2_event_subscription *sub)
> +{
> +	switch (sub->type) {
> +	case V4L2_EVENT_SOURCE_CHANGE:
> +		return v4l2_src_change_event_subdev_subscribe(sd, fh, sub);
> +	case V4L2_EVENT_CTRL:
> +		return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>  	.s_std = adv7180_s_std,
>  	.g_std = adv7180_g_std,
> @@ -756,10 +789,13 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>  	.g_mbus_config = adv7180_g_mbus_config,
>  	.cropcap = adv7180_cropcap,
>  	.g_tvnorms = adv7180_g_tvnorms,
> +	.s_stream = adv7180_s_stream,
>  };
>  
>  static const struct v4l2_subdev_core_ops adv7180_core_ops = {
>  	.s_power = adv7180_s_power,
> +	.subscribe_event = adv7180_subscribe_event,
> +	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>  };
>  
>  static const struct v4l2_subdev_pad_ops adv7180_pad_ops = {
> @@ -784,8 +820,14 @@ static irqreturn_t adv7180_irq(int irq, void *devid)
>  	/* clear */
>  	adv7180_write(state, ADV7180_REG_ICR3, isr3);
>  
> -	if (isr3 & ADV7180_IRQ3_AD_CHANGE && state->autodetect)
> -		__adv7180_status(state, NULL, &state->curr_norm);
> +	if (isr3 & ADV7180_IRQ3_AD_CHANGE) {
> +		static const struct v4l2_event src_ch = {
> +			.type = V4L2_EVENT_SOURCE_CHANGE,
> +			.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
> +		};
> +
> +		v4l2_subdev_notify_event(&state->sd, &src_ch);
> +	}
>  	mutex_unlock(&state->mutex);
>  
>  	return IRQ_HANDLED;
> @@ -1230,7 +1272,7 @@ static int adv7180_probe(struct i2c_client *client,
>  
>  	state->irq = client->irq;
>  	mutex_init(&state->mutex);
> -	state->autodetect = true;
> +	state->curr_norm = V4L2_STD_NTSC;
>  	if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
>  		state->powered = true;
>  	else
> @@ -1238,7 +1280,7 @@ static int adv7180_probe(struct i2c_client *client,
>  	state->input = 0;
>  	sd = &state->sd;
>  	v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
> -	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
>  
>  	ret = adv7180_init_controls(state);
>  	if (ret)
> -- 
> 2.8.0.rc3
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/6] rcar-vin: support the source change event and fix s_std

[Niklas Söderlund]

Hi Hans,

Great patch,

On 2016-04-22 15:03:39 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The patch adds support for the source change event and it fixes
> the s_std support: changing the standard will also change the
> resolution, and that was never updated.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Cc: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 48 +++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 8ac6149..49058ea 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -421,8 +421,25 @@ static int rvin_s_std(struct file *file, void *priv, v4l2_std_id a)
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
>  	struct v4l2_subdev *sd = vin_to_sd(vin);
> +	struct v4l2_subdev_format fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +	struct v4l2_mbus_framefmt *mf = &fmt.format;
> +	int ret = v4l2_subdev_call(sd, video, s_std, a);
> +
> +	if (ret < 0)
> +		return ret;
>  
> -	return v4l2_subdev_call(sd, video, s_std, a);
> +	/* Changing the standard will change the width/height */
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> +	if (ret) {
> +		vin_err(vin, "Failed to get initial format\n");
> +		return ret;
> +	}
> +
> +	vin->format.width = mf->width;
> +	vin->format.height = mf->height;

I think you should reset the vin->crop and vin->compose v4l2_rect to 
match the new size here.

On this note I feel the documentation at 
https://linuxtv.org/downloads/v4l-dvb-apis/selection-api.html are a bit 
unclear. In the section 'Configuration of video capture' one can read:

"Each capture device has a default source rectangle, given by the 
V4L2_SEL_TGT_CROP_DEFAULT target. This rectangle shall over what the 
driver writer considers the complete picture. Drivers shall set the 
active crop rectangle to the default when the driver is first loaded, 
but not later."

For the driver to change this rectangle in s_std is that not in the 'but 
not later' category?

On the same note, should the crop and compose rectangles be reset if the 
user requests a resolution change with vidioc_s_fmt_vid_cap? 

> +	return 0;
>  }
>  
>  static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
> @@ -433,6 +450,16 @@ static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
>  	return v4l2_subdev_call(sd, video, g_std, a);
>  }
>  
> +static int rvin_subscribe_event(struct v4l2_fh *fh,
> +				const struct v4l2_event_subscription *sub)
> +{
> +	switch (sub->type) {
> +	case V4L2_EVENT_SOURCE_CHANGE:
> +		return v4l2_event_subscribe(fh, sub, 4, NULL);
> +	}
> +	return v4l2_ctrl_subscribe_event(fh, sub);
> +}
> +
>  static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_querycap		= rvin_querycap,
>  	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
> @@ -464,7 +491,7 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_streamoff		= vb2_ioctl_streamoff,
>  
>  	.vidioc_log_status		= v4l2_ctrl_log_status,
> -	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
> +	.vidioc_subscribe_event		= rvin_subscribe_event,
>  	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
>  };
>  
> @@ -623,6 +650,21 @@ void rvin_v4l2_remove(struct rvin_dev *vin)
>  	video_unregister_device(&vin->vdev);
>  }
>  
> +static void rvin_notify(struct v4l2_subdev *sd,
> +			unsigned int notification, void *arg)
> +{
> +	struct rvin_dev *vin =
> +		container_of(sd->v4l2_dev, struct rvin_dev, v4l2_dev);
> +
> +	switch (notification) {
> +	case V4L2_DEVICE_NOTIFY_EVENT:
> +		v4l2_event_queue(&vin->vdev, arg);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  int rvin_v4l2_probe(struct rvin_dev *vin)
>  {
>  	struct v4l2_subdev_format fmt = {
> @@ -635,6 +677,8 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>  
>  	v4l2_set_subdev_hostdata(sd, vin);
>  
> +	vin->v4l2_dev.notify = rvin_notify;
> +
>  	ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms);
>  	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>  		return ret;
> -- 
> 2.8.0.rc3
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0)

[Niklas Söderlund]

Hi Hans,

Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

On 2016-04-22 15:03:42 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This can leave adv7180 in an inconsistent state
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 644ec9b..087c30c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1058,8 +1058,10 @@ static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	ret = rvin_capture_start(vin);
>  out:
>  	/* Return all buffers if something went wrong */
> -	if (ret)
> +	if (ret) {
>  		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
> +		v4l2_subdev_call(sd, video, s_stream, 0);
> +	}
>  
>  	spin_unlock_irqrestore(&vin->qlock, flags);
>  
> -- 
> 2.8.0.rc3
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/6] adv7180: fix broken standards handling

[Lars-Peter Clausen]

On 04/22/2016 03:03 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The adv7180 attempts to autodetect the standard. Unfortunately this
> is seriously broken.
> 
> This patch removes the autodetect completely. Only the querystd op
> will detect the standard. Since the design of the adv7180 requires
> that you switch to a special autodetect mode you cannot call querystd
> when you are streaming.
> 
> So the s_stream op has been added so we know whether we are streaming
> or not, and if we are, then querystd returns EBUSY.
> 
> After testing this with a signal generator is became obvious that
> a sleep is needed between changing the standard to autodetect and
> reading the status. So the autodetect can never have worked well.
> 
> The s_std call now just sets the new standard without any querying.
> 
> If the driver supports the interrupt, then when it detects a standard
> change it will signal that by sending the V4L2_EVENT_SOURCE_CHANGE
> event.
> 
> With these changes this driver now behaves like all other video
> receivers.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Federico Vaga <federico.vaga@gmail.com>

Thanks for cleaning this up.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>