Re: [PATCH 00/19] media: i2c: imx290: Miscellaneous improvements

Sakari Ailus
2022-10-19T11:05:22Z

Hi Dave,

On Mon, Oct 17, 2022 at 07:07:20PM +0100, Dave Stevenson wrote:
> Hi Laurent
>
> On Sun, 16 Oct 2022 at 08:34, Dave Stevenson
> <dave.stevenson@raspberrypi.com> wrote:
> >
> > Hi Laurent
> >
> > On Sun, 16 Oct 2022 at 06:37, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > >
> > > Hi Dave,
> > >
> > > On Mon, Oct 10, 2022 at 11:31:33AM +0100, Dave Stevenson wrote:
> > > > Hi Laurent
> > > >
> > > > Do you have plans for a v2 on this patch set? I also have a number of
> > > > patches for imx290 and there's little point in causing grief to each
> > > > other with conflicts.
> > > > Or I could take the non-controversial patches from your set and send a
> > > > combined patch set?
> > >
> > > I'm working on a v2, I'll send it shortly. Do I assume correctly you'll
> > > submit IMX327 support on top ? :-)
> >
> > Thanks - I'll review it tomorrow and sort my patches on top again.
>
> I've reworked my patches on top of yours. It gives r/w VBLANK and
> HBLANK, and corrects PIXEL_RATE.
> I'm testing on our 6.0 branch, but
> https://github.com/6by9/linux/commits/linuxtv_imx290/drivers/media/i2c
> is against the linux-media branch.
>
> I've messed something up in the "media: i2c: imx290: Support 60fps in
> 2 lane operation" patch at present - I'm looking into what has gone
> wrong, as the earlier versions of that patch worked fine. The branch
> will get force-pushed once I've fixed it.
>
> > This driver is effectively IMX327 - max 1920x1080@60fps in 12 bit.
> >
> > IMX290 adds a 1920x1080@120fps 10bit only mode which isn't currently
> > supported by the driver. I have patches to add 10bit support, but I
> > don't increase the frame rate in them.
> >
> > IMX462 adds that 1920x1080@120fps mode in both 10 and 12 bit, but
> > again I haven't looked at adding support, partly as I don't have a
> > datasheet for that variant. I may see if the change for 120fps 10bit
> > on imx290 works in 12 bit mode for IMX462.
> > For IMX290, 1080p120 needs a link frequency of 445.5MHz on 4 lanes to
> > be supported (2 lanes not permitted), so there will be more link
> > frequency messing required to support it. The basic numbers say that
> > is fast enough for 12bit as well, so there's hope.
>
> I guess seeing as I'm messing with the clock setup, I may as well keep
> going and look at the 120fps modes. It's a little trickier as the Pi
> ISP will be on the edge at those rates, but it should be good enough.
>
> There is an awkward question with regard link-frequencies. Is there a
> need to support multiple sets of link-frequency, or do we support any
> set of 2?
> ie for imx290, on 4 lanes do we want:
> - 891Mbit/s for 1080p120 10bit
> - 445.5Mbit/s for 1080p60 10 or 12 bit
> - 594Mbit/s for 720p120 10bit
> - 297Mbit/s for 720p60 10 and 12 bit
> all to be present in DT?
> If only 891 and 594 then you're limited to 10 bit images, but
> otherwise it should be fully functional. The max frame interval would
> be half that of 445.5 and 297 though, so there are compromises, but
> who/what then controls the link_frequency to switch between the
> ranges?

It's up to the user space to set that control in a general case. I guess
there are no specific rules on how many you should put to DT, but generally
those that are useful should be there.

I wonder why 12 bpp output isn't possible at the double frequency. Of
course it is possible the sensor's clock tree makes that impossible but it
is still unusual.

>
> I can see another can of worms being opened here!

If this is what the sensor does, how else it should be operated?

--
Kind regards,

Sakari Ailus