diff mbox series

media: rcar-vin: add G/S_PARM ioctls

Message ID 20210924084115.2340-1-nikita.yoush@cogentembedded.com (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series media: rcar-vin: add G/S_PARM ioctls | expand

Commit Message

Nikita Yushchenko Sept. 24, 2021, 8:41 a.m. UTC
From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>

This adds g/s_parm ioctls for parallel interface.

Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Niklas Söderlund Sept. 24, 2021, 9:54 a.m. UTC | #1
Hello Nikita and Vladimir,

Thanks for your patch.

On 2021-09-24 11:41:15 +0300, Nikita Yushchenko wrote:
> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> 
> This adds g/s_parm ioctls for parallel interface.

I would like to ask your use-case for this. I'm trying to make up the 
courage to move Gen2 inline with Gen3, that is move Gen2 to use the 
media graph interface to allow it more complex use-cases, including 
controlling parameters on the subdevice level.

So I'm curious if this solve a particular problem for you or if it's 
done "just" for completeness. If it solves a real problem then I think 
we should move a head with a v2 (with the small comment below fixed) if 
not I would like to try and reduce the non media graph usage of the API 
as much as possible.

> 
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index bdeff51bf768..de9f8dd55a30 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -527,6 +527,24 @@ static int rvin_s_selection(struct file *file, void *fh,
>  	return 0;
>  }
>  
> +static int rvin_g_parm(struct file *file, void *priv,
> +		       struct v4l2_streamparm *parm)
> +{
> +	struct rvin_dev *vin = video_drvdata(file);
> +	struct v4l2_subdev *sd = vin_to_source(vin);
> +
> +	return v4l2_g_parm_cap(video_devdata(file), sd, parm);

Please use &vin->vdev instead of video_devdata(file). 

> +}
> +
> +static int rvin_s_parm(struct file *file, void *priv,
> +		       struct v4l2_streamparm *parm)
> +{
> +	struct rvin_dev *vin = video_drvdata(file);
> +	struct v4l2_subdev *sd = vin_to_source(vin);
> +
> +	return v4l2_s_parm_cap(video_devdata(file), sd, parm);

Please use &vin->vdev instead of video_devdata(file). 

> +}
> +
>  static int rvin_g_pixelaspect(struct file *file, void *priv,
>  			      int type, struct v4l2_fract *f)
>  {
> @@ -743,6 +761,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_g_selection		= rvin_g_selection,
>  	.vidioc_s_selection		= rvin_s_selection,
>  
> +	.vidioc_g_parm			= rvin_g_parm,
> +	.vidioc_s_parm			= rvin_s_parm,
> +
>  	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
>  
>  	.vidioc_enum_input		= rvin_enum_input,
> -- 
> 2.30.2
>
Nikita Yushchenko Sept. 24, 2021, 1:48 p.m. UTC | #2
> I would like to ask your use-case for this. I'm trying to make up the
> courage to move Gen2 inline with Gen3, that is move Gen2 to use the
> media graph interface to allow it more complex use-cases, including
> controlling parameters on the subdevice level.
> 
> So I'm curious if this solve a particular problem for you or if it's
> done "just" for completeness. If it solves a real problem then I think
> we should move a head with a v2 (with the small comment below fixed) if
> not I would like to try and reduce the non media graph usage of the API
> as much as possible.

I believe parallel camera - such as ov5642 - connected to Kingfisher's parallel interface still has to 
be controlled over v4l operations on vin device. And, to control frame rate of such cameras [which is 
the usecase we have here at Cogent], the operations that this patch adds are requied.

> Please use &vin->vdev instead of video_devdata(file).

Preparing v2 now.

Nikita
diff mbox series

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index bdeff51bf768..de9f8dd55a30 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -527,6 +527,24 @@  static int rvin_s_selection(struct file *file, void *fh,
 	return 0;
 }
 
+static int rvin_g_parm(struct file *file, void *priv,
+		       struct v4l2_streamparm *parm)
+{
+	struct rvin_dev *vin = video_drvdata(file);
+	struct v4l2_subdev *sd = vin_to_source(vin);
+
+	return v4l2_g_parm_cap(video_devdata(file), sd, parm);
+}
+
+static int rvin_s_parm(struct file *file, void *priv,
+		       struct v4l2_streamparm *parm)
+{
+	struct rvin_dev *vin = video_drvdata(file);
+	struct v4l2_subdev *sd = vin_to_source(vin);
+
+	return v4l2_s_parm_cap(video_devdata(file), sd, parm);
+}
+
 static int rvin_g_pixelaspect(struct file *file, void *priv,
 			      int type, struct v4l2_fract *f)
 {
@@ -743,6 +761,9 @@  static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_g_selection		= rvin_g_selection,
 	.vidioc_s_selection		= rvin_s_selection,
 
+	.vidioc_g_parm			= rvin_g_parm,
+	.vidioc_s_parm			= rvin_s_parm,
+
 	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
 
 	.vidioc_enum_input		= rvin_enum_input,