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? I can see another can of worms being opened here! Dave > Dave > > > > On Tue, 23 Aug 2022 at 02:12, Laurent Pinchart wrote: > > > > > > > > Hi Sakari, > > > > > > > > Could you already pick up patches 01/19, 02/19, 04/19, 05/19 and 06/19 > > > > in your tree ? Your opinion on 03/19 woud be appreciated too, I think > > > > it's a candidate for merge as well. > > > > > > > > On Thu, Jul 21, 2022 at 11:35:21AM +0300, Laurent Pinchart wrote: > > > > > Hello, > > > > > > > > > > This patch series gathers miscellaneous improvements for the imx290 > > > > > driver. The most notable changes on the kernel side is patch 07/19 that > > > > > simplifies register access, and on the userspace API side patches 14/19, > > > > > 15/19 and 18/19 that extend the driver with controls and selection > > > > > rectangles required by libcamera. > > > > > > > > > > Laurent Pinchart (19): > > > > > media: i2c: imx290: Use device lock for the control handler > > > > > media: i2c: imx290: Print error code when I2C transfer fails > > > > > media: i2c: imx290: Specify HMAX values in decimal > > > > > media: i2c: imx290: Replace macro with explicit ARRAY_SIZE() > > > > > media: i2c: imx290: Drop imx290_write_buffered_reg() > > > > > media: i2c: imx290: Drop regmap cache > > > > > media: i2c: imx290: Support variable-sized registers > > > > > media: i2c: imx290: Correct register sizes > > > > > media: i2c: imx290: Simplify error handling when writing registers > > > > > media: i2c: imx290: Define more register macros > > > > > media: i2c: imx290: Add exposure time control > > > > > media: i2c: imx290: Fix max gain value > > > > > media: i2c: imx290: Split control initialization to separate function > > > > > media: i2c: imx290: Implement HBLANK and VBLANK controls > > > > > media: i2c: imx290: Create controls for fwnode properties > > > > > media: i2c: imx290: Move registers with fixed value to init array > > > > > media: i2c: imx290: Factor out format retrieval to separate function > > > > > media: i2c: imx290: Add crop selection targets support > > > > > media: i2c: imx290: Replace GAIN control with ANALOGUE_GAIN > > > > > > > > > > drivers/media/i2c/imx290.c | 781 ++++++++++++++++++++++--------------- > > > > > 1 file changed, 458 insertions(+), 323 deletions(-) > > > > -- > > Regards, > > > > Laurent Pinchart
