Re: [PATCH 2/2] tw686x: Support VIDIOC_{S,G}_PARM ioctls

[Ezequiel Garcia]

On 01 Jul 12:46 PM, Hans Verkuil wrote:
> On 06/29/2016 04:17 AM, Ezequiel Garcia wrote:
> > Now that the frame rate can be properly set, this commit adds support
> > for S_PARM and G_PARM.
> 
> As mentioned in our irc discussion you also need to add 
> enum_framesize/intervals.
> Otherwise applications won't know what values to use with s_parm.
> 

Sure, I'll add those as follow-up patches.

> > 
> > Signed-off-by: Ezequiel Garcia 
> > ---
> >  drivers/media/pci/tw686x/tw686x-video.c | 46 
> > ++---
> >  1 file changed, 43 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
> > b/drivers/media/pci/tw686x/tw686x-video.c
> > index 3131f9305313..40b5b835d452 100644
> > --- a/drivers/media/pci/tw686x/tw686x-video.c
> > +++ b/drivers/media/pci/tw686x/tw686x-video.c
> > @@ -437,9 +437,6 @@ static void tw686x_set_framerate(struct 
> > tw686x_video_channel *vc,
> >  {
> > unsigned int i;
> >  
> > -   if (vc->fps == fps)
> > -   return;
> > -
> > i = tw686x_fps_idx(fps, TW686X_MAX_FPS(vc->video_standard));
> > reg_write(vc->dev, VIDEO_FIELD_CTRL[vc->ch], fps_map[i]);
> > vc->fps = tw686x_real_fps(i, TW686X_MAX_FPS(vc->video_standard));
> > @@ -843,6 +840,12 @@ static int tw686x_s_std(struct file *file, void *priv, 
> > v4l2_std_id id)
> > ret = tw686x_g_fmt_vid_cap(file, priv, );
> > if (!ret)
> > tw686x_s_fmt_vid_cap(file, priv, );
> > +
> > +   /*
> > +* Frame decimation depends on the chosen standard,
> > +* so reset it to the current value.
> > +*/
> > +   tw686x_set_framerate(vc, vc->fps);
> > return 0;
> >  }
> >  
> > @@ -912,6 +915,40 @@ static int tw686x_g_std(struct file *file, void *priv, 
> > v4l2_std_id *id)
> > return 0;
> >  }
> >  
> > +static int tw686x_g_parm(struct file *file, void *priv,
> > +struct v4l2_streamparm *sp)
> > +{
> > +   struct tw686x_video_channel *vc = video_drvdata(file);
> > +   struct v4l2_captureparm *cp = >parm.capture;
> > +
> > +   if (sp->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +   return -EINVAL;
> 
> You need this check in s_parm as well.
> 

Will do.

> > +   sp->parm.capture.readbuffers = 3;
> > +
> > +   cp->capability = V4L2_CAP_TIMEPERFRAME;
> > +   cp->timeperframe.numerator = 1;
> > +   cp->timeperframe.denominator = vc->fps;
> > +   return 0;
> > +}
> > +
> > +static int tw686x_s_parm(struct file *file, void *priv,
> > +struct v4l2_streamparm *sp)
> > +{
> > +   struct tw686x_video_channel *vc = video_drvdata(file);
> > +   struct v4l2_captureparm *cp = >parm.capture;
> > +   unsigned int denominator = cp->timeperframe.denominator;
> > +   unsigned int numerator = cp->timeperframe.numerator;
> > +   unsigned int fps;
> > +
> > +   if (vb2_is_busy(>vidq))
> > +   return -EBUSY;
> > +
> > +   fps = (!numerator || !denominator) ? 0 : denominator / numerator;
> > +   if (vc->fps != fps)
> > +   tw686x_set_framerate(vc, fps);
> > +   return tw686x_g_parm(file, priv, sp);
> > +}
> > +
> >  static int tw686x_enum_fmt_vid_cap(struct file *file, void *priv,
> >struct v4l2_fmtdesc *f)
> >  {
> > @@ -998,6 +1035,9 @@ static const struct v4l2_ioctl_ops 
> > tw686x_video_ioctl_ops = {
> > .vidioc_g_std   = tw686x_g_std,
> > .vidioc_s_std   = tw686x_s_std,
> >  
> > +   .vidioc_g_parm  = tw686x_g_parm,
> > +   .vidioc_s_parm  = tw686x_s_parm,
> > +
> > .vidioc_enum_input  = tw686x_enum_input,
> > .vidioc_g_input = tw686x_g_input,
> > .vidioc_s_input = tw686x_s_input,
> > 

Thanks,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] tw686x: Support VIDIOC_{S,G}_PARM ioctls

[Hans Verkuil]

On 06/29/2016 04:17 AM, Ezequiel Garcia wrote:
> Now that the frame rate can be properly set, this commit adds support
> for S_PARM and G_PARM.

As mentioned in our irc discussion you also need to add 
enum_framesize/intervals.
Otherwise applications won't know what values to use with s_parm.

> 
> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/media/pci/tw686x/tw686x-video.c | 46 
> ++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
> b/drivers/media/pci/tw686x/tw686x-video.c
> index 3131f9305313..40b5b835d452 100644
> --- a/drivers/media/pci/tw686x/tw686x-video.c
> +++ b/drivers/media/pci/tw686x/tw686x-video.c
> @@ -437,9 +437,6 @@ static void tw686x_set_framerate(struct 
> tw686x_video_channel *vc,
>  {
>   unsigned int i;
>  
> - if (vc->fps == fps)
> - return;
> -
>   i = tw686x_fps_idx(fps, TW686X_MAX_FPS(vc->video_standard));
>   reg_write(vc->dev, VIDEO_FIELD_CTRL[vc->ch], fps_map[i]);
>   vc->fps = tw686x_real_fps(i, TW686X_MAX_FPS(vc->video_standard));
> @@ -843,6 +840,12 @@ static int tw686x_s_std(struct file *file, void *priv, 
> v4l2_std_id id)
>   ret = tw686x_g_fmt_vid_cap(file, priv, );
>   if (!ret)
>   tw686x_s_fmt_vid_cap(file, priv, );
> +
> + /*
> +  * Frame decimation depends on the chosen standard,
> +  * so reset it to the current value.
> +  */
> + tw686x_set_framerate(vc, vc->fps);
>   return 0;
>  }
>  
> @@ -912,6 +915,40 @@ static int tw686x_g_std(struct file *file, void *priv, 
> v4l2_std_id *id)
>   return 0;
>  }
>  
> +static int tw686x_g_parm(struct file *file, void *priv,
> +  struct v4l2_streamparm *sp)
> +{
> + struct tw686x_video_channel *vc = video_drvdata(file);
> + struct v4l2_captureparm *cp = >parm.capture;
> +
> + if (sp->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + return -EINVAL;

You need this check in s_parm as well.

> + sp->parm.capture.readbuffers = 3;
> +
> + cp->capability = V4L2_CAP_TIMEPERFRAME;
> + cp->timeperframe.numerator = 1;
> + cp->timeperframe.denominator = vc->fps;
> + return 0;
> +}
> +
> +static int tw686x_s_parm(struct file *file, void *priv,
> +  struct v4l2_streamparm *sp)
> +{
> + struct tw686x_video_channel *vc = video_drvdata(file);
> + struct v4l2_captureparm *cp = >parm.capture;
> + unsigned int denominator = cp->timeperframe.denominator;
> + unsigned int numerator = cp->timeperframe.numerator;
> + unsigned int fps;
> +
> + if (vb2_is_busy(>vidq))
> + return -EBUSY;
> +
> + fps = (!numerator || !denominator) ? 0 : denominator / numerator;
> + if (vc->fps != fps)
> + tw686x_set_framerate(vc, fps);
> + return tw686x_g_parm(file, priv, sp);
> +}
> +
>  static int tw686x_enum_fmt_vid_cap(struct file *file, void *priv,
>  struct v4l2_fmtdesc *f)
>  {
> @@ -998,6 +1035,9 @@ static const struct v4l2_ioctl_ops 
> tw686x_video_ioctl_ops = {
>   .vidioc_g_std   = tw686x_g_std,
>   .vidioc_s_std   = tw686x_s_std,
>  
> + .vidioc_g_parm  = tw686x_g_parm,
> + .vidioc_s_parm  = tw686x_s_parm,
> +
>   .vidioc_enum_input  = tw686x_enum_input,
>   .vidioc_g_input = tw686x_g_input,
>   .vidioc_s_input = tw686x_s_input,
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] tw686x: Support VIDIOC_{S,G}_PARM ioctls

[Ezequiel Garcia]

Now that the frame rate can be properly set, this commit adds support
for S_PARM and G_PARM.

Signed-off-by: Ezequiel Garcia 
---
 drivers/media/pci/tw686x/tw686x-video.c | 46 ++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c
index 3131f9305313..40b5b835d452 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -437,9 +437,6 @@ static void tw686x_set_framerate(struct 
tw686x_video_channel *vc,
 {
unsigned int i;
 
-   if (vc->fps == fps)
-   return;
-
i = tw686x_fps_idx(fps, TW686X_MAX_FPS(vc->video_standard));
reg_write(vc->dev, VIDEO_FIELD_CTRL[vc->ch], fps_map[i]);
vc->fps = tw686x_real_fps(i, TW686X_MAX_FPS(vc->video_standard));
@@ -843,6 +840,12 @@ static int tw686x_s_std(struct file *file, void *priv, 
v4l2_std_id id)
ret = tw686x_g_fmt_vid_cap(file, priv, );
if (!ret)
tw686x_s_fmt_vid_cap(file, priv, );
+
+   /*
+* Frame decimation depends on the chosen standard,
+* so reset it to the current value.
+*/
+   tw686x_set_framerate(vc, vc->fps);
return 0;
 }
 
@@ -912,6 +915,40 @@ static int tw686x_g_std(struct file *file, void *priv, 
v4l2_std_id *id)
return 0;
 }
 
+static int tw686x_g_parm(struct file *file, void *priv,
+struct v4l2_streamparm *sp)
+{
+   struct tw686x_video_channel *vc = video_drvdata(file);
+   struct v4l2_captureparm *cp = >parm.capture;
+
+   if (sp->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   return -EINVAL;
+   sp->parm.capture.readbuffers = 3;
+
+   cp->capability = V4L2_CAP_TIMEPERFRAME;
+   cp->timeperframe.numerator = 1;
+   cp->timeperframe.denominator = vc->fps;
+   return 0;
+}
+
+static int tw686x_s_parm(struct file *file, void *priv,
+struct v4l2_streamparm *sp)
+{
+   struct tw686x_video_channel *vc = video_drvdata(file);
+   struct v4l2_captureparm *cp = >parm.capture;
+   unsigned int denominator = cp->timeperframe.denominator;
+   unsigned int numerator = cp->timeperframe.numerator;
+   unsigned int fps;
+
+   if (vb2_is_busy(>vidq))
+   return -EBUSY;
+
+   fps = (!numerator || !denominator) ? 0 : denominator / numerator;
+   if (vc->fps != fps)
+   tw686x_set_framerate(vc, fps);
+   return tw686x_g_parm(file, priv, sp);
+}
+
 static int tw686x_enum_fmt_vid_cap(struct file *file, void *priv,
   struct v4l2_fmtdesc *f)
 {
@@ -998,6 +1035,9 @@ static const struct v4l2_ioctl_ops tw686x_video_ioctl_ops 
= {
.vidioc_g_std   = tw686x_g_std,
.vidioc_s_std   = tw686x_s_std,
 
+   .vidioc_g_parm  = tw686x_g_parm,
+   .vidioc_s_parm  = tw686x_s_parm,
+
.vidioc_enum_input  = tw686x_enum_input,
.vidioc_g_input = tw686x_g_input,
.vidioc_s_input = tw686x_s_input,
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html