All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 13:35 ` Deepthy Ravi
  0 siblings, 0 replies; 42+ messages in thread
From: Deepthy Ravi @ 2011-09-08 13:35 UTC (permalink / raw)
  To: linux-media
  Cc: tony, linux, linux-omap, linux-arm-kernel, linux-kernel, mchehab,
	laurent.pinchart, g.liakhovetski, Vaibhav Hiremath, 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


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 13:35 ` Deepthy Ravi
  0 siblings, 0 replies; 42+ messages in thread
From: Deepthy Ravi @ 2011-09-08 13:35 UTC (permalink / raw)
  To: linux-media
  Cc: tony, linux, linux-omap, linux-arm-kernel, linux-kernel, mchehab,
	laurent.pinchart, g.liakhovetski, Vaibhav Hiremath, 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

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 13:35 ` Deepthy Ravi
  0 siblings, 0 replies; 42+ messages in thread
From: Deepthy Ravi @ 2011-09-08 13:35 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-08 13:35 ` Deepthy Ravi
  (?)
@ 2011-09-08 17:21   ` Laurent Pinchart
  -1 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-08 17:21 UTC (permalink / raw)
  To: Deepthy Ravi
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski, Vaibhav Hiremath

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 17:21   ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-08 17:21 UTC (permalink / raw)
  To: Deepthy Ravi
  Cc: linux, tony, linux-kernel, Vaibhav Hiremath, mchehab, linux-omap,
	g.liakhovetski, linux-arm-kernel, linux-media

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 17:21   ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-08 17:21 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-08 13:35 ` Deepthy Ravi
  (?)
@ 2011-09-08 21:38   ` Sakari Ailus
  -1 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-08 21:38 UTC (permalink / raw)
  To: Deepthy Ravi
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, laurent.pinchart, g.liakhovetski,
	Vaibhav Hiremath

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 21:38   ` Sakari Ailus
  0 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-08 21:38 UTC (permalink / raw)
  To: Deepthy Ravi
  Cc: linux, tony, linux-kernel, Vaibhav Hiremath, mchehab,
	laurent.pinchart, linux-omap, g.liakhovetski, linux-arm-kernel,
	linux-media

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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-08 21:38   ` Sakari Ailus
  0 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-08 21:38 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-08 17:21   ` Laurent Pinchart
  (?)
@ 2011-09-16 13:00     ` Ravi, Deepthy
  -1 siblings, 0 replies; 42+ messages in thread
From: Ravi, Deepthy @ 2011-09-16 13:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski, Hiremath, Vaibhav

Hi,
Sorry for the delayed response.
> ________________________________________
> From: Laurent Pinchart [laurent.pinchart@ideasonboard.com]
> Sent: Thursday, September 08, 2011 10:51 PM
> To: Ravi, Deepthy
> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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
>



^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-16 13:00     ` Ravi, Deepthy
  0 siblings, 0 replies; 42+ messages in thread
From: Ravi, Deepthy @ 2011-09-16 13:00 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski, Hiremath, Vaibhav

Hi,
Sorry for the delayed response.
> ________________________________________
> From: Laurent Pinchart [laurent.pinchart@ideasonboard.com]
> Sent: Thursday, September 08, 2011 10:51 PM
> To: Ravi, Deepthy
> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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
>



^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-16 13:00     ` Ravi, Deepthy
  0 siblings, 0 replies; 42+ messages in thread
From: Ravi, Deepthy @ 2011-09-16 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

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
>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-16 13:00     ` Ravi, Deepthy
  (?)
@ 2011-09-16 13:06       ` Laurent Pinchart
  -1 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-16 13:06 UTC (permalink / raw)
  To: Ravi, Deepthy
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski, Hiremath, Vaibhav

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-16 13:06       ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-16 13:06 UTC (permalink / raw)
  To: Ravi, Deepthy
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski, Hiremath, Vaibhav

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-16 13:06       ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-16 13:06 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-16 13:06       ` Laurent Pinchart
  (?)
@ 2011-09-19 15:31         ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-19 15:31 UTC (permalink / raw)
  To: Laurent Pinchart, Ravi, Deepthy
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski


> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Friday, September 16, 2011 6:36 PM
> To: Ravi, Deepthy
> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-19 15:31         ` Hiremath, Vaibhav
  0 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-19 15:31 UTC (permalink / raw)
  To: Laurent Pinchart, Ravi, Deepthy
  Cc: linux-media, tony, linux, linux-omap, linux-arm-kernel,
	linux-kernel, mchehab, g.liakhovetski


> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Friday, September 16, 2011 6:36 PM
> To: Ravi, Deepthy
> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-19 15:31         ` Hiremath, Vaibhav
  0 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-19 15:31 UTC (permalink / raw)
  To: linux-arm-kernel


> -----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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-19 15:31         ` Hiremath, Vaibhav
  (?)
@ 2011-09-27 18:06           ` Laurent Pinchart
  -1 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-27 18:06 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Ravi, Deepthy, linux-media, tony, linux, linux-omap,
	linux-arm-kernel, linux-kernel, mchehab, g.liakhovetski

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 18:06           ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-27 18:06 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Ravi, Deepthy, linux-media, tony, linux, linux-omap,
	linux-arm-kernel, linux-kernel, mchehab, g.liakhovetski

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 18:06           ` Laurent Pinchart
  0 siblings, 0 replies; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-27 18:06 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-19 15:31         ` Hiremath, Vaibhav
  (?)
@ 2011-09-27 22:31           ` Mauro Carvalho Chehab
  -1 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-27 22:31 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Laurent Pinchart, Ravi, Deepthy, linux-media, tony, linux,
	linux-omap, linux-arm-kernel, linux-kernel, g.liakhovetski

Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> 
>> -----Original Message-----
>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>> Sent: Friday, September 16, 2011 6:36 PM
>> To: Ravi, Deepthy
>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 22:31           ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-27 22:31 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Laurent Pinchart, Ravi, Deepthy, linux-media, tony, linux,
	linux-omap, linux-arm-kernel, linux-kernel, g.liakhovetski

Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> 
>> -----Original Message-----
>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>> Sent: Friday, September 16, 2011 6:36 PM
>> To: Ravi, Deepthy
>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 22:31           ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-27 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-27 22:31           ` Mauro Carvalho Chehab
  (?)
  (?)
@ 2011-09-27 22:41           ` Laurent Pinchart
  2011-09-28  9:59             ` Mauro Carvalho Chehab
  -1 siblings, 1 reply; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-27 22:41 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hiremath, Vaibhav, Ravi, Deepthy, linux-media, g.liakhovetski,
	Sakari Ailus

Hi Mauro,

On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> > 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?
> 
> 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.

No. Please see my reply to Vaibhav's e-mail. Standard selection should be done 
on the subdev pads, for the exact same reason why formats and selection 
rectangles are configured on subdev pads.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-27 22:31           ` Mauro Carvalho Chehab
  (?)
@ 2011-09-27 22:49             ` Gary Thomas
  -1 siblings, 0 replies; 42+ messages in thread
From: Gary Thomas @ 2011-09-27 22:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hiremath, Vaibhav, Laurent Pinchart, Ravi, Deepthy, linux-media,
	tony, linux, linux-omap, linux-arm-kernel, linux-kernel,
	g.liakhovetski

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@ideasonboard.com]
>>> Sent: Friday, September 16, 2011 6:36 PM
>>> To: Ravi, Deepthy
>>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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
------------------------------------------------------------

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 22:49             ` Gary Thomas
  0 siblings, 0 replies; 42+ messages in thread
From: Gary Thomas @ 2011-09-27 22:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hiremath, Vaibhav, Laurent Pinchart, Ravi, Deepthy, linux-media,
	tony, linux, linux-omap, linux-arm-kernel, linux-kernel,
	g.liakhovetski

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@ideasonboard.com]
>>> Sent: Friday, September 16, 2011 6:36 PM
>>> To: Ravi, Deepthy
>>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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
------------------------------------------------------------

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-27 22:49             ` Gary Thomas
  0 siblings, 0 replies; 42+ messages in thread
