* [PATCH] media: rcar-vin: add G/S_PARM ioctls
@ 2021-09-24 8:41 Nikita Yushchenko
2021-09-24 9:54 ` Niklas Söderlund
0 siblings, 1 reply; 7+ messages in thread
From: Nikita Yushchenko @ 2021-09-24 8:41 UTC (permalink / raw)
To: Niklas Söderlund, Mauro Carvalho Chehab
Cc: linux-media, linux-renesas-soc, linux-kernel, Vladimir Barinov,
Nikita Yushchenko
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(+)
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,
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] media: rcar-vin: add G/S_PARM ioctls
2021-09-24 8:41 [PATCH] media: rcar-vin: add G/S_PARM ioctls Nikita Yushchenko
@ 2021-09-24 9:54 ` Niklas Söderlund
2021-09-24 13:48 ` Nikita Yushchenko
2021-09-24 13:51 ` [PATCH v2] " Nikita Yushchenko
0 siblings, 2 replies; 7+ messages in thread
From: Niklas Söderlund @ 2021-09-24 9:54 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
linux-kernel, Vladimir Barinov
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
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] media: rcar-vin: add G/S_PARM ioctls
2021-09-24 9:54 ` Niklas Söderlund
@ 2021-09-24 13:48 ` Nikita Yushchenko
2021-09-24 13:51 ` [PATCH v2] " Nikita Yushchenko
1 sibling, 0 replies; 7+ messages in thread
From: Nikita Yushchenko @ 2021-09-24 13:48 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
linux-kernel, Vladimir Barinov
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] media: rcar-vin: add G/S_PARM ioctls
2021-09-24 9:54 ` Niklas Söderlund
2021-09-24 13:48 ` Nikita Yushchenko
@ 2021-09-24 13:51 ` Nikita Yushchenko
2021-09-28 0:03 ` Niklas Söderlund
2021-09-28 8:43 ` Jacopo Mondi
1 sibling, 2 replies; 7+ messages in thread
From: Nikita Yushchenko @ 2021-09-24 13:51 UTC (permalink / raw)
To: Niklas Söderlund, Mauro Carvalho Chehab
Cc: linux-media, linux-renesas-soc, linux-kernel, Vladimir Barinov,
Nikita Yushchenko
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>
---
Changes from v1:
- use &vin->vdev to access vin's struct video_device
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..a5bfa76fdac6 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(&vin->vdev, 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(&vin->vdev, 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,
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] media: rcar-vin: add G/S_PARM ioctls
2021-09-24 13:51 ` [PATCH v2] " Nikita Yushchenko
@ 2021-09-28 0:03 ` Niklas Söderlund
2021-09-28 8:43 ` Jacopo Mondi
1 sibling, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2021-09-28 0:03 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
linux-kernel, Vladimir Barinov
Hello Nikita and Vladimir,
Thanks for your work.
On 2021-09-24 16:51:38 +0300, Nikita Yushchenko wrote:
> 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>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> Changes from v1:
> - use &vin->vdev to access vin's struct video_device
>
> 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..a5bfa76fdac6 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(&vin->vdev, 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(&vin->vdev, 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,
> --
> 2.30.2
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] media: rcar-vin: add G/S_PARM ioctls
2021-09-24 13:51 ` [PATCH v2] " Nikita Yushchenko
2021-09-28 0:03 ` Niklas Söderlund
@ 2021-09-28 8:43 ` Jacopo Mondi
2021-10-12 7:33 ` Hans Verkuil
1 sibling, 1 reply; 7+ messages in thread
From: Jacopo Mondi @ 2021-09-28 8:43 UTC (permalink / raw)
To: Nikita Yushchenko, Hans Verkuil
Cc: Niklas Söderlund, Mauro Carvalho Chehab, linux-media,
linux-renesas-soc, linux-kernel, Vladimir Barinov
Hello
explicit Cc Hans, as I recall the usage of s/g_parm was deprecated
and discouraged in mainline.
Hans: Support for g/s_param is required by Nikita to maintain
compatibility with (out of tree?) subdevice drivers. Should we add it
to the mainline receiver driver ?
What other API should be used to control the subdevice framerate ?
Should it go through VIDIOC_SUBDEV_S_FRAME_INTERVAL instead ?
Thanks
j
On Fri, Sep 24, 2021 at 04:51:38PM +0300, Nikita Yushchenko wrote:
> 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>
> ---
> Changes from v1:
> - use &vin->vdev to access vin's struct video_device
>
> 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..a5bfa76fdac6 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(&vin->vdev, 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(&vin->vdev, 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,
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] media: rcar-vin: add G/S_PARM ioctls
2021-09-28 8:43 ` Jacopo Mondi
@ 2021-10-12 7:33 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2021-10-12 7:33 UTC (permalink / raw)
To: Jacopo Mondi, Nikita Yushchenko
Cc: Niklas Söderlund, Mauro Carvalho Chehab, linux-media,
linux-renesas-soc, linux-kernel, Vladimir Barinov
Hi Jacopo,
On 28/09/2021 10:43, Jacopo Mondi wrote:
> Hello
> explicit Cc Hans, as I recall the usage of s/g_parm was deprecated
> and discouraged in mainline.
It's perfectly fine to use it, but for the non-MC case only. Which is what
this patch does, so I'm accepting it.
s/g_parm isn't deprecated for non-MC drivers, it's ugly but it's the only
way to set or report the framerate for such drivers.
Regards,
Hans
>
> Hans: Support for g/s_param is required by Nikita to maintain
> compatibility with (out of tree?) subdevice drivers. Should we add it
> to the mainline receiver driver ?
>
> What other API should be used to control the subdevice framerate ?
> Should it go through VIDIOC_SUBDEV_S_FRAME_INTERVAL instead ?
>
> Thanks
> j
>
> On Fri, Sep 24, 2021 at 04:51:38PM +0300, Nikita Yushchenko wrote:
>> 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>
>> ---
>> Changes from v1:
>> - use &vin->vdev to access vin's struct video_device
>>
>> 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..a5bfa76fdac6 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(&vin->vdev, 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(&vin->vdev, 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,
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-12 7:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 8:41 [PATCH] media: rcar-vin: add G/S_PARM ioctls Nikita Yushchenko
2021-09-24 9:54 ` Niklas Söderlund
2021-09-24 13:48 ` Nikita Yushchenko
2021-09-24 13:51 ` [PATCH v2] " Nikita Yushchenko
2021-09-28 0:03 ` Niklas Söderlund
2021-09-28 8:43 ` Jacopo Mondi
2021-10-12 7:33 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).