Hi Jacopo,
Thank you for the patch.
On Thu, Feb 10, 2022 at 12:10:43PM +0100, Jacopo Mondi wrote:
> The ov5640 driver supports different sizes for different mbus_codes.
> In particular:
>
> - 8bpp modes: high resolution sizes (>= 1280x720)
> - 16bpp modes: all sizes
> - 24bpp modes: low resolutions sizes (< 1280x720)
>
> Restrict the frame sizes enumerations to the above constraints.
>
> While at it, make sure the fse->mbus_code parameter is valid, and return
> -EINVAL if it's not.
>
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> ---
> drivers/media/i2c/ov5640.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index f894570ac53c..81bf547a923e 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -3315,14 +3315,28 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_frame_size_enum *fse)
> {
> + u32 bpp = ov5640_code_to_bpp(fse->code);
> + unsigned int index = fse->index;
> +
> if (fse->pad != 0)
> return -EINVAL;
> - if (fse->index >= OV5640_NUM_MODES)
> + if (!bpp)
> + return -EINVAL;
> +
> + /* Only low-resolution modes are supported for 24bpp modes. */
s/24bpp modes/24bpp formats/
Do you think this is a limitation of the sensor, or an issue in the
driver ? In the latter case, I'd write
/* FIXME: High resolution modes don't work in 24bpp formats. */
> + if (bpp == 24 && index >= OV5640_MODE_720P_1280_720)
> + return -EINVAL;
> +
> + /* Only high-resolutions modes are supported for 8bpp formats. */
And here
/* FIXME: Low resolution modes don't work in 8bpp formats. */
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + if (bpp == 8)
> + index += OV5640_MODE_720P_1280_720;
> +
> + if (index >= OV5640_NUM_MODES)
> return -EINVAL;
>
> - fse->min_width = ov5640_mode_data[fse->index].crop.width;
> + fse->min_width = ov5640_mode_data[index].crop.width;
> fse->max_width = fse->min_width;
> - fse->min_height = ov5640_mode_data[fse->index].crop.height;
> + fse->min_height = ov5640_mode_data[index].crop.height;
> fse->max_height = fse->min_height;
>
> return 0;
--
Regards,
Laurent Pinchart
