| Message ID | 20240429051333.1306453-1-dongcheng.yan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] media: i2c: Add ar0234 camera sensor driver | expand |
Hi Dongcheng Thanks for the patch. As I've previously looked at this sensor, and had most of a driver working [1], there are a couple of things that immediately stand out to me in this patch. [1] https://github.com/6by9/linux/blob/rpi-6.4.y-ar0234/drivers/media/i2c/ar0234.c On Mon, 29 Apr 2024 at 06:14, Dongcheng Yan <dongcheng.yan@intel.com> wrote: > > The driver is implemented with V4L2 framework, > and supports following features: > > - manual exposure and analog/digital gain control > - vblank/hblank control > - runtime PM support > - 1280x960 at 30FPS > > Signed-off-by: Dongcheng Yan <dongcheng.yan@intel.com> > --- > drivers/media/i2c/Kconfig | 11 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/ar0234.c | 1053 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 1065 insertions(+) > create mode 100644 drivers/media/i2c/ar0234.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 56f276b920ab..4f50a489cfcc 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -51,6 +51,17 @@ config VIDEO_ALVIUM_CSI2 > To compile this driver as a module, choose M here: the > module will be called alvium-csi2. > > +config VIDEO_AR0234 > + tristate "ON Semiconductor AR0234 sensor support" > + depends on ACPI || COMPILE_TEST > + select V4L2_CCI_I2C > + help > + This is a Video4Linux2 sensor driver for the ON Semiconductor > + AR0234 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called ar0234. > + > config VIDEO_AR0521 > tristate "ON Semiconductor AR0521 sensor support" > help > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index dfbe6448b549..57b4f62106d9 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o > obj-$(CONFIG_VIDEO_AK881X) += ak881x.o > obj-$(CONFIG_VIDEO_ALVIUM_CSI2) += alvium-csi2.o > obj-$(CONFIG_VIDEO_APTINA_PLL) += aptina-pll.o > +obj-$(CONFIG_VIDEO_AR0234) += ar0234.o > obj-$(CONFIG_VIDEO_AR0521) += ar0521.o > obj-$(CONFIG_VIDEO_BT819) += bt819.o > obj-$(CONFIG_VIDEO_BT856) += bt856.o > diff --git a/drivers/media/i2c/ar0234.c b/drivers/media/i2c/ar0234.c > new file mode 100644 > index 000000000000..cdddd29d6469 > --- /dev/null > +++ b/drivers/media/i2c/ar0234.c > @@ -0,0 +1,1053 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2019 - 2024 Intel Corporation. > + > +#include <asm/unaligned.h> > +#include <linux/acpi.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > + > +#include <media/v4l2-cci.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-fwnode.h> > + > +#define PIXEL_RATE 180000000ULL > + > +/* Chip ID */ > +#define AR0234_REG_CHIP_ID CCI_REG16(0x3000) > +#define AR0234_CHIP_ID 0x0a56 > + > +#define AR0234_REG_MODE_SELECT CCI_REG16(0x301a) > +#define AR0234_MODE_RESET 0x00d9 > +#define AR0234_MODE_STANDBY 0x2058 > +#define AR0234_MODE_STREAMING 0x205c > + > +/* V_TIMING internal */ > +#define AR0234_REG_VTS CCI_REG16(0x300a) > +#define AR0234_VTS_MAX 0xffff > + > +/* Exposure control */ > +#define AR0234_REG_EXPOSURE CCI_REG16(0x3012) > +#define AR0234_EXPOSURE_MIN 0 > +#define AR0234_EXPOSURE_MAX_MARGIN 80 > +#define AR0234_EXPOSURE_STEP 1 > + > +/* Analog gain control */ > +#define AR0234_REG_ANALOG_GAIN CCI_REG16(0x3060) > +#define AR0234_ANAL_GAIN_MIN 0 > +#define AR0234_ANAL_GAIN_MAX 0x7f > +#define AR0234_ANAL_GAIN_STEP 1 > +#define AR0234_ANAL_GAIN_DEFAULT 0xe Default gain of 0x0e would be x1.77778. Any particular reason for not having the default as x1 ? > + > +/* Digital gain control */ > +#define AR0234_REG_GLOBAL_GAIN CCI_REG16(0x305e) > +#define AR0234_DGTL_GAIN_MIN 0 > +#define AR0234_DGTL_GAIN_MAX 0x7ff > +#define AR0234_DGTL_GAIN_STEP 1 > +#define AR0234_DGTL_GAIN_DEFAULT 0x80 > + > +#define AR0234_NATIVE_WIDTH 1280 > +#define AR0234_NATIVE_HEIGHT 960 AR0234 is a native 1920x1200 sensor at up to 120fps, not 1280x960 https://www.onsemi.com/products/sensors/image-sensors/ar0234cs > + > +#define to_ar0234(_sd) container_of(_sd, struct ar0234, sd) > + > +struct ar0234_reg_list { > + u32 num_of_regs; > + const struct cci_reg_sequence *regs; > +}; > + > +struct ar0234_link_freq_config { > + const struct ar0234_reg_list reg_list; > +}; > + > +struct ar0234_mode { > + u32 width; > + u32 height; > + > + u32 vts_def; > + u32 vts_min; > + > + u32 link_freq_index; > + u32 code; > + > + u32 hblank; > + u32 vblank; > + > + /* Sensor register settings for this mode */ > + const struct ar0234_reg_list reg_list; > +}; > + > +static const struct cci_reg_sequence link_freq_360M_1280x960_10bit_2lane[] = { This array is selected based on link frequency, but the name refers to the resolution. > + { CCI_REG16(0x302a), 0x0005 }, > + { CCI_REG16(0x302c), 0x0004 }, > + { CCI_REG16(0x302e), 0x0003 }, > + { CCI_REG16(0x3030), 0x0050 }, > + { CCI_REG16(0x3036), 0x000a }, > + { CCI_REG16(0x3038), 0x0002 }, > + { CCI_REG16(0x31b0), 0x006e }, > + { CCI_REG16(0x31b2), 0x0050 }, > + { CCI_REG16(0x31b4), 0x4207 }, > + { CCI_REG16(0x31b6), 0x2213 }, > + { CCI_REG16(0x31b8), 0x704a }, > + { CCI_REG16(0x31ba), 0x0289 }, > + { CCI_REG16(0x31bc), 0x8c08 }, > + { CCI_REG16(0x31ae), 0x0202 }, > + { CCI_REG16(0x3002), 0x0080 }, > + { CCI_REG16(0x3004), 0x0148 }, Y_START and X_START of 0x80,0x148 is an offset of 328, 128 (why did OnSemi put Y first?) > + { CCI_REG16(0x3006), 0x043f }, > + { CCI_REG16(0x3008), 0x0647 }, Y_END, X_END of 0x43f, 0x647, or more logically 1607,1087. 1087 is 113 pixels from the end of the 1200 lines of array, so we're not centre cropped as the left side is 128 lines. Likewise 1607 is 313 pixels from the end of the 1920 pixel width, so not centred horizontally either. More fundamentally, why are these registers in a table selected by link frequency? They should be in mode_1280x960_10bit_2lane. > + { CCI_REG16(0x300a), 0x05b5 }, Duplicates AR0234_REG_VTS controlled via V4L2_CID_VBLANK, so will be programmed via __v4l2_ctrl_handler_setup > + { CCI_REG16(0x300c), 0x0268 }, LINE_LENGTH_PCK aka HTS. 0x268 is decimal 616. My work said that the actual value is this x4, which is 2464. Your mode lists HTS as 3600 + 1280 = 4880, so the numbers seem inconsistent. > + { CCI_REG16(0x3012), 0x058c }, > + { CCI_REG16(0x31ac), 0x0a0a }, > + { CCI_REG16(0x306e), 0x9010 }, > + { CCI_REG16(0x30a2), 0x0001 }, > + { CCI_REG16(0x30a6), 0x0001 }, > + { CCI_REG16(0x3082), 0x0003 }, > + { CCI_REG16(0x3040), 0x0000 }, > + { CCI_REG16(0x31d0), 0x0000 }, > +}; > + > +static const struct cci_reg_sequence mode_1280x960_10bit_2lane[] = { > + { CCI_REG16(0x3f4c), 0x121f }, > + { CCI_REG16(0x3f4e), 0x121f }, > + { CCI_REG16(0x3f50), 0x0b81 }, > + { CCI_REG16(0x31e0), 0x0003 }, > + { CCI_REG16(0x30b0), 0x0028 }, > + /* R0X3088 specify the sequencer RAM access address. */ > + { CCI_REG16(0x3088), 0x8000 }, > + /* R0X3086 write the sequencer RAM. */ > + { CCI_REG16(0x3086), 0xc1ae }, > + { CCI_REG16(0x3086), 0x327f }, > + { CCI_REG16(0x3086), 0x5780 }, > + { CCI_REG16(0x3086), 0x272f }, > + { CCI_REG16(0x3086), 0x7416 }, > + { CCI_REG16(0x3086), 0x7e13 }, > + { CCI_REG16(0x3086), 0x8000 }, > + { CCI_REG16(0x3086), 0x307e }, > + { CCI_REG16(0x3086), 0xff80 }, > + { CCI_REG16(0x3086), 0x20c3 }, > + { CCI_REG16(0x3086), 0xb00e }, > + { CCI_REG16(0x3086), 0x8190 }, > + { CCI_REG16(0x3086), 0x1643 }, > + { CCI_REG16(0x3086), 0x1651 }, > + { CCI_REG16(0x3086), 0x9d3e }, > + { CCI_REG16(0x3086), 0x9545 }, > + { CCI_REG16(0x3086), 0x2209 }, > + { CCI_REG16(0x3086), 0x3781 }, > + { CCI_REG16(0x3086), 0x9016 }, > + { CCI_REG16(0x3086), 0x4316 }, > + { CCI_REG16(0x3086), 0x7f90 }, > + { CCI_REG16(0x3086), 0x8000 }, > + { CCI_REG16(0x3086), 0x387f }, > + { CCI_REG16(0x3086), 0x1380 }, > + { CCI_REG16(0x3086), 0x233b }, > + { CCI_REG16(0x3086), 0x7f93 }, > + { CCI_REG16(0x3086), 0x4502 }, > + { CCI_REG16(0x3086), 0x8000 }, > + { CCI_REG16(0x3086), 0x7fb0 }, > + { CCI_REG16(0x3086), 0x8d66 }, > + { CCI_REG16(0x3086), 0x7f90 }, > + { CCI_REG16(0x3086), 0x8192 }, > + { CCI_REG16(0x3086), 0x3c16 }, > + { CCI_REG16(0x3086), 0x357f }, > + { CCI_REG16(0x3086), 0x9345 }, > + { CCI_REG16(0x3086), 0x0280 }, > + { CCI_REG16(0x3086), 0x007f }, > + { CCI_REG16(0x3086), 0xb08d }, > + { CCI_REG16(0x3086), 0x667f }, > + { CCI_REG16(0x3086), 0x9081 }, > + { CCI_REG16(0x3086), 0x8237 }, > + { CCI_REG16(0x3086), 0x4502 }, > + { CCI_REG16(0x3086), 0x3681 }, > + { CCI_REG16(0x3086), 0x8044 }, > + { CCI_REG16(0x3086), 0x1631 }, > + { CCI_REG16(0x3086), 0x4374 }, > + { CCI_REG16(0x3086), 0x1678 }, > + { CCI_REG16(0x3086), 0x7b7d }, > + { CCI_REG16(0x3086), 0x4502 }, > + { CCI_REG16(0x3086), 0x450a }, > + { CCI_REG16(0x3086), 0x7e12 }, > + { CCI_REG16(0x3086), 0x8180 }, > + { CCI_REG16(0x3086), 0x377f }, > + { CCI_REG16(0x3086), 0x1045 }, > + { CCI_REG16(0x3086), 0x0a0e }, > + { CCI_REG16(0x3086), 0x7fd4 }, > + { CCI_REG16(0x3086), 0x8024 }, > + { CCI_REG16(0x3086), 0x0e82 }, > + { CCI_REG16(0x3086), 0x9cc2 }, > + { CCI_REG16(0x3086), 0xafa8 }, > + { CCI_REG16(0x3086), 0xaa03 }, > + { CCI_REG16(0x3086), 0x430d }, > + { CCI_REG16(0x3086), 0x2d46 }, > + { CCI_REG16(0x3086), 0x4316 }, > + { CCI_REG16(0x3086), 0x5f16 }, > + { CCI_REG16(0x3086), 0x530d }, > + { CCI_REG16(0x3086), 0x1660 }, > + { CCI_REG16(0x3086), 0x401e }, > + { CCI_REG16(0x3086), 0x2904 }, > + { CCI_REG16(0x3086), 0x2984 }, > + { CCI_REG16(0x3086), 0x81e7 }, > + { CCI_REG16(0x3086), 0x816f }, > + { CCI_REG16(0x3086), 0x1706 }, > + { CCI_REG16(0x3086), 0x81e7 }, > + { CCI_REG16(0x3086), 0x7f81 }, > + { CCI_REG16(0x3086), 0x5c0d }, > + { CCI_REG16(0x3086), 0x5754 }, > + { CCI_REG16(0x3086), 0x495f }, > + { CCI_REG16(0x3086), 0x5305 }, > + { CCI_REG16(0x3086), 0x5307 }, > + { CCI_REG16(0x3086), 0x4d2b }, > + { CCI_REG16(0x3086), 0xf810 }, > + { CCI_REG16(0x3086), 0x164c }, > + { CCI_REG16(0x3086), 0x0755 }, > + { CCI_REG16(0x3086), 0x562b }, > + { CCI_REG16(0x3086), 0xb82b }, > + { CCI_REG16(0x3086), 0x984e }, > + { CCI_REG16(0x3086), 0x1129 }, > + { CCI_REG16(0x3086), 0x9460 }, > + { CCI_REG16(0x3086), 0x5c09 }, > + { CCI_REG16(0x3086), 0x5c1b }, > + { CCI_REG16(0x3086), 0x4002 }, > + { CCI_REG16(0x3086), 0x4500 }, > + { CCI_REG16(0x3086), 0x4580 }, > + { CCI_REG16(0x3086), 0x29b6 }, > + { CCI_REG16(0x3086), 0x7f80 }, > + { CCI_REG16(0x3086), 0x4004 }, > + { CCI_REG16(0x3086), 0x7f88 }, > + { CCI_REG16(0x3086), 0x4109 }, > + { CCI_REG16(0x3086), 0x5c0b }, > + { CCI_REG16(0x3086), 0x29b2 }, > + { CCI_REG16(0x3086), 0x4115 }, > + { CCI_REG16(0x3086), 0x5c03 }, > + { CCI_REG16(0x3086), 0x4105 }, > + { CCI_REG16(0x3086), 0x5f2b }, > + { CCI_REG16(0x3086), 0x902b }, > + { CCI_REG16(0x3086), 0x8081 }, > + { CCI_REG16(0x3086), 0x6f40 }, > + { CCI_REG16(0x3086), 0x1041 }, > + { CCI_REG16(0x3086), 0x0160 }, > + { CCI_REG16(0x3086), 0x29a2 }, > + { CCI_REG16(0x3086), 0x29a3 }, > + { CCI_REG16(0x3086), 0x5f4d }, > + { CCI_REG16(0x3086), 0x1c17 }, > + { CCI_REG16(0x3086), 0x0281 }, > + { CCI_REG16(0x3086), 0xe729 }, > + { CCI_REG16(0x3086), 0x8345 }, > + { CCI_REG16(0x3086), 0x8840 }, > + { CCI_REG16(0x3086), 0x0f7f }, > + { CCI_REG16(0x3086), 0x8a40 }, > + { CCI_REG16(0x3086), 0x2345 }, > + { CCI_REG16(0x3086), 0x8024 }, > + { CCI_REG16(0x3086), 0x4008 }, > + { CCI_REG16(0x3086), 0x7f88 }, > + { CCI_REG16(0x3086), 0x5d29 }, > + { CCI_REG16(0x3086), 0x9288 }, > + { CCI_REG16(0x3086), 0x102b }, > + { CCI_REG16(0x3086), 0x0489 }, > + { CCI_REG16(0x3086), 0x165c }, > + { CCI_REG16(0x3086), 0x4386 }, > + { CCI_REG16(0x3086), 0x170b }, > + { CCI_REG16(0x3086), 0x5c03 }, > + { CCI_REG16(0x3086), 0x8a48 }, > + { CCI_REG16(0x3086), 0x4d4e }, > + { CCI_REG16(0x3086), 0x2b80 }, > + { CCI_REG16(0x3086), 0x4c09 }, > + { CCI_REG16(0x3086), 0x4119 }, > + { CCI_REG16(0x3086), 0x816f }, > + { CCI_REG16(0x3086), 0x4110 }, > + { CCI_REG16(0x3086), 0x4001 }, > + { CCI_REG16(0x3086), 0x6029 }, > + { CCI_REG16(0x3086), 0x8229 }, > + { CCI_REG16(0x3086), 0x8329 }, > + { CCI_REG16(0x3086), 0x435c }, > + { CCI_REG16(0x3086), 0x055f }, > + { CCI_REG16(0x3086), 0x4d1c }, > + { CCI_REG16(0x3086), 0x81e7 }, > + { CCI_REG16(0x3086), 0x4502 }, > + { CCI_REG16(0x3086), 0x8180 }, > + { CCI_REG16(0x3086), 0x7f80 }, > + { CCI_REG16(0x3086), 0x410a }, > + { CCI_REG16(0x3086), 0x9144 }, > + { CCI_REG16(0x3086), 0x1609 }, > + { CCI_REG16(0x3086), 0x2fc3 }, > + { CCI_REG16(0x3086), 0xb130 }, > + { CCI_REG16(0x3086), 0xc3b1 }, > + { CCI_REG16(0x3086), 0x0343 }, > + { CCI_REG16(0x3086), 0x164a }, > + { CCI_REG16(0x3086), 0x0a43 }, > + { CCI_REG16(0x3086), 0x160b }, > + { CCI_REG16(0x3086), 0x4316 }, > + { CCI_REG16(0x3086), 0x8f43 }, > + { CCI_REG16(0x3086), 0x1690 }, > + { CCI_REG16(0x3086), 0x4316 }, > + { CCI_REG16(0x3086), 0x7f81 }, > + { CCI_REG16(0x3086), 0x450a }, > + { CCI_REG16(0x3086), 0x410f }, > + { CCI_REG16(0x3086), 0x7f83 }, > + { CCI_REG16(0x3086), 0x5d29 }, > + { CCI_REG16(0x3086), 0x4488 }, > + { CCI_REG16(0x3086), 0x102b }, > + { CCI_REG16(0x3086), 0x0453 }, > + { CCI_REG16(0x3086), 0x0d40 }, > + { CCI_REG16(0x3086), 0x2345 }, > + { CCI_REG16(0x3086), 0x0240 }, > + { CCI_REG16(0x3086), 0x087f }, > + { CCI_REG16(0x3086), 0x8053 }, > + { CCI_REG16(0x3086), 0x0d89 }, > + { CCI_REG16(0x3086), 0x165c }, > + { CCI_REG16(0x3086), 0x4586 }, > + { CCI_REG16(0x3086), 0x170b }, > + { CCI_REG16(0x3086), 0x5c05 }, > + { CCI_REG16(0x3086), 0x8a60 }, > + { CCI_REG16(0x3086), 0x4b91 }, > + { CCI_REG16(0x3086), 0x4416 }, > + { CCI_REG16(0x3086), 0x09c1 }, > + { CCI_REG16(0x3086), 0x2ca9 }, > + { CCI_REG16(0x3086), 0xab30 }, > + { CCI_REG16(0x3086), 0x51b3 }, > + { CCI_REG16(0x3086), 0x3d5a }, > + { CCI_REG16(0x3086), 0x7e3d }, > + { CCI_REG16(0x3086), 0x7e19 }, > + { CCI_REG16(0x3086), 0x8000 }, > + { CCI_REG16(0x3086), 0x8b1f }, > + { CCI_REG16(0x3086), 0x2a1f }, > + { CCI_REG16(0x3086), 0x83a2 }, > + { CCI_REG16(0x3086), 0x7516 }, > + { CCI_REG16(0x3086), 0xad33 }, > + { CCI_REG16(0x3086), 0x450a }, > + { CCI_REG16(0x3086), 0x7f53 }, > + { CCI_REG16(0x3086), 0x8023 }, > + { CCI_REG16(0x3086), 0x8c66 }, > + { CCI_REG16(0x3086), 0x7f13 }, > + { CCI_REG16(0x3086), 0x8184 }, > + { CCI_REG16(0x3086), 0x1481 }, > + { CCI_REG16(0x3086), 0x8031 }, > + { CCI_REG16(0x3086), 0x3d64 }, > + { CCI_REG16(0x3086), 0x452a }, > + { CCI_REG16(0x3086), 0x9451 }, > + { CCI_REG16(0x3086), 0x9e96 }, > + { CCI_REG16(0x3086), 0x3d2b }, > + { CCI_REG16(0x3086), 0x3d1b }, > + { CCI_REG16(0x3086), 0x529f }, > + { CCI_REG16(0x3086), 0x0e3d }, > + { CCI_REG16(0x3086), 0x083d }, > + { CCI_REG16(0x3086), 0x167e }, > + { CCI_REG16(0x3086), 0x307e }, > + { CCI_REG16(0x3086), 0x1175 }, > + { CCI_REG16(0x3086), 0x163e }, > + { CCI_REG16(0x3086), 0x970e }, > + { CCI_REG16(0x3086), 0x82b2 }, > + { CCI_REG16(0x3086), 0x3d7f }, > + { CCI_REG16(0x3086), 0xac3e }, > + { CCI_REG16(0x3086), 0x4502 }, > + { CCI_REG16(0x3086), 0x7e11 }, > + { CCI_REG16(0x3086), 0x7fd0 }, > + { CCI_REG16(0x3086), 0x8000 }, > + { CCI_REG16(0x3086), 0x8c66 }, > + { CCI_REG16(0x3086), 0x7f90 }, > + { CCI_REG16(0x3086), 0x8194 }, > + { CCI_REG16(0x3086), 0x3f44 }, > + { CCI_REG16(0x3086), 0x1681 }, > + { CCI_REG16(0x3086), 0x8416 }, > + { CCI_REG16(0x3086), 0x2c2c }, > + { CCI_REG16(0x3086), 0x2c2c }, > + { CCI_REG16(0x302a), 0x0005 }, > + { CCI_REG16(0x302c), 0x0001 }, > + { CCI_REG16(0x302e), 0x0003 }, > + { CCI_REG16(0x3030), 0x0032 }, > + { CCI_REG16(0x3036), 0x000a }, > + { CCI_REG16(0x3038), 0x0001 }, > + { CCI_REG16(0x30b0), 0x0028 }, > + { CCI_REG16(0x31b0), 0x0082 }, > + { CCI_REG16(0x31b2), 0x005c }, > + { CCI_REG16(0x31b4), 0x5248 }, > + { CCI_REG16(0x31b6), 0x3257 }, > + { CCI_REG16(0x31b8), 0x904b }, > + { CCI_REG16(0x31ba), 0x030b }, > + { CCI_REG16(0x31bc), 0x8e09 }, > + { CCI_REG16(0x3354), 0x002b }, > + { CCI_REG16(0x31d0), 0x0000 }, > + { CCI_REG16(0x31ae), 0x0204 }, > + { CCI_REG16(0x3002), 0x00d0 }, > + { CCI_REG16(0x3004), 0x0148 }, > + { CCI_REG16(0x3006), 0x048f }, > + { CCI_REG16(0x3008), 0x0647 }, We've got 0x3002-0x3008 (start and end offsets) repeated both in link_freq_360M_1280x960_10bit_2lane and here. If these are repeated, then the whole table needs close scrutiny (sorry, not enough time to check it now). > + { CCI_REG16(0x3064), 0x1802 }, > + { CCI_REG16(0x300a), 0x04c4 }, > + { CCI_REG16(0x300c), 0x04c4 }, > + { CCI_REG16(0x30a2), 0x0001 }, > + { CCI_REG16(0x30a6), 0x0001 }, > + { CCI_REG16(0x3012), 0x010c }, > + { CCI_REG16(0x3786), 0x0006 }, > + { CCI_REG16(0x31ae), 0x0202 }, > + { CCI_REG16(0x3088), 0x8050 }, > + { CCI_REG16(0x3086), 0x9237 }, > + { CCI_REG16(0x3044), 0x0410 }, > + { CCI_REG16(0x3094), 0x03d4 }, > + { CCI_REG16(0x3096), 0x0280 }, > + { CCI_REG16(0x30ba), 0x7606 }, > + { CCI_REG16(0x30b0), 0x0028 }, > + { CCI_REG16(0x30ba), 0x7600 }, > + { CCI_REG16(0x30fe), 0x002a }, > + { CCI_REG16(0x31de), 0x0410 }, > + { CCI_REG16(0x3ed6), 0x1435 }, > + { CCI_REG16(0x3ed8), 0x9865 }, > + { CCI_REG16(0x3eda), 0x7698 }, > + { CCI_REG16(0x3edc), 0x99ff }, > + { CCI_REG16(0x3ee2), 0xbb88 }, > + { CCI_REG16(0x3ee4), 0x8836 }, > + { CCI_REG16(0x3ef0), 0x1cf0 }, > + { CCI_REG16(0x3ef2), 0x0000 }, > + { CCI_REG16(0x3ef8), 0x6166 }, > + { CCI_REG16(0x3efa), 0x3333 }, > + { CCI_REG16(0x3efc), 0x6634 }, > + { CCI_REG16(0x3088), 0x81ba }, > + { CCI_REG16(0x3086), 0x3d02 }, > + { CCI_REG16(0x3276), 0x05dc }, > + { CCI_REG16(0x3f00), 0x9d05 }, > + { CCI_REG16(0x3ed2), 0xfa86 }, > + { CCI_REG16(0x3eee), 0xa4fe }, > + { CCI_REG16(0x3ecc), 0x6e42 }, > + { CCI_REG16(0x3ecc), 0x0e42 }, > + { CCI_REG16(0x3eec), 0x0c0c }, > + { CCI_REG16(0x3ee8), 0xaae4 }, > + { CCI_REG16(0x3ee6), 0x3363 }, > + { CCI_REG16(0x3ee6), 0x3363 }, > + { CCI_REG16(0x3ee8), 0xaae4 }, > + { CCI_REG16(0x3ee8), 0xaae4 }, > + { CCI_REG16(0x3180), 0xc24f }, > + { CCI_REG16(0x3102), 0x5000 }, > + { CCI_REG16(0x3060), 0x000d }, > + { CCI_REG16(0x3ed0), 0xff44 }, > + { CCI_REG16(0x3ed2), 0xaa86 }, > + { CCI_REG16(0x3ed4), 0x031f }, > + { CCI_REG16(0x3eee), 0xa4aa }, > +}; > + > +static const s64 link_freq_menu_items[] = { > + 360000000ULL, > +}; > + > +static const struct ar0234_link_freq_config link_freq_configs[] = { > + { > + .reg_list = { > + .num_of_regs = > + ARRAY_SIZE(link_freq_360M_1280x960_10bit_2lane), > + .regs = link_freq_360M_1280x960_10bit_2lane, > + } > + }, > +}; > + > +static const struct ar0234_mode supported_modes[] = { > + { > + .width = AR0234_NATIVE_WIDTH, > + .height = AR0234_NATIVE_HEIGHT, > + .vts_def = 2435, > + .vts_min = 2435, Really, or is that just the minimum you've used it with? My testing appears to have found that VTS only needs to be image height + 4. > + .code = MEDIA_BUS_FMT_SGRBG10_1X10, > + .reg_list = { > + .num_of_regs = ARRAY_SIZE(mode_1280x960_10bit_2lane), > + .regs = mode_1280x960_10bit_2lane, > + }, > + .link_freq_index = 0, > + .hblank = 3600, Queried above as that doesn't appear to match the register values. I do note that this is V4L2_CID_HBLANK value, so it is NOT the HTS value in the registers. There could be some debate as to which is the better domain to work in. > + .vblank = 1475, That's vts_min - height (2435 - 960 = 1475). Duplicated data like that increases the chance of errors when adding new modes. > + }, > +}; > + > +struct ar0234 { > + struct v4l2_subdev sd; > + struct media_pad pad; > + struct v4l2_ctrl_handler ctrl_handler; > + > + /* V4L2 Controls */ > + struct v4l2_ctrl *link_freq; > + struct v4l2_ctrl *exposure; > + struct v4l2_ctrl *analogue_gain; > + struct v4l2_ctrl *digital_gain; Neither analogue_gain nor digital_gain are ever written to or read, so no need to store it. > + struct v4l2_ctrl *pixel_rate; Ditto that it's never accessed once initialised. > + struct v4l2_ctrl *hblank; > + struct v4l2_ctrl *vblank; > + > + struct regmap *regmap; > + unsigned long link_freq_bitmap; > + const struct ar0234_mode *cur_mode; > +}; > + > +static int ar0234_set_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct ar0234 *ar0234 = > + container_of(ctrl->handler, struct ar0234, ctrl_handler); > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + s64 exposure_max, exposure_def; > + struct v4l2_subdev_state *state; > + const struct v4l2_mbus_framefmt *format; > + int ret; > + > + state = v4l2_subdev_get_locked_active_state(&ar0234->sd); > + format = v4l2_subdev_state_get_format(state, 0); > + > + /* Propagate change of current control to all related controls */ > + if (ctrl->id == V4L2_CID_VBLANK) { > + /* Update max exposure while meeting expected vblanking */ > + exposure_max = format->height + ctrl->val - > + AR0234_EXPOSURE_MAX_MARGIN; > + exposure_def = format->height - AR0234_EXPOSURE_MAX_MARGIN; > + __v4l2_ctrl_modify_range(ar0234->exposure, > + ar0234->exposure->minimum, > + exposure_max, ar0234->exposure->step, > + exposure_def); > + } > + > + /* V4L2 controls values will be applied only when power is already up */ > + if (!pm_runtime_get_if_in_use(&client->dev)) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_ANALOGUE_GAIN: > + ret = cci_write(ar0234->regmap, AR0234_REG_ANALOG_GAIN, > + ctrl->val, NULL); > + break; > + > + case V4L2_CID_DIGITAL_GAIN: > + ret = cci_write(ar0234->regmap, AR0234_REG_GLOBAL_GAIN, > + ctrl->val, NULL); > + break; > + > + case V4L2_CID_EXPOSURE: > + ret = cci_write(ar0234->regmap, AR0234_REG_EXPOSURE, > + ctrl->val, NULL); > + break; > + > + case V4L2_CID_VBLANK: > + ret = cci_write(ar0234->regmap, AR0234_REG_VTS, > + ar0234->cur_mode->height + ctrl->val, NULL); > + break; > + > + default: > + ret = -EINVAL; > + break; > + } > + > + pm_runtime_put(&client->dev); > + > + return ret; > +} > + > +static const struct v4l2_ctrl_ops ar0234_ctrl_ops = { > + .s_ctrl = ar0234_set_ctrl, > +}; > + > +static int ar0234_init_controls(struct ar0234 *ar0234) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + struct v4l2_fwnode_device_properties props; > + struct v4l2_ctrl_handler *ctrl_hdlr; > + s64 exposure_max, vblank, hblank; > + u32 link_freq_size; > + int ret; > + > + ctrl_hdlr = &ar0234->ctrl_handler; > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8); > + if (ret) > + return ret; > + > + link_freq_size = ARRAY_SIZE(link_freq_menu_items) - 1; > + ar0234->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, > + &ar0234_ctrl_ops, > + V4L2_CID_LINK_FREQ, > + link_freq_size, 0, > + link_freq_menu_items); > + if (ar0234->link_freq) > + ar0234->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + > + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, > + AR0234_ANAL_GAIN_MIN, AR0234_ANAL_GAIN_MAX, > + AR0234_ANAL_GAIN_STEP, AR0234_ANAL_GAIN_DEFAULT); > + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, V4L2_CID_DIGITAL_GAIN, > + AR0234_DGTL_GAIN_MIN, AR0234_DGTL_GAIN_MAX, > + AR0234_DGTL_GAIN_STEP, AR0234_DGTL_GAIN_DEFAULT); > + > + exposure_max = ar0234->cur_mode->vts_def - AR0234_EXPOSURE_MAX_MARGIN; > + ar0234->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, > + V4L2_CID_EXPOSURE, > + AR0234_EXPOSURE_MIN, exposure_max, > + AR0234_EXPOSURE_STEP, > + exposure_max); > + > + ar0234->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, > + V4L2_CID_PIXEL_RATE, > + PIXEL_RATE, > + PIXEL_RATE, 1, > + PIXEL_RATE); > + if (ar0234->pixel_rate) > + ar0234->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY; V4L2_CID_PIXEL_RATE defaults to read only by the core (v4l2_ctrl_fill), so no need to set it manually. > + > + vblank = AR0234_VTS_MAX - ar0234->cur_mode->height; > + ar0234->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, > + V4L2_CID_VBLANK, 0, vblank, 1, > + ar0234->cur_mode->vblank); > + hblank = ar0234->cur_mode->hblank; > + ar0234->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, > + V4L2_CID_HBLANK, hblank, hblank, 1, > + hblank); > + if (ar0234->hblank) > + ar0234->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > + It would be nice to add V4L2_CID_HFLIP and V4L2_CID_VFLIP too. It's fairly easy on this module as it doesn't change the Bayer order. Register 0x3040 bits 14 & 15, or 0x301d bits 0 & 1. > + if (ctrl_hdlr->error) > + return ctrl_hdlr->error; > + > + ret = v4l2_fwnode_device_parse(&client->dev, &props); > + if (ret) > + return ret; > + > + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ar0234_ctrl_ops, > + &props); > + if (ret) > + return ret; > + > + ar0234->sd.ctrl_handler = ctrl_hdlr; > + > + return 0; > +} > + > +static void ar0234_update_pad_format(const struct ar0234_mode *mode, > + struct v4l2_mbus_framefmt *fmt) > +{ > + fmt->width = mode->width; > + fmt->height = mode->height; > + fmt->code = mode->code; > + fmt->field = V4L2_FIELD_NONE; > +} > + > +static int ar0234_start_streaming(struct ar0234 *ar0234) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + const struct ar0234_reg_list *reg_list; > + int link_freq_index, ret; > + > + ret = pm_runtime_resume_and_get(&client->dev); > + if (ret < 0) > + return ret; > + > + /* > + * Setting 0X301A.bit[0] will initiate a reset sequence: > + * the frame being generated will be truncated. > + */ > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, > + AR0234_MODE_RESET, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to reset"); > + goto err_rpm_put; > + } > + > + usleep_range(1000, 1500); > + > + reg_list = &ar0234->cur_mode->reg_list; > + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs, > + reg_list->num_of_regs, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to set mode"); > + goto err_rpm_put; > + } > + > + link_freq_index = ar0234->cur_mode->link_freq_index; > + if (link_freq_index > 0) { > + reg_list = &link_freq_configs[link_freq_index].reg_list; > + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs, > + reg_list->num_of_regs, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to set plls"); > + goto err_rpm_put; > + } > + } > + > + ret = __v4l2_ctrl_handler_setup(ar0234->sd.ctrl_handler); > + if (ret) > + goto err_rpm_put; > + > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, > + AR0234_MODE_STREAMING, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to start stream"); > + goto err_rpm_put; > + } > + > + return 0; > + > +err_rpm_put: > + pm_runtime_put(&client->dev); > + return ret; > +} > + > +static int ar0234_stop_streaming(struct ar0234 *ar0234) > +{ > + int ret; > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, > + AR0234_MODE_STANDBY, NULL); > + if (ret < 0) > + dev_err(&client->dev, "failed to stop stream"); > + > + pm_runtime_put(&client->dev); > + return ret; > +} > + > +static int ar0234_set_stream(struct v4l2_subdev *sd, int enable) > +{ > + struct ar0234 *ar0234 = to_ar0234(sd); > + struct v4l2_subdev_state *state; > + int ret = 0; > + > + state = v4l2_subdev_lock_and_get_active_state(sd); > + > + if (enable) > + ret = ar0234_start_streaming(ar0234); > + else > + ret = ar0234_stop_streaming(ar0234); > + > + v4l2_subdev_unlock_state(state); > + > + return ret; > +} > + > +static int ar0234_set_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_format *fmt) > +{ > + struct ar0234 *ar0234 = to_ar0234(sd); > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + struct v4l2_rect *crop; > + const struct ar0234_mode *mode; > + s64 hblank; > + int ret; > + > + mode = v4l2_find_nearest_size(supported_modes, > + ARRAY_SIZE(supported_modes), > + width, height, > + fmt->format.width, > + fmt->format.height); > + > + crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad); > + crop->width = mode->width; > + crop->height = mode->height; > + > + ar0234_update_pad_format(mode, &fmt->format); > + *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format; > + > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) > + return 0; > + > + ar0234->cur_mode = mode; > + ret = __v4l2_ctrl_s_ctrl(ar0234->link_freq, mode->link_freq_index); > + if (ret) { > + dev_err(&client->dev, "Link Freq ctrl set failed\n"); > + return ret; > + } > + > + hblank = ar0234->cur_mode->hblank; > + ret = __v4l2_ctrl_modify_range(ar0234->hblank, hblank, hblank, > + 1, hblank); > + if (ret) { > + dev_err(&client->dev, "HB ctrl range update failed\n"); > + return ret; > + } > + > + /* Update limits and set FPS to default */ > + ret = __v4l2_ctrl_modify_range(ar0234->vblank, 0, > + AR0234_VTS_MAX - mode->height, 1, > + ar0234->cur_mode->vblank); > + if (ret) { > + dev_err(&client->dev, "VB ctrl range update failed\n"); > + return ret; > + } > + > + ret = __v4l2_ctrl_s_ctrl(ar0234->vblank, ar0234->cur_mode->vblank); > + if (ret) { > + dev_err(&client->dev, "VB ctrl set failed\n"); > + return ret; > + } > + > + return 0; > +} > + > +static int ar0234_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + if (code->index > 0) > + return -EINVAL; > + > + code->code = MEDIA_BUS_FMT_SGRBG10_1X10; > + > + return 0; > +} > + > +static int ar0234_enum_frame_size(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_frame_size_enum *fse) > +{ > + if (fse->index >= ARRAY_SIZE(supported_modes)) > + return -EINVAL; > + > + if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10) > + return -EINVAL; > + > + fse->min_width = supported_modes[fse->index].width; > + fse->max_width = fse->min_width; > + fse->min_height = supported_modes[fse->index].height; > + fse->max_height = fse->min_height; > + > + return 0; > +} > + > +static int ar0234_get_selection(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_selection *sel) > +{ > + switch (sel->target) { > + case V4L2_SEL_TGT_CROP_DEFAULT: > + case V4L2_SEL_TGT_CROP: > + sel->r = *v4l2_subdev_state_get_crop(state, 0); > + break; > + case V4L2_SEL_TGT_CROP_BOUNDS: > + sel->r.top = 0; > + sel->r.left = 0; > + sel->r.width = AR0234_NATIVE_WIDTH; > + sel->r.height = AR0234_NATIVE_HEIGHT; > + break; The numbers look wrong here based on the fact the mode is cropped. Implement V4L2_SEL_TGT_NATIVE_SIZE as well? > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ar0234_init_state(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *sd_state) > +{ > + struct v4l2_subdev_format fmt = { > + .which = V4L2_SUBDEV_FORMAT_TRY, > + .pad = 0, > + .format = { > + .code = MEDIA_BUS_FMT_SGRBG10_1X10, > + .width = AR0234_NATIVE_WIDTH, > + .height = AR0234_NATIVE_HEIGHT, > + }, > + }; > + > + ar0234_set_format(sd, sd_state, &fmt); > + > + return 0; > +} > + > +static const struct v4l2_subdev_video_ops ar0234_video_ops = { > + .s_stream = ar0234_set_stream, > +}; > + > +static const struct v4l2_subdev_pad_ops ar0234_pad_ops = { > + .set_fmt = ar0234_set_format, > + .get_fmt = v4l2_subdev_get_fmt, > + .enum_mbus_code = ar0234_enum_mbus_code, > + .enum_frame_size = ar0234_enum_frame_size, > + .get_selection = ar0234_get_selection, > +}; > + > +static const struct v4l2_subdev_core_ops ar0234_core_ops = { > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > +}; > + > +static const struct v4l2_subdev_ops ar0234_subdev_ops = { > + .core = &ar0234_core_ops, > + .video = &ar0234_video_ops, > + .pad = &ar0234_pad_ops, > +}; > + > +static const struct media_entity_operations ar0234_subdev_entity_ops = { > + .link_validate = v4l2_subdev_link_validate, > +}; > + > +static const struct v4l2_subdev_internal_ops ar0234_internal_ops = { > + .init_state = ar0234_init_state, > +}; > + > +static int ar0234_parse_fwnode(struct ar0234 *ar0234, struct device *dev) > +{ > + struct fwnode_handle *endpoint; > + struct v4l2_fwnode_endpoint bus_cfg = { > + .bus_type = V4L2_MBUS_CSI2_DPHY, > + }; > + int ret; > + > + endpoint = > + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, > + FWNODE_GRAPH_ENDPOINT_NEXT); > + if (!endpoint) { > + dev_err(dev, "endpoint node not found\n"); > + return -EPROBE_DEFER; > + } > + > + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg); > + if (ret) { > + dev_err(dev, "parsing endpoint node failed\n"); > + goto out_err; > + } > + > + ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies, > + bus_cfg.nr_of_link_frequencies, > + link_freq_menu_items, > + ARRAY_SIZE(link_freq_menu_items), > + &ar0234->link_freq_bitmap); > + if (ret) > + goto out_err; Validate the number of data lanes too? Whilst this driver currently only supports 2 lanes, the sensor supports 1, 2, or 4. Your register configuration also puts the sensor in a continuous clock mode. It would be nice to at least document that, if not check for it in the configuration. > + > +out_err: > + v4l2_fwnode_endpoint_free(&bus_cfg); > + fwnode_handle_put(endpoint); > + return ret; > +} > + > +static int ar0234_identify_module(struct ar0234 *ar0234) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); > + int ret; > + u64 val; > + > + ret = cci_read(ar0234->regmap, AR0234_REG_CHIP_ID, &val, NULL); An easy future extension is to add support for the mono variant of the sensor. It reports as CHIP_ID 0x1a56. This isn't a request to add it now, but more requesting that we don't make it hard to implement later (simplest support just changes the MBUS_CODE. > + if (ret) > + return ret; > + > + if (val != AR0234_CHIP_ID) { > + dev_err(&client->dev, "chip id mismatch: %x!=%llx", > + AR0234_CHIP_ID, val); > + return -ENXIO; > + } > + > + return 0; > +} > + > +static void ar0234_remove(struct i2c_client *client) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct ar0234 *ar0234 = to_ar0234(sd); > + > + v4l2_async_unregister_subdev(&ar0234->sd); > + v4l2_subdev_cleanup(sd); > + media_entity_cleanup(&ar0234->sd.entity); > + v4l2_ctrl_handler_free(&ar0234->ctrl_handler); > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > +} > + > +static int ar0234_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct ar0234 *ar0234; > + int ret; > + > + ar0234 = devm_kzalloc(&client->dev, sizeof(*ar0234), GFP_KERNEL); > + if (!ar0234) > + return -ENOMEM; > + > + ret = ar0234_parse_fwnode(ar0234, dev); > + if (ret) > + return ret; You don't look for a clock feeding the sensor. What frequency is that clock at? The PLL settings you program into the registers are going to be dependent on that. On the basis that Intel normally use 19.2MHz clocks for their sensors, plugging your register settings in I get a pixel rate of 51.2MPix/s, not the 180MPix/s defined as PIXEL_RATE. I could be totally wrong, but that makes me dubious of almost all the timing values. If I use the register settings from my driver with the 24MHz clock that it expects, the pixel clock does come out as 180MPix/s. To get the expected numbers out with your register settings, I think you'd need an external clock of 67.5MHz which is out of spec (max 54 MHz), and the PLL will be out of spec too. Dave > + > + ar0234->regmap = devm_cci_regmap_init_i2c(client, 16); > + if (IS_ERR(ar0234->regmap)) > + return dev_err_probe(dev, PTR_ERR(ar0234->regmap), > + "failed to init CCI\n"); > + > + v4l2_i2c_subdev_init(&ar0234->sd, client, &ar0234_subdev_ops); > + > + ret = ar0234_identify_module(ar0234); > + if (ret) { > + dev_err(&client->dev, "failed to find sensor: %d", ret); > + return ret; > + } > + > + /* Set default mode to max resolution */ > + ar0234->cur_mode = &supported_modes[0]; > + ret = ar0234_init_controls(ar0234); > + if (ret) { > + dev_err(&client->dev, "failed to init controls: %d", ret); > + goto probe_error_v4l2_ctrl_handler_free; > + } > + > + ar0234->sd.internal_ops = &ar0234_internal_ops; > + ar0234->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | > + V4L2_SUBDEV_FL_HAS_EVENTS; > + ar0234->sd.entity.ops = &ar0234_subdev_entity_ops; > + ar0234->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > + > + ar0234->pad.flags = MEDIA_PAD_FL_SOURCE; > + ret = media_entity_pads_init(&ar0234->sd.entity, 1, &ar0234->pad); > + if (ret) { > + dev_err(&client->dev, "failed to init entity pads: %d", ret); > + goto probe_error_v4l2_ctrl_handler_free; > + } > + > + ar0234->sd.state_lock = ar0234->ctrl_handler.lock; > + ret = v4l2_subdev_init_finalize(&ar0234->sd); > + if (ret < 0) { > + dev_err(dev, "v4l2 subdev init error: %d\n", ret); > + goto probe_error_media_entity_cleanup; > + } > + > + /* > + * Device is already turned on by i2c-core with ACPI domain PM. > + * Enable runtime PM and turn off the device. > + */ > + pm_runtime_set_active(&client->dev); > + pm_runtime_enable(&client->dev); > + pm_runtime_idle(&client->dev); > + > + ret = v4l2_async_register_subdev_sensor(&ar0234->sd); > + if (ret < 0) { > + dev_err(&client->dev, "failed to register V4L2 subdev: %d", > + ret); > + goto probe_error_rpm; > + } > + > + return 0; > +probe_error_rpm: > + pm_runtime_disable(&client->dev); > + v4l2_subdev_cleanup(&ar0234->sd); > + > +probe_error_media_entity_cleanup: > + media_entity_cleanup(&ar0234->sd.entity); > + > +probe_error_v4l2_ctrl_handler_free: > + v4l2_ctrl_handler_free(ar0234->sd.ctrl_handler); > + > + return ret; > +} > + > +static const struct acpi_device_id ar0234_acpi_ids[] = { > + { "INTC10C0" }, > + {} > +}; > + > +MODULE_DEVICE_TABLE(acpi, ar0234_acpi_ids); > + > +static struct i2c_driver ar0234_i2c_driver = { > + .driver = { > + .name = "ar0234", > + .acpi_match_table = ACPI_PTR(ar0234_acpi_ids), > + }, > + .probe = ar0234_probe, > + .remove = ar0234_remove, > +}; > + > +module_i2c_driver(ar0234_i2c_driver); > + > +MODULE_DESCRIPTION("ON Semiconductor ar0234 sensor driver"); > +MODULE_AUTHOR("Dongcheng Yan <dongcheng.yan@intel.com>"); > +MODULE_AUTHOR("Hao Yao <hao.yao@intel.com>"); > +MODULE_LICENSE("GPL"); > -- > 2.34.1 > >
Hi Dongcheng, kernel test robot noticed the following build warnings: [auto build test WARNING on media-tree/master] [also build test WARNING on sailus-media-tree/master linuxtv-media-stage/master linus/master v6.9-rc6 next-20240430] [cannot apply to sailus-media-tree/streams] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dongcheng-Yan/media-i2c-Add-ar0234-camera-sensor-driver/20240430-133240 base: git://linuxtv.org/media_tree.git master patch link: https://lore.kernel.org/r/20240429051333.1306453-1-dongcheng.yan%40intel.com patch subject: [PATCH v2] media: i2c: Add ar0234 camera sensor driver config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20240430/202404302344.158Y810h-lkp@intel.com/config) compiler: arceb-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240430/202404302344.158Y810h-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202404302344.158Y810h-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/media/i2c/ar0234.c:1032:36: warning: 'ar0234_acpi_ids' defined but not used [-Wunused-const-variable=] 1032 | static const struct acpi_device_id ar0234_acpi_ids[] = { | ^~~~~~~~~~~~~~~ vim +/ar0234_acpi_ids +1032 drivers/media/i2c/ar0234.c 1031 > 1032 static const struct acpi_device_id ar0234_acpi_ids[] = { 1033 { "INTC10C0" }, 1034 {} 1035 }; 1036
Hi Dongcheng,
kernel test robot noticed the following build warnings:
[auto build test WARNING on media-tree/master]
[also build test WARNING on sailus-media-tree/master linuxtv-media-stage/master linus/master v6.9-rc6 next-20240430]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dongcheng-Yan/media-i2c-Add-ar0234-camera-sensor-driver/20240430-133240
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20240429051333.1306453-1-dongcheng.yan%40intel.com
patch subject: [PATCH v2] media: i2c: Add ar0234 camera sensor driver
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20240501/202405011552.aHb15tq0-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 37ae4ad0eef338776c7e2cffb3896153d43dcd90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240501/202405011552.aHb15tq0-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405011552.aHb15tq0-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/media/i2c/ar0234.c:7:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/media/i2c/ar0234.c:7:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/media/i2c/ar0234.c:7:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/media/i2c/ar0234.c:7:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/media/i2c/ar0234.c:1032:36: warning: unused variable 'ar0234_acpi_ids' [-Wunused-const-variable]
1032 | static const struct acpi_device_id ar0234_acpi_ids[] = {
| ^~~~~~~~~~~~~~~
8 warnings generated.
vim +/ar0234_acpi_ids +1032 drivers/media/i2c/ar0234.c
1031
> 1032 static const struct acpi_device_id ar0234_acpi_ids[] = {
1033 { "INTC10C0" },
1034 {}
1035 };
1036
Hi Dongcheng, A few more comments below... On Mon, Apr 29, 2024 at 01:13:33PM +0800, Dongcheng Yan wrote: > +/* Analog gain control */ > +#define AR0234_REG_ANALOG_GAIN CCI_REG16(0x3060) > +#define AR0234_ANAL_GAIN_MIN 0 > +#define AR0234_ANAL_GAIN_MAX 0x7f > +#define AR0234_ANAL_GAIN_STEP 1 > +#define AR0234_ANAL_GAIN_DEFAULT 0xe Please use the same prefix here as for the register, apart from REG_ part. I.e. AR0234_ANALOG_. > + > +/* Digital gain control */ > +#define AR0234_REG_GLOBAL_GAIN CCI_REG16(0x305e) > +#define AR0234_DGTL_GAIN_MIN 0 > +#define AR0234_DGTL_GAIN_MAX 0x7ff > +#define AR0234_DGTL_GAIN_STEP 1 > +#define AR0234_DGTL_GAIN_DEFAULT 0x80 Ditto. ... > +static const struct cci_reg_sequence mode_1280x960_10bit_2lane[] = { > + { CCI_REG16(0x3f4c), 0x121f }, > + { CCI_REG16(0x3f4e), 0x121f }, > + { CCI_REG16(0x3f50), 0x0b81 }, > + { CCI_REG16(0x31e0), 0x0003 }, > + { CCI_REG16(0x30b0), 0x0028 }, > + /* R0X3088 specify the sequencer RAM access address. */ s/0\KX/x/ Same elsewhere. > + { CCI_REG16(0x3088), 0x8000 }, > + /* R0X3086 write the sequencer RAM. */
Hi Dave,
I'm sorry for not quoting your original email, which makes it difficult to understand my reply, so I have corrected my response. Thanks to Sakari for the reminder. Here is my reply:
Thanks for your valuable reviews. After reviewing your patch, I believe you have made significant progress on the AR0234 driver working, and many comments have been enlightening to me. Below firstly are some responses I concerned, and all replies are attached too. Furthermore, I hope to collaborate with you to upstream the AR0234 sensor, so that community developers can all benefit from it.
--------------------------------------------------------
First and foremost, I must honestly admit that I made a mistake in link_freq_360M, actually driver don't use this reg setting because of code in ar0234_start_streaming here:
if (link_freq_index > 0) {
            reg_list = &link_freq_configs[link_freq_index].reg_list;
            ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs,
                               reg_list->num_of_regs, NULL);
            if (ret) {
                  dev_err(&client->dev, "failed to set plls");
                  goto err_rpm_put;
            }
      }
So I'm sorry for the confusing review about this particular reg setting, I have removed link_freq_360M and related codes now.
------------------------------------------------------
Secondly, I've accepted all the suggestions you made regarding additions, deletions, and others.
• Switched AR0234_NATIVE_WIDTH/HEIGHT to 1920x1200;
• Eliminated vblank/hblank in ar0234_mode struct to prevent errors caused by redundant calculations;
• Eliminated v4l2_ctrl *analogue_gain & digital_gain & pixel_rate;
• Eliminated the RO flag of V4L2_CID_PIXEL_RATE;
• Added V4L2_CID_HFLIP and V4L2_CID_VFLIP;
• Implemented V4L2_SEL_TGT_NATIVE_SIZE;
-----------------------------------------------------
Furthermore, I need to respond to the calculations about parameters in the sensor here. If there are any inaccuracies, I would appreciate your correction.
1.
According to spec PLL Block Diagram:
used your reglist config, pre_pll_clk_div=0x8=8, pll_multiplier=0x96=150, vt_sys_clk_div=0x1=1, vt_pix_clk_div=0x5=5, so PIXCLK=24M/8*150/1/5=90MHz; op_sys_clk_div=0x1=1, so FSERIAL_CLK=24/8*150/1*2=900MHz; op_pix_clk_div=0xa=10, so OP_CLK=FSERIAL/10=90MHz. pixel_rate=4*OP_CLK is 360M according to pic below.
In my driver, pre_pll_clk_div=0x3=3, pll_multiplier=0x32=50, vt_sys_clk_div=0x1=1, vt_pix_clk_div=0x5=5, so PIXCLK=19.2/3*50/1/5/2=32MHz. op_sys_clk_div=0x1=1, so FSERIAL_CLK=19.2/3*50/1*2=640MHz; op_pix_clk_div=0xa=10, so OP_CLK=FSERIAL/10=64MHz. pixel_rate=2*OP_CLK is 128MHz.
I have set to 128Mhz now. Also can calculated by folmula here:
Pixel_rate = 2 * lanes * link_freq /bits = 2*2*360M/10=128Mhz.
2. In our reg setting, reg_vts=0x4c4, so VTS=1220, reg_hts=0x4c4, that HTS=4880, we can calculate PPL and FPS as below:
• PPL=HTS/SCLK*pixel_rate=3498
• FPS=SCLK/HTS/VTS=30
-------------------------------------------------
Lastly, there are some sections that require discussion.
> -----Original Message-----
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Sent: Monday, April 29, 2024 9:59 PM
> To: Yan, Dongcheng <dongcheng.yan@intel.com>
> Cc: sakari.ailus@linux.intel.com; linux-media@vger.kernel.org;
> tomi.valkeinen@ideasonboard.com; jacopo.mondi@ideasonboard.com;
> bingbu.cao@linux.intel.com; Li, Daxing <daxing.li@intel.com>
> Subject: Re: [PATCH v2] media: i2c: Add ar0234 camera sensor driver
>
> Hi Dongcheng
>
> Thanks for the patch.
>
> As I've previously looked at this sensor, and had most of a driver working [1],
> there are a couple of things that immediately stand out to me in this patch.
>
> [1]
> https://github.com/6by9/linux/blob/rpi-6.4.y-ar0234/drivers/media/i2c/ar023
> 4.c
>
> On Mon, 29 Apr 2024 at 06:14, Dongcheng Yan <dongcheng.yan@intel.com>
> wrote:
> >
> > The driver is implemented with V4L2 framework, and supports following
> > features:
> >
> > - manual exposure and analog/digital gain control
> > - vblank/hblank control
> > - runtime PM support
> > - 1280x960 at 30FPS
> >
> > Signed-off-by: Dongcheng Yan <dongcheng.yan@intel.com>
> > ---
> > drivers/media/i2c/Kconfig | 11 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/ar0234.c | 1053
> > ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1065 insertions(+)
> > create mode 100644 drivers/media/i2c/ar0234.c
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 56f276b920ab..4f50a489cfcc 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -51,6 +51,17 @@ config VIDEO_ALVIUM_CSI2
> > To compile this driver as a module, choose M here: the
> > module will be called alvium-csi2.
> >
> > +config VIDEO_AR0234
> > + tristate "ON Semiconductor AR0234 sensor support"
> > + depends on ACPI || COMPILE_TEST
> > + select V4L2_CCI_I2C
> > + help
> > + This is a Video4Linux2 sensor driver for the ON Semiconductor
> > + AR0234 camera.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called ar0234.
> > +
> > config VIDEO_AR0521
> > tristate "ON Semiconductor AR0521 sensor support"
> > help
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index dfbe6448b549..57b4f62106d9 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o
> > obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
> > obj-$(CONFIG_VIDEO_ALVIUM_CSI2) += alvium-csi2.o
> > obj-$(CONFIG_VIDEO_APTINA_PLL) += aptina-pll.o
> > +obj-$(CONFIG_VIDEO_AR0234) += ar0234.o
> > obj-$(CONFIG_VIDEO_AR0521) += ar0521.o
> > obj-$(CONFIG_VIDEO_BT819) += bt819.o
> > obj-$(CONFIG_VIDEO_BT856) += bt856.o
> > diff --git a/drivers/media/i2c/ar0234.c b/drivers/media/i2c/ar0234.c
> > new file mode 100644 index 000000000000..cdddd29d6469
> > --- /dev/null
> > +++ b/drivers/media/i2c/ar0234.c
> > @@ -0,0 +1,1053 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2019 - 2024 Intel Corporation.
> > +
> > +#include <asm/unaligned.h>
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include <media/v4l2-cci.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fwnode.h>
> > +
> > +#define PIXEL_RATE 180000000ULL
> > +
> > +/* Chip ID */
> > +#define AR0234_REG_CHIP_ID CCI_REG16(0x3000)
> > +#define AR0234_CHIP_ID 0x0a56
> > +
> > +#define AR0234_REG_MODE_SELECT CCI_REG16(0x301a)
> > +#define AR0234_MODE_RESET 0x00d9
> > +#define AR0234_MODE_STANDBY 0x2058
> > +#define AR0234_MODE_STREAMING 0x205c
> > +
> > +/* V_TIMING internal */
> > +#define AR0234_REG_VTS CCI_REG16(0x300a)
> > +#define AR0234_VTS_MAX 0xffff
> > +
> > +/* Exposure control */
> > +#define AR0234_REG_EXPOSURE CCI_REG16(0x3012)
> > +#define AR0234_EXPOSURE_MIN 0
> > +#define AR0234_EXPOSURE_MAX_MARGIN 80
> > +#define AR0234_EXPOSURE_STEP 1
> > +
> > +/* Analog gain control */
> > +#define AR0234_REG_ANALOG_GAIN CCI_REG16(0x3060)
> > +#define AR0234_ANAL_GAIN_MIN 0
> > +#define AR0234_ANAL_GAIN_MAX 0x7f
> > +#define AR0234_ANAL_GAIN_STEP 1
> > +#define AR0234_ANAL_GAIN_DEFAULT 0xe
>
> Default gain of 0x0e would be x1.77778. Any particular reason for not having
> the default as x1 ?
>
[Yan, Dongcheng] in ar0234 spec, R0x3060's default value is 0xe. I don't think it's necessary to modify it. We can dialogue again?
> > +
> > +/* Digital gain control */
> > +#define AR0234_REG_GLOBAL_GAIN CCI_REG16(0x305e)
> > +#define AR0234_DGTL_GAIN_MIN 0
> > +#define AR0234_DGTL_GAIN_MAX 0x7ff
> > +#define AR0234_DGTL_GAIN_STEP 1
> > +#define AR0234_DGTL_GAIN_DEFAULT 0x80
> > +
> > +#define AR0234_NATIVE_WIDTH 1280
> > +#define AR0234_NATIVE_HEIGHT 960
>
> AR0234 is a native 1920x1200 sensor at up to 120fps, not 1280x960
> https://www.onsemi.com/products/sensors/image-sensors/ar0234cs
>
[Yan, Dongcheng] You are right, I have revised as your comments, and still support 1280*960.
> > +
> > +#define to_ar0234(_sd) container_of(_sd, struct ar0234, sd)
> > +
> > +struct ar0234_reg_list {
> > + u32 num_of_regs;
> > + const struct cci_reg_sequence *regs; };
> > +
> > +struct ar0234_link_freq_config {
> > + const struct ar0234_reg_list reg_list; };
> > +
> > +struct ar0234_mode {
> > + u32 width;
> > + u32 height;
> > +
> > + u32 vts_def;
> > + u32 vts_min;
> > +
> > + u32 link_freq_index;
> > + u32 code;
> > +
> > + u32 hblank;
> > + u32 vblank;
> > +
> > + /* Sensor register settings for this mode */
> > + const struct ar0234_reg_list reg_list; };
> > +
> > +static const struct cci_reg_sequence
> > +link_freq_360M_1280x960_10bit_2lane[] = {
>
> This array is selected based on link frequency, but the name refers to the
> resolution.
>
> > + { CCI_REG16(0x302a), 0x0005 },
> > + { CCI_REG16(0x302c), 0x0004 },
> > + { CCI_REG16(0x302e), 0x0003 },
> > + { CCI_REG16(0x3030), 0x0050 },
> > + { CCI_REG16(0x3036), 0x000a },
> > + { CCI_REG16(0x3038), 0x0002 },
> > + { CCI_REG16(0x31b0), 0x006e },
> > + { CCI_REG16(0x31b2), 0x0050 },
> > + { CCI_REG16(0x31b4), 0x4207 },
> > + { CCI_REG16(0x31b6), 0x2213 },
> > + { CCI_REG16(0x31b8), 0x704a },
> > + { CCI_REG16(0x31ba), 0x0289 },
> > + { CCI_REG16(0x31bc), 0x8c08 },
> > + { CCI_REG16(0x31ae), 0x0202 },
> > + { CCI_REG16(0x3002), 0x0080 },
> > + { CCI_REG16(0x3004), 0x0148 },
>
> Y_START and X_START of 0x80,0x148 is an offset of 328, 128 (why did OnSemi
> put Y first?)
>
> > + { CCI_REG16(0x3006), 0x043f },
> > + { CCI_REG16(0x3008), 0x0647 },
>
> Y_END, X_END of 0x43f, 0x647, or more logically 1607,1087.
>
> 1087 is 113 pixels from the end of the 1200 lines of array, so we're not centre
> cropped as the left side is 128 lines.
> Likewise 1607 is 313 pixels from the end of the 1920 pixel width, so not
> centred horizontally either.
>
> More fundamentally, why are these registers in a table selected by link
> frequency? They should be in mode_1280x960_10bit_2lane.
[Yan, Dongcheng]
I think you miscalculated here.
Y_START/END are 0x0080/0x043f, logically are 128/1087===>960.
X_START/END are 0x0148/0x0647, logically are 328/1607===>1280.
Just as symmetrical as your setup.
Y_START/END are 0x00d0/0x03ef, logically are 208/1007===>800.
X_START/END are 0x0148/0x0647, logically are 328/1607===>1280.
>
> > + { CCI_REG16(0x300a), 0x05b5 },
>
> Duplicates AR0234_REG_VTS controlled via V4L2_CID_VBLANK, so will be
> programmed via __v4l2_ctrl_handler_setup
>
> > + { CCI_REG16(0x300c), 0x0268 },
>
> LINE_LENGTH_PCK aka HTS. 0x268 is decimal 616. My work said that the
> actual value is this x4, which is 2464. Your mode lists HTS as 3600 +
> 1280 = 4880, so the numbers seem inconsistent.
>
> > + { CCI_REG16(0x3012), 0x058c },
> > + { CCI_REG16(0x31ac), 0x0a0a },
> > + { CCI_REG16(0x306e), 0x9010 },
> > + { CCI_REG16(0x30a2), 0x0001 },
> > + { CCI_REG16(0x30a6), 0x0001 },
> > + { CCI_REG16(0x3082), 0x0003 },
> > + { CCI_REG16(0x3040), 0x0000 },
> > + { CCI_REG16(0x31d0), 0x0000 }, };
> > +
> > +static const struct cci_reg_sequence mode_1280x960_10bit_2lane[] = {
> > + { CCI_REG16(0x3f4c), 0x121f },
> > + { CCI_REG16(0x3f4e), 0x121f },
> > + { CCI_REG16(0x3f50), 0x0b81 },
> > + { CCI_REG16(0x31e0), 0x0003 },
> > + { CCI_REG16(0x30b0), 0x0028 },
> > + /* R0X3088 specify the sequencer RAM access address. */
> > + { CCI_REG16(0x3088), 0x8000 },
> > + /* R0X3086 write the sequencer RAM. */
> > + { CCI_REG16(0x3086), 0xc1ae },
> > + { CCI_REG16(0x3086), 0x327f },
> > + { CCI_REG16(0x3086), 0x5780 },
> > + { CCI_REG16(0x3086), 0x272f },
> > + { CCI_REG16(0x3086), 0x7416 },
> > + { CCI_REG16(0x3086), 0x7e13 },
> > + { CCI_REG16(0x3086), 0x8000 },
> > + { CCI_REG16(0x3086), 0x307e },
> > + { CCI_REG16(0x3086), 0xff80 },
> > + { CCI_REG16(0x3086), 0x20c3 },
> > + { CCI_REG16(0x3086), 0xb00e },
> > + { CCI_REG16(0x3086), 0x8190 },
> > + { CCI_REG16(0x3086), 0x1643 },
> > + { CCI_REG16(0x3086), 0x1651 },
> > + { CCI_REG16(0x3086), 0x9d3e },
> > + { CCI_REG16(0x3086), 0x9545 },
> > + { CCI_REG16(0x3086), 0x2209 },
> > + { CCI_REG16(0x3086), 0x3781 },
> > + { CCI_REG16(0x3086), 0x9016 },
> > + { CCI_REG16(0x3086), 0x4316 },
> > + { CCI_REG16(0x3086), 0x7f90 },
> > + { CCI_REG16(0x3086), 0x8000 },
> > + { CCI_REG16(0x3086), 0x387f },
> > + { CCI_REG16(0x3086), 0x1380 },
> > + { CCI_REG16(0x3086), 0x233b },
> > + { CCI_REG16(0x3086), 0x7f93 },
> > + { CCI_REG16(0x3086), 0x4502 },
> > + { CCI_REG16(0x3086), 0x8000 },
> > + { CCI_REG16(0x3086), 0x7fb0 },
> > + { CCI_REG16(0x3086), 0x8d66 },
> > + { CCI_REG16(0x3086), 0x7f90 },
> > + { CCI_REG16(0x3086), 0x8192 },
> > + { CCI_REG16(0x3086), 0x3c16 },
> > + { CCI_REG16(0x3086), 0x357f },
> > + { CCI_REG16(0x3086), 0x9345 },
> > + { CCI_REG16(0x3086), 0x0280 },
> > + { CCI_REG16(0x3086), 0x007f },
> > + { CCI_REG16(0x3086), 0xb08d },
> > + { CCI_REG16(0x3086), 0x667f },
> > + { CCI_REG16(0x3086), 0x9081 },
> > + { CCI_REG16(0x3086), 0x8237 },
> > + { CCI_REG16(0x3086), 0x4502 },
> > + { CCI_REG16(0x3086), 0x3681 },
> > + { CCI_REG16(0x3086), 0x8044 },
> > + { CCI_REG16(0x3086), 0x1631 },
> > + { CCI_REG16(0x3086), 0x4374 },
> > + { CCI_REG16(0x3086), 0x1678 },
> > + { CCI_REG16(0x3086), 0x7b7d },
> > + { CCI_REG16(0x3086), 0x4502 },
> > + { CCI_REG16(0x3086), 0x450a },
> > + { CCI_REG16(0x3086), 0x7e12 },
> > + { CCI_REG16(0x3086), 0x8180 },
> > + { CCI_REG16(0x3086), 0x377f },
> > + { CCI_REG16(0x3086), 0x1045 },
> > + { CCI_REG16(0x3086), 0x0a0e },
> > + { CCI_REG16(0x3086), 0x7fd4 },
> > + { CCI_REG16(0x3086), 0x8024 },
> > + { CCI_REG16(0x3086), 0x0e82 },
> > + { CCI_REG16(0x3086), 0x9cc2 },
> > + { CCI_REG16(0x3086), 0xafa8 },
> > + { CCI_REG16(0x3086), 0xaa03 },
> > + { CCI_REG16(0x3086), 0x430d },
> > + { CCI_REG16(0x3086), 0x2d46 },
> > + { CCI_REG16(0x3086), 0x4316 },
> > + { CCI_REG16(0x3086), 0x5f16 },
> > + { CCI_REG16(0x3086), 0x530d },
> > + { CCI_REG16(0x3086), 0x1660 },
> > + { CCI_REG16(0x3086), 0x401e },
> > + { CCI_REG16(0x3086), 0x2904 },
> > + { CCI_REG16(0x3086), 0x2984 },
> > + { CCI_REG16(0x3086), 0x81e7 },
> > + { CCI_REG16(0x3086), 0x816f },
> > + { CCI_REG16(0x3086), 0x1706 },
> > + { CCI_REG16(0x3086), 0x81e7 },
> > + { CCI_REG16(0x3086), 0x7f81 },
> > + { CCI_REG16(0x3086), 0x5c0d },
> > + { CCI_REG16(0x3086), 0x5754 },
> > + { CCI_REG16(0x3086), 0x495f },
> > + { CCI_REG16(0x3086), 0x5305 },
> > + { CCI_REG16(0x3086), 0x5307 },
> > + { CCI_REG16(0x3086), 0x4d2b },
> > + { CCI_REG16(0x3086), 0xf810 },
> > + { CCI_REG16(0x3086), 0x164c },
> > + { CCI_REG16(0x3086), 0x0755 },
> > + { CCI_REG16(0x3086), 0x562b },
> > + { CCI_REG16(0x3086), 0xb82b },
> > + { CCI_REG16(0x3086), 0x984e },
> > + { CCI_REG16(0x3086), 0x1129 },
> > + { CCI_REG16(0x3086), 0x9460 },
> > + { CCI_REG16(0x3086), 0x5c09 },
> > + { CCI_REG16(0x3086), 0x5c1b },
> > + { CCI_REG16(0x3086), 0x4002 },
> > + { CCI_REG16(0x3086), 0x4500 },
> > + { CCI_REG16(0x3086), 0x4580 },
> > + { CCI_REG16(0x3086), 0x29b6 },
> > + { CCI_REG16(0x3086), 0x7f80 },
> > + { CCI_REG16(0x3086), 0x4004 },
> > + { CCI_REG16(0x3086), 0x7f88 },
> > + { CCI_REG16(0x3086), 0x4109 },
> > + { CCI_REG16(0x3086), 0x5c0b },
> > + { CCI_REG16(0x3086), 0x29b2 },
> > + { CCI_REG16(0x3086), 0x4115 },
> > + { CCI_REG16(0x3086), 0x5c03 },
> > + { CCI_REG16(0x3086), 0x4105 },
> > + { CCI_REG16(0x3086), 0x5f2b },
> > + { CCI_REG16(0x3086), 0x902b },
> > + { CCI_REG16(0x3086), 0x8081 },
> > + { CCI_REG16(0x3086), 0x6f40 },
> > + { CCI_REG16(0x3086), 0x1041 },
> > + { CCI_REG16(0x3086), 0x0160 },
> > + { CCI_REG16(0x3086), 0x29a2 },
> > + { CCI_REG16(0x3086), 0x29a3 },
> > + { CCI_REG16(0x3086), 0x5f4d },
> > + { CCI_REG16(0x3086), 0x1c17 },
> > + { CCI_REG16(0x3086), 0x0281 },
> > + { CCI_REG16(0x3086), 0xe729 },
> > + { CCI_REG16(0x3086), 0x8345 },
> > + { CCI_REG16(0x3086), 0x8840 },
> > + { CCI_REG16(0x3086), 0x0f7f },
> > + { CCI_REG16(0x3086), 0x8a40 },
> > + { CCI_REG16(0x3086), 0x2345 },
> > + { CCI_REG16(0x3086), 0x8024 },
> > + { CCI_REG16(0x3086), 0x4008 },
> > + { CCI_REG16(0x3086), 0x7f88 },
> > + { CCI_REG16(0x3086), 0x5d29 },
> > + { CCI_REG16(0x3086), 0x9288 },
> > + { CCI_REG16(0x3086), 0x102b },
> > + { CCI_REG16(0x3086), 0x0489 },
> > + { CCI_REG16(0x3086), 0x165c },
> > + { CCI_REG16(0x3086), 0x4386 },
> > + { CCI_REG16(0x3086), 0x170b },
> > + { CCI_REG16(0x3086), 0x5c03 },
> > + { CCI_REG16(0x3086), 0x8a48 },
> > + { CCI_REG16(0x3086), 0x4d4e },
> > + { CCI_REG16(0x3086), 0x2b80 },
> > + { CCI_REG16(0x3086), 0x4c09 },
> > + { CCI_REG16(0x3086), 0x4119 },
> > + { CCI_REG16(0x3086), 0x816f },
> > + { CCI_REG16(0x3086), 0x4110 },
> > + { CCI_REG16(0x3086), 0x4001 },
> > + { CCI_REG16(0x3086), 0x6029 },
> > + { CCI_REG16(0x3086), 0x8229 },
> > + { CCI_REG16(0x3086), 0x8329 },
> > + { CCI_REG16(0x3086), 0x435c },
> > + { CCI_REG16(0x3086), 0x055f },
> > + { CCI_REG16(0x3086), 0x4d1c },
> > + { CCI_REG16(0x3086), 0x81e7 },
> > + { CCI_REG16(0x3086), 0x4502 },
> > + { CCI_REG16(0x3086), 0x8180 },
> > + { CCI_REG16(0x3086), 0x7f80 },
> > + { CCI_REG16(0x3086), 0x410a },
> > + { CCI_REG16(0x3086), 0x9144 },
> > + { CCI_REG16(0x3086), 0x1609 },
> > + { CCI_REG16(0x3086), 0x2fc3 },
> > + { CCI_REG16(0x3086), 0xb130 },
> > + { CCI_REG16(0x3086), 0xc3b1 },
> > + { CCI_REG16(0x3086), 0x0343 },
> > + { CCI_REG16(0x3086), 0x164a },
> > + { CCI_REG16(0x3086), 0x0a43 },
> > + { CCI_REG16(0x3086), 0x160b },
> > + { CCI_REG16(0x3086), 0x4316 },
> > + { CCI_REG16(0x3086), 0x8f43 },
> > + { CCI_REG16(0x3086), 0x1690 },
> > + { CCI_REG16(0x3086), 0x4316 },
> > + { CCI_REG16(0x3086), 0x7f81 },
> > + { CCI_REG16(0x3086), 0x450a },
> > + { CCI_REG16(0x3086), 0x410f },
> > + { CCI_REG16(0x3086), 0x7f83 },
> > + { CCI_REG16(0x3086), 0x5d29 },
> > + { CCI_REG16(0x3086), 0x4488 },
> > + { CCI_REG16(0x3086), 0x102b },
> > + { CCI_REG16(0x3086), 0x0453 },
> > + { CCI_REG16(0x3086), 0x0d40 },
> > + { CCI_REG16(0x3086), 0x2345 },
> > + { CCI_REG16(0x3086), 0x0240 },
> > + { CCI_REG16(0x3086), 0x087f },
> > + { CCI_REG16(0x3086), 0x8053 },
> > + { CCI_REG16(0x3086), 0x0d89 },
> > + { CCI_REG16(0x3086), 0x165c },
> > + { CCI_REG16(0x3086), 0x4586 },
> > + { CCI_REG16(0x3086), 0x170b },
> > + { CCI_REG16(0x3086), 0x5c05 },
> > + { CCI_REG16(0x3086), 0x8a60 },
> > + { CCI_REG16(0x3086), 0x4b91 },
> > + { CCI_REG16(0x3086), 0x4416 },
> > + { CCI_REG16(0x3086), 0x09c1 },
> > + { CCI_REG16(0x3086), 0x2ca9 },
> > + { CCI_REG16(0x3086), 0xab30 },
> > + { CCI_REG16(0x3086), 0x51b3 },
> > + { CCI_REG16(0x3086), 0x3d5a },
> > + { CCI_REG16(0x3086), 0x7e3d },
> > + { CCI_REG16(0x3086), 0x7e19 },
> > + { CCI_REG16(0x3086), 0x8000 },
> > + { CCI_REG16(0x3086), 0x8b1f },
> > + { CCI_REG16(0x3086), 0x2a1f },
> > + { CCI_REG16(0x3086), 0x83a2 },
> > + { CCI_REG16(0x3086), 0x7516 },
> > + { CCI_REG16(0x3086), 0xad33 },
> > + { CCI_REG16(0x3086), 0x450a },
> > + { CCI_REG16(0x3086), 0x7f53 },
> > + { CCI_REG16(0x3086), 0x8023 },
> > + { CCI_REG16(0x3086), 0x8c66 },
> > + { CCI_REG16(0x3086), 0x7f13 },
> > + { CCI_REG16(0x3086), 0x8184 },
> > + { CCI_REG16(0x3086), 0x1481 },
> > + { CCI_REG16(0x3086), 0x8031 },
> > + { CCI_REG16(0x3086), 0x3d64 },
> > + { CCI_REG16(0x3086), 0x452a },
> > + { CCI_REG16(0x3086), 0x9451 },
> > + { CCI_REG16(0x3086), 0x9e96 },
> > + { CCI_REG16(0x3086), 0x3d2b },
> > + { CCI_REG16(0x3086), 0x3d1b },
> > + { CCI_REG16(0x3086), 0x529f },
> > + { CCI_REG16(0x3086), 0x0e3d },
> > + { CCI_REG16(0x3086), 0x083d },
> > + { CCI_REG16(0x3086), 0x167e },
> > + { CCI_REG16(0x3086), 0x307e },
> > + { CCI_REG16(0x3086), 0x1175 },
> > + { CCI_REG16(0x3086), 0x163e },
> > + { CCI_REG16(0x3086), 0x970e },
> > + { CCI_REG16(0x3086), 0x82b2 },
> > + { CCI_REG16(0x3086), 0x3d7f },
> > + { CCI_REG16(0x3086), 0xac3e },
> > + { CCI_REG16(0x3086), 0x4502 },
> > + { CCI_REG16(0x3086), 0x7e11 },
> > + { CCI_REG16(0x3086), 0x7fd0 },
> > + { CCI_REG16(0x3086), 0x8000 },
> > + { CCI_REG16(0x3086), 0x8c66 },
> > + { CCI_REG16(0x3086), 0x7f90 },
> > + { CCI_REG16(0x3086), 0x8194 },
> > + { CCI_REG16(0x3086), 0x3f44 },
> > + { CCI_REG16(0x3086), 0x1681 },
> > + { CCI_REG16(0x3086), 0x8416 },
> > + { CCI_REG16(0x3086), 0x2c2c },
> > + { CCI_REG16(0x3086), 0x2c2c },
> > + { CCI_REG16(0x302a), 0x0005 },
> > + { CCI_REG16(0x302c), 0x0001 },
> > + { CCI_REG16(0x302e), 0x0003 },
> > + { CCI_REG16(0x3030), 0x0032 },
> > + { CCI_REG16(0x3036), 0x000a },
> > + { CCI_REG16(0x3038), 0x0001 },
> > + { CCI_REG16(0x30b0), 0x0028 },
> > + { CCI_REG16(0x31b0), 0x0082 },
> > + { CCI_REG16(0x31b2), 0x005c },
> > + { CCI_REG16(0x31b4), 0x5248 },
> > + { CCI_REG16(0x31b6), 0x3257 },
> > + { CCI_REG16(0x31b8), 0x904b },
> > + { CCI_REG16(0x31ba), 0x030b },
> > + { CCI_REG16(0x31bc), 0x8e09 },
> > + { CCI_REG16(0x3354), 0x002b },
> > + { CCI_REG16(0x31d0), 0x0000 },
> > + { CCI_REG16(0x31ae), 0x0204 },
> > + { CCI_REG16(0x3002), 0x00d0 },
> > + { CCI_REG16(0x3004), 0x0148 },
> > + { CCI_REG16(0x3006), 0x048f },
> > + { CCI_REG16(0x3008), 0x0647 },
>
> We've got 0x3002-0x3008 (start and end offsets) repeated both in
> link_freq_360M_1280x960_10bit_2lane and here. If these are repeated, then
> the whole table needs close scrutiny (sorry, not enough time to check it now).
>
> > + { CCI_REG16(0x3064), 0x1802 },
> > + { CCI_REG16(0x300a), 0x04c4 },
> > + { CCI_REG16(0x300c), 0x04c4 },
> > + { CCI_REG16(0x30a2), 0x0001 },
> > + { CCI_REG16(0x30a6), 0x0001 },
> > + { CCI_REG16(0x3012), 0x010c },
> > + { CCI_REG16(0x3786), 0x0006 },
> > + { CCI_REG16(0x31ae), 0x0202 },
> > + { CCI_REG16(0x3088), 0x8050 },
> > + { CCI_REG16(0x3086), 0x9237 },
> > + { CCI_REG16(0x3044), 0x0410 },
> > + { CCI_REG16(0x3094), 0x03d4 },
> > + { CCI_REG16(0x3096), 0x0280 },
> > + { CCI_REG16(0x30ba), 0x7606 },
> > + { CCI_REG16(0x30b0), 0x0028 },
> > + { CCI_REG16(0x30ba), 0x7600 },
> > + { CCI_REG16(0x30fe), 0x002a },
> > + { CCI_REG16(0x31de), 0x0410 },
> > + { CCI_REG16(0x3ed6), 0x1435 },
> > + { CCI_REG16(0x3ed8), 0x9865 },
> > + { CCI_REG16(0x3eda), 0x7698 },
> > + { CCI_REG16(0x3edc), 0x99ff },
> > + { CCI_REG16(0x3ee2), 0xbb88 },
> > + { CCI_REG16(0x3ee4), 0x8836 },
> > + { CCI_REG16(0x3ef0), 0x1cf0 },
> > + { CCI_REG16(0x3ef2), 0x0000 },
> > + { CCI_REG16(0x3ef8), 0x6166 },
> > + { CCI_REG16(0x3efa), 0x3333 },
> > + { CCI_REG16(0x3efc), 0x6634 },
> > + { CCI_REG16(0x3088), 0x81ba },
> > + { CCI_REG16(0x3086), 0x3d02 },
> > + { CCI_REG16(0x3276), 0x05dc },
> > + { CCI_REG16(0x3f00), 0x9d05 },
> > + { CCI_REG16(0x3ed2), 0xfa86 },
> > + { CCI_REG16(0x3eee), 0xa4fe },
> > + { CCI_REG16(0x3ecc), 0x6e42 },
> > + { CCI_REG16(0x3ecc), 0x0e42 },
> > + { CCI_REG16(0x3eec), 0x0c0c },
> > + { CCI_REG16(0x3ee8), 0xaae4 },
> > + { CCI_REG16(0x3ee6), 0x3363 },
> > + { CCI_REG16(0x3ee6), 0x3363 },
> > + { CCI_REG16(0x3ee8), 0xaae4 },
> > + { CCI_REG16(0x3ee8), 0xaae4 },
> > + { CCI_REG16(0x3180), 0xc24f },
> > + { CCI_REG16(0x3102), 0x5000 },
> > + { CCI_REG16(0x3060), 0x000d },
> > + { CCI_REG16(0x3ed0), 0xff44 },
> > + { CCI_REG16(0x3ed2), 0xaa86 },
> > + { CCI_REG16(0x3ed4), 0x031f },
> > + { CCI_REG16(0x3eee), 0xa4aa }, };
> > +
> > +static const s64 link_freq_menu_items[] = {
> > + 360000000ULL,
> > +};
> > +
> > +static const struct ar0234_link_freq_config link_freq_configs[] = {
> > + {
> > + .reg_list = {
> > + .num_of_regs =
> > +
> ARRAY_SIZE(link_freq_360M_1280x960_10bit_2lane),
> > + .regs = link_freq_360M_1280x960_10bit_2lane,
> > + }
> > + },
> > +};
> > +
> > +static const struct ar0234_mode supported_modes[] = {
> > + {
> > + .width = AR0234_NATIVE_WIDTH,
> > + .height = AR0234_NATIVE_HEIGHT,
> > + .vts_def = 2435,
> > + .vts_min = 2435,
>
> Really, or is that just the minimum you've used it with? My testing appears to
> have found that VTS only needs to be image height + 4.
[Yan, Dongcheng] Yes, vts_def needs to be set, vts_min will be removed because actually I set to 0 in vblank v4l2_ctl init and modify, and vts_def are set to 0x4c4 that same to reg setting . I'm confused that all your modes are 60fps but you used "AR0234_VTS_30FPS 0x04c4". Could your driver run in proper fps?
>
> > + .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > + .reg_list = {
> > + .num_of_regs =
> ARRAY_SIZE(mode_1280x960_10bit_2lane),
> > + .regs = mode_1280x960_10bit_2lane,
> > + },
> > + .link_freq_index = 0,
> > + .hblank = 3600,
>
> Queried above as that doesn't appear to match the register values.
> I do note that this is V4L2_CID_HBLANK value, so it is NOT the HTS value in the
> registers. There could be some debate as to which is the better domain to work
> in.
>
> > + .vblank = 1475,
>
> That's vts_min - height (2435 - 960 = 1475). Duplicated data like that increases
> the chance of errors when adding new modes.
>
> > + },
> > +};
> > +
> > +struct ar0234 {
> > + struct v4l2_subdev sd;
> > + struct media_pad pad;
> > + struct v4l2_ctrl_handler ctrl_handler;
> > +
> > + /* V4L2 Controls */
> > + struct v4l2_ctrl *link_freq;
> > + struct v4l2_ctrl *exposure;
> > + struct v4l2_ctrl *analogue_gain;
> > + struct v4l2_ctrl *digital_gain;
>
> Neither analogue_gain nor digital_gain are ever written to or read, so no need
> to store it.
>
> > + struct v4l2_ctrl *pixel_rate;
>
> Ditto that it's never accessed once initialised.
>
> > + struct v4l2_ctrl *hblank;
> > + struct v4l2_ctrl *vblank;
> > +
> > + struct regmap *regmap;
> > + unsigned long link_freq_bitmap;
> > + const struct ar0234_mode *cur_mode; };
> > +
> > +static int ar0234_set_ctrl(struct v4l2_ctrl *ctrl) {
> > + struct ar0234 *ar0234 =
> > + container_of(ctrl->handler, struct ar0234, ctrl_handler);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > + s64 exposure_max, exposure_def;
> > + struct v4l2_subdev_state *state;
> > + const struct v4l2_mbus_framefmt *format;
> > + int ret;
> > +
> > + state = v4l2_subdev_get_locked_active_state(&ar0234->sd);
> > + format = v4l2_subdev_state_get_format(state, 0);
> > +
> > + /* Propagate change of current control to all related controls */
> > + if (ctrl->id == V4L2_CID_VBLANK) {
> > + /* Update max exposure while meeting expected
> vblanking */
> > + exposure_max = format->height + ctrl->val -
> > + AR0234_EXPOSURE_MAX_MARGIN;
> > + exposure_def = format->height -
> AR0234_EXPOSURE_MAX_MARGIN;
> > + __v4l2_ctrl_modify_range(ar0234->exposure,
> > +
> ar0234->exposure->minimum,
> > + exposure_max,
> ar0234->exposure->step,
> > + exposure_def);
> > + }
> > +
> > + /* V4L2 controls values will be applied only when power is already
> up */
> > + if (!pm_runtime_get_if_in_use(&client->dev))
> > + return 0;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_ANALOGUE_GAIN:
> > + ret = cci_write(ar0234->regmap,
> AR0234_REG_ANALOG_GAIN,
> > + ctrl->val, NULL);
> > + break;
> > +
> > + case V4L2_CID_DIGITAL_GAIN:
> > + ret = cci_write(ar0234->regmap,
> AR0234_REG_GLOBAL_GAIN,
> > + ctrl->val, NULL);
> > + break;
> > +
> > + case V4L2_CID_EXPOSURE:
> > + ret = cci_write(ar0234->regmap, AR0234_REG_EXPOSURE,
> > + ctrl->val, NULL);
> > + break;
> > +
> > + case V4L2_CID_VBLANK:
> > + ret = cci_write(ar0234->regmap, AR0234_REG_VTS,
> > + ar0234->cur_mode->height + ctrl->val,
> NULL);
> > + break;
> > +
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + pm_runtime_put(&client->dev);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops ar0234_ctrl_ops = {
> > + .s_ctrl = ar0234_set_ctrl,
> > +};
> > +
> > +static int ar0234_init_controls(struct ar0234 *ar0234) {
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > + struct v4l2_fwnode_device_properties props;
> > + struct v4l2_ctrl_handler *ctrl_hdlr;
> > + s64 exposure_max, vblank, hblank;
> > + u32 link_freq_size;
> > + int ret;
> > +
> > + ctrl_hdlr = &ar0234->ctrl_handler;
> > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> > + if (ret)
> > + return ret;
> > +
> > + link_freq_size = ARRAY_SIZE(link_freq_menu_items) - 1;
> > + ar0234->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > +
> &ar0234_ctrl_ops,
> > +
> V4L2_CID_LINK_FREQ,
> > + link_freq_size, 0,
> > +
> link_freq_menu_items);
> > + if (ar0234->link_freq)
> > + ar0234->link_freq->flags |=
> V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops,
> V4L2_CID_ANALOGUE_GAIN,
> > + AR0234_ANAL_GAIN_MIN,
> AR0234_ANAL_GAIN_MAX,
> > + AR0234_ANAL_GAIN_STEP,
> AR0234_ANAL_GAIN_DEFAULT);
> > + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops,
> V4L2_CID_DIGITAL_GAIN,
> > + AR0234_DGTL_GAIN_MIN,
> AR0234_DGTL_GAIN_MAX,
> > + AR0234_DGTL_GAIN_STEP,
> > + AR0234_DGTL_GAIN_DEFAULT);
> > +
> > + exposure_max = ar0234->cur_mode->vts_def -
> AR0234_EXPOSURE_MAX_MARGIN;
> > + ar0234->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops,
> > + V4L2_CID_EXPOSURE,
> > +
> AR0234_EXPOSURE_MIN, exposure_max,
> > +
> AR0234_EXPOSURE_STEP,
> > + exposure_max);
> > +
> > + ar0234->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr,
> &ar0234_ctrl_ops,
> > +
> V4L2_CID_PIXEL_RATE,
> > + PIXEL_RATE,
> > + PIXEL_RATE, 1,
> > + PIXEL_RATE);
> > + if (ar0234->pixel_rate)
> > + ar0234->pixel_rate->flags |=
> V4L2_CTRL_FLAG_READ_ONLY;
>
> V4L2_CID_PIXEL_RATE defaults to read only by the core (v4l2_ctrl_fill), so no
> need to set it manually.
>
> > +
> > + vblank = AR0234_VTS_MAX - ar0234->cur_mode->height;
> > + ar0234->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops,
> > + V4L2_CID_VBLANK, 0,
> vblank, 1,
> > +
> ar0234->cur_mode->vblank);
> > + hblank = ar0234->cur_mode->hblank;
> > + ar0234->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops,
> > + V4L2_CID_HBLANK,
> hblank, hblank, 1,
> > + hblank);
> > + if (ar0234->hblank)
> > + ar0234->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
>
> It would be nice to add V4L2_CID_HFLIP and V4L2_CID_VFLIP too. It's fairly
> easy on this module as it doesn't change the Bayer order.
> Register 0x3040 bits 14 & 15, or 0x301d bits 0 & 1.
>
> > + if (ctrl_hdlr->error)
> > + return ctrl_hdlr->error;
> > +
> > + ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > + if (ret)
> > + return ret;
> > +
> > + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr,
> &ar0234_ctrl_ops,
> > + &props);
> > + if (ret)
> > + return ret;
> > +
> > + ar0234->sd.ctrl_handler = ctrl_hdlr;
> > +
> > + return 0;
> > +}
> > +
> > +static void ar0234_update_pad_format(const struct ar0234_mode *mode,
> > + struct v4l2_mbus_framefmt *fmt)
> {
> > + fmt->width = mode->width;
> > + fmt->height = mode->height;
> > + fmt->code = mode->code;
> > + fmt->field = V4L2_FIELD_NONE;
> > +}
> > +
> > +static int ar0234_start_streaming(struct ar0234 *ar0234) {
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > + const struct ar0234_reg_list *reg_list;
> > + int link_freq_index, ret;
> > +
> > + ret = pm_runtime_resume_and_get(&client->dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * Setting 0X301A.bit[0] will initiate a reset sequence:
> > + * the frame being generated will be truncated.
> > + */
> > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT,
> > + AR0234_MODE_RESET, NULL);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to reset");
> > + goto err_rpm_put;
> > + }
> > +
> > + usleep_range(1000, 1500);
> > +
> > + reg_list = &ar0234->cur_mode->reg_list;
> > + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs,
> > + reg_list->num_of_regs, NULL);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to set mode");
> > + goto err_rpm_put;
> > + }
> > +
> > + link_freq_index = ar0234->cur_mode->link_freq_index;
> > + if (link_freq_index > 0) {
> > + reg_list = &link_freq_configs[link_freq_index].reg_list;
> > + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs,
> > + reg_list->num_of_regs,
> NULL);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to set plls");
> > + goto err_rpm_put;
> > + }
> > + }
> > +
> > + ret = __v4l2_ctrl_handler_setup(ar0234->sd.ctrl_handler);
> > + if (ret)
> > + goto err_rpm_put;
> > +
> > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT,
> > + AR0234_MODE_STREAMING, NULL);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to start stream");
> > + goto err_rpm_put;
> > + }
> > +
> > + return 0;
> > +
> > +err_rpm_put:
> > + pm_runtime_put(&client->dev);
> > + return ret;
> > +}
> > +
> > +static int ar0234_stop_streaming(struct ar0234 *ar0234) {
> > + int ret;
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > +
> > + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT,
> > + AR0234_MODE_STANDBY, NULL);
> > + if (ret < 0)
> > + dev_err(&client->dev, "failed to stop stream");
> > +
> > + pm_runtime_put(&client->dev);
> > + return ret;
> > +}
> > +
> > +static int ar0234_set_stream(struct v4l2_subdev *sd, int enable) {
> > + struct ar0234 *ar0234 = to_ar0234(sd);
> > + struct v4l2_subdev_state *state;
> > + int ret = 0;
> > +
> > + state = v4l2_subdev_lock_and_get_active_state(sd);
> > +
> > + if (enable)
> > + ret = ar0234_start_streaming(ar0234);
> > + else
> > + ret = ar0234_stop_streaming(ar0234);
> > +
> > + v4l2_subdev_unlock_state(state);
> > +
> > + return ret;
> > +}
> > +
> > +static int ar0234_set_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state,
> > + struct v4l2_subdev_format *fmt) {
> > + struct ar0234 *ar0234 = to_ar0234(sd);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > + struct v4l2_rect *crop;
> > + const struct ar0234_mode *mode;
> > + s64 hblank;
> > + int ret;
> > +
> > + mode = v4l2_find_nearest_size(supported_modes,
> > + ARRAY_SIZE(supported_modes),
> > + width, height,
> > + fmt->format.width,
> > + fmt->format.height);
> > +
> > + crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
> > + crop->width = mode->width;
> > + crop->height = mode->height;
> > +
> > + ar0234_update_pad_format(mode, &fmt->format);
> > + *v4l2_subdev_state_get_format(sd_state, fmt->pad) =
> > + fmt->format;
> > +
> > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > + return 0;
> > +
> > + ar0234->cur_mode = mode;
> > + ret = __v4l2_ctrl_s_ctrl(ar0234->link_freq, mode->link_freq_index);
> > + if (ret) {
> > + dev_err(&client->dev, "Link Freq ctrl set failed\n");
> > + return ret;
> > + }
> > +
> > + hblank = ar0234->cur_mode->hblank;
> > + ret = __v4l2_ctrl_modify_range(ar0234->hblank, hblank, hblank,
> > + 1, hblank);
> > + if (ret) {
> > + dev_err(&client->dev, "HB ctrl range update failed\n");
> > + return ret;
> > + }
> > +
> > + /* Update limits and set FPS to default */
> > + ret = __v4l2_ctrl_modify_range(ar0234->vblank, 0,
> > + AR0234_VTS_MAX -
> mode->height, 1,
> > + ar0234->cur_mode->vblank);
> > + if (ret) {
> > + dev_err(&client->dev, "VB ctrl range update failed\n");
> > + return ret;
> > + }
> > +
> > + ret = __v4l2_ctrl_s_ctrl(ar0234->vblank,
> ar0234->cur_mode->vblank);
> > + if (ret) {
> > + dev_err(&client->dev, "VB ctrl set failed\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ar0234_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state,
> > + struct v4l2_subdev_mbus_code_enum
> > +*code) {
> > + if (code->index > 0)
> > + return -EINVAL;
> > +
> > + code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > +
> > + return 0;
> > +}
> > +
> > +static int ar0234_enum_frame_size(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state,
> > + struct
> v4l2_subdev_frame_size_enum
> > +*fse) {
> > + if (fse->index >= ARRAY_SIZE(supported_modes))
> > + return -EINVAL;
> > +
> > + if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
> > + return -EINVAL;
> > +
> > + fse->min_width = supported_modes[fse->index].width;
> > + fse->max_width = fse->min_width;
> > + fse->min_height = supported_modes[fse->index].height;
> > + fse->max_height = fse->min_height;
> > +
> > + return 0;
> > +}
> > +
> > +static int ar0234_get_selection(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + struct v4l2_subdev_selection *sel) {
> > + switch (sel->target) {
> > + case V4L2_SEL_TGT_CROP_DEFAULT:
> > + case V4L2_SEL_TGT_CROP:
> > + sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > + break;
> > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > + sel->r.top = 0;
> > + sel->r.left = 0;
> > + sel->r.width = AR0234_NATIVE_WIDTH;
> > + sel->r.height = AR0234_NATIVE_HEIGHT;
> > + break;
>
> The numbers look wrong here based on the fact the mode is cropped.
>
> Implement V4L2_SEL_TGT_NATIVE_SIZE as well?
>
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ar0234_init_state(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *sd_state) {
> > + struct v4l2_subdev_format fmt = {
> > + .which = V4L2_SUBDEV_FORMAT_TRY,
> > + .pad = 0,
> > + .format = {
> > + .code = MEDIA_BUS_FMT_SGRBG10_1X10,
> > + .width = AR0234_NATIVE_WIDTH,
> > + .height = AR0234_NATIVE_HEIGHT,
> > + },
> > + };
> > +
> > + ar0234_set_format(sd, sd_state, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops ar0234_video_ops = {
> > + .s_stream = ar0234_set_stream, };
> > +
> > +static const struct v4l2_subdev_pad_ops ar0234_pad_ops = {
> > + .set_fmt = ar0234_set_format,
> > + .get_fmt = v4l2_subdev_get_fmt,
> > + .enum_mbus_code = ar0234_enum_mbus_code,
> > + .enum_frame_size = ar0234_enum_frame_size,
> > + .get_selection = ar0234_get_selection, };
> > +
> > +static const struct v4l2_subdev_core_ops ar0234_core_ops = {
> > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, };
> > +
> > +static const struct v4l2_subdev_ops ar0234_subdev_ops = {
> > + .core = &ar0234_core_ops,
> > + .video = &ar0234_video_ops,
> > + .pad = &ar0234_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations ar0234_subdev_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate, };
> > +
> > +static const struct v4l2_subdev_internal_ops ar0234_internal_ops = {
> > + .init_state = ar0234_init_state, };
> > +
> > +static int ar0234_parse_fwnode(struct ar0234 *ar0234, struct device
> > +*dev) {
> > + struct fwnode_handle *endpoint;
> > + struct v4l2_fwnode_endpoint bus_cfg = {
> > + .bus_type = V4L2_MBUS_CSI2_DPHY,
> > + };
> > + int ret;
> > +
> > + endpoint =
> > + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> 0,
> > +
> FWNODE_GRAPH_ENDPOINT_NEXT);
> > + if (!endpoint) {
> > + dev_err(dev, "endpoint node not found\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
> > + if (ret) {
> > + dev_err(dev, "parsing endpoint node failed\n");
> > + goto out_err;
> > + }
> > +
> > + ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies,
> > +
> bus_cfg.nr_of_link_frequencies,
> > + link_freq_menu_items,
> > +
> ARRAY_SIZE(link_freq_menu_items),
> > + &ar0234->link_freq_bitmap);
> > + if (ret)
> > + goto out_err;
>
> Validate the number of data lanes too? Whilst this driver currently only
> supports 2 lanes, the sensor supports 1, 2, or 4.
>
> Your register configuration also puts the sensor in a continuous clock mode. It
> would be nice to at least document that, if not check for it in the
> configuration.
>
[Yan, Dongcheng] Since our scene only requires 2 lanes, I thought that additional configurations should be added to reg setting lists when there is a demand (maybe after upstream), so I just set 0x0202 to 0x31ae. Conversely, you set its value dynamically in start_streaming func.
Lanes validation is necessary and I add it based on your patch and thanks your work.
Continuous Clock? I can't see it. Can you please indicate which register controls this mode?
> > +
> > +out_err:
> > + v4l2_fwnode_endpoint_free(&bus_cfg);
> > + fwnode_handle_put(endpoint);
> > + return ret;
> > +}
> > +
> > +static int ar0234_identify_module(struct ar0234 *ar0234) {
> > + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd);
> > + int ret;
> > + u64 val;
> > +
> > + ret = cci_read(ar0234->regmap, AR0234_REG_CHIP_ID, &val,
> > + NULL);
>
> An easy future extension is to add support for the mono variant of the sensor.
> It reports as CHIP_ID 0x1a56. This isn't a request to add it now, but more
> requesting that we don't make it hard to implement later (simplest support
> just changes the MBUS_CODE.
>
[Yan, Dongcheng] Sounds very attractive, I will keep an eye on this feature.
> > + if (ret)
> > + return ret;
> > +
> > + if (val != AR0234_CHIP_ID) {
> > + dev_err(&client->dev, "chip id mismatch: %x!=%llx",
> > + AR0234_CHIP_ID, val);
> > + return -ENXIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void ar0234_remove(struct i2c_client *client) {
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ar0234 *ar0234 = to_ar0234(sd);
> > +
> > + v4l2_async_unregister_subdev(&ar0234->sd);
> > + v4l2_subdev_cleanup(sd);
> > + media_entity_cleanup(&ar0234->sd.entity);
> > + v4l2_ctrl_handler_free(&ar0234->ctrl_handler);
> > + pm_runtime_disable(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > +}
> > +
> > +static int ar0234_probe(struct i2c_client *client) {
> > + struct device *dev = &client->dev;
> > + struct ar0234 *ar0234;
> > + int ret;
> > +
> > + ar0234 = devm_kzalloc(&client->dev, sizeof(*ar0234),
> GFP_KERNEL);
> > + if (!ar0234)
> > + return -ENOMEM;
> > +
> > + ret = ar0234_parse_fwnode(ar0234, dev);
> > + if (ret)
> > + return ret;
>
> You don't look for a clock feeding the sensor. What frequency is that clock at?
> The PLL settings you program into the registers are going to be dependent on
> that.
> On the basis that Intel normally use 19.2MHz clocks for their sensors, plugging
> your register settings in I get a pixel rate of 51.2MPix/s, not the 180MPix/s
> defined as PIXEL_RATE. I could be totally wrong, but that makes me dubious of
> almost all the timing values.
>
> If I use the register settings from my driver with the 24MHz clock that it
> expects, the pixel clock does come out as 180MPix/s. To get the expected
> numbers out with your register settings, I think you'd need an external clock of
> 67.5MHz which is out of spec (max 54 MHz), and the PLL will be out of spec
> too.
>
> Dave
>
[Yan, Dongcheng] Thanks, I added xclk check. Because of using ACPI, I don't need to concern other pm_resources anymore, so I just check xclk and don't add other resources like poweron/reset.
> > +
> > + ar0234->regmap = devm_cci_regmap_init_i2c(client, 16);
> > + if (IS_ERR(ar0234->regmap))
> > + return dev_err_probe(dev, PTR_ERR(ar0234->regmap),
> > + "failed to init CCI\n");
> > +
> > + v4l2_i2c_subdev_init(&ar0234->sd, client, &ar0234_subdev_ops);
> > +
> > + ret = ar0234_identify_module(ar0234);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to find sensor: %d", ret);
> > + return ret;
> > + }
> > +
> > + /* Set default mode to max resolution */
> > + ar0234->cur_mode = &supported_modes[0];
> > + ret = ar0234_init_controls(ar0234);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to init controls: %d", ret);
> > + goto probe_error_v4l2_ctrl_handler_free;
> > + }
> > +
> > + ar0234->sd.internal_ops = &ar0234_internal_ops;
> > + ar0234->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > + V4L2_SUBDEV_FL_HAS_EVENTS;
> > + ar0234->sd.entity.ops = &ar0234_subdev_entity_ops;
> > + ar0234->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + ar0234->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + ret = media_entity_pads_init(&ar0234->sd.entity, 1,
> &ar0234->pad);
> > + if (ret) {
> > + dev_err(&client->dev, "failed to init entity pads: %d", ret);
> > + goto probe_error_v4l2_ctrl_handler_free;
> > + }
> > +
> > + ar0234->sd.state_lock = ar0234->ctrl_handler.lock;
> > + ret = v4l2_subdev_init_finalize(&ar0234->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "v4l2 subdev init error: %d\n", ret);
> > + goto probe_error_media_entity_cleanup;
> > + }
> > +
> > + /*
> > + * Device is already turned on by i2c-core with ACPI domain PM.
> > + * Enable runtime PM and turn off the device.
> > + */
> > + pm_runtime_set_active(&client->dev);
> > + pm_runtime_enable(&client->dev);
> > + pm_runtime_idle(&client->dev);
> > +
> > + ret = v4l2_async_register_subdev_sensor(&ar0234->sd);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "failed to register V4L2 subdev: %d",
> > + ret);
> > + goto probe_error_rpm;
> > + }
> > +
> > + return 0;
> > +probe_error_rpm:
> > + pm_runtime_disable(&client->dev);
> > + v4l2_subdev_cleanup(&ar0234->sd);
> > +
> > +probe_error_media_entity_cleanup:
> > + media_entity_cleanup(&ar0234->sd.entity);
> > +
> > +probe_error_v4l2_ctrl_handler_free:
> > + v4l2_ctrl_handler_free(ar0234->sd.ctrl_handler);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct acpi_device_id ar0234_acpi_ids[] = {
> > + { "INTC10C0" },
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(acpi, ar0234_acpi_ids);
> > +
> > +static struct i2c_driver ar0234_i2c_driver = {
> > + .driver = {
> > + .name = "ar0234",
> > + .acpi_match_table = ACPI_PTR(ar0234_acpi_ids),
> > + },
> > + .probe = ar0234_probe,
> > + .remove = ar0234_remove,
> > +};
> > +
> > +module_i2c_driver(ar0234_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("ON Semiconductor ar0234 sensor driver");
> > +MODULE_AUTHOR("Dongcheng Yan <dongcheng.yan@intel.com>");
> > +MODULE_AUTHOR("Hao Yao <hao.yao@intel.com>");
> MODULE_LICENSE("GPL");
> > --
> > 2.34.1
> >
> >
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 56f276b920ab..4f50a489cfcc 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -51,6 +51,17 @@ config VIDEO_ALVIUM_CSI2 To compile this driver as a module, choose M here: the module will be called alvium-csi2. +config VIDEO_AR0234 + tristate "ON Semiconductor AR0234 sensor support" + depends on ACPI || COMPILE_TEST + select V4L2_CCI_I2C + help + This is a Video4Linux2 sensor driver for the ON Semiconductor + AR0234 camera. + + To compile this driver as a module, choose M here: the + module will be called ar0234. + config VIDEO_AR0521 tristate "ON Semiconductor AR0521 sensor support" help diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index dfbe6448b549..57b4f62106d9 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_VIDEO_AK7375) += ak7375.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_ALVIUM_CSI2) += alvium-csi2.o obj-$(CONFIG_VIDEO_APTINA_PLL) += aptina-pll.o +obj-$(CONFIG_VIDEO_AR0234) += ar0234.o obj-$(CONFIG_VIDEO_AR0521) += ar0521.o obj-$(CONFIG_VIDEO_BT819) += bt819.o obj-$(CONFIG_VIDEO_BT856) += bt856.o diff --git a/drivers/media/i2c/ar0234.c b/drivers/media/i2c/ar0234.c new file mode 100644 index 000000000000..cdddd29d6469 --- /dev/null +++ b/drivers/media/i2c/ar0234.c @@ -0,0 +1,1053 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2019 - 2024 Intel Corporation. + +#include <asm/unaligned.h> +#include <linux/acpi.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> + +#include <media/v4l2-cci.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-event.h> +#include <media/v4l2-device.h> +#include <media/v4l2-fwnode.h> + +#define PIXEL_RATE 180000000ULL + +/* Chip ID */ +#define AR0234_REG_CHIP_ID CCI_REG16(0x3000) +#define AR0234_CHIP_ID 0x0a56 + +#define AR0234_REG_MODE_SELECT CCI_REG16(0x301a) +#define AR0234_MODE_RESET 0x00d9 +#define AR0234_MODE_STANDBY 0x2058 +#define AR0234_MODE_STREAMING 0x205c + +/* V_TIMING internal */ +#define AR0234_REG_VTS CCI_REG16(0x300a) +#define AR0234_VTS_MAX 0xffff + +/* Exposure control */ +#define AR0234_REG_EXPOSURE CCI_REG16(0x3012) +#define AR0234_EXPOSURE_MIN 0 +#define AR0234_EXPOSURE_MAX_MARGIN 80 +#define AR0234_EXPOSURE_STEP 1 + +/* Analog gain control */ +#define AR0234_REG_ANALOG_GAIN CCI_REG16(0x3060) +#define AR0234_ANAL_GAIN_MIN 0 +#define AR0234_ANAL_GAIN_MAX 0x7f +#define AR0234_ANAL_GAIN_STEP 1 +#define AR0234_ANAL_GAIN_DEFAULT 0xe + +/* Digital gain control */ +#define AR0234_REG_GLOBAL_GAIN CCI_REG16(0x305e) +#define AR0234_DGTL_GAIN_MIN 0 +#define AR0234_DGTL_GAIN_MAX 0x7ff +#define AR0234_DGTL_GAIN_STEP 1 +#define AR0234_DGTL_GAIN_DEFAULT 0x80 + +#define AR0234_NATIVE_WIDTH 1280 +#define AR0234_NATIVE_HEIGHT 960 + +#define to_ar0234(_sd) container_of(_sd, struct ar0234, sd) + +struct ar0234_reg_list { + u32 num_of_regs; + const struct cci_reg_sequence *regs; +}; + +struct ar0234_link_freq_config { + const struct ar0234_reg_list reg_list; +}; + +struct ar0234_mode { + u32 width; + u32 height; + + u32 vts_def; + u32 vts_min; + + u32 link_freq_index; + u32 code; + + u32 hblank; + u32 vblank; + + /* Sensor register settings for this mode */ + const struct ar0234_reg_list reg_list; +}; + +static const struct cci_reg_sequence link_freq_360M_1280x960_10bit_2lane[] = { + { CCI_REG16(0x302a), 0x0005 }, + { CCI_REG16(0x302c), 0x0004 }, + { CCI_REG16(0x302e), 0x0003 }, + { CCI_REG16(0x3030), 0x0050 }, + { CCI_REG16(0x3036), 0x000a }, + { CCI_REG16(0x3038), 0x0002 }, + { CCI_REG16(0x31b0), 0x006e }, + { CCI_REG16(0x31b2), 0x0050 }, + { CCI_REG16(0x31b4), 0x4207 }, + { CCI_REG16(0x31b6), 0x2213 }, + { CCI_REG16(0x31b8), 0x704a }, + { CCI_REG16(0x31ba), 0x0289 }, + { CCI_REG16(0x31bc), 0x8c08 }, + { CCI_REG16(0x31ae), 0x0202 }, + { CCI_REG16(0x3002), 0x0080 }, + { CCI_REG16(0x3004), 0x0148 }, + { CCI_REG16(0x3006), 0x043f }, + { CCI_REG16(0x3008), 0x0647 }, + { CCI_REG16(0x300a), 0x05b5 }, + { CCI_REG16(0x300c), 0x0268 }, + { CCI_REG16(0x3012), 0x058c }, + { CCI_REG16(0x31ac), 0x0a0a }, + { CCI_REG16(0x306e), 0x9010 }, + { CCI_REG16(0x30a2), 0x0001 }, + { CCI_REG16(0x30a6), 0x0001 }, + { CCI_REG16(0x3082), 0x0003 }, + { CCI_REG16(0x3040), 0x0000 }, + { CCI_REG16(0x31d0), 0x0000 }, +}; + +static const struct cci_reg_sequence mode_1280x960_10bit_2lane[] = { + { CCI_REG16(0x3f4c), 0x121f }, + { CCI_REG16(0x3f4e), 0x121f }, + { CCI_REG16(0x3f50), 0x0b81 }, + { CCI_REG16(0x31e0), 0x0003 }, + { CCI_REG16(0x30b0), 0x0028 }, + /* R0X3088 specify the sequencer RAM access address. */ + { CCI_REG16(0x3088), 0x8000 }, + /* R0X3086 write the sequencer RAM. */ + { CCI_REG16(0x3086), 0xc1ae }, + { CCI_REG16(0x3086), 0x327f }, + { CCI_REG16(0x3086), 0x5780 }, + { CCI_REG16(0x3086), 0x272f }, + { CCI_REG16(0x3086), 0x7416 }, + { CCI_REG16(0x3086), 0x7e13 }, + { CCI_REG16(0x3086), 0x8000 }, + { CCI_REG16(0x3086), 0x307e }, + { CCI_REG16(0x3086), 0xff80 }, + { CCI_REG16(0x3086), 0x20c3 }, + { CCI_REG16(0x3086), 0xb00e }, + { CCI_REG16(0x3086), 0x8190 }, + { CCI_REG16(0x3086), 0x1643 }, + { CCI_REG16(0x3086), 0x1651 }, + { CCI_REG16(0x3086), 0x9d3e }, + { CCI_REG16(0x3086), 0x9545 }, + { CCI_REG16(0x3086), 0x2209 }, + { CCI_REG16(0x3086), 0x3781 }, + { CCI_REG16(0x3086), 0x9016 }, + { CCI_REG16(0x3086), 0x4316 }, + { CCI_REG16(0x3086), 0x7f90 }, + { CCI_REG16(0x3086), 0x8000 }, + { CCI_REG16(0x3086), 0x387f }, + { CCI_REG16(0x3086), 0x1380 }, + { CCI_REG16(0x3086), 0x233b }, + { CCI_REG16(0x3086), 0x7f93 }, + { CCI_REG16(0x3086), 0x4502 }, + { CCI_REG16(0x3086), 0x8000 }, + { CCI_REG16(0x3086), 0x7fb0 }, + { CCI_REG16(0x3086), 0x8d66 }, + { CCI_REG16(0x3086), 0x7f90 }, + { CCI_REG16(0x3086), 0x8192 }, + { CCI_REG16(0x3086), 0x3c16 }, + { CCI_REG16(0x3086), 0x357f }, + { CCI_REG16(0x3086), 0x9345 }, + { CCI_REG16(0x3086), 0x0280 }, + { CCI_REG16(0x3086), 0x007f }, + { CCI_REG16(0x3086), 0xb08d }, + { CCI_REG16(0x3086), 0x667f }, + { CCI_REG16(0x3086), 0x9081 }, + { CCI_REG16(0x3086), 0x8237 }, + { CCI_REG16(0x3086), 0x4502 }, + { CCI_REG16(0x3086), 0x3681 }, + { CCI_REG16(0x3086), 0x8044 }, + { CCI_REG16(0x3086), 0x1631 }, + { CCI_REG16(0x3086), 0x4374 }, + { CCI_REG16(0x3086), 0x1678 }, + { CCI_REG16(0x3086), 0x7b7d }, + { CCI_REG16(0x3086), 0x4502 }, + { CCI_REG16(0x3086), 0x450a }, + { CCI_REG16(0x3086), 0x7e12 }, + { CCI_REG16(0x3086), 0x8180 }, + { CCI_REG16(0x3086), 0x377f }, + { CCI_REG16(0x3086), 0x1045 }, + { CCI_REG16(0x3086), 0x0a0e }, + { CCI_REG16(0x3086), 0x7fd4 }, + { CCI_REG16(0x3086), 0x8024 }, + { CCI_REG16(0x3086), 0x0e82 }, + { CCI_REG16(0x3086), 0x9cc2 }, + { CCI_REG16(0x3086), 0xafa8 }, + { CCI_REG16(0x3086), 0xaa03 }, + { CCI_REG16(0x3086), 0x430d }, + { CCI_REG16(0x3086), 0x2d46 }, + { CCI_REG16(0x3086), 0x4316 }, + { CCI_REG16(0x3086), 0x5f16 }, + { CCI_REG16(0x3086), 0x530d }, + { CCI_REG16(0x3086), 0x1660 }, + { CCI_REG16(0x3086), 0x401e }, + { CCI_REG16(0x3086), 0x2904 }, + { CCI_REG16(0x3086), 0x2984 }, + { CCI_REG16(0x3086), 0x81e7 }, + { CCI_REG16(0x3086), 0x816f }, + { CCI_REG16(0x3086), 0x1706 }, + { CCI_REG16(0x3086), 0x81e7 }, + { CCI_REG16(0x3086), 0x7f81 }, + { CCI_REG16(0x3086), 0x5c0d }, + { CCI_REG16(0x3086), 0x5754 }, + { CCI_REG16(0x3086), 0x495f }, + { CCI_REG16(0x3086), 0x5305 }, + { CCI_REG16(0x3086), 0x5307 }, + { CCI_REG16(0x3086), 0x4d2b }, + { CCI_REG16(0x3086), 0xf810 }, + { CCI_REG16(0x3086), 0x164c }, + { CCI_REG16(0x3086), 0x0755 }, + { CCI_REG16(0x3086), 0x562b }, + { CCI_REG16(0x3086), 0xb82b }, + { CCI_REG16(0x3086), 0x984e }, + { CCI_REG16(0x3086), 0x1129 }, + { CCI_REG16(0x3086), 0x9460 }, + { CCI_REG16(0x3086), 0x5c09 }, + { CCI_REG16(0x3086), 0x5c1b }, + { CCI_REG16(0x3086), 0x4002 }, + { CCI_REG16(0x3086), 0x4500 }, + { CCI_REG16(0x3086), 0x4580 }, + { CCI_REG16(0x3086), 0x29b6 }, + { CCI_REG16(0x3086), 0x7f80 }, + { CCI_REG16(0x3086), 0x4004 }, + { CCI_REG16(0x3086), 0x7f88 }, + { CCI_REG16(0x3086), 0x4109 }, + { CCI_REG16(0x3086), 0x5c0b }, + { CCI_REG16(0x3086), 0x29b2 }, + { CCI_REG16(0x3086), 0x4115 }, + { CCI_REG16(0x3086), 0x5c03 }, + { CCI_REG16(0x3086), 0x4105 }, + { CCI_REG16(0x3086), 0x5f2b }, + { CCI_REG16(0x3086), 0x902b }, + { CCI_REG16(0x3086), 0x8081 }, + { CCI_REG16(0x3086), 0x6f40 }, + { CCI_REG16(0x3086), 0x1041 }, + { CCI_REG16(0x3086), 0x0160 }, + { CCI_REG16(0x3086), 0x29a2 }, + { CCI_REG16(0x3086), 0x29a3 }, + { CCI_REG16(0x3086), 0x5f4d }, + { CCI_REG16(0x3086), 0x1c17 }, + { CCI_REG16(0x3086), 0x0281 }, + { CCI_REG16(0x3086), 0xe729 }, + { CCI_REG16(0x3086), 0x8345 }, + { CCI_REG16(0x3086), 0x8840 }, + { CCI_REG16(0x3086), 0x0f7f }, + { CCI_REG16(0x3086), 0x8a40 }, + { CCI_REG16(0x3086), 0x2345 }, + { CCI_REG16(0x3086), 0x8024 }, + { CCI_REG16(0x3086), 0x4008 }, + { CCI_REG16(0x3086), 0x7f88 }, + { CCI_REG16(0x3086), 0x5d29 }, + { CCI_REG16(0x3086), 0x9288 }, + { CCI_REG16(0x3086), 0x102b }, + { CCI_REG16(0x3086), 0x0489 }, + { CCI_REG16(0x3086), 0x165c }, + { CCI_REG16(0x3086), 0x4386 }, + { CCI_REG16(0x3086), 0x170b }, + { CCI_REG16(0x3086), 0x5c03 }, + { CCI_REG16(0x3086), 0x8a48 }, + { CCI_REG16(0x3086), 0x4d4e }, + { CCI_REG16(0x3086), 0x2b80 }, + { CCI_REG16(0x3086), 0x4c09 }, + { CCI_REG16(0x3086), 0x4119 }, + { CCI_REG16(0x3086), 0x816f }, + { CCI_REG16(0x3086), 0x4110 }, + { CCI_REG16(0x3086), 0x4001 }, + { CCI_REG16(0x3086), 0x6029 }, + { CCI_REG16(0x3086), 0x8229 }, + { CCI_REG16(0x3086), 0x8329 }, + { CCI_REG16(0x3086), 0x435c }, + { CCI_REG16(0x3086), 0x055f }, + { CCI_REG16(0x3086), 0x4d1c }, + { CCI_REG16(0x3086), 0x81e7 }, + { CCI_REG16(0x3086), 0x4502 }, + { CCI_REG16(0x3086), 0x8180 }, + { CCI_REG16(0x3086), 0x7f80 }, + { CCI_REG16(0x3086), 0x410a }, + { CCI_REG16(0x3086), 0x9144 }, + { CCI_REG16(0x3086), 0x1609 }, + { CCI_REG16(0x3086), 0x2fc3 }, + { CCI_REG16(0x3086), 0xb130 }, + { CCI_REG16(0x3086), 0xc3b1 }, + { CCI_REG16(0x3086), 0x0343 }, + { CCI_REG16(0x3086), 0x164a }, + { CCI_REG16(0x3086), 0x0a43 }, + { CCI_REG16(0x3086), 0x160b }, + { CCI_REG16(0x3086), 0x4316 }, + { CCI_REG16(0x3086), 0x8f43 }, + { CCI_REG16(0x3086), 0x1690 }, + { CCI_REG16(0x3086), 0x4316 }, + { CCI_REG16(0x3086), 0x7f81 }, + { CCI_REG16(0x3086), 0x450a }, + { CCI_REG16(0x3086), 0x410f }, + { CCI_REG16(0x3086), 0x7f83 }, + { CCI_REG16(0x3086), 0x5d29 }, + { CCI_REG16(0x3086), 0x4488 }, + { CCI_REG16(0x3086), 0x102b }, + { CCI_REG16(0x3086), 0x0453 }, + { CCI_REG16(0x3086), 0x0d40 }, + { CCI_REG16(0x3086), 0x2345 }, + { CCI_REG16(0x3086), 0x0240 }, + { CCI_REG16(0x3086), 0x087f }, + { CCI_REG16(0x3086), 0x8053 }, + { CCI_REG16(0x3086), 0x0d89 }, + { CCI_REG16(0x3086), 0x165c }, + { CCI_REG16(0x3086), 0x4586 }, + { CCI_REG16(0x3086), 0x170b }, + { CCI_REG16(0x3086), 0x5c05 }, + { CCI_REG16(0x3086), 0x8a60 }, + { CCI_REG16(0x3086), 0x4b91 }, + { CCI_REG16(0x3086), 0x4416 }, + { CCI_REG16(0x3086), 0x09c1 }, + { CCI_REG16(0x3086), 0x2ca9 }, + { CCI_REG16(0x3086), 0xab30 }, + { CCI_REG16(0x3086), 0x51b3 }, + { CCI_REG16(0x3086), 0x3d5a }, + { CCI_REG16(0x3086), 0x7e3d }, + { CCI_REG16(0x3086), 0x7e19 }, + { CCI_REG16(0x3086), 0x8000 }, + { CCI_REG16(0x3086), 0x8b1f }, + { CCI_REG16(0x3086), 0x2a1f }, + { CCI_REG16(0x3086), 0x83a2 }, + { CCI_REG16(0x3086), 0x7516 }, + { CCI_REG16(0x3086), 0xad33 }, + { CCI_REG16(0x3086), 0x450a }, + { CCI_REG16(0x3086), 0x7f53 }, + { CCI_REG16(0x3086), 0x8023 }, + { CCI_REG16(0x3086), 0x8c66 }, + { CCI_REG16(0x3086), 0x7f13 }, + { CCI_REG16(0x3086), 0x8184 }, + { CCI_REG16(0x3086), 0x1481 }, + { CCI_REG16(0x3086), 0x8031 }, + { CCI_REG16(0x3086), 0x3d64 }, + { CCI_REG16(0x3086), 0x452a }, + { CCI_REG16(0x3086), 0x9451 }, + { CCI_REG16(0x3086), 0x9e96 }, + { CCI_REG16(0x3086), 0x3d2b }, + { CCI_REG16(0x3086), 0x3d1b }, + { CCI_REG16(0x3086), 0x529f }, + { CCI_REG16(0x3086), 0x0e3d }, + { CCI_REG16(0x3086), 0x083d }, + { CCI_REG16(0x3086), 0x167e }, + { CCI_REG16(0x3086), 0x307e }, + { CCI_REG16(0x3086), 0x1175 }, + { CCI_REG16(0x3086), 0x163e }, + { CCI_REG16(0x3086), 0x970e }, + { CCI_REG16(0x3086), 0x82b2 }, + { CCI_REG16(0x3086), 0x3d7f }, + { CCI_REG16(0x3086), 0xac3e }, + { CCI_REG16(0x3086), 0x4502 }, + { CCI_REG16(0x3086), 0x7e11 }, + { CCI_REG16(0x3086), 0x7fd0 }, + { CCI_REG16(0x3086), 0x8000 }, + { CCI_REG16(0x3086), 0x8c66 }, + { CCI_REG16(0x3086), 0x7f90 }, + { CCI_REG16(0x3086), 0x8194 }, + { CCI_REG16(0x3086), 0x3f44 }, + { CCI_REG16(0x3086), 0x1681 }, + { CCI_REG16(0x3086), 0x8416 }, + { CCI_REG16(0x3086), 0x2c2c }, + { CCI_REG16(0x3086), 0x2c2c }, + { CCI_REG16(0x302a), 0x0005 }, + { CCI_REG16(0x302c), 0x0001 }, + { CCI_REG16(0x302e), 0x0003 }, + { CCI_REG16(0x3030), 0x0032 }, + { CCI_REG16(0x3036), 0x000a }, + { CCI_REG16(0x3038), 0x0001 }, + { CCI_REG16(0x30b0), 0x0028 }, + { CCI_REG16(0x31b0), 0x0082 }, + { CCI_REG16(0x31b2), 0x005c }, + { CCI_REG16(0x31b4), 0x5248 }, + { CCI_REG16(0x31b6), 0x3257 }, + { CCI_REG16(0x31b8), 0x904b }, + { CCI_REG16(0x31ba), 0x030b }, + { CCI_REG16(0x31bc), 0x8e09 }, + { CCI_REG16(0x3354), 0x002b }, + { CCI_REG16(0x31d0), 0x0000 }, + { CCI_REG16(0x31ae), 0x0204 }, + { CCI_REG16(0x3002), 0x00d0 }, + { CCI_REG16(0x3004), 0x0148 }, + { CCI_REG16(0x3006), 0x048f }, + { CCI_REG16(0x3008), 0x0647 }, + { CCI_REG16(0x3064), 0x1802 }, + { CCI_REG16(0x300a), 0x04c4 }, + { CCI_REG16(0x300c), 0x04c4 }, + { CCI_REG16(0x30a2), 0x0001 }, + { CCI_REG16(0x30a6), 0x0001 }, + { CCI_REG16(0x3012), 0x010c }, + { CCI_REG16(0x3786), 0x0006 }, + { CCI_REG16(0x31ae), 0x0202 }, + { CCI_REG16(0x3088), 0x8050 }, + { CCI_REG16(0x3086), 0x9237 }, + { CCI_REG16(0x3044), 0x0410 }, + { CCI_REG16(0x3094), 0x03d4 }, + { CCI_REG16(0x3096), 0x0280 }, + { CCI_REG16(0x30ba), 0x7606 }, + { CCI_REG16(0x30b0), 0x0028 }, + { CCI_REG16(0x30ba), 0x7600 }, + { CCI_REG16(0x30fe), 0x002a }, + { CCI_REG16(0x31de), 0x0410 }, + { CCI_REG16(0x3ed6), 0x1435 }, + { CCI_REG16(0x3ed8), 0x9865 }, + { CCI_REG16(0x3eda), 0x7698 }, + { CCI_REG16(0x3edc), 0x99ff }, + { CCI_REG16(0x3ee2), 0xbb88 }, + { CCI_REG16(0x3ee4), 0x8836 }, + { CCI_REG16(0x3ef0), 0x1cf0 }, + { CCI_REG16(0x3ef2), 0x0000 }, + { CCI_REG16(0x3ef8), 0x6166 }, + { CCI_REG16(0x3efa), 0x3333 }, + { CCI_REG16(0x3efc), 0x6634 }, + { CCI_REG16(0x3088), 0x81ba }, + { CCI_REG16(0x3086), 0x3d02 }, + { CCI_REG16(0x3276), 0x05dc }, + { CCI_REG16(0x3f00), 0x9d05 }, + { CCI_REG16(0x3ed2), 0xfa86 }, + { CCI_REG16(0x3eee), 0xa4fe }, + { CCI_REG16(0x3ecc), 0x6e42 }, + { CCI_REG16(0x3ecc), 0x0e42 }, + { CCI_REG16(0x3eec), 0x0c0c }, + { CCI_REG16(0x3ee8), 0xaae4 }, + { CCI_REG16(0x3ee6), 0x3363 }, + { CCI_REG16(0x3ee6), 0x3363 }, + { CCI_REG16(0x3ee8), 0xaae4 }, + { CCI_REG16(0x3ee8), 0xaae4 }, + { CCI_REG16(0x3180), 0xc24f }, + { CCI_REG16(0x3102), 0x5000 }, + { CCI_REG16(0x3060), 0x000d }, + { CCI_REG16(0x3ed0), 0xff44 }, + { CCI_REG16(0x3ed2), 0xaa86 }, + { CCI_REG16(0x3ed4), 0x031f }, + { CCI_REG16(0x3eee), 0xa4aa }, +}; + +static const s64 link_freq_menu_items[] = { + 360000000ULL, +}; + +static const struct ar0234_link_freq_config link_freq_configs[] = { + { + .reg_list = { + .num_of_regs = + ARRAY_SIZE(link_freq_360M_1280x960_10bit_2lane), + .regs = link_freq_360M_1280x960_10bit_2lane, + } + }, +}; + +static const struct ar0234_mode supported_modes[] = { + { + .width = AR0234_NATIVE_WIDTH, + .height = AR0234_NATIVE_HEIGHT, + .vts_def = 2435, + .vts_min = 2435, + .code = MEDIA_BUS_FMT_SGRBG10_1X10, + .reg_list = { + .num_of_regs = ARRAY_SIZE(mode_1280x960_10bit_2lane), + .regs = mode_1280x960_10bit_2lane, + }, + .link_freq_index = 0, + .hblank = 3600, + .vblank = 1475, + }, +}; + +struct ar0234 { + struct v4l2_subdev sd; + struct media_pad pad; + struct v4l2_ctrl_handler ctrl_handler; + + /* V4L2 Controls */ + struct v4l2_ctrl *link_freq; + struct v4l2_ctrl *exposure; + struct v4l2_ctrl *analogue_gain; + struct v4l2_ctrl *digital_gain; + struct v4l2_ctrl *pixel_rate; + struct v4l2_ctrl *hblank; + struct v4l2_ctrl *vblank; + + struct regmap *regmap; + unsigned long link_freq_bitmap; + const struct ar0234_mode *cur_mode; +}; + +static int ar0234_set_ctrl(struct v4l2_ctrl *ctrl) +{ + struct ar0234 *ar0234 = + container_of(ctrl->handler, struct ar0234, ctrl_handler); + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + s64 exposure_max, exposure_def; + struct v4l2_subdev_state *state; + const struct v4l2_mbus_framefmt *format; + int ret; + + state = v4l2_subdev_get_locked_active_state(&ar0234->sd); + format = v4l2_subdev_state_get_format(state, 0); + + /* Propagate change of current control to all related controls */ + if (ctrl->id == V4L2_CID_VBLANK) { + /* Update max exposure while meeting expected vblanking */ + exposure_max = format->height + ctrl->val - + AR0234_EXPOSURE_MAX_MARGIN; + exposure_def = format->height - AR0234_EXPOSURE_MAX_MARGIN; + __v4l2_ctrl_modify_range(ar0234->exposure, + ar0234->exposure->minimum, + exposure_max, ar0234->exposure->step, + exposure_def); + } + + /* V4L2 controls values will be applied only when power is already up */ + if (!pm_runtime_get_if_in_use(&client->dev)) + return 0; + + switch (ctrl->id) { + case V4L2_CID_ANALOGUE_GAIN: + ret = cci_write(ar0234->regmap, AR0234_REG_ANALOG_GAIN, + ctrl->val, NULL); + break; + + case V4L2_CID_DIGITAL_GAIN: + ret = cci_write(ar0234->regmap, AR0234_REG_GLOBAL_GAIN, + ctrl->val, NULL); + break; + + case V4L2_CID_EXPOSURE: + ret = cci_write(ar0234->regmap, AR0234_REG_EXPOSURE, + ctrl->val, NULL); + break; + + case V4L2_CID_VBLANK: + ret = cci_write(ar0234->regmap, AR0234_REG_VTS, + ar0234->cur_mode->height + ctrl->val, NULL); + break; + + default: + ret = -EINVAL; + break; + } + + pm_runtime_put(&client->dev); + + return ret; +} + +static const struct v4l2_ctrl_ops ar0234_ctrl_ops = { + .s_ctrl = ar0234_set_ctrl, +}; + +static int ar0234_init_controls(struct ar0234 *ar0234) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + struct v4l2_fwnode_device_properties props; + struct v4l2_ctrl_handler *ctrl_hdlr; + s64 exposure_max, vblank, hblank; + u32 link_freq_size; + int ret; + + ctrl_hdlr = &ar0234->ctrl_handler; + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8); + if (ret) + return ret; + + link_freq_size = ARRAY_SIZE(link_freq_menu_items) - 1; + ar0234->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, + &ar0234_ctrl_ops, + V4L2_CID_LINK_FREQ, + link_freq_size, 0, + link_freq_menu_items); + if (ar0234->link_freq) + ar0234->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, + AR0234_ANAL_GAIN_MIN, AR0234_ANAL_GAIN_MAX, + AR0234_ANAL_GAIN_STEP, AR0234_ANAL_GAIN_DEFAULT); + v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, V4L2_CID_DIGITAL_GAIN, + AR0234_DGTL_GAIN_MIN, AR0234_DGTL_GAIN_MAX, + AR0234_DGTL_GAIN_STEP, AR0234_DGTL_GAIN_DEFAULT); + + exposure_max = ar0234->cur_mode->vts_def - AR0234_EXPOSURE_MAX_MARGIN; + ar0234->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, + V4L2_CID_EXPOSURE, + AR0234_EXPOSURE_MIN, exposure_max, + AR0234_EXPOSURE_STEP, + exposure_max); + + ar0234->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, + V4L2_CID_PIXEL_RATE, + PIXEL_RATE, + PIXEL_RATE, 1, + PIXEL_RATE); + if (ar0234->pixel_rate) + ar0234->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + vblank = AR0234_VTS_MAX - ar0234->cur_mode->height; + ar0234->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, + V4L2_CID_VBLANK, 0, vblank, 1, + ar0234->cur_mode->vblank); + hblank = ar0234->cur_mode->hblank; + ar0234->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ar0234_ctrl_ops, + V4L2_CID_HBLANK, hblank, hblank, 1, + hblank); + if (ar0234->hblank) + ar0234->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + if (ctrl_hdlr->error) + return ctrl_hdlr->error; + + ret = v4l2_fwnode_device_parse(&client->dev, &props); + if (ret) + return ret; + + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ar0234_ctrl_ops, + &props); + if (ret) + return ret; + + ar0234->sd.ctrl_handler = ctrl_hdlr; + + return 0; +} + +static void ar0234_update_pad_format(const struct ar0234_mode *mode, + struct v4l2_mbus_framefmt *fmt) +{ + fmt->width = mode->width; + fmt->height = mode->height; + fmt->code = mode->code; + fmt->field = V4L2_FIELD_NONE; +} + +static int ar0234_start_streaming(struct ar0234 *ar0234) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + const struct ar0234_reg_list *reg_list; + int link_freq_index, ret; + + ret = pm_runtime_resume_and_get(&client->dev); + if (ret < 0) + return ret; + + /* + * Setting 0X301A.bit[0] will initiate a reset sequence: + * the frame being generated will be truncated. + */ + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, + AR0234_MODE_RESET, NULL); + if (ret) { + dev_err(&client->dev, "failed to reset"); + goto err_rpm_put; + } + + usleep_range(1000, 1500); + + reg_list = &ar0234->cur_mode->reg_list; + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs, + reg_list->num_of_regs, NULL); + if (ret) { + dev_err(&client->dev, "failed to set mode"); + goto err_rpm_put; + } + + link_freq_index = ar0234->cur_mode->link_freq_index; + if (link_freq_index > 0) { + reg_list = &link_freq_configs[link_freq_index].reg_list; + ret = cci_multi_reg_write(ar0234->regmap, reg_list->regs, + reg_list->num_of_regs, NULL); + if (ret) { + dev_err(&client->dev, "failed to set plls"); + goto err_rpm_put; + } + } + + ret = __v4l2_ctrl_handler_setup(ar0234->sd.ctrl_handler); + if (ret) + goto err_rpm_put; + + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, + AR0234_MODE_STREAMING, NULL); + if (ret) { + dev_err(&client->dev, "failed to start stream"); + goto err_rpm_put; + } + + return 0; + +err_rpm_put: + pm_runtime_put(&client->dev); + return ret; +} + +static int ar0234_stop_streaming(struct ar0234 *ar0234) +{ + int ret; + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + + ret = cci_write(ar0234->regmap, AR0234_REG_MODE_SELECT, + AR0234_MODE_STANDBY, NULL); + if (ret < 0) + dev_err(&client->dev, "failed to stop stream"); + + pm_runtime_put(&client->dev); + return ret; +} + +static int ar0234_set_stream(struct v4l2_subdev *sd, int enable) +{ + struct ar0234 *ar0234 = to_ar0234(sd); + struct v4l2_subdev_state *state; + int ret = 0; + + state = v4l2_subdev_lock_and_get_active_state(sd); + + if (enable) + ret = ar0234_start_streaming(ar0234); + else + ret = ar0234_stop_streaming(ar0234); + + v4l2_subdev_unlock_state(state); + + return ret; +} + +static int ar0234_set_format(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_format *fmt) +{ + struct ar0234 *ar0234 = to_ar0234(sd); + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + struct v4l2_rect *crop; + const struct ar0234_mode *mode; + s64 hblank; + int ret; + + mode = v4l2_find_nearest_size(supported_modes, + ARRAY_SIZE(supported_modes), + width, height, + fmt->format.width, + fmt->format.height); + + crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad); + crop->width = mode->width; + crop->height = mode->height; + + ar0234_update_pad_format(mode, &fmt->format); + *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format; + + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) + return 0; + + ar0234->cur_mode = mode; + ret = __v4l2_ctrl_s_ctrl(ar0234->link_freq, mode->link_freq_index); + if (ret) { + dev_err(&client->dev, "Link Freq ctrl set failed\n"); + return ret; + } + + hblank = ar0234->cur_mode->hblank; + ret = __v4l2_ctrl_modify_range(ar0234->hblank, hblank, hblank, + 1, hblank); + if (ret) { + dev_err(&client->dev, "HB ctrl range update failed\n"); + return ret; + } + + /* Update limits and set FPS to default */ + ret = __v4l2_ctrl_modify_range(ar0234->vblank, 0, + AR0234_VTS_MAX - mode->height, 1, + ar0234->cur_mode->vblank); + if (ret) { + dev_err(&client->dev, "VB ctrl range update failed\n"); + return ret; + } + + ret = __v4l2_ctrl_s_ctrl(ar0234->vblank, ar0234->cur_mode->vblank); + if (ret) { + dev_err(&client->dev, "VB ctrl set failed\n"); + return ret; + } + + return 0; +} + +static int ar0234_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_mbus_code_enum *code) +{ + if (code->index > 0) + return -EINVAL; + + code->code = MEDIA_BUS_FMT_SGRBG10_1X10; + + return 0; +} + +static int ar0234_enum_frame_size(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_frame_size_enum *fse) +{ + if (fse->index >= ARRAY_SIZE(supported_modes)) + return -EINVAL; + + if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10) + return -EINVAL; + + fse->min_width = supported_modes[fse->index].width; + fse->max_width = fse->min_width; + fse->min_height = supported_modes[fse->index].height; + fse->max_height = fse->min_height; + + return 0; +} + +static int ar0234_get_selection(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, + struct v4l2_subdev_selection *sel) +{ + switch (sel->target) { + case V4L2_SEL_TGT_CROP_DEFAULT: + case V4L2_SEL_TGT_CROP: + sel->r = *v4l2_subdev_state_get_crop(state, 0); + break; + case V4L2_SEL_TGT_CROP_BOUNDS: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = AR0234_NATIVE_WIDTH; + sel->r.height = AR0234_NATIVE_HEIGHT; + break; + default: + return -EINVAL; + } + + return 0; +} + +static int ar0234_init_state(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state) +{ + struct v4l2_subdev_format fmt = { + .which = V4L2_SUBDEV_FORMAT_TRY, + .pad = 0, + .format = { + .code = MEDIA_BUS_FMT_SGRBG10_1X10, + .width = AR0234_NATIVE_WIDTH, + .height = AR0234_NATIVE_HEIGHT, + }, + }; + + ar0234_set_format(sd, sd_state, &fmt); + + return 0; +} + +static const struct v4l2_subdev_video_ops ar0234_video_ops = { + .s_stream = ar0234_set_stream, +}; + +static const struct v4l2_subdev_pad_ops ar0234_pad_ops = { + .set_fmt = ar0234_set_format, + .get_fmt = v4l2_subdev_get_fmt, + .enum_mbus_code = ar0234_enum_mbus_code, + .enum_frame_size = ar0234_enum_frame_size, + .get_selection = ar0234_get_selection, +}; + +static const struct v4l2_subdev_core_ops ar0234_core_ops = { + .subscribe_event = v4l2_ctrl_subdev_subscribe_event, + .unsubscribe_event = v4l2_event_subdev_unsubscribe, +}; + +static const struct v4l2_subdev_ops ar0234_subdev_ops = { + .core = &ar0234_core_ops, + .video = &ar0234_video_ops, + .pad = &ar0234_pad_ops, +}; + +static const struct media_entity_operations ar0234_subdev_entity_ops = { + .link_validate = v4l2_subdev_link_validate, +}; + +static const struct v4l2_subdev_internal_ops ar0234_internal_ops = { + .init_state = ar0234_init_state, +}; + +static int ar0234_parse_fwnode(struct ar0234 *ar0234, struct device *dev) +{ + struct fwnode_handle *endpoint; + struct v4l2_fwnode_endpoint bus_cfg = { + .bus_type = V4L2_MBUS_CSI2_DPHY, + }; + int ret; + + endpoint = + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, + FWNODE_GRAPH_ENDPOINT_NEXT); + if (!endpoint) { + dev_err(dev, "endpoint node not found\n"); + return -EPROBE_DEFER; + } + + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg); + if (ret) { + dev_err(dev, "parsing endpoint node failed\n"); + goto out_err; + } + + ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies, + bus_cfg.nr_of_link_frequencies, + link_freq_menu_items, + ARRAY_SIZE(link_freq_menu_items), + &ar0234->link_freq_bitmap); + if (ret) + goto out_err; + +out_err: + v4l2_fwnode_endpoint_free(&bus_cfg); + fwnode_handle_put(endpoint); + return ret; +} + +static int ar0234_identify_module(struct ar0234 *ar0234) +{ + struct i2c_client *client = v4l2_get_subdevdata(&ar0234->sd); + int ret; + u64 val; + + ret = cci_read(ar0234->regmap, AR0234_REG_CHIP_ID, &val, NULL); + if (ret) + return ret; + + if (val != AR0234_CHIP_ID) { + dev_err(&client->dev, "chip id mismatch: %x!=%llx", + AR0234_CHIP_ID, val); + return -ENXIO; + } + + return 0; +} + +static void ar0234_remove(struct i2c_client *client) +{ + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct ar0234 *ar0234 = to_ar0234(sd); + + v4l2_async_unregister_subdev(&ar0234->sd); + v4l2_subdev_cleanup(sd); + media_entity_cleanup(&ar0234->sd.entity); + v4l2_ctrl_handler_free(&ar0234->ctrl_handler); + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); +} + +static int ar0234_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct ar0234 *ar0234; + int ret; + + ar0234 = devm_kzalloc(&client->dev, sizeof(*ar0234), GFP_KERNEL); + if (!ar0234) + return -ENOMEM; + + ret = ar0234_parse_fwnode(ar0234, dev); + if (ret) + return ret; + + ar0234->regmap = devm_cci_regmap_init_i2c(client, 16); + if (IS_ERR(ar0234->regmap)) + return dev_err_probe(dev, PTR_ERR(ar0234->regmap), + "failed to init CCI\n"); + + v4l2_i2c_subdev_init(&ar0234->sd, client, &ar0234_subdev_ops); + + ret = ar0234_identify_module(ar0234); + if (ret) { + dev_err(&client->dev, "failed to find sensor: %d", ret); + return ret; + } + + /* Set default mode to max resolution */ + ar0234->cur_mode = &supported_modes[0]; + ret = ar0234_init_controls(ar0234); + if (ret) { + dev_err(&client->dev, "failed to init controls: %d", ret); + goto probe_error_v4l2_ctrl_handler_free; + } + + ar0234->sd.internal_ops = &ar0234_internal_ops; + ar0234->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | + V4L2_SUBDEV_FL_HAS_EVENTS; + ar0234->sd.entity.ops = &ar0234_subdev_entity_ops; + ar0234->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; + + ar0234->pad.flags = MEDIA_PAD_FL_SOURCE; + ret = media_entity_pads_init(&ar0234->sd.entity, 1, &ar0234->pad); + if (ret) { + dev_err(&client->dev, "failed to init entity pads: %d", ret); + goto probe_error_v4l2_ctrl_handler_free; + } + + ar0234->sd.state_lock = ar0234->ctrl_handler.lock; + ret = v4l2_subdev_init_finalize(&ar0234->sd); + if (ret < 0) { + dev_err(dev, "v4l2 subdev init error: %d\n", ret); + goto probe_error_media_entity_cleanup; + } + + /* + * Device is already turned on by i2c-core with ACPI domain PM. + * Enable runtime PM and turn off the device. + */ + pm_runtime_set_active(&client->dev); + pm_runtime_enable(&client->dev); + pm_runtime_idle(&client->dev); + + ret = v4l2_async_register_subdev_sensor(&ar0234->sd); + if (ret < 0) { + dev_err(&client->dev, "failed to register V4L2 subdev: %d", + ret); + goto probe_error_rpm; + } + + return 0; +probe_error_rpm: + pm_runtime_disable(&client->dev); + v4l2_subdev_cleanup(&ar0234->sd); + +probe_error_media_entity_cleanup: + media_entity_cleanup(&ar0234->sd.entity); + +probe_error_v4l2_ctrl_handler_free: + v4l2_ctrl_handler_free(ar0234->sd.ctrl_handler); + + return ret; +} + +static const struct acpi_device_id ar0234_acpi_ids[] = { + { "INTC10C0" }, + {} +}; + +MODULE_DEVICE_TABLE(acpi, ar0234_acpi_ids); + +static struct i2c_driver ar0234_i2c_driver = { + .driver = { + .name = "ar0234", + .acpi_match_table = ACPI_PTR(ar0234_acpi_ids), + }, + .probe = ar0234_probe, + .remove = ar0234_remove, +}; + +module_i2c_driver(ar0234_i2c_driver); + +MODULE_DESCRIPTION("ON Semiconductor ar0234 sensor driver"); +MODULE_AUTHOR("Dongcheng Yan <dongcheng.yan@intel.com>"); +MODULE_AUTHOR("Hao Yao <hao.yao@intel.com>"); +MODULE_LICENSE("GPL");
The driver is implemented with V4L2 framework, and supports following features: - manual exposure and analog/digital gain control - vblank/hblank control - runtime PM support - 1280x960 at 30FPS Signed-off-by: Dongcheng Yan <dongcheng.yan@intel.com> --- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ar0234.c | 1053 ++++++++++++++++++++++++++++++++++++ 3 files changed, 1065 insertions(+) create mode 100644 drivers/media/i2c/ar0234.c