From: Gary Thomas @ 2011-09-27 22:49 UTC (permalink / raw)
  To: linux-arm-kernel

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
------------------------------------------------------------

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-27 18:06           ` Laurent Pinchart
  (?)
@ 2011-09-28  6:10             ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-28  6:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Ravi, Deepthy, linux-media, tony, linux, linux-omap,
	linux-arm-kernel, linux-kernel, mchehab, g.liakhovetski

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Tuesday, September 27, 2011 11:36 PM
> To: Hiremath, Vaibhav
> Cc: Ravi, Deepthy; linux-media@vger.kernel.org; tony@atomide.com;
> linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  6:10             ` Hiremath, Vaibhav
  0 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-28  6:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Ravi, Deepthy, linux-media, tony, linux, linux-omap,
	linux-arm-kernel, linux-kernel, mchehab, g.liakhovetski

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Tuesday, September 27, 2011 11:36 PM
> To: Hiremath, Vaibhav
> Cc: Ravi, Deepthy; linux-media@vger.kernel.org; tony@atomide.com;
> linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  6:10             ` Hiremath, Vaibhav
  0 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-28  6:10 UTC (permalink / raw)
  To: linux-arm-kernel

> -----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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-28  6:10             ` Hiremath, Vaibhav
  (?)
@ 2011-09-28  9:28               ` Sakari Ailus
  -1 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-28  9:28 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Laurent Pinchart, Ravi, Deepthy, linux-media, tony, linux,
	linux-omap, linux-arm-kernel, linux-kernel, mchehab,
	g.liakhovetski

Hi Hiremath,

Hiremath, Vaibhav wrote:
>> -----Original Message----- From: Laurent Pinchart
>> [mailto:laurent.pinchart@ideasonboard.com] Sent: Tuesday, September
>> 27, 2011 11:36 PM To: Hiremath, Vaibhav Cc: Ravi, Deepthy;
>> linux-media@vger.kernel.org; tony@atomide.com; 
>> linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm- 
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; 
>> mchehab@infradead.org; g.liakhovetski@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@iki.fi

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  9:28               ` Sakari Ailus
  0 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-28  9:28 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Laurent Pinchart, Ravi, Deepthy, linux-media, tony, linux,
	linux-omap, linux-arm-kernel, linux-kernel, mchehab,
	g.liakhovetski

Hi Hiremath,

Hiremath, Vaibhav wrote:
>> -----Original Message----- From: Laurent Pinchart
>> [mailto:laurent.pinchart@ideasonboard.com] Sent: Tuesday, September
>> 27, 2011 11:36 PM To: Hiremath, Vaibhav Cc: Ravi, Deepthy;
>> linux-media@vger.kernel.org; tony@atomide.com; 
>> linux@arm.linux.org.uk; linux-omap@vger.kernel.org; linux-arm- 
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; 
>> mchehab@infradead.org; g.liakhovetski@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@iki.fi

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  9:28               ` Sakari Ailus
  0 siblings, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-28  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-27 22:49             ` Gary Thomas
  (?)
  (?)
@ 2011-09-28  9:58               ` Mauro Carvalho Chehab
  -1 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-28  9:58 UTC (permalink / raw)
  To: Gary Thomas
  Cc: Hiremath, Vaibhav, Laurent Pinchart, Ravi, Deepthy, linux-media,
	tony, linux, linux-omap, linux-arm-kernel, linux-kernel,
	g.liakhovetski

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@ideasonboard.com]
>>>> Sent: Friday, September 16, 2011 6:36 PM
>>>> To: Ravi, Deepthy
>>>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>>>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>>>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  9:58               ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-28  9:58 UTC (permalink / raw)
  To: Gary Thomas
  Cc: Hiremath, Vaibhav, Laurent Pinchart, Ravi, Deepthy, linux-media,
	tony, linux, linux-omap, linux-arm-kernel, linux-kernel,
	g.liakhovetski

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@ideasonboard.com]
>>>> Sent: Friday, September 16, 2011 6:36 PM
>>>> To: Ravi, Deepthy
>>>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>>>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>>>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  9:58               ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-28  9:58 UTC (permalink / raw)
  To: Gary Thomas
  Cc: Ravi, Deepthy, linux, tony, linux-kernel, Hiremath, Vaibhav,
	Laurent Pinchart, linux-omap, g.liakhovetski, linux-arm-kernel,
	linux-media

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@ideasonboard.com]
>>>> Sent: Friday, September 16, 2011 6:36 PM
>>>> To: Ravi, Deepthy
>>>> Cc: linux-media@vger.kernel.org; tony@atomide.com; linux@arm.linux.org.uk;
>>>> linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>>>> kernel@vger.kernel.org; mchehab@infradead.org; g.liakhovetski@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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
@ 2011-09-28  9:58               ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-28  9:58 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-27 22:41           ` Laurent Pinchart
@ 2011-09-28  9:59             ` Mauro Carvalho Chehab
  2011-09-28 10:03               ` Sakari Ailus
  2011-09-28 13:29               ` Laurent Pinchart
  0 siblings, 2 replies; 42+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-28  9:59 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hiremath, Vaibhav, Ravi, Deepthy, linux-media, g.liakhovetski,
	Sakari Ailus

Em 27-09-2011 19:41, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
>> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
>>> 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?
>>
>> 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.
> 
> No. Please see my reply to Vaibhav's e-mail. Standard selection should be done 
> on the subdev pads, for the exact same reason why formats and selection 
> rectangles are configured on subdev pads.

NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API.

Mauro.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-28  9:59             ` Mauro Carvalho Chehab
@ 2011-09-28 10:03               ` Sakari Ailus
  2011-09-28 13:29               ` Laurent Pinchart
  1 sibling, 0 replies; 42+ messages in thread
From: Sakari Ailus @ 2011-09-28 10:03 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Laurent Pinchart, Hiremath, Vaibhav, Ravi, Deepthy, linux-media,
	g.liakhovetski

Mauro Carvalho Chehab wrote:
> Em 27-09-2011 19:41, Laurent Pinchart escreveu:
>> Hi Mauro,
>>
>> On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
>>> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
>>>> 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?
>>>
>>> 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.
>>
>> No. Please see my reply to Vaibhav's e-mail. Standard selection should be done 
>> on the subdev pads, for the exact same reason why formats and selection 
>> rectangles are configured on subdev pads.
> 
> NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API.

That has never been the intention.

My proposal is that the {G,S,ENUM}_STD is implemented in the V4L2 subdev
user space interface and then the libv4l plugin redirects the call to
the right subdev, in a similar fashion which is planned for V4L2 controls.

Kind regards,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-28  9:59             ` Mauro Carvalho Chehab
  2011-09-28 10:03               ` Sakari Ailus
@ 2011-09-28 13:29               ` Laurent Pinchart
  2011-09-28 13:46                 ` Hiremath, Vaibhav
  1 sibling, 1 reply; 42+ messages in thread
From: Laurent Pinchart @ 2011-09-28 13:29 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hiremath, Vaibhav, Ravi, Deepthy, linux-media, g.liakhovetski,
	Sakari Ailus

Hi Mauro,

On Wednesday 28 September 2011 11:59:30 Mauro Carvalho Chehab wrote:
> Em 27-09-2011 19:41, Laurent Pinchart escreveu:
> > On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
> >> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> >>> 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?
> >> 
> >> 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.
> > 
> > No. Please see my reply to Vaibhav's e-mail. Standard selection should be
> > done on the subdev pads, for the exact same reason why formats and
> > selection rectangles are configured on subdev pads.
> 
> NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API.

I will NACK any patch that implements G/S/ENUM_STD in the OMAP3 ISP driver 
itself, so we got a deadlock here :-)

Maybe it would be easier to discuss this face to face in Prague ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
  2011-09-28 13:29               ` Laurent Pinchart
@ 2011-09-28 13:46                 ` Hiremath, Vaibhav
  0 siblings, 0 replies; 42+ messages in thread
