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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
> ---
> 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
>
