Re: [PATCH 13/22] [media] tvp5150: disable output while signal not locked

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Thu, 28 Jun 2018 18:20:45 +0200
Marco Felsch <m.felsch@pengutronix.de> escreveu:

> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> To avoid short frames on stream start, keep output pins at high impedance
> while we are not properly locked onto the input signal.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/media/i2c/tvp5150.c | 39 ++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index d8bdbedd8826..27cfd08be3d2 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -60,6 +60,7 @@ struct tvp5150 {
>  	v4l2_std_id detected_norm;
>  	u32 input;
>  	u32 output;
> +	u32 oe;
>  	int enable;
>  	bool lock;
>  
> @@ -799,14 +800,20 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id)
>  {
>  	struct tvp5150 *decoder = dev_id;
>  	struct regmap *map = decoder->regmap;
> -	unsigned int active = 0, status = 0;
> +	unsigned int mask, active = 0, status = 0;
> +
> +	mask = TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_SYNC_OE |
> +	       TVP5150_MISC_CTL_CLOCK_OE;
>  
>  	regmap_read(map, TVP5150_INT_STATUS_REG_A, &status);
>  	if (status) {
>  		regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
>  
> -		if (status & TVP5150_INT_A_LOCK)
> +		if (status & TVP5150_INT_A_LOCK) {
>  			decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
> +			regmap_update_bits(map, TVP5150_MISC_CTL, mask,
> +					   decoder->lock ? decoder->oe : 0);
> +		}
>  
>  		return IRQ_HANDLED;
>  	}
> @@ -872,10 +879,26 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
>  	/* Disable autoswitch mode */
>  	tvp5150_set_std(sd, std);
>  
> -	if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> +	/*
> +	 * Enable the YCbCr and clock outputs. In discrete sync mode
> +	 * (non-BT.656) additionally enable the the sync outputs.
> +	 */
> +	switch (decoder->mbus_type) {
> +	case V4L2_MBUS_PARALLEL:
>  		/* 8-bit 4:2:2 YUV with discrete sync output */
>  		regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
>  				   0x7, 0x0);
> +		decoder->oe = TVP5150_MISC_CTL_YCBCR_OE |
> +			      TVP5150_MISC_CTL_CLOCK_OE |
> +			      TVP5150_MISC_CTL_SYNC_OE;
> +		break;
> +	case V4L2_MBUS_BT656:
> +		decoder->oe = TVP5150_MISC_CTL_YCBCR_OE |
> +			      TVP5150_MISC_CTL_CLOCK_OE;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	return 0;
>  };
> @@ -1190,14 +1213,8 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
>  	if (enable) {
>  		tvp5150_enable(sd);
>  
> -		/*
> -		 * Enable the YCbCr and clock outputs. In discrete sync mode
> -		 * (non-BT.656) additionally enable the the sync outputs.
> -		 */
> -		val = TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_CLOCK_OE;
> -		if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> -			val |= TVP5150_MISC_CTL_SYNC_OE;
> -
> +		/* Enable outputs if decoder is locked */
> +		val = decoder->lock ? decoder->oe : 0;

Hmm... this only works if the tvp5150 has the interrupts enabled.

The code should be, instead:

	if (c->irq)
		val = decoder->lock ? decoder->oe : 0;
	else
		val = decoder->oe;


>  		int_val = TVP5150_INT_A_LOCK;
>  	}
>  



Thanks,
Mauro