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

Hi Dave, Jai, Laurent,

On Fri, Jul 31, 2026 at 01:11:25PM +0530, Jai Luthra wrote:
> Hi Dave,
> 
> + Sakari, Laurent
> 
> Quoting Dave Stevenson (2026-07-30 16:42:01)
> > Hi Jai
> > 
> > On Tue, 21 Jul 2026 at 06:06, Jai Luthra <[email protected]> wrote:
> > >
> > > Hi Dave,
> > 
> > <snip>
> > 
> > > > > 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 :-)
> > 
> > I'd also noticed that all the Starvis 2 sensors are very similar, to
> > the extent that it seemed worth a very quick test to see how much
> > needed to change for the imx678 driver to work with imx662.
> > 
> > The answer turned out to be really not much:
> > - PIXEL_RATE
> > - PIX_PER_CLK
> > - ID
> > - native and active areas
> > - min_hmax
> > - common_regs (culled nearly all of them, although I do have the
> > spreadsheet from Sony which needs to be added to my next version)
> > - VMAX_DEFAULT setup if you want the full frame rate.
> > 
> 
> Great! I was hoping for similarities too, but if you got IMX662 streaming
> already with the IMX678 driver that's quite good news.
> 
> > Those would all parameterise quite easily.
> > 
> > I also have an IMX675 module from Soho Enterprises which is a 5MPix
> > Starvis 2 sensor. I don't have a datasheet for it at present.
> > That streams OK at the requested rates by just updating the active
> > area to 2608x1960 and ignoring the ID. However I can only receive test
> > patterns as all the active images I get are pure black :( I'm hoping
> > it's a faulty module, but will keep poking.
> > 
> > There is also IMX585 as an 8MPix Starvis2 sensor. I'd hoped Naush had
> > one to test, but it seems not. In the meantime I've compared against
> > the driver Will Whang sent to linux-media a while back [1] and that
> > also looks largely the same.
> > 
> 
> Indeed looking at the documentation for IMX676, IMX678 and IMX662, I see
> the features are mostly same, except minor differences:
> 
> IMX676 and IMX678 support 8 and 4x2-lane (with XSIZE overlap) features, but
> IMX662 does not (probably because it's only ~2MP).
> 
> IMX676 supports dual-speed streaming (DSS) using MIPI VC1 for sending
> 2x/4x/.. FPS data for a smaller region-of-interest.
> 
> The registers otherwise look identical, so a shared driver would make
> sense.

I agree.

> 
> > And the imx908 Starvis 3 driver that has just been posted [2] is also
> > looking incredibly similar.
> > 
> 
> The flyer for IMX908 mentions some extra HDR modes. If there are some other
> big architectural differences in Starvis 3 I couldn't immediately make them
> out from the posted driver.

I wouldn't worry about this too much. If there are still shared features,
the differences can be usually well compartmentalised and it's not a
problem if only some supported devices implement these features. Think of
e.g. the CCS driver. I recall the i915 driver supports more than 10
generations of Intel GPUs, with quite a bit of differences in features and
implementation. Also the ipu6 driver will soonish gain support for IPU7 and
IPU7.5, albeit the functionality of these is roughly similar.

> 
> > So the big question is whether it is better to have separate drivers
> > for all these sensors, or one combined Starvis2/3 driver? Do we shoot
> > ourselves in the foot when we come to add functionality and find that
> > it only applies to some models?
> > The awkward part would be testing all variants when patches are
> > submitted, as I suspect there won't be one person that has access to
> > all of them.
> > 
> 
> Honestly, I have the same question. I would love to share as much code as
> possible so we only do the painful things like moving to new APIs once,
> which I've already done for IMX678, and it was not a quick exercise.
> 
> I haven't deep-dived enough on the different HDR modes (DOL, ClearHDR,
> and Starvis 3 hybrid HDR) or the DSS feature to know how easy it would be
> to test and maintain all of those in a single driver. The HDR modes support
> using MIPI VC 0,1,2 or line-data to distinguish between short/long
> exposure/gain frames, and DSS also uses VC 0 and 1, so the book-keeping
> around the combinatorial possibilities of what is allowed or not allowed
> across different sensors may make a single driver a bit messy.
> 
> An alternative could be to create a common starvis2.c module with helpers
> for shared boilerplate that separate sensor drivers can use. Even if we go
> the helper route, we would still need multiple people testing or acking
> patches that touch the shared code. But I see that in the same way as other
> common parts of the framework that effect multiple drivers.

I'd suggest implementing a single driver. Using helpers requires defining
APIs and you'll have lots of users of these APIs, too, like we have for
e.g. V4L2 sub-devices. If you don't assume a common data structure, e.g.
"starvis" struct that pretty much would assume what the device can do
(compare with a single driver!), this easily becomes cumbersome.

> 
> > Thoughts appreciated.
> > 
> 
> Given we don't support HDR modes or the DSS feature today, and probably
> lack proper APIs in the framework for both, I feel like having a shared
> module of helpers will be quite an effort and a bit of premature
> optimization for uncertain gains.
> 
> So, I am leaning towards having a common driver for all the Starvis 2
> drivers with multiple maintainers. I lack enough information on IMX908
> (Starvis 3) to be sure if that can also be squeezed in.
> 
> If it does get hard to maintain in a single driver in future, we could
> split out common parts at that point without losing the effort we put
> today.
> 
> But let's see what Laurent and Sakari think as well.

-- 
Regards,

Sakari Ailus