Re: [PATCH 3/5] media: atomisp: ov2680: Remove unnecessary registers from ov2680_global_setting[]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 4, 2023 at 7:14 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Many of the values in ov2680_global_setting[] match the default/reset
> register value for the ov2680 sensor (verified with both datasheet
> and actual hw) so they are no-ops.
>
> And there are also a coupe of others which are later overwritten
> by ctrls or by ov2680_set_mode().
>
> Remove all the unnecessary entries and add annotations to
> the remaining entries documenting what they change
> (in sofar as things are documented in the datasheet).

My spell checker suggests either "in so far as" or "insofar as".

> This also removes the double writing of OV2680_REG_SOFT_RESET in
> ov2680_init_registers() (one direct write, one in ov2680_global_setting[])
> instead add a short sleep after the first write to give the sensor
> time to reset.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  .../media/atomisp/i2c/atomisp-ov2680.c        |   1 +
>  drivers/staging/media/atomisp/i2c/ov2680.h    | 117 ++++++++----------
>  2 files changed, 56 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index 1db2eb9f9e25..dcc06c725544 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -175,6 +175,7 @@ static int ov2680_init_registers(struct v4l2_subdev *sd)
>         int ret;
>
>         ret = ov_write_reg8(client, OV2680_SW_RESET, 0x01);
> +       usleep_range(1000, 2000);

Perhaps surround this with blank lines and s little comment?
Also does this sleep is anyhow required by a datasheet, or is it
purely empirical?

>         ret |= ov2680_write_reg_array(client, ov2680_global_setting);

Side note: this is weird for an int returned variable.

