On Fri, 28 Feb 2020 at 14:05, <Eugen.Hristev@xxxxxxxxxxxxx> wrote: > > On 28.02.2020 15:44, Dave Stevenson wrote: > > Hi Eugen. > > > > On Thu, 27 Feb 2020 at 15:19, Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> wrote: > >> > >> Add support for enum frame intervals IOCTL. > >> The current supported framerates are only available as comments inside > >> the code. > >> Add support for VIDIOC_ENUM_FRAMEINTERVALS as the enum_frame_interval > >> callback as pad ops. > >> > >> # v4l2-ctl --list-frameintervals width=1920,height=1080,pixelformat=RG10 > >> ioctl: VIDIOC_ENUM_FRAMEINTERVALS > >> Interval: Discrete 0.067s (15.000 fps) > >> Interval: Discrete 0.033s (30.000 fps) > >> Interval: Discrete 0.033s (30.000 fps) > > > > But the frame rates are not discrete. You have frame rate control via > > V4L2_CID_VBLANK, which can be used in conjunction with V4L2_CID_HBLANK > > and V4L2_CID_PIXEL_RATE to determine actual frame period. > > > > See https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/ext-ctrls-image-source.html?highlight=v4l2_cid_vblank > > I believe this is the preferred route to doing frame rate control on > > image sensors. I assume someone will correct me if I'm wrong on that. > > > Okay... , I was guided towards this by the comments in the code, saying > that the three supported modes are at a constant frame per second... > > Those comments are wrong then ? Yes, the comments for each of the modes (eg "/* 8MPix 15fps mode */") probably shouldn't have the frame rate in them. I don't see any other references. Those frame rates are the defaults only, as set via eg IMX219_VTS_15FPS. I originally wrote the driver without frame rate control, and the comments obviously didn't get updated when VTS/HTS support was added :-/ > Thanks for replying, > > Eugen > > > > > Dave > > > >> Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> > >> --- > >> > >> Hello, > >> > >> This is on top of Sakari's tree in linuxtv.org > >> > >> Thanks > >> Eugen > >> > >> drivers/media/i2c/imx219.c | 27 +++++++++++++++++++++++++++ > >> 1 file changed, 27 insertions(+) > >> > >> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > >> index f1effb5a5f66..17fcedd4edb6 100644 > >> --- a/drivers/media/i2c/imx219.c > >> +++ b/drivers/media/i2c/imx219.c > >> @@ -127,6 +127,8 @@ struct imx219_mode { > >> unsigned int width; > >> /* Frame height */ > >> unsigned int height; > >> + /* Frame rate */ > >> + u8 fps; > >> > >> /* V-timing */ > >> unsigned int vts_def; > >> @@ -381,6 +383,7 @@ static const struct imx219_mode supported_modes[] = { > >> /* 8MPix 15fps mode */ > >> .width = 3280, > >> .height = 2464, > >> + .fps = 15, > >> .vts_def = IMX219_VTS_15FPS, > >> .reg_list = { > >> .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs), > >> @@ -391,6 +394,7 @@ static const struct imx219_mode supported_modes[] = { > >> /* 1080P 30fps cropped */ > >> .width = 1920, > >> .height = 1080, > >> + .fps = 30, > >> .vts_def = IMX219_VTS_30FPS_1080P, > >> .reg_list = { > >> .num_of_regs = ARRAY_SIZE(mode_1920_1080_regs), > >> @@ -401,6 +405,7 @@ static const struct imx219_mode supported_modes[] = { > >> /* 2x2 binned 30fps mode */ > >> .width = 1640, > >> .height = 1232, > >> + .fps = 30, > >> .vts_def = IMX219_VTS_30FPS_BINNED, > >> .reg_list = { > >> .num_of_regs = ARRAY_SIZE(mode_1640_1232_regs), > >> @@ -680,6 +685,27 @@ static int imx219_enum_frame_size(struct v4l2_subdev *sd, > >> return 0; > >> } > >> > >> +static int imx219_enum_frame_interval(struct v4l2_subdev *sd, > >> + struct v4l2_subdev_pad_config *cfg, > >> + struct v4l2_subdev_frame_interval_enum *fie) > >> +{ > >> + struct imx219 *imx219 = to_imx219(sd); > >> + > >> + if (fie->index >= ARRAY_SIZE(supported_modes)) > >> + return -EINVAL; > >> + > >> + if (fie->code != imx219_get_format_code(imx219)) > >> + return -EINVAL; > >> + > >> + if (fie->pad) > >> + return -EINVAL; > >> + > >> + fie->interval.numerator = 1; > >> + fie->interval.denominator = supported_modes[fie->index].fps; > >> + > >> + return 0; > >> +} > >> + > >> static void imx219_reset_colorspace(struct v4l2_mbus_framefmt *fmt) > >> { > >> fmt->colorspace = V4L2_COLORSPACE_SRGB; > >> @@ -1004,6 +1030,7 @@ static const struct v4l2_subdev_pad_ops imx219_pad_ops = { > >> .get_fmt = imx219_get_pad_format, > >> .set_fmt = imx219_set_pad_format, > >> .enum_frame_size = imx219_enum_frame_size, > >> + .enum_frame_interval = imx219_enum_frame_interval, > >> }; > >> > >> static const struct v4l2_subdev_ops imx219_subdev_ops = { > >> -- > >> 2.20.1 > >> >
