* [PATCH 1/6] adv7180: fix broken standards handling
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 13:46 ` Federico Vaga
` (2 more replies)
2016-04-22 13:03 ` [PATCH 2/6] sta2x11_vip: fix s_std Hans Verkuil
` (4 subsequent siblings)
5 siblings, 3 replies; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media
Cc: Hans Verkuil, Niklas Söderlund, Lars-Peter Clausen,
Federico Vaga
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] adv7180: fix broken standards handling
2016-04-22 13:03 ` [PATCH 1/6] adv7180: fix broken standards handling Hans Verkuil
@ 2016-04-22 13:46 ` Federico Vaga
2016-04-22 14:19 ` Niklas Söderlund
2016-04-24 14:57 ` Lars-Peter Clausen
2 siblings, 0 replies; 14+ messages in thread
From: Federico Vaga @ 2016-04-22 13:46 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, Hans Verkuil, Niklas Söderlund,
Lars-Peter Clausen
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] adv7180: fix broken standards handling
2016-04-22 13:03 ` [PATCH 1/6] adv7180: fix broken standards handling Hans Verkuil
2016-04-22 13:46 ` Federico Vaga
@ 2016-04-22 14:19 ` Niklas Söderlund
2016-04-24 14:57 ` Lars-Peter Clausen
2 siblings, 0 replies; 14+ messages in thread
From: Niklas Söderlund @ 2016-04-22 14:19 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil, Lars-Peter Clausen, Federico Vaga
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] adv7180: fix broken standards handling
2016-04-22 13:03 ` [PATCH 1/6] adv7180: fix broken standards handling Hans Verkuil
2016-04-22 13:46 ` Federico Vaga
2016-04-22 14:19 ` Niklas Söderlund
@ 2016-04-24 14:57 ` Lars-Peter Clausen
2 siblings, 0 replies; 14+ messages in thread
From: Lars-Peter Clausen @ 2016-04-24 14:57 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Hans Verkuil, Niklas Söderlund, Federico Vaga
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] sta2x11_vip: fix s_std
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
2016-04-22 13:03 ` [PATCH 1/6] adv7180: fix broken standards handling Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 13:15 ` Federico Vaga
2016-04-22 13:03 ` [PATCH 3/6] rcar-vin: support the source change event and " Hans Verkuil
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Federico Vaga
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] sta2x11_vip: fix s_std
2016-04-22 13:03 ` [PATCH 2/6] sta2x11_vip: fix s_std Hans Verkuil
@ 2016-04-22 13:15 ` Federico Vaga
2016-04-22 13:17 ` Hans Verkuil
0 siblings, 1 reply; 14+ messages in thread
From: Federico Vaga @ 2016-04-22 13:15 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] sta2x11_vip: fix s_std
2016-04-22 13:15 ` Federico Vaga
@ 2016-04-22 13:17 ` Hans Verkuil
0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:17 UTC (permalink / raw)
To: Federico Vaga; +Cc: linux-media, 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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] rcar-vin: support the source change event and fix s_std
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
2016-04-22 13:03 ` [PATCH 1/6] adv7180: fix broken standards handling Hans Verkuil
2016-04-22 13:03 ` [PATCH 2/6] sta2x11_vip: fix s_std Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 18:55 ` Niklas Söderlund
2016-04-22 13:03 ` [PATCH 4/6] tc358743: drop bogus comment Hans Verkuil
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Niklas Söderlund, Ulrich Hecht
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/6] rcar-vin: support the source change event and fix s_std
2016-04-22 13:03 ` [PATCH 3/6] rcar-vin: support the source change event and " Hans Verkuil
@ 2016-04-22 18:55 ` Niklas Söderlund
0 siblings, 0 replies; 14+ messages in thread
From: Niklas Söderlund @ 2016-04-22 18:55 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil, Ulrich Hecht
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] tc358743: drop bogus comment
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
` (2 preceding siblings ...)
2016-04-22 13:03 ` [PATCH 3/6] rcar-vin: support the source change event and " Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 13:03 ` [PATCH 5/6] media/i2c/adv*: make controls inheritable instead of private Hans Verkuil
2016-04-22 13:03 ` [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0) Hans Verkuil
5 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] media/i2c/adv*: make controls inheritable instead of private
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
` (3 preceding siblings ...)
2016-04-22 13:03 ` [PATCH 4/6] tc358743: drop bogus comment Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 13:03 ` [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0) Hans Verkuil
5 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media; +Cc: 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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0)
2016-04-22 13:03 [PATCH 0/6] Various rcar-vin and adv* fixes Hans Verkuil
` (4 preceding siblings ...)
2016-04-22 13:03 ` [PATCH 5/6] media/i2c/adv*: make controls inheritable instead of private Hans Verkuil
@ 2016-04-22 13:03 ` Hans Verkuil
2016-04-22 18:57 ` Niklas Söderlund
5 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2016-04-22 13:03 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Niklas Söderlund
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0)
2016-04-22 13:03 ` [PATCH 6/6] rcar-vin: failed start_streaming didn't call s_stream(0) Hans Verkuil
@ 2016-04-22 18:57 ` Niklas Söderlund
0 siblings, 0 replies; 14+ messages in thread
From: Niklas Söderlund @ 2016-04-22 18:57 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
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
^ permalink raw reply [flat|nested] 14+ messages in thread