>         return ret;
> diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
> index b6c0ef591c69..6a71de55600b 100644
> --- a/drivers/staging/media/atomisp/i2c/ov2680.h
> +++ b/drivers/staging/media/atomisp/i2c/ov2680.h
> @@ -172,82 +172,75 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
>  }
>
>  static struct ov2680_reg const ov2680_global_setting[] = {
> -       {0x0103, 0x01},
> -       {0x3002, 0x00},
> +       /* MIPI PHY, 0x10 -> 0x1c enable bp_c_hs_en_lat and bp_d_hs_en_lat */
>         {0x3016, 0x1c},
> -       {0x3018, 0x44},
> -       {0x3020, 0x00},
> -       {0x3080, 0x02},
> +
> +       /* PLL MULT bits 0-7, datasheet default 0x37 for 24MHz extclk, use 0x45 for 19.2 Mhz extclk */
>         {0x3082, 0x45},
> -       {0x3084, 0x09},
> -       {0x3085, 0x04},
> -       {0x3086, 0x00},
> +
> +       /* R MANUAL set exposure (0x01) and gain (0x02) to manual (hw does not do auto) */
>         {0x3503, 0x03},
> -       {0x350b, 0x36},
> -       {0x3600, 0xb4},
> -       {0x3603, 0x39},
> -       {0x3604, 0x24},
> -       {0x3605, 0x00},
> -       {0x3620, 0x26},
> -       {0x3621, 0x37},
> -       {0x3622, 0x04},
> -       {0x3628, 0x00},
> -       {0x3705, 0x3c},
> -       {0x370c, 0x50},
> -       {0x370d, 0xc0},
> -       {0x3718, 0x88},
> -       {0x3720, 0x00},
> -       {0x3721, 0x00},
> -       {0x3722, 0x00},
> -       {0x3723, 0x00},
> -       {0x3738, 0x00},
> -       {0x3717, 0x58},
> -       {0x3781, 0x80},
> -       {0x3789, 0x60},
> -       {0x3800, 0x00},
> -       {0x3819, 0x04},
> +
> +       /* Analog control register tweaks */
> +       {0x3603, 0x39}, /* Reset value 0x99 */
> +       {0x3604, 0x24}, /* Reset value 0x74 */
> +       {0x3621, 0x37}, /* Reset value 0x44 */
> +
> +       /* Sensor control register tweaks */
> +       {0x3701, 0x64}, /* Reset value 0x61 */
> +       {0x3705, 0x3c}, /* Reset value 0x21 */
> +       {0x370c, 0x50}, /* Reset value 0x10 */
> +       {0x370d, 0xc0}, /* Reset value 0x00 */
> +       {0x3718, 0x88}, /* Reset value 0x80 */
> +
> +       /* PSRAM tweaks */
> +       {0x3781, 0x80}, /* Reset value 0x00 */
> +       {0x3784, 0x0c}, /* Reset value 0x00, based on OV2680_R1A_AM10.ovt */
> +       {0x3789, 0x60}, /* Reset value 0x50 */
> +
> +       /* BLC CTRL00 0x01 -> 0x81 set avg_weight to 8 */
>         {0x4000, 0x81},
> -       {0x4001, 0x40},
> +
> +       /* Set black level compensation range to 0 - 3 (default 0 - 11) */
>         {0x4008, 0x00},
>         {0x4009, 0x03},
> +
> +       /* VFIFO R2 0x00 -> 0x02 set Frame reset enable */
>         {0x4602, 0x02},
> +
> +       /* MIPI ctrl CLK PREPARE MIN change from 0x26 (38) -> 0x36 (54) */
>         {0x481f, 0x36},
> +
> +       /* MIPI ctrl CLK LPX P MIN change from 0x32 (50) -> 0x36 (54) */
>         {0x4825, 0x36},
> -       {0x4837, 0x18},
> +
> +       /* R ISP CTRL2 0x20 -> 0x30, set sof_sel bit */
>         {0x5002, 0x30},
> -       {0x5004, 0x04},//manual awb 1x
> -       {0x5005, 0x00},
> -       {0x5006, 0x04},
> -       {0x5007, 0x00},
> -       {0x5008, 0x04},
> -       {0x5009, 0x00},
> -       {0x5080, 0x00},
> -       {0x5081, 0x41},
> -       {0x5708, 0x01},  /* add for full size flip off and mirror off 2014/09/11 */
> -       {0x3701, 0x64},  //add on 14/05/13
> -       {0x3784, 0x0c},  //based OV2680_R1A_AM10.ovt add on 14/06/13
> -       {0x5780, 0x3e},  //based OV2680_R1A_AM10.ovt,Adjust DPC setting (57xx) on 14/06/13
> -       {0x5781, 0x0f},
> -       {0x5782, 0x04},
> -       {0x5783, 0x02},
> -       {0x5784, 0x01},
> -       {0x5785, 0x01},
> -       {0x5786, 0x00},
> -       {0x5787, 0x04},
> +
> +       /*
> +        * Window CONTROL 0x00 -> 0x01, enable manual window control,
> +        * this is necessary for full size flip and mirror support.
> +        */
> +       {0x5708, 0x01},
> +
> +       /*
> +        * DPC CTRL0 0x14 -> 0x3e, set enable_tail, enable_3x3_cluster
> +        * and enable_general_tail bits based OV2680_R1A_AM10.ovt.
> +        */
> +       {0x5780, 0x3e},
> +
> +       /* DPC MORE CONNECTION CASE THRE 0x0c (12) -> 0x02 (2) */
>         {0x5788, 0x02},
> -       {0x5789, 0x00},
> -       {0x578a, 0x01},
> -       {0x578b, 0x02},
> -       {0x578c, 0x03},
> -       {0x578d, 0x03},
> +
> +       /* DPC GAIN LIST1 0x0f (15) -> 0x08 (8) */
>         {0x578e, 0x08},
> +
> +       /* DPC GAIN LIST2 0x3f (63) -> 0x0c (12) */
>         {0x578f, 0x0c},
> -       {0x5790, 0x08},
> -       {0x5791, 0x04},
> +
> +       /* DPC THRE RATIO 0x04 (4) -> 0x00 (0) */
>         {0x5792, 0x00},
> -       {0x5793, 0x00},
> -       {0x5794, 0x03}, //based OV2680_R1A_AM10.ovt,Adjust DPC setting (57xx) on 14/06/13
> -       {0x0100, 0x00}, //stream off
> +
>         {}
>  };
>
> --
> 2.40.1
>


-- 
With Best Regards,
Andy Shevchenko





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux