Re: [PATCH v2 2/2] media: i2c: add support for ov4689

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

 



Hi Dave,

and thanks for reviewing this!

On 2022-09-22 at 11:54 +01, Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote:

> Hi Mikhail
>
> On Sun, 11 Sept 2022 at 21:02, Mikhail Rudenko <mike.rudenko@xxxxxxxxx> wrote:
>>
>> Add a V4L2 sub-device driver for OmniVision OV4689 image sensor. This
>> is a 4 Mpx image sensor using the I2C bus for control and the CSI-2
>> bus for data.
>>
>> This driver supports following features:
>> - manual exposure and analog gain control support
>> - test pattern support
>> - media controller support
>> - runtime PM support
>> - support following resolutions:
>>   + 2688x1520 at 30 fps
>>
>> The driver provides all mandatory V4L2 controls for compatibility with
>> libcamera. The sensor supports 1/2/4-lane CSI-2 modes, but the driver
>> implements 4 lane mode only at this moment.
>>
>> Signed-off-by: Mikhail Rudenko <mike.rudenko@xxxxxxxxx>
>> ---
>>  MAINTAINERS                |   1 +
>>  drivers/media/i2c/Kconfig  |  14 +
>>  drivers/media/i2c/Makefile |   1 +
>>  drivers/media/i2c/ov4689.c | 951 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 967 insertions(+)
>>  create mode 100644 drivers/media/i2c/ov4689.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 63c4844f26e6..1857f3864e1b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -14529,6 +14529,7 @@ L:      linux-media@xxxxxxxxxxxxxxx
>>  S:     Maintained
>>  T:     git git://linuxtv.org/media_tree.git
>>  F:     Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> +F:     drivers/media/i2c/ov5647.c
>>
>>  OMNIVISION OV5640 SENSOR DRIVER
>>  M:     Steve Longerbeam <slongerbeam@xxxxxxxxx>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index fae2baabb773..4993e1ae2ea8 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -429,6 +429,20 @@ config VIDEO_OV2740
>>           To compile this driver as a module, choose M here: the
>>           module will be called ov2740.
>>
>> +config VIDEO_OV4689
>> +       tristate "OmniVision OV4689 sensor support"
>> +       depends on OF
>> +       depends on GPIOLIB && VIDEO_DEV && I2C
>> +       select MEDIA_CONTROLLER
>> +       select VIDEO_V4L2_SUBDEV_API
>> +       select V4L2_FWNODE
>> +       help
>> +         This is a Video4Linux2 sensor-level driver for the OmniVision
>> +         OV4689 camera.
>> +
>> +         To compile this driver as a module, choose M here: the
>> +         module will be called ov4689.
>> +
>>  config VIDEO_OV5640
>>         tristate "OmniVision OV5640 sensor support"
>>         depends on OF
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 3e1696963e7f..7446c0a1eed0 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -78,6 +78,7 @@ obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
>>  obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
>>  obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
>>  obj-$(CONFIG_VIDEO_OV2740) += ov2740.o
>> +obj-$(CONFIG_VIDEO_OV4689) += ov4689.o
>>  obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
>>  obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
>>  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
>> new file mode 100644
>> index 000000000000..9f05e812acf8
>> --- /dev/null
>> +++ b/drivers/media/i2c/ov4689.c
>> @@ -0,0 +1,951 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * ov4689 driver
>> + *
>> + * Copyright (C) 2017 Fuzhou Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <media/media-entity.h>
>> +#include <media/v4l2-async.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-subdev.h>
>> +#include <media/v4l2-fwnode.h>
>> +
>> +#define CHIP_ID                                0x004688
>> +#define OV4689_REG_CHIP_ID             0x300a
>> +
>> +#define OV4689_XVCLK_FREQ              24000000
>> +
>> +#define OV4689_REG_CTRL_MODE           0x0100
>> +#define OV4689_MODE_SW_STANDBY         0x0
>> +#define OV4689_MODE_STREAMING          BIT(0)
>> +
>> +#define OV4689_REG_EXPOSURE            0x3500
>> +#define OV4689_EXPOSURE_MIN            4
>> +#define OV4689_EXPOSURE_STEP           1
>> +#define OV4689_VTS_MAX                 0x7fff
>> +
>> +#define OV4689_REG_GAIN_H              0x3508
>> +#define OV4689_REG_GAIN_L              0x3509
>> +#define OV4689_GAIN_H_MASK             0x07
>> +#define OV4689_GAIN_H_SHIFT            8
>> +#define OV4689_GAIN_L_MASK             0xff
>> +#define OV4689_GAIN_MIN                        0x10
>> +#define OV4689_GAIN_MAX                        0xf8
>> +#define OV4689_GAIN_STEP               1
>> +#define OV4689_GAIN_DEFAULT            0x10
>> +
>> +#define OV4689_REG_TEST_PATTERN                0x5040
>> +#define OV4689_TEST_PATTERN_ENABLE     0x80
>> +#define OV4689_TEST_PATTERN_DISABLE    0x0
>> +
>> +#define OV4689_REG_VTS                 0x380e
>> +
>> +#define REG_NULL                       0xFFFF
>> +
>> +#define OV4689_REG_VALUE_08BIT         1
>> +#define OV4689_REG_VALUE_16BIT         2
>> +#define OV4689_REG_VALUE_24BIT         3
>> +
>> +#define OV4689_LANES                   4
>> +#define OV4689_BITS_PER_SAMPLE         10
>> +
>> +static const char *const ov4689_supply_names[] = {
>> +       "avdd", /* Analog power */
>> +       "dovdd", /* Digital I/O power */
>> +       "dvdd", /* Digital core power */
>> +};
>> +
>> +#define OV4689_NUM_SUPPLIES ARRAY_SIZE(ov4689_supply_names)
>> +
>> +struct regval {
>> +       u16 addr;
>> +       u8 val;
>> +};
>> +
>> +struct ov4689_mode {
>> +       u32 width;
>> +       u32 height;
>> +       u32 max_fps;
>> +       u32 hts_def;
>> +       u32 vts_def;
>> +       u32 exp_def;
>> +       const struct regval *reg_list;
>> +};
>> +
>> +struct ov4689 {
>> +       struct i2c_client *client;
>> +       struct clk *xvclk;
>> +       struct gpio_desc *reset_gpio;
>> +       struct gpio_desc *pwdn_gpio;
>> +       struct regulator_bulk_data supplies[OV4689_NUM_SUPPLIES];
>> +
>> +       struct v4l2_subdev subdev;
>> +       struct media_pad pad;
>> +
>> +       struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */
>> +       bool streaming;
>> +       struct v4l2_ctrl_handler ctrl_handler;
>> +       struct v4l2_ctrl *exposure;
>> +       struct v4l2_ctrl *anal_gain;
>> +       struct v4l2_ctrl *digi_gain;
>> +       struct v4l2_ctrl *hblank;
>> +       struct v4l2_ctrl *vblank;
>> +       struct v4l2_ctrl *test_pattern;
>> +
>> +       const struct ov4689_mode *cur_mode;
>> +};
>> +
>> +#define to_ov4689(sd) container_of(sd, struct ov4689, subdev)
>> +
>> +/*
>> + * Xclk 24Mhz
>> + */
>> +static const struct regval ov4689_global_regs[] = {
>> +       { REG_NULL, 0x00 },
>> +};
>> +
>> +/*
>> + * Xclk 24Mhz
>> + * max_framerate 30fps
>> + * mipi_datarate per lane 1008Mbps
>
> Data rate stated as 1008Mbps here....
>
>> + */
>> +static const struct regval ov4689_2688x1520_regs[] = {
>> +       {0x0103, 0x01}, {0x3638, 0x00}, {0x0300, 0x00},
>> +       {0x0302, 0x2a}, {0x0303, 0x00}, {0x0304, 0x03},
>> +       {0x030b, 0x00}, {0x030d, 0x1e}, {0x030e, 0x04},
>> +       {0x030f, 0x01}, {0x0312, 0x01}, {0x031e, 0x00},
>> +       {0x3000, 0x20}, {0x3002, 0x00}, {0x3018, 0x72},
>> +       {0x3020, 0x93}, {0x3021, 0x03}, {0x3022, 0x01},
>> +       {0x3031, 0x0a}, {0x303f, 0x0c}, {0x3305, 0xf1},
>> +       {0x3307, 0x04}, {0x3309, 0x29}, {0x3500, 0x00},
>> +       {0x3501, 0x60}, {0x3502, 0x00}, {0x3503, 0x04},
>> +       {0x3504, 0x00}, {0x3505, 0x00}, {0x3506, 0x00},
>> +       {0x3507, 0x00}, {0x3508, 0x00}, {0x3509, 0x80},
>> +       {0x350a, 0x00}, {0x350b, 0x00}, {0x350c, 0x00},
>> +       {0x350d, 0x00}, {0x350e, 0x00}, {0x350f, 0x80},
>> +       {0x3510, 0x00}, {0x3511, 0x00}, {0x3512, 0x00},
>> +       {0x3513, 0x00}, {0x3514, 0x00}, {0x3515, 0x80},
>> +       {0x3516, 0x00}, {0x3517, 0x00}, {0x3518, 0x00},
>> +       {0x3519, 0x00}, {0x351a, 0x00}, {0x351b, 0x80},
>> +       {0x351c, 0x00}, {0x351d, 0x00}, {0x351e, 0x00},
>> +       {0x351f, 0x00}, {0x3520, 0x00}, {0x3521, 0x80},
>> +       {0x3522, 0x08}, {0x3524, 0x08}, {0x3526, 0x08},
>> +       {0x3528, 0x08}, {0x352a, 0x08}, {0x3602, 0x00},
>> +       {0x3603, 0x40}, {0x3604, 0x02}, {0x3605, 0x00},
>> +       {0x3606, 0x00}, {0x3607, 0x00}, {0x3609, 0x12},
>> +       {0x360a, 0x40}, {0x360c, 0x08}, {0x360f, 0xe5},
>> +       {0x3608, 0x8f}, {0x3611, 0x00}, {0x3613, 0xf7},
>> +       {0x3616, 0x58}, {0x3619, 0x99}, {0x361b, 0x60},
>> +       {0x361c, 0x7a}, {0x361e, 0x79}, {0x361f, 0x02},
>> +       {0x3632, 0x00}, {0x3633, 0x10}, {0x3634, 0x10},
>> +       {0x3635, 0x10}, {0x3636, 0x15}, {0x3646, 0x86},
>> +       {0x364a, 0x0b}, {0x3700, 0x17}, {0x3701, 0x22},
>> +       {0x3703, 0x10}, {0x370a, 0x37}, {0x3705, 0x00},
>> +       {0x3706, 0x63}, {0x3709, 0x3c}, {0x370b, 0x01},
>> +       {0x370c, 0x30}, {0x3710, 0x24}, {0x3711, 0x0c},
>> +       {0x3716, 0x00}, {0x3720, 0x28}, {0x3729, 0x7b},
>> +       {0x372a, 0x84}, {0x372b, 0xbd}, {0x372c, 0xbc},
>> +       {0x372e, 0x52}, {0x373c, 0x0e}, {0x373e, 0x33},
>> +       {0x3743, 0x10}, {0x3744, 0x88}, {0x3745, 0xc0},
>> +       {0x374a, 0x43}, {0x374c, 0x00}, {0x374e, 0x23},
>> +       {0x3751, 0x7b}, {0x3752, 0x84}, {0x3753, 0xbd},
>> +       {0x3754, 0xbc}, {0x3756, 0x52}, {0x375c, 0x00},
>> +       {0x3760, 0x00}, {0x3761, 0x00}, {0x3762, 0x00},
>> +       {0x3763, 0x00}, {0x3764, 0x00}, {0x3767, 0x04},
>> +       {0x3768, 0x04}, {0x3769, 0x08}, {0x376a, 0x08},
>> +       {0x376b, 0x20}, {0x376c, 0x00}, {0x376d, 0x00},
>> +       {0x376e, 0x00}, {0x3773, 0x00}, {0x3774, 0x51},
>> +       {0x3776, 0xbd}, {0x3777, 0xbd}, {0x3781, 0x18},
>> +       {0x3783, 0x25}, {0x3798, 0x1b}, {0x3800, 0x00},
>> +       {0x3801, 0x08}, {0x3802, 0x00}, {0x3803, 0x04},
>> +       {0x3804, 0x0a}, {0x3805, 0x97}, {0x3806, 0x05},
>> +       {0x3807, 0xfb}, {0x3808, 0x0a}, {0x3809, 0x80},
>> +       {0x380a, 0x05}, {0x380b, 0xf0}, {0x380c, 0x0a},
>> +       {0x380d, 0x80}, {0x380e, 0x06}, {0x380f, 0x12},
>> +       {0x3810, 0x00}, {0x3811, 0x08}, {0x3812, 0x00},
>> +       {0x3813, 0x04}, {0x3814, 0x01}, {0x3815, 0x01},
>> +       {0x3819, 0x01}, {0x3820, 0x00}, {0x3821, 0x06},
>> +       {0x3829, 0x00}, {0x382a, 0x01}, {0x382b, 0x01},
>> +       {0x382d, 0x7f}, {0x3830, 0x04}, {0x3836, 0x01},
>> +       {0x3837, 0x00}, {0x3841, 0x02}, {0x3846, 0x08},
>> +       {0x3847, 0x07}, {0x3d85, 0x36}, {0x3d8c, 0x71},
>> +       {0x3d8d, 0xcb}, {0x3f0a, 0x00}, {0x4000, 0xf1},
>> +       {0x4001, 0x40}, {0x4002, 0x04}, {0x4003, 0x14},
>> +       {0x400e, 0x00}, {0x4011, 0x00}, {0x401a, 0x00},
>> +       {0x401b, 0x00}, {0x401c, 0x00}, {0x401d, 0x00},
>> +       {0x401f, 0x00}, {0x4020, 0x00}, {0x4021, 0x10},
>> +       {0x4022, 0x07}, {0x4023, 0xcf}, {0x4024, 0x09},
>> +       {0x4025, 0x60}, {0x4026, 0x09}, {0x4027, 0x6f},
>> +       {0x4028, 0x00}, {0x4029, 0x02}, {0x402a, 0x06},
>> +       {0x402b, 0x04}, {0x402c, 0x02}, {0x402d, 0x02},
>> +       {0x402e, 0x0e}, {0x402f, 0x04}, {0x4302, 0xff},
>> +       {0x4303, 0xff}, {0x4304, 0x00}, {0x4305, 0x00},
>> +       {0x4306, 0x00}, {0x4308, 0x02}, {0x4500, 0x6c},
>> +       {0x4501, 0xc4}, {0x4502, 0x40}, {0x4503, 0x01},
>> +       {0x4601, 0xa7}, {0x4800, 0x04}, {0x4813, 0x08},
>> +       {0x481f, 0x40}, {0x4829, 0x78}, {0x4837, 0x10},
>> +       {0x4b00, 0x2a}, {0x4b0d, 0x00}, {0x4d00, 0x04},
>> +       {0x4d01, 0x42}, {0x4d02, 0xd1}, {0x4d03, 0x93},
>> +       {0x4d04, 0xf5}, {0x4d05, 0xc1}, {0x5000, 0xf3},
>> +       {0x5001, 0x11}, {0x5004, 0x00}, {0x500a, 0x00},
>> +       {0x500b, 0x00}, {0x5032, 0x00}, {0x5040, 0x00},
>> +       {0x5050, 0x0c}, {0x5500, 0x00}, {0x5501, 0x10},
>> +       {0x5502, 0x01}, {0x5503, 0x0f}, {0x8000, 0x00},
>> +       {0x8001, 0x00}, {0x8002, 0x00}, {0x8003, 0x00},
>> +       {0x8004, 0x00}, {0x8005, 0x00}, {0x8006, 0x00},
>> +       {0x8007, 0x00}, {0x8008, 0x00}, {0x3638, 0x00},
>> +       {REG_NULL, 0x00},
>> +};
>> +
>> +static const struct ov4689_mode supported_modes[] = {
>> +       {
>> +               .width = 2688,
>> +               .height = 1520,
>> +               .max_fps = 30,
>> +               .exp_def = 0x0600,
>> +               .hts_def = 0x0a80,
>> +               .vts_def = 0x0612,
>> +               .reg_list = ov4689_2688x1520_regs,
>> +       },
>> +};
>> +
>> +#define OV4689_LINK_FREQ_500MHZ 500000000
>> +static const s64 link_freq_menu_items[] = { OV4689_LINK_FREQ_500MHZ };
>
> ... but a link frequency of 500MHz (ie 1000Mbit/s) here.
> Seeing as you compute the pixel rate based on the link frequency,
> that's going to mean that the pixel rate is incorrect.
> Link frequency should be 504MHz.
>
> Your PLL settings appear to match the 24MHz configuration in table
> 2-11 "sample PLL configuration" of the datasheet, so it would confirm
> that MIPI_SCLK is 1008MHz and MIPI_PCLK is 126MHz (at 1008/8 seems to
> be more byte clock than pixel (10bpp) clock).

Those are leftovers from the bsp driver, and they will be definitely
changed in v3. Recovering correct pixel clock, hblank and vblank
matching the observed fps needs some experimentation, which I hope to
carry out over the weekend.

>
>   Dave
>

--
Best regards,
Mikhail



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux