Re: [PATCH 2/2] media: i2c: Add driver for Sony IMX662 sensor

[Jai Luthra <jai.luthra@ideasonboard.com>]

Hi Dave,

Quoting Dave Stevenson (2026-07-20 23:09:34)
> Hi Jai
> 
> On Mon, 20 Jul 2026 at 15:44, Jai Luthra <jai.luthra@ideasonboard.com> wrote:
> >
> > Hi Dave, Alexander,
> >
> > Quoting Dave Stevenson (2026-03-13 01:20:15)
> > > Hi Alexander
> > >
> > > On Thu, 12 Mar 2026 at 15:11, Alexander Shiyan
> > > <eagle.alexander923@gmail.com> wrote:
> > > >
> > > > This patch adds a V4L2 subdevice driver for the Sony IMX662 CMOS image
> > > > sensor. The sensor has a native resolution of 1936x1100 (effective
> > > > 1920x1080) and can achieve up to 90 frames per second depending on
> > > > the configuration. The driver supports:
> > > > - MIPI CSI-2 with 2 or 4 data lanes.
> > > > - RAW10 and RAW12 formats (both colour and monochrome).
> > > > - Controls: exposure, analogue gain, horizontal/vertical blanking,
> > > >   horizontal/vertical flip, brightness.
> > > > - A placeholder V4L2_CID_HDR_SENSOR_MODE control for future Clear HDR
> > > >   support (the actual HDR modes are not yet implemented).
> > > > - Runtime PM.
> > > > - Cropping via the selection API.
> > > > - Multiple link frequencies selectable via device tree.
> > > >
> > > > Tested on ARM64 Rockchip RK3568 platform with a 24 MHz external clock
> > > > and various link frequencies.
> > >
> > > Interesting timing as I've been looking at IMX662 too, but on a Raspberry Pi.
> > > FWIW my driver is at
> > > https://github.com/6by9/linux/blob/rpi-6.12.y-imx662/drivers/media/i2c/imx662.c
> > >
> >
> > I saw libcamera patches for IMX676,
> 
> IMX676? I assume that's a typo.
> 

Oops, yes I meant IMX662. Names are hard.

> > does the below branch have the latest
> > driver you plan to post as v2?
> >
> > https://github.com/6by9/linux/blob/rpi-6.18.y-imx662-mainline-clean/drivers/media/i2c/imx662.c
> 
> That's a downstream Pi tree. My mainline branch is
> https://github.com/6by9/linux/tree/mainline_imx662 and is basically
> the patches I intend to send.

Thank you.

> There is one niggle to resolve first in imx662_ctrls_init. There is
> currently a catch-22 in setting up the state vs initialising the
> blanking and exposure ranges as they refer to each other. I need to
> come back and look at that one.
> 

I remember I faced something similar in IMX678 and ended up doing that
calculation explicity while initializing the blanking/exposure controls,
without reusing the set_framing_limits() function that is called on
resolution updates.

> > > Comments are based on brief testing and reading of the code. This is
> > > not a full review.
> > >
> > > > Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> >
> > [...]
> >
> > > > +static int imx662_set_selection(struct v4l2_subdev *sd,
> > > > +                               struct v4l2_subdev_state *sd_state,
> > > > +                               struct v4l2_subdev_selection *sel)
> > > > +{
> >
> > Can we keep cropping and binning out of the initial driver,
> 
> There is no binning currently implemented in this driver, only cropping.
> 

Okay, that might make things easier to port to the new model.

> > and implement
> > it using the common raw sensor model directly?
> >
> > I did it for IMX678 [1] on Sakari's suggestion [2]. The two sensors are
> > quite similar, so I'm happy to help in whatever way I can on getting this
> > working with the new model too :-)
> 
> Is this effectively saying that linux-media is closed to sensor
> drivers not adopting the common raw sensor model, even though AIUI
> that isn't ready yet? I can accept that for a period, but how long is
> it reasonable for that to be the case? A year?
> With all due respect to those involved, the series has been around so
> long in various guises that it's a case of I'll believe it when I see
> it merged.
> 
> There are already a number of sensor drivers in the tree that support
> set_selection (imx296 being the main one I care about). Presumably
> there is a plan for how to migrate those to work with the common
> sensor model without regression for existing users. Is there a reason
> that plan isn't also valid for imx662 and imx678?
> 

Yes we can migrate in a way to support both the old and new models
depending upon the client capability flags, Sakari has done that for
IMX219. But it gets a bit cumbersome in the driver, especially for handling
binning through the new controls while supporting it through some other way
in old model.

Given you don't plan to add binning support, I think it shouldn't be as
complicated. The selection rectangle for analogue crop will move to the
internal image pad from the source pad if the client has the
COMMON_RAW_SENSOR capability.

> I would offer to review the common raw sensor patches, but as so much
> of it is core framework stuff and so is outside my area of knowledge.
> I guess I can do the same as your imx678 branch and give it a go for a
> Tested-by, but that feels fairly limited in scope.
> Have you got a libcamera branch that supports the new model for imx678?
> 

I looked into it briefly, here's a branch with bare minimum support to get
libcamera streaming with the new model:
https://github.com/jailuthra/libcamera/tree/raw-sensors

It doesn't support crop/binning yet.

> Thanks
>   Dave
> 

Thanks,
Jai