[PATCH v2 0/3] media: i2c: imx471: Add line length PCK setting and calculate pixel rate based on external clock rate

[PATCH v2 0/3] media: i2c: imx471: Add line length PCK setting and calculate pixel rate based on external clock rate

[Kate Hsuan]

This patchset adds the line length PCK setting and calculates the pixel
rate based on the external clock rate.

Changes in v2:
The patchset contains three patches:
1. Add line length PCK setting
2. Name the PLL registers in the OP domain
3. Calculate pixel rate based on the external clock rate and the PLL
   configurations in the OP domain.

The line length PCK setting is added to the sensor driver to ensure the
pixel rate is correct. The pixel rate is calculated based on the external
clock rate. Moreover, since the sensor runs in the PLL DUAL mode, the
configurations of the OP domain are considered.

Tested with libcamera 0.7.2 and the patchset mitigated the horizontal
line noise.

Changes in v1:
1. Add line length PCK setting

Kate Hsuan (3):
  media: i2c: imx471: Add line length PCK setting
  media: i2c: imx471: Name the PLL registers in the OP domain
  media: i2c: imx471: Calculate pixel rate based on external clock rate

 drivers/media/i2c/imx471.c | 39 ++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] media: i2c: imx471: Add line length PCK setting

[Kate Hsuan]

Add the line length PCK setting to the IMX471 driver. This parameter
determines the horizontal line scan duration and is used to calculate
and control the sensor frame rate.
Set the line length PCK value to 0x0a00 (matching the original value
in ipu6-drivers), which maintains a frame rate close to the datasheet
spec (~150 FPS in 2328x1304 binning mode).

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/media/i2c/imx471.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
index 4053aed84340..3c939f2a9056 100644
--- a/drivers/media/i2c/imx471.c
+++ b/drivers/media/i2c/imx471.c
@@ -91,6 +91,7 @@
 #define IMX471_CSI_2_LANE_MODE			1
 #define IMX471_CSI_4_LANE_MODE			3
 
+#define IMX471_REG_LINE_LENGTH_PCK		CCI_REG16(0x0342)
 #define IMX471_REG_X_ADD_STA			CCI_REG16(0x0344)
 #define IMX471_REG_Y_ADD_STA			CCI_REG16(0x0346)
 #define IMX471_REG_X_ADD_END			CCI_REG16(0x0348)
