Re: [PATCH] media: i2c: imx471: Fix pixel rate and line length

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Christian,

Thanks for the patch.

On Sun, Aug 30, 2026 at 06:33:45PM +0100, Christian Murphy wrote:
> The pixel rate is derived from the CSI-2 link frequency, 200 MHz * 2 *
> 4 lanes / 10 bits = 160 MHz, and the line length is the binned array
> width, 2328. Neither describes the pixel array.
> 
> The VT PLL gives 19.2 MHz / PREPLLCK_VT_DIV 2 * PLL_VT_MPY 121 /
> VTPXCK_DIV 6 = 193.6 MHz and the array reads two pixels per clock, so
> the pixel rate is 387.2 MHz. LINE_LENGTH_PCK is never written and its
> power-on default reads back 5120. Timing frames at several written
> line lengths confirms 387.2 MHz at every value.
> 
> With the declared values the line duration comes out 14.55 us instead
> of 13.22 us and exposure is overstated by 10%: libcamera reports a
> maximum ExposureTime longer than the frame.
> 
> Set the pixel rate to 387.2 MHz and the line length to 5120, which
> changes HBLANK from 400 to 3192. No register write is added.
> 
> Fixes: be1589e567ae ("media: i2c: imx471: Add Sony IMX471 image sensor driver")
> Link: https://lore.kernel.org/linux-media/20260728042013.23707-1-hpa@redhat.com/
> Assisted-by: Claude-Code:claude-fable-5
> Signed-off-by: Christian Murphy <christian@themurphys.eu>
> ---
> 
> Notes:
>     LINE_LENGTH_PCK reads back 0x1400 (5120) from the streaming sensor.

Interestingly, 2328 would appear to be a seemigly valid value.

Cc Antti.

>     Written to 5632, 6144, 5008 and 5120 in one stream, the line period is
>     llp / 387.2 MHz at every value (14.5457, 15.8680, 12.9341, 13.2234 us).
>     
>     Tested on a ThinkPad X1 Carbon Gen 14 (Debian linux 7.1.8-2, libcamera
>     0.7.2): controls read back 387200000 / 3192, 17.296 ms frames, maximum
>     ExposureTime 17.057 ms. v4l2-compliance 46/46; W=1 and sparse clean.
>     The driver is in no release (7.3 merge window), so no Cc: stable.
> 
>  drivers/media/i2c/imx471.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> index 4053aed84340..7a1335873270 100644
> --- a/drivers/media/i2c/imx471.c
> +++ b/drivers/media/i2c/imx471.c
> @@ -282,7 +282,7 @@ static const struct imx471_mode imx471_modes[] = {
>  		.height = 1088,
>  		.fll_def = 1308,
>  		.fll_min = 1308,
> -		.llp = 2328,
> +		.llp = 5120,
>  		.default_mode_regs = mode_1928x1088_regs,
>  		.default_mode_regs_length = ARRAY_SIZE(mode_1928x1088_regs),
>  	},
> @@ -691,8 +691,8 @@ static int imx471_init_controls(struct imx471 *sensor)
>  					   0,
>  					   link_freq_menu_items);
>  
> -	/* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> -	pixel_rate = div_u64(IMX471_LINK_FREQ_DEFAULT * 2 * 4, 10);
> +	/* pixel_rate = 2 * vt_pix_clk, with vt_pix_clk = 19.2 MHz / 2 * 121 / 6 */
> +	pixel_rate = 387200000;

Can you calculate the value instead, please?

>  
>  	v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
>  			  V4L2_CID_PIXEL_RATE, pixel_rate,
> 

-- 
Kind regards,

Sakari Ailus