Message-ID: <c661e7f3faec269f73d6240fbe7b84e3bc97157a.camel@collabora.com>
Date: Thu, 03 Jul 2025 15:54:20 +0200
From: Julien Massot <julien.massot@...labora.com>
To: Cosmin Tanislav <demonsingur@...il.com>, Cosmin Tanislav	
 <cosmin.tanislav@...log.com>, Tomi Valkeinen	
 <tomi.valkeinen+renesas@...asonboard.com>, Mauro Carvalho Chehab	
 <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, Niklas
 Söderlund	 <niklas.soderlund@...natech.se>, Sakari Ailus
 <sakari.ailus@...ux.intel.com>,  Laurent Pinchart
 <laurent.pinchart@...asonboard.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, Linus Walleij <linus.walleij@...aro.org>
Cc: linux-media@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-staging@...ts.linux.dev, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v5 00/24] media: i2c: add Maxim GMSL2/3 serializer and
 deserializer drivers

On Thu, 2025-07-03 at 15:17 +0300, Cosmin Tanislav wrote:
> 
> 
> On 7/3/25 3:07 PM, Julien Massot wrote:
> > Hi Cosmin,
> > 
> > On Wed, 2025-07-02 at 16:20 +0300, Cosmin Tanislav wrote:
> > > This series adds new drivers for multiple Maxim GMSL2 and GMSL3 devices,
> > > replacing the few GMSL2 drivers already in upstream, and introducing a
> > > common framework that can be used to implement such GMSL chips, which
> > > avoids code duplication while also adding support for previously
> > > unsupported features.
> > > 
> > > While the normally acceptable and polite way would be to extend the
> > > current mainline drivers, the choice was made here to add a totally new
> > > set of drivers. The current drivers support only a small subset of the
> > > possible features, and only a few devices, so the end result after
> > > extending them would in any case be essentially fully rewritten, new
> > > drivers.
> > > 
> > Thanks for your work,
> > The common framework will help a lot to drive new GMSL chips, and most of the
> > features are covered.
> > 
> > > This series depends on support for internal pads, for which a patch has
> > > been added.
> > > 
> > > The previous version is at:
> > > https://lore.kernel.org/lkml/20250618095858.2145209-1-demonsingur@gmail.com
> > > 
> > > The following deserializers are supported:
> > >   * MAX96712 (already exists in staging)
> > >   * MAX96714 (already exists)
> > >   * MAX96714F (already exists)
> > >   * MAX96714R (GMSL2)
> > >   * MAX96716 (GMSL2)
> > >   * MAX96724 (already exists as part of existing MAX96712 driver)
> > >   * MAX96724F (GMSL2)
> > >   * MAX96724R (GMSL2)
> > >   * MAX9296A (GMSL2)
> > >   * MAX96792A (GMSL3)
> > > 
> > > The following serializers are supported:
> > >   * MAX96717 (already exists)
> > >   * MAX9295A (GMSL2)
> > >   * MAX96793 (GMSL3)
> > > 
> > > Known backward compatibility breakages:
> > >   * No default routing. Default routing has been intentionally ommitted
> > >     as the devices support quite complex routing and it would be
> > >     unfeasible to provide sane defaults for multi-link deserialziers.
> > >     It is expected that userspace programs would set appropritate
> > >     routing.
> > > 
> > This part is the most annoying one: at the moment, there is no way to set the routing except by
> > manually enabling a boolean within the kernel source.
> > You can't guess what routing the user really wants, but please at least provide a default
> > routing
> > table that allows using your drivers — for example, the device's default routing.
> > 
> 
> It's a very delicate issue... I'll try to see if I can do that.
> It would be great if we could enable the streams API globally since it's
> been merged since Jan 15 2023. It's been over two years.
> 
> 
> Thanks,
> 
> > 
> > 
> > > The following list enumerates new features that are supported by the
> > > common framework and their respective chip-specific drivers:
> > >   * Full Streams API support. Most deserializers have support for more
> > >     than one link, and more than one PHY. Streams support allows
> > >     configuration of routing between these links and PHYs.
> > > 
> > >   * .get_frame_desc() support. Both the serializers and deserializers
> > >     implement this to query and provide frame descriptor data. This is
> > >     used in features explained in-depth below.
> > 
> > So are almost all the sensor drivers incompatible?
> > 
> 
> Yes, sensor drivers need to have .get_frame_desc() implemented... It's
> not a huge feat and it's the only way this type of bridge could work
> properly.
> 
> Alternatively, we could add a fallback that bases its decision on the
> stream format, but I'd prefer if we didn't and we would just implement
> .get_frame_desc(). After this series is merged I can submit my patches
> for imx219.
There is already one pending on the mailing list
"media: i2c: imx219: Report streams using frame descriptors"
I guess it's fine if we require the sensor to implement this function.

But I had to do it for vgxy61.

Btw I tested:
TI AM62x + max96716 + 1 x max96717f + stvg5661 (tunnel mode)
With special lanes mapping and polarities.

And I had to:

- Apply pending patches for j721e to support the enable_stream API instead of s_stream
- Enable the experimental v4l2_subdev_enable_streams_api
- Add get_frame_desc to the sensor driver

Powered by blists - more mailing lists