@@ -240,6 +241,7 @@ static const struct cci_reg_sequence mode_1928x1088_regs[] = {
 	{ CCI_REG8(0x030d), 0x02 },
 	{ CCI_REG8(0x030e), 0x00 },
 	{ CCI_REG8(0x030f), 0x53 },
+	{ IMX471_REG_LINE_LENGTH_PCK, 2560 },
 	{ IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
 	{ CCI_REG8(0x3f4c), 0x81 },
 	{ CCI_REG8(0x3f4d), 0x81 },
-- 
2.55.0

Re: [PATCH v2 1/3] media: i2c: imx471: Add line length PCK setting

[Sakari Ailus]

Hi Kate,

On Fri, Sep 04, 2026 at 03:51:11PM +0800, Kate Hsuan wrote:
> Add the line length PCK setting to the IMX471 driver. This parameter
> determines the horizontal line scan duration and is used to calculate
> and control the sensor frame rate.
> Set the line length PCK value to 0x0a00 (matching the original value
> in ipu6-drivers), which maintains a frame rate close to the datasheet
> spec (~150 FPS in 2328x1304 binning mode).

This fits on fewer lines, please rewrap.

> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
>  drivers/media/i2c/imx471.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> index 4053aed84340..3c939f2a9056 100644
> --- a/drivers/media/i2c/imx471.c
> +++ b/drivers/media/i2c/imx471.c
> @@ -91,6 +91,7 @@
>  #define IMX471_CSI_2_LANE_MODE			1
>  #define IMX471_CSI_4_LANE_MODE			3
>  
> +#define IMX471_REG_LINE_LENGTH_PCK		CCI_REG16(0x0342)
>  #define IMX471_REG_X_ADD_STA			CCI_REG16(0x0344)
>  #define IMX471_REG_Y_ADD_STA			CCI_REG16(0x0346)
>  #define IMX471_REG_X_ADD_END			CCI_REG16(0x0348)
> @@ -240,6 +241,7 @@ static const struct cci_reg_sequence mode_1928x1088_regs[] = {
>  	{ CCI_REG8(0x030d), 0x02 },
>  	{ CCI_REG8(0x030e), 0x00 },
>  	{ CCI_REG8(0x030f), 0x53 },
> +	{ IMX471_REG_LINE_LENGTH_PCK, 2560 },

The register definition is nice but there's already a control for this.
Could you do this via s_ctrl()?

There's also another set addressing related issues here
<URL:https://lore.kernel.org/linux-media/20260904075113.125751-1-hpa@redhat.com/T/#t>.

>  	{ IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
>  	{ CCI_REG8(0x3f4c), 0x81 },
>  	{ CCI_REG8(0x3f4d), 0x81 },

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 1/3] media: i2c: imx471: Add line length PCK setting

[Sakari Ailus]

On Fri, Sep 04, 2026 at 11:18:16AM +0300, Sakari Ailus wrote:
> Hi Kate,
> 
> On Fri, Sep 04, 2026 at 03:51:11PM +0800, Kate Hsuan wrote:
> > Add the line length PCK setting to the IMX471 driver. This parameter
> > determines the horizontal line scan duration and is used to calculate
> > and control the sensor frame rate.
> > Set the line length PCK value to 0x0a00 (matching the original value
> > in ipu6-drivers), which maintains a frame rate close to the datasheet
> > spec (~150 FPS in 2328x1304 binning mode).
> 
> This fits on fewer lines, please rewrap.
> 
> > 
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> >  drivers/media/i2c/imx471.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> > index 4053aed84340..3c939f2a9056 100644
> > --- a/drivers/media/i2c/imx471.c
> > +++ b/drivers/media/i2c/imx471.c
> > @@ -91,6 +91,7 @@
> >  #define IMX471_CSI_2_LANE_MODE			1
> >  #define IMX471_CSI_4_LANE_MODE			3
> >  
> > +#define IMX471_REG_LINE_LENGTH_PCK		CCI_REG16(0x0342)
> >  #define IMX471_REG_X_ADD_STA			CCI_REG16(0x0344)
> >  #define IMX471_REG_Y_ADD_STA			CCI_REG16(0x0346)
> >  #define IMX471_REG_X_ADD_END			CCI_REG16(0x0348)
> > @@ -240,6 +241,7 @@ static const struct cci_reg_sequence mode_1928x1088_regs[] = {
> >  	{ CCI_REG8(0x030d), 0x02 },
> >  	{ CCI_REG8(0x030e), 0x00 },
> >  	{ CCI_REG8(0x030f), 0x53 },
> > +	{ IMX471_REG_LINE_LENGTH_PCK, 2560 },
> 
> The register definition is nice but there's already a control for this.
> Could you do this via s_ctrl()?
> 
> There's also another set addressing related issues here
> <URL:https://lore.kernel.org/linux-media/20260904075113.125751-1-hpa@redhat.com/T/#t>.

Wrong URL, I meant this one
<URL:https://patchwork.linuxtv.org/project/linux-media/patch/ede568ada4304d2dc92cd43986fc07aa68628154.1788377269.git.christian@themurphys.eu/>.

-- 
Sakari Ailus

Re: [PATCH v2 1/3] media: i2c: imx471: Add line length PCK setting

[Kate Hsuan]

Hi Sakari,

Thank you for reviewing this.

On Fri, Sep 4, 2026 at 4:32 PM Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> On Fri, Sep 04, 2026 at 11:18:16AM +0300, Sakari Ailus wrote:
> > Hi Kate,
> >
> > On Fri, Sep 04, 2026 at 03:51:11PM +0800, Kate Hsuan wrote:
> > > Add the line length PCK setting to the IMX471 driver. This parameter
> > > determines the horizontal line scan duration and is used to calculate
> > > and control the sensor frame rate.
> > > Set the line length PCK value to 0x0a00 (matching the original value
> > > in ipu6-drivers), which maintains a frame rate close to the datasheet
> > > spec (~150 FPS in 2328x1304 binning mode).
> >
> > This fits on fewer lines, please rewrap.
Okay

> >
> > >
> > > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > > ---
> > >  drivers/media/i2c/imx471.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
> > > index 4053aed84340..3c939f2a9056 100644
> > > --- a/drivers/media/i2c/imx471.c
> > > +++ b/drivers/media/i2c/imx471.c
> > > @@ -91,6 +91,7 @@
> > >  #define IMX471_CSI_2_LANE_MODE                     1
> > >  #define IMX471_CSI_4_LANE_MODE                     3
> > >
> > > +#define IMX471_REG_LINE_LENGTH_PCK         CCI_REG16(0x0342)
> > >  #define IMX471_REG_X_ADD_STA                       CCI_REG16(0x0344)
> > >  #define IMX471_REG_Y_ADD_STA                       CCI_REG16(0x0346)
> > >  #define IMX471_REG_X_ADD_END                       CCI_REG16(0x0348)
> > > @@ -240,6 +241,7 @@ static const struct cci_reg_sequence mode_1928x1088_regs[] = {
> > >     { CCI_REG8(0x030d), 0x02 },
> > >     { CCI_REG8(0x030e), 0x00 },
> > >     { CCI_REG8(0x030f), 0x53 },
> > > +   { IMX471_REG_LINE_LENGTH_PCK, 2560 },
> >
> > The register definition is nice but there's already a control for this.
> > Could you do this via s_ctrl()?
okay. I'll update the value when set V4L2_CID_HBLANK.

The original driver in the ipu6-drivers sets this register with 2560
and I found that a larger value slows down the fps. I try to make the
fps close to the datasheet.

> >
> > There's also another set addressing related issues here
> > <URL:https://lore.kernel.org/linux-media/20260904075113.125751-1-hpa@redhat.com/T/#t>.
>
> Wrong URL, I meant this one
> <URL:https://patchwork.linuxtv.org/project/linux-media/patch/ede568ada4304d2dc92cd43986fc07aa68628154.1788377269.git.christian@themurphys.eu/>.
It's ok :)
Ah yes, I noticed that. We can discuss the pixel rate there.

>
> --
> Sakari Ailus
>


-- 
BR,
Kate

[PATCH v2 2/3] media: i2c: imx471: Name the PLL registers in the OP domain

[Kate Hsuan]

Define symbolic names for the registers in the OP domain, including
IMX471_REG_OP_PREPLLCK_DIV and IMX471_REG_OP_PLL_MULTIPLIER.Replaces
raw register addresses with meaningful macros to improve code readability.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/media/i2c/imx471.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
index 3c939f2a9056..316383076b3d 100644
--- a/drivers/media/i2c/imx471.c
+++ b/drivers/media/i2c/imx471.c
@@ -71,8 +71,17 @@
 #define IMX471_REG_OPPXCK_DIV			CCI_REG8(0x0309)
 #define IMX471_REG_OPSYCK_DIV			CCI_REG8(0x030b)
 #define IMX471_REG_PLL_MULT_DRIV		CCI_REG8(0x0310)
+#define IMX471_REG_OP_PREPLLCK_DIV		CCI_REG16(0x030c)
+#define IMX471_REG_OP_MPY			CCI_REG16(0x030e)
 #define IMX471_PLL_SINGLE			0
 #define IMX471_PLL_DUAL				1
+#define IMX471_VTPXCK_DIV			6
+#define IMX471_VTSYCK_DIV			2
+#define IMX471_PREPLLCK_VT_DIV			2
+#define IMX471_PLL_VT_MPY			121
+#define IMX471_OPSYCK_DIV			1
+#define IMX471_PLL_OP_MPY			83
+#define IMX471_PREPLLCK_OP_DIV			2
 
 /* IMX471 native and active pixel array size */
 #define IMX471_NATIVE_WIDTH			4672
@@ -233,14 +242,13 @@ static const struct cci_reg_sequence mode_1928x1088_regs[] = {
 	{ IMX471_REG_DIG_CROP_HEIGHT, 1088 },
 	{ IMX471_REG_X_OUTPUT_SIZE, 1928 },
 	{ IMX471_REG_Y_OUTPUT_SIZE, 1088 },
-	{ IMX471_REG_VTPXCK_DIV, 0x06 },
-	{ IMX471_REG_VTSYCK_DIV, 0x02 },
-	{ IMX471_REG_PREPLLCK_VT_DIV, 0x02 },
-	{ IMX471_REG_PLL_VT_MPY, 0x0079 },
-	{ IMX471_REG_OPSYCK_DIV, 0x01 },
-	{ CCI_REG8(0x030d), 0x02 },
-	{ CCI_REG8(0x030e), 0x00 },
-	{ CCI_REG8(0x030f), 0x53 },
+	{ IMX471_REG_VTPXCK_DIV, IMX471_VTPXCK_DIV },
+	{ IMX471_REG_VTSYCK_DIV, IMX471_VTSYCK_DIV },
+	{ IMX471_REG_PREPLLCK_VT_DIV, IMX471_PREPLLCK_VT_DIV },
+	{ IMX471_REG_PLL_VT_MPY, IMX471_PLL_VT_MPY },
+	{ IMX471_REG_OPSYCK_DIV, IMX471_OPSYCK_DIV },
+	{ IMX471_REG_OP_PREPLLCK_DIV, IMX471_PREPLLCK_OP_DIV },
+	{ IMX471_REG_OP_MPY, IMX471_PLL_OP_MPY },
 	{ IMX471_REG_LINE_LENGTH_PCK, 2560 },
 	{ IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
 	{ CCI_REG8(0x3f4c), 0x81 },
-- 
2.55.0

[PATCH v2 3/3] media: i2c: imx471: Calculate pixel rate based on external clock rate

[Kate Hsuan]

The IMX471 sensor operates in PLL Dual Mode, so OP domain configurations
must be used to determine the pixel rate. Based on the IMX471 clock tree
topology, the pixel rate is calculated as:

(extclk * IOP_PLL_MPY * DDR_FACTOR) / (IOP_PREPLLCK_DIV * IOP_SYNCK_DIV * BPP)

With the register values, this yields a pixel rate of 159.36 MPix/s.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/media/i2c/imx471.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/imx471.c b/drivers/media/i2c/imx471.c
index 316383076b3d..d81d64dd8761 100644
--- a/drivers/media/i2c/imx471.c
+++ b/drivers/media/i2c/imx471.c
@@ -701,8 +701,17 @@ 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);
+	/*
+	 * The sensor runs in the PLL DUAL mode so the configurations of the
+	 * OP domain are considered. According to the imx471 clock tree,
+	 * the pixel rate can be calculated as follows:
+	 * (exck_freq * IOP_PLL_MPY * DDR factor) /
+	 * (IOP_PREPLLCK_DIV * IOP_SYNCK_DIV * RAW10 bits) = 159.36 MPix/s
+	 */
+	pixel_rate = (IMX471_EXT_CLK * IMX471_PLL_OP_MPY * (u64)2) /
+		     (IMX471_PREPLLCK_OP_DIV * IMX471_OPSYCK_DIV * (u64)10);
+
+	dev_dbg(sensor->dev, "pixel_rate: %llu\n", pixel_rate);
 
 	v4l2_ctrl_new_std(ctrl_hdlr, &imx471_ctrl_ops,
 			  V4L2_CID_PIXEL_RATE, pixel_rate,
-- 
2.55.0