[PATCH] media: i2c: imx471: Add line length PCK setting

[PATCH] 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 for
controlling the frame rate.

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 6d358b11e96d..e661f5b78dcc 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, 5008 },
 	{ IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
 	{ CCI_REG8(0x3f4c), 0x81 },
 	{ CCI_REG8(0x3f4d), 0x81 },
-- 
2.55.0

Re: [PATCH] media: i2c: imx471: Add line length PCK setting

[Jai Luthra]

Hi Kate,

Thank you for the patch.

Quoting Kate Hsuan (2026-07-28 09:50:13)
> Add the line length PCK setting to the IMX471 driver. This parameter
> determines the horizontal line scan duration and is used for
> controlling the frame rate.
> 
> 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 6d358b11e96d..e661f5b78dcc 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, 5008 },

This does not match the mode->llp value (2328) used for 1928x1088, any idea
why?

Aside from the patch, I also see that the value for the PIXEL_RATE control
is derived using the link-frequency and output bit-depth.

It is likely that the sensor's frame rate depends solely on the LLP and FLL
timing registers, so it should be possible to derive a constant pixel rate
value (for a given PLL configuration) that is independent of link-freq and
bit-depth.

Thanks,
    Jai

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

Re: [PATCH] media: i2c: imx471: Add line length PCK setting

[Kate Hsuan]

Hi Jai,

Thank you for the review.

On Tue, Jul 28, 2026 at 1:27 PM Jai Luthra <jai.luthra@ideasonboard.com> wrote:
>
> Hi Kate,
>
> Thank you for the patch.
>
> Quoting Kate Hsuan (2026-07-28 09:50:13)
> > Add the line length PCK setting to the IMX471 driver. This parameter
> > determines the horizontal line scan duration and is used for
> > controlling the frame rate.
> >
> > 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 6d358b11e96d..e661f5b78dcc 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, 5008 },
>
> This does not match the mode->llp value (2328) used for 1928x1088, any idea
> why?

I tried to resolve the horizontal line pattern on the moving objects
when the sensor runs with libcamera. (The image is better for 0.7.2
and more horizontal noise for 0.7.1)
https://bugzilla.redhat.com/show_bug.cgi?id=2502786
I guess the horizontal line pattern can be mitigated by reducing the
frame rate. (image is better but the issue still happens.) I'm trying
to figure out if the root cause is in the driver or SoftISP.
If this is not the right direction for the solution, this patch can be
drop. I'll try another way to resolve it.

>
> Aside from the patch, I also see that the value for the PIXEL_RATE control
> is derived using the link-frequency and output bit-depth.
>
> It is likely that the sensor's frame rate depends solely on the LLP and FLL
> timing registers, so it should be possible to derive a constant pixel rate
> value (for a given PLL configuration) that is independent of link-freq and
> bit-depth.
The datasheet mentioned the the link frequency determines the pixel
rate so that is why pixel rate is based on link frequency.



>
> Thanks,
>     Jai
>
> >         { IMX471_REG_PLL_MULT_DRIV, IMX471_PLL_DUAL },
> >         { CCI_REG8(0x3f4c), 0x81 },
> >         { CCI_REG8(0x3f4d), 0x81 },
> > --
> > 2.55.0
> >
> >
>


-- 
BR,
Kate

Re: [PATCH] media: i2c: imx471: Add line length PCK setting

[Sakari Ailus]

Hi Kate,

On Tue, Jul 28, 2026 at 02:54:49PM +0800, Kate Hsuan wrote:
> Hi Jai,
> 
> Thank you for the review.
> 
> On Tue, Jul 28, 2026 at 1:27 PM Jai Luthra <jai.luthra@ideasonboard.com> wrote:
> >
> > Hi Kate,
> >
> > Thank you for the patch.
> >
> > Quoting Kate Hsuan (2026-07-28 09:50:13)
> > > Add the line length PCK setting to the IMX471 driver. This parameter
> > > determines the horizontal line scan duration and is used for
> > > controlling the frame rate.
> > >
> > > 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 6d358b11e96d..e661f5b78dcc 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, 5008 },
> >
> > This does not match the mode->llp value (2328) used for 1928x1088, any idea
> > why?
> 
> I tried to resolve the horizontal line pattern on the moving objects
> when the sensor runs with libcamera. (The image is better for 0.7.2
> and more horizontal noise for 0.7.1)
> https://bugzilla.redhat.com/show_bug.cgi?id=2502786
> I guess the horizontal line pattern can be mitigated by reducing the
> frame rate. (image is better but the issue still happens.) I'm trying
> to figure out if the root cause is in the driver or SoftISP.
> If this is not the right direction for the solution, this patch can be
> drop. I'll try another way to resolve it.

My guess is that there's something wrong with the sensor configuration if
there are obvious systematic image defects that don't exist on other
sensor, given the same SoftISP. Something to check indeed is the horizontal
blanking as you've already found out. Very likely the other parameters such
as vertical blanking could still be changed without affecting the issue.

> 
> >
> > Aside from the patch, I also see that the value for the PIXEL_RATE control
> > is derived using the link-frequency and output bit-depth.
> >
> > It is likely that the sensor's frame rate depends solely on the LLP and FLL
> > timing registers, so it should be possible to derive a constant pixel rate
> > value (for a given PLL configuration) that is independent of link-freq and
> > bit-depth.
> The datasheet mentioned the the link frequency determines the pixel
> rate so that is why pixel rate is based on link frequency.

The PIXEL_RATE control refers to the pixel read rate in the pixel array and
configurations where pixel rate on the CSI-2 bus differs from that are
possible on all but the most trivial sensors (typically VGA).

In this case the PLL configuration could be used to find the actual
PIXEL_RATE (on the pixel array) as Jai mentioned.

-- 
Regards,

Sakari Ailus

Re: [PATCH] media: i2c: imx471: Add line length PCK setting

[Kate Hsuan]

Hi Sakari,

On Tue, Jul 28, 2026 at 3:51 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Kate,
>
> On Tue, Jul 28, 2026 at 02:54:49PM +0800, Kate Hsuan wrote:
> > Hi Jai,
> >
> > Thank you for the review.
> >
> > On Tue, Jul 28, 2026 at 1:27 PM Jai Luthra <jai.luthra@ideasonboard.com> wrote:
> > >
> > > Hi Kate,
> > >
> > > Thank you for the patch.
> > >
> > > Quoting Kate Hsuan (2026-07-28 09:50:13)
> > > > Add the line length PCK setting to the IMX471 driver. This parameter
> > > > determines the horizontal line scan duration and is used for
> > > > controlling the frame rate.
> > > >
> > > > 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 6d358b11e96d..e661f5b78dcc 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, 5008 },
> > >
> > > This does not match the mode->llp value (2328) used for 1928x1088, any idea
> > > why?
> >
> > I tried to resolve the horizontal line pattern on the moving objects
> > when the sensor runs with libcamera. (The image is better for 0.7.2
> > and more horizontal noise for 0.7.1)
> > https://bugzilla.redhat.com/show_bug.cgi?id=2502786
> > I guess the horizontal line pattern can be mitigated by reducing the
> > frame rate. (image is better but the issue still happens.) I'm trying
> > to figure out if the root cause is in the driver or SoftISP.
> > If this is not the right direction for the solution, this patch can be
> > drop. I'll try another way to resolve it.
>
> My guess is that there's something wrong with the sensor configuration if
> there are obvious systematic image defects that don't exist on other
> sensor, given the same SoftISP. Something to check indeed is the horizontal
> blanking as you've already found out. Very likely the other parameters such
> as vertical blanking could still be changed without affecting the issue.

Good suggestion. I'll try to tune vblank and possible configs and hope it works.
I scratched my head trying to find the solutions and now I can focus
on tuning the parameter to fix the issue.

>
> >
> > >
> > > Aside from the patch, I also see that the value for the PIXEL_RATE control
> > > is derived using the link-frequency and output bit-depth.
> > >
> > > It is likely that the sensor's frame rate depends solely on the LLP and FLL
> > > timing registers, so it should be possible to derive a constant pixel rate
> > > value (for a given PLL configuration) that is independent of link-freq and
> > > bit-depth.
> > The datasheet mentioned the the link frequency determines the pixel
> > rate so that is why pixel rate is based on link frequency.
>
> The PIXEL_RATE control refers to the pixel read rate in the pixel array and
> configurations where pixel rate on the CSI-2 bus differs from that are
> possible on all but the most trivial sensors (typically VGA).
>
> In this case the PLL configuration could be used to find the actual
> PIXEL_RATE (on the pixel array) as Jai mentioned.

Got it. I'll try to find a pixel rate based on the PLL settings.
Perhaps that can resolve the issue.

Thank you :)

>
> --
> Regards,
>
> Sakari Ailus
>


-- 
BR,
Kate

Re: [PATCH] media: i2c: imx471: Add line length PCK setting

[Sakari Ailus]

Hi Kate,

On Tue, Jul 28, 2026 at 04:33:38PM +0800, Kate Hsuan wrote:
> Good suggestion. I'll try to tune vblank and possible configs and hope it works.
> I scratched my head trying to find the solutions and now I can focus
> on tuning the parameter to fix the issue.

Horizontal blanking is likely to affect image defects whereas vertical
blanking can be used to adjust the frame rate. If changing the horizontal
blanking does have that effect, we should probably think of using a fixed
value for it. Then the question is what is that value and what does it
depend on (likely PLL configuration and external clock frequency, possibly
binning etc.).

-- 
Regards,

Sakari Ailus