From: Hiremath, Vaibhav @ 2011-09-28 13:46 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab
  Cc: Ravi, Deepthy, linux-media, g.liakhovetski, Sakari Ailus

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Wednesday, September 28, 2011 6:59 PM
> To: Mauro Carvalho Chehab
> Cc: Hiremath, Vaibhav; Ravi, Deepthy; linux-media@vger.kernel.org;
> g.liakhovetski@gmx.de; Sakari Ailus
> Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
> 
> Hi Mauro,
> 
> On Wednesday 28 September 2011 11:59:30 Mauro Carvalho Chehab wrote:
> > Em 27-09-2011 19:41, Laurent Pinchart escreveu:
> > > On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
> > >> Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
> > >>> 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?
> > >>
> > >> 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.
> > >
> > > No. Please see my reply to Vaibhav's e-mail. Standard selection should
> be
> > > done on the subdev pads, for the exact same reason why formats and
> > > selection rectangles are configured on subdev pads.
> >
> > NACK. Let's not reinvent the wheel. the MC should not replace the V4L2
> API.
> 
> I will NACK any patch that implements G/S/ENUM_STD in the OMAP3 ISP driver
> itself, so we got a deadlock here :-)
> 
> Maybe it would be easier to discuss this face to face in Prague ?
> 
[Hiremath, Vaibhav] I am surprised and afraid to say that, we are breaking
existing V4L2 standard applications. 

We are referring to libv4l, which anyway should follow (compliant to)
standard V4L2 spec; and for analog device interface we must support
G/S/ENUM_STD ioctl interface.

What if I don't want to use libv4l and writing my own application?
What if I only want to validate my driver (without libv4l) and the
underneath TVP5146 device is being used in both MC and non-MC compatible devices?

For example,

In my case, I have OMAP3 (with MC support) and AM3517 (without MC support),
And I do my whole testing with simple and plain C application which works on both without any change/issues. 
Streaming application is exactly same, only in case of OMAP3, I have to set the links with media-ctl before streaming.


I think we had enough discussion on this, and I don't see you are getting 
into alignment with this (I am surprised). I vote for F2F discussion, but
I will not be available. Hope you will update me on this.

Thanks,
Vaibhav

> --
> Regards,
> 
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2011-09-28 13:47 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-08 13:35 [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Deepthy Ravi
2011-09-08 13:35 ` Deepthy Ravi
2011-09-08 13:35 ` Deepthy Ravi
2011-09-08 17:21 ` Laurent Pinchart
2011-09-08 17:21   ` Laurent Pinchart
2011-09-08 17:21   ` Laurent Pinchart
2011-09-16 13:00   ` Ravi, Deepthy
2011-09-16 13:00     ` Ravi, Deepthy
2011-09-16 13:00     ` Ravi, Deepthy
2011-09-16 13:06     ` Laurent Pinchart
2011-09-16 13:06       ` Laurent Pinchart
2011-09-16 13:06       ` Laurent Pinchart
2011-09-19 15:31       ` Hiremath, Vaibhav
2011-09-19 15:31         ` Hiremath, Vaibhav
2011-09-19 15:31         ` Hiremath, Vaibhav
2011-09-27 18:06         ` Laurent Pinchart
2011-09-27 18:06           ` Laurent Pinchart
2011-09-27 18:06           ` Laurent Pinchart
2011-09-28  6:10           ` Hiremath, Vaibhav
2011-09-28  6:10             ` Hiremath, Vaibhav
2011-09-28  6:10             ` Hiremath, Vaibhav
2011-09-28  9:28             ` Sakari Ailus
2011-09-28  9:28               ` Sakari Ailus
2011-09-28  9:28               ` Sakari Ailus
2011-09-27 22:31         ` Mauro Carvalho Chehab
2011-09-27 22:31           ` Mauro Carvalho Chehab
2011-09-27 22:31           ` Mauro Carvalho Chehab
2011-09-27 22:41           ` Laurent Pinchart
2011-09-28  9:59             ` Mauro Carvalho Chehab
2011-09-28 10:03               ` Sakari Ailus
2011-09-28 13:29               ` Laurent Pinchart
2011-09-28 13:46                 ` Hiremath, Vaibhav
2011-09-27 22:49           ` Gary Thomas
2011-09-27 22:49             ` Gary Thomas
2011-09-27 22:49             ` Gary Thomas
2011-09-28  9:58             ` Mauro Carvalho Chehab
2011-09-28  9:58               ` Mauro Carvalho Chehab
2011-09-28  9:58               ` Mauro Carvalho Chehab
2011-09-28  9:58               ` Mauro Carvalho Chehab
2011-09-08 21:38 ` Sakari Ailus
2011-09-08 21:38   ` Sakari Ailus
2011-09-08 21:38   ` Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.