Acked-by: Federico Vaga <federico.vaga@xxxxxxxxx>
On Friday, April 22, 2016 03:03:37 PM Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Federico Vaga <federico.vaga@xxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
