[PATCH 1/2] media: tvp5150 Fix default input selection.

[PATCH 1/2] media: tvp5150 Fix default input selection.

[Javier Martin]

In page 23 of the datasheet of this chip (SLES098A)
it is stated that de default input for this chip
is Composite AIP1A which is the same as COMPOSITE0
in the driver.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/media/video/tvp5150.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
index e927d25..26cc75b 100644
--- a/drivers/media/video/tvp5150.c
+++ b/drivers/media/video/tvp5150.c
@@ -993,7 +993,7 @@ static int tvp5150_probe(struct i2c_client *c,
 	}
 
 	core->norm = V4L2_STD_ALL;	/* Default is autodetect */
-	core->input = TVP5150_COMPOSITE1;
+	core->input = TVP5150_COMPOSITE0;
 	core->enable = 1;
 
 	v4l2_ctrl_handler_init(&core->hdl, 4);
-- 
1.7.0.4

[PATCH 2/2] media: tvp5150: Add mbus_fmt callbacks.

[Javier Martin]

These callbacks allow a host video driver
to poll video supported video formats of tvp5150.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/media/video/tvp5150.c |   72 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
index 26cc75b..8f01f08 100644
--- a/drivers/media/video/tvp5150.c
+++ b/drivers/media/video/tvp5150.c
@@ -778,6 +778,75 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
 	return -EINVAL;
 }
 
+static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
+{
+	int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
+
+	switch (val & 0x0F) {
+	case 0x01:
+		return V4L2_STD_NTSC;
+	case 0x03:
+		return V4L2_STD_PAL;
+	case 0x05:
+		return V4L2_STD_PAL_M;
+	case 0x07:
+		return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
+	case 0x09:
+		return V4L2_STD_NTSC_443;
+	case 0xb:
+		return V4L2_STD_SECAM;
+	default:
+		return V4L2_STD_UNKNOWN;
+	}
+}
+
+static int tvp5150_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned index,
+						enum v4l2_mbus_pixelcode *code)
+{
+	if (index)
+		return -EINVAL;
+
+	*code = V4L2_MBUS_FMT_YUYV8_2X8;
+	return 0;
+}
+
+static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
+			    struct v4l2_mbus_framefmt *f)
+{
+	struct tvp5150 *decoder = to_tvp5150(sd);
+	v4l2_std_id std;
+
+	if (f == NULL)
+		return -EINVAL;
+
+	tvp5150_reset(sd, 0);
+
+	/* Calculate height and width based on current standard */
+	if (decoder->norm == V4L2_STD_ALL)
+		std = tvp5150_read_std(sd);
+	else
+		std = decoder->norm;
+
+	if ((std == V4L2_STD_NTSC) || (std == V4L2_STD_NTSC_443) ||
+		(std == V4L2_STD_PAL_M)) {
+		f->width = 720;
+		f->height = 480;
+	}
+	if ((std == V4L2_STD_PAL) ||
+		(std == (V4L2_STD_PAL_N | V4L2_STD_PAL_Nc)) ||
+		(std == V4L2_STD_SECAM)) {
+		f->width = 720;
+		f->height = 576;
+	}
+	f->code = V4L2_MBUS_FMT_YUYV8_2X8;
+	f->field = V4L2_FIELD_SEQ_TB;
+	f->colorspace = V4L2_COLORSPACE_SMPTE170M;
+
+	v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
+			f->height);
+	return 0;
+}
+
 /****************************************************************************
 			I2C Command
  ****************************************************************************/
