Re: [PATCH] media: i2c: imx219: Rewrite tables and implement more modes

From: Dave Stevenson
Date: Sun Jan 17 2021 - 15:16:03 EST

Hi Angelo

On Sun, 17 Jan 2021 at 17:33, AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx> wrote:
>
> Il 17/01/21 00:13, Sakari Ailus ha scritto:
> > Hi AngeloGioacchino,
> >
> > On Fri, Jan 15, 2021 at 07:52:33PM +0100, AngeloGioacchino Del Regno wrote:
> >> Enhance the feature set for this camera sensor by in rewriting the
> >> entire tables (as they were just meaningless magic register writes)
> >> in a similar form, but giving some names to the actual registers
> >> we write to, separating common sequences and reusing them for the
> >> various configuration variations that are now supported, hence
> >> implementing support for:
> >> - 8MHz XCLK, as used by (and not only) some Sony Xperia smartphones
> >> - 4-Lane Mode in both 24MHz and 8MHz XCLK configuration
> >> - High Frame Rate output modes support on 4-Lane operation, up to
> >> 1000FPS (also on 2-Lane but, being bandwidth-constrained, the
> >> maximum achievable frame rate gets lower there)
> >
> > That's a lot of changes for a single patch. Could you split each of these
> > into separate patches, please?
> >
>
> Sure! I agree with you, let's split them for the V2 patch series!
>
> >> - Frame Bank Control Groups, in order to support a fast output
> >> resolution switch, without resetting the entire sensor during
> >> a streaming session: here the new mode gets configured on the
> >> secondary (or primary, read: "the other") bank and the sensor
> >> will be able to switch to it at the end of the "current frame".
> >
> > You basically need to stop streaming to reconfigure the sensor; V4L2
> > currently does not doing this on the fly.
> >
> > There's no need to rest the sensor though, and I don't think the driver did
> > that before either.
> >
>
> If V4L2 needs to stop streaming, then the sensor is "put to rest".
> By the way... okay, I can remove the implementation to fast-switch
> between the frame banks, but the registers are still laid out in frame
> banks in hardware so, in my opinion, the "new" layout should be kept.
>
> I wrote something more about this in reply to D. Stevenson so, please,
> for more information, look at my reply to him.
> Copy-pasting should not be necessary. Thank you!
>
> >>
> >> Please note: an unknown register write sequence was found in both
> >> the Raspberry Pi and a Sony Xperia smartphone i2c dump, but this
> >> seems to do literally nothing, as the sensor seems to work
> >> in the exact same way when sending and when not sending this
> >> write sequence, which is undocumented in the datasheet.
> >>
> >> Both the authentication and magic sequences were left in the
> >> driver with a big comment explaining what's going on so that,
> >> in the event that someone discovers the meaning of it (or
> >> Sony distributes documentation for that), it'll be pretty
> >> straightforward to insert them when needed.
> >>
> >> All the modes that got implemented in this commit have been tested
> >> with all combinations of 24/8MHz, 2/4Lane, all resolutions, on the
> >> following smartphones:
> >> - Sony Xperia XA2 (SDM630)
> >> - Sony Xperia XA2 Ultra (SDM630)
> >>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
> >> ---
> >> drivers/media/i2c/imx219.c | 884 ++++++++++++++++++++++++-------------
> >> 1 file changed, 580 insertions(+), 304 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> >> index 92a8d52776b8..360730d5b81c 100644
> >> --- a/drivers/media/i2c/imx219.c
> >> +++ b/drivers/media/i2c/imx219.c
> >> @@ -12,6 +12,10 @@
> >> * Flip handling taken from the Sony IMX319 driver.
> >> * Copyright (C) 2018 Intel Corporation
> >> *
> >> + * 8MHz, 4-Lane, High Frame Rate modes, Frame Bank Control groups,
> >> + * fast mode switching
> >> + * Copyright (C) 2020, AngeloGioacchino Del Regno
> >> + * <angelogioacchino.delregno@xxxxxxxxxxxxxx>
> >> */
> >>
> >> #include <linux/clk.h>
> >> @@ -35,24 +39,93 @@
> >> #define IMX219_MODE_STANDBY 0x00
> >> #define IMX219_MODE_STREAMING 0x01
> >>
> >> +#define IMX219_REG_SW_RESET 0x0103
> >> +
> >> +/* Output Set-up */
> >> +#define IMX219_REG_CSI_LANE_MODE 0x0114
> >> +#define IMX219_CSI_LANE_MODE_2LANE BIT(0)
> >> +#define IMX219_CSI_LANE_MODE_4LANE (BIT(0) | BIT(1))
> >> +
> >> +#define IMX219_REG_DPHY_CTRL 0x0128
> >> +#define IMX219_DPHY_CTRL_AUTO 0
> >> +#define IMX219_DPHY_CTRL_MANUAL 1
> >> +
> >> +/* Use as 16-bits reg */
> >> +#define IMX219_REG_EXCK_FREQ_MHZ 0x012A
> >> +#define IMX219_EXCK_FREQ_MHZ_MIN 6
> >> +#define IMX219_EXCK_FREQ_MHZ_MAX 27
> >> +
> >> +/* Frame Bank Control Registers*/
> >> +#define IMX219_REG_FRAME_BANK_CTRL 0x0150
> >> +#define IMX219_FRAME_BANK_EN_SHIFT BIT(0)
> >> +#define IMX219_FRAME_BANK_STAT_SHIFT BIT(1)
> >> +
> >> +#define IMX219_REG_FRAME_COUNT 0x0151
> >> +#define IMX219_REG_FRAME_FAST_TRACKING 0x0152
> >> +
> >> +/* Frame Bank 0: Group "A" - 1: Group "B" */
> >> +#define IMX219_REG_FRAME_BANK_BASE(x) ((0x100 * x) + 0x154)
> >> +#define IMX219_REG_ANALOG_GAIN 0x03
> >> +#define IMX219_REG_DIGITAL_GAIN 0x04
> >> +#define IMX219_REG_EXPOSURE 0x06
> >> +#define IMX219_REG_FRAME_LEN_LINES 0x0c
> >> +#define IMX219_REG_LINE_LEN_PCK 0x0e
> >> +#define IMX219_REG_X_ADDR_START 0x10
> >> +#define IMX219_REG_X_ADDR_END 0x12
> >> +#define IMX219_REG_Y_ADDR_START 0x14
> >> +#define IMX219_REG_Y_ADDR_END 0x16
> >> +#define IMX219_REG_X_OUTPUT_SIZE 0x18
> >> +#define IMX219_REG_Y_OUTPUT_SIZE 0x1a
> >> +#define IMX219_REG_X_ODD_INC 0x1c
> >> +#define IMX219_REG_Y_ODD_INC 0x1d
> >> +#define IMX219_REG_ORIENTATION 0x1e
> >> +#define IMX219_REG_BINNING_MODE_H 0x20
> >> +#define IMX219_REG_BINNING_MODE_V 0x21
> >> +#define IMX219_REG_BINNING_CAL_H 0x22
> >> +#define IMX219_REG_BINNING_CAL_V 0x23
> >> +#define IMX219_REG_CSI_DATA_FORMAT_HI 0x38
> >> +#define IMX219_REG_CSI_DATA_FORMAT_LO 0x39
> >> +#define IMX219_CSI_DATA_FMT_8BIT 8
> >> +#define IMX219_CSI_DATA_FMT_10BIT 10
> >> +
> >> +#define IMX219_REG_CLK_SETUP 0x300
> >> +#define IMX219_REG_VT_PIX_CLK_DIV (IMX219_REG_CLK_SETUP + 0x01)
> >> +#define IMX219_REG_VT_SYS_CLK_DIV (IMX219_REG_CLK_SETUP + 0x03)
> >> +#define IMX219_REG_PREPLLCK_VT_DIV (IMX219_REG_CLK_SETUP + 0x04)
> >> +#define IMX219_REG_PREPLLCK_OP_DIV (IMX219_REG_CLK_SETUP + 0x05)
> >> +#define IMX219_REG_PLL_VT_MULTIPLIER (IMX219_REG_CLK_SETUP + 0x06)
> >> +#define IMX219_REG_OP_PIX_CLK_DIV (IMX219_REG_CLK_SETUP + 0x09)
> >> +#define IMX219_REG_OP_SYS_CLK_DIV (IMX219_REG_CLK_SETUP + 0x0b)
> >> +#define IMX219_REG_PLL_OP_MULTIPLIER (IMX219_REG_CLK_SETUP + 0x0c)
> >> +
> >> /* Chip ID */
> >> #define IMX219_REG_CHIP_ID 0x0000
> >> #define IMX219_CHIP_ID 0x0219
> >>
> >> -/* External clock frequency is 24.0M */
> >> -#define IMX219_XCLK_FREQ 24000000
> >> +/* External clock frequency 8.0M or 24.0M */
> >> +#define IMX219_XCLK_FREQ_8M 8000000
> >> +#define IMX219_XCLK_FREQ_24M 24000000
> >> +
> >> +#define IMX219_2LANE_PIXEL_RATE 182400000
> >> +#define IMX219_4LANE_PIXEL_RATE 280800000
> >>
> >> -/* Pixel rate is fixed at 182.4M for all the modes */
> >> -#define IMX219_PIXEL_RATE 182400000
> >> +#define IMX219_2LANE_DEFAULT_LINK_FREQ 456000000
> >> +#define IMX219_4LANE_DEFAULT_LINK_FREQ 702000000
> >>
> >> -#define IMX219_DEFAULT_LINK_FREQ 456000000
> >> +/* Temperature */
> >> +#define IMX219_REG_TEMPERATURE 0x0140
> >> +#define IMX219_TEMPERATURE_EN BIT(7)
> >> +#define IMX219_TEMPERATURE_MASK 0x7f
> >>
> >> /* V_TIMING internal */
> >> #define IMX219_REG_VTS 0x0160
> >> -#define IMX219_VTS_15FPS 0x0dc6
> >> -#define IMX219_VTS_30FPS_1080P 0x06e3
> >> -#define IMX219_VTS_30FPS_BINNED 0x06e3
> >> -#define IMX219_VTS_30FPS_640x480 0x06e3
> >> +#define IMX219_VTS_8MP_30FPS_4LANE 2691
> >> +#define IMX219_VTS_8MP_15FPS_2LANE 3526
> >> +#define IMX219_VTS_60FPS_1080P 1237
> >> +#define IMX219_VTS_30FPS_BINNED 1346
> >> +#define IMX219_VTS_120FPS_720P 850
> >> +#define IMX219_VTS_200FPS_BINNED 481
> >> +#define IMX219_VTS_1000FPS_BINNED 481
> >> #define IMX219_VTS_MAX 0xffff
> >>
> >> #define IMX219_VBLANK_MIN 4
> >> @@ -67,30 +140,27 @@
> >> #define IMX219_PPL_DEFAULT 3448
> >>
> >> /* Exposure control */
> >> -#define IMX219_REG_EXPOSURE 0x015a
> >> #define IMX219_EXPOSURE_MIN 4
> >> #define IMX219_EXPOSURE_STEP 1
> >> #define IMX219_EXPOSURE_DEFAULT 0x640
> >> #define IMX219_EXPOSURE_MAX 65535
> >>
> >> /* Analog gain control */
> >> -#define IMX219_REG_ANALOG_GAIN 0x0157
> >> #define IMX219_ANA_GAIN_MIN 0
> >> #define IMX219_ANA_GAIN_MAX 232
> >> #define IMX219_ANA_GAIN_STEP 1
> >> #define IMX219_ANA_GAIN_DEFAULT 0x0
> >>
> >> /* Digital gain control */
> >> -#define IMX219_REG_DIGITAL_GAIN 0x0158
> >> #define IMX219_DGTL_GAIN_MIN 0x0100
> >> #define IMX219_DGTL_GAIN_MAX 0x0fff
> >> #define IMX219_DGTL_GAIN_DEFAULT 0x0100
> >> #define IMX219_DGTL_GAIN_STEP 1
> >>
> >> -#define IMX219_REG_ORIENTATION 0x0172
> >> -
> >> /* Test Pattern Control */
> >> #define IMX219_REG_TEST_PATTERN 0x0600
> >> +#define IMX219_REG_TEST_PATTERN_WIDTH 0x0624
> >> +#define IMX219_REG_TEST_PATTERN_HEIGHT 0x0626
> >> #define IMX219_TEST_PATTERN_DISABLE 0
> >> #define IMX219_TEST_PATTERN_SOLID_COLOR 1
> >> #define IMX219_TEST_PATTERN_COLOR_BARS 2
> >> @@ -120,7 +190,9 @@
> >>
> >> struct imx219_reg {
> >> u16 address;
> >> - u8 val;
> >> + u16 val;
> >> + u8 reg_len;
> >> + bool is_banked;
> >> };
> >>
> >> struct imx219_reg_list {
> >> @@ -134,11 +206,13 @@ struct imx219_mode {
> >> unsigned int width;
> >> /* Frame height */
> >> unsigned int height;
> >> + /* Maximum achievable FPS */
> >> + unsigned int max_fps;
> >>
> >> /* Analog crop rectangle. */
> >> struct v4l2_rect crop;
> >>
> >> - /* V-timing */
> >> + /* V-timing default */
> >> unsigned int vts_def;
> >>
> >> /* Default register values */
> >> @@ -146,248 +220,196 @@ struct imx219_mode {
> >> };
> >>
> >> /*
> >> - * Register sets lifted off the i2C interface from the Raspberry Pi firmware
> >> - * driver.
> >> - * 3280x2464 = mode 2, 1920x1080 = mode 1, 1640x1232 = mode 4, 640x480 = mode 7.
> >> + * The authentication sequence is needed to access registers beyond 0x3000,
> >> + * which the datasheet calls "Manufacturer Specific Registers", with a range
> >> + * going from 0x3000 to 0x5fff, but its documentation is full of holes:
> >> + * it is infact documenting registers from 0x3000 to 0x32ff as OTP Data
> >> + * and, as for the others, it documents 0x4053, 0x5e54 and 0x5e59.. and
> >> + * nothing else.
> >> + *
> >> + * On both Raspberry Pi and Xperia XA2 i2c dumps, there is an unknown
> >> + * write sequence that is totally the same between the two, but the sensor
> >> + * seems to work regardless of this being sent.
> >> + * Spirit here is to not send unknown things, especially if they don't seem
> >> + * to do anything... and that's what was done. Also, remember that the auth
> >> + * commands will allow to write to the OTP area, which may actually damage
> >> + * the sensor functionality (for example, factory calibrations may be damaged).
> >> + *
> >> + * The authentication sequence and the unknown one are kept here in case one
> >> + * day they get documented somehow, or any use for them is found.
> >> */
> >> -static const struct imx219_reg mode_3280x2464_regs[] = {
> >> - {0x0100, 0x00},
> >> - {0x30eb, 0x0c},
> >> - {0x30eb, 0x05},
> >> - {0x300a, 0xff},
> >> - {0x300b, 0xff},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x09},
> >> - {0x0114, 0x01},
> >> - {0x0128, 0x00},
> >> - {0x012a, 0x18},
> >> - {0x012b, 0x00},
> >> - {0x0164, 0x00},
> >> - {0x0165, 0x00},
> >> - {0x0166, 0x0c},
> >> - {0x0167, 0xcf},
> >> - {0x0168, 0x00},
> >> - {0x0169, 0x00},
> >> - {0x016a, 0x09},
> >> - {0x016b, 0x9f},
> >> - {0x016c, 0x0c},
> >> - {0x016d, 0xd0},
> >> - {0x016e, 0x09},
> >> - {0x016f, 0xa0},
> >> - {0x0170, 0x01},
> >> - {0x0171, 0x01},
> >> - {0x0174, 0x00},
> >> - {0x0175, 0x00},
> >> - {0x0301, 0x05},
> >> - {0x0303, 0x01},
> >> - {0x0304, 0x03},
> >> - {0x0305, 0x03},
> >> - {0x0306, 0x00},
> >> - {0x0307, 0x39},
> >> - {0x030b, 0x01},
> >> - {0x030c, 0x00},
> >> - {0x030d, 0x72},
> >> - {0x0624, 0x0c},
> >> - {0x0625, 0xd0},
> >> - {0x0626, 0x09},
> >> - {0x0627, 0xa0},
> >> - {0x455e, 0x00},
> >> - {0x471e, 0x4b},
> >> - {0x4767, 0x0f},
> >> - {0x4750, 0x14},
> >> - {0x4540, 0x00},
> >> - {0x47b4, 0x14},
> >> - {0x4713, 0x30},
> >> - {0x478b, 0x10},
> >> - {0x478f, 0x10},
> >> - {0x4793, 0x10},
> >> - {0x4797, 0x0e},
> >> - {0x479b, 0x0e},
> >> - {0x0162, 0x0d},
> >> - {0x0163, 0x78},
> >> +static const struct imx219_reg __maybe_unused imx219_auth[] = {
> >> + { 0x30eb, 0x05, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x30eb, 0x0c, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x300a, 0xff, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x300b, 0xff, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x30eb, 0x05, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x30eb, 0x09, IMX219_REG_VALUE_08BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg __maybe_unused imx219_unknown_seq[] = {
> >
> > Why are these __maybe_unused?
> >
> >> + { 0x455E, 0x00, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x471E, 0x4B, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4767, 0x0F, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4750, 0x14, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4540, 0x00, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x47B4, 0x14, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4713, 0x30, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x478B, 0x10, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x478F, 0x10, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4793, 0x10, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x4797, 0x0e, IMX219_REG_VALUE_08BIT, false },
> >> + { 0x479b, 0x0e, IMX219_REG_VALUE_08BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_24mhz_regs[] = {
> >> + { IMX219_REG_EXCK_FREQ_MHZ, (24 << 8), IMX219_REG_VALUE_16BIT, false },
> >> + { IMX219_REG_DPHY_CTRL, 0, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_VT_PIX_CLK_DIV, 5, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_VT_SYS_CLK_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PREPLLCK_VT_DIV, 3, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PREPLLCK_OP_DIV, 3, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_OP_PIX_CLK_DIV, 10, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_OP_SYS_CLK_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_OP_MULTIPLIER, 84, IMX219_REG_VALUE_16BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_24mhz_2lane[] = {
> >> + { IMX219_REG_CSI_LANE_MODE, IMX219_CSI_LANE_MODE_2LANE,
> >> + IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_VT_MULTIPLIER, 57, IMX219_REG_VALUE_16BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_24mhz_4lane[] = {
> >> + { IMX219_REG_CSI_LANE_MODE, IMX219_CSI_LANE_MODE_4LANE,
> >> + IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_VT_MULTIPLIER, 82, IMX219_REG_VALUE_16BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_8mhz_regs[] = {
> >> + { IMX219_REG_EXCK_FREQ_MHZ, (8 << 8), IMX219_REG_VALUE_16BIT, false },
> >> + { IMX219_REG_DPHY_CTRL, 0, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_VT_PIX_CLK_DIV, 5, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_VT_SYS_CLK_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PREPLLCK_VT_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PREPLLCK_OP_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_OP_PIX_CLK_DIV, 10, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_OP_SYS_CLK_DIV, 1, IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_OP_MULTIPLIER, 90, IMX219_REG_VALUE_16BIT, false },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_8mhz_2lane[] = {
> >> + { IMX219_REG_CSI_LANE_MODE, IMX219_CSI_LANE_MODE_2LANE,
> >> + IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_VT_MULTIPLIER, 63, IMX219_REG_VALUE_16BIT, false },
> >> };
> >>
> >> -static const struct imx219_reg mode_1920_1080_regs[] = {
> >> - {0x0100, 0x00},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x0c},
> >> - {0x300a, 0xff},
> >> - {0x300b, 0xff},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x09},
> >> - {0x0114, 0x01},
> >> - {0x0128, 0x00},
> >> - {0x012a, 0x18},
> >> - {0x012b, 0x00},
> >> - {0x0162, 0x0d},
> >> - {0x0163, 0x78},
> >> - {0x0164, 0x02},
> >> - {0x0165, 0xa8},
> >> - {0x0166, 0x0a},
> >> - {0x0167, 0x27},
> >> - {0x0168, 0x02},
> >> - {0x0169, 0xb4},
> >> - {0x016a, 0x06},
> >> - {0x016b, 0xeb},
> >> - {0x016c, 0x07},
> >> - {0x016d, 0x80},
> >> - {0x016e, 0x04},
> >> - {0x016f, 0x38},
> >> - {0x0170, 0x01},
> >> - {0x0171, 0x01},
> >> - {0x0174, 0x00},
> >> - {0x0175, 0x00},
> >> - {0x0301, 0x05},
> >> - {0x0303, 0x01},
> >> - {0x0304, 0x03},
> >> - {0x0305, 0x03},
> >> - {0x0306, 0x00},
> >> - {0x0307, 0x39},
> >> - {0x030b, 0x01},
> >> - {0x030c, 0x00},
> >> - {0x030d, 0x72},
> >> - {0x0624, 0x07},
> >> - {0x0625, 0x80},
> >> - {0x0626, 0x04},
> >> - {0x0627, 0x38},
> >> - {0x455e, 0x00},
> >> - {0x471e, 0x4b},
> >> - {0x4767, 0x0f},
> >> - {0x4750, 0x14},
> >> - {0x4540, 0x00},
> >> - {0x47b4, 0x14},
> >> - {0x4713, 0x30},
> >> - {0x478b, 0x10},
> >> - {0x478f, 0x10},
> >> - {0x4793, 0x10},
> >> - {0x4797, 0x0e},
> >> - {0x479b, 0x0e},
> >> +static const struct imx219_reg mode_8mhz_4lane[] = {
> >> + { IMX219_REG_CSI_LANE_MODE, IMX219_CSI_LANE_MODE_4LANE,
> >> + IMX219_REG_VALUE_08BIT, false },
> >> + { IMX219_REG_PLL_VT_MULTIPLIER, 88, IMX219_REG_VALUE_16BIT, false },
> >> };
> >>
> >> -static const struct imx219_reg mode_1640_1232_regs[] = {
> >> - {0x0100, 0x00},
> >> - {0x30eb, 0x0c},
> >> - {0x30eb, 0x05},
> >> - {0x300a, 0xff},
> >> - {0x300b, 0xff},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x09},
> >> - {0x0114, 0x01},
> >> - {0x0128, 0x00},
> >> - {0x012a, 0x18},
> >> - {0x012b, 0x00},
> >> - {0x0164, 0x00},
> >> - {0x0165, 0x00},
> >> - {0x0166, 0x0c},
> >> - {0x0167, 0xcf},
> >> - {0x0168, 0x00},
> >> - {0x0169, 0x00},
> >> - {0x016a, 0x09},
> >> - {0x016b, 0x9f},
> >> - {0x016c, 0x06},
> >> - {0x016d, 0x68},
> >> - {0x016e, 0x04},
> >> - {0x016f, 0xd0},
> >> - {0x0170, 0x01},
> >> - {0x0171, 0x01},
> >> - {0x0174, 0x01},
> >> - {0x0175, 0x01},
> >> - {0x0301, 0x05},
> >> - {0x0303, 0x01},
> >> - {0x0304, 0x03},
> >> - {0x0305, 0x03},
> >> - {0x0306, 0x00},
> >> - {0x0307, 0x39},
> >> - {0x030b, 0x01},
> >> - {0x030c, 0x00},
> >> - {0x030d, 0x72},
> >> - {0x0624, 0x06},
> >> - {0x0625, 0x68},
> >> - {0x0626, 0x04},
> >> - {0x0627, 0xd0},
> >> - {0x455e, 0x00},
> >> - {0x471e, 0x4b},
> >> - {0x4767, 0x0f},
> >> - {0x4750, 0x14},
> >> - {0x4540, 0x00},
> >> - {0x47b4, 0x14},
> >> - {0x4713, 0x30},
> >> - {0x478b, 0x10},
> >> - {0x478f, 0x10},
> >> - {0x4793, 0x10},
> >> - {0x4797, 0x0e},
> >> - {0x479b, 0x0e},
> >> - {0x0162, 0x0d},
> >> - {0x0163, 0x78},
> >> +/* Note: Never forget to select BANK OFFSET when using these modes */
> >> +static const struct imx219_reg mode_3280x2464[] = {
> >> + /* MAX: 4-Lane @ 30FPS - 2-Lane @ 15FPS */
> >> + { IMX219_REG_FRAME_LEN_LINES, 2691, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_LINE_LEN_PCK, 3448, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_START, 0, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_END, 3279, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_START, 0, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_END, 2463, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_OUTPUT_SIZE, 3280, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_OUTPUT_SIZE, 2464, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_Y_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> };
> >>
> >> -static const struct imx219_reg mode_640_480_regs[] = {
> >> - {0x0100, 0x00},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x0c},
> >> - {0x300a, 0xff},
> >> - {0x300b, 0xff},
> >> - {0x30eb, 0x05},
> >> - {0x30eb, 0x09},
> >> - {0x0114, 0x01},
> >> - {0x0128, 0x00},
> >> - {0x012a, 0x18},
> >> - {0x012b, 0x00},
> >> - {0x0162, 0x0d},
> >> - {0x0163, 0x78},
> >> - {0x0164, 0x03},
> >> - {0x0165, 0xe8},
> >> - {0x0166, 0x08},
> >> - {0x0167, 0xe7},
> >> - {0x0168, 0x02},
> >> - {0x0169, 0xf0},
> >> - {0x016a, 0x06},
> >> - {0x016b, 0xaf},
> >> - {0x016c, 0x02},
> >> - {0x016d, 0x80},
> >> - {0x016e, 0x01},
> >> - {0x016f, 0xe0},
> >> - {0x0170, 0x01},
> >> - {0x0171, 0x01},
> >> - {0x0174, 0x03},
> >> - {0x0175, 0x03},
> >> - {0x0301, 0x05},
> >> - {0x0303, 0x01},
> >> - {0x0304, 0x03},
> >> - {0x0305, 0x03},
> >> - {0x0306, 0x00},
> >> - {0x0307, 0x39},
> >> - {0x030b, 0x01},
> >> - {0x030c, 0x00},
> >> - {0x030d, 0x72},
> >> - {0x0624, 0x06},
> >> - {0x0625, 0x68},
> >> - {0x0626, 0x04},
> >> - {0x0627, 0xd0},
> >> - {0x455e, 0x00},
> >> - {0x471e, 0x4b},
> >> - {0x4767, 0x0f},
> >> - {0x4750, 0x14},
> >> - {0x4540, 0x00},
> >> - {0x47b4, 0x14},
> >> - {0x4713, 0x30},
> >> - {0x478b, 0x10},
> >> - {0x478f, 0x10},
> >> - {0x4793, 0x10},
> >> - {0x4797, 0x0e},
> >> - {0x479b, 0x0e},
> >> +static const struct imx219_reg mode_1920x1080_cropped_60fps[] = {
> >> + { IMX219_REG_FRAME_LEN_LINES, 1237, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_LINE_LEN_PCK, 3448, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_START, 680, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_END, 2599, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_START, 692, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_END, 1771, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_OUTPUT_SIZE, 1920, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_OUTPUT_SIZE, 1080, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_Y_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_1280x720_cropped_120fps[] = {
> >> + /* Recommended coarse integration time value: 846 */
> >> + { IMX219_REG_FRAME_LEN_LINES, 850, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_LINE_LEN_PCK, 3448, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_START, 1000, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_END, 2280, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_START, 872, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_END, 1592, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_OUTPUT_SIZE, 1280, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_OUTPUT_SIZE, 720, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_Y_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_640x480_x2_analog_200fps[] = {
> >> + /* Recommended coarse integration time value: 477 */
> >> + { IMX219_REG_FRAME_LEN_LINES, 481, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_LINE_LEN_PCK, 3448, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_START, 1000, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_END, 2280, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_START, 752, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_END, 1712, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_OUTPUT_SIZE, 640, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_OUTPUT_SIZE, 480, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_Y_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_H, 3, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_V, 3, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> +};
> >> +
> >> +static const struct imx219_reg mode_640x80_x2_analog_1000fps[] = {
> >> + /* Recommended coarse integration time value: 92 */
> >> + { IMX219_REG_FRAME_LEN_LINES, 481, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_LINE_LEN_PCK, 3448, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_START, 1320, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ADDR_END, 2600, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_START, 990, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_ADDR_END, 1561, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_OUTPUT_SIZE, 640, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_Y_OUTPUT_SIZE, 80, IMX219_REG_VALUE_16BIT, true },
> >> + { IMX219_REG_X_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_Y_ODD_INC, 1, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_H, 3, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_MODE_V, 3, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_H, 0, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_BINNING_CAL_V, 0, IMX219_REG_VALUE_08BIT, true },
> >> };
> >>
> >> static const struct imx219_reg raw8_framefmt_regs[] = {
> >> - {0x018c, 0x08},
> >> - {0x018d, 0x08},
> >> - {0x0309, 0x08},
> >> + { IMX219_REG_CSI_DATA_FORMAT_HI, 8, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_CSI_DATA_FORMAT_LO, 8, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_OP_PIX_CLK_DIV, 8, IMX219_REG_VALUE_08BIT, false },
> >> };
> >>
> >> static const struct imx219_reg raw10_framefmt_regs[] = {
> >> - {0x018c, 0x0a},
> >> - {0x018d, 0x0a},
> >> - {0x0309, 0x0a},
> >> + { IMX219_REG_CSI_DATA_FORMAT_HI, 10, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_CSI_DATA_FORMAT_LO, 10, IMX219_REG_VALUE_08BIT, true },
> >> + { IMX219_REG_OP_PIX_CLK_DIV, 10, IMX219_REG_VALUE_08BIT, false },
> >> };
> >>
> >> static const char * const imx219_test_pattern_menu[] = {
> >> @@ -461,73 +483,98 @@ static const u32 codes[] = {
> >> * case of DT for regulator-fixed one should define the startup-delay-us
> >> * property.
> >> */
> >> -#define IMX219_XCLR_MIN_DELAY_US 6200
> >> +#define IMX219_XCLR_MIN_DELAY_US 7200
> >
> > Why?
> >
>
> Because I forgot to update the comment, that's why you're asking!!!
> ...but you definitely deserve your answer before me updating the
> comment, so here's the gold:
>
> This driver was clearly made with the Raspberry Pi IMX219 camera sensor
> board in mind - and there's nothing wrong with this: the issue here is
> that this board (combined to the SoC on which it was tested) seems to
> require less time to bring up the XCLR compared to the camera module
> that is attached to the Sony Xperia XA2 and Sony Xperia XA2 Ultra
> smartphones.
>
> On these smartphones, it looks like the minimum delay (from my own
> tests) is around 6800uS, but that doesn't work in all of the conditions
> that I've tested (sometimes does, sometimes does not).
> The best *stable* delay that I've found (stable meaning that it always
> worked) was 7000uS so - accounting for eventual other factors (that I
> don't need to explain, as you surely know what I'm talking about, basic
> electronics anyway), I chose to go for sure stability, giving it 200
> more microseconds - reaching 7200uS.
>
> So.. that's why :)

If there is an additional delay required for the regulators to
stabilise on a particular board then that should be accounted for in
the regulator framework.
The last paragraph of the comment above even states this:

* This delay doesn't account for the power supply startup time. If needed,
* this should be taken care of via the regulator framework. E.g. in the
* case of DT for regulator-fixed one should define the startup-delay-us
* property.

The duration of this delay is lifted straight from the sensor
datasheet so is not open for debate (beyond potential for a very small
optimisation).
Computing the value was explicitly asked for when the original driver
was submitted [1].

We have a similar "slow" response to being enabled for our IMX477
boards. If you look at the DT overlay for that [2] we define
"startup-delay-us" for the regulator to compensate for the slowness of
the board.

Dave

[1] https://patchwork.linuxtv.org/project/linux-media/patch/20191227122114.23075-3-andrey.konovalov@xxxxxxxxxx/#114407
[2] https://github.com/raspberrypi/linux/blob/rpi-5.4.y/arch/arm/boot/dts/overlays/imx477-overlay.dts#L79