| Message ID | 20230831135747.23148-4-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx219: Miscellaneous fixes | expand |
Hi Laurent On Thu, Aug 31, 2023 at 04:57:47PM +0300, Laurent Pinchart wrote: > The .set_fmt() handler tries to avoid updating the sensor configuration > when the mode hasn't changed. It does so by comparing both the mode and > the media bus code. While the latter correctly uses the media bus code > stored in the subdev state, the former compares the mode being set with > the active mode, regardless of whether .set_fmt() is called for the > ACTIVE or TRY format. This can lead to .set_fmt() returning early when > operating on TRY formats. Ah yes indeed! My bad as I introduced this in the latest series > > This could be fixed by replacing the mode comparison with width and > height comparisons, using the frame size stored in the subdev state. > However, the optimization that avoids updates to the sensor > configuration is not very useful, and is not commonly found in sensor > drivers. To improve consistency across sensor drivers, it is better, in > addition to being easier, to simply drop it. Do so. Agreed this is better dropped ! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > > Fixes: e8a5b1df000e ("media: i2c: imx219: Use subdev active state") Can we make sure this is collected together with my previous series in v6.6 ? > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx219.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index f19c828b6943..ec53abe2e84e 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -762,9 +762,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, > format = v4l2_subdev_get_pad_format(sd, sd_state, 0); > crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0); > > - if (imx219->mode == mode && format->code == fmt->format.code) > - return 0; > - > *format = fmt->format; > *crop = mode->crop; > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index f19c828b6943..ec53abe2e84e 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -762,9 +762,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, format = v4l2_subdev_get_pad_format(sd, sd_state, 0); crop = v4l2_subdev_get_pad_crop(sd, sd_state, 0); - if (imx219->mode == mode && format->code == fmt->format.code) - return 0; - *format = fmt->format; *crop = mode->crop;
The .set_fmt() handler tries to avoid updating the sensor configuration when the mode hasn't changed. It does so by comparing both the mode and the media bus code. While the latter correctly uses the media bus code stored in the subdev state, the former compares the mode being set with the active mode, regardless of whether .set_fmt() is called for the ACTIVE or TRY format. This can lead to .set_fmt() returning early when operating on TRY formats. This could be fixed by replacing the mode comparison with width and height comparisons, using the frame size stored in the subdev state. However, the optimization that avoids updates to the sensor configuration is not very useful, and is not commonly found in sensor drivers. To improve consistency across sensor drivers, it is better, in addition to being easier, to simply drop it. Do so. Fixes: e8a5b1df000e ("media: i2c: imx219: Use subdev active state") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/imx219.c | 3 --- 1 file changed, 3 deletions(-)