Hi Jacopo,
On 2/24/22 10:43 AM, Jacopo Mondi wrote:
In MIPI mode the frame rate control is performed by adjusting the
frame blankings and the s_frame_interval function is not used anymore.
Only check for the per-mode supported frame rate in DVP mode and do not
restrict MIPI mode.
Disallow enum/s/g_frame_interval if the chip is used in MIPI mode.
This is breaking userspace which set framerate through media-ctl:
media-ctl -d /dev/media0 --set-v4l2 "'ov5640
1-003c':0[fmt:JPEG_1X8/640x480@1/15 field:none]"
because of unsupported framerate, all the rest is ignored (resolution
and format).
I can understand use of vblank to tune framerate but I would expect that
compatibility with frame interval setting is kept, it's far more simple
for an application to set the frame interval versus finding the right
vblank to apply (not straightforward...)
On my side I have reverted this patch and added support of both, see
patch proposal in reply to [PATCH v5 16/27] media: ov5640: Add VBLANK
control.
While at it re-indent one function which whose parameters were wrongly
aligned.
Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
drivers/media/i2c/ov5640.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index baf368a39e0f..6b955163eb4d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2005,9 +2005,14 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
(mode->width != width || mode->height != height)))
return NULL;
- /* Check to see if the current mode exceeds the max frame rate */
+ /*
+ * Check to see if the current mode exceeds the max frame rate.
+ * Only DVP mode uses the frame rate set by s_frame_interval, MIPI
+ * mode controls framerate by setting blankings.
+ */
timings = &mode->dvp_timings;
- if (ov5640_framerates[fr] > ov5640_framerates[timings->max_fps])
+ if (!ov5640_is_csi2(sensor) &&
+ ov5640_framerates[fr] > ov5640_framerates[timings->max_fps])
return NULL;
return mode;
@@ -3439,6 +3444,9 @@ static int ov5640_enum_frame_interval(
struct v4l2_fract tpf;
int ret;
+ if (ov5640_is_csi2(sensor))
+ return -EINVAL;
+
if (fie->pad != 0)
return -EINVAL;
if (fie->index >= OV5640_NUM_FRAMERATES)
@@ -3461,6 +3469,9 @@ static int ov5640_g_frame_interval(struct v4l2_subdev *sd,
{
struct ov5640_dev *sensor = to_ov5640_dev(sd);
+ if (ov5640_is_csi2(sensor))
+ return -EINVAL;
+
mutex_lock(&sensor->lock);
fi->interval = sensor->frame_interval;
mutex_unlock(&sensor->lock);
@@ -3475,6 +3486,9 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
const struct ov5640_mode_info *mode;
int frame_rate, ret = 0;
+ if (ov5640_is_csi2(sensor))
+ return -EINVAL;
+
if (fi->pad != 0)
return -EINVAL;
