Hi Jacopo,
Thank you for the patch.
On Mon, Jan 31, 2022 at 03:32:33PM +0100, Jacopo Mondi wrote:
> The ov5640_mode_init_data is a fictional sensor more which is used to
> program the initial sensor settings.
>
> It is only used to initialize the sensor and can be replaced
> it with a throw-away mode which just wraps the register table.
>
> Also rename the register table to drop the format from the name to make
> it clear an actual sensor mode has to be applied after the initial
> programming.
>
> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> ---
> drivers/media/i2c/ov5640.c | 32 +++++---------------------------
> 1 file changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index d966cca78e92..1e2f37c93f0d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -348,7 +348,7 @@ static inline bool ov5640_is_mipi(struct ov5640_dev *sensor)
> * over i2c.
> */
> /* YUV422 UYVY VGA@30fps */
> -static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
> +static const struct reg_value ov5640_init_setting[] = {
> {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
> {0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
> {0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
> @@ -586,30 +586,6 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
> {0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 70},
> };
>
> -/* power-on sensor init reg table */
> -static const struct ov5640_mode_info ov5640_mode_init_data = {
> - .id = 0,
> - .dn_mode = SUBSAMPLING,
> - .pixel_rate = OV5640_PIXEL_RATE_96M,
> - .analog_crop = {
> - .left = 0,
> - .top = 4,
> - .width = 2623,
> - .height = 1947,
> - },
> - .crop = {
> - .left = 16,
> - .top = 6,
> - .width = 640,
> - .height = 480,
> - },
> - .ppl = 1896,
> - .vblank_def = 504,
> - .reg_data = ov5640_init_setting_30fps_VGA,
> - .reg_data_size = ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> - .max_fps = OV5640_30_FPS
> -};
> -
> static const struct ov5640_mode_info
> ov5640_mode_data[OV5640_NUM_MODES] = {
> {
> @@ -2117,13 +2093,15 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
> /* restore the last set video mode after chip power-on */
> static int ov5640_restore_mode(struct ov5640_dev *sensor)
> {
> + struct ov5640_mode_info init_data = {};
> int ret;
>
> /* first load the initial register values */
> - ret = ov5640_load_regs(sensor, &ov5640_mode_init_data);
> + init_data.reg_data = ov5640_init_setting;
> + init_data.reg_data_size = ARRAY_SIZE(ov5640_init_setting);
> + ret = ov5640_load_regs(sensor, &init_data);
Could we change ov5640_load_regs() to take a reg_value array and size as
parameters, to avoid the local init_data variable ?
> if (ret < 0)
> return ret;
> - sensor->last_mode = &ov5640_mode_init_data;
>
> ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
> (ilog2(OV5640_SCLK2X_ROOT_DIV) << 2) |
--
Regards,
Laurent Pinchart
