Re: [PATCH 00/21] imx258 improvements series

From: Arnaud Ferraris <arnaud.ferraris@collabora.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 00/21] imx258 improvements series
Date: Tue, 18 Jul 2023 14:08:06 +0200	[thread overview]
Message-ID: <02a66bb5-3029-3392-f2d2-47fa0d8a343c@collabora.com> (raw)
In-Reply-To: <20230530173000.3060865-1-dave.stevenson@raspberrypi.com>

Hi Dave,

Le 30/05/2023 à 19:29, Dave Stevenson a écrit :
> This is a set of patches for imx258 that allow it to work with alternate clock
> frequencies, over either 2 or 4 lanes, and generally adding flexibility to the
> driver.
>
> Tested with an IMX258 module from Soho Enterprises that has a 24MHz oscillator.
> Both 2 and 4 lane configurations work with correct link frequencies and pixel
> rates.
>
> Jacopo has tested on a PinePhone Pro which has an ~19.2MHz clock fed from the SoC,
> He confirms that the two lower resolution modes work, but not the full res mode.
> Comparing to the BSP it looks like they have some weird clock configuration in
> the 4208x3120 mode (nominally 1224Mb/s/lane instead of 1267).
> As it has never previously worked directly with the mainline driver this isn't a
> regression but may indicate that there is a need for support of additional link
> frequencies in the future.

FYI, I have tested this series on PinePhone Pro as well, but this time 
changing the clock to 24 MHz.
I didn't test all modes (not sure how to do this), but feel free to add 
my Tested-by to the whole series for v2.

Thanks for your work on this,
Arnaud

>
> The last patch that makes HFLIP and VFLIP configurable may be contentious as I've
> retained the default configuration of inverted from the original driver. I know
> this was discussed recently, but I can't recall the final outcome.
>
> I am relying on someone from Intel testing this out, as correcting the cropping
> and supporting flips has changed the Bayer order. Seeing as this is all above
> board in V4L2 terms I really hope that the layers above it behave themselves.
>
> Cheers
>    Dave
>
> Dave Stevenson (21):
>    media: i2c: imx258: Remove unused defines
>    media: i2c: imx258: Make image geometry meet sensor requirements
>    media: i2c: imx258: Disable digital cropping on binned modes
>    media: i2c: imx258: Remove redundant I2C writes.
>    media: i2c: imx258: Add regulator control
>    media: i2c: imx258: Make V4L2_CID_VBLANK configurable.
>    media: i2c: imx258: Split out common registers from the mode based
>      ones
>    media: i2c: imx258: Add support for 24MHz clock
>    media: i2c: imx258: Add support for running on 2 CSI data lanes
>    media: i2c: imx258: Follow normal V4L2 behaviours for clipping
>      exposure
>    media: i2c: imx258: Add get_selection for pixel array information
>    media: i2c: imx258: Allow configuration of clock lane behaviour
>    media: i2c: imx258: Correct max FRM_LENGTH_LINES value
>    media: i2c: imx258: Add support for long exposure modes
>    media: i2c: imx258: Issue reset before starting streaming
>    media: i2c: imx258: Set pixel_rate range to the same as the value
>    media: i2c: imx258: Support faster pixel rate on binned modes
>    dt-bindings: media: imx258: Rename to include vendor prefix
>    dt-bindings: media: imx258: Add alternate compatible strings
>    media: i2c: imx258: Change register settings for variants of the
>      sensor
>    media: i2c: imx258: Make HFLIP and VFLIP controls writable
>
>   .../i2c/{imx258.yaml => sony,imx258.yaml}     |   9 +-
>   MAINTAINERS                                   |   2 +-
>   drivers/media/i2c/imx258.c                    | 994 ++++++++++++------
>   3 files changed, 675 insertions(+), 330 deletions(-)
>   rename Documentation/devicetree/bindings/media/i2c/{imx258.yaml => sony,imx258.yaml} (90%)
>