On Thu, Feb 03, 2022 at 12:38:12AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Mon, Jan 31, 2022 at 03:44:43PM +0100, 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. > > This certainly aligns better with how the sensor driver is supposed to > operate. I however wonder why you don't do so in DVP mode too. Is it for > backward-compatibility ? If so a comment would be useful. yes, and mostly because DVP mode seems to work well and I didn't want to change the way subdev is operated for DVP users. I would be more than happy to remove frame_interval completely. > > > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > --- > > drivers/media/i2c/ov5640.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c > > index ae22300b9655..ec46e16223af 100644 > > --- a/drivers/media/i2c/ov5640.c > > +++ b/drivers/media/i2c/ov5640.c > > @@ -1845,8 +1845,13 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr, > > (mode->crop.width != width || mode->crop.height != height))) > > return NULL; > > > > - /* Check to see if the current mode exceeds the max frame rate */ > > - if (ov5640_framerates[fr] > ov5640_framerates[mode->max_fps]) > > + /* > > + * 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. > > + */ > > + if (!ov5640_is_mipi(sensor) && > > + ov5640_framerates[fr] > ov5640_framerates[mode->max_fps]) > > return NULL; > > > > return mode; > > -- > Regards, > > Laurent Pinchart
