Hi Mauro,
I've just sent a series of patches ("[PATCH 0/6] Fix tvp5150 regression with
em28xx") that should fix this problem properly. I unfortunately haven't been
able to test it with an em28xx device as I don't own any.
On Thursday 08 Dec 2016 17:46:53 Mauro Carvalho Chehab wrote:
> commit 460b6c0831cb ("[media] tvp5150: Add s_stream subdev operation
> support") added a logic that overrides TVP5150_CONF_SHARED_PIN setting,
> depending on the type of bus set via the .set_fmt() subdev callback.
>
> This is known to cause trobules on devices that don't use a V4L2
> subdev devnode, and a fix for it was made by commit 47de9bf8931e
> ("[media] tvp5150: Fix breakage for serial usage"). Unfortunately,
> such fix doesn't consider the case of progressive video inputs,
> causing chroma decoding issues on such videos, as it overrides not
> only the type of video output, but also other unrelated bits.
>
> So, instead of trying to guess, let's detect if the device configuration
> is set via Device Tree. If not, just ignore the new logic, restoring
> the original behavior.
>
> Fixes: 460b6c0831cb ("[media] tvp5150: Add s_stream subdev operation
> support") Cc: Devin Heitmueller <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: [email protected]
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
>
> changes since version 2:
> - fixed settings for register 0x0d
> - tested on WinTV USB2 with S-Video input
>
> I'll do an extra test with HVR-950 on both S-Video and composite soon enough
>
> changes since version 1: added a notice about what's broken at the
> tvp5150_stream() logic, and improved patch's description.
>
> changes since RFC: don't touch at enum v4l2_mbus_type.
>
> drivers/media/i2c/tvp5150.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 6737685d5be5..8b9d2fad17df 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -57,6 +57,7 @@ struct tvp5150 {
> u16 rom_ver;
>
> enum v4l2_mbus_type mbus_type;
> + bool has_dt;
> };
>
> static inline struct tvp5150 *to_tvp5150(struct v4l2_subdev *sd)
> @@ -1053,6 +1054,20 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd,
> int enable) /* Output format: 8-bit ITU-R BT.656 with embedded syncs */
> int val = 0x09;
>
> + if (!decoder->has_dt)
> + return 0;
> +
> + /*
> + * FIXME: the logic below is hardcoded to work with some OMAP3
> + * hardware with tvp5151. As such, it hardcodes values for
> + * both TVP5150_CONF_SHARED_PIN and TVP5150_MISC_CTL, and ignores
> + * what was set before at the driver. Ideally, we should have
> + * DT nodes describing the setup, instead of hardcoding those
> + * values, and doing a read before writing values to
> + * TVP5150_MISC_CTL, but any patch adding support for it should
> + * keep DT backward-compatible.
> + */
> +
> /* Output format: 8-bit 4:2:2 YUV with discrete sync */
> if (decoder->mbus_type == V4L2_MBUS_PARALLEL)
> val = 0x0d;
> @@ -1471,6 +1486,7 @@ static int tvp5150_probe(struct i2c_client *c,
> dev_err(sd->dev, "DT parsing error: %d\n", res);
> return res;
> }
> + decoder->has_dt = true;
> } else {
> /* Default to BT.656 embedded sync */
> core->mbus_type = V4L2_MBUS_BT656;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
