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@xxxxxxxxxxxxxxx> wrote: > > > > Hi Laurent > > > > On Sun, 16 Oct 2022 at 06:37, Laurent Pinchart > > <laurent.pinchart@xxxxxxxxxxxxxxxx> 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