@@ -930,6 +999,9 @@ static const struct v4l2_subdev_tuner_ops tvp5150_tuner_ops = {
 
 static const struct v4l2_subdev_video_ops tvp5150_video_ops = {
 	.s_routing = tvp5150_s_routing,
+	.enum_mbus_fmt = tvp5150_enum_mbus_fmt,
+	.s_mbus_fmt = tvp5150_mbus_fmt,
+	.try_mbus_fmt = tvp5150_mbus_fmt,
 };
 
 static const struct v4l2_subdev_vbi_ops tvp5150_vbi_ops = {
-- 
1.7.0.4

Re: [PATCH 2/2] media: tvp5150: Add mbus_fmt callbacks.

[Mauro Carvalho Chehab]

On 15-12-2011 07:39, Javier Martin wrote:
> These callbacks allow a host video driver
> to poll video supported video formats of tvp5150.
> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/video/tvp5150.c |   72 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 72 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
> index 26cc75b..8f01f08 100644
> --- a/drivers/media/video/tvp5150.c
> +++ b/drivers/media/video/tvp5150.c
> @@ -778,6 +778,75 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
>  	return -EINVAL;
>  }
>  
> +static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
> +{
> +	int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
> +
> +	switch (val & 0x0F) {
> +	case 0x01:
> +		return V4L2_STD_NTSC;
> +	case 0x03:
> +		return V4L2_STD_PAL;
> +	case 0x05:
> +		return V4L2_STD_PAL_M;
> +	case 0x07:
> +		return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
> +	case 0x09:
> +		return V4L2_STD_NTSC_443;
> +	case 0xb:
> +		return V4L2_STD_SECAM;
> +	default:
> +		return V4L2_STD_UNKNOWN;
> +	}
> +}
> +
> +static int tvp5150_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned index,
> +						enum v4l2_mbus_pixelcode *code)
> +{
> +	if (index)
> +		return -EINVAL;
> +
> +	*code = V4L2_MBUS_FMT_YUYV8_2X8;
> +	return 0;
> +}
> +
> +static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
> +			    struct v4l2_mbus_framefmt *f)
> +{
> +	struct tvp5150 *decoder = to_tvp5150(sd);
> +	v4l2_std_id std;
> +
> +	if (f == NULL)
> +		return -EINVAL;
> +
> +	tvp5150_reset(sd, 0);
> +
> +	/* Calculate height and width based on current standard */
> +	if (decoder->norm == V4L2_STD_ALL)
> +		std = tvp5150_read_std(sd);
> +	else
> +		std = decoder->norm;
> +
> +	if ((std == V4L2_STD_NTSC) || (std == V4L2_STD_NTSC_443) ||
> +		(std == V4L2_STD_PAL_M)) {
> +		f->width = 720;
> +		f->height = 480;
> +	}
> +	if ((std == V4L2_STD_PAL) ||
> +		(std == (V4L2_STD_PAL_N | V4L2_STD_PAL_Nc)) ||
> +		(std == V4L2_STD_SECAM)) {
> +		f->width = 720;
> +		f->height = 576;
> +	}

The above is wrong, as std is a bitmask. So, userspace can pass more than
one bit set there. It should be, instead:

	f->width = 720;
	if (std & V4L2_STD_525_60)
		f->height = 480;
	else
		f->height = 576;

> +	f->code = V4L2_MBUS_FMT_YUYV8_2X8;
> +	f->field = V4L2_FIELD_SEQ_TB;
> +	f->colorspace = V4L2_COLORSPACE_SMPTE170M;
> +
> +	v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
> +			f->height);
> +	return 0;
> +}
> +
>  /****************************************************************************
>  			I2C Command
>   ****************************************************************************/
> @@ -930,6 +999,9 @@ static const struct v4l2_subdev_tuner_ops tvp5150_tuner_ops = {
>  
>  static const struct v4l2_subdev_video_ops tvp5150_video_ops = {
>  	.s_routing = tvp5150_s_routing,
> +	.enum_mbus_fmt = tvp5150_enum_mbus_fmt,
> +	.s_mbus_fmt = tvp5150_mbus_fmt,
> +	.try_mbus_fmt = tvp5150_mbus_fmt,
>  };
>  
>  static const struct v4l2_subdev_vbi_ops tvp5150_vbi_ops = {

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[Mauro Carvalho Chehab]

On 15-12-2011 07:39, Javier Martin wrote:
> In page 23 of the datasheet of this chip (SLES098A)
> it is stated that de default input for this chip
> is Composite AIP1A which is the same as COMPOSITE0
> in the driver.
> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/video/tvp5150.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
> index e927d25..26cc75b 100644
> --- a/drivers/media/video/tvp5150.c
> +++ b/drivers/media/video/tvp5150.c
> @@ -993,7 +993,7 @@ static int tvp5150_probe(struct i2c_client *c,
>  	}
>  
>  	core->norm = V4L2_STD_ALL;	/* Default is autodetect */
> -	core->input = TVP5150_COMPOSITE1;
> +	core->input = TVP5150_COMPOSITE0;
>  	core->enable = 1;
>  
>  	v4l2_ctrl_handler_init(&core->hdl, 4);

Changing this could break em28xx that might be expecting it
to be set to composite1. On a quick look, the code there seems to be
doing the right thing: during the probe procedure, it explicitly 
calls s_routing, in order to initialize the device input to the
first input type found at the cards structure. So, this patch
is likely harmless.

Yet, why do you need to change it? Any bridge driver that uses it should
be doing the same: at initialization, it should set the input to a
value that it is compatible with the way the device is wired, and not
to assume a particular arrangement.

Regards,
Mauro

Re: [PATCH 2/2] media: tvp5150: Add mbus_fmt callbacks.

[javier Martin]

Hi Mauro,
thank you for your review, I will prepare a new version with those fixes.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[javier Martin]

> Changing this could break em28xx that might be expecting it
> to be set to composite1. On a quick look, the code there seems to be
> doing the right thing: during the probe procedure, it explicitly
> calls s_routing, in order to initialize the device input to the
> first input type found at the cards structure. So, this patch
> is likely harmless.
>
> Yet, why do you need to change it? Any bridge driver that uses it should
> be doing the same: at initialization, it should set the input to a
> value that it is compatible with the way the device is wired, and not
> to assume a particular arrangement.

What I'm trying to do with these patches and my previous one related
to mx2_camera,
is to be able to use mx2_camera host driver with tvp5150 video decoder.

I'm not sure how mx2_camera could be aware that the sensor or decoder
attached to it
needs s_routing function to be called with a certain parameter without
making it too board specific.
The only solution I could think of was assuming that if s_routing
function was not called at all,
the enabled input in tvp5150 would be the default COMPOSITE0 as it is
specified in the datasheet.

However, If you or anyone suggests a cleaner approach I'm totally
open. But still, changing default
value of the selected input in tvp5150 probe function is a bit dirty IMHO.

Thank you.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[Mauro Carvalho Chehab]

On 15-12-2011 08:24, javier Martin wrote:
>> Changing this could break em28xx that might be expecting it
>> to be set to composite1. On a quick look, the code there seems to be
>> doing the right thing: during the probe procedure, it explicitly
>> calls s_routing, in order to initialize the device input to the
>> first input type found at the cards structure. So, this patch
>> is likely harmless.
>>
>> Yet, why do you need to change it? Any bridge driver that uses it should
>> be doing the same: at initialization, it should set the input to a
>> value that it is compatible with the way the device is wired, and not
>> to assume a particular arrangement.
> 
> What I'm trying to do with these patches and my previous one related
> to mx2_camera,
> is to be able to use mx2_camera host driver with tvp5150 video decoder.
> 
> I'm not sure how mx2_camera could be aware that the sensor or decoder
> attached to it
> needs s_routing function to be called with a certain parameter without
> making it too board specific.
> The only solution I could think of was assuming that if s_routing
> function was not called at all,
> the enabled input in tvp5150 would be the default COMPOSITE0 as it is
> specified in the datasheet.
> 
> However, If you or anyone suggests a cleaner approach I'm totally
> open. But still, changing default
> value of the selected input in tvp5150 probe function is a bit dirty IMHO.

Board-specific information is needed anyway, if someone wants to use any
driver. It doesn't matter if the pipelines are set via libv4l, via direct
calls or whatever.

On em28xx, the board-specific info is stored in Kernel. It would be possible
to put that info on userspace, but that would require some code at libv4l.

The mx2_camera needs some code to forward calls to S_INPUT/S_ROUTING to
tvp5150, in order to set the pipelines there.

The libv4l plugin also needs to know about that.

Regards,
Mauro

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[javier Martin]

> The mx2_camera needs some code to forward calls to S_INPUT/S_ROUTING to
> tvp5150, in order to set the pipelines there.

This sounds like a sensible solution I will work on that soon.

Regards.
-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[javier Martin]

On 15 December 2011 13:01, javier Martin
<javier.martin@vista-silicon.com> wrote:
>> The mx2_camera needs some code to forward calls to S_INPUT/S_ROUTING to
>> tvp5150, in order to set the pipelines there.
>
> This sounds like a sensible solution I will work on that soon.
>

Hi Mauro,
regarding this subject it seems soc-camera assumes the attached sensor
has only one input: input 0. This means I am not able to forward
S_INPUT/S_ROUTING as you suggested:
http://lxr.linux.no/#linux+v3.1.5/drivers/media/video/soc_camera.c#L213

This trick is clearly a loss of functionality because it restricts
sensors to output 0, but it should work since the subsystem can assume
a sensor whose inputs have not been configured has input 0 as the one
selected.

However, this trick in the tvp5150 which selects input 1 (instead of
0) as the default input is breaking that assumption. The solution
could be either apply my patch to set input 0 (COMPOSITE0) as default
or swap input numbers so that COMPOSITE1 input is input 0.

Personally I find my approach more convenient since it matches with
the default behavior expected in the datasheet.

Regards.
-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH 1/2] media: tvp5150 Fix default input selection.

[Mauro Carvalho Chehab]

On 15-12-2011 10:33, javier Martin wrote:
> On 15 December 2011 13:01, javier Martin
> <javier.martin@vista-silicon.com> wrote:
>>> The mx2_camera needs some code to forward calls to S_INPUT/S_ROUTING to
>>> tvp5150, in order to set the pipelines there.
>>
>> This sounds like a sensible solution I will work on that soon.
>>
> 
> Hi Mauro,
> regarding this subject it seems soc-camera assumes the attached sensor
> has only one input: input 0. This means I am not able to forward
> S_INPUT/S_ROUTING as you suggested:
> http://lxr.linux.no/#linux+v3.1.5/drivers/media/video/soc_camera.c#L213

Then, you need to submit a patch for soc_camera, in order to allow it
to work with devices that provide more than one input.

> This trick is clearly a loss of functionality because it restricts
> sensors to output 0, but it should work since the subsystem can assume
> a sensor whose inputs have not been configured has input 0 as the one
> selected.
> 
> However, this trick in the tvp5150 which selects input 1 (instead of
> 0) as the default input is breaking that assumption. The solution
> could be either apply my patch to set input 0 (COMPOSITE0) as default
> or swap input numbers so that COMPOSITE1 input is input 0.
> 
> Personally I find my approach more convenient since it matches with
> the default behavior expected in the datasheet.

Both of your described ways are just hacks. tvp5150 has more than one
input. So, the bridge should be supporting the selection between
them.

Regards,
Mauro
> 
> Regards.