* [PATCH 1/3] media: atomisp-ov2680: implement enum frame intervals
2021-11-11 17:27 [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
@ 2021-11-11 17:27 ` Mauro Carvalho Chehab
2021-11-11 17:27 ` [PATCH 2/3] media: atomisp-ov2680: adjust the maximum frame rate Mauro Carvalho Chehab
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-11 17:27 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen, linux-kernel,
linux-media, linux-staging
Allow the sensor to report the per-resolution frame interval.
It should be noticed that, despite the ov2680 datasheet says
that, for resolutions <= 1280x720, the sensor supports 60fps,
the data at ov2680.h tells otherwise, limiting it to 30fps.
Not sure yet if this is a limitation of atomisp or if it is
just because the tables are incorrect.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636651027.git.mchehab+huawei@kernel.org/
.../media/atomisp/i2c/atomisp-ov2680.c | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index d24f8830fd94..428e98310b3c 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -1075,6 +1075,26 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd,
return 0;
}
+static int ov2680_enum_frame_interval(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_frame_interval_enum *fie)
+{
+ struct v4l2_fract fract;
+
+ if (fie->index >= N_RES_PREVIEW ||
+ fie->width > ov2680_res_preview[0].width ||
+ fie->height > ov2680_res_preview[0].height ||
+ fie->which > V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ fract.denominator =ov2680_res_preview[fie->index].fps;
+ fract.numerator = 1;
+
+ fie->interval = fract;
+
+ return 0;
+}
+
static int ov2680_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
{
struct ov2680_device *dev = to_ov2680_sensor(sd);
@@ -1103,6 +1123,7 @@ static const struct v4l2_subdev_core_ops ov2680_core_ops = {
static const struct v4l2_subdev_pad_ops ov2680_pad_ops = {
.enum_mbus_code = ov2680_enum_mbus_code,
.enum_frame_size = ov2680_enum_frame_size,
+ .enum_frame_interval = ov2680_enum_frame_interval,
.get_fmt = ov2680_get_fmt,
.set_fmt = ov2680_set_fmt,
};
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] media: atomisp-ov2680: adjust the maximum frame rate
2021-11-11 17:27 [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
2021-11-11 17:27 ` [PATCH 1/3] media: atomisp-ov2680: implement enum frame intervals Mauro Carvalho Chehab
@ 2021-11-11 17:27 ` Mauro Carvalho Chehab
2021-11-11 17:27 ` [PATCH 3/3] media: atomisp: implement enum framesize/frameinterval Mauro Carvalho Chehab
2021-11-12 12:03 ` [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
3 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-11 17:27 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Bhaskar Chowdhury,
Deepak R Varma, Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
Mauro Carvalho Chehab, Randy Dunlap, Sakari Ailus, linux-kernel,
linux-media, linux-staging
The ov2680 supports a maximum bandwidth of 660Mbps and allows
up to 60fps when the resolution is below 720p.
Adjust the sensor's table to reflect that.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636651027.git.mchehab+huawei@kernel.org/
drivers/staging/media/atomisp/i2c/ov2680.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
index cb3c37d1c38e..4e351196fe34 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.h
+++ b/drivers/staging/media/atomisp/i2c/ov2680.h
@@ -822,7 +822,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 1280,
.height = 720,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -835,7 +835,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 800,
.height = 600,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -848,7 +848,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 720,
.height = 592,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -861,7 +861,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 656,
.height = 496,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -874,7 +874,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 336,
.height = 256,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -887,7 +887,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 352,
.height = 288,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
@@ -900,7 +900,7 @@ static struct ov2680_resolution ov2680_res_preview[] = {
{
.width = 176,
.height = 144,
- .fps = 30,
+ .fps = 60,
.pix_clk_freq = 66,
.pixels_per_line = 1698,//1704,
.lines_per_frame = 1294,
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] media: atomisp: implement enum framesize/frameinterval
2021-11-11 17:27 [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
2021-11-11 17:27 ` [PATCH 1/3] media: atomisp-ov2680: implement enum frame intervals Mauro Carvalho Chehab
2021-11-11 17:27 ` [PATCH 2/3] media: atomisp-ov2680: adjust the maximum frame rate Mauro Carvalho Chehab
@ 2021-11-11 17:27 ` Mauro Carvalho Chehab
2021-11-12 12:03 ` [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
3 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-11 17:27 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Dan Carpenter,
Greg Kroah-Hartman, Ingo Molnar, Kaixu Xia, Mauro Carvalho Chehab,
Peter Zijlstra, Sakari Ailus, Tsuchiya Yuto, linux-kernel,
linux-media, linux-staging
Add logic to enumerate sensor's frame size and interval.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1636651027.git.mchehab+huawei@kernel.org/
.../staging/media/atomisp/pci/atomisp_ioctl.c | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 2fb64d5cbead..b2d3b8349234 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -766,6 +766,57 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
return ret;
}
+static int atomisp_enum_framesizes(struct file *file, void *priv,
+ struct v4l2_frmsizeenum *fsize)
+{
+ struct video_device *vdev = video_devdata(file);
+ struct atomisp_device *isp = video_get_drvdata(vdev);
+ struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
+ struct v4l2_subdev_frame_size_enum fse = {
+ .index = fsize->index,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ int ret;
+
+ ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+ pad, enum_frame_size, NULL, &fse);
+ if (ret)
+ return ret;
+
+ fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+ fsize->discrete.width = fse.max_width - pad_w;
+ fsize->discrete.height = fse.max_height - pad_h;
+
+ return 0;
+}
+
+static int atomisp_enum_frameintervals(struct file *file, void *priv,
+ struct v4l2_frmivalenum *fival)
+{
+ struct video_device *vdev = video_devdata(file);
+ struct atomisp_device *isp = video_get_drvdata(vdev);
+ struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
+ struct v4l2_subdev_frame_interval_enum fie = {
+ .code = atomisp_in_fmt_conv[0].code,
+ .index = fival->index,
+ .width = fival->width,
+ .height = fival->height,
+ .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+ };
+ int ret;
+
+ ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+ pad, enum_frame_interval, NULL,
+ &fie);
+ if (ret)
+ return ret;
+
+ fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+ fival->discrete = fie.interval;
+
+ return ret;
+}
+
static int atomisp_enum_fmt_cap(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
{
@@ -3214,6 +3265,8 @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = {
.vidioc_g_ctrl = atomisp_g_ctrl,
.vidioc_s_ext_ctrls = atomisp_s_ext_ctrls,
.vidioc_g_ext_ctrls = atomisp_g_ext_ctrls,
+ .vidioc_enum_framesizes = atomisp_enum_framesizes,
+ .vidioc_enum_frameintervals = atomisp_enum_frameintervals,
.vidioc_enum_fmt_vid_cap = atomisp_enum_fmt_cap,
.vidioc_try_fmt_vid_cap = atomisp_try_fmt_cap,
.vidioc_g_fmt_vid_cap = atomisp_g_fmt_cap,
--
2.33.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] atomisp: add support for enum frame rate and sizes
2021-11-11 17:27 [PATCH 0/3] atomisp: add support for enum frame rate and sizes Mauro Carvalho Chehab
` (2 preceding siblings ...)
2021-11-11 17:27 ` [PATCH 3/3] media: atomisp: implement enum framesize/frameinterval Mauro Carvalho Chehab
@ 2021-11-12 12:03 ` Mauro Carvalho Chehab
3 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-12 12:03 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Bhaskar Chowdhury, Dan Carpenter,
Deepak R Varma, Greg Kroah-Hartman, Hans Verkuil, Hans de Goede,
Ingo Molnar, Kaixu Xia, Mauro Carvalho Chehab, Peter Zijlstra,
Randy Dunlap, Sakari Ailus, Tomi Valkeinen, Tsuchiya Yuto,
linux-kernel, linux-media, linux-staging
Em Thu, 11 Nov 2021 17:27:55 +0000
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Adding support for those two ioctls are trivial, and allow testing
> different resolutions with the driver.
>
> Together with some improvements I made at camorama, it is now
> possible to change the atomisp sensor's resolution at the GUI.
>
> Talking about camorama, I also added there an option to disable
> the Gtk cairo_scale() calls. On my tests here, placing ov2680 on
> its maximum resolution, I'm getting a framerate of 26 fps, which
> sounds reasonable, as the maximum would be 30fps, and I'm not
> using daylight.
>
> When letting cairo_scale() to run, the rate reduces to 8 fps, meaning
> that Gtk is not using GPU acceleration.
>
> Funny enough, when resolution is lower, atomisp is giving a very
> bad framerate (around 3fps, even with the scaler disabled).
>
> This is a very weird result, probably indicating some problems inside
> the driver. That requires further investigation.
Ok, when the sensor's resolution is not the highest one, what happens is
that the frame is retrieved via some timeout logic:
[10447.865064] ov2680 i2c-OVTI2680:00: ov2680_s_stream one
[10447.865491] atomisp-isp2 0000:00:03.0: timeout recovery handling done
[10450.029956] atomisp-isp2 0000:00:03.0: [WARNING]asd 0 pipe ATOMISP ISP PREVIEW output ISP timeout 1!
[10450.030099] atomisp-isp2 0000:00:03.0: pipe on asd0 timeout cnt: (0, 0, 1, 0) of 2, recover = 1
[10450.030137] atomisp-isp2 0000:00:03.0: pipe on asd1 timeout cnt: (0, 0, 0, 0) of 2, recover = 1
It sounds that it can't properly detect the end of frame when the
sensor is not on its highest resolution.
Basically atomisp_wdt_work() is triggered by a
IA_CSS_EVENT_TYPE_FW_ASSERT event. Identifying why this is happening
is not trivial, though.
Regards,
Mauro
^ permalink raw reply [flat|nested] 5+ messages in thread