[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Deepthy Ravi]

From: Vaibhav Hiremath <hvaibhav@ti.com>

In order to support TVP5146 (for that matter any video decoder),
it is important to support G/S/ENUM_STD ioctl on /dev/videoX
device node.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Deepthy Ravi <deepthy.ravi@ti.com>
---
 drivers/media/video/omap3isp/ispvideo.c |   98 ++++++++++++++++++++++++++++++-
 drivers/media/video/omap3isp/ispvideo.h |    1 +
 2 files changed, 98 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index d5b8236..ff0ffed 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -37,6 +37,7 @@
 #include <plat/iovmm.h>
 #include <plat/omap-pm.h>
 
+#include <media/tvp514x.h>
 #include "ispvideo.h"
 #include "isp.h"
 
@@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh, unsigned int *input)
 static int
 isp_video_s_input(struct file *file, void *fh, unsigned int input)
 {
-	return input == 0 ? 0 : -EINVAL;
+	struct isp_video *video = video_drvdata(file);
+	struct media_entity *entity = &video->video.entity;
+	struct media_entity_graph graph;
+	struct v4l2_subdev *subdev;
+	struct v4l2_routing route;
+	int ret = 0;
+
+	media_entity_graph_walk_start(&graph, entity);
+	while ((entity = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity) ==
+				MEDIA_ENT_T_V4L2_SUBDEV) {
+			subdev = media_entity_to_v4l2_subdev(entity);
+			if (subdev != NULL) {
+				if (input == 0)
+					route.input = INPUT_CVBS_VI4A;
+				else
+					route.input = INPUT_SVIDEO_VI2C_VI1C;
+				route.output = 0;
+				ret = v4l2_subdev_call(subdev, video, s_routing,
+						route.input, route.output, 0);
+				if (ret < 0 && ret != -ENOIOCTLCMD)
+					return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
+{
+	struct isp_video_fh *vfh = to_isp_video_fh(fh);
+	struct isp_video *video = video_drvdata(file);
+	struct media_entity *entity = &video->video.entity;
+	struct media_entity_graph graph;
+	struct v4l2_subdev *subdev;
+	int ret = 0;
+
+	media_entity_graph_walk_start(&graph, entity);
+	while ((entity = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity) ==
+				MEDIA_ENT_T_V4L2_SUBDEV) {
+			subdev = media_entity_to_v4l2_subdev(entity);
+			if (subdev != NULL) {
+				ret = v4l2_subdev_call(subdev, video, querystd,
+						a);
+				if (ret < 0 && ret != -ENOIOCTLCMD)
+					return ret;
+			}
+		}
+	}
+
+	vfh->standard.id = *a;
+	return 0;
+}
+
+static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+	struct isp_video_fh *vfh = to_isp_video_fh(fh);
+	struct isp_video *video = video_drvdata(file);
+
+	mutex_lock(&video->mutex);
+	*norm = vfh->standard.id;
+	mutex_unlock(&video->mutex);
+
+	return 0;
+}
+
+static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+	struct isp_video *video = video_drvdata(file);
+	struct media_entity *entity = &video->video.entity;
+	struct media_entity_graph graph;
+	struct v4l2_subdev *subdev;
+	int ret = 0;
+
+	media_entity_graph_walk_start(&graph, entity);
+	while ((entity = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity) ==
+				MEDIA_ENT_T_V4L2_SUBDEV) {
+			subdev = media_entity_to_v4l2_subdev(entity);
+			if (subdev != NULL) {
+				ret = v4l2_subdev_call(subdev, core, s_std,
+						*norm);
+				if (ret < 0 && ret != -ENOIOCTLCMD)
+					return ret;
+			}
+		}
+	}
+
+	return 0;
 }
 
 static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
@@ -1161,6 +1252,9 @@ static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
 	.vidioc_enum_input		= isp_video_enum_input,
 	.vidioc_g_input			= isp_video_g_input,
 	.vidioc_s_input			= isp_video_s_input,
+	.vidioc_querystd		= isp_video_querystd,
+	.vidioc_g_std			= isp_video_g_std,
+	.vidioc_s_std			= isp_video_s_std,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1325,6 +1419,8 @@ int omap3isp_video_register(struct isp_video *video, struct v4l2_device *vdev)
 		printk(KERN_ERR "%s: could not register video device (%d)\n",
 			__func__, ret);
 
+	video->video.tvnorms		= V4L2_STD_NTSC | V4L2_STD_PAL;
+	video->video.current_norm	= V4L2_STD_NTSC;
 	return ret;
 }
 
diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
index 53160aa..bb8feb6 100644
--- a/drivers/media/video/omap3isp/ispvideo.h
+++ b/drivers/media/video/omap3isp/ispvideo.h
@@ -182,6 +182,7 @@ struct isp_video_fh {
 	struct isp_video *video;
 	struct isp_video_queue queue;
 	struct v4l2_format format;
+	struct v4l2_standard standard;
 	struct v4l2_fract timeperframe;
 };
 
-- 
1.7.0.4

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Laurent Pinchart]

Hi,

Thanks for the patch.

On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
> From: Vaibhav Hiremath <hvaibhav@ti.com>
> 
> In order to support TVP5146 (for that matter any video decoder),
> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> device node.

Why so ? Shouldn't it be queried on the subdev output pad directly ?

> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Signed-off-by: Deepthy Ravi <deepthy.ravi@ti.com>
> ---
>  drivers/media/video/omap3isp/ispvideo.c |   98
> ++++++++++++++++++++++++++++++- drivers/media/video/omap3isp/ispvideo.h | 
>   1 +
>  2 files changed, 98 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/omap3isp/ispvideo.c
> b/drivers/media/video/omap3isp/ispvideo.c index d5b8236..ff0ffed 100644
> --- a/drivers/media/video/omap3isp/ispvideo.c
> +++ b/drivers/media/video/omap3isp/ispvideo.c
> @@ -37,6 +37,7 @@
>  #include <plat/iovmm.h>
>  #include <plat/omap-pm.h>
> 
> +#include <media/tvp514x.h>
>  #include "ispvideo.h"
>  #include "isp.h"
> 
> @@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh,
> unsigned int *input) static int
>  isp_video_s_input(struct file *file, void *fh, unsigned int input)
>  {
> -	return input == 0 ? 0 : -EINVAL;
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	struct v4l2_routing route;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				if (input == 0)
> +					route.input = INPUT_CVBS_VI4A;
> +				else
> +					route.input = INPUT_SVIDEO_VI2C_VI1C;
> +				route.output = 0;
> +				ret = v4l2_subdev_call(subdev, video, s_routing,
> +						route.input, route.output, 0);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
> +{
> +	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				ret = v4l2_subdev_call(subdev, video, querystd,
> +						a);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	vfh->standard.id = *a;
> +	return 0;
> +}
> +
> +static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
> +{
> +	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> +	struct isp_video *video = video_drvdata(file);
> +
> +	mutex_lock(&video->mutex);
> +	*norm = vfh->standard.id;
> +	mutex_unlock(&video->mutex);
> +
> +	return 0;
> +}
> +
> +static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
> +{
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				ret = v4l2_subdev_call(subdev, core, s_std,
> +						*norm);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
>  }
> 
>  static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
> @@ -1161,6 +1252,9 @@ static const struct v4l2_ioctl_ops
> isp_video_ioctl_ops = { .vidioc_enum_input		= isp_video_enum_input,
>  	.vidioc_g_input			= isp_video_g_input,
>  	.vidioc_s_input			= isp_video_s_input,
> +	.vidioc_querystd		= isp_video_querystd,
> +	.vidioc_g_std			= isp_video_g_std,
> +	.vidioc_s_std			= isp_video_s_std,
>  };
> 
>  /*
> --------------------------------------------------------------------------
> --- @@ -1325,6 +1419,8 @@ int omap3isp_video_register(struct isp_video
> *video, struct v4l2_device *vdev) printk(KERN_ERR "%s: could not register
> video device (%d)\n",
>  			__func__, ret);
> 
> +	video->video.tvnorms		= V4L2_STD_NTSC | V4L2_STD_PAL;
> +	video->video.current_norm	= V4L2_STD_NTSC;
>  	return ret;
>  }
> 
> diff --git a/drivers/media/video/omap3isp/ispvideo.h
> b/drivers/media/video/omap3isp/ispvideo.h index 53160aa..bb8feb6 100644
> --- a/drivers/media/video/omap3isp/ispvideo.h
> +++ b/drivers/media/video/omap3isp/ispvideo.h
> @@ -182,6 +182,7 @@ struct isp_video_fh {
>  	struct isp_video *video;
>  	struct isp_video_queue queue;
>  	struct v4l2_format format;
> +	struct v4l2_standard standard;
>  	struct v4l2_fract timeperframe;
>  };

-- 
Regards,

Laurent Pinchart

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Sakari Ailus]

Hi Deepathy,

Thanks for the patches.

On Thu, Sep 08, 2011 at 07:05:22PM +0530, Deepthy Ravi wrote:
> From: Vaibhav Hiremath <hvaibhav@ti.com>
> 
> In order to support TVP5146 (for that matter any video decoder),
> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> device node.

Why on video nodes rather than the subdev node?

I don't think *_STD ioctls should be handled differently from any others,
i.e. this is directly related to that subdev, so the control should go
through the subdev node.

That said, generic applications aren't necessarily aware of the subdev nodes
and I think this is something that should be handled in a libv4l plugin.
This appears quite generic to me; walking the graph and accessing the right
subdev node can be done in user space.

> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Signed-off-by: Deepthy Ravi <deepthy.ravi@ti.com>
> ---
>  drivers/media/video/omap3isp/ispvideo.c |   98 ++++++++++++++++++++++++++++++-
>  drivers/media/video/omap3isp/ispvideo.h |    1 +
>  2 files changed, 98 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
> index d5b8236..ff0ffed 100644
> --- a/drivers/media/video/omap3isp/ispvideo.c
> +++ b/drivers/media/video/omap3isp/ispvideo.c
> @@ -37,6 +37,7 @@
>  #include <plat/iovmm.h>
>  #include <plat/omap-pm.h>
>  
> +#include <media/tvp514x.h>
>  #include "ispvideo.h"
>  #include "isp.h"
>  
> @@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh, unsigned int *input)
>  static int
>  isp_video_s_input(struct file *file, void *fh, unsigned int input)
>  {
> -	return input == 0 ? 0 : -EINVAL;
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	struct v4l2_routing route;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				if (input == 0)
> +					route.input = INPUT_CVBS_VI4A;
> +				else
> +					route.input = INPUT_SVIDEO_VI2C_VI1C;
> +				route.output = 0;
> +				ret = v4l2_subdev_call(subdev, video, s_routing,
> +						route.input, route.output, 0);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
> +{
> +	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				ret = v4l2_subdev_call(subdev, video, querystd,
> +						a);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	vfh->standard.id = *a;
> +	return 0;
> +}
> +
> +static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
> +{
> +	struct isp_video_fh *vfh = to_isp_video_fh(fh);
> +	struct isp_video *video = video_drvdata(file);
> +
> +	mutex_lock(&video->mutex);
> +	*norm = vfh->standard.id;
> +	mutex_unlock(&video->mutex);
> +
> +	return 0;
> +}
> +
> +static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
> +{
> +	struct isp_video *video = video_drvdata(file);
> +	struct media_entity *entity = &video->video.entity;
> +	struct media_entity_graph graph;
> +	struct v4l2_subdev *subdev;
> +	int ret = 0;
> +
> +	media_entity_graph_walk_start(&graph, entity);
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (media_entity_type(entity) ==
> +				MEDIA_ENT_T_V4L2_SUBDEV) {
> +			subdev = media_entity_to_v4l2_subdev(entity);
> +			if (subdev != NULL) {
> +				ret = v4l2_subdev_call(subdev, core, s_std,
> +						*norm);
> +				if (ret < 0 && ret != -ENOIOCTLCMD)
> +					return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
>  }
>  
>  static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
> @@ -1161,6 +1252,9 @@ static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
>  	.vidioc_enum_input		= isp_video_enum_input,
>  	.vidioc_g_input			= isp_video_g_input,
>  	.vidioc_s_input			= isp_video_s_input,
> +	.vidioc_querystd		= isp_video_querystd,
> +	.vidioc_g_std			= isp_video_g_std,
> +	.vidioc_s_std			= isp_video_s_std,
>  };
>  
>  /* -----------------------------------------------------------------------------
> @@ -1325,6 +1419,8 @@ int omap3isp_video_register(struct isp_video *video, struct v4l2_device *vdev)
>  		printk(KERN_ERR "%s: could not register video device (%d)\n",
>  			__func__, ret);
>  
> +	video->video.tvnorms		= V4L2_STD_NTSC | V4L2_STD_PAL;
> +	video->video.current_norm	= V4L2_STD_NTSC;
>  	return ret;
>  }
>  
> diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
> index 53160aa..bb8feb6 100644
> --- a/drivers/media/video/omap3isp/ispvideo.h
> +++ b/drivers/media/video/omap3isp/ispvideo.h
> @@ -182,6 +182,7 @@ struct isp_video_fh {
>  	struct isp_video *video;
>  	struct isp_video_queue queue;
>  	struct v4l2_format format;
> +	struct v4l2_standard standard;
>  	struct v4l2_fract timeperframe;
>  };
>  
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sakari Ailus
e-mail: sakari.ailus at iki.fi	jabber/XMPP/Gmail: sailus at retiisi.org.uk

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Ravi, Deepthy]

Hi,
Sorry for the delayed response.
> ________________________________________
> From: Laurent Pinchart [laurent.pinchart at ideasonboard.com]
> Sent: Thursday, September 08, 2011 10:51 PM
> To: Ravi, Deepthy
> Cc: linux-media at vger.kernel.org; tony at atomide.com; linux at arm.linux.org.uk; linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; mchehab at infradead.org; g.liakhovetski at gmx.de; Hiremath, Vaibhav
> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
>
> Hi,
>
> Thanks for the patch.
>
> On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
>> From: Vaibhav Hiremath <hvaibhav@ti.com>
>>
>> In order to support TVP5146 (for that matter any video decoder),
>> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
>> device node.
>
> Why so ? Shouldn't it be queried on the subdev output pad directly ?
>
[Deepthy Ravi] Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work.

>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>> Signed-off-by: Deepthy Ravi <deepthy.ravi@ti.com>
>> ---
>>  drivers/media/video/omap3isp/ispvideo.c |   98
>> ++++++++++++++++++++++++++++++- drivers/media/video/omap3isp/ispvideo.h |
>>   1 +
>>  2 files changed, 98 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/omap3isp/ispvideo.c
>> b/drivers/media/video/omap3isp/ispvideo.c index d5b8236..ff0ffed 100644
>> --- a/drivers/media/video/omap3isp/ispvideo.c
>> +++ b/drivers/media/video/omap3isp/ispvideo.c
>> @@ -37,6 +37,7 @@
>>  #include <plat/iovmm.h>
>>  #include <plat/omap-pm.h>
>>
>> +#include <media/tvp514x.h>
>>  #include "ispvideo.h"
>>  #include "isp.h"
>>
>> @@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh,
>> unsigned int *input) static int
>>  isp_video_s_input(struct file *file, void *fh, unsigned int input)
>>  {
>> -     return input == 0 ? 0 : -EINVAL;
>> +     struct isp_video *video = video_drvdata(file);
>> +     struct media_entity *entity = &video->video.entity;
>> +     struct media_entity_graph graph;
>> +     struct v4l2_subdev *subdev;
>> +     struct v4l2_routing route;
>> +     int ret = 0;
>> +
>> +     media_entity_graph_walk_start(&graph, entity);
>> +     while ((entity = media_entity_graph_walk_next(&graph))) {
>> +             if (media_entity_type(entity) ==
>> +                             MEDIA_ENT_T_V4L2_SUBDEV) {
>> +                     subdev = media_entity_to_v4l2_subdev(entity);
>> +                     if (subdev != NULL) {
>> +                             if (input == 0)
>> +                                     route.input = INPUT_CVBS_VI4A;
>> +                             else
>> +                                     route.input = INPUT_SVIDEO_VI2C_VI1C;
>> +                             route.output = 0;
>> +                             ret = v4l2_subdev_call(subdev, video, s_routing,
>> +                                             route.input, route.output, 0);
>> +                             if (ret < 0 && ret != -ENOIOCTLCMD)
>> +                                     return ret;
>> +                     }
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
>> +{
>> +     struct isp_video_fh *vfh = to_isp_video_fh(fh);
>> +     struct isp_video *video = video_drvdata(file);
>> +     struct media_entity *entity = &video->video.entity;
>> +     struct media_entity_graph graph;
>> +     struct v4l2_subdev *subdev;
>> +     int ret = 0;
>> +
>> +     media_entity_graph_walk_start(&graph, entity);
>> +     while ((entity = media_entity_graph_walk_next(&graph))) {
>> +             if (media_entity_type(entity) ==
>> +                             MEDIA_ENT_T_V4L2_SUBDEV) {
>> +                     subdev = media_entity_to_v4l2_subdev(entity);
>> +                     if (subdev != NULL) {
>> +                             ret = v4l2_subdev_call(subdev, video, querystd,
>> +                                             a);
>> +                             if (ret < 0 && ret != -ENOIOCTLCMD)
>> +                                     return ret;
>> +                     }
>> +             }
>> +     }
>> +
>> +     vfh->standard.id = *a;
>> +     return 0;
>> +}
>> +
>> +static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
>> +{
>> +     struct isp_video_fh *vfh = to_isp_video_fh(fh);
>> +     struct isp_video *video = video_drvdata(file);
>> +
>> +     mutex_lock(&video->mutex);
>> +     *norm = vfh->standard.id;
>> +     mutex_unlock(&video->mutex);
>> +
>> +     return 0;
>> +}
>> +
>> +static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
>> +{
>> +     struct isp_video *video = video_drvdata(file);
>> +     struct media_entity *entity = &video->video.entity;
>> +     struct media_entity_graph graph;
>> +     struct v4l2_subdev *subdev;
>> +     int ret = 0;
>> +
>> +     media_entity_graph_walk_start(&graph, entity);
>> +     while ((entity = media_entity_graph_walk_next(&graph))) {
>> +             if (media_entity_type(entity) ==
>> +                             MEDIA_ENT_T_V4L2_SUBDEV) {
>> +                     subdev = media_entity_to_v4l2_subdev(entity);
>> +                     if (subdev != NULL) {
>> +                             ret = v4l2_subdev_call(subdev, core, s_std,
>> +                                             *norm);
>> +                             if (ret < 0 && ret != -ENOIOCTLCMD)
>> +                                     return ret;
>> +                     }
>> +             }
>> +     }
>> +
>> +     return 0;
>>  }
>>
>>  static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
>> @@ -1161,6 +1252,9 @@ static const struct v4l2_ioctl_ops
>> isp_video_ioctl_ops = { .vidioc_enum_input            = isp_video_enum_input,
>>       .vidioc_g_input                 = isp_video_g_input,
>>       .vidioc_s_input                 = isp_video_s_input,
>> +     .vidioc_querystd                = isp_video_querystd,
>> +     .vidioc_g_std                   = isp_video_g_std,
>> +     .vidioc_s_std                   = isp_video_s_std,
>>  };
>>
>>  /*
>> --------------------------------------------------------------------------
>> --- @@ -1325,6 +1419,8 @@ int omap3isp_video_register(struct isp_video
>> *video, struct v4l2_device *vdev) printk(KERN_ERR "%s: could not register
>> video device (%d)\n",
>>                       __func__, ret);
>>
>> +     video->video.tvnorms            = V4L2_STD_NTSC | V4L2_STD_PAL;
>> +     video->video.current_norm       = V4L2_STD_NTSC;
>>       return ret;
>>  }
>>
>> diff --git a/drivers/media/video/omap3isp/ispvideo.h
>> b/drivers/media/video/omap3isp/ispvideo.h index 53160aa..bb8feb6 100644
>> --- a/drivers/media/video/omap3isp/ispvideo.h
>> +++ b/drivers/media/video/omap3isp/ispvideo.h
>> @@ -182,6 +182,7 @@ struct isp_video_fh {
>>       struct isp_video *video;
>>       struct isp_video_queue queue;
>>       struct v4l2_format format;
>> +     struct v4l2_standard standard;
>>       struct v4l2_fract timeperframe;
>>  };
>
> --
> Regards,
>
> Laurent Pinchart
>

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Laurent Pinchart]

Hi Deepthy,

On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
> On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: 
> > On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
> >> From: Vaibhav Hiremath <hvaibhav@ti.com>
> >> 
> >> In order to support TVP5146 (for that matter any video decoder),
> >> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> >> device node.
> > 
> > Why so ? Shouldn't it be queried on the subdev output pad directly ?
> 
> Because standard v4l2 application for analog devices will call these std
> ioctls on the streaming device node. So it's done on /dev/video to make the
> existing apllication work.

Existing applications can't work with the OMAP3 ISP (and similar complex 
embedded devices) without userspace support anyway, either in the form of a 
GStreamer element or a libv4l plugin. I still believe that analog video 
standard operations should be added to the subdev pad operations and exposed 
through subdev device nodes, exactly as done with formats.

-- 
Regards,

Laurent Pinchart

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart at ideasonboard.com]
> Sent: Friday, September 16, 2011 6:36 PM
> To: Ravi, Deepthy
> Cc: linux-media at vger.kernel.org; tony at atomide.com; linux at arm.linux.org.uk;
> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; mchehab at infradead.org; g.liakhovetski at gmx.de;
> Hiremath, Vaibhav
> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
> 
> Hi Deepthy,
> 
> On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
> > On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
> > > On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
> > >> From: Vaibhav Hiremath <hvaibhav@ti.com>
> > >>
> > >> In order to support TVP5146 (for that matter any video decoder),
> > >> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> > >> device node.
> > >
> > > Why so ? Shouldn't it be queried on the subdev output pad directly ?
> >
> > Because standard v4l2 application for analog devices will call these std
> > ioctls on the streaming device node. So it's done on /dev/video to make
> the
> > existing apllication work.
> 
> Existing applications can't work with the OMAP3 ISP (and similar complex
> embedded devices) without userspace support anyway, either in the form of
> a
> GStreamer element or a libv4l plugin. I still believe that analog video
> standard operations should be added to the subdev pad operations and
> exposed
> through subdev device nodes, exactly as done with formats.
> 
[Hiremath, Vaibhav] Laurent,

I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it?

The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming...

I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node.

Thanks,
Vaibhav

> --
> Regards,
> 
> Laurent Pinchart

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Laurent Pinchart]

Hi Vaibhav,

On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote:
> On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote:
> > On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
> > > On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
> > > > On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
> > > >> From: Vaibhav Hiremath <hvaibhav@ti.com>
> > > >> 
> > > >> In order to support TVP5146 (for that matter any video decoder),
> > > >> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> > > >> device node.
> > > > 
> > > > Why so ? Shouldn't it be queried on the subdev output pad directly ?
> > > 
> > > Because standard v4l2 application for analog devices will call these
> > > std ioctls on the streaming device node. So it's done on /dev/video to
> > > make the existing apllication work.
> > 
> > Existing applications can't work with the OMAP3 ISP (and similar complex
> > embedded devices) without userspace support anyway, either in the form of
> > a GStreamer element or a libv4l plugin. I still believe that analog video
> > standard operations should be added to the subdev pad operations and
> > exposed through subdev device nodes, exactly as done with formats.
> 
> I completely agree with your point that, existing application will not work
> without setting links properly. But I believe the assumption here is,
> media-controller should set the links (along with pad formants) and all
> existing application should work as is. Isn't it?

The media controller is an API used (among other things) to set the links. You 
still need to call it from userspace. That won't happen magically. The 
userspace component that sets up the links and configures the formats, be it a 
GStreamer element, a libv4l plugin, or something else, can as well setup the 
standard on the TVP5146 subdev.

> The way it is being done currently is, set the format at the pad level
> which is same as analog standard resolution and use existing application
> for streaming...

At then end of the OMAP3 ISP pipeline video data has long lost its analog 
roots. I don't think standards make sense there.

> I am ok, if we add s/g/enum_std api support at sub-dev level but this
> should also be supported on streaming device node.

-- 
Regards,

Laurent Pinchart

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Mauro Carvalho Chehab]

Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> 
>> -----Original Message-----
>> From: Laurent Pinchart [mailto:laurent.pinchart at ideasonboard.com]
>> Sent: Friday, September 16, 2011 6:36 PM
>> To: Ravi, Deepthy
>> Cc: linux-media at vger.kernel.org; tony at atomide.com; linux at arm.linux.org.uk;
>> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
>> kernel at vger.kernel.org; mchehab at infradead.org; g.liakhovetski at gmx.de;
>> Hiremath, Vaibhav
>> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
>>
>> Hi Deepthy,
>>
>> On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
>>> On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
>>>> On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
>>>>> From: Vaibhav Hiremath <hvaibhav@ti.com>
>>>>>
>>>>> In order to support TVP5146 (for that matter any video decoder),
>>>>> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
>>>>> device node.
>>>>
>>>> Why so ? Shouldn't it be queried on the subdev output pad directly ?
>>>
>>> Because standard v4l2 application for analog devices will call these std
>>> ioctls on the streaming device node. So it's done on /dev/video to make
>> the
>>> existing apllication work.
>>
>> Existing applications can't work with the OMAP3 ISP (and similar complex
>> embedded devices) without userspace support anyway, either in the form of
>> a
>> GStreamer element or a libv4l plugin. I still believe that analog video
>> standard operations should be added to the subdev pad operations and
>> exposed
>> through subdev device nodes, exactly as done with formats.
>>
> [Hiremath, Vaibhav] Laurent,
> 
> I completely agree with your point that, existing application will not work without setting links properly.
> But I believe the assumption here is, media-controller should set the links (along with pad formants) and 
> all existing application should work as is. Isn't it?

Yes.

> The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming...

Yes.

> I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node.

Agreed. Standards selection should be done at device node, just like any other
device.

Regards,
Mauro

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Gary Thomas]

On 2011-09-27 16:31, Mauro Carvalho Chehab wrote:
> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
>>
>>> -----Original Message-----
>>> From: Laurent Pinchart [mailto:laurent.pinchart at ideasonboard.com]
>>> Sent: Friday, September 16, 2011 6:36 PM
>>> To: Ravi, Deepthy
>>> Cc: linux-media at vger.kernel.org; tony at atomide.com; linux at arm.linux.org.uk;
>>> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
>>> kernel at vger.kernel.org; mchehab at infradead.org; g.liakhovetski at gmx.de;
>>> Hiremath, Vaibhav
>>> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
>>>
>>> Hi Deepthy,
>>>
>>> On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
>>>> On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
>>>>> On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
>>>>>> From: Vaibhav Hiremath<hvaibhav@ti.com>
>>>>>>
>>>>>> In order to support TVP5146 (for that matter any video decoder),
>>>>>> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
>>>>>> device node.
>>>>>
>>>>> Why so ? Shouldn't it be queried on the subdev output pad directly ?
>>>>
>>>> Because standard v4l2 application for analog devices will call these std
>>>> ioctls on the streaming device node. So it's done on /dev/video to make
>>> the
>>>> existing apllication work.
>>>
>>> Existing applications can't work with the OMAP3 ISP (and similar complex
>>> embedded devices) without userspace support anyway, either in the form of
>>> a
>>> GStreamer element or a libv4l plugin. I still believe that analog video
>>> standard operations should be added to the subdev pad operations and
>>> exposed
>>> through subdev device nodes, exactly as done with formats.
>>>
>> [Hiremath, Vaibhav] Laurent,
>>
>> I completely agree with your point that, existing application will not work without setting links properly.
>> But I believe the assumption here is, media-controller should set the links (along with pad formants) and
>> all existing application should work as is. Isn't it?
>
> Yes.
>
>> The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming...
>
> Yes.
>
>> I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node.
>
> Agreed. Standards selection should be done at device node, just like any other
> device.

So how do you handle a part like the TVP5150 that is standard agnostic?
That device can sense the standard from the input signal and sets the
result appropriately.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart at ideasonboard.com]
> Sent: Tuesday, September 27, 2011 11:36 PM
> To: Hiremath, Vaibhav
> Cc: Ravi, Deepthy; linux-media at vger.kernel.org; tony at atomide.com;
> linux at arm.linux.org.uk; linux-omap at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> mchehab at infradead.org; g.liakhovetski at gmx.de
> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
> 
> Hi Vaibhav,
> 
> On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote:
> > On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote:
> > > On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
> > > > On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
> > > > > On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
> > > > >> From: Vaibhav Hiremath <hvaibhav@ti.com>
> > > > >>
> > > > >> In order to support TVP5146 (for that matter any video decoder),
> > > > >> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
> > > > >> device node.
> > > > >
> > > > > Why so ? Shouldn't it be queried on the subdev output pad
> directly ?
> > > >
> > > > Because standard v4l2 application for analog devices will call these
> > > > std ioctls on the streaming device node. So it's done on /dev/video
> to
> > > > make the existing apllication work.
> > >
> > > Existing applications can't work with the OMAP3 ISP (and similar
> complex
> > > embedded devices) without userspace support anyway, either in the form
> of
> > > a GStreamer element or a libv4l plugin. I still believe that analog
> video
> > > standard operations should be added to the subdev pad operations and
> > > exposed through subdev device nodes, exactly as done with formats.
> >
> > I completely agree with your point that, existing application will not
> work
> > without setting links properly. But I believe the assumption here is,
> > media-controller should set the links (along with pad formants) and all
> > existing application should work as is. Isn't it?
> 
> The media controller is an API used (among other things) to set the links.
> You
> still need to call it from userspace. That won't happen magically. The
> userspace component that sets up the links and configures the formats, be
> it a
> GStreamer element, a libv4l plugin, or something else, can as well setup
> the
> standard on the TVP5146 subdev.
> 
Please look at from analog device point of view which is interfaced to ISP.

OMAP3 ISP => TVP5146 (video decoder)

As a user I would want to expect the standard to be supported on streaming device node, since all standard streaming applications (for analog devices/interfaces) does this.

Setting up the links and format is still something got added with MC framework, and I would consider it as a separate plug-in along with existing applications.

Why do I need to write/use two different streaming application one for MC compatible device and another for non-MC?

> > The way it is being done currently is, set the format at the pad level
> > which is same as analog standard resolution and use existing application
> > for streaming...
> 
> At then end of the OMAP3 ISP pipeline video data has long lost its analog
> roots. I don't think standards make sense there.
> 
I don't agree with you here, I think we made it clear when we started with MC development activity, we will not break existing standard applications. Media-controller will play a roll to setup the links, connecting the pads and stuff.


Thanks,
Vaibhav

> > I am ok, if we add s/g/enum_std api support at sub-dev level but this
> > should also be supported on streaming device node.
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Sakari Ailus]

Hi Hiremath,

Hiremath, Vaibhav wrote:
>> -----Original Message----- From: Laurent Pinchart
>> [mailto:laurent.pinchart at ideasonboard.com] Sent: Tuesday, September
>> 27, 2011 11:36 PM To: Hiremath, Vaibhav Cc: Ravi, Deepthy;
>> linux-media at vger.kernel.org; tony at atomide.com; 
>> linux at arm.linux.org.uk; linux-omap at vger.kernel.org; linux-arm- 
>> kernel at lists.infradead.org; linux-kernel at vger.kernel.org; 
>> mchehab at infradead.org; g.liakhovetski at gmx.de Subject: Re: [PATCH
>> 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
>> 
>> Hi Vaibhav,
>> 
>> On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote:
>>> On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote:
>>>> On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
>>>>> On Thursday, September 08, 2011 10:51 PM Laurent Pinchart
>>>>> wrote:
>>>>>> On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
>>>>>>> From: Vaibhav Hiremath <hvaibhav@ti.com>
>>>>>>> 
>>>>>>> In order to support TVP5146 (for that matter any video
>>>>>>> decoder), it is important to support G/S/ENUM_STD ioctl
>>>>>>> on /dev/videoX device node.
>>>>>> 
>>>>>> Why so ? Shouldn't it be queried on the subdev output pad
>> directly ?
>>>>> 
>>>>> Because standard v4l2 application for analog devices will
>>>>> call these std ioctls on the streaming device node. So it's
>>>>> done on /dev/video
>> to
>>>>> make the existing apllication work.
>>>> 
>>>> Existing applications can't work with the OMAP3 ISP (and
>>>> similar
>> complex
>>>> embedded devices) without userspace support anyway, either in
>>>> the form
>> of
>>>> a GStreamer element or a libv4l plugin. I still believe that
>>>> analog
>> video
>>>> standard operations should be added to the subdev pad
>>>> operations and exposed through subdev device nodes, exactly as
>>>> done with formats.
>>> 
>>> I completely agree with your point that, existing application
>>> will not
>> work
>>> without setting links properly. But I believe the assumption here
>>> is, media-controller should set the links (along with pad
>>> formants) and all existing application should work as is. Isn't
>>> it?
>> 
>> The media controller is an API used (among other things) to set the
>> links. You still need to call it from userspace. That won't happen
>> magically. The userspace component that sets up the links and
>> configures the formats, be it a GStreamer element, a libv4l plugin,
>> or something else, can as well setup the standard on the TVP5146
>> subdev.
>> 
> Please look at from analog device point of view which is interfaced
> to ISP.
> 
> OMAP3 ISP => TVP5146 (video decoder)
> 
> As a user I would want to expect the standard to be supported on
> streaming device node, since all standard streaming applications (for
> analog devices/interfaces) does this.
> 
> Setting up the links and format is still something got added with MC
> framework, and I would consider it as a separate plug-in along with
> existing applications.
> 
> Why do I need to write/use two different streaming application one
> for MC compatible device and another for non-MC?

You musn't need to.

This is something that will have to be handled by the libv4l plugin, as
the rest of controlling the device. Controls related ioctls will be
passed from the source to downstream once they apply, and I don't see
why the same shouldn't be done to the {G,S,ENUM}_STD.

Regards,

-- 
Sakari Ailus
sakari.ailus at iki.fi

[PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl

[Mauro Carvalho Chehab]

Em 27-09-2011 19:49, Gary Thomas escreveu:
> On 2011-09-27 16:31, Mauro Carvalho Chehab wrote:
>> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
>>>
>>>> -----Original Message-----
>>>> From: Laurent Pinchart [mailto:laurent.pinchart at ideasonboard.com]
>>>> Sent: Friday, September 16, 2011 6:36 PM
>>>> To: Ravi, Deepthy
>>>> Cc: linux-media at vger.kernel.org; tony at atomide.com; linux at arm.linux.org.uk;
>>>> linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
>>>> kernel at vger.kernel.org; mchehab at infradead.org; g.liakhovetski at gmx.de;
>>>> Hiremath, Vaibhav
>>>> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
>>>>
>>>> Hi Deepthy,
>>>>
>>>> On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
>>>>> On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
>>>>>> On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
>>>>>>> From: Vaibhav Hiremath<hvaibhav@ti.com>
>>>>>>>
>>>>>>> In order to support TVP5146 (for that matter any video decoder),
>>>>>>> it is important to support G/S/ENUM_STD ioctl on /dev/videoX
>>>>>>> device node.
>>>>>>
>>>>>> Why so ? Shouldn't it be queried on the subdev output pad directly ?
>>>>>
>>>>> Because standard v4l2 application for analog devices will call these std
>>>>> ioctls on the streaming device node. So it's done on /dev/video to make
>>>> the
>>>>> existing apllication work.
>>>>
>>>> Existing applications can't work with the OMAP3 ISP (and similar complex
>>>> embedded devices) without userspace support anyway, either in the form of
>>>> a
>>>> GStreamer element or a libv4l plugin. I still believe that analog video
>>>> standard operations should be added to the subdev pad operations and
>>>> exposed
>>>> through subdev device nodes, exactly as done with formats.
>>>>
>>> [Hiremath, Vaibhav] Laurent,
>>>
>>> I completely agree with your point that, existing application will not work without setting links properly.
>>> But I believe the assumption here is, media-controller should set the links (along with pad formants) and
>>> all existing application should work as is. Isn't it?
>>
>> Yes.
>>
>>> The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming...
>>
>> Yes.
>>
>>> I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node.
>>
>> Agreed. Standards selection should be done at device node, just like any other
>> device.
> 
> So how do you handle a part like the TVP5150 that is standard agnostic?
> That device can sense the standard from the input signal and sets the
> result appropriately.
> 
See the em28xx driver. It uses tvp5150 at the device node, and works properly.

It should be noticed, however, that the implementation at tvp5150 doesn't
implement standards detection properly, due to hardware limitation.

A proper implementation of standards detection is to get the standards mask from
userspace and let the driver detect between the standards that the userspace is
expecting.

So, userspace could, for example, do things like:

	v4l2_std_id std = V4L2_STD_PAL_M | V4L2_STD_NTSC_M | V4L2_STD_PAL_DK;

	ioctl (fd, VIDIOC_S_STD, &std);
	msleep (100);
	ioctl (fd, VIDIOC_G_STD, &std);
	if (std & V4L2_STD_625_50)
		height = 576;
	else
		height = 480;

The above code won't work with the current tvp5150 driver, due to two reasons:

1) The tvp5150 register 0x28 doesn't allow an arbitrary standards mask like the above.
The driver does support standards detection, if V4L2_STD_ALL is passed into it.

2) even if V4L2_STD_ALL is used, the driver currently doesn't implement a
vidioc_g_std callback. So, the driver cannot return back to userspace and to
the bridge driver what standard were detected. Without this information, userspace
might use the wrong number of lines when allocating the buffer, and this won't
work.

Of course, patches for fixing this are welcome.

Thanks,
Mauro