Re: [PATCH v2 0/3] media: imx415: Improvements for framerate and link frequency control

[Michael Riesch]

Hi Dave,

Thanks for your patches!

On 1/16/25 18:43, Dave Stevenson wrote:
> The driver was using reverse engineered pixel rates, and had different
> values for different link frequencies.
> In common with the other Starvis sensors (eg imx290), this isn't
> actually the situation, and the pixel array runs off a different
> PLL to the MIPI block, and there is a FIFO between the two.
> 
> It's not quite so simple as HMAX is said to be in units of INCK,
> not pixel clocks. Working through the various modes that appears that
> even that is slightly wrong as it is units of a scaled INCK of
> either 72MHz (for INCK = 24 or 72MHz) or 74.25MHz (for INCK = 27,
> 37.125, or 74.25MHz).
> 
> Rework the configuration so that the pixel rate is fixed, with the
> different link frequencies adjusting the minimum hmax value so that
> the FIFO doesn't overflow.
> 
> Switching between 2 and 4 data lanes also only changes the MIPI data
> rate, so also can be handled by altering the minimum HMAX.
> 
> Once you've got HMAX / V4L2_CID_HBLANK represented sensibly, then
> frame rate control through V4L2_CID_HBLANK and V4L2_CID_VBLANK is
> trivial, so add in frame rate control too.
> Exposure is set in lines (as usual), so amending VBLANK changes the
> range for the exposure control.
> 
> This has been tested on a Pi5 with Waveshare "IMX415-98 IR-CUT Camera"
> module (SKU 28524). That uses a 24MHz clock, so only the 360MHz and
> 720MHz link frequencies have been tested (other frequencies require
> a 27, 37.125, or 74.25MHz clock input.
> 
> It'd be nice to add support for 12bit readout and windowed mode readout,
> but those are a job for another day.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

Reviewed-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx>

Regards,
Michael

> ---
> Changes in v2:
> - Fixed VBLANK fall-through for setting the exposure correctly
> - Sorted the checkpatch complaint over using fallthrough; vs commenting
>   why we're falling through.
> - Corrected HMAX for 891Mbit/s mode. I now have a module with 37.125MHz
>   clock, and have validated that.
> - Link to v1: https://lore.kernel.org/r/20250109-media-imx415-v1-0-366ba6a234ab@xxxxxxxxxxxxxxx
> 
> ---
> Dave Stevenson (3):
>       media: i2c: imx415: Add read/write control of VBLANK
>       media: i2c: imx415: Make HBLANK controllable and in consistent units
>       media: i2c: imx415: Link frequencies are not exclusive to num lanes
> 
>  drivers/media/i2c/imx415.c | 182 +++++++++++++++++++++++----------------------
>  1 file changed, 93 insertions(+), 89 deletions(-)
> ---
> base-commit: 94794b5ce4d90ab134b0b101a02fddf6e74c437d
> change-id: 20250107-media-imx415-2df67ae21107
> 
> Best regards,