Re: [PATCH RFC 3/4] media: i2c: imx678: Add imx662 support to the driver

[Dave Stevenson]

Hi Jai

On Thu, 3 Sept 2026 at 05:20, Jai Luthra <jai.luthra@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Dave,
>
> Quoting Dave Stevenson (2026-08-05 21:39:18)
> > IMX662 is a 1080p Starvis 2 sensor that follows the same programming
> > pattern as IMX678, so add it to the newly parameterised driver.
> >
> > FIXME: Update the common registers based on the Sony spreadsheet.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/media/i2c/imx678.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/drivers/media/i2c/imx678.c b/drivers/media/i2c/imx678.c
> > index 40ba6661b366..9083f2744510 100644
> > --- a/drivers/media/i2c/imx678.c
> > +++ b/drivers/media/i2c/imx678.c
> > @@ -673,6 +673,47 @@ const struct imx678_variant imx678_variant_def = {
> >         .hmax_min = imx678_min_hmax_4lane,
> >  };
> >
> > +static const struct cci_reg_sequence imx662_common_regs[] = {
> > +       { IMX678_REG_WDMODE, 0x00 },
> > +       { IMX678_REG_MDBIT, 0x01 },
> > +       { IMX678_REG_XXS_DRV, 0x00 },
> > +};
> > +
> > +static const u16 imx662_min_hmax_4lane[] = {
> > +       [IMX678_LINK_FREQ_297MHZ] = 990,
> > +       [IMX678_LINK_FREQ_360MHZ] = 990,
> > +       [IMX678_LINK_FREQ_445MHZ] = 990,
> > +       [IMX678_LINK_FREQ_594MHZ] = 990,
> > +       [IMX678_LINK_FREQ_720MHZ] = 990,
> > +       [IMX678_LINK_FREQ_891MHZ] = 990,
> > +       [IMX678_LINK_FREQ_1039MHZ] = 990,
> > +       [IMX678_LINK_FREQ_1188MHZ] = 990,
> > +};
> > +
> > +const struct imx678_variant imx662_variant_def = {
> > +       .name = "imx662",
> > +       .id_reg = IMX678_REG_MODULE_ID,
> > +       .id_value = 0x296,
> > +       .native_area = {
> > +               .top = 0,
> > +               .left = 0,
> > +               .width = 1937,
> > +               .height = 1101,
> > +       },
> > +       .active_area = {
> > +               .top = 0,
> > +               .left = 0,
> > +               .width = 1936,
> > +               .height = 1096,
> > +       },
> > +       .pixel_rate = 222750000,
> > +       .pix_per_clk = 3,
> > +       .common_regs = imx662_common_regs,
> > +       .num_common_regs = ARRAY_SIZE(imx662_common_regs),
> > +       .vmax_default = 1096 + 40,
>
> From the IMX662 SRM I see the following constraints for minimum:
>
>     VTTL >= PIX_VWIDTH + 70
>     VTTL >= 820
>
> And the table giving round FPS numbers like 30/60/90 assumes VMAX = 1250.

I find it weird that Sony's datasheets fix the number of vertical
lines and extend horizontal blanking for frame rate control. That'll
give you the worst case for the rolling shutter effect, so why would
you want to do that?

> Did you manually verify that setting blanking to 40 also works?

Yes, I've verified that vblank of 40 works.
This is why I queried the values in the IMX908 driver that Lachlan
sent, as the datasheets always seem overly conservative. There may be
further restrictions if you enable the various HDR modes, but we're
not there yet.

  Dave

> Thanks,
>     Jai
> > +       .hmax_min = imx662_min_hmax_4lane,
> > +};
> > +
> >  struct imx678_model_info {
> >         enum imx678_type type;
> >         const u32 *codes;
> > @@ -703,6 +744,26 @@ static const struct imx678_model_info imx678_autodetect_info = {
> >         .auto_detect_mono = &imx678_aamr_info,
> >  };
> >
> > +static const struct imx678_model_info imx662_aaqr_info = {
> > +       .type = IMX678_COLOR,
> > +       .codes = codes_bayer,
> > +       .num_codes = ARRAY_SIZE(codes_bayer),
> > +       .variant = &imx662_variant_def,
> > +};
> > +
> > +static const struct imx678_model_info imx662_aamr_info = {
> > +       .type = IMX678_MONOCHROME,
> > +       .codes = codes_monochrome,
> > +       .num_codes = ARRAY_SIZE(codes_monochrome),
> > +       .variant = &imx662_variant_def,
> > +};
> > +
> > +static const struct imx678_model_info imx662_autodetect_info = {
> > +       .variant = &imx662_variant_def,
> > +       .auto_detect_colour = &imx662_aaqr_info,
> > +       .auto_detect_mono = &imx662_aamr_info,
> > +};
> > +
> >  static const char * const imx678_supply_name[] = {
> >         "avdd",  /* Analog (3.3V) supply */
> >         "dvdd",  /* Digital Core (1.1V) supply */
> > @@ -1470,8 +1531,11 @@ static const struct dev_pm_ops imx678_pm_ops = {
> >  static const struct of_device_id imx678_of_match[] = {
> >         { .compatible = "sony,imx678-aamr", .data = &imx678_aamr_info },
> >         { .compatible = "sony,imx678-aaqr", .data = &imx678_aaqr_info },
> > +       { .compatible = "sony,imx662-aamr", .data = &imx662_aamr_info },
> > +       { .compatible = "sony,imx662-aaqr", .data = &imx662_aaqr_info },
> >         /* for non-conforming DTs that rely on runtime check */
> >         { .compatible = "sony,imx678", .data = &imx678_autodetect_info },
> > +       { .compatible = "sony,imx662", .data = &imx662_autodetect_info },
> >         { /* sentinel */ }
> >  };
> >
> >
> > --
> > 2.34